Messages in this thread | | | Date | Wed, 4 Feb 2015 17:50:54 +0200 | From | Mika Westerberg <> | Subject | Re: [PATCH v1] clkdev: change prototype of clk_register_clkdev() |
| |
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>
> --- > 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.
> */ > -int clk_register_clkdev(struct clk *clk, const char *con_id, > +struct clk_lookup *clk_register_clkdev(struct clk *clk, const char *con_id, > const char *dev_fmt, ...) > { > struct clk_lookup *cl; > va_list ap; > > if (IS_ERR(clk)) > - return PTR_ERR(clk); > + return ERR_CAST(clk); > > va_start(ap, dev_fmt); > cl = vclkdev_alloc(clk, con_id, dev_fmt, ap); > va_end(ap); > > if (!cl) > - return -ENOMEM; > + return ERR_PTR(-ENOMEM); > > clkdev_add(cl); > > - return 0; > + return cl; > } > EXPORT_SYMBOL(clk_register_clkdev);
| |