On Sat, Dec 17, 2016 at 05:04:46PM +0100, Sebastian Reichel wrote:

Hi,

On Sat, Dec 10, 2016 at 09:56:27PM +0200, Krzysztof Kozlowski wrote:

quoted

On Sat, Dec 10, 2016 at 07:49:25PM +0530, Srikant Ritolia wrote:

quoted

On Sat, Dec 10, 2016 at 1:18 PM, Krzysztof Kozlowski [off-list ref] wrote:

quoted

On Fri, Dec 09, 2016 at 02:21:38PM +0530, Srikant Ritolia wrote:

 >

quoted

quoted

quoted

                                              &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.

By broken things I meant possible errors introduced with devm
conversion (like that one spotted above). To me personally, converting
to devm on its own mostly does not bring benefits except few cases when
a lot of code disappears. On the other hand it hides the order of
cleanup making it slightly more difficult to review. Overall - not many
benefits, some things hidden.

However I understand that this is highly subjective so I am not against
if others like this approach.

Best regards,
Krzysztof