Linux PCI subsystem development
 help / color / mirror / Atom feed
* Re: [RFC][PATCH] thunderbolt: Quiesce AMD NHI during suspend
       [not found] <20260706023422.3647320-1-acelan.kao@canonical.com>
@ 2026-07-08 15:34 ` Mario Limonciello
  2026-07-08 18:37   ` Mario Limonciello
  0 siblings, 1 reply; 2+ messages in thread
From: Mario Limonciello @ 2026-07-08 15:34 UTC (permalink / raw)
  To: Chia-Lin Kao (AceLan), Andreas Noever, Mika Westerberg,
	Yehezkel Bernat
  Cc: Yo-Jung Leo Lin, linux-usb, linux-kernel, S, Sanath,
	Natikar, Basavaraj, open list:PCI SUBSYSTEM

++ linux-pci
++ Sanath && Basavaraj

On 7/5/26 21:34, Chia-Lin Kao (AceLan) wrote:
> On some AMD USB4 systems, resuming from system suspend with a
> Thunderbolt dock in the picture hard-hangs the machine very early in the
> resume path, before the Thunderbolt driver gets a chance to rescan and
> re-establish the topology. It is a PCIe bus wedge: the system never
> reaches the point where the thunderbolt module could recover the dock,
> and the kernel journal does not persist past "PM: suspend entry
> (s2idle)".
> 
> Reproduce:
> 
>    1. Affected platform: an AMD USB4 host with the thunderbolt driver
>       bound to its NHI PCI functions. Observed on a Dell Pro 14 Plus
>       (PB14255, AMD Phoenix) which exposes two USB4 NHI functions
>       (0000:c6:00.5 and 0000:c6:00.6).
> 
>    2. Put the system into s2idle (Low Power S0 Idle / Modern Standby)
>       with no dock attached.
> 
>    3. While the system is suspended, plug in a Dell SD25TB4 Thunderbolt 4
>       dock (Goshen Ridge). The plug event wakes the system, and the dock's
>       downstream PCIe bridges are enumerated behind the AMD USB4 PCIe
>       tunnel during the resume.
> 
>    4. The machine hard-hangs during the noirq resume phase, before the
>       thunderbolt driver re-establishes the topology. The Dell power
>       indicator flashes the "3A5W" code (3 amber + 5 white lights).
> 
>    Removing the trigger makes resume reliable: unbinding the two NHI PCI
>    functions, or unloading the thunderbolt module, before suspend both
>    avoid the hang completely. The difference in those cases is the PCI
>    state the core restores for the driver-bound NHI on resume.
> 
> EC team findings (from the platform EC log):
> 
> The EC/PD/dock side is healthy across the failing cycle, which points
> the finger at the host resume path rather than the dock:
> 
>    - Modern Standby entry and resume are seen on the EC side; the dock
>      plug interrupt during MS is handled and the full power sequence
>      recovers (SLP_S3# okay, PLTRST# received, U8-U20 sequence completed).
> 
>    - The EC-to-dock DDM command exchange fully succeeds: all CMsd/CMgd
>      transactions return success (device type, TBT cable type, dock
>      module config for USB/TBT/Audio/LAN).
> 
>    - PD/Type-C renegotiation is normal (power contracts up to 95W/100W);
>      no persistent protocol error appears in the log.
> 
>    - The only suspicious symptom is host-side: the EC repeatedly logs
>      PmSt_Timeout_Count while waiting for the BIOS/host to consume the
>      wake (PWRB pressed) event. That indicates the host resume path is
>      late or stuck, not an EC-to-dock transport failure. The Dell "3A5W"
>      diagnostic LED code (3 amber + 5 white) maps in the EC code to an
>      SLP_S3#/PLTRST#/RUNPWROK failure indication.
> 
> Root cause:
> 
> With the driver bound, the PCI core restores the saved NHI config space
> during the noirq resume phase, including PCI_COMMAND with memory space
> and bus mastering enabled. This is the same phase in which the core
> restores the config space of the tunneled PCI bridges and whatever sits
> behind them (the dock). If the AMD host router is made active (memory +
> bus mastering) while the downstream USB4 PCIe tunnel to the dock is still
> being restored and is not yet ready, the host interface starts bus
> activity toward a not-yet-ready endpoint and the PCIe hierarchy wedges.
> 
> This matches the workarounds: unbinding the NHI or unloading the module
> means the core does not restore an active NHI over the tunnel during
> noirq, and the hang disappears. Several Thunderbolt-side attempts did not
> help (returning early from nhi_resume_noirq(), disabling host-router wake
> flags, adding an unconditional delay before tunnel activation), which is
> why the actionable boundary is the PCI PM state of the driver-bound AMD
> NHI rather than a later Thunderbolt protocol step. We do not have
> visibility into what the host router does internally in that window; that
> uncertainty is the main reason for sending this as an RFC (see open
> questions below).
> 
> What this patch does:
> 
> Add a QUIRK_QUIESCE_ON_SUSPEND quirk, currently set for all AMD USB4 host
> routers (matched by vendor AMD + USB4 PCI class), and:
> 
>    - clear PCI_COMMAND_MEMORY and PCI_COMMAND_MASTER before the PCI core
>      saves the NHI config space in runtime and system suspend, so the NHI
>      is restored inactive on resume;
> 
>    - keep the NHI quiescent across nhi_resume_noirq() (return early without
>      touching the controller), so it stays inactive while the core
>      restores the tunneled bridges and the dock;
> 
>    - once the whole system has resumed, re-enable memory space and bus
>      mastering and bring the domain back up from the PM complete callback
>      via tb_domain_resume_noirq(), and from nhi_runtime_resume() for the
>      runtime PM path.
> 
> The result is that Thunderbolt/USB4 stays functional after resume instead
> of the machine hanging or the tunnels staying down until the module is
> reloaded.
> 
> Test results:
> 
> Tested on the Dell Pro 14 Plus (PB14255, AMD Phoenix, two USB4 host
> routers) with the Dell SD25TB4 dock. Without the patch, the sequence
> above hard-hangs during resume. With the patch, repeated suspend/resume
> cycles with the dock connected complete cleanly with no hang. After
> resume the NHI PCI_COMMAND reads back with MEMORY|MASTER set again, and
> the dock's downstream USB devices re-enumerate, confirming the USB tunnel
> is restored and the domain is functional.
> 
> Caveats observed while the NHI is quiescent across noirq: config space
> reads to the host router time out and TMU configuration restore fails
> during that window; the controller and domain are fully brought back in
> nhi_complete(). The DP tunnel restore path was not exercised in this
> test, as no external display was driven through the dock during the
> tested cycle.
> 
> Open questions for RFC discussion:
> 
>    1. Root cause: what does the AMD host router actually do when it is
>       restored with memory + bus mastering enabled during noirq while the
>       downstream tunnel is not ready? Is this an expected ordering
>       constraint, a firmware behavior, or a hardware erratum?
> 
>    2. Scope: the quirk currently matches all AMD USB4 host routers by
>       class. Which parts are actually affected? Should this be narrowed
>       (e.g. by DMI or specific device IDs) instead of applied to all AMD
>       USB4?
> 
>    3. Mechanism: is clearing both PCI_COMMAND_MEMORY and PCI_COMMAND_MASTER
>       the right lever, or would clearing bus mastering alone suffice and be
>       less invasive? Is poking PCI_COMMAND directly acceptable in the NHI
>       driver, or should this move into a PCI backend op / PCI quirk?
> 
> Signed-off-by: Chia-Lin Kao (AceLan) <acelan.kao@canonical.com>

I have no idea why I didn't receive this patch in my mailbox - I'm 
obviously CC'ed.  But I checked my spam and it's not there either.

But anyway, I am wondering if the real issue here is that we're 
accessing the hardware before it's ready.  If you can readily reproduce 
the issue, can you try this series to see if it helps?

https://lore.kernel.org/linux-pci/20260708152650.536604-1-mario.limonciello@amd.com/

> ---
>   drivers/thunderbolt/nhi.c | 78 +++++++++++++++++++++++++++++++++++++--
>   drivers/thunderbolt/nhi.h |  5 ++-
>   drivers/thunderbolt/pci.c |  8 ++++
>   3 files changed, 85 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/thunderbolt/nhi.c b/drivers/thunderbolt/nhi.c
> index 698fb124d529..787185c745ae 100644
> --- a/drivers/thunderbolt/nhi.c
> +++ b/drivers/thunderbolt/nhi.c
> @@ -16,6 +16,7 @@
>   #include <linux/interrupt.h>
>   #include <linux/iommu.h>
>   #include <linux/module.h>
> +#include <linux/pci.h>
>   #include <linux/delay.h>
>   #include <linux/property.h>
>   #include <linux/string_choices.h>
> @@ -40,6 +41,31 @@ static bool host_reset = true;
>   module_param(host_reset, bool, 0444);
>   MODULE_PARM_DESC(host_reset, "reset USB4 host router (default: true)");
>   
> +static bool nhi_quirk_quiesce_on_suspend(const struct tb_nhi *nhi)
> +{
> +	return nhi->quirks & QUIRK_QUIESCE_ON_SUSPEND;
> +}
> +
> +static void nhi_disable_pci_mem_master(struct tb_nhi *nhi)
> +{
> +	struct pci_dev *pdev = to_pci_dev(nhi->dev);
> +	u16 cmd;
> +
> +	pci_read_config_word(pdev, PCI_COMMAND, &cmd);
> +	cmd &= ~(PCI_COMMAND_MEMORY | PCI_COMMAND_MASTER);
> +	pci_write_config_word(pdev, PCI_COMMAND, cmd);
> +}
> +
> +static void nhi_enable_pci_mem_master(struct tb_nhi *nhi)
> +{
> +	struct pci_dev *pdev = to_pci_dev(nhi->dev);
> +	u16 cmd;
> +
> +	pci_read_config_word(pdev, PCI_COMMAND, &cmd);
> +	cmd |= PCI_COMMAND_MEMORY | PCI_COMMAND_MASTER;
> +	pci_write_config_word(pdev, PCI_COMMAND, cmd);
> +}
> +
>   static int ring_interrupt_index(const struct tb_ring *ring)
>   {
>   	int bit = ring->hop;
> @@ -988,6 +1014,9 @@ static int __nhi_suspend_noirq(struct device *dev, bool wakeup)
>   			return ret;
>   	}
>   
> +	if (nhi_quirk_quiesce_on_suspend(nhi))
> +		nhi_disable_pci_mem_master(nhi);
> +
>   	return 0;
>   }
>   
> @@ -1038,6 +1067,18 @@ static int nhi_resume_noirq(struct device *dev)
>   	struct tb_nhi *nhi = tb->nhi;
>   	int ret;
>   
> +	/*
> +	 * If the NHI was quiesced on suspend keep it quiescent (bus
> +	 * mastering and memory access disabled) across the noirq phase.
> +	 * The PCI core restores the config space of the tunneled PCI
> +	 * bridges and anything behind them (e.g. a dock) here; letting the
> +	 * AMD host router become active now can wedge the PCIe bus before
> +	 * the dock side is ready. The controller and domain are brought
> +	 * back up in nhi_complete() once the whole system has resumed.
> +	 */
> +	if (nhi_quirk_quiesce_on_suspend(nhi))
> +		return 0;
> +
>   	/*
>   	 * Check that the device is still there. It may be that the user
>   	 * unplugged last device which causes the host controller to go
> @@ -1064,16 +1105,33 @@ static int nhi_suspend(struct device *dev)
>   static void nhi_complete(struct device *dev)
>   {
>   	struct tb *tb = dev_get_drvdata(dev);
> +	struct tb_nhi *nhi = tb->nhi;
>   
>   	/*
>   	 * If we were runtime suspended when system suspend started,
>   	 * schedule runtime resume now. It should bring the domain back
> -	 * to functional state.
> +	 * to functional state (and undo the quiesce, if any, via
> +	 * nhi_runtime_resume()).
>   	 */
> -	if (pm_runtime_suspended(dev))
> +	if (pm_runtime_suspended(dev)) {
>   		pm_runtime_resume(dev);
> -	else
> -		tb_domain_complete(tb);
> +		return;
> +	}
> +
> +	/*
> +	 * A quiesced NHI (see __nhi_suspend_noirq()) was intentionally
> +	 * left with bus mastering and memory access disabled and its
> +	 * domain was not resumed in nhi_resume_noirq(). Now that the whole
> +	 * system has resumed and the PCI core has finished restoring the
> +	 * tunneled bridges, re-enable the controller and bring the domain
> +	 * back up so Thunderbolt/USB4 keeps working after resume.
> +	 */
> +	if (nhi_quirk_quiesce_on_suspend(nhi)) {
> +		nhi_enable_pci_mem_master(nhi);
> +		tb_domain_resume_noirq(tb);
> +	}
> +
> +	tb_domain_complete(tb);
>   }
>   
>   static int nhi_runtime_suspend(struct device *dev)
> @@ -1091,6 +1149,10 @@ static int nhi_runtime_suspend(struct device *dev)
>   		if (ret)
>   			return ret;
>   	}
> +
> +	if (nhi_quirk_quiesce_on_suspend(nhi))
> +		nhi_disable_pci_mem_master(nhi);
> +
>   	return 0;
>   }
>   
> @@ -1100,6 +1162,14 @@ static int nhi_runtime_resume(struct device *dev)
>   	struct tb_nhi *nhi = tb->nhi;
>   	int ret;
>   
> +	/*
> +	 * The NHI is quiesced (bus mastering and memory access disabled)
> +	 * across runtime suspend as well; re-enable it before resuming the
> +	 * domain.
> +	 */
> +	if (nhi_quirk_quiesce_on_suspend(nhi))
> +		nhi_enable_pci_mem_master(nhi);
> +
>   	if (nhi->ops->runtime_resume) {
>   		ret = nhi->ops->runtime_resume(nhi);
>   		if (ret)
> diff --git a/drivers/thunderbolt/nhi.h b/drivers/thunderbolt/nhi.h
> index d488eadadfce..c8b973150434 100644
> --- a/drivers/thunderbolt/nhi.h
> +++ b/drivers/thunderbolt/nhi.h
> @@ -119,8 +119,9 @@ struct tb_nhi_ops {
>   #define PCI_CLASS_SERIAL_USB_USB4			0x0c0340
>   
>   /* Host interface quirks */
> -#define QUIRK_AUTO_CLEAR_INT	BIT(0)
> -#define QUIRK_E2E		BIT(1)
> +#define QUIRK_AUTO_CLEAR_INT		BIT(0)
> +#define QUIRK_E2E			BIT(1)
> +#define QUIRK_QUIESCE_ON_SUSPEND	BIT(2)
>   
>   /*
>    * Minimal number of vectors when we use MSI-X. Two for control channel
> diff --git a/drivers/thunderbolt/pci.c b/drivers/thunderbolt/pci.c
> index dbb6badda867..4cf8107f1ce8 100644
> --- a/drivers/thunderbolt/pci.c
> +++ b/drivers/thunderbolt/pci.c
> @@ -43,6 +43,14 @@ static void nhi_pci_check_quirks(struct tb_nhi_pci *nhi_pci)
>   	struct tb_nhi *nhi = &nhi_pci->nhi;
>   	struct pci_dev *pdev = to_pci_dev(nhi->dev);
>   
> +	/*
> +	 * AMD USB4 host routers may otherwise become active too early during
> +	 * resume after the PCI core restores the saved COMMAND register.
> +	 */
> +	if (pdev->vendor == PCI_VENDOR_ID_AMD &&
> +	    pdev->class == PCI_CLASS_SERIAL_USB_USB4)
> +		nhi->quirks |= QUIRK_QUIESCE_ON_SUSPEND;
> +
>   	if (pdev->vendor == PCI_VENDOR_ID_INTEL) {
>   		/*
>   		 * Intel hardware supports auto clear of the interrupt


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

* Re: [RFC][PATCH] thunderbolt: Quiesce AMD NHI during suspend
  2026-07-08 15:34 ` [RFC][PATCH] thunderbolt: Quiesce AMD NHI during suspend Mario Limonciello
@ 2026-07-08 18:37   ` Mario Limonciello
  0 siblings, 0 replies; 2+ messages in thread
From: Mario Limonciello @ 2026-07-08 18:37 UTC (permalink / raw)
  To: Chia-Lin Kao (AceLan), Andreas Noever, Mika Westerberg,
	Yehezkel Bernat
  Cc: Yo-Jung Leo Lin, linux-usb, linux-kernel, S, Sanath,
	Natikar, Basavaraj, open list:PCI SUBSYSTEM

Some comments about your changes too.

On 7/8/26 10:34, Mario Limonciello wrote:
> ++ linux-pci
> ++ Sanath && Basavaraj
> 
> On 7/5/26 21:34, Chia-Lin Kao (AceLan) wrote:
>> On some AMD USB4 systems, resuming from system suspend with a
>> Thunderbolt dock in the picture hard-hangs the machine very early in the
>> resume path, before the Thunderbolt driver gets a chance to rescan and
>> re-establish the topology. It is a PCIe bus wedge: the system never
>> reaches the point where the thunderbolt module could recover the dock,
>> and the kernel journal does not persist past "PM: suspend entry
>> (s2idle)".
>>
>> Reproduce:
>>
>>    1. Affected platform: an AMD USB4 host with the thunderbolt driver
>>       bound to its NHI PCI functions. Observed on a Dell Pro 14 Plus
>>       (PB14255, AMD Phoenix) which exposes two USB4 NHI functions
>>       (0000:c6:00.5 and 0000:c6:00.6).
>>
>>    2. Put the system into s2idle (Low Power S0 Idle / Modern Standby)
>>       with no dock attached.
>>
>>    3. While the system is suspended, plug in a Dell SD25TB4 Thunderbolt 4
>>       dock (Goshen Ridge). The plug event wakes the system, and the 
>> dock's
>>       downstream PCIe bridges are enumerated behind the AMD USB4 PCIe
>>       tunnel during the resume.
>>
>>    4. The machine hard-hangs during the noirq resume phase, before the
>>       thunderbolt driver re-establishes the topology. The Dell power
>>       indicator flashes the "3A5W" code (3 amber + 5 white lights).
>>
>>    Removing the trigger makes resume reliable: unbinding the two NHI PCI
>>    functions, or unloading the thunderbolt module, before suspend both
>>    avoid the hang completely. The difference in those cases is the PCI
>>    state the core restores for the driver-bound NHI on resume.
>>
>> EC team findings (from the platform EC log):
>>
>> The EC/PD/dock side is healthy across the failing cycle, which points
>> the finger at the host resume path rather than the dock:
>>
>>    - Modern Standby entry and resume are seen on the EC side; the dock
>>      plug interrupt during MS is handled and the full power sequence
>>      recovers (SLP_S3# okay, PLTRST# received, U8-U20 sequence 
>> completed).
>>
>>    - The EC-to-dock DDM command exchange fully succeeds: all CMsd/CMgd
>>      transactions return success (device type, TBT cable type, dock
>>      module config for USB/TBT/Audio/LAN).
>>
>>    - PD/Type-C renegotiation is normal (power contracts up to 95W/100W);
>>      no persistent protocol error appears in the log.
>>
>>    - The only suspicious symptom is host-side: the EC repeatedly logs
>>      PmSt_Timeout_Count while waiting for the BIOS/host to consume the
>>      wake (PWRB pressed) event. That indicates the host resume path is
>>      late or stuck, not an EC-to-dock transport failure. The Dell "3A5W"
>>      diagnostic LED code (3 amber + 5 white) maps in the EC code to an
>>      SLP_S3#/PLTRST#/RUNPWROK failure indication.
>>
>> Root cause:
>>
>> With the driver bound, the PCI core restores the saved NHI config space
>> during the noirq resume phase, including PCI_COMMAND with memory space
>> and bus mastering enabled. This is the same phase in which the core
>> restores the config space of the tunneled PCI bridges and whatever sits
>> behind them (the dock). If the AMD host router is made active (memory +
>> bus mastering) while the downstream USB4 PCIe tunnel to the dock is still
>> being restored and is not yet ready, the host interface starts bus
>> activity toward a not-yet-ready endpoint and the PCIe hierarchy wedges.
>>
>> This matches the workarounds: unbinding the NHI or unloading the module
>> means the core does not restore an active NHI over the tunnel during
>> noirq, and the hang disappears. Several Thunderbolt-side attempts did not
>> help (returning early from nhi_resume_noirq(), disabling host-router wake
>> flags, adding an unconditional delay before tunnel activation), which is
>> why the actionable boundary is the PCI PM state of the driver-bound AMD
>> NHI rather than a later Thunderbolt protocol step. We do not have
>> visibility into what the host router does internally in that window; that
>> uncertainty is the main reason for sending this as an RFC (see open
>> questions below).
>>
>> What this patch does:
>>
>> Add a QUIRK_QUIESCE_ON_SUSPEND quirk, currently set for all AMD USB4 host
>> routers (matched by vendor AMD + USB4 PCI class), and:
>>
>>    - clear PCI_COMMAND_MEMORY and PCI_COMMAND_MASTER before the PCI core
>>      saves the NHI config space in runtime and system suspend, so the NHI
>>      is restored inactive on resume;
>>
>>    - keep the NHI quiescent across nhi_resume_noirq() (return early 
>> without
>>      touching the controller), so it stays inactive while the core
>>      restores the tunneled bridges and the dock;
>>
>>    - once the whole system has resumed, re-enable memory space and bus
>>      mastering and bring the domain back up from the PM complete callback
>>      via tb_domain_resume_noirq(), and from nhi_runtime_resume() for the
>>      runtime PM path.
>>
>> The result is that Thunderbolt/USB4 stays functional after resume instead
>> of the machine hanging or the tunnels staying down until the module is
>> reloaded.
>>
>> Test results:
>>
>> Tested on the Dell Pro 14 Plus (PB14255, AMD Phoenix, two USB4 host
>> routers) with the Dell SD25TB4 dock. Without the patch, the sequence
>> above hard-hangs during resume. With the patch, repeated suspend/resume
>> cycles with the dock connected complete cleanly with no hang. After
>> resume the NHI PCI_COMMAND reads back with MEMORY|MASTER set again, and
>> the dock's downstream USB devices re-enumerate, confirming the USB tunnel
>> is restored and the domain is functional.
>>
>> Caveats observed while the NHI is quiescent across noirq: config space
>> reads to the host router time out and TMU configuration restore fails
>> during that window; the controller and domain are fully brought back in
>> nhi_complete(). The DP tunnel restore path was not exercised in this
>> test, as no external display was driven through the dock during the
>> tested cycle.
>>
>> Open questions for RFC discussion:
>>
>>    1. Root cause: what does the AMD host router actually do when it is
>>       restored with memory + bus mastering enabled during noirq while the
>>       downstream tunnel is not ready? Is this an expected ordering
>>       constraint, a firmware behavior, or a hardware erratum?
>>
>>    2. Scope: the quirk currently matches all AMD USB4 host routers by
>>       class. Which parts are actually affected? Should this be narrowed
>>       (e.g. by DMI or specific device IDs) instead of applied to all AMD
>>       USB4?
>>
>>    3. Mechanism: is clearing both PCI_COMMAND_MEMORY and 
>> PCI_COMMAND_MASTER
>>       the right lever, or would clearing bus mastering alone suffice 
>> and be
>>       less invasive? Is poking PCI_COMMAND directly acceptable in the NHI
>>       driver, or should this move into a PCI backend op / PCI quirk?
>>
>> Signed-off-by: Chia-Lin Kao (AceLan) <acelan.kao@canonical.com>
> 
> I have no idea why I didn't receive this patch in my mailbox - I'm 
> obviously CC'ed.  But I checked my spam and it's not there either.
> 
> But anyway, I am wondering if the real issue here is that we're 
> accessing the hardware before it's ready.  If you can readily reproduce 
> the issue, can you try this series to see if it helps?
> 
> https://lore.kernel.org/linux-pci/20260708152650.536604-1- 
> mario.limonciello@amd.com/
> 
>> ---
>>   drivers/thunderbolt/nhi.c | 78 +++++++++++++++++++++++++++++++++++++--
>>   drivers/thunderbolt/nhi.h |  5 ++-
>>   drivers/thunderbolt/pci.c |  8 ++++
>>   3 files changed, 85 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/thunderbolt/nhi.c b/drivers/thunderbolt/nhi.c
>> index 698fb124d529..787185c745ae 100644
>> --- a/drivers/thunderbolt/nhi.c
>> +++ b/drivers/thunderbolt/nhi.c
>> @@ -16,6 +16,7 @@
>>   #include <linux/interrupt.h>
>>   #include <linux/iommu.h>
>>   #include <linux/module.h>
>> +#include <linux/pci.h>
>>   #include <linux/delay.h>
>>   #include <linux/property.h>
>>   #include <linux/string_choices.h>
>> @@ -40,6 +41,31 @@ static bool host_reset = true;
>>   module_param(host_reset, bool, 0444);
>>   MODULE_PARM_DESC(host_reset, "reset USB4 host router (default: true)");
>> +static bool nhi_quirk_quiesce_on_suspend(const struct tb_nhi *nhi)
>> +{
>> +    return nhi->quirks & QUIRK_QUIESCE_ON_SUSPEND;
>> +}
>> +
>> +static void nhi_disable_pci_mem_master(struct tb_nhi *nhi)
>> +{
>> +    struct pci_dev *pdev = to_pci_dev(nhi->dev);
>> +    u16 cmd;
>> +
>> +    pci_read_config_word(pdev, PCI_COMMAND, &cmd);
>> +    cmd &= ~(PCI_COMMAND_MEMORY | PCI_COMMAND_MASTER);
>> +    pci_write_config_word(pdev, PCI_COMMAND, cmd);
>> +}
>> +
>> +static void nhi_enable_pci_mem_master(struct tb_nhi *nhi)
>> +{
>> +    struct pci_dev *pdev = to_pci_dev(nhi->dev);
>> +    u16 cmd;
>> +
>> +    pci_read_config_word(pdev, PCI_COMMAND, &cmd);
>> +    cmd |= PCI_COMMAND_MEMORY | PCI_COMMAND_MASTER;
>> +    pci_write_config_word(pdev, PCI_COMMAND, cmd);
>> +}
>> +

Is this basically an open coded version of pci_set_master() and 
pci_clear_master()?

Or is the PCI_COMMAND_MEMORY actually important for this issue?

>>   static int ring_interrupt_index(const struct tb_ring *ring)
>>   {
>>       int bit = ring->hop;
>> @@ -988,6 +1014,9 @@ static int __nhi_suspend_noirq(struct device 
>> *dev, bool wakeup)
>>               return ret;
>>       }
>> +    if (nhi_quirk_quiesce_on_suspend(nhi))
>> +        nhi_disable_pci_mem_master(nhi);
>> +

Isn't this going to break wakeup interrupts?

>>       return 0;
>>   }
>> @@ -1038,6 +1067,18 @@ static int nhi_resume_noirq(struct device *dev)
>>       struct tb_nhi *nhi = tb->nhi;
>>       int ret;
>> +    /*
>> +     * If the NHI was quiesced on suspend keep it quiescent (bus
>> +     * mastering and memory access disabled) across the noirq phase.
>> +     * The PCI core restores the config space of the tunneled PCI
>> +     * bridges and anything behind them (e.g. a dock) here; letting the
>> +     * AMD host router become active now can wedge the PCIe bus before
>> +     * the dock side is ready. The controller and domain are brought
>> +     * back up in nhi_complete() once the whole system has resumed.
>> +     */
>> +    if (nhi_quirk_quiesce_on_suspend(nhi))
>> +        return 0;
>> +
>>       /*
>>        * Check that the device is still there. It may be that the user
>>        * unplugged last device which causes the host controller to go
>> @@ -1064,16 +1105,33 @@ static int nhi_suspend(struct device *dev)
>>   static void nhi_complete(struct device *dev)
>>   {
>>       struct tb *tb = dev_get_drvdata(dev);
>> +    struct tb_nhi *nhi = tb->nhi;
>>       /*
>>        * If we were runtime suspended when system suspend started,
>>        * schedule runtime resume now. It should bring the domain back
>> -     * to functional state.
>> +     * to functional state (and undo the quiesce, if any, via
>> +     * nhi_runtime_resume()).
>>        */
>> -    if (pm_runtime_suspended(dev))
>> +    if (pm_runtime_suspended(dev)) {
>>           pm_runtime_resume(dev);
>> -    else
>> -        tb_domain_complete(tb);
>> +        return;
>> +    }
>> +
>> +    /*
>> +     * A quiesced NHI (see __nhi_suspend_noirq()) was intentionally
>> +     * left with bus mastering and memory access disabled and its
>> +     * domain was not resumed in nhi_resume_noirq(). Now that the whole
>> +     * system has resumed and the PCI core has finished restoring the
>> +     * tunneled bridges, re-enable the controller and bring the domain
>> +     * back up so Thunderbolt/USB4 keeps working after resume.
>> +     */
>> +    if (nhi_quirk_quiesce_on_suspend(nhi)) {
>> +        nhi_enable_pci_mem_master(nhi);
>> +        tb_domain_resume_noirq(tb);
>> +    }
>> +
>> +    tb_domain_complete(tb);
>>   }
>>   static int nhi_runtime_suspend(struct device *dev)
>> @@ -1091,6 +1149,10 @@ static int nhi_runtime_suspend(struct device *dev)
>>           if (ret)
>>               return ret;
>>       }
>> +
>> +    if (nhi_quirk_quiesce_on_suspend(nhi))
>> +        nhi_disable_pci_mem_master(nhi);
>> +
>>       return 0;
>>   }
>> @@ -1100,6 +1162,14 @@ static int nhi_runtime_resume(struct device *dev)
>>       struct tb_nhi *nhi = tb->nhi;
>>       int ret;
>> +    /*
>> +     * The NHI is quiesced (bus mastering and memory access disabled)
>> +     * across runtime suspend as well; re-enable it before resuming the
>> +     * domain.
>> +     */
>> +    if (nhi_quirk_quiesce_on_suspend(nhi))
>> +        nhi_enable_pci_mem_master(nhi);
>> +
>>       if (nhi->ops->runtime_resume) {
>>           ret = nhi->ops->runtime_resume(nhi);
>>           if (ret)
>> diff --git a/drivers/thunderbolt/nhi.h b/drivers/thunderbolt/nhi.h
>> index d488eadadfce..c8b973150434 100644
>> --- a/drivers/thunderbolt/nhi.h
>> +++ b/drivers/thunderbolt/nhi.h
>> @@ -119,8 +119,9 @@ struct tb_nhi_ops {
>>   #define PCI_CLASS_SERIAL_USB_USB4            0x0c0340
>>   /* Host interface quirks */
>> -#define QUIRK_AUTO_CLEAR_INT    BIT(0)
>> -#define QUIRK_E2E        BIT(1)
>> +#define QUIRK_AUTO_CLEAR_INT        BIT(0)
>> +#define QUIRK_E2E            BIT(1)
>> +#define QUIRK_QUIESCE_ON_SUSPEND    BIT(2)
>>   /*
>>    * Minimal number of vectors when we use MSI-X. Two for control channel
>> diff --git a/drivers/thunderbolt/pci.c b/drivers/thunderbolt/pci.c
>> index dbb6badda867..4cf8107f1ce8 100644
>> --- a/drivers/thunderbolt/pci.c
>> +++ b/drivers/thunderbolt/pci.c
>> @@ -43,6 +43,14 @@ static void nhi_pci_check_quirks(struct tb_nhi_pci 
>> *nhi_pci)
>>       struct tb_nhi *nhi = &nhi_pci->nhi;
>>       struct pci_dev *pdev = to_pci_dev(nhi->dev);
>> +    /*
>> +     * AMD USB4 host routers may otherwise become active too early 
>> during
>> +     * resume after the PCI core restores the saved COMMAND register.
>> +     */
>> +    if (pdev->vendor == PCI_VENDOR_ID_AMD &&
>> +        pdev->class == PCI_CLASS_SERIAL_USB_USB4)
>> +        nhi->quirks |= QUIRK_QUIESCE_ON_SUSPEND;
>> +

If quirks do end up being correct direction thunderbolt quirks are 
applied via drivers/thunderbolt/quirks.c

>>       if (pdev->vendor == PCI_VENDOR_ID_INTEL) {
>>           /*
>>            * Intel hardware supports auto clear of the interrupt
> 


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

end of thread, other threads:[~2026-07-08 18:37 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20260706023422.3647320-1-acelan.kao@canonical.com>
2026-07-08 15:34 ` [RFC][PATCH] thunderbolt: Quiesce AMD NHI during suspend Mario Limonciello
2026-07-08 18:37   ` Mario Limonciello

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