| Message ID | 20191113014927.11915-1-henryl@nvidia.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2] usb: xhci: only set D3hot for pci device | expand |
On 13.11.2019 3.49, Henry Lin wrote: > Xhci driver cannot call pci_set_power_state() on non-pci xhci host > controllers. For example, NVIDIA Tegra XHCI host controller which acts > as platform device with XHCI_SPURIOUS_WAKEUP quirk set in some platform > hits this issue during shutdown. > The XHCI_SPURIOUS_WAKEUP quirk both resets the controller at shutdown and sets it to PCI D3 at remove/shutdown. Is it so that the NVIDIA Tegra xHC just needs to be reset at shutdown? The quirk is not set for Tegra xHC in current mainline kernel. > Signed-off-by: Henry Lin <henryl@nvidia.com> > --- > drivers/usb/host/xhci.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c > index 6c17e3fe181a..61718b126d2b 100644 > --- a/drivers/usb/host/xhci.c > +++ b/drivers/usb/host/xhci.c > @@ -791,8 +791,11 @@ static void xhci_shutdown(struct usb_hcd *hcd) > readl(&xhci->op_regs->status)); > > /* Yet another workaround for spurious wakeups at shutdown with HSW */ > - if (xhci->quirks & XHCI_SPURIOUS_WAKEUP) > - pci_set_power_state(to_pci_dev(hcd->self.sysdev), PCI_D3hot); > + if (xhci->quirks & XHCI_SPURIOUS_WAKEUP) { > + if (dev_is_pci(hcd->self.sysdev)) > + pci_set_power_state(to_pci_dev(hcd->self.sysdev), > + PCI_D3hot); > + } Agree that we shouldn't call PCI specific functions in the generic shutdown code. Would be better if we could move the PCI parts to xhci-pci.c or hcd-pci.c Maybe we need to add a xhci_pci_shutdown() function for the xhci pci driver that could take care of the pci related shutdown quirks before calling usb_hcd_pci_shutdown(), and call that instead of directly calling usb_hcd_pci_shutdown(). -Mathias
> The XHCI_SPURIOUS_WAKEUP quirk both resets the controller at shutdown > and sets it to PCI D3 at remove/shutdown. > Is it so that the NVIDIA Tegra xHC just needs to be reset at shutdown? > The quirk is not set for Tegra xHC in current mainline kernel. Some versions of NVIDIA Tegra xHC support VF on virtualization environment. In that case, they need reset at shutdown. For now, Tegra xHC's VF support has not been submitted to mainline kernel. > Agree that we shouldn't call PCI specific functions in the generic shutdown code. > Would be better if we could move the PCI parts to xhci-pci.c or hcd-pci.c > Maybe we need to add a xhci_pci_shutdown() function for the xhci pci driver > that could take care of the pci related shutdown quirks before calling > usb_hcd_pci_shutdown(), and call that instead of directly calling > usb_hcd_pci_shutdown(). To keep original code sequence, I implemented this in a similar way to set xhci_pci_shutdown() as hc_driver.shutdown(). In xhci_pci_shutdown(), it calls xhci_shutdown() first and then does XHCI_SPURIOUS_WAKEUP quirk. Will post it as v3 patch.
>> @@ -770,7 +770,7 @@ static void xhci_stop(struct usb_hcd *hcd) >> * >> * This will only ever be called with the main usb_hcd (the USB3 roothub). >> */ >> -static void xhci_shutdown(struct usb_hcd *hcd) >> +void xhci_shutdown(struct usb_hcd *hcd) >> { >> struct xhci_hcd *xhci = hcd_to_xhci(hcd); >> >> @@ -789,10 +789,6 @@ static void xhci_shutdown(struct usb_hcd *hcd) >> xhci_dbg_trace(xhci, trace_xhci_dbg_init, >> "xhci_shutdown completed - status = %x", >> readl(&xhci->op_regs->status)); >> - >> - /* Yet another workaround for spurious wakeups at shutdown with HSW */ >> - if (xhci->quirks & XHCI_SPURIOUS_WAKEUP) >> - pci_set_power_state(to_pci_dev(hcd->self.sysdev), PCI_D3hot); >> } >Shouldn't this function also now need to be EXPORTed? Yes. I will add EXPORT_SYMBOL_GPL() for it.
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index 6c17e3fe181a..61718b126d2b 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -791,8 +791,11 @@ static void xhci_shutdown(struct usb_hcd *hcd) readl(&xhci->op_regs->status)); /* Yet another workaround for spurious wakeups at shutdown with HSW */ - if (xhci->quirks & XHCI_SPURIOUS_WAKEUP) - pci_set_power_state(to_pci_dev(hcd->self.sysdev), PCI_D3hot); + if (xhci->quirks & XHCI_SPURIOUS_WAKEUP) { + if (dev_is_pci(hcd->self.sysdev)) + pci_set_power_state(to_pci_dev(hcd->self.sysdev), + PCI_D3hot); + } } #ifdef CONFIG_PM
Xhci driver cannot call pci_set_power_state() on non-pci xhci host controllers. For example, NVIDIA Tegra XHCI host controller which acts as platform device with XHCI_SPURIOUS_WAKEUP quirk set in some platform hits this issue during shutdown. Signed-off-by: Henry Lin <henryl@nvidia.com> --- drivers/usb/host/xhci.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)