USB: ehci-omap: Fix deferred probe for phy handling

USB: ehci-omap: Fix deferred probe for phy handling

[Tony Lindgren]

* Roger Quadros <rogerq@ti.com> [181213 12:09]:
> Tony,
> 
> On 13/12/18 04:17, Tony Lindgren wrote:
> > The only case where we can bail out safely without a phy is port_mode
> > is OMAP_USBHS_PORT_MODE_UNUSED. It used to be that OMAP_EHCI_PORT_MODE_PHY
> > was optional, but that's not a good assumption. We should already have
> > "ehci-phy" in all the dts files using OMAP_EHCI_PORT_MODE_PHY.
> > 
> 
> We don't use the PHY at all if port mode is not OMAP_EHCI_PORT_MODE_PHY.

Hmm thanks for looking. That logic seems like it was valid
something like 10 years ago for omap3, I don't think that applies
any longer.

We see -EPROBE_DEFER errors for OMAP_EHCI_PORT_MODE_HSIC on
omap5-uevm for SMSC Ethernet. So that's with "ehci-hsic" in the dts
files.

> What issue is this patch fixing?

We ignore -EPROBE_DEFER errors now. That's reproducable with
current Linux next on omap5-uevm with phy-generic, ehci-omap and
smsc Ethernet built-in. After boot ifconfig -a won't show eth0.

Care to recheck what the safe logic to ignore errors can be?

Regards,

Tony

USB: ehci-omap: Fix deferred probe for phy handling

[Roger Quadros]

Tony,

On 13/12/18 04:17, Tony Lindgren wrote:
> The only case where we can bail out safely without a phy is port_mode
> is OMAP_USBHS_PORT_MODE_UNUSED. It used to be that OMAP_EHCI_PORT_MODE_PHY
> was optional, but that's not a good assumption. We should already have
> "ehci-phy" in all the dts files using OMAP_EHCI_PORT_MODE_PHY.
> 

We don't use the PHY at all if port mode is not OMAP_EHCI_PORT_MODE_PHY.

> Note that this fix should not be needed for kernels earlier than v4.19
> as that's when we started moving devices to probe with ti-sysc. We now
> probe l4 interconnects separately, which can cause deferred probe with
> the phy being on a separate l4 interconnect from EHCI.
> 
> And old kernels would need to be checked for "ehci-phy" property for
> this fix to avoid regressions.
> 

What issue is this patch fixing?

> Cc: Alan Stern <stern@rowland.harvard.edu>
> Cc: Johan Hovold <johan@kernel.org>
> Cc: Ladislav Michl <ladis@linux-mips.org>
> Cc: Peter Ujfalusi <peter.ujfalusi@ti.com>
> Cc: Roger Quadros <rogerq@ti.com>
> Reported-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
> Signed-off-by: Tony Lindgren <tony@atomide.com>
> ---
>  drivers/usb/host/ehci-omap.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/host/ehci-omap.c b/drivers/usb/host/ehci-omap.c
> --- a/drivers/usb/host/ehci-omap.c
> +++ b/drivers/usb/host/ehci-omap.c
> @@ -159,8 +159,8 @@ static int ehci_hcd_omap_probe(struct platform_device *pdev)
>  		/* get the PHY device */
>  		phy = devm_usb_get_phy_by_phandle(dev, "phys", i);
>  		if (IS_ERR(phy)) {
> -			/* Don't bail out if PHY is not absolutely necessary */
> -			if (pdata->port_mode[i] != OMAP_EHCI_PORT_MODE_PHY)
> +			/* Don't bail out if PHY is unused */
> +			if (pdata->port_mode[i] == OMAP_USBHS_PORT_MODE_UNUSED)
>  				continue;
>  
>  			ret = PTR_ERR(phy);

devm_usb_get_phy_by_phandle() returns -ENODEV if PHY phandle is not present
which would be the case in PHY'less modes (e.g. "ehci-tll" or "ehci-hsic").
This change will error out instead of proceeding normally in those cases.

cheers,
-roger

USB: ehci-omap: Fix deferred probe for phy handling

[Peter Ujfalusi]

On 13/12/2018 4.17, Tony Lindgren wrote:
> The only case where we can bail out safely without a phy is port_mode
> is OMAP_USBHS_PORT_MODE_UNUSED. It used to be that OMAP_EHCI_PORT_MODE_PHY
> was optional, but that's not a good assumption. We should already have
> "ehci-phy" in all the dts files using OMAP_EHCI_PORT_MODE_PHY.
> 
> Note that this fix should not be needed for kernels earlier than v4.19
> as that's when we started moving devices to probe with ti-sysc. We now
> probe l4 interconnects separately, which can cause deferred probe with
> the phy being on a separate l4 interconnect from EHCI.
> 
> And old kernels would need to be checked for "ehci-phy" property for
> this fix to avoid regressions.

Tested-by: Peter Ujfalusi <peter.ujfalusi@ti.com>

> Cc: Alan Stern <stern@rowland.harvard.edu>
> Cc: Johan Hovold <johan@kernel.org>
> Cc: Ladislav Michl <ladis@linux-mips.org>
> Cc: Peter Ujfalusi <peter.ujfalusi@ti.com>
> Cc: Roger Quadros <rogerq@ti.com>
> Reported-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
> Signed-off-by: Tony Lindgren <tony@atomide.com>
> ---
>  drivers/usb/host/ehci-omap.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/host/ehci-omap.c b/drivers/usb/host/ehci-omap.c
> --- a/drivers/usb/host/ehci-omap.c
> +++ b/drivers/usb/host/ehci-omap.c
> @@ -159,8 +159,8 @@ static int ehci_hcd_omap_probe(struct platform_device *pdev)
>  		/* get the PHY device */
>  		phy = devm_usb_get_phy_by_phandle(dev, "phys", i);
>  		if (IS_ERR(phy)) {
> -			/* Don't bail out if PHY is not absolutely necessary */
> -			if (pdata->port_mode[i] != OMAP_EHCI_PORT_MODE_PHY)
> +			/* Don't bail out if PHY is unused */
> +			if (pdata->port_mode[i] == OMAP_USBHS_PORT_MODE_UNUSED)
>  				continue;
>  
>  			ret = PTR_ERR(phy);
> 

- Péter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

USB: ehci-omap: Fix deferred probe for phy handling

[Tony Lindgren]

The only case where we can bail out safely without a phy is port_mode
is OMAP_USBHS_PORT_MODE_UNUSED. It used to be that OMAP_EHCI_PORT_MODE_PHY
was optional, but that's not a good assumption. We should already have
"ehci-phy" in all the dts files using OMAP_EHCI_PORT_MODE_PHY.

Note that this fix should not be needed for kernels earlier than v4.19
as that's when we started moving devices to probe with ti-sysc. We now
probe l4 interconnects separately, which can cause deferred probe with
the phy being on a separate l4 interconnect from EHCI.

And old kernels would need to be checked for "ehci-phy" property for
this fix to avoid regressions.

Cc: Alan Stern <stern@rowland.harvard.edu>
Cc: Johan Hovold <johan@kernel.org>
Cc: Ladislav Michl <ladis@linux-mips.org>
Cc: Peter Ujfalusi <peter.ujfalusi@ti.com>
Cc: Roger Quadros <rogerq@ti.com>
Reported-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
 drivers/usb/host/ehci-omap.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/host/ehci-omap.c b/drivers/usb/host/ehci-omap.c
--- a/drivers/usb/host/ehci-omap.c
+++ b/drivers/usb/host/ehci-omap.c
@@ -159,8 +159,8 @@ static int ehci_hcd_omap_probe(struct platform_device *pdev)
 		/* get the PHY device */
 		phy = devm_usb_get_phy_by_phandle(dev, "phys", i);
 		if (IS_ERR(phy)) {
-			/* Don't bail out if PHY is not absolutely necessary */
-			if (pdata->port_mode[i] != OMAP_EHCI_PORT_MODE_PHY)
+			/* Don't bail out if PHY is unused */
+			if (pdata->port_mode[i] == OMAP_USBHS_PORT_MODE_UNUSED)
 				continue;
 
 			ret = PTR_ERR(phy);