lkml.org 
[lkml]   [2023]   [May]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH v16 1/1] clk: npcm8xx: add clock controller
On Mon, 22 May 2023 at 20:36, Christophe JAILLET
<christophe.jaillet@wanadoo.fr> wrote:
>
> Le 22/05/2023 à 14:56, Tomer Maimon a écrit :
> > Hi Christophe,
> >
> > Thanks for your comments
> >
>
> [...]
>
> >>> +static struct clk_hw *
> >>> +npcm8xx_clk_register_pll(struct device *dev, void __iomem *pllcon,
> >>> + const char *name, const struct clk_parent_data *parent,
> >>> + unsigned long flags)
> >>> +{
> >>> + struct npcm8xx_clk_pll *pll;
> >>> + struct clk_init_data init = {};
> >>> + int ret;
> >>> +
> >>> + pll = kzalloc(sizeof(*pll), GFP_KERNEL);
> >>
> >> Everything looks devm_()'ed in this driver, except this kzalloc.
> >> Except the one below, there is no kfree to free this memory, and no
> >> .remove() function.
> > Also clk_hw_register_divider_parent_data doesn't use devm_
> > about free the pll, we use it, return at the end of the function.
> > about adding remove, we had a dissection about it in V4, since the
> > clock is a service driver it shouldn't be removed.
> > https://patchwork.kernel.org/project/linux-watchdog/patch/20220621131424.162355-7-tmaimon77@gmail.com/
>
> LoL.
> At least, I'm consistent :).
>
> Just to make it clear, what I mean about kfree() is not to add one here,
> but either:
> - to use devm_kzalloc() here, to avoid a leak, should loading the
> driver fails OR
> - have some kfree() where needed (at least in the error handling
> path of the probe, if the remove function makes no point)
O.K. Thanks for your clarification.
>
> CJ

Best regards,

Tomer

\
 
 \ /
  Last update: 2023-05-29 17:53    [W:0.064 / U:0.872 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site