On 10/27/2015 01:41 AM, Uwe Kleine-König wrote:
Hello,
On Mon, Oct 26, 2015 at 08:08:58PM -0600, Stephen Warren wrote:
On 10/26/2015 03:28 AM, Alexander Aring wrote:
This patch changes devm_phy_get to devm_phy_optional_get. Which fixes the
error handling when trying to get the phy. Currently on all errors we
try to look for an old style USB PHY. With this patch we try to look for
an old USB PHY when devm_phy_get returns -ENOENT only. Other values like
-EPROBE_DEFER need to be returned from the probe function, because it
could be that a phy is specified but not probed before.
diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c
@@ -222,12 +222,15 @@ static int dwc2_driver_probe(struct platform_device *dev)
hsotg->dr_mode = of_usb_get_dr_mode(dev->dev.of_node);
+ phy = devm_phy_optional_get(&dev->dev, "usb2-phy");
+ if (IS_ERR(phy))
+ return PTR_ERR(phy);
Here, phy is checked using IS_ERR(), ...
IS_ERR(phy) is true, if something went wrong getting a phy (probably
from dt).
/*
* Attempt to find a generic PHY, then look for an old style
* USB PHY
*/
- phy = devm_phy_get(&dev->dev, "usb2-phy");
- if (IS_ERR(phy)) {
+ if (!phy) {
... and here the same phy value is checked against NULL. Hopefully,
and phy is NULL if dt didn't specify a phy. That's the semantic of
devm_phy_optional_get (which is identically to what
devm_gpiod_get_optional) does.
devm_phy_get() doesn't actually return either an ERR value or a NULL
pointer as errors, but restricts itself to either an ERR value or
The relevant part is that for a function with "optinal" in it's name not
finding the respective resource isn't an error. So checking for NULL
isn't error handling but handling the 2nd valid case.
If devm_phy_optional_get() uses IS_ERR to check for errors, then ANY
other (non-IS_ERR) value is an opaque handle to a PHY. The values should
be meaningless outside of the PHY subsystem. Any other set of return
values is broken.
The whole point about optional APIs is that they return one of:
a) An error value that can be checked for. Any error is a hard error.
b) A valid PHY value that can be passed to all other PHY-related API.
(b) can be further divided into two cases:
b1) An actual PHY handle if the PHY exists
b2) A dummy PHY value that the PHY APIs internally map to no-ops.
However, the distinction between (b1) and (b2) is something that clients
should not know about, and hence assuming "NULL" is that dummy value is
wrong.
--
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
