Re: [PATCH stable-5.15 3/3] usb: dwc3: disable USB core PHY management

[Johan Hovold]

On Tue, Oct 18, 2022 at 05:27:24PM +0200, Stefan Agner wrote:
> Hi Johan,
> 
> On 2022-09-06 14:07, Johan Hovold wrote:
> > From: Johan Hovold <johan+linaro@xxxxxxxxxx>
> > 
> > commit 6000b8d900cd5f52fbcd0776d0cc396e88c8c2ea upstream.
> > 
> > The dwc3 driver manages its PHYs itself so the USB core PHY management
> > needs to be disabled.
> > 
> > Use the struct xhci_plat_priv hack added by commits 46034a999c07 ("usb:
> > host: xhci-plat: add platform data support") and f768e718911e ("usb:
> > host: xhci-plat: add priv quirk for skip PHY initialization") to
> > propagate the setting for now.
> 
> [adding also Samsung/ODROID device tree authors Krzysztof and Marek]
> 
> For some reason, this commit seems to break detection of the USB to
> S-ATA controller on ODROID-HC1 devices (Exynos 5422).
> 
> We have a known to work OS release of v5.15.60, and known to not be
> working of v5.15.67. By reverting suspicious commits, I was able to
> pinpoint the problem to this particular commit.
> 
> From what I understand, on that particular hardware the S-ATA controller
> power is controlled via the V-BUS signal VBUSCTRL_U2 (Schematic [1]).
> Presumably this signal is no longer controlled with this change.
> 
> This came up in our HAOS issue #2153 [2].

Thanks for the report and sorry about the breakage. This wasn't supposed
to go to stable but Greg thought otherwise (and I helped with the
backporting to prevent autosel from pulling in even more changes).

But at least this way we found out sooner that this platform depends on
having both USB core and dwc3 managing the same PHY.

I think this may be related to the calibration calls added to dwc3 and
later removed again by commits:

	d8c80bb3b55b ("phy: exynos5-usbdrd: Calibrate LOS levels for exynos5420/5800")
	a0a465569b45 ("usb: dwc3: remove generic PHY calibrate() calls")

The removal explicitly mentions that the expectation is that USB core
will do the PHY calibration.

There could be other changes in the sequencing of events that this
platform has been implicitly relying on, but as a start, could try
adding the missing calibration calls (patch below) and see if that makes a
difference?

Johan


diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 46cf8edf7f93..f8a0e340eb63 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -198,6 +198,8 @@ static void __dwc3_set_mode(struct work_struct *work)
                                otg_set_vbus(dwc->usb2_phy->otg, true);
                        phy_set_mode(dwc->usb2_generic_phy, PHY_MODE_USB_HOST);
                        phy_set_mode(dwc->usb3_generic_phy, PHY_MODE_USB_HOST);
+                       phy_calibrate(dwc->usb2_generic_phy);
+                       phy_calibrate(dwc->usb3_generic_phy);
                        if (dwc->dis_split_quirk) {
                                reg = dwc3_readl(dwc->regs, DWC3_GUCTL3);
                                reg |= DWC3_GUCTL3_SPLITDISABLE;
@@ -1369,6 +1371,8 @@ static int dwc3_core_init_mode(struct dwc3 *dwc)
                        otg_set_vbus(dwc->usb2_phy->otg, true);
                phy_set_mode(dwc->usb2_generic_phy, PHY_MODE_USB_HOST);
                phy_set_mode(dwc->usb3_generic_phy, PHY_MODE_USB_HOST);
+               phy_calibrate(dwc->usb2_generic_phy);
+               phy_calibrate(dwc->usb3_generic_phy);
 
                ret = dwc3_host_init(dwc);
                if (ret)