usb: dwc3: 5.15 backports
Subsystems (2)
cover usb: dwc3: 5.15 backports
Here are backports of the three patches that failed to apply to 5.15 due to trivial context conflicts.
Hopefully they apply to the older stable trees as well as-is.
Note that the last patch depends on features that were not added until 5.9 as mentioned in the commit message. Note that the author of that patch did not add a stable tag for this one, but backporting shouldn't hurt.
Johan
Johan Hovold (3):
usb: dwc3: fix PHY disable sequence
usb: dwc3: qcom: fix use-after-free on runtime-PM wakeup
usb: dwc3: disable USB core PHY management
drivers/usb/dwc3/core.c | 19 ++++++++++--------- drivers/usb/dwc3/dwc3-qcom.c | 14 +++++++++++++- drivers/usb/dwc3/host.c | 11 +++++++++++ 3 files changed, 34 insertions(+), 10 deletions(-)
Signature · 1 line
2.35.1
PATCH 1/3 usb: dwc3: fix PHY disable sequence
From: Johan Hovold <[email protected]>
commit d2ac7bef95c9ead307801ccb6cb6dfbeb14247bf upstream.
Generic PHYs must be powered-off before they can be tore down.
Similarly, suspending legacy PHYs after having powered them off makes no sense.
Fix the dwc3_core_exit() (e.g. called during suspend) and open-coded dwc3_probe() error-path sequences that got this wrong.
Note that this makes dwc3_core_exit() match the dwc3_core_init() error path with respect to powering off the PHYs.
--- drivers/usb/dwc3/core.c | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-)
drivers/usb/dwc3/core.c +10 -9
@@ -731,15 +731,16 @@ static void dwc3_core_exit(struct dwc3 *dwc) {dwc3_event_buffers_cleanup (dwc); +usb_phy_set_suspend (dwc->usb2_phy,1 ); +usb_phy_set_suspend (dwc->usb3_phy,1 ); +phy_power_off (dwc->usb2_generic_phy); +phy_power_off (dwc->usb3_generic_phy); +usb_phy_shutdown (dwc->usb2_phy);usb_phy_shutdown (dwc->usb3_phy);phy_exit (dwc->usb2_generic_phy);phy_exit (dwc->usb3_generic_phy); -usb_phy_set_suspend (dwc->usb2_phy,1 ); -usb_phy_set_suspend (dwc->usb3_phy,1 ); -phy_power_off (dwc->usb2_generic_phy); -phy_power_off (dwc->usb3_generic_phy);clk_bulk_disable_unprepare (dwc->num_clks, dwc->clks);reset_control_assert (dwc->reset); }
@@ -1662,16 +1663,16 @@ static int dwc3_probe(struct platform_device *pdev)dwc3_debugfs_exit (dwc);dwc3_event_buffers_cleanup (dwc); -usb_phy_shutdown (dwc->usb2_phy); -usb_phy_shutdown (dwc->usb3_phy); -phy_exit (dwc->usb2_generic_phy); -phy_exit (dwc->usb3_generic_phy); -usb_phy_set_suspend (dwc->usb2_phy,1 );usb_phy_set_suspend (dwc->usb3_phy,1 );phy_power_off (dwc->usb2_generic_phy);phy_power_off (dwc->usb3_generic_phy); +usb_phy_shutdown (dwc->usb2_phy); +usb_phy_shutdown (dwc->usb3_phy); +phy_exit (dwc->usb2_generic_phy); +phy_exit (dwc->usb3_generic_phy); +dwc3_ulpi_exit (dwc); err4:
Signature · 1 line
2.35.1
Greg Kroah-Hartman
maintainerRe: [PATCH stable-5.15 1/3] usb: dwc3: fix PHY disable sequence
Quoted 26 lines · Johan Hovold
From: Johan Hovold <[email protected]>
commit
d2ac7bef95c9ead307801ccb6cb6dfbeb14247bfupstream.Generic PHYs must be powered-off before they can be tore down.
Similarly, suspending legacy PHYs after having powered them off makes no sense.
Fix the
dwc3_core_exit()(e.g. called during suspend) and open-codeddwc3_probe()error-path sequences that got this wrong.Note that this makes
dwc3_core_exit()match thedwc3_core_init()error path with respect to powering off the PHYs.Fixes 03c1fd622f72 ("usb: dwc3: core: add phy cleanup for probe error handling")Fixes c499ff71ff2a ("usb: dwc3: core: re-factor init and exit paths")Cc [email protected] # 4.8Reviewed-by Andrew Halaney <[email protected]>Reviewed-by Matthias Kaehlcke <[email protected]>Reviewed-by Manivannan Sadhasivam <[email protected]>Signed-off-by Johan Hovold <[email protected]>Signed-off-by Greg Kroah-Hartman <[email protected]>[ johan: adjust context to 5.15 ]Signed-off-by Johan Hovold <[email protected]>
Quoted 3 lines
--- drivers/usb/dwc3/core.c | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-)
This one did not apply to 4.9.y, 4.14.y, or 4.19.y :(
Re: [PATCH stable-5.15 1/3] usb: dwc3: fix PHY disable sequence
Quoted 31 lines · Greg Kroah-Hartman
Quoted 26 lines · Johan Hovold
From: Johan Hovold <[email protected]>
commit
d2ac7bef95c9ead307801ccb6cb6dfbeb14247bfupstream.Generic PHYs must be powered-off before they can be tore down.
Similarly, suspending legacy PHYs after having powered them off makes no sense.
Fix the
dwc3_core_exit()(e.g. called during suspend) and open-codeddwc3_probe()error-path sequences that got this wrong.Note that this makes
dwc3_core_exit()match thedwc3_core_init()error path with respect to powering off the PHYs.Fixes 03c1fd622f72 ("usb: dwc3: core: add phy cleanup for probe error handling")Fixes c499ff71ff2a ("usb: dwc3: core: re-factor init and exit paths")Cc [email protected] # 4.8Reviewed-by Andrew Halaney <[email protected]>Reviewed-by Matthias Kaehlcke <[email protected]>Reviewed-by Manivannan Sadhasivam <[email protected]>Signed-off-by Johan Hovold <[email protected]>Signed-off-by Greg Kroah-Hartman <[email protected]>[ johan: adjust context to 5.15 ]Signed-off-by Johan Hovold <[email protected]>Quoted 3 lines
--- drivers/usb/dwc3/core.c | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-)
Quoted 1 line
This one did not apply to 4.9.y, 4.14.y, or 4.19.y :(
Perhaps someone who cares about these old trees can do the backports. Should be as trivial. Can't be the patch submitters responsibility to maintain 8 stable trees.
Johan
Greg Kroah-Hartman
maintainerRe: [PATCH stable-5.15 1/3] usb: dwc3: fix PHY disable sequence
Quoted 32 lines · Johan Hovold
Quoted 31 lines · Greg Kroah-Hartman
Quoted 29 lines · Johan Hovold
From: Johan Hovold <[email protected]>
commit
d2ac7bef95c9ead307801ccb6cb6dfbeb14247bfupstream.Generic PHYs must be powered-off before they can be tore down.
Similarly, suspending legacy PHYs after having powered them off makes no sense.
Fix the
dwc3_core_exit()(e.g. called during suspend) and open-codeddwc3_probe()error-path sequences that got this wrong.Note that this makes
dwc3_core_exit()match thedwc3_core_init()error path with respect to powering off the PHYs.Fixes 03c1fd622f72 ("usb: dwc3: core: add phy cleanup for probe error handling")Fixes c499ff71ff2a ("usb: dwc3: core: re-factor init and exit paths")Cc [email protected] # 4.8Reviewed-by Andrew Halaney <[email protected]>Reviewed-by Matthias Kaehlcke <[email protected]>Reviewed-by Manivannan Sadhasivam <[email protected]>Signed-off-by Johan Hovold <[email protected]>Signed-off-by Greg Kroah-Hartman <[email protected]>[ johan: adjust context to 5.15 ]Signed-off-by Johan Hovold <[email protected]>--- drivers/usb/dwc3/core.c | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-)
Quoted 5 lines
Quoted 1 line
This one did not apply to 4.9.y, 4.14.y, or 4.19.y :(
Perhaps someone who cares about these old trees can do the backports. Should be as trivial. Can't be the patch submitters responsibility to maintain 8 stable trees.
I agree! :)
PATCH 2/3 usb: dwc3: qcom: fix use-after-free on runtime-PM wakeup
In reply to Johan HovoldFrom: Johan Hovold <[email protected]>
commit a872ab303d5ddd4c965f9cd868677781a33ce35a upstream.
The Qualcomm dwc3 runtime-PM implementation checks the xhci platform-device pointer in the wakeup-interrupt handler to determine whether the controller is in host mode and if so triggers a resume.
After a role switch in OTG mode the xhci platform-device would have been freed and the next wakeup from runtime suspend would access the freed memory.
Note that role switching is executed from a freezable workqueue, which guarantees that the pointer is stable during suspend.
Also note that runtime PM has been broken since commit 2664deb09306 ("usb: dwc3: qcom: Honor wakeup enabled/disabled state"), which incidentally also prevents this issue from being triggered.
--- drivers/usb/dwc3/dwc3-qcom.c | 14 +++++++++++++- drivers/usb/dwc3/host.c | 1 + 2 files changed, 14 insertions(+), 1 deletion(-)
drivers/usb/dwc3/dwc3-qcom.c +13 -1
@@ -296,6 +296,14 @@ static void dwc3_qcom_interconnect_exit(struct dwc3_qcom *qcom)icc_put (qcom->icc_path_apps); } +/* Only usable in contexts where the role can not change. */ +static bool dwc3_qcom_is_host (struct dwc3_qcom *qcom) +{ +struct dwc3 *dwc =platform_get_drvdata (qcom->dwc3); + +return dwc->xhci; +} +static void dwc3_qcom_disable_interrupts (struct dwc3_qcom *qcom) {if (qcom->hs_phy_irq) {
@@ -411,7 +419,11 @@ static irqreturn_t qcom_dwc3_resume_irq(int irq, void *data)if (qcom->pm_suspended)return IRQ_HANDLED ; -if (dwc->xhci) +/* +* This is safe as role switching is done from a freezable workqueue +* and the wakeup interrupts are disabled as part of resume. +*/ +if (dwc3_qcom_is_host (qcom))pm_runtime_resume (&dwc->xhci->dev);return IRQ_HANDLED ;
drivers/usb/dwc3/host.c +1 -0
@@ -130,4 +130,5 @@ int dwc3_host_init(struct dwc3 *dwc)void dwc3_host_exit (struct dwc3 *dwc) {platform_device_unregister (dwc->xhci); + dwc->xhci =NULL ; }
Signature · 1 line
2.35.1
Greg Kroah-Hartman
maintainerRe: [PATCH stable-5.15 2/3] usb: dwc3: qcom: fix use-after-free on runtime-PM wakeup
Quoted 28 lines · Johan Hovold
From: Johan Hovold <[email protected]>
commit
a872ab303d5ddd4c965f9cd868677781a33ce35aupstream.The Qualcomm dwc3 runtime-PM implementation checks the xhci platform-device pointer in the wakeup-interrupt handler to determine whether the controller is in host mode and if so triggers a resume.
After a role switch in OTG mode the xhci platform-device would have been freed and the next wakeup from runtime suspend would access the freed memory.
Note that role switching is executed from a freezable workqueue, which guarantees that the pointer is stable during suspend.
Also note that runtime PM has been broken since commit
2664deb09306("usb: dwc3: qcom: Honor wakeup enabled/disabled state"), which incidentally also prevents this issue from being triggered.Fixes a4333c3a6ba9 ("usb: dwc3: Add Qualcomm DWC3 glue driver")Cc [email protected] # 4.18Reviewed-by Matthias Kaehlcke <[email protected]>Reviewed-by Manivannan Sadhasivam <[email protected]>Signed-off-by Johan Hovold <[email protected]>Signed-off-by Greg Kroah-Hartman <[email protected]>[ johan: adjust context for 5.15 ]Signed-off-by Johan Hovold <[email protected]>
Quoted 4 lines
--- drivers/usb/dwc3/dwc3-qcom.c | 14 +++++++++++++- drivers/usb/dwc3/host.c | 1 + 2 files changed, 14 insertions(+), 1 deletion(-)
This one did not apply to 5.4.y or 4.19.y :(
PATCH 3/3 usb: dwc3: disable USB core PHY management
In reply to Johan HovoldFrom: Johan Hovold <[email protected]>
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.
--- drivers/usb/dwc3/host.c | 10 ++++++++++ 1 file changed, 10 insertions(+)
drivers/usb/dwc3/host.c +10 -0
@@ -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)
Signature · 1 line
2.35.1
Greg Kroah-Hartman
maintainerRe: usb: dwc3: disable USB core PHY management
Quoted 23 lines · Johan Hovold
From: Johan Hovold <[email protected]>
commit
6000b8d900cd5f52fbcd0776d0cc396e88c8c2eaupstream.The dwc3 driver manages its PHYs itself so the USB core PHY management needs to be disabled.
Use the struct
xhci_plat_privhack added by commits46034a999c07("usb:
host: xhci-plat: add platform data support") andf768e718911e("usb:
host: xhci-plat: add priv quirk for skip PHY initialization") to propagate the setting for now.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 <[email protected]>Cc stable <[email protected]>Reviewed-by Matthias Kaehlcke <[email protected]>Signed-off-by Johan Hovold <[email protected]>Signed-off-by Greg Kroah-Hartman <[email protected]>[ johan: adjust context to 5.15 ]Signed-off-by Johan Hovold <[email protected]>Signed-off-by Greg Kroah-Hartman <[email protected]>
Quoted 3 lines
--- drivers/usb/dwc3/host.c | 10 ++++++++++ 1 file changed, 10 insertions(+)
Breaks the build on 4.19.y :(
Re: usb: dwc3: disable USB core PHY management
Quoted 28 lines · Greg Kroah-Hartman
Quoted 23 lines · Johan Hovold
From: Johan Hovold <[email protected]>
commit
6000b8d900cd5f52fbcd0776d0cc396e88c8c2eaupstream.The dwc3 driver manages its PHYs itself so the USB core PHY management needs to be disabled.
Use the struct
xhci_plat_privhack added by commits46034a999c07("usb:
host: xhci-plat: add platform data support") andf768e718911e("usb:
host: xhci-plat: add priv quirk for skip PHY initialization") to propagate the setting for now.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 <[email protected]>Cc stable <[email protected]>Reviewed-by Matthias Kaehlcke <[email protected]>Signed-off-by Johan Hovold <[email protected]>Signed-off-by Greg Kroah-Hartman <[email protected]>[ johan: adjust context to 5.15 ]Signed-off-by Johan Hovold <[email protected]>Signed-off-by Greg Kroah-Hartman <[email protected]>Quoted 3 lines
--- drivers/usb/dwc3/host.c | 10 ++++++++++ 1 file changed, 10 insertions(+)
Quoted 1 line
Breaks the build on 4.19.y :(
It's OK to not to backport this one.
Johan
Re: [PATCH stable-5.15 3/3] usb: dwc3: disable USB core PHY management
In reply to Johan HovoldHi Johan,
Quoted 11 lines · Johan Hovold
From: Johan Hovold <[email protected]>
commit
6000b8d900cd5f52fbcd0776d0cc396e88c8c2eaupstream.The dwc3 driver manages its PHYs itself so the USB core PHY management needs to be disabled.
Use the struct
xhci_plat_privhack added by commits46034a999c07("usb:
host: xhci-plat: add platform data support") andf768e718911e("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
Quoted 47 lines
Fixes 4e88d4c08301 ("usb: add a flag to skip PHY initialization tostruct
usb_hcd")Fixes 178a0bce05cb ("usb: core: hcd: integrate the PHY wrapper intothe HCD core")
Tested-by Matthias Kaehlcke <[email protected]>Cc stable <[email protected]>Reviewed-by Matthias Kaehlcke <[email protected]>Signed-off-by Johan Hovold <[email protected]>Signed-off-by Greg Kroah-Hartman <[email protected]>[ johan: adjust context to 5.15 ]Signed-off-by Johan Hovold <[email protected]>--- drivers/usb/dwc3/host.c | 10 ++++++++++ 1 file changed, 10 insertions(+)drivers/usb/dwc3/host.c +10
-0@@ -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)
Re: [PATCH stable-5.15 3/3] usb: dwc3: disable USB core PHY management
Quoted 24 lines · Stefan Agner
Hi Johan,
Quoted 11 lines · Johan Hovold
From: Johan Hovold <[email protected]>
commit
6000b8d900cd5f52fbcd0776d0cc396e88c8c2eaupstream.The dwc3 driver manages its PHYs itself so the USB core PHY management needs to be disabled.
Use the struct
xhci_plat_privhack added by commits46034a999c07("usb:
host: xhci-plat: add platform data support") andf768e718911e("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.
Quoted 5 lines
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
drivers/usb/dwc3/core.c +4 -0
@@ -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)
Re: [PATCH stable-5.15 3/3] usb: dwc3: disable USB core PHY management
Hi Johan,
Quoted 44 lines · Johan Hovold
Quoted 24 lines · Stefan Agner
Hi Johan,
Quoted 11 lines · Johan Hovold
From: Johan Hovold <[email protected]>
commit
6000b8d900cd5f52fbcd0776d0cc396e88c8c2eaupstream.The dwc3 driver manages its PHYs itself so the USB core PHY management needs to be disabled.
Use the struct
xhci_plat_privhack added by commits46034a999c07("usb:
host: xhci-plat: add platform data support") andf768e718911e("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.
Quoted 5 lines
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")
Quoted 7 lines
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
drivers/usb/dwc3/core.c +0 -0
@@ -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
Re: [PATCH stable-5.15 3/3] usb: dwc3: disable USB core PHY management
Quoted 27 lines · Stefan Agner
Quoted 26 lines · Johan Hovold
Quoted 8 lines · Stefan Agner
Quoted 7 lines · Johan Hovold
From: Johan Hovold <[email protected]>
commit
6000b8d900cd5f52fbcd0776d0cc396e88c8c2eaupstream.The dwc3 driver manages its PHYs itself so the USB core PHY management needs to be disabled.
Quoted 7 lines
Quoted 4 lines
Use the struct
xhci_plat_privhack added by commits46034a999c07("usb:
host: xhci-plat: add platform data support") andf768e718911e("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.
Quoted 7 lines
Quoted 4 lines
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.
Quoted 2 lines
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
Re: [PATCH stable-5.15 3/3] usb: dwc3: disable USB core PHY management
Quoted 36 lines · Johan Hovold
Quoted 27 lines · Stefan Agner
Quoted 26 lines · Johan Hovold
Quoted 8 lines · Stefan Agner
Quoted 7 lines · Johan Hovold
From: Johan Hovold <[email protected]>
commit
6000b8d900cd5f52fbcd0776d0cc396e88c8c2eaupstream.The dwc3 driver manages its PHYs itself so the USB core PHY management needs to be disabled.
Quoted 7 lines
Quoted 4 lines
Use the struct
xhci_plat_privhack added by commits46034a999c07("usb:
host: xhci-plat: add platform data support") andf768e718911e("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.
Quoted 7 lines
Quoted 4 lines
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)
Quoted 6 lines
Looks good to me.
Quoted 2 lines
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
Re: [PATCH stable-5.15 3/3] usb: dwc3: disable USB core PHY management
Quoted 34 lines · Stefan Agner
Quoted 33 lines · Johan Hovold
Quoted 32 lines · Stefan Agner
Quoted 30 lines · Johan Hovold
Quoted 8 lines · Stefan Agner
Quoted 7 lines · Johan Hovold
From: Johan Hovold <[email protected]>
commit
6000b8d900cd5f52fbcd0776d0cc396e88c8c2eaupstream.The dwc3 driver manages its PHYs itself so the USB core PHY management needs to be disabled.
Quoted 7 lines
Quoted 4 lines
Use the struct
xhci_plat_privhack added by commits46034a999c07("usb:
host: xhci-plat: add platform data support") andf768e718911e("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?
Quoted 5 lines
Quoted 2 lines
Quoted 2 lines
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
drivers/usb/dwc3/core.c +5 -0
@@ -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);
Re: [PATCH stable-5.15 3/3] usb: dwc3: disable USB core PHY management
Quoted 14 lines · Johan Hovold
Quoted 2 lines · Stefan Agner
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 PHYcalibrate()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.
Quoted 5 lines
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
Quoted 23 lines
drivers/usb/dwc3/core.c +5
-0@@ -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);
Re: [PATCH stable-5.15 3/3] usb: dwc3: disable USB core PHY management
Hi Johan,
Quoted 13 lines · Johan Hovold
Quoted 12 lines · Johan Hovold
Quoted 2 lines · Stefan Agner
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 PHYcalibrate()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.
Quoted 7 lines
Quoted 5 lines
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.
Quoted 18 lines
Johan
Quoted 15 lines
drivers/usb/dwc3/core.c +2
-0@@ -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);
Quoted 8 lines
drivers/usb/dwc3/core.c static int dwc3_core_init_mode(struct dwc3 *dwc) · line 1401
Quoted 8 lines
drivers/usb/dwc3/core.c +3
-0if (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
Signature · 2 lines
Marek Szyprowski, PhD Samsung R&D Institute Poland
Re: [PATCH stable-5.15 3/3] usb: dwc3: disable USB core PHY management
Quoted 20 lines · Marek Szyprowski
Hi Johan,
Quoted 17 lines · Johan Hovold
Quoted 16 lines · Johan Hovold
Quoted 2 lines · Stefan Agner
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 PHYcalibrate()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.
Quoted 8 lines
Quoted 3 lines
Quoted 1 line
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_hcdwere 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
Re: [PATCH stable-5.15 3/3] usb: dwc3: disable USB core PHY management
In reply to Stefan AgnerDear All,
Quoted 23 lines · Stefan Agner
Hi Johan,
Quoted 11 lines · Johan Hovold
From: Johan Hovold <[email protected]>
commit
6000b8d900cd5f52fbcd0776d0cc396e88c8c2eaupstream.The dwc3 driver manages its PHYs itself so the USB core PHY management needs to be disabled.
Use the struct
xhci_plat_privhack added by commits46034a999c07("usb:
host: xhci-plat: add platform data support") andf768e718911e("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.
Quoted 5 lines
Quoted 1 line
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
Signature · 2 lines
Marek Szyprowski, PhD Samsung R&D Institute Poland