Re: [PATCH] power_supply: Adjust devm usage

Vaishali Thakkar
2015-07-24T13:14:44Z

On Fri, Jul 24, 2015 at 6:29 PM, Sebastian Reichel <sre@kernel.org> wrote:
> Hi,
>
> On Fri, Jul 24, 2015 at 06:03:38PM +0530, Vaishali Thakkar wrote:
>> On Fri, Jul 24, 2015 at 5:56 PM, Sebastian Reichel <sre@kernel.org> wrote:
>> > On Fri, Jul 24, 2015 at 05:28:13PM +0530, Vaishali Thakkar wrote:
>> >> @@ -267,8 +267,9 @@ static int bq24735_charger_probe() {}
>> >> [...]
>> >
>> > Your patch is missing removal of the
>> > kfree(charger->charger_desc.name) in bq24735_charger_remove().
>>
>> Yes. Because it seems that this kfree is freeing some other data which
>> is not related to devm_kzalloc. I was not sure about removing it.
>> So, I was about to discuss it in a separate thread.s
>
> it's assigned in the probe function:
>
> name = kasprintf(...);
> ...
> supply_desc->name = name;
> ...
> power_supply_register(..., supply_desc, ...);
>

Oh. Yes. I missed that.

>> Also, in the remove function we have devm_free_irq. I am unsure it
>> too. Because normally remove functions do not use devm
>> counterparts.
>
> It's required to free the irq before removing the power supply
> device.
>
> If the power supply is registered with devm, that should happen
> automatically, since it is requested before the irq. Thus the
> remove function can be removed completely at that point :)

This makes sense. Thanks for explanation and review :)
So, can I send all changes along with getting rid of remove
function here in a single patch?

> -- Sebastian

--
Vaishali