Re: [PATCH] power: max77693_charger: Better sysfs creation and using devm APIs

[Sebastian Reichel <sre@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 1362 bytes --]

Hi,

On Sat, Dec 10, 2016 at 09:56:27PM +0200, Krzysztof Kozlowski wrote:
> On Sat, Dec 10, 2016 at 07:49:25PM +0530, Srikant Ritolia wrote:
> > On Sat, Dec 10, 2016 at 1:18 PM, Krzysztof Kozlowski <krzk@kernel.org> wrote:
> > > On Fri, Dec 09, 2016 at 02:21:38PM +0530, Srikant Ritolia wrote:
>  >
> > >>                                               &max77693_charger_desc,
> > >>                                               &psy_cfg);
> > >>       if (IS_ERR(chg->charger)) {
> > >>               dev_err(&pdev->dev, "failed: power supply register\n");
> > >>               ret = PTR_ERR(chg->charger);
> > >> -             goto err;
> > >
> > > Missing sysfs cleanup.
> > >
> > > Best regards,
> > > Krzysztof
> > >
> > 
> > Thanks for pointing this out.
> > To overcome this I will use sysfs_create_group after devm_power_supply_register.
> > Then I would not need to do this sysfs cleanup on failure of power
> > supply register.
> 
> I am not sure if this is good idea. This patch does not bring any
> particular noticeable benefit except less lines of code. It is not worth
> breaking things just for that reason...

I like less lines of code. How does the changed registration order
break anything? The changed registration order makes sense anyways,
since it then matches the (reversed) removal order.

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]