Regressions List Tracking
 help / color / mirror / Atom feed
* Re: [PATCH stable-5.15 3/3] usb: dwc3: disable USB core PHY management
       [not found] ` <20220906120702.19219-4-johan@kernel.org>
@ 2022-10-18 15:27   ` Stefan Agner
  2022-10-19  8:59     ` Johan Hovold
  2022-10-28 13:46     ` Marek Szyprowski
  0 siblings, 2 replies; 10+ messages in thread
From: Stefan Agner @ 2022-10-18 15:27 UTC (permalink / raw
  To: Johan Hovold
  Cc: Greg Kroah-Hartman, stable, linux-kernel, Johan Hovold,
	Matthias Kaehlcke, stable, regressions, m.szyprowski, krzk

Hi Johan,

On 2022-09-06 14:07, Johan Hovold wrote:
> From: Johan Hovold <johan+linaro@kernel.org>
> 
> 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].

--
Stefan

[1]
https://dn.odroid.com/5422/ODROID-HC1_MC1/Schematics/HC1_MAIN_REV0.1_20170630.pdf
[2] https://github.com/home-assistant/operating-system/issues/2153

> 
> Fixes: 4e88d4c08301 ("usb: add a flag to skip PHY initialization to
> struct usb_hcd")
> Fixes: 178a0bce05cb ("usb: core: hcd: integrate the PHY wrapper into
> the HCD core")
> Tested-by: Matthias Kaehlcke <mka@chromium.org>
> Cc: stable <stable@kernel.org>
> Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> Link: https://lore.kernel.org/r/20220825131836.19769-1-johan+linaro@kernel.org
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> [ johan: adjust context to 5.15 ]
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> ---
>  drivers/usb/dwc3/host.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/usb/dwc3/host.c b/drivers/usb/dwc3/host.c
> index 2078e9d70292..85165a972076 100644
> --- a/drivers/usb/dwc3/host.c
> +++ b/drivers/usb/dwc3/host.c
> @@ -10,8 +10,13 @@
>  #include <linux/acpi.h>
>  #include <linux/platform_device.h>
>  
> +#include "../host/xhci-plat.h"
>  #include "core.h"
>  
> +static const struct xhci_plat_priv dwc3_xhci_plat_priv = {
> +	.quirks = XHCI_SKIP_PHY_INIT,
> +};
> +
>  static int dwc3_host_get_irq(struct dwc3 *dwc)
>  {
>  	struct platform_device	*dwc3_pdev = to_platform_device(dwc->dev);
> @@ -87,6 +92,11 @@ int dwc3_host_init(struct dwc3 *dwc)
>  		goto err;
>  	}
>  
> +	ret = platform_device_add_data(xhci, &dwc3_xhci_plat_priv,
> +					sizeof(dwc3_xhci_plat_priv));
> +	if (ret)
> +		goto err;
> +
>  	memset(props, 0, sizeof(struct property_entry) * ARRAY_SIZE(props));
>  
>  	if (dwc->usb3_lpm_capable)

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH stable-5.15 3/3] usb: dwc3: disable USB core PHY management
  2022-10-18 15:27   ` [PATCH stable-5.15 3/3] usb: dwc3: disable USB core PHY management Stefan Agner
@ 2022-10-19  8:59     ` Johan Hovold
  2022-10-20 22:06       ` Stefan Agner
  2022-10-28 13:46     ` Marek Szyprowski
  1 sibling, 1 reply; 10+ messages in thread
From: Johan Hovold @ 2022-10-19  8:59 UTC (permalink / raw
  To: Stefan Agner
  Cc: Greg Kroah-Hartman, stable, linux-kernel, Johan Hovold,
	Matthias Kaehlcke, stable, regressions, m.szyprowski, krzk

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@kernel.org>
> > 
> > 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)

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH stable-5.15 3/3] usb: dwc3: disable USB core PHY management
  2022-10-19  8:59     ` Johan Hovold
@ 2022-10-20 22:06       ` Stefan Agner
  2022-10-21  6:54         ` Johan Hovold
  0 siblings, 1 reply; 10+ messages in thread
From: Stefan Agner @ 2022-10-20 22:06 UTC (permalink / raw
  To: Johan Hovold
  Cc: Greg Kroah-Hartman, stable, linux-kernel, Johan Hovold,
	Matthias Kaehlcke, stable, regressions, m.szyprowski, krzk

Hi Johan,

On 2022-10-19 10:59, Johan Hovold wrote:
> 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@kernel.org>
>> >
>> > 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?

The patch below did not apply to 5.15.74 directly, but I think I was
able to get the corrected patch applied (see below)

That said, I do not have direct access to that hardware, but I created a
build and asked the user test it.

Best regards,
Stefan

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index c32ca691bcc7..964f512603ec 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -196,6 +196,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;
@@ -1227,6 +1229,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)



--
Stefan


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH stable-5.15 3/3] usb: dwc3: disable USB core PHY management
  2022-10-20 22:06       ` Stefan Agner
@ 2022-10-21  6:54         ` Johan Hovold
  2022-10-26 13:11           ` Stefan Agner
  0 siblings, 1 reply; 10+ messages in thread
From: Johan Hovold @ 2022-10-21  6:54 UTC (permalink / raw
  To: Stefan Agner
  Cc: Greg Kroah-Hartman, stable, linux-kernel, Johan Hovold,
	Matthias Kaehlcke, stable, regressions, m.szyprowski, krzk

On Fri, Oct 21, 2022 at 12:06:12AM +0200, Stefan Agner wrote:
> On 2022-10-19 10:59, Johan Hovold wrote:
> > On Tue, Oct 18, 2022 at 05:27:24PM +0200, Stefan Agner wrote:
> >> On 2022-09-06 14:07, Johan Hovold wrote:
> >> > From: Johan Hovold <johan+linaro@kernel.org>
> >> >
> >> > 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.

> >> For some reason, this commit seems to break detection of the USB to
> >> S-ATA controller on ODROID-HC1 devices (Exynos 5422).

> > 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?
> 
> The patch below did not apply to 5.15.74 directly, but I think I was
> able to get the corrected patch applied (see below)

Looks good to me.

> That said, I do not have direct access to that hardware, but I created a
> build and asked the user test it.

Thanks, let me know how it goes.

Johan

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH stable-5.15 3/3] usb: dwc3: disable USB core PHY management
  2022-10-21  6:54         ` Johan Hovold
@ 2022-10-26 13:11           ` Stefan Agner
  2022-10-27 12:45             ` Johan Hovold
  0 siblings, 1 reply; 10+ messages in thread
From: Stefan Agner @ 2022-10-26 13:11 UTC (permalink / raw
  To: Johan Hovold
  Cc: Greg Kroah-Hartman, stable, linux-kernel, Johan Hovold,
	Matthias Kaehlcke, stable, regressions, m.szyprowski, krzk

On 2022-10-21 08:54, Johan Hovold wrote:
> On Fri, Oct 21, 2022 at 12:06:12AM +0200, Stefan Agner wrote:
>> On 2022-10-19 10:59, Johan Hovold wrote:
>> > On Tue, Oct 18, 2022 at 05:27:24PM +0200, Stefan Agner wrote:
>> >> On 2022-09-06 14:07, Johan Hovold wrote:
>> >> > From: Johan Hovold <johan+linaro@kernel.org>
>> >> >
>> >> > 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.
> 
>> >> For some reason, this commit seems to break detection of the USB to
>> >> S-ATA controller on ODROID-HC1 devices (Exynos 5422).
> 
>> > 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?
>> 
>> The patch below did not apply to 5.15.74 directly, but I think I was
>> able to get the corrected patch applied (see below)
> 
> Looks good to me.
> 
>> That said, I do not have direct access to that hardware, but I created a
>> build and asked the user test it.
> 
> Thanks, let me know how it goes.

The user reports the S-ATA disk is *not* recognized with that patch
applied.

--
Stefan


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH stable-5.15 3/3] usb: dwc3: disable USB core PHY management
  2022-10-26 13:11           ` Stefan Agner
@ 2022-10-27 12:45             ` Johan Hovold
  2022-11-03 14:49               ` Johan Hovold
  0 siblings, 1 reply; 10+ messages in thread
From: Johan Hovold @ 2022-10-27 12:45 UTC (permalink / raw
  To: Stefan Agner
  Cc: Greg Kroah-Hartman, stable, linux-kernel, Johan Hovold,
	Matthias Kaehlcke, stable, regressions, m.szyprowski, krzk

On Wed, Oct 26, 2022 at 03:11:00PM +0200, Stefan Agner wrote:
> On 2022-10-21 08:54, Johan Hovold wrote:
> > On Fri, Oct 21, 2022 at 12:06:12AM +0200, Stefan Agner wrote:
> >> On 2022-10-19 10:59, Johan Hovold wrote:
> >> > On Tue, Oct 18, 2022 at 05:27:24PM +0200, Stefan Agner wrote:
> >> >> On 2022-09-06 14:07, Johan Hovold wrote:
> >> >> > From: Johan Hovold <johan+linaro@kernel.org>
> >> >> >
> >> >> > 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.
> > 
> >> >> For some reason, this commit seems to break detection of the USB to
> >> >> S-ATA controller on ODROID-HC1 devices (Exynos 5422).
> > 
> >> > 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?
> >> 
> >> The patch below did not apply to 5.15.74 directly, but I think I was
> >> able to get the corrected patch applied (see below)

> The user reports the S-ATA disk is *not* recognized with that patch
> applied.

I just noticed a mistake in the instrumentation patch I sent you. Could
you try moving the calibrations calls after dwc3_host_init() (e.g. as in
the second chunk in the diff below)?

As mentioned in the commit message for a0a465569b45 ("usb: dwc3: remove
generic PHY calibrate() calls"), this may not work if the xhci-plat
driver is built as a module and there are some corner cases that it does
not cover.

It seems we should revert the offending commit and then try to find some
time to untangle this mess, but please check if the below addresses the
issue first so we know what the problem is.

I'll prepare a revert in the meantime.

Johan


diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 31156d4dec9f..37d49a394912 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -197,6 +197,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;
@@ -1391,6 +1393,9 @@ static int dwc3_core_init_mode(struct dwc3 *dwc)
                ret = dwc3_host_init(dwc);
                if (ret)
                        return dev_err_probe(dev, ret, "failed to initialize host\n");
+
+               phy_calibrate(dwc->usb2_generic_phy);
+               phy_calibrate(dwc->usb3_generic_phy);
                break;
        case USB_DR_MODE_OTG:
                INIT_WORK(&dwc->drd_work, __dwc3_set_mode);

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH stable-5.15 3/3] usb: dwc3: disable USB core PHY management
  2022-10-18 15:27   ` [PATCH stable-5.15 3/3] usb: dwc3: disable USB core PHY management Stefan Agner
  2022-10-19  8:59     ` Johan Hovold
@ 2022-10-28 13:46     ` Marek Szyprowski
  1 sibling, 0 replies; 10+ messages in thread
From: Marek Szyprowski @ 2022-10-28 13:46 UTC (permalink / raw
  To: Stefan Agner, Johan Hovold
  Cc: Greg Kroah-Hartman, stable, linux-kernel, Johan Hovold,
	Matthias Kaehlcke, stable, regressions, krzk

Dear All,

On 18.10.2022 17:27, Stefan Agner wrote:
> Hi Johan,
>
> On 2022-09-06 14:07, Johan Hovold wrote:
>> From: Johan Hovold <johan+linaro@kernel.org>
>>
>> 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].

I confirm this issue and I've managed to reproduce it locally. The 
mainline is also affected. I will try to prepare a proper patch soon.

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH stable-5.15 3/3] usb: dwc3: disable USB core PHY management
  2022-10-27 12:45             ` Johan Hovold
@ 2022-11-03 14:49               ` Johan Hovold
  2022-11-03 15:18                 ` Marek Szyprowski
  0 siblings, 1 reply; 10+ messages in thread
From: Johan Hovold @ 2022-11-03 14:49 UTC (permalink / raw
  To: Stefan Agner
  Cc: Greg Kroah-Hartman, stable, linux-kernel, Johan Hovold,
	Matthias Kaehlcke, stable, regressions, m.szyprowski, krzk

On Thu, Oct 27, 2022 at 02:45:15PM +0200, Johan Hovold wrote:
> On Wed, Oct 26, 2022 at 03:11:00PM +0200, Stefan Agner wrote:

> > The user reports the S-ATA disk is *not* recognized with that patch
> > applied.
> 
> I just noticed a mistake in the instrumentation patch I sent you. Could
> you try moving the calibrations calls after dwc3_host_init() (e.g. as in
> the second chunk in the diff below)?
> 
> As mentioned in the commit message for a0a465569b45 ("usb: dwc3: remove
> generic PHY calibrate() calls"), this may not work if the xhci-plat
> driver is built as a module and there are some corner cases that it does
> not cover.
> 
> It seems we should revert the offending commit and then try to find some
> time to untangle this mess, but please check if the below addresses the
> issue first so we know what the problem is.
> 
> I'll prepare a revert in the meantime.

I've now posted the revert, but please do check if the below patch was
enough to resolve the immediate issue.

Johan

> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index 31156d4dec9f..37d49a394912 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -197,6 +197,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;
> @@ -1391,6 +1393,9 @@ static int dwc3_core_init_mode(struct dwc3 *dwc)
>                 ret = dwc3_host_init(dwc);
>                 if (ret)
>                         return dev_err_probe(dev, ret, "failed to initialize host\n");
> +
> +               phy_calibrate(dwc->usb2_generic_phy);
> +               phy_calibrate(dwc->usb3_generic_phy);
>                 break;
>         case USB_DR_MODE_OTG:
>                 INIT_WORK(&dwc->drd_work, __dwc3_set_mode);

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH stable-5.15 3/3] usb: dwc3: disable USB core PHY management
  2022-11-03 14:49               ` Johan Hovold
@ 2022-11-03 15:18                 ` Marek Szyprowski
  2022-11-03 15:29                   ` Johan Hovold
  0 siblings, 1 reply; 10+ messages in thread
From: Marek Szyprowski @ 2022-11-03 15:18 UTC (permalink / raw
  To: Johan Hovold, Stefan Agner
  Cc: Greg Kroah-Hartman, stable, linux-kernel, Johan Hovold,
	Matthias Kaehlcke, stable, regressions, krzk

Hi Johan,

On 03.11.2022 15:49, Johan Hovold wrote:
> On Thu, Oct 27, 2022 at 02:45:15PM +0200, Johan Hovold wrote:
>> On Wed, Oct 26, 2022 at 03:11:00PM +0200, Stefan Agner wrote:
>>> The user reports the S-ATA disk is *not* recognized with that patch
>>> applied.
>> I just noticed a mistake in the instrumentation patch I sent you. Could
>> you try moving the calibrations calls after dwc3_host_init() (e.g. as in
>> the second chunk in the diff below)?
>>
>> As mentioned in the commit message for a0a465569b45 ("usb: dwc3: remove
>> generic PHY calibrate() calls"), this may not work if the xhci-plat
>> driver is built as a module and there are some corner cases that it does
>> not cover.
>>
>> It seems we should revert the offending commit and then try to find some
>> time to untangle this mess, but please check if the below addresses the
>> issue first so we know what the problem is.
>>
>> I'll prepare a revert in the meantime.
> I've now posted the revert, but please do check if the below patch was
> enough to resolve the immediate issue.

The below patch was a half-fix. It worked only if both dwc3 and 
xhci_plat_hcd were compiled into the kernel. Afair Debian-based distros 
used xhci compiled as a module, so this didn't work for that case due to 
timing issues.


>
> Johan
>
>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
>> index 31156d4dec9f..37d49a394912 100644
>> --- a/drivers/usb/dwc3/core.c
>> +++ b/drivers/usb/dwc3/core.c
>> @@ -197,6 +197,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;
>> @@ -1391,6 +1393,9 @@ static int dwc3_core_init_mode(struct dwc3 *dwc)
>>                  ret = dwc3_host_init(dwc);
>>                  if (ret)
>>                          return dev_err_probe(dev, ret, "failed to initialize host\n");
>> +
>> +               phy_calibrate(dwc->usb2_generic_phy);
>> +               phy_calibrate(dwc->usb3_generic_phy);
>>                  break;
>>          case USB_DR_MODE_OTG:
>>                  INIT_WORK(&dwc->drd_work, __dwc3_set_mode);

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH stable-5.15 3/3] usb: dwc3: disable USB core PHY management
  2022-11-03 15:18                 ` Marek Szyprowski
@ 2022-11-03 15:29                   ` Johan Hovold
  0 siblings, 0 replies; 10+ messages in thread
From: Johan Hovold @ 2022-11-03 15:29 UTC (permalink / raw
  To: Marek Szyprowski
  Cc: Stefan Agner, Greg Kroah-Hartman, stable, linux-kernel,
	Johan Hovold, Matthias Kaehlcke, stable, regressions, krzk

On Thu, Nov 03, 2022 at 04:18:12PM +0100, Marek Szyprowski wrote:
> Hi Johan,
> 
> On 03.11.2022 15:49, Johan Hovold wrote:
> > On Thu, Oct 27, 2022 at 02:45:15PM +0200, Johan Hovold wrote:
> >> On Wed, Oct 26, 2022 at 03:11:00PM +0200, Stefan Agner wrote:
> >>> The user reports the S-ATA disk is *not* recognized with that patch
> >>> applied.
> >> I just noticed a mistake in the instrumentation patch I sent you. Could
> >> you try moving the calibrations calls after dwc3_host_init() (e.g. as in
> >> the second chunk in the diff below)?
> >>
> >> As mentioned in the commit message for a0a465569b45 ("usb: dwc3: remove
> >> generic PHY calibrate() calls"), this may not work if the xhci-plat
> >> driver is built as a module and there are some corner cases that it does
> >> not cover.
> >>
> >> It seems we should revert the offending commit and then try to find some
> >> time to untangle this mess, but please check if the below addresses the
> >> issue first so we know what the problem is.
> >>
> >> I'll prepare a revert in the meantime.
> > I've now posted the revert, but please do check if the below patch was
> > enough to resolve the immediate issue.
> 
> The below patch was a half-fix. It worked only if both dwc3 and 
> xhci_plat_hcd were compiled into the kernel. Afair Debian-based distros 
> used xhci compiled as a module, so this didn't work for that case due to 
> timing issues.

Yeah, I mentioned that above too. The intention here was just to confirm
the hypothesis that the regression was due to the missing calibration
calls.

Johan

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2022-11-03 15:30 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20220906120702.19219-1-johan@kernel.org>
     [not found] ` <20220906120702.19219-4-johan@kernel.org>
2022-10-18 15:27   ` [PATCH stable-5.15 3/3] usb: dwc3: disable USB core PHY management Stefan Agner
2022-10-19  8:59     ` Johan Hovold
2022-10-20 22:06       ` Stefan Agner
2022-10-21  6:54         ` Johan Hovold
2022-10-26 13:11           ` Stefan Agner
2022-10-27 12:45             ` Johan Hovold
2022-11-03 14:49               ` Johan Hovold
2022-11-03 15:18                 ` Marek Szyprowski
2022-11-03 15:29                   ` Johan Hovold
2022-10-28 13:46     ` Marek Szyprowski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).