Re: [PATCH -next] thunderbolt: Add quirk to reset host interface on DMA path teardown for AMD USB4 routers

[Mika Westerberg <mika.westerberg@linux.intel.com>]

Hi,

On Tue, Aug 04, 2026 at 05:56:38PM +0530, Basavaraj Natikar wrote:
> Some AMD USB4 host routers have a bug in the Host Interface where
> DMA path setup and teardown cycles may cause the Tx ring to hang.
> 
> Fix this by issuing a Host Interface Reset on every DMA path teardown
> for affected routers. The Host Interface Reset brings the registers in
> the memory BAR to their default state and clears the End-to-End Flow
> Control state, preventing the hang condition.
> 
> Co-developed-by: Sanath S <Sanath.S@amd.com>
> Signed-off-by: Sanath S <Sanath.S@amd.com>
> Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
> ---
>  drivers/thunderbolt/nhi.c      | 48 ++++++++++++++++++++++++++++++++++
>  drivers/thunderbolt/nhi_regs.h |  4 +++
>  drivers/thunderbolt/quirks.c   | 16 ++++++++++++
>  drivers/thunderbolt/tb.c       |  7 +++++
>  drivers/thunderbolt/tb.h       |  3 +++
>  5 files changed, 78 insertions(+)
> 
> diff --git a/drivers/thunderbolt/nhi.c b/drivers/thunderbolt/nhi.c
> index 383a36212f70..0865cab582e7 100644
> --- a/drivers/thunderbolt/nhi.c
> +++ b/drivers/thunderbolt/nhi.c
> @@ -1160,6 +1160,54 @@ static void nhi_reset(struct tb_nhi *nhi)
>  	dev_warn(nhi->dev, "timeout resetting host router\n");
>  }
>  
> +/**
> + * nhi_host_interface_reset() - Issue a host interface reset
> + * @nhi: Pointer to the NHI structure
> + *
> + * Resets the Host Interface by setting the RST bit in the Host Interface
> + * Reset register. This brings the registers in the memory BAR to their
> + * default state and clears the End-to-End Flow Control state.
> + *
> + * The caller must ensure that the control channel (Ring 0) is stopped
> + * before calling this function, since the reset clears ring state.
> + * The caller is responsible for restarting Ring 0 afterward.
> + *
> + * After setting the RST bit, waits for tHIReset (10 ms) for the reset
> + * to complete.
> + */
> +static void nhi_host_interface_reset(struct tb_nhi *nhi)
> +{
> +	struct device *dev = nhi->dev;
> +	u32 val;
> +
> +	val = ioread32(nhi->iobase + REG_CAPS);
> +	/* Host Interface Reset only applies to Ver. 1 routers */
> +	if (FIELD_GET(REG_CAPS_VERSION_MASK, val) >= REG_CAPS_VERSION_2)
> +		return;
> +
> +	dev_dbg(dev, "issuing host interface reset\n");
> +
> +	iowrite32(REG_HOST_INTERFACE_RESET_RST,
> +		  nhi->iobase + REG_HOST_INTERFACE_RESET);
> +
> +	/* Wait for tHIReset (10 ms) for the reset to complete */
> +	usleep_range(10000, 20000);
> +}
> +
> +/**
> + * tb_nhi_host_interface_reset() - Reset host interface with control channel
> + * @tb: Pointer to the thunderbolt domain
> + *
> + * Stops the control channel, issues a Host Interface Reset, and restarts
> + * the control channel.
> + */
> +void tb_nhi_host_interface_reset(struct tb *tb)
> +{
> +	tb_ctl_stop(tb->ctl);
> +	nhi_host_interface_reset(tb->nhi);
> +	tb_ctl_start(tb->ctl);
> +}
> +
>  static struct tb *nhi_select_cm(struct tb_nhi *nhi)
>  {
>  	struct tb *tb;
> diff --git a/drivers/thunderbolt/nhi_regs.h b/drivers/thunderbolt/nhi_regs.h
> index d6a197fabc74..99df60b6db36 100644
> --- a/drivers/thunderbolt/nhi_regs.h
> +++ b/drivers/thunderbolt/nhi_regs.h
> @@ -115,6 +115,10 @@ struct ring_desc {
>  #define REG_CAPS_VERSION_MASK		GENMASK(23, 16)
>  #define REG_CAPS_VERSION_2		0x40
>  
> +/* Host Interface Reset - resets TX/RX rings and E2E flow control counters */
> +#define REG_HOST_INTERFACE_RESET	0x39858
> +#define REG_HOST_INTERFACE_RESET_RST	BIT(0)
> +
>  #define REG_DMA_MISC			0x39864
>  #define REG_DMA_MISC_INT_AUTO_CLEAR     BIT(2)
>  #define REG_DMA_MISC_DISABLE_AUTO_CLEAR	BIT(17)
> diff --git a/drivers/thunderbolt/quirks.c b/drivers/thunderbolt/quirks.c
> index 9f7914ac2f48..cc04d0873931 100644
> --- a/drivers/thunderbolt/quirks.c
> +++ b/drivers/thunderbolt/quirks.c
> @@ -52,6 +52,12 @@ static void quirk_block_rpm_in_redrive(struct tb_switch *sw)
>  	tb_sw_dbg(sw, "preventing runtime PM in DP redrive mode\n");
>  }
>  
> +static void quirk_host_interface_reset(struct tb_switch *sw)
> +{
> +	sw->quirks |= QUIRK_HOST_INTERFACE_RESET;
> +	tb_sw_dbg(sw, "enabling host interface reset on DMA path teardown\n");
> +}
> +
>  struct tb_quirk {
>  	u16 hw_vendor_id;
>  	u16 hw_device_id;
> @@ -114,6 +120,16 @@ static const struct tb_quirk tb_quirks[] = {
>  	{ 0x0438, 0x0209, 0x0000, 0x0000, quirk_clx_disable },
>  	{ 0x0438, 0x020a, 0x0000, 0x0000, quirk_clx_disable },
>  	{ 0x0438, 0x020b, 0x0000, 0x0000, quirk_clx_disable },
> +	/*
> +	 * AMD USB4 host routers may hang the Tx ring after repeated
> +	 * DMA path teardowns. Issue a Host Interface Reset on each
> +	 * teardown to prevent the hang.
> +	 */
> +	{ 0x0438, 0x020d, 0x0000, 0x0000, quirk_host_interface_reset },
> +	{ 0x0438, 0x020e, 0x0000, 0x0000, quirk_host_interface_reset },
> +	{ 0x0438, 0x020f, 0x0000, 0x0000, quirk_host_interface_reset },
> +	{ 0x0438, 0x0210, 0x0000, 0x0000, quirk_host_interface_reset },
> +	{ 0x0438, 0x0211, 0x0000, 0x0000, quirk_host_interface_reset },

These quirks are for things inside USB4 fabric. Since this is the HI, we
have quirks for those in pci.c/nhi.c like QUIRK_AUTO_CLEAR_INT. Can you
make this one follow that convention instead?

>  };
>  
>  /**
> diff --git a/drivers/thunderbolt/tb.c b/drivers/thunderbolt/tb.c
> index 47753a5c0f2e..d40cc9e57364 100644
> --- a/drivers/thunderbolt/tb.c
> +++ b/drivers/thunderbolt/tb.c
> @@ -2395,6 +2395,13 @@ static void __tb_disconnect_xdomain_paths(struct tb *tb, struct tb_xdomain *xd,
>  	 * the same host router USB4 downstream port.
>  	 */
>  	tb_enable_clx(sw);
> +
> +	/*
> +	 * Some host routers may hang the Tx ring after DMA path teardowns.
> +	 * Issue a Host Interface Reset to prevent it.
> +	 */
> +	if (tb->root_switch->quirks & QUIRK_HOST_INTERFACE_RESET)
> +		tb_nhi_host_interface_reset(tb);
>  }
>  
>  static int tb_disconnect_xdomain_paths(struct tb *tb, struct tb_xdomain *xd,
> diff --git a/drivers/thunderbolt/tb.h b/drivers/thunderbolt/tb.h
> index 4373336d9425..d21feb631f3e 100644
> --- a/drivers/thunderbolt/tb.h
> +++ b/drivers/thunderbolt/tb.h
> @@ -26,6 +26,8 @@
>  #define QUIRK_NO_CLX					BIT(1)
>  /* Need to keep power on while USB4 port is in redrive mode */
>  #define QUIRK_KEEP_POWER_IN_DP_REDRIVE			BIT(2)
> +/* Reset Host Interface on DMA path teardown to prevent Tx ring hang */
> +#define QUIRK_HOST_INTERFACE_RESET			BIT(3)
>  
>  /**
>   * struct tb_nvm - Structure holding NVM information
> @@ -1507,6 +1509,7 @@ static inline bool usb4_port_device_is_offline(const struct usb4_port *usb4)
>  }
>  
>  void tb_check_quirks(struct tb_switch *sw);
> +void tb_nhi_host_interface_reset(struct tb *tb);

Also we don't need to expose this then (and if it needs to be exposed it
belongs to nhi.h not here.).

>  
>  #ifdef CONFIG_ACPI
>  bool tb_acpi_add_links(struct tb_nhi *nhi);
> -- 
> 2.34.1