Messages in this thread | | | Subject | Re: [PATCH v1] clkdev: change prototype of clk_register_clkdev() | From | Andy Shevchenko <> | Date | Wed, 04 Feb 2015 18:03:55 +0200 |
| |
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 <tomeu.vizoso@collabora.com> > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > One comment, see below. Nothing major so feel free to add, > > Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
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 <andriy.shevchenko@intel.com> Intel Finland Oy
| |