On 24-08-27 13:44:02, Mario Limonciello wrote: > On 8/27/2024 01:32, Peter Chen wrote: > > On 24-07-12 13:54:18, superm1@xxxxxxxxxx wrote: > > > From: Mario Limonciello <mario.limonciello@xxxxxxx> > > > > > > A workaround was put in place for Haswell systems with spurious events > > > to put XHCI controllers into D3hot at shutdown. This solution actually > > > makes sense for all XHCI controllers though because XHCI controllers > > > left in D0 by the OS may remain in D0 when the SoC goes into S5. > > > > > > Explicitly put all XHCI controllers into D3hot at shutdown and when > > > module is unloaded. > > > > > > Signed-off-by: Mario Limonciello <mario.limonciello@xxxxxxx> > > > --- > > > drivers/usb/host/xhci-pci.c | 8 ++------ > > > 1 file changed, 2 insertions(+), 6 deletions(-) > > > > > > diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c > > > index 4408d4caf66d2..dde5e4a210719 100644 > > > --- a/drivers/usb/host/xhci-pci.c > > > +++ b/drivers/usb/host/xhci-pci.c > > > @@ -667,9 +667,7 @@ static void xhci_pci_remove(struct pci_dev *dev) > > > xhci->shared_hcd = NULL; > > > } > > > - /* Workaround for spurious wakeups at shutdown with HSW */ > > > - if (xhci->quirks & XHCI_SPURIOUS_WAKEUP) > > > - pci_set_power_state(dev, PCI_D3hot); > > > + pci_set_power_state(dev, PCI_D3hot); > > > usb_hcd_pci_remove(dev); > > > } > > > @@ -882,9 +880,7 @@ static void xhci_pci_shutdown(struct usb_hcd *hcd) > > > xhci_shutdown(hcd); > > > xhci_cleanup_msix(xhci); > > > - /* Yet another workaround for spurious wakeups at shutdown with HSW */ > > > - if (xhci->quirks & XHCI_SPURIOUS_WAKEUP) > > > - pci_set_power_state(pdev, PCI_D3hot); > > > + pci_set_power_state(pdev, PCI_D3hot); > > > > Hi Mario & Mathias, > > > > According to xHCI spec v1.2: A.1.2 Power State Definitions: > > > > Software shall place each downstream USB port with power > > enabled into the Suspend or Disabled state before it > > attempts to move the xHC out of the D0 power state. > > > > But I have not found any USB core code does it, do you have any ideas > > about it? > > > > We have added the similar codes at non-PCI USB platform, but met above > > concerns. In fact, we met kernel dump that the thread usb-storage try > > to access the port status when the platform xHCI code has already put > > the controller to D3. > > > > Best regards, > > Peter > > > > > > This is pretty tangential to my patch. But FWIW in case you missed we're > going to discard this patch in favor of another approach in PCI core. > > Regarding your point though If I'm not mistaken this should be handled by > the Linux parent/child device model. Each of the ports should be children > of the hub they're connected to and the hub a child of the controller. So > when doing any actions that start runtime PM on the host controller the > children need to first be in runtime PM. > It seems there is no runtime PM suspend for xhci and USB core at .shutdown currently. Alan & Mathias, please correct me if I was wrong. Thanks, Peter
