| Message ID | 20181213021723.49798-1-tony@atomide.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | USB: ehci-omap: Fix deferred probe for phy handling | expand |
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
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
* 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
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);
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(-)