[PATCH] drivers: mfd: tps65912: Convert to use devm_kzalloc

[PATCH] drivers: mfd: tps65912: Convert to use devm_kzalloc

[Leon Romanovsky]

There is no need to call kfree while using devm_kzalloc,
remove it from *_device_init function.

Signed-off-by: Leon Romanovsky <leon@leon.nu>
---
 drivers/mfd/tps65912-core.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/mfd/tps65912-core.c b/drivers/mfd/tps65912-core.c
index aeb8e40..f25e552 100644
--- a/drivers/mfd/tps65912-core.c
+++ b/drivers/mfd/tps65912-core.c
@@ -127,7 +127,8 @@ int tps65912_device_init(struct tps65912 *tps65912)
 	struct tps65912_platform_data *init_data;
 	int ret, dcdc_avs, value;
 
-	init_data = kzalloc(sizeof(struct tps65912_platform_data), GFP_KERNEL);
+	init_data = devm_kzalloc(&tps65912->dev,
+			sizeof(struct tps65912_platform_data), GFP_KERNEL);
 	if (init_data == NULL)
 		return -ENOMEM;
 
@@ -156,11 +157,9 @@ int tps65912_device_init(struct tps65912 *tps65912)
 	if (ret < 0)
 		goto err;
 
-	kfree(init_data);
 	return ret;
 
 err:
-	kfree(init_data);
 	mfd_remove_devices(tps65912->dev);
 	kfree(tps65912);
 	return ret;
-- 
1.8.1.5

Re: [PATCH] drivers: mfd: tps65912: Convert to use devm_kzalloc

[Leon Romanovsky]

Hi Samuel,

On Wed, May 15, 2013 at 12:19 PM, Leon Romanovsky <leon@leon.nu> wrote:
> There is no need to call kfree while using devm_kzalloc,
> remove it from *_device_init function.
>
> Signed-off-by: Leon Romanovsky <leon@leon.nu>
> ---
>  drivers/mfd/tps65912-core.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)

To be honest the whole tps65912 looks strange to me.
For example,  kfree(tps65912) calls all over the code.

tps65912_i2c_probe allocates tps65912 struct, but tps65912_device_exit
releases it.


--
Leon Romanovsky | Independent Linux Consultant
        www.leon.nu | leon@leon.nu

Re: [PATCH] drivers: mfd: tps65912: Convert to use devm_kzalloc

[Lee Jones]

On Wed, 15 May 2013, Leon Romanovsky wrote:

> There is no need to call kfree while using devm_kzalloc,
> remove it from *_device_init function.
> 
> Signed-off-by: Leon Romanovsky <leon@leon.nu>
> ---
>  drivers/mfd/tps65912-core.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)

Applied thanks.

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH] drivers: mfd: tps65912: Convert to use devm_kzalloc

[Lee Jones]

> > There is no need to call kfree while using devm_kzalloc,
> > remove it from *_device_init function.
> >
> > Signed-off-by: Leon Romanovsky <leon@leon.nu>
> > ---
> >  drivers/mfd/tps65912-core.c | 5 ++---
> >  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> To be honest the whole tps65912 looks strange to me.
> For example,  kfree(tps65912) calls all over the code.
> 
> tps65912_i2c_probe allocates tps65912 struct, but tps65912_device_exit
> releases it.

Yes, this is expected.

tps65912_[spi|i2c]_remove() both call tps65912_device_exit() which in
turn frees tps65912. Saves duplicating code.

I'll clean thinks up a little, so this is less confusing.

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH] drivers: mfd: tps65912: Convert to use devm_kzalloc

[Lee Jones]

On Wed, 15 May 2013, Leon Romanovsky wrote:

> There is no need to call kfree while using devm_kzalloc,
> remove it from *_device_init function.
> 
> Signed-off-by: Leon Romanovsky <leon@leon.nu>
> ---
>  drivers/mfd/tps65912-core.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/mfd/tps65912-core.c b/drivers/mfd/tps65912-core.c
> index aeb8e40..f25e552 100644
> --- a/drivers/mfd/tps65912-core.c
> +++ b/drivers/mfd/tps65912-core.c
> @@ -127,7 +127,8 @@ int tps65912_device_init(struct tps65912 *tps65912)
>  	struct tps65912_platform_data *init_data;
>  	int ret, dcdc_avs, value;
>  
> -	init_data = kzalloc(sizeof(struct tps65912_platform_data), GFP_KERNEL);
> +	init_data = devm_kzalloc(&tps65912->dev,
> +			sizeof(struct tps65912_platform_data), GFP_KERNEL);

On second thoughts, have you actually built this?

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH] drivers: mfd: tps65912: Convert to use devm_kzalloc

[Lee Jones]

On Mon, 27 May 2013, Leon Romanovsky wrote:

> On May 23, 2013 11:37 AM, "Lee Jones" <lee.jones@linaro.org> wrote:
> >
> > On Wed, 15 May 2013, Leon Romanovsky wrote:
> >
> > > There is no need to call kfree while using devm_kzalloc,
> > > remove it from *_device_init function.
> > >
> > > Signed-off-by: Leon Romanovsky <leon@leon.nu>
> > > ---
> > >  drivers/mfd/tps65912-core.c | 5 ++---
> > >  1 file changed, 2 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/mfd/tps65912-core.c b/drivers/mfd/tps65912-core.c
> > > index aeb8e40..f25e552 100644
> > > --- a/drivers/mfd/tps65912-core.c
> > > +++ b/drivers/mfd/tps65912-core.c
> > > @@ -127,7 +127,8 @@ int tps65912_device_init(struct tps65912 *tps65912)
> > >       struct tps65912_platform_data *init_data;
> > >       int ret, dcdc_avs, value;
> > >
> > > -     init_data = kzalloc(sizeof(struct tps65912_platform_data),
> GFP_KERNEL);
> > > +     init_data = devm_kzalloc(&tps65912->dev,
> > > +                     sizeof(struct tps65912_platform_data),
> GFP_KERNEL);
> >
> > On second thoughts, have you actually built this?
>
> Sure,  I hope,  that I didn't miss anything.
 
Can you make absolutely sure that you did please?

I mention it because I would expect lots of warnings if you did.

Something like:

drivers/mfd/tps65912-core.c: In function ‘tps65912_reg_write’:
drivers/mfd/tps65912-core.c:117:3: warning: passing argument 1 of ‘dev_err’ from incompatible pointer type [enabled by default]
In file included from include/linux/platform_device.h:14:0,
                 from include/linux/mfd/core.h:17,
                 from drivers/mfd/tps65912-core.c:21:
include/linux/device.h:954:5: note: expected ‘const struct device *’ but argument is of type ‘struct device **’
drivers/mfd/tps65912-core.c: In function ‘tps65912_device_init’:
drivers/mfd/tps65912-core.c:131:18: warning: passing argument 1 of ‘devm_kzalloc’ from incompatible pointer type [enabled by default]
In file included from include/linux/platform_device.h:14:0,
                 from include/linux/mfd/core.h:17,
                 from drivers/mfd/tps65912-core.c:21:
include/linux/device.h:574:14: note: expected ‘struct device *’ but argument is of type ‘struct device **’

Can you check and make sure, because there's some disparity
somewhere.

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH] drivers: mfd: tps65912: Convert to use devm_kzalloc

[Andy Shevchenko]

On Tue, May 28, 2013 at 12:02 PM, Lee Jones <lee.jones@linaro.org> wrote:
> On Mon, 27 May 2013, Leon Romanovsky wrote:

>> > > +     init_data = devm_kzalloc(&tps65912->dev,

There is an obvious typo: no need of & here if I got it correctly.

> drivers/mfd/tps65912-core.c: In function ‘tps65912_device_init’:
> drivers/mfd/tps65912-core.c:131:18: warning: passing argument 1 of ‘devm_kzalloc’ from incompatible pointer type [enabled by default]
> In file included from include/linux/platform_device.h:14:0,
>                  from include/linux/mfd/core.h:17,
>                  from drivers/mfd/tps65912-core.c:21:
> include/linux/device.h:574:14: note: expected ‘struct device *’ but argument is of type ‘struct device **’

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] drivers: mfd: tps65912: Convert to use devm_kzalloc

[Leon Romanovsky]

On Tue, May 28, 2013 at 7:24 PM, Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Tue, May 28, 2013 at 12:02 PM, Lee Jones <lee.jones@linaro.org> wrote:
>> On Mon, 27 May 2013, Leon Romanovsky wrote:
>
>>> > > +     init_data = devm_kzalloc(&tps65912->dev,
>
> There is an obvious typo: no need of & here if I got it correctly.
>
>> drivers/mfd/tps65912-core.c: In function ‘tps65912_device_init’:
>> drivers/mfd/tps65912-core.c:131:18: warning: passing argument 1 of ‘devm_kzalloc’ from incompatible pointer type [enabled by default]
>> In file included from include/linux/platform_device.h:14:0,
>>                  from include/linux/mfd/core.h:17,
>>                  from drivers/mfd/tps65912-core.c:21:
>> include/linux/device.h:574:14: note: expected ‘struct device *’ but argument is of type ‘struct device **’
You are absolutely right, I sent the wrong patch with typo :(

Lee,
Do I need to resend it ? Or you can fix it by yourself ?


>
> --
> With Best Regards,
> Andy Shevchenko



--
Leon Romanovsky | Independent Linux Consultant
        www.leon.nu | leon@leon.nu

Re: [PATCH] drivers: mfd: tps65912: Convert to use devm_kzalloc

[Lee Jones]

On Tue, 28 May 2013, Leon Romanovsky wrote:

> On Tue, May 28, 2013 at 7:24 PM, Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
> > On Tue, May 28, 2013 at 12:02 PM, Lee Jones <lee.jones@linaro.org> wrote:
> >> On Mon, 27 May 2013, Leon Romanovsky wrote:
> >
> >>> > > +     init_data = devm_kzalloc(&tps65912->dev,
> >
> > There is an obvious typo: no need of & here if I got it correctly.
> >
> >> drivers/mfd/tps65912-core.c: In function ‘tps65912_device_init’:
> >> drivers/mfd/tps65912-core.c:131:18: warning: passing argument 1 of ‘devm_kzalloc’ from incompatible pointer type [enabled by default]
> >> In file included from include/linux/platform_device.h:14:0,
> >>                  from include/linux/mfd/core.h:17,
> >>                  from drivers/mfd/tps65912-core.c:21:
> >> include/linux/device.h:574:14: note: expected ‘struct device *’ but argument is of type ‘struct device **’
> You are absolutely right, I sent the wrong patch with typo :(
> 
> Lee,
> Do I need to resend it ? Or you can fix it by yourself ?

I've already fixed it up and tested.

I just wanted to clarify exactly what happened, as it couldn't have
been tested.

No worries, patch applied.

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog