Messages in this thread | | | Date | Mon, 22 May 2023 19:36:47 +0200 | Subject | Re: [PATCH v16 1/1] clk: npcm8xx: add clock controller | From | Christophe JAILLET <> |
| |
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)
CJ
| |