Re: [PATCH -next] thunderbolt: Add quirk to reset host interface on DMA path teardown for AMD USB4 routers
Basavaraj Natikar <[email protected]> Wed, 5 Aug 2026 16:40:43 +0530
| Newsgroups | org.kernel.vger.linux-usb |
|---|---|
| Message-ID | <[email protected]> |
On 8/5/2026 9:19 AM, Mika Westerberg wrote: > 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 <[email protected]> >> Signed-off-by: Sanath S <[email protected]> >> Signed-off-by: Basavaraj Natikar <[email protected]> >> --- >> 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.). Sure Mika, I'll address both in v2: - move the quirk to the NHI - set nhi->quirks in nhi_pci_check_quirks() by PCI ID, following the QUIRK_AUTO_CLEAR_INT convention. - move the helper declaration to nhi.h (out of tb.h). Will send v2 shortly. Thanks, -- Basavaraj > >> >> #ifdef CONFIG_ACPI >> bool tb_acpi_add_links(struct tb_nhi *nhi); >> -- >> 2.34.1