Re: [PATCH 5/5] usb: add pxa1928 ehci support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On Thursday 14 May 2015 11:55:36 Alan Stern wrote:
> 
> > +     hcd->phy = devm_of_phy_get(&pdev->dev, pdev->dev.of_node, NULL);
> > +     if (IS_ERR_OR_NULL(hcd->phy)) {
> > +             retval = PTR_ERR(hcd->phy);
> > +             if (retval != -EPROBE_DEFER && retval != -ENODEV)
> > +                     dev_err(&pdev->dev, "failed to get the phy\n");
> > +             else
> > +                     return -EPROBE_DEFER;
> > +             goto err_put_hcd;
> > +     }
> 
> Please straighten out this convoluted logic.  It should go like this:
> 
>         hcd->phy = devm_of_phy_get(&pdev->dev, pdev->dev.of_node, NULL);
>         if (IS_ERR_OR_NULL(hcd->phy)) {
>                 retval = PTR_ERR(hcd->phy);
>                 if (retval == -EPROBE_DEFER || retval == -ENODEV)
>                         return -EPROBE_DEFER;
>                 dev_err(&pdev->dev, "failed to get the phy\n");
>                 goto err_put_hcd;
>         }
> 

IS_ERR_OR_NULL is almost always wrong. Kernel interfaces that can return
an error pointer should never return a NULL pointer, and I'm pretty sure
that is also the case for the phy subsystem (or else it should be fixed).

In some cases, we have interfaces that return NULL pointers or error pointers,
but those are designed to treat NULL as success.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]
  Powered by Linux