Re: [PATCH net-next] smsc911x: propagate reset GPIO errors

[Bui Duc Phuc]

Hi Paolo,

Thanks for looking at this.

> > - [Medium] net: smsc911x: clock reference leak when regulator_bulk_get()
> >    fails
>
> The above is IMHO worth addressing.
>

Agreed, the leak is real: the regulator error is saved and returned
after clk_get()
has taken a reference, and smsc911x_drv_probe() skips
smsc911x_free_resources()  on that path.

Since probe fails whenever ret is set anyway, the "try to continue"
comment is stale.

I plan to respin as a 2-patch series:

Patch 1:  returns early when regulator_bulk_get() fails, so no clock
reference is held on any error return:
--------------------------------------------------------------------------------------
ret = regulator_bulk_get(&pdev->dev,
                                        ARRAY_SIZE(pdata->supplies),
                                       pdata->supplies);
- if (ret) {
- /*
- * Retry on deferrals, else just report the error
- * and try to continue.
- */
- if (ret == -EPROBE_DEFER)
-         return ret;
- netdev_err(ndev, "couldn't get regulators %d\n",
-                    ret);
- }
+ if (ret)
+ return dev_err_probe(&pdev->dev, ret,
+                                     "couldn't get regulators\n");
...
- return ret;
+ return 0;
-------------------------------------------------------------------------------------------


Patch 2: is the reset GPIO change from v1, rebased on top of patch 1.
The "if (!ret)" check is no longer needed since the regulators are
always held at that point:

--------------------------------------------------------------------------------------------
pdata->reset_gpiod = devm_gpiod_get_optional(&pdev->dev,

       "reset",

       GPIOD_OUT_LOW);
+ if (IS_ERR(pdata->reset_gpiod)) {
+ regulator_bulk_free(ARRAY_SIZE(pdata->supplies),
+                                  pdata->supplies);
+ return dev_err_probe(&pdev->dev, PTR_ERR(pdata->reset_gpiod),
+                                    "failed to get reset gpio\n");
+ }
---------------------------------------------------------------------------------------------

Does this look OK to you?

Also, as the review suggests, both patches fix existing bugs, so I'd
like to add Fixes: tags and target net instead of net-next for v2.
Would that be fine, or would you prefer to keep this in net-next?

Best regards,
Phuc