Linux USB
 help / color / mirror / Atom feed
* [PATCH V3] xhci: Reset TI USB controller on xhci shutdown
@ 2026-10-21 16:06 wenxiong
  2026-10-21 16:16 ` sashiko-bot
  2026-10-28  9:36 ` Michal Pecio
  0 siblings, 2 replies; 5+ messages in thread
From: wenxiong @ 2026-10-21 16:06 UTC (permalink / raw)
  To: mathias.nyman, gregkh
  Cc: linux-usb, linux-kernel, stable, wenxiong, Wen Xiong

From: Wen Xiong <wenxiong@linux.ibm.com>

Fixes kexec boot. Without a hard reset, TI pci usb chips will fail to
initialize in a kexec booted kernel.

Signed-off-by: Wen Xiong <wenxiong@linux.ibm.com>
---
 drivers/usb/host/xhci-pci.c | 4 +++-
 drivers/usb/host/xhci.c     | 8 ++++++++
 drivers/usb/host/xhci.h     | 1 +
 3 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index a8889081ae82..782178da4057 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -479,8 +479,10 @@ static void xhci_pci_quirks(struct device *dev, struct xhci_hcd *xhci)
 	    pdev->device == PCI_DEVICE_ID_ASMEDIA_3042_XHCI)
 		xhci->quirks |= XHCI_RESET_ON_RESUME;
 
-	if (pdev->vendor == PCI_VENDOR_ID_TI && pdev->device == 0x8241)
+	if (pdev->vendor == PCI_VENDOR_ID_TI && pdev->device == 0x8241) {
 		xhci->quirks |= XHCI_LIMIT_ENDPOINT_INTERVAL_7;
+		xhci->quirks |= XHCI_RESET_ON_SHUTDOWN;
+	}
 
 	if ((pdev->vendor == PCI_VENDOR_ID_BROADCOM ||
 	     pdev->vendor == PCI_VENDOR_ID_CAVIUM) &&
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index a9e47e178c28..8e5e25ebb6b5 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -750,6 +750,7 @@ EXPORT_SYMBOL_GPL(xhci_stop);
 void xhci_shutdown(struct usb_hcd *hcd)
 {
 	struct xhci_hcd *xhci = hcd_to_xhci(hcd);
+	int err;
 
 	if (xhci->quirks & XHCI_SPURIOUS_REBOOT)
 		usb_disable_xhci_ports(to_pci_dev(hcd->self.sysdev));
@@ -781,6 +782,13 @@ 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));
+
+	/*
+	 * TI xHCI controllers fail to initialize in a kexec
+	 * kernel without a PCI function reset
+	 */
+	if (err && (xhci->quirks & XHCI_RESET_ON_SHUTDOWN))
+		pci_reset_function_locked(to_pci_dev(hcd->self.sysdev));
 }
 EXPORT_SYMBOL_GPL(xhci_shutdown);
 
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index c7bfa7f028d3..e94a38e5f4ac 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1647,6 +1647,7 @@ struct xhci_hcd {
 #define XHCI_CDNS_SCTX_QUIRK	BIT_ULL(48)
 #define XHCI_ETRON_HOST	BIT_ULL(49)
 #define XHCI_LIMIT_ENDPOINT_INTERVAL_9 BIT_ULL(50)
+#define XHCI_RESET_ON_SHUTDOWN	BIT_ULL(51)
 
 	unsigned int		num_active_eps;
 	unsigned int		limit_active_eps;
-- 
2.52.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH V3] xhci: Reset TI USB controller on xhci shutdown
  2026-10-21 16:06 [PATCH V3] xhci: Reset TI USB controller on xhci shutdown wenxiong
@ 2026-10-21 16:16 ` sashiko-bot
  2026-10-28  9:36 ` Michal Pecio
  1 sibling, 0 replies; 5+ messages in thread
From: sashiko-bot @ 2026-10-21 16:16 UTC (permalink / raw)
  To: wenxiong; +Cc: linux-usb

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] xhci: uninitialized variable err in xhci_shutdown()
- [Low] xhci: build failure in xhci_shutdown() when CONFIG_PCI is disabled
--

commit b61fa6ad71541414d2813b27997006fef547aa43
Author: Wen Xiong <wenxiong@linux.ibm.com>

xhci: Reset TI USB controller on xhci shutdown

This commit adds an XHCI_RESET_ON_SHUTDOWN quirk for TI 0x8241 xHCI PCI
controllers and calls pci_reset_function_locked() in xhci_shutdown() so the
controller can initialize properly in a kexec-booted kernel.

> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> index a9e47e178c280..8e5e25ebb6b5e 100644
> --- a/drivers/usb/host/xhci.c
> +++ b/drivers/usb/host/xhci.c
> @@ -750,6 +750,7 @@ EXPORT_SYMBOL_GPL(xhci_stop);
>  void xhci_shutdown(struct usb_hcd *hcd)
>  {
>  	struct xhci_hcd *xhci = hcd_to_xhci(hcd);
> +	int err;
>  
>  	if (xhci->quirks & XHCI_SPURIOUS_REBOOT)
>  		usb_disable_xhci_ports(to_pci_dev(hcd->self.sysdev));
> @@ -781,6 +782,13 @@ 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));
> +
> +	/*
> +	 * TI xHCI controllers fail to initialize in a kexec
> +	 * kernel without a PCI function reset
> +	 */
> +	if (err && (xhci->quirks & XHCI_RESET_ON_SHUTDOWN))

[Severity: High]
Is err used uninitialized here in xhci_shutdown() in drivers/usb/host/xhci.c?

In xhci_shutdown(), err is declared on the stack without an initializer and is
never assigned a value before being checked in:

	if (err && (xhci->quirks & XHCI_RESET_ON_SHUTDOWN))
		pci_reset_function_locked(to_pci_dev(hcd->self.sysdev));

When system shutdown or a kexec reboot runs:

  device_shutdown()
    -> pci_device_shutdown()
      -> usb_hcd_pci_shutdown()
        -> xhci_pci_shutdown()
          -> xhci_shutdown()

this reads uninitialized stack memory (and triggers an uninitialized variable
warning at build time), and when stack variables are zero-initialized,
pci_reset_function_locked() will be skipped completely.

Should err be dropped so the condition only checks xhci->quirks &
XHCI_RESET_ON_SHUTDOWN?

> +		pci_reset_function_locked(to_pci_dev(hcd->self.sysdev));

[Severity: Low]
Will building drivers/usb/host/xhci.c fail when CONFIG_USB_XHCI_HCD is enabled
and CONFIG_PCI is disabled?

In include/linux/pci.h, pci_reset_function_locked() is only declared inside
the #ifdef CONFIG_PCI block and does not have a stub when CONFIG_PCI is
disabled:

  #ifdef CONFIG_PCI
  ...
  int __pci_reset_function_locked(struct pci_dev *dev);
  int pci_reset_function(struct pci_dev *dev);
  int pci_reset_function_locked(struct pci_dev *dev);

Since drivers/usb/host/xhci.c can be built without CONFIG_PCI, calling
pci_reset_function_locked() directly in xhci_shutdown() causes an implicit
function declaration error on non-PCI configurations.

Would it be better to perform this PCI function reset in xhci_pci_shutdown()
in drivers/usb/host/xhci-pci.c alongside the existing PCI-specific shutdown
handling?

>  }
>  EXPORT_SYMBOL_GPL(xhci_shutdown);

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261021160608.8229-1-wenxiong@linux.ibm.com?part=1

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH V3] xhci: Reset TI USB controller on xhci shutdown
  2026-10-21 16:06 [PATCH V3] xhci: Reset TI USB controller on xhci shutdown wenxiong
  2026-10-21 16:16 ` sashiko-bot
@ 2026-10-28  9:36 ` Michal Pecio
  2026-10-28 15:41   ` Wen Xiong
  2026-10-28 20:50   ` Wen Xiong
  1 sibling, 2 replies; 5+ messages in thread
From: Michal Pecio @ 2026-10-28  9:36 UTC (permalink / raw)
  To: wenxiong, mathias.nyman, Bjorn Helgaas
  Cc: gregkh, linux-usb, linux-kernel, stable, wenxiong, linux-pci

On Wed, 21 Oct 2026 12:06:08 -0400, wenxiong@linux.ibm.com wrote:
> From: Wen Xiong <wenxiong@linux.ibm.com>
> 
> Fixes kexec boot. Without a hard reset, TI pci usb chips will fail to
> initialize in a kexec booted kernel.
> 
> Signed-off-by: Wen Xiong <wenxiong@linux.ibm.com>

[ Adding linux-pci and Bjorn Helgaas ]

I tried to make this code a bit more "proper", by moving the
pci_reset_function_locked() call to xhci-pci.c from xhci.c.

I found that when I actually run this, I get a warning from PCI:

  unlocked secondary bus reset via: __pci_reset_function_locked

Bjorn, any tips what would be a proper way to whack a malfunctioning
PCI device with a heavy hammer to ensure it will work after kexec?
Seems we have pci_reset_function(), which locks the parent bridge for
us, but it also preserves some "state". Do we want that?

I'm also starting to wonder if we shouldn't do this reset after kexec,
to cover for kdump. Or maybe both, because shutdown() is supposed to
quiesce the chip and keep it from doing DMA, and we have no idea what
such chip is doing (if anything) when it refuses to halt.

Thanks,
Michal

