[PATCH] usb: cdns3: continue probe even when USB PHY device does not exist

[Siddharth Vadapalli]

On Tue, Jul 02, 2024 at 04:20:43PM +0300, Roger Quadros wrote:
> 
> 
> On 02/07/2024 15:07, Siddharth Vadapalli wrote:
> > Prior to commit cd295286c786 ("usb: cdns3: avoid error messages if phys
> > don't exist"), cdns3_probe() errors out only on failing to initialize the
> > USB2/USB3 PHY. However, since commit cd295286c786, absence of the PHY
> > device is also treated as an error, resulting in a regression.
> > 
> > Extend commit cd295286c786 to treat -ENODEV as an acceptable return value
> > of generic_phy_get_by_name() and continue device probe as was the case
> > prior to the commit.
> > 
> > Fixes: cd295286c786 ("usb: cdns3: avoid error messages if phys don't exist")
> > Signed-off-by: Siddharth Vadapalli <s-vadapalli at ti.com>
> > ---
> > 
> > Hello,
> > 
> > This patch is based on commit
> > b4cbd1a257 Merge tag 'u-boot-amlogic-20240701' of https://source.denx.de/u-boot/custodians/u-boot-amlogic into next
> > of the next branch of U-Boot.
> > 
> > Regards,
> > Siddharth.
> > 
> >  drivers/usb/cdns3/core.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/usb/cdns3/core.c b/drivers/usb/cdns3/core.c
> > index b4e931646b..5b3e32953e 100644
> > --- a/drivers/usb/cdns3/core.c
> > +++ b/drivers/usb/cdns3/core.c
> > @@ -338,7 +338,7 @@ static int cdns3_probe(struct cdns3 *cdns)
> >  			dev_err(dev, "USB2 PHY init failed: %d\n", ret);
> >  			return ret;
> >  		}
> > -	} else if (ret != -ENOENT && ret != -ENODATA) {
> > +	} else if (ret != -ENOENT && ret != -ENODATA && ret != -ENODEV) {
> 
> With this change we will not error out on a genuine error condition
> that produces ENODEV.

It isn't necessarily a genuine error condition which is why it was a
"dev_warn" earlier for any error. If the previous stage has already
configured the PHY, or if the PHY present in the device-tree in Linux is
not the same as the PHY being used at U-Boot (USB 2 PHY at U-Boot vs
SERDES in Linux), then it isn't an error.

> 
> If PHY phandle is not present the API should return ENOENT right?
> 
> static int __of_parse_phandle_with_args(const struct device_node *np,
> 
>         /* Retrieve the phandle list property */
>         list = of_get_property(np, list_name, &size);
>         if (!list)
>                 return -ENOENT;

The PHY phandle is present, but it isn't the one being used by U-Boot.
The device-tree could be pointing to SERDES as the PHY, since Linux uses
USB with SERDES. So the entry exists, but the error is -ENODEV rather
than -ENOENT.

> 
> Can you please check and point where the -ENODEV error is coming from?

The sequence of function calls is as follows:
generic_phy_get_by_name
  generic_phy_get_by_index
    generic_phy_get_by_index_nodev
      uclass_get_device_by_ofnode
        uclass_find_device_by_ofnode
	  -ENODEV

In the above sequence, the device-tree contains SERDES PHY as the USB
PHY since Linux uses the same and U-Boot's device-tree is in sync with
Linux's. However, USB at U-Boot will use the USB 2 PHY. So one option is
to remove the SERDES PHY from USB node to have it fallback to USB 2 PHY.
At the same time, if the previous stage has configured SERDES for example,
it might not be necessary to reconfigure SERDES. -ENODEV might be an
acceptable error in such a situation as well. Please let me know.

[...]

Regards,
Siddharth.