On Wed, 2015-02-04 at 17:50 +0200, Mika Westerberg wrote: > On Tue, Feb 03, 2015 at 08:18:54PM +0200, Andy Shevchenko wrote: > > Since clk_register_clkdev() is exported for modules the caller should get a > > pointer to the allocated resources. Otherwise the memory leak is guaranteed > > on > > the ->remove() stage. > > > > Cc: Tomeu Vizoso <[email protected]> > > Signed-off-by: Andy Shevchenko <[email protected]> > > One comment, see below. Nothing major so feel free to add, > > Reviewed-by: Mika Westerberg <[email protected]>
Thanks for review! Though still wait for others to comment and test if possible. By the way, I have to add that we have one unpublished (yet) user of this. So, we are testing this internally on x86. Just to note that the user is a module which might be unloaded. That's why important to release the acquired resources. > > > --- > > arch/arm/mach-msm/clock-pcom.c | 9 +++++---- > > arch/arm/mach-vexpress/spc.c | 5 ++++- > > arch/mips/ath79/clock.c | 6 +++--- > > drivers/clk/clk-bcm2835.c | 12 +++++++----- > > drivers/clk/clk-max-gen.c | 9 ++++----- > > drivers/clk/clk-xgene.c | 6 +++--- > > drivers/clk/clkdev.c | 14 +++++++++----- > > drivers/clk/samsung/clk-pll.c | 13 ++++++++----- > > drivers/clk/samsung/clk-s3c2410-dclk.c | 19 +++++++++--------- > > drivers/clk/samsung/clk.c | 35 > > +++++++++++++++++++--------------- > > include/linux/clkdev.h | 2 +- > > 11 files changed, 74 insertions(+), 56 deletions(-) > > ... > > > index 6e5c504..e07b1e2 100644 > > --- a/drivers/clk/clkdev.c > > +++ b/drivers/clk/clkdev.c > > @@ -307,29 +307,33 @@ EXPORT_SYMBOL(clkdev_drop); > > * clkdev. > > * > > * To make things easier for mass registration, we detect error clks > > - * from a previous clk_register() call, and return the error code for > > + * from a previous clk_register() call, and return the error pointer for > > * those. This is to permit this function to be called immediately > > * after clk_register(). > > + * > > + * Return: > > + * pointer to the allocated struct clk_lookup on success, or error pointer > > + * otherwise. > > This should probably say how these resources are supposed to be > released. Agree. I would add this in v2. -- Andy Shevchenko <[email protected]> Intel Finland Oy -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