> [...]
> +	/*
> +	 * TI xHCI controllers fail to initialize in a kexec
> +	 * kernel without a PCI function reset
> +	 */
> +	if (err && (xhci->quirks & XHCI_RESET_ON_SHUTDOWN))
> +		pci_reset_function_locked(to_pci_dev(hcd->self.sysdev));
> [...]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH V3] xhci: Reset TI USB controller on xhci shutdown
  2026-10-28  9:36 ` Michal Pecio
@ 2026-10-28 15:41   ` Wen Xiong
  2026-10-28 20:50   ` Wen Xiong
  1 sibling, 0 replies; 5+ messages in thread
From: Wen Xiong @ 2026-10-28 15:41 UTC (permalink / raw)
  To: Michal Pecio
  Cc: mathias.nyman, Bjorn Helgaas, gregkh, linux-usb, linux-kernel,
	stable, wenxiong, linux-pci


> 
> I tried to make this code a bit more "proper", by moving the
> pci_reset_function_locked() call to xhci-pci.c from xhci.c.
> 
> I found that when I actually run this, I get a warning from PCI:
> 
>   unlocked secondary bus reset via: __pci_reset_function_locked
> 

Thanks for your reviewing!
Can we do?
if device is TI/0x8241 device, we just reset it in xhci-pci.c.
We don't need to bother other USB devices.

Thanks,
Wendy
> Bjorn, any tips what would be a proper way to whack a malfunctioning
> PCI device with a heavy hammer to ensure it will work after kexec?
> Seems we have pci_reset_function(), which locks the parent bridge for
> us, but it also preserves some "state". Do we want that?
> 
> I'm also starting to wonder if we shouldn't do this reset after kexec,
> to cover for kdump. Or maybe both, because shutdown() is supposed to
> quiesce the chip and keep it from doing DMA, and we have no idea what
> such chip is doing (if anything) when it refuses to halt.
> 
> Thanks,
> Michal
> 
>> [...]
>> +	/*
>> +	 * TI xHCI controllers fail to initialize in a kexec
>> +	 * kernel without a PCI function reset
>> +	 */
>> +	if (err && (xhci->quirks & XHCI_RESET_ON_SHUTDOWN))
>> +		pci_reset_function_locked(to_pci_dev(hcd->self.sysdev));
>> [...]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH V3] xhci: Reset TI USB controller on xhci shutdown
  2026-10-28  9:36 ` Michal Pecio
  2026-10-28 15:41   ` Wen Xiong
@ 2026-10-28 20:50   ` Wen Xiong
  1 sibling, 0 replies; 5+ messages in thread
From: Wen Xiong @ 2026-10-28 20:50 UTC (permalink / raw)
  To: Michal Pecio
  Cc: mathias.nyman, Bjorn Helgaas, gregkh, linux-usb, linux-kernel,
	stable, wenxiong, linux-pci


> I tried to make this code a bit more "proper", by moving the
> pci_reset_function_locked() call to xhci-pci.c from xhci.c.
> 
> I found that when I actually run this, I get a warning from PCI:
> 
>   unlocked secondary bus reset via: __pci_reset_function_locked
> 

Hi Michal,

How about we add check TI/failling usb controller in xhci_halt and 
xhci_pci_shutdown?

Thanks,
Wendy
diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index a8889081ae82..af6ca5447f22 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -950,6 +950,10 @@ static void xhci_pci_shutdown(struct usb_hcd *hcd)
         /* Yet another workaround for spurious wakeups at shutdown with 
HSW */
         if (xhci->quirks & XHCI_SPURIOUS_WAKEUP)
                 pci_set_power_state(pdev, PCI_D3hot);
+
+       /* TI XHCI controllers do not come back after kexec without this 
hack */
+       if (xhci->quirks & XHCI_HALT_FAIL_TO_RESET)
+               pci_reset_function_locked(pdev);
  }
  
/*-------------------------------------------------------------------------*/
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.cW
index a9e47e178c28..67108692a354 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -120,6 +120,8 @@ void xhci_quiesce(struct xhci_hcd *xhci)
   */
  int xhci_halt(struct xhci_hcd *xhci)
  {
+       struct usb_hcd *hcd = xhci_to_hcd(xhci);
+       struct pci_dev *pdev = to_pci_dev(hcd->self.controller);
         int ret;

         xhci_dbg_trace(xhci, trace_xhci_dbg_init, "// Halt the HC");
@@ -130,6 +132,11 @@ int xhci_halt(struct xhci_hcd *xhci)
         if (ret) {
                 if (!(xhci->xhc_state & XHCI_STATE_DYING))
                         xhci_warn(xhci, "Host halt failed, %d\n", ret);
+
+               if (pdev->vendor == PCI_VENDOR_ID_TI &&
+                               pdev->device == 0x8241)
+                       xhci->quirks |= XHCI_HALT_FAIL_TO_RESET;
+
                 return ret;
         }



^ permalink raw reply related	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-10-28 20:50 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-21 16:06 [PATCH V3] xhci: Reset TI USB controller on xhci shutdown wenxiong
2026-10-21 16:16 ` sashiko-bot
2026-10-28  9:36 ` Michal Pecio
2026-10-28 15:41   ` Wen Xiong
2026-10-28 20:50   ` Wen Xiong

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox