Hello Gregory CLEMENT,
The patch d445913ce0ab: "usb: ehci-orion: add optional PHY support"
from May 15, 2014, leads to the following static checker warning:
drivers/usb/host/ehci-orion.c:275 ehci_orion_drv_probe()
warn: 'priv->phy' isn't an ERR_PTR
drivers/usb/host/ehci-orion.c
226 priv->phy = devm_phy_optional_get(&pdev->dev, "usb");
227 if (IS_ERR(priv->phy)) {
228 err = PTR_ERR(priv->phy);
229 goto err_phy_get;
230 } else {
231 err = phy_init(priv->phy);
232 if (err)
233 goto err_phy_init;
234
235 err = phy_power_on(priv->phy);
236 if (err)
237 goto err_phy_power_on;
238 }
239
[ snip ]
267 err = usb_add_hcd(hcd, irq, IRQF_SHARED);
268 if (err)
269 goto err_add_hcd;
270
271 device_wakeup_enable(hcd->self.controller);
272 return 0;
273
274 err_add_hcd:
275 if (!IS_ERR(priv->phy))
^^^^^^^^^^^^^^^^^^
Not an ERR_PTR.
276 phy_power_off(priv->phy);
277 err_phy_power_on:
278 if (!IS_ERR(priv->phy))
^^^^^^^^^^^^^^^^
This one isn't an error pointer either.
I think the problem here why we are confused about what is here is maybe
because of the else statement on line 230. If we just remove the else
statement and pull that code in one indent level the it is more clear.
Also, it's better if the label name describes the label location instead
of the goto location. Because when you are reading the code you first
hit the goto location and you see "goto err_phy_exit;" and you think
"Oh, that must call phy_exit() and that makes sense because we called
phy_init() on the line before." Then when we reach the end of the
function we verify that all the labels did what we expected.
But if the label is based on the goto line that doesn't add any
information when you are reading from top to bottom (normal direction)
so you have to keep flipping back and forth between the end and middle
of the function and it interrupts your thought process.
279 phy_exit(priv->phy);
280 err_phy_init:
281 err_phy_get:
282 if (!IS_ERR(priv->clk))
283 clk_disable_unprepare(priv->clk);
284 usb_put_hcd(hcd);
285 err:
286 dev_err(&pdev->dev, "init %s fail, %d\n",
287 dev_name(&pdev->dev), err);
288
289 return err;
290 }
regards,
dan carpenter
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
