* [PATCH] usb: xhci: only set D3hot for pci device @ 2019-11-12 7:18 Henry Lin 2019-11-13 1:49 ` [PATCH v2] " Henry Lin 0 siblings, 1 reply; 9+ messages in thread From: Henry Lin @ 2019-11-12 7:18 UTC (permalink / raw) Cc: Henry Lin, Mathias Nyman, Greg Kroah-Hartman, linux-usb, linux-kernel xhci driver cannot call pci_set_power_state() on non-pci xhci host controllers. Signed-off-by: Henry Lin <henryl@nvidia.com> --- drivers/usb/host/xhci.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index 6c17e3fe181a..1fc16763dcda 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -761,6 +761,8 @@ static void xhci_stop(struct usb_hcd *hcd) mutex_unlock(&xhci->mutex); } +extern struct device_type pci_dev_type; + /* * Shutdown HC (not bus-specific) * @@ -791,8 +793,12 @@ 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) { + struct device *dev = hcd->self.sysdev; + + if (dev->type == &pci_dev_type) + pci_set_power_state(to_pci_dev(dev), PCI_D3hot); + } } #ifdef CONFIG_PM -- 2.17.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v2] usb: xhci: only set D3hot for pci device 2019-11-12 7:18 [PATCH] usb: xhci: only set D3hot for pci device Henry Lin @ 2019-11-13 1:49 ` Henry Lin 2019-11-15 15:05 ` Mathias Nyman 2019-11-19 8:16 ` [PATCH v3] " Henry Lin 0 siblings, 2 replies; 9+ messages in thread From: Henry Lin @ 2019-11-13 1:49 UTC (permalink / raw) Cc: hch, Henry Lin, Mathias Nyman, Greg Kroah-Hartman, linux-usb, linux-kernel 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(-) 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 -- 2.17.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2] usb: xhci: only set D3hot for pci device 2019-11-13 1:49 ` [PATCH v2] " Henry Lin @ 2019-11-15 15:05 ` Mathias Nyman 2019-11-19 7:46 ` Henry Lin 2019-11-19 8:16 ` [PATCH v3] " Henry Lin 1 sibling, 1 reply; 9+ messages in thread From: Mathias Nyman @ 2019-11-15 15:05 UTC (permalink / raw) To: Henry Lin; +Cc: hch, Mathias Nyman, Greg Kroah-Hartman, linux-usb, linux-kernel 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] usb: xhci: only set D3hot for pci device 2019-11-15 15:05 ` Mathias Nyman @ 2019-11-19 7:46 ` Henry Lin 0 siblings, 0 replies; 9+ messages in thread From: Henry Lin @ 2019-11-19 7:46 UTC (permalink / raw) To: Mathias Nyman Cc: hch, Mathias Nyman, Greg Kroah-Hartman, linux-usb, linux-kernel > 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. ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v3] usb: xhci: only set D3hot for pci device 2019-11-13 1:49 ` [PATCH v2] " Henry Lin 2019-11-15 15:05 ` Mathias Nyman @ 2019-11-19 8:16 ` Henry Lin 2019-11-19 14:57 ` Mathias Nyman ` (3 more replies) 1 sibling, 4 replies; 9+ messages in thread From: Henry Lin @ 2019-11-19 8:16 UTC (permalink / raw) Cc: Henry Lin, Mathias Nyman, Greg Kroah-Hartman, linux-usb, linux-kernel 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-pci.c | 13 +++++++++++++ drivers/usb/host/xhci.c | 6 +----- drivers/usb/host/xhci.h | 1 + 3 files changed, 15 insertions(+), 5 deletions(-) diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c index 1e0236e90687..1904ef56f61c 100644 --- a/drivers/usb/host/xhci-pci.c +++ b/drivers/usb/host/xhci-pci.c @@ -519,6 +519,18 @@ static int xhci_pci_resume(struct usb_hcd *hcd, bool hibernated) } #endif /* CONFIG_PM */ +static void xhci_pci_shutdown(struct usb_hcd *hcd) +{ + struct xhci_hcd *xhci = hcd_to_xhci(hcd); + struct pci_dev *pdev = to_pci_dev(hcd->self.controller); + + xhci_shutdown(hcd); + + /* Yet another workaround for spurious wakeups at shutdown with HSW */ + if (xhci->quirks & XHCI_SPURIOUS_WAKEUP) + pci_set_power_state(pdev, PCI_D3hot); +} + /*-------------------------------------------------------------------------*/ /* PCI driver selection metadata; PCI hotplugging uses this */ @@ -554,6 +566,7 @@ static int __init xhci_pci_init(void) #ifdef CONFIG_PM xhci_pci_hc_driver.pci_suspend = xhci_pci_suspend; xhci_pci_hc_driver.pci_resume = xhci_pci_resume; + xhci_pci_hc_driver.shutdown = xhci_pci_shutdown; #endif return pci_register_driver(&xhci_pci_driver); } diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index 6c17e3fe181a..e59346488f64 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -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); } #ifdef CONFIG_PM diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h index f9f88626a57a..973d665052a2 100644 --- a/drivers/usb/host/xhci.h +++ b/drivers/usb/host/xhci.h @@ -2050,6 +2050,7 @@ int xhci_start(struct xhci_hcd *xhci); int xhci_reset(struct xhci_hcd *xhci); int xhci_run(struct usb_hcd *hcd); int xhci_gen_setup(struct usb_hcd *hcd, xhci_get_quirks_t get_quirks); +void xhci_shutdown(struct usb_hcd *hcd); void xhci_init_driver(struct hc_driver *drv, const struct xhci_driver_overrides *over); int xhci_disable_slot(struct xhci_hcd *xhci, u32 slot_id); -- 2.17.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v3] usb: xhci: only set D3hot for pci device 2019-11-19 8:16 ` [PATCH v3] " Henry Lin @ 2019-11-19 14:57 ` Mathias Nyman 2019-11-20 19:16 ` Jack Pham ` (2 subsequent siblings) 3 siblings, 0 replies; 9+ messages in thread From: Mathias Nyman @ 2019-11-19 14:57 UTC (permalink / raw) To: Henry Lin; +Cc: Mathias Nyman, Greg Kroah-Hartman, linux-usb, linux-kernel On 19.11.2019 10.16, 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. > > Signed-off-by: Henry Lin <henryl@nvidia.com> Thanks, looks great. I like this solution. Keeps original code sequence, removes pci specific calls from generic xhci code, and at the same time it fixes the NVIDIA Tegra xHC shutdown issue. Adding to queue -Mathias ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3] usb: xhci: only set D3hot for pci device 2019-11-19 8:16 ` [PATCH v3] " Henry Lin 2019-11-19 14:57 ` Mathias Nyman @ 2019-11-20 19:16 ` Jack Pham [not found] ` <0101016e8a3ed405-a70f7e87-8c4b-4759-910f-9b9753a9dabb-000000@us-west-2.amazonses.com> 2019-11-21 2:07 ` [PATCH v4] " Henry Lin 3 siblings, 0 replies; 9+ messages in thread From: Jack Pham @ 2019-11-20 19:16 UTC (permalink / raw) To: Henry Lin; +Cc: Mathias Nyman, Greg Kroah-Hartman, linux-usb, linux-kernel On Tue, Nov 19, 2019 at 04:16:56PM +0800, Henry Lin wrote: > diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c > index 6c17e3fe181a..e59346488f64 100644 > --- a/drivers/usb/host/xhci.c > +++ b/drivers/usb/host/xhci.c > @@ -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? Jack -- The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project ^ permalink raw reply [flat|nested] 9+ messages in thread
[parent not found: <0101016e8a3ed405-a70f7e87-8c4b-4759-910f-9b9753a9dabb-000000@us-west-2.amazonses.com>]
* Re: [PATCH v3] usb: xhci: only set D3hot for pci device [not found] ` <0101016e8a3ed405-a70f7e87-8c4b-4759-910f-9b9753a9dabb-000000@us-west-2.amazonses.com> @ 2019-11-21 1:53 ` Henry Lin 0 siblings, 0 replies; 9+ messages in thread From: Henry Lin @ 2019-11-21 1:53 UTC (permalink / raw) To: Jack Pham; +Cc: Mathias Nyman, Greg Kroah-Hartman, linux-usb, linux-kernel >> @@ -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. ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v4] usb: xhci: only set D3hot for pci device 2019-11-19 8:16 ` [PATCH v3] " Henry Lin ` (2 preceding siblings ...) [not found] ` <0101016e8a3ed405-a70f7e87-8c4b-4759-910f-9b9753a9dabb-000000@us-west-2.amazonses.com> @ 2019-11-21 2:07 ` Henry Lin 3 siblings, 0 replies; 9+ messages in thread From: Henry Lin @ 2019-11-21 2:07 UTC (permalink / raw) Cc: jackp, Henry Lin, Mathias Nyman, Greg Kroah-Hartman, linux-usb, linux-kernel 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> --- v4: 1. export xhci_shutdown for CONFIG_USB_XHCI_PCI=m --- drivers/usb/host/xhci-pci.c | 13 +++++++++++++ drivers/usb/host/xhci.c | 7 ++----- drivers/usb/host/xhci.h | 1 + 3 files changed, 16 insertions(+), 5 deletions(-) diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c index 1e0236e90687..1904ef56f61c 100644 --- a/drivers/usb/host/xhci-pci.c +++ b/drivers/usb/host/xhci-pci.c @@ -519,6 +519,18 @@ static int xhci_pci_resume(struct usb_hcd *hcd, bool hibernated) } #endif /* CONFIG_PM */ +static void xhci_pci_shutdown(struct usb_hcd *hcd) +{ + struct xhci_hcd *xhci = hcd_to_xhci(hcd); + struct pci_dev *pdev = to_pci_dev(hcd->self.controller); + + xhci_shutdown(hcd); + + /* Yet another workaround for spurious wakeups at shutdown with HSW */ + if (xhci->quirks & XHCI_SPURIOUS_WAKEUP) + pci_set_power_state(pdev, PCI_D3hot); +} + /*-------------------------------------------------------------------------*/ /* PCI driver selection metadata; PCI hotplugging uses this */ @@ -554,6 +566,7 @@ static int __init xhci_pci_init(void) #ifdef CONFIG_PM xhci_pci_hc_driver.pci_suspend = xhci_pci_suspend; xhci_pci_hc_driver.pci_resume = xhci_pci_resume; + xhci_pci_hc_driver.shutdown = xhci_pci_shutdown; #endif return pci_register_driver(&xhci_pci_driver); } diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index 6c17e3fe181a..90aa811165f1 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -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,11 +789,8 @@ 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); } +EXPORT_SYMBOL_GPL(xhci_shutdown); #ifdef CONFIG_PM static void xhci_save_registers(struct xhci_hcd *xhci) diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h index f9f88626a57a..973d665052a2 100644 --- a/drivers/usb/host/xhci.h +++ b/drivers/usb/host/xhci.h @@ -2050,6 +2050,7 @@ int xhci_start(struct xhci_hcd *xhci); int xhci_reset(struct xhci_hcd *xhci); int xhci_run(struct usb_hcd *hcd); int xhci_gen_setup(struct usb_hcd *hcd, xhci_get_quirks_t get_quirks); +void xhci_shutdown(struct usb_hcd *hcd); void xhci_init_driver(struct hc_driver *drv, const struct xhci_driver_overrides *over); int xhci_disable_slot(struct xhci_hcd *xhci, u32 slot_id); -- 2.17.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
end of thread, other threads:[~2019-11-21 2:28 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-12 7:18 [PATCH] usb: xhci: only set D3hot for pci device Henry Lin
2019-11-13 1:49 ` [PATCH v2] " Henry Lin
2019-11-15 15:05 ` Mathias Nyman
2019-11-19 7:46 ` Henry Lin
2019-11-19 8:16 ` [PATCH v3] " Henry Lin
2019-11-19 14:57 ` Mathias Nyman
2019-11-20 19:16 ` Jack Pham
[not found] ` <0101016e8a3ed405-a70f7e87-8c4b-4759-910f-9b9753a9dabb-000000@us-west-2.amazonses.com>
2019-11-21 1:53 ` Henry Lin
2019-11-21 2:07 ` [PATCH v4] " Henry Lin
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 NNTP newsgroup(s).