patches-7.0: replace cdnsp-sky1 shutdown SError mitigations with host teardown - #37
patches-7.0: replace cdnsp-sky1 shutdown SError mitigations with host teardown#37npmccallum wants to merge 1 commit into
Conversation
… teardown
Remove 0061-DPTSW-24991 and 0062-DPTSW-25423 and replace them with
0061-usb-cdns3-cdnsp-sky1-tear-down-host-on-shutdown.patch.
cdnsp_sky1_shutdown() gates the USB clocks while the child xHCI host and
its devices are still live above the controller. If anything touches the
controller's registers after the clocks are gated, the access raises a
fatal asynchronous SError that hangs shutdown/reboot. Three distinct paths
can do this:
1. the gadget (cdnsp_gadget_pullup) - addressed by 0061 (release the
gadget driver);
2. the xHCI IRQ handler (xhci_irq reading USBSTS) - addressed by 0062
(disable_irq + clear HCD_FLAG_HW_ACCESSIBLE);
3. a host device's async error recovery - not covered by either. The
panic we captured (via netconsole) came from usb-storage ->
xhci_urb_dequeue: an attached mass-storage device's SCSI
error-recovery thread resetting its port and dequeuing URBs touches
xHCI MMIO after the clocks are gated. 0061 (gadget) and 0062 (IRQ)
do not stop this thread, so the hang still reproduces with both.
Instead of a third targeted mitigation, route ->shutdown() through the
existing ->remove() path, which calls of_platform_depopulate() to remove
the xHCI host first - disconnecting its devices and stopping all of their
work (gadget, IRQ, and async recovery) - before the resets are asserted
and the clocks gated. One patch supersedes both mitigations and covers the
host async-recovery path they miss.
Tested: 13/13 consecutive `systemctl reboot` with an attached composite USB
device (HID + mass-storage), zero SErrors on every teardown (captured via
netconsole), versus a deterministic hang before; an 8-reboot stress run
was also clean.
|
tested on 7.0.14. Thanks, your patch works. |
|
This change uses of_platform_depopulate, which should not work with ACPI. The current patchset is focusing on ACPI now, DT mode support need further work. |
|
You're right that The fix doesn't depend on it, though. The child xHCI device is already quiesced before the wrapper's shutdown runs, on either firmware:
By the time That's also the bug I'm fixing: 0061 and 0062 both run after that point. Releasing the gadget driver and calling One gap in my own patch, which your comment led me to. 0061 covered the gadget, and unlike the host the gadget has no child device the core tears down first. Under DT, |
Remove 0061-DPTSW-24991 and 0062-DPTSW-25423 and replace them with 0061-usb-cdns3-cdnsp-sky1-tear-down-host-on-shutdown.patch.
cdnsp_sky1_shutdown() gates the USB clocks while the child xHCI host and its devices are still live above the controller. If anything touches the controller's registers after the clocks are gated, the access raises a fatal asynchronous SError that hangs shutdown/reboot. Three distinct paths can do this:
Instead of a third targeted mitigation, route ->shutdown() through the existing ->remove() path, which calls of_platform_depopulate() to remove the xHCI host first - disconnecting its devices and stopping all of their work (gadget, IRQ, and async recovery) - before the resets are asserted and the clocks gated. One patch supersedes both mitigations and covers the host async-recovery path they miss.
Reusing ->remove() for ->shutdown() is an established pattern for USB
dual-role / glue drivers in mainline, for example:
(drivers/usb/dwc3/dwc3-qcom.c)
(drivers/usb/dwc3/dwc3-xilinx.c)
(drivers/usb/chipidea/ci_hdrc_imx.c)