Messages in this thread | | | From | Tomer Maimon <> | Date | Mon, 29 May 2023 18:52:32 +0300 | Subject | Re: [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
| |