* [PATCH 0/4] add support platform nhi driver
@ 2026-10-30 8:25 Chunfeng Yun
2026-10-30 8:25 ` [PATCH 1/4] thunderbolt: add " Chunfeng Yun
` (3 more replies)
0 siblings, 4 replies; 13+ messages in thread
From: Chunfeng Yun @ 2026-10-30 8:25 UTC (permalink / raw)
To: Mika Westerberg
Cc: Andreas Noever, Yehezkel Bernat, Matthias Brugger,
Greg Kroah-Hartman, AngeloGioacchino Del Regno, linux-kernel,
linux-usb, linux-arm-kernel, linux-mediatek, Chunfeng Yun
This patch series introduces support for platform bus-based USB4/Thunderbolt
Host Interface (NHI) controllers on embedded SoCs, along with two fixes/
improvements in the Thunderbolt core connection manager.
Historically, the Thunderbolt NHI driver has assumed a PCI bus binding
(pci.c), with tb_domain_init()/exit() tightly coupled to the PCI module
lifecycle. Modern ARM64 SoCs integrate the USB4 NHI controller directly
onto the platform bus, described via Device Tree or ACPI.
This series decouples the core domain initialization from the PCI driver
and adds a dedicated platform driver (plat.c) supporting standard platform
probing, OF/ACPI device tables, runtime PM, and an optional dedicated
out-of-band wakeup IRQ.
Additionally, this series resolves two issues observed during runtime PM
and link training:
1. Premature link-up timeout when bringing up USB3 adapters on links with
multiple retimers.
2. Kernel warnings when authorizing/de-authorizing PCIe tunnels on switches
that have been marked unplugged or lost during suspend.
Patches in this series:
======================
Patch 1: thunderbolt: add platform nhi driver
Decouples tb_domain_init()/exit() using subsys_initcall() and adds plat.c
to support memory-mapped NHI host controllers on the platform bus with
Device Tree (CONFIG_OF) and ACPI (CONFIG_ACPI) match tables.
Patch 2: thunderbolt: add support optional dedicated wakeup irq
Adds support for an optional dedicated wakeup interrupt for platform NHI
controllers using dev_pm_set_dedicated_wake_irq(), allowing out-of-band
wake-up lines from SoC power management units during low-power states.
Patch 3: thunderbolt: improve usb3 adapter link up timeout value
Increases tb_wait_for_port() timeout from 1.0s to 2.5s. In the USB4 spec,
Phase 5 (Link Equalization) alone is permitted up to 1000 ms. When factoring
in Phase 3/4 and retimer transaction delays across multiple retimers, link
training can legitimately take 1.2s to 1.5s+. The previous 1.0s timeout
caused premature aborts and disconnects upon wakeup.
Patch 4: thunderbolt: fix pcie tunnel create/destroy core dump
Checks sw->is_unplugged in tb_switch_set_authorized(), tb_disconnect_pci(),
and tb_tunnel_pci(). When de-authorizing an unplugged/lost switch via sysfs,
this prevents redundant lookups and eliminates false-positive
WARN_ON(!tunnel) stack traces when tunnels have already been destroyed.
Testing:
========
Tested on ARM64 platform hardware with USB4 hubs and downstream PCIe/USB3
devices across plug/unplug, system suspend/resume, and runtime PM transitions,
the kernel boots from UEFI/ACPI.
Chunfeng Yun (4):
thunderbolt: add platform nhi driver
thunderbolt: add support optional dedicated wakeup irq
thunderbolt: improve usb3 adapter link up timeout value
thunderbolt: prevent pcie tunnel activation/teardown warnings
drivers/thunderbolt/Makefile | 2 +-
drivers/thunderbolt/domain.c | 10 +-
drivers/thunderbolt/nhi.c | 2 +
drivers/thunderbolt/nhi.h | 1 -
drivers/thunderbolt/pci.c | 26 +-----
drivers/thunderbolt/plat.c | 171 +++++++++++++++++++++++++++++++++++
drivers/thunderbolt/switch.c | 8 +-
drivers/thunderbolt/tb.c | 7 ++
drivers/thunderbolt/tb.h | 2 -
include/linux/thunderbolt.h | 2 +
10 files changed, 200 insertions(+), 31 deletions(-)
create mode 100644 drivers/thunderbolt/plat.c
--
2.43.0
^ permalink raw reply [flat|nested] 13+ messages in thread* [PATCH 1/4] thunderbolt: add platform nhi driver 2026-10-30 8:25 [PATCH 0/4] add support platform nhi driver Chunfeng Yun @ 2026-10-30 8:25 ` Chunfeng Yun 2026-10-30 8:40 ` sashiko-bot 2026-10-30 8:47 ` Mika Westerberg 2026-10-30 8:25 ` [PATCH 2/4] thunderbolt: add support optional dedicated wakeup irq Chunfeng Yun ` (2 subsequent siblings) 3 siblings, 2 replies; 13+ messages in thread From: Chunfeng Yun @ 2026-10-30 8:25 UTC (permalink / raw) To: Mika Westerberg Cc: Andreas Noever, Yehezkel Bernat, Matthias Brugger, Greg Kroah-Hartman, AngeloGioacchino Del Regno, linux-kernel, linux-usb, linux-arm-kernel, linux-mediatek, Chunfeng Yun Add a new file plat.c to register platform NHI driver for MediaTek mt8901/mt8992 SoCs. Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com> --- drivers/thunderbolt/Makefile | 2 +- drivers/thunderbolt/domain.c | 10 ++- drivers/thunderbolt/nhi.c | 2 + drivers/thunderbolt/nhi.h | 1 - drivers/thunderbolt/pci.c | 26 +----- drivers/thunderbolt/plat.c | 155 +++++++++++++++++++++++++++++++++++ drivers/thunderbolt/tb.h | 2 - include/linux/thunderbolt.h | 1 + 8 files changed, 169 insertions(+), 30 deletions(-) create mode 100644 drivers/thunderbolt/plat.c diff --git a/drivers/thunderbolt/Makefile b/drivers/thunderbolt/Makefile index beb054c3126b..9c941682f3d6 100644 --- a/drivers/thunderbolt/Makefile +++ b/drivers/thunderbolt/Makefile @@ -3,7 +3,7 @@ ccflags-y := -I$(src) obj-${CONFIG_USB4} := thunderbolt.o thunderbolt-objs := nhi.o ctl.o tb.o switch.o cap.o pci.o path.o tunnel.o eeprom.o thunderbolt-objs += domain.o dma_port.o icm.o property.o xdomain.o lc.o tmu.o usb4.o -thunderbolt-objs += usb4_port.o nvm.o retimer.o quirks.o clx.o +thunderbolt-objs += usb4_port.o nvm.o retimer.o quirks.o clx.o plat.o thunderbolt-${CONFIG_ACPI} += acpi.o thunderbolt-$(CONFIG_DEBUG_FS) += debugfs.o diff --git a/drivers/thunderbolt/domain.c b/drivers/thunderbolt/domain.c index 24611f05b3cd..bec7e6ebc4a8 100644 --- a/drivers/thunderbolt/domain.c +++ b/drivers/thunderbolt/domain.c @@ -881,7 +881,7 @@ int tb_domain_unregister_unplugged_xdomains(struct tb *tb) return ctx.n; } -int tb_domain_init(void) +static int __init tb_domain_init(void) { int ret; @@ -907,7 +907,7 @@ int tb_domain_init(void) return ret; } -void tb_domain_exit(void) +static void __exit tb_domain_exit(void) { bus_unregister(&tb_bus_type); ida_destroy(&tb_domain_ida); @@ -917,3 +917,9 @@ void tb_domain_exit(void) tb_debugfs_exit(); tb_configfs_exit(); } + +subsys_initcall(tb_domain_init); +module_exit(tb_domain_exit); + +MODULE_DESCRIPTION("Thunderbolt/USB4 domain driver"); +MODULE_LICENSE("GPL"); diff --git a/drivers/thunderbolt/nhi.c b/drivers/thunderbolt/nhi.c index e99a3fcc4a29..c0f9f1855e8f 100644 --- a/drivers/thunderbolt/nhi.c +++ b/drivers/thunderbolt/nhi.c @@ -1379,8 +1379,10 @@ int nhi_probe(struct tb_nhi *nhi) device_wakeup_enable(dev); pm_runtime_allow(dev); + device_enable_async_suspend(dev); pm_runtime_set_autosuspend_delay(dev, TB_AUTOSUSPEND_DELAY); pm_runtime_use_autosuspend(dev); + pm_runtime_mark_last_busy(dev); pm_runtime_put_autosuspend(dev); return 0; diff --git a/drivers/thunderbolt/nhi.h b/drivers/thunderbolt/nhi.h index 88af9ab9fde7..37c11de167ad 100644 --- a/drivers/thunderbolt/nhi.h +++ b/drivers/thunderbolt/nhi.h @@ -29,7 +29,6 @@ enum nhi_mailbox_cmd { int nhi_mailbox_cmd(struct tb_nhi *nhi, enum nhi_mailbox_cmd cmd, u32 data); enum nhi_fw_mode nhi_mailbox_mode(struct tb_nhi *nhi); -void nhi_enable_int_throttling(struct tb_nhi *nhi); void nhi_disable_interrupts(struct tb_nhi *nhi); void nhi_interrupt_work(struct work_struct *work); irqreturn_t nhi_msi(int irq, void *data); diff --git a/drivers/thunderbolt/pci.c b/drivers/thunderbolt/pci.c index 4408229d8ac7..91d236300703 100644 --- a/drivers/thunderbolt/pci.c +++ b/drivers/thunderbolt/pci.c @@ -715,7 +715,7 @@ static struct pci_device_id nhi_ids[] = { }; MODULE_DEVICE_TABLE(pci, nhi_ids); -MODULE_DESCRIPTION("Thunderbolt/USB4 core driver"); +MODULE_DESCRIPTION("Thunderbolt/USB4 PCI host interface driver"); MODULE_LICENSE("GPL"); static struct pci_driver nhi_driver = { @@ -727,26 +727,4 @@ static struct pci_driver nhi_driver = { .driver.pm = &nhi_pm_ops, }; -static int __init nhi_init(void) -{ - int ret; - - ret = tb_domain_init(); - if (ret) - return ret; - - ret = pci_register_driver(&nhi_driver); - if (ret) - tb_domain_exit(); - - return ret; -} - -static void __exit nhi_unload(void) -{ - pci_unregister_driver(&nhi_driver); - tb_domain_exit(); -} - -rootfs_initcall(nhi_init); -module_exit(nhi_unload); +module_pci_driver(nhi_driver); diff --git a/drivers/thunderbolt/plat.c b/drivers/thunderbolt/plat.c new file mode 100644 index 000000000000..85a00cd391d4 --- /dev/null +++ b/drivers/thunderbolt/plat.c @@ -0,0 +1,155 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Thunderbolt driver - NHI driver (Platform bus specific implementation) + * + * Author: Chunfeng Yun <chunfeng.yun@mediatek.com> + */ + +#include <linux/acpi.h> +#include <linux/interrupt.h> +#include <linux/errno.h> +#include <linux/of.h> +#include <linux/platform_device.h> +#include <linux/pm_runtime.h> +#include <linux/slab.h> + +#include "nhi.h" +#include "tb.h" + +static int nhi_plat_init(struct tb_nhi *nhi) +{ + /* + * avoid device link fail for pcie tunnel + */ + pm_runtime_set_active(nhi->dev); + pm_runtime_get_noresume(nhi->dev); + devm_pm_runtime_enable(nhi->dev); + + return 0; +} + +static irqreturn_t nhi_plat_isr(int irq, void *data) +{ + struct tb_nhi *nhi = data; + schedule_work(&nhi->interrupt_work); + return IRQ_HANDLED; +} + +static int nhi_plat_init_irq(struct tb_nhi *nhi) +{ + int ret; + + INIT_WORK(&nhi->interrupt_work, nhi_interrupt_work); + + ret = devm_request_irq(nhi->dev, nhi->irq, nhi_plat_isr, IRQF_NO_SUSPEND, + dev_name(nhi->dev), nhi); + if (ret) + return dev_err_probe(nhi->dev, ret, "request irq failed\n"); + + return 0; +} + +static void nhi_plat_exit(struct tb_nhi *nhi) +{ + devm_free_irq(nhi->dev, nhi->irq, nhi); + flush_work(&nhi->interrupt_work); + + pm_runtime_put_noidle(nhi->dev); + pm_runtime_disable(nhi->dev); + pm_runtime_set_suspended(nhi->dev); +} + +static const struct tb_nhi_ops nhi_plat_default_ops = { + .init = nhi_plat_init, + .shutdown = nhi_plat_exit, + .init_interrupts = nhi_plat_init_irq, +}; + +static int nhi_plat_probe(struct platform_device *pdev) +{ + struct device *dev = &pdev->dev; + struct tb_nhi *nhi; + + nhi = devm_kzalloc(dev, sizeof(*nhi), GFP_KERNEL); + if (!nhi) + return -ENOMEM; + + nhi->dev = dev; + nhi->ops = &nhi_plat_default_ops; + + nhi->iobase = devm_platform_ioremap_resource(pdev, 0); + if (IS_ERR(nhi->iobase)) + return PTR_ERR(nhi->iobase); + + nhi->irq = platform_get_irq(pdev, 0); + if (nhi->irq < 0) + return dev_err_probe(dev, nhi->irq, "get irq failed\n"); + + return nhi_probe(nhi); +} + +static void nhi_plat_do_remove(struct platform_device *pdev, bool reset) +{ + struct device *dev = &pdev->dev; + struct tb *tb = platform_get_drvdata(pdev); + struct tb_nhi *nhi = tb->nhi; + + /* + * On system shutdown/reboot force a host router reset so the + * CM asserts DPR on connected tbt3 devices before the router tree + * is removed (see tb_stop()). + */ + if (reset) + nhi->host_reset = true; + + pm_runtime_get_sync(dev); + pm_runtime_dont_use_autosuspend(dev); + + tb_domain_remove(tb); + wait_for_completion(&nhi->domain_released); + nhi_shutdown(nhi); +} + +static void nhi_plat_remove(struct platform_device *pdev) +{ + nhi_plat_do_remove(pdev, false); +} + +static void nhi_plat_shutdown(struct platform_device *pdev) +{ + nhi_plat_do_remove(pdev, true); +} + + +#ifdef CONFIG_OF +static const struct of_device_id nhi_plat_of_match[] = { + { .compatible = "thunderbolt-platform" }, + { } +}; +MODULE_DEVICE_TABLE(of, nhi_plat_of_match); +#endif + +#ifdef CONFIG_ACPI +static const struct acpi_device_id nhi_plat_acpi_match[] = { + { "ACPI0015", 0 }, + { } +}; +MODULE_DEVICE_TABLE(acpi, nhi_plat_acpi_match); +#endif + +static struct platform_driver nhi_plat_driver = { + .probe = nhi_plat_probe, + .remove = nhi_plat_remove, + .shutdown = nhi_plat_shutdown, + .driver = { + .name = "thunderbolt", + .of_match_table = of_match_ptr(nhi_plat_of_match), + .acpi_match_table = ACPI_PTR(nhi_plat_acpi_match), + .pm = &nhi_pm_ops, + }, +}; + +module_platform_driver(nhi_plat_driver); +MODULE_ALIAS("platform:thunderbolt"); +MODULE_DESCRIPTION("Thunderbolt/USB4 platform host interface driver"); +MODULE_LICENSE("GPL v2"); diff --git a/drivers/thunderbolt/tb.h b/drivers/thunderbolt/tb.h index e86845762e22..6dfdc47e05d2 100644 --- a/drivers/thunderbolt/tb.h +++ b/drivers/thunderbolt/tb.h @@ -768,8 +768,6 @@ extern const struct device_type tb_retimer_type; extern const struct device_type tb_switch_type; extern const struct device_type usb4_port_device_type; -int tb_domain_init(void); -void tb_domain_exit(void); int tb_xdomain_init(void); void tb_xdomain_exit(void); diff --git a/include/linux/thunderbolt.h b/include/linux/thunderbolt.h index 57502da29080..212c8fea6571 100644 --- a/include/linux/thunderbolt.h +++ b/include/linux/thunderbolt.h @@ -532,6 +532,7 @@ struct tb_nhi { u32 *interrupt_mask; bool going_away; bool iommu_dma_protection; + int irq; struct work_struct interrupt_work; u32 hop_count; unsigned long quirks; -- 2.43.0 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 1/4] thunderbolt: add platform nhi driver 2026-10-30 8:25 ` [PATCH 1/4] thunderbolt: add " Chunfeng Yun @ 2026-10-30 8:40 ` sashiko-bot 2026-10-30 8:47 ` Mika Westerberg 1 sibling, 0 replies; 13+ messages in thread From: sashiko-bot @ 2026-10-30 8:40 UTC (permalink / raw) To: Chunfeng Yun; +Cc: linux-usb Thank you for your contribution! Sashiko AI review found 7 potential issue(s) to consider: - [High] thunderbolt: duplicate module_init() across domain.c, pci.c, and plat.c - [High] thunderbolt: invalid to_pci_dev() cast on platform device in icm_probe() - [High] thunderbolt: missing nhi_shutdown() cleanup on nhi_probe() error paths - [Medium] thunderbolt: double pm_runtime_disable() in nhi_plat_init() and exit - [Low] thunderbolt: missing kernel-doc entry for irq member in struct tb_nhi - [Low] thunderbolt: undocumented DT compatible and redundant calls in plat.c - [Low] thunderbolt: nhi_plat_isr() in plat.c duplicates nhi_msi() in nhi.c -- commit 27d62cb037ec7e4b78f736b27dff7bee6d842f3a Author: Chunfeng Yun <chunfeng.yun@mediatek.com> thunderbolt: add platform nhi driver This commit adds a platform bus NHI driver in drivers/thunderbolt/plat.c and converts domain.c and pci.c to use separate initcall and module_pci_driver registrations. > diff --git a/drivers/thunderbolt/Makefile b/drivers/thunderbolt/Makefile > index beb054c3126b1..9c941682f3d6c 100644 > --- a/drivers/thunderbolt/Makefile > +++ b/drivers/thunderbolt/Makefile > @@ -3,7 +3,7 @@ ccflags-y := -I$(src) > obj-${CONFIG_USB4} := thunderbolt.o > thunderbolt-objs := nhi.o ctl.o tb.o switch.o cap.o pci.o path.o tunnel.o eeprom.o > thunderbolt-objs += domain.o dma_port.o icm.o property.o xdomain.o lc.o tmu.o usb4.o > -thunderbolt-objs += usb4_port.o nvm.o retimer.o quirks.o clx.o > +thunderbolt-objs += usb4_port.o nvm.o retimer.o quirks.o clx.o plat.o > > thunderbolt-${CONFIG_ACPI} += acpi.o > thunderbolt-$(CONFIG_DEBUG_FS) += debugfs.o > diff --git a/drivers/thunderbolt/domain.c b/drivers/thunderbolt/domain.c > index 24611f05b3cda..bec7e6ebc4a88 100644 > --- a/drivers/thunderbolt/domain.c > +++ b/drivers/thunderbolt/domain.c > @@ -881,7 +881,7 @@ int tb_domain_unregister_unplugged_xdomains(struct tb *tb) > return ctx.n; > } > > -int tb_domain_init(void) > +static int __init tb_domain_init(void) > { > int ret; > > @@ -907,7 +907,7 @@ int tb_domain_init(void) > return ret; > } > > -void tb_domain_exit(void) > +static void __exit tb_domain_exit(void) > { > bus_unregister(&tb_bus_type); > ida_destroy(&tb_domain_ida); > @@ -917,3 +917,9 @@ void tb_domain_exit(void) > tb_debugfs_exit(); > tb_configfs_exit(); > } > + > +subsys_initcall(tb_domain_init); > +module_exit(tb_domain_exit); > + > +MODULE_DESCRIPTION("Thunderbolt/USB4 domain driver"); > +MODULE_LICENSE("GPL"); > diff --git a/drivers/thunderbolt/nhi.c b/drivers/thunderbolt/nhi.c > index e99a3fcc4a299..c0f9f1855e8f3 100644 > --- a/drivers/thunderbolt/nhi.c > +++ b/drivers/thunderbolt/nhi.c > @@ -1379,8 +1379,10 @@ int nhi_probe(struct tb_nhi *nhi) > device_wakeup_enable(dev); > > pm_runtime_allow(dev); > + device_enable_async_suspend(dev); > pm_runtime_set_autosuspend_delay(dev, TB_AUTOSUSPEND_DELAY); > pm_runtime_use_autosuspend(dev); > + pm_runtime_mark_last_busy(dev); > pm_runtime_put_autosuspend(dev); > > return 0; [ ... ] > diff --git a/drivers/thunderbolt/pci.c b/drivers/thunderbolt/pci.c > index 4408229d8ac7b..91d2363007034 100644 > --- a/drivers/thunderbolt/pci.c > +++ b/drivers/thunderbolt/pci.c > @@ -715,7 +715,7 @@ static struct pci_device_id nhi_ids[] = { > }; > > MODULE_DEVICE_TABLE(pci, nhi_ids); > -MODULE_DESCRIPTION("Thunderbolt/USB4 core driver"); > +MODULE_DESCRIPTION("Thunderbolt/USB4 PCI host interface driver"); > MODULE_LICENSE("GPL"); > > static struct pci_driver nhi_driver = { > @@ -727,26 +727,4 @@ static struct pci_driver nhi_driver = { > .driver.pm = &nhi_pm_ops, > }; > > -static int __init nhi_init(void) > -{ > - int ret; > - > - ret = tb_domain_init(); > - if (ret) > - return ret; > - > - ret = pci_register_driver(&nhi_driver); > - if (ret) > - tb_domain_exit(); > - > - return ret; > -} > - > -static void __exit nhi_unload(void) > -{ > - pci_unregister_driver(&nhi_driver); > - tb_domain_exit(); > -} > - > -rootfs_initcall(nhi_init); > -module_exit(nhi_unload); > +module_pci_driver(nhi_driver); [Severity: High] Does this break modular builds when CONFIG_USB4=m? In drivers/thunderbolt/Makefile, domain.o, pci.o, and plat.o are all linked into the single thunderbolt.o module. When CONFIG_USB4=m, subsys_initcall(tb_domain_init) and module_exit(tb_domain_exit) in domain.c, module_pci_driver(nhi_driver) in pci.c, and module_platform_driver(nhi_plat_driver) in plat.c all expand to definitions of init_module and cleanup_module, causing multiple-definition linker errors (along with duplicate MODULE_DESCRIPTION and MODULE_LICENSE entries). Also, when CONFIG_USB4=y, decoupling tb_domain_init() from driver registration means pci_register_driver(&nhi_driver) and platform_driver_register(&nhi_plat_driver) will still run even if tb_domain_init() fails, and replacing rootfs_initcall(nhi_init) with module_pci_driver(nhi_driver) moves PCI NHI initialization back to device_initcall, before rootfs_initcall IOMMU initialization (regressing commit eafa717bc145 ("thunderbolt: Initialize after IOMMUs")). Should thunderbolt.ko keep a single module init/exit pair that initializes the domain first, registers both the PCI and platform drivers in order, and unregisters them before calling tb_domain_exit()? > diff --git a/drivers/thunderbolt/plat.c b/drivers/thunderbolt/plat.c > new file mode 100644 > index 0000000000000..85a00cd391d4e > --- /dev/null > +++ b/drivers/thunderbolt/plat.c > @@ -0,0 +1,155 @@ [ ... ] > +static int nhi_plat_init(struct tb_nhi *nhi) > +{ > + /* > + * avoid device link fail for pcie tunnel > + */ > + pm_runtime_set_active(nhi->dev); > + pm_runtime_get_noresume(nhi->dev); > + devm_pm_runtime_enable(nhi->dev); > + > + return 0; > +} > + > +static irqreturn_t nhi_plat_isr(int irq, void *data) > +{ > + struct tb_nhi *nhi = data; > + schedule_work(&nhi->interrupt_work); > + return IRQ_HANDLED; > +} [Severity: Low] This isn't a bug, but nhi_plat_isr() is identical to nhi_msi() in drivers/thunderbolt/nhi.c, which is already declared in nhi.h. Could nhi_plat_init_irq() reuse nhi_msi() directly instead of duplicating it here? > + > +static int nhi_plat_init_irq(struct tb_nhi *nhi) > +{ > + int ret; > + > + INIT_WORK(&nhi->interrupt_work, nhi_interrupt_work); > + > + ret = devm_request_irq(nhi->dev, nhi->irq, nhi_plat_isr, IRQF_NO_SUSPEND, > + dev_name(nhi->dev), nhi); > + if (ret) > + return dev_err_probe(nhi->dev, ret, "request irq failed\n"); > + > + return 0; > +} > + > +static void nhi_plat_exit(struct tb_nhi *nhi) > +{ > + devm_free_irq(nhi->dev, nhi->irq, nhi); > + flush_work(&nhi->interrupt_work); > + > + pm_runtime_put_noidle(nhi->dev); > + pm_runtime_disable(nhi->dev); > + pm_runtime_set_suspended(nhi->dev); > +} [Severity: Medium] Won't calling devm_pm_runtime_enable(nhi->dev) in nhi_plat_init() and pm_runtime_disable(nhi->dev) in nhi_plat_exit() disable runtime PM twice on driver removal or probe failure? When nhi_plat_init() calls devm_pm_runtime_enable(nhi->dev) (whose return value is also unchecked), devres registers pm_runtime_disable_action() to call pm_runtime_disable() automatically on detach. When nhi_plat_remove() -> nhi_shutdown() -> nhi_plat_exit() calls pm_runtime_disable(nhi->dev) manually, and devres then runs pm_runtime_disable_action(), dev->power.disable_depth is incremented to 2, leaving runtime PM disabled if the device is bound again. [Severity: High] What happens if nhi_probe() fails after nhi_plat_init_irq() or nhi_plat_init(), but before tb_domain_add()? In drivers/thunderbolt/nhi.c: nhi_probe() nhi->ops->init_interrupts(nhi); -> nhi_plat_init_irq() dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64)); nhi->ops->init(nhi); -> nhi_plat_init() tb = nhi_select_cm(nhi); if (!tb) return dev_err_probe(...); If dma_set_mask_and_coherent(), nhi->ops->init(), or nhi_select_cm() fails, nhi_probe() returns without calling nhi_shutdown(nhi). As a result, nhi_plat_exit() is not called: the runtime PM reference acquired by pm_runtime_get_noresume(nhi->dev) in nhi_plat_init() is leaked, and devres will free the IRQ and the devm-allocated struct tb_nhi without calling flush_work(&nhi->interrupt_work), which can leave nhi_interrupt_work running on freed memory. > + > +static const struct tb_nhi_ops nhi_plat_default_ops = { > + .init = nhi_plat_init, > + .shutdown = nhi_plat_exit, > + .init_interrupts = nhi_plat_init_irq, > +}; > + > +static int nhi_plat_probe(struct platform_device *pdev) > +{ > + struct device *dev = &pdev->dev; > + struct tb_nhi *nhi; > + > + nhi = devm_kzalloc(dev, sizeof(*nhi), GFP_KERNEL); > + if (!nhi) > + return -ENOMEM; > + > + nhi->dev = dev; > + nhi->ops = &nhi_plat_default_ops; > + > + nhi->iobase = devm_platform_ioremap_resource(pdev, 0); > + if (IS_ERR(nhi->iobase)) > + return PTR_ERR(nhi->iobase); > + > + nhi->irq = platform_get_irq(pdev, 0); > + if (nhi->irq < 0) > + return dev_err_probe(dev, nhi->irq, "get irq failed\n"); > + > + return nhi_probe(nhi); > +} [Severity: High] Can nhi_plat_probe() -> nhi_probe() -> nhi_select_cm() -> icm_probe() cause an out-of-bounds read on nhi->dev? In nhi_plat_probe(), nhi->dev is &pdev->dev (a struct platform_device). nhi_probe() calls nhi_select_cm(nhi): drivers/thunderbolt/nhi.c:nhi_select_cm() { ... if (!tb_acpi_is_native()) { tb = icm_probe(nhi); if (tb) return tb; } ... } When CONFIG_ACPI=y, tb_acpi_is_native() returns false on all Device Tree boots (such as on MediaTek SoCs) as well as on ACPI boots where firmware has not granted native USB4 control. icm_probe() then unconditionally casts nhi->dev to struct pci_dev: drivers/thunderbolt/icm.c:icm_probe() { struct pci_dev *pdev = to_pci_dev(nhi->dev); ... switch (pdev->device) { Because nhi->dev is embedded in a struct platform_device rather than a struct pci_dev, to_pci_dev(nhi->dev) computes an invalid container pointer and reads pdev->device out of bounds before the start of the platform_device allocation. Should icm_probe() check dev_is_pci(nhi->dev) first? [ ... ] > +#ifdef CONFIG_OF > +static const struct of_device_id nhi_plat_of_match[] = { > + { .compatible = "thunderbolt-platform" }, > + { } > +}; > +MODULE_DEVICE_TABLE(of, nhi_plat_of_match); > +#endif > + > +#ifdef CONFIG_ACPI > +static const struct acpi_device_id nhi_plat_acpi_match[] = { > + { "ACPI0015", 0 }, > + { } > +}; > +MODULE_DEVICE_TABLE(acpi, nhi_plat_acpi_match); > +#endif [Severity: Low] This isn't a runtime bug, but there are a few binding and cleanup issues here: 1. The commit message states this driver is for MediaTek mt8901/mt8992 SoCs, but nhi_plat_of_match uses a generic "thunderbolt-platform" compatible string that is not documented in Documentation/devicetree/bindings/, and nhi_plat_acpi_match adds "ACPI0015" without mention in the commit message. Should the DT compatible use a documented SoC-specific binding? 2. In nhi_plat_probe(), platform_get_irq() already calls dev_err_probe() on failure, so calling dev_err_probe() again on nhi->irq < 0 logs a duplicate message. Similarly, nhi_plat_init_irq() calls dev_err_probe() and its caller nhi_probe() calls dev_err_probe() again on the returned error. 3. In nhi_probe(), calling pm_runtime_mark_last_busy(dev) right before pm_runtime_put_autosuspend(dev) is redundant because pm_runtime_put_autosuspend() already calls pm_runtime_mark_last_busy(). [ ... ] > diff --git a/include/linux/thunderbolt.h b/include/linux/thunderbolt.h > index 57502da290805..212c8fea65711 100644 > --- a/include/linux/thunderbolt.h > +++ b/include/linux/thunderbolt.h > @@ -532,6 +532,7 @@ struct tb_nhi { > u32 *interrupt_mask; > bool going_away; > bool iommu_dma_protection; > + int irq; > struct work_struct interrupt_work; > u32 hop_count; > unsigned long quirks; [Severity: Low] struct tb_nhi has a kernel-doc comment block right above its definition. Could @irq be added to that kernel-doc comment so W=1 builds do not warn about an undescribed struct member? -- Sashiko AI review · https://sashiko.dev/#/patchset/20261030082539.1208733-1-chunfeng.yun@mediatek.com?part=1 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/4] thunderbolt: add platform nhi driver 2026-10-30 8:25 ` [PATCH 1/4] thunderbolt: add " Chunfeng Yun 2026-10-30 8:40 ` sashiko-bot @ 2026-10-30 8:47 ` Mika Westerberg 1 sibling, 0 replies; 13+ messages in thread From: Mika Westerberg @ 2026-10-30 8:47 UTC (permalink / raw) To: Chunfeng Yun Cc: Mika Westerberg, Andreas Noever, Yehezkel Bernat, Matthias Brugger, Greg Kroah-Hartman, AngeloGioacchino Del Regno, linux-kernel, linux-usb, linux-arm-kernel, linux-mediatek, Konrad Dybcio, Sven Peter Hi, Adding Konrad and Sven because they are also working towards the same goal to get non-PCIe "platform" hosts supported. On Fri, Oct 30, 2026 at 04:25:36PM +0800, Chunfeng Yun wrote: > Add a new file plat.c to register platform NHI driver for MediaTek > mt8901/mt8992 SoCs. > > Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com> > --- > drivers/thunderbolt/Makefile | 2 +- > drivers/thunderbolt/domain.c | 10 ++- > drivers/thunderbolt/nhi.c | 2 + > drivers/thunderbolt/nhi.h | 1 - > drivers/thunderbolt/pci.c | 26 +----- > drivers/thunderbolt/plat.c | 155 +++++++++++++++++++++++++++++++++++ Call it platform.c > drivers/thunderbolt/tb.h | 2 - > include/linux/thunderbolt.h | 1 + > 8 files changed, 169 insertions(+), 30 deletions(-) > create mode 100644 drivers/thunderbolt/plat.c > > diff --git a/drivers/thunderbolt/Makefile b/drivers/thunderbolt/Makefile > index beb054c3126b..9c941682f3d6 100644 > --- a/drivers/thunderbolt/Makefile > +++ b/drivers/thunderbolt/Makefile > @@ -3,7 +3,7 @@ ccflags-y := -I$(src) > obj-${CONFIG_USB4} := thunderbolt.o > thunderbolt-objs := nhi.o ctl.o tb.o switch.o cap.o pci.o path.o tunnel.o eeprom.o > thunderbolt-objs += domain.o dma_port.o icm.o property.o xdomain.o lc.o tmu.o usb4.o > -thunderbolt-objs += usb4_port.o nvm.o retimer.o quirks.o clx.o > +thunderbolt-objs += usb4_port.o nvm.o retimer.o quirks.o clx.o plat.o > > thunderbolt-${CONFIG_ACPI} += acpi.o > thunderbolt-$(CONFIG_DEBUG_FS) += debugfs.o > diff --git a/drivers/thunderbolt/domain.c b/drivers/thunderbolt/domain.c > index 24611f05b3cd..bec7e6ebc4a8 100644 > --- a/drivers/thunderbolt/domain.c > +++ b/drivers/thunderbolt/domain.c > @@ -881,7 +881,7 @@ int tb_domain_unregister_unplugged_xdomains(struct tb *tb) > return ctx.n; > } > > -int tb_domain_init(void) > +static int __init tb_domain_init(void) > { > int ret; > > @@ -907,7 +907,7 @@ int tb_domain_init(void) > return ret; > } > > -void tb_domain_exit(void) > +static void __exit tb_domain_exit(void) > { > bus_unregister(&tb_bus_type); > ida_destroy(&tb_domain_ida); > @@ -917,3 +917,9 @@ void tb_domain_exit(void) > tb_debugfs_exit(); > tb_configfs_exit(); > } > + > +subsys_initcall(tb_domain_init); > +module_exit(tb_domain_exit); > + > +MODULE_DESCRIPTION("Thunderbolt/USB4 domain driver"); > +MODULE_LICENSE("GPL"); > diff --git a/drivers/thunderbolt/nhi.c b/drivers/thunderbolt/nhi.c > index e99a3fcc4a29..c0f9f1855e8f 100644 > --- a/drivers/thunderbolt/nhi.c > +++ b/drivers/thunderbolt/nhi.c > @@ -1379,8 +1379,10 @@ int nhi_probe(struct tb_nhi *nhi) > device_wakeup_enable(dev); > > pm_runtime_allow(dev); > + device_enable_async_suspend(dev); Why you add this? > pm_runtime_set_autosuspend_delay(dev, TB_AUTOSUSPEND_DELAY); > pm_runtime_use_autosuspend(dev); > + pm_runtime_mark_last_busy(dev); and this > pm_runtime_put_autosuspend(dev); > > return 0; > diff --git a/drivers/thunderbolt/nhi.h b/drivers/thunderbolt/nhi.h > index 88af9ab9fde7..37c11de167ad 100644 > --- a/drivers/thunderbolt/nhi.h > +++ b/drivers/thunderbolt/nhi.h > @@ -29,7 +29,6 @@ enum nhi_mailbox_cmd { > > int nhi_mailbox_cmd(struct tb_nhi *nhi, enum nhi_mailbox_cmd cmd, u32 data); > enum nhi_fw_mode nhi_mailbox_mode(struct tb_nhi *nhi); > -void nhi_enable_int_throttling(struct tb_nhi *nhi); > void nhi_disable_interrupts(struct tb_nhi *nhi); > void nhi_interrupt_work(struct work_struct *work); > irqreturn_t nhi_msi(int irq, void *data); > diff --git a/drivers/thunderbolt/pci.c b/drivers/thunderbolt/pci.c > index 4408229d8ac7..91d236300703 100644 > --- a/drivers/thunderbolt/pci.c > +++ b/drivers/thunderbolt/pci.c > @@ -715,7 +715,7 @@ static struct pci_device_id nhi_ids[] = { > }; > > MODULE_DEVICE_TABLE(pci, nhi_ids); > -MODULE_DESCRIPTION("Thunderbolt/USB4 core driver"); > +MODULE_DESCRIPTION("Thunderbolt/USB4 PCI host interface driver"); > MODULE_LICENSE("GPL"); > > static struct pci_driver nhi_driver = { > @@ -727,26 +727,4 @@ static struct pci_driver nhi_driver = { > .driver.pm = &nhi_pm_ops, > }; > > -static int __init nhi_init(void) > -{ > - int ret; > - > - ret = tb_domain_init(); > - if (ret) > - return ret; > - > - ret = pci_register_driver(&nhi_driver); > - if (ret) > - tb_domain_exit(); > - > - return ret; > -} > - > -static void __exit nhi_unload(void) > -{ > - pci_unregister_driver(&nhi_driver); > - tb_domain_exit(); > -} > - > -rootfs_initcall(nhi_init); > -module_exit(nhi_unload); > +module_pci_driver(nhi_driver); > diff --git a/drivers/thunderbolt/plat.c b/drivers/thunderbolt/plat.c > new file mode 100644 > index 000000000000..85a00cd391d4 > --- /dev/null > +++ b/drivers/thunderbolt/plat.c > @@ -0,0 +1,155 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * Thunderbolt driver - NHI driver (Platform bus specific implementation) > + * > + * Author: Chunfeng Yun <chunfeng.yun@mediatek.com> > + */ > + > +#include <linux/acpi.h> > +#include <linux/interrupt.h> > +#include <linux/errno.h> > +#include <linux/of.h> > +#include <linux/platform_device.h> > +#include <linux/pm_runtime.h> > +#include <linux/slab.h> > + > +#include "nhi.h" > +#include "tb.h" > + > +static int nhi_plat_init(struct tb_nhi *nhi) nhi_platform_ > +{ > + /* > + * avoid device link fail for pcie tunnel > + */ What's this? And the comment style is not the expected. > + pm_runtime_set_active(nhi->dev); > + pm_runtime_get_noresume(nhi->dev); > + devm_pm_runtime_enable(nhi->dev); This means we never runtime suspend? Then this is not good. > + > + return 0; > +} > + > +static irqreturn_t nhi_plat_isr(int irq, void *data) > +{ > + struct tb_nhi *nhi = data; Did you run checkpatch.pl against this? If not please do. > + schedule_work(&nhi->interrupt_work); > + return IRQ_HANDLED; > +} > + > +static int nhi_plat_init_irq(struct tb_nhi *nhi) > +{ > + int ret; > + > + INIT_WORK(&nhi->interrupt_work, nhi_interrupt_work); > + > + ret = devm_request_irq(nhi->dev, nhi->irq, nhi_plat_isr, IRQF_NO_SUSPEND, > + dev_name(nhi->dev), nhi); > + if (ret) > + return dev_err_probe(nhi->dev, ret, "request irq failed\n"); > + > + return 0; > +} > + > +static void nhi_plat_exit(struct tb_nhi *nhi) > +{ > + devm_free_irq(nhi->dev, nhi->irq, nhi); > + flush_work(&nhi->interrupt_work); > + > + pm_runtime_put_noidle(nhi->dev); > + pm_runtime_disable(nhi->dev); > + pm_runtime_set_suspended(nhi->dev); > +} > + > +static const struct tb_nhi_ops nhi_plat_default_ops = { > + .init = nhi_plat_init, > + .shutdown = nhi_plat_exit, > + .init_interrupts = nhi_plat_init_irq, > +}; > + > +static int nhi_plat_probe(struct platform_device *pdev) > +{ > + struct device *dev = &pdev->dev; > + struct tb_nhi *nhi; > + > + nhi = devm_kzalloc(dev, sizeof(*nhi), GFP_KERNEL); > + if (!nhi) > + return -ENOMEM; > + > + nhi->dev = dev; > + nhi->ops = &nhi_plat_default_ops; > + > + nhi->iobase = devm_platform_ioremap_resource(pdev, 0); > + if (IS_ERR(nhi->iobase)) > + return PTR_ERR(nhi->iobase); > + > + nhi->irq = platform_get_irq(pdev, 0); > + if (nhi->irq < 0) > + return dev_err_probe(dev, nhi->irq, "get irq failed\n"); > + > + return nhi_probe(nhi); > +} > + > +static void nhi_plat_do_remove(struct platform_device *pdev, bool reset) > +{ > + struct device *dev = &pdev->dev; > + struct tb *tb = platform_get_drvdata(pdev); > + struct tb_nhi *nhi = tb->nhi; > + > + /* > + * On system shutdown/reboot force a host router reset so the > + * CM asserts DPR on connected tbt3 devices before the router tree > + * is removed (see tb_stop()). > + */ > + if (reset) > + nhi->host_reset = true; > + > + pm_runtime_get_sync(dev); > + pm_runtime_dont_use_autosuspend(dev); > + > + tb_domain_remove(tb); > + wait_for_completion(&nhi->domain_released); > + nhi_shutdown(nhi); > +} > + > +static void nhi_plat_remove(struct platform_device *pdev) > +{ > + nhi_plat_do_remove(pdev, false); > +} > + > +static void nhi_plat_shutdown(struct platform_device *pdev) > +{ > + nhi_plat_do_remove(pdev, true); > +} > + > + > +#ifdef CONFIG_OF > +static const struct of_device_id nhi_plat_of_match[] = { > + { .compatible = "thunderbolt-platform" }, Not sure if this is correct? Should this have mediatek,... ? > + { } > +}; > +MODULE_DEVICE_TABLE(of, nhi_plat_of_match); > +#endif > + > +#ifdef CONFIG_ACPI > +static const struct acpi_device_id nhi_plat_acpi_match[] = { > + { "ACPI0015", 0 }, Same here, where is this ID defined in the ACPI spec? > + { } > +}; > +MODULE_DEVICE_TABLE(acpi, nhi_plat_acpi_match); > +#endif > + > +static struct platform_driver nhi_plat_driver = { > + .probe = nhi_plat_probe, > + .remove = nhi_plat_remove, > + .shutdown = nhi_plat_shutdown, > + .driver = { > + .name = "thunderbolt", > + .of_match_table = of_match_ptr(nhi_plat_of_match), > + .acpi_match_table = ACPI_PTR(nhi_plat_acpi_match), > + .pm = &nhi_pm_ops, > + }, > +}; > + > +module_platform_driver(nhi_plat_driver); > +MODULE_ALIAS("platform:thunderbolt"); > +MODULE_DESCRIPTION("Thunderbolt/USB4 platform host interface driver"); > +MODULE_LICENSE("GPL v2"); > diff --git a/drivers/thunderbolt/tb.h b/drivers/thunderbolt/tb.h > index e86845762e22..6dfdc47e05d2 100644 > --- a/drivers/thunderbolt/tb.h > +++ b/drivers/thunderbolt/tb.h > @@ -768,8 +768,6 @@ extern const struct device_type tb_retimer_type; > extern const struct device_type tb_switch_type; > extern const struct device_type usb4_port_device_type; > > -int tb_domain_init(void); > -void tb_domain_exit(void); > int tb_xdomain_init(void); > void tb_xdomain_exit(void); > > diff --git a/include/linux/thunderbolt.h b/include/linux/thunderbolt.h > index 57502da29080..212c8fea6571 100644 > --- a/include/linux/thunderbolt.h > +++ b/include/linux/thunderbolt.h > @@ -532,6 +532,7 @@ struct tb_nhi { > u32 *interrupt_mask; > bool going_away; > bool iommu_dma_protection; > + int irq; You are missing kernel-doc. > struct work_struct interrupt_work; > u32 hop_count; > unsigned long quirks; > -- > 2.43.0 ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 2/4] thunderbolt: add support optional dedicated wakeup irq 2026-10-30 8:25 [PATCH 0/4] add support platform nhi driver Chunfeng Yun 2026-10-30 8:25 ` [PATCH 1/4] thunderbolt: add " Chunfeng Yun @ 2026-10-30 8:25 ` Chunfeng Yun 2026-10-30 8:48 ` Mika Westerberg 2026-10-30 8:50 ` sashiko-bot 2026-10-30 8:25 ` [PATCH 3/4] thunderbolt: improve usb3 adapter link up timeout value Chunfeng Yun 2026-10-30 8:25 ` [PATCH 4/4] thunderbolt: prevent pcie tunnel activation/teardown warnings Chunfeng Yun 3 siblings, 2 replies; 13+ messages in thread From: Chunfeng Yun @ 2026-10-30 8:25 UTC (permalink / raw) To: Mika Westerberg Cc: Andreas Noever, Yehezkel Bernat, Matthias Brugger, Greg Kroah-Hartman, AngeloGioacchino Del Regno, linux-kernel, linux-usb, linux-arm-kernel, linux-mediatek, Chunfeng Yun There is a dedicated wakeup irq on nvidia n1/n2 SoCs. Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com> --- drivers/thunderbolt/plat.c | 16 ++++++++++++++++ include/linux/thunderbolt.h | 1 + 2 files changed, 17 insertions(+) diff --git a/drivers/thunderbolt/plat.c b/drivers/thunderbolt/plat.c index 85a00cd391d4..8d83b484d942 100644 --- a/drivers/thunderbolt/plat.c +++ b/drivers/thunderbolt/plat.c @@ -11,6 +11,7 @@ #include <linux/of.h> #include <linux/platform_device.h> #include <linux/pm_runtime.h> +#include <linux/pm_wakeirq.h> #include <linux/slab.h> #include "nhi.h" @@ -46,6 +47,15 @@ static int nhi_plat_init_irq(struct tb_nhi *nhi) if (ret) return dev_err_probe(nhi->dev, ret, "request irq failed\n"); + if (nhi->wake_irq) { + ret = dev_pm_set_dedicated_wake_irq(nhi->dev, nhi->wake_irq); + if (ret) { + dev_err_probe(nhi->dev, ret, "set wake irq %d failed\n", nhi->wake_irq); + return ret; + } + dev_info(nhi->dev, "wake irq %d\n", nhi->wake_irq); + } + return 0; } @@ -53,6 +63,7 @@ static void nhi_plat_exit(struct tb_nhi *nhi) { devm_free_irq(nhi->dev, nhi->irq, nhi); flush_work(&nhi->interrupt_work); + dev_pm_clear_wake_irq(nhi->dev); pm_runtime_put_noidle(nhi->dev); pm_runtime_disable(nhi->dev); @@ -85,6 +96,11 @@ static int nhi_plat_probe(struct platform_device *pdev) if (nhi->irq < 0) return dev_err_probe(dev, nhi->irq, "get irq failed\n"); + /* ignore error */ + nhi->wake_irq = platform_get_irq_optional(pdev, 1); + if (nhi->wake_irq < 0) + nhi->wake_irq = 0; + return nhi_probe(nhi); } diff --git a/include/linux/thunderbolt.h b/include/linux/thunderbolt.h index 212c8fea6571..06b90d452ee2 100644 --- a/include/linux/thunderbolt.h +++ b/include/linux/thunderbolt.h @@ -533,6 +533,7 @@ struct tb_nhi { bool going_away; bool iommu_dma_protection; int irq; + int wake_irq; struct work_struct interrupt_work; u32 hop_count; unsigned long quirks; -- 2.43.0 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 2/4] thunderbolt: add support optional dedicated wakeup irq 2026-10-30 8:25 ` [PATCH 2/4] thunderbolt: add support optional dedicated wakeup irq Chunfeng Yun @ 2026-10-30 8:48 ` Mika Westerberg 2026-10-30 8:50 ` sashiko-bot 1 sibling, 0 replies; 13+ messages in thread From: Mika Westerberg @ 2026-10-30 8:48 UTC (permalink / raw) To: Chunfeng Yun Cc: Mika Westerberg, Andreas Noever, Yehezkel Bernat, Matthias Brugger, Greg Kroah-Hartman, AngeloGioacchino Del Regno, linux-kernel, linux-usb, linux-arm-kernel, linux-mediatek On Fri, Oct 30, 2026 at 04:25:37PM +0800, Chunfeng Yun wrote: > There is a dedicated wakeup irq on nvidia n1/n2 SoCs. Okay but why it is needed? Can you add this to the commit message? And why you did not add it already in the first commit that added the platform driver there anyway? > > Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com> > --- > drivers/thunderbolt/plat.c | 16 ++++++++++++++++ > include/linux/thunderbolt.h | 1 + > 2 files changed, 17 insertions(+) > > diff --git a/drivers/thunderbolt/plat.c b/drivers/thunderbolt/plat.c > index 85a00cd391d4..8d83b484d942 100644 > --- a/drivers/thunderbolt/plat.c > +++ b/drivers/thunderbolt/plat.c > @@ -11,6 +11,7 @@ > #include <linux/of.h> > #include <linux/platform_device.h> > #include <linux/pm_runtime.h> > +#include <linux/pm_wakeirq.h> > #include <linux/slab.h> > > #include "nhi.h" > @@ -46,6 +47,15 @@ static int nhi_plat_init_irq(struct tb_nhi *nhi) > if (ret) > return dev_err_probe(nhi->dev, ret, "request irq failed\n"); > > + if (nhi->wake_irq) { > + ret = dev_pm_set_dedicated_wake_irq(nhi->dev, nhi->wake_irq); > + if (ret) { > + dev_err_probe(nhi->dev, ret, "set wake irq %d failed\n", nhi->wake_irq); > + return ret; > + } > + dev_info(nhi->dev, "wake irq %d\n", nhi->wake_irq); > + } > + > return 0; > } > > @@ -53,6 +63,7 @@ static void nhi_plat_exit(struct tb_nhi *nhi) > { > devm_free_irq(nhi->dev, nhi->irq, nhi); > flush_work(&nhi->interrupt_work); > + dev_pm_clear_wake_irq(nhi->dev); > > pm_runtime_put_noidle(nhi->dev); > pm_runtime_disable(nhi->dev); > @@ -85,6 +96,11 @@ static int nhi_plat_probe(struct platform_device *pdev) > if (nhi->irq < 0) > return dev_err_probe(dev, nhi->irq, "get irq failed\n"); > > + /* ignore error */ > + nhi->wake_irq = platform_get_irq_optional(pdev, 1); > + if (nhi->wake_irq < 0) > + nhi->wake_irq = 0; > + > return nhi_probe(nhi); > } > > diff --git a/include/linux/thunderbolt.h b/include/linux/thunderbolt.h > index 212c8fea6571..06b90d452ee2 100644 > --- a/include/linux/thunderbolt.h > +++ b/include/linux/thunderbolt.h > @@ -533,6 +533,7 @@ struct tb_nhi { > bool going_away; > bool iommu_dma_protection; > int irq; > + int wake_irq; > struct work_struct interrupt_work; > u32 hop_count; > unsigned long quirks; > -- > 2.43.0 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/4] thunderbolt: add support optional dedicated wakeup irq 2026-10-30 8:25 ` [PATCH 2/4] thunderbolt: add support optional dedicated wakeup irq Chunfeng Yun 2026-10-30 8:48 ` Mika Westerberg @ 2026-10-30 8:50 ` sashiko-bot 1 sibling, 0 replies; 13+ messages in thread From: sashiko-bot @ 2026-10-30 8:50 UTC (permalink / raw) To: Chunfeng Yun; +Cc: linux-usb Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider: - [Medium] thunderbolt: wake_irq never armed for suspend due to unset can_wakeup - [Medium] thunderbolt: wake_irq leaked on nhi_probe() error after nhi_plat_init_irq() - [Medium] thunderbolt: nhi_plat_probe() ignores -EPROBE_DEFER for optional wake_irq - [Low] thunderbolt: missing kernel-doc entry for wake_irq in struct tb_nhi -- commit 0039f17f466f2a19ba5fd45e19c6de293662f588 Author: Chunfeng Yun <chunfeng.yun@mediatek.com> thunderbolt: add support optional dedicated wakeup irq This commit adds support for an optional dedicated wakeup IRQ to the platform Thunderbolt NHI driver for NVIDIA N1/N2 SoCs. > diff --git a/drivers/thunderbolt/plat.c b/drivers/thunderbolt/plat.c > index 85a00cd391d4e..8d83b484d942c 100644 > --- a/drivers/thunderbolt/plat.c > +++ b/drivers/thunderbolt/plat.c > @@ -11,6 +11,7 @@ > #include <linux/of.h> > #include <linux/platform_device.h> > #include <linux/pm_runtime.h> > +#include <linux/pm_wakeirq.h> > #include <linux/slab.h> > > #include "nhi.h" > @@ -46,6 +47,15 @@ static int nhi_plat_init_irq(struct tb_nhi *nhi) > if (ret) > return dev_err_probe(nhi->dev, ret, "request irq failed\n"); > > + if (nhi->wake_irq) { > + ret = dev_pm_set_dedicated_wake_irq(nhi->dev, nhi->wake_irq); > + if (ret) { > + dev_err_probe(nhi->dev, ret, "set wake irq %d failed\n", nhi->wake_irq); > + return ret; > + } > + dev_info(nhi->dev, "wake irq %d\n", nhi->wake_irq); > + } [Severity: Medium] Will nhi->wake_irq ever be armed for system suspend when dev->power.can_wakeup has not been set on the platform device? In nhi_plat_probe() -> nhi_probe() -> nhi_plat_init_irq(), dev_pm_set_dedicated_wake_irq(nhi->dev, nhi->wake_irq) attaches wirq to dev->power.wakeirq while dev->power.wakeup is still NULL. Later at the end of nhi_probe() in drivers/thunderbolt/nhi.c: device_wakeup_enable(dev); In drivers/base/power/wakeup.c, device_wakeup_enable() checks: if (!dev || !dev->power.can_wakeup) return -EINVAL; Unlike PCI devices (where pci_pm_init() marks the device wakeup-capable), neither the platform bus nor nhi_plat_probe() / nhi_plat_init_irq() calls device_set_wakeup_capable(nhi->dev, true) or device_init_wakeup(nhi->dev, true) for DT or ACPI platform devices without _PRW. As a result, device_wakeup_enable() returns -EINVAL, dev->power.wakeup remains NULL, device_may_wakeup() returns false in nhi_suspend_noirq() and nhi_poweroff_noirq(), and device_wakeup_arm_wake_irqs() never arms nhi->wake_irq during system suspend. Should device_set_wakeup_capable(nhi->dev, true) be called before device_wakeup_enable(dev) is invoked? > + > return 0; > } > > @@ -53,6 +63,7 @@ static void nhi_plat_exit(struct tb_nhi *nhi) > { > devm_free_irq(nhi->dev, nhi->irq, nhi); > flush_work(&nhi->interrupt_work); > + dev_pm_clear_wake_irq(nhi->dev); [Severity: Medium] Does this leak the dedicated wake_irq if nhi_probe() fails after nhi_plat_init_irq() succeeds? Unlike devm_request_irq() used for nhi->irq, dev_pm_set_dedicated_wake_irq() is not devres-managed: it allocates struct wake_irq, requests a threaded IRQ, and sets dev->power.wakeirq. However, dev_pm_clear_wake_irq(nhi->dev) is only called in nhi_plat_exit() via nhi_shutdown(). In drivers/thunderbolt/nhi.c, nhi_probe() has several error paths after nhi->ops->init_interrupts(nhi) that return directly without calling nhi_shutdown(): res = nhi->ops->init_interrupts(nhi); if (res) return dev_err_probe(dev, res, "cannot enable interrupts, aborting\n"); spin_lock_init(&nhi->lock); res = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64)); if (res) return dev_err_probe(dev, res, "failed to set DMA mask\n"); if (nhi->ops->init) { res = nhi->ops->init(nhi); if (res) return dev_err_probe(dev, res, "NHI specific init failed\n"); } init_completion(&nhi->domain_released); tb = nhi_select_cm(nhi); if (!tb) return dev_err_probe(dev, -ENODEV, "failed to determine connection manager, aborting\n"); If any of those steps fail, the requested threaded wake IRQ and struct wake_irq remain allocated and dev->power.wakeirq stays set, causing any later probe attempt to fail with -EEXIST or -EBUSY. Could devm_add_action_or_reset() or proper error unwinding be used to ensure dev_pm_clear_wake_irq() is called on probe failure? > > pm_runtime_put_noidle(nhi->dev); > pm_runtime_disable(nhi->dev); > @@ -85,6 +96,11 @@ static int nhi_plat_probe(struct platform_device *pdev) > if (nhi->irq < 0) > return dev_err_probe(dev, nhi->irq, "get irq failed\n"); > > + /* ignore error */ > + nhi->wake_irq = platform_get_irq_optional(pdev, 1); > + if (nhi->wake_irq < 0) > + nhi->wake_irq = 0; [Severity: Medium] Can this silently drop -EPROBE_DEFER when the optional wake_irq provider is not ready yet? In nhi_plat_probe(), platform_get_irq_optional(pdev, 1) returns -ENXIO when the optional interrupt is absent, but returns -EPROBE_DEFER (or another negative error code) if the interrupt is specified in firmware and its interrupt controller has not probed yet. Clamping all negative values to 0 causes probe to proceed without the wakeup IRQ instead of deferring. Should this only ignore -ENXIO and return dev_err_probe() on other negative errors? > diff --git a/include/linux/thunderbolt.h b/include/linux/thunderbolt.h > index 212c8fea65711..06b90d452ee24 100644 > --- a/include/linux/thunderbolt.h > +++ b/include/linux/thunderbolt.h > @@ -533,6 +533,7 @@ struct tb_nhi { > bool going_away; > bool iommu_dma_protection; > int irq; > + int wake_irq; [Severity: Low] This isn't a bug, but struct tb_nhi in include/linux/thunderbolt.h has a kernel-doc comment block above it; should @wake_irq be documented there to avoid a kernel-doc warning under W=1 builds? -- Sashiko AI review · https://sashiko.dev/#/patchset/20261030082539.1208733-1-chunfeng.yun@mediatek.com?part=2 ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 3/4] thunderbolt: improve usb3 adapter link up timeout value 2026-10-30 8:25 [PATCH 0/4] add support platform nhi driver Chunfeng Yun 2026-10-30 8:25 ` [PATCH 1/4] thunderbolt: add " Chunfeng Yun 2026-10-30 8:25 ` [PATCH 2/4] thunderbolt: add support optional dedicated wakeup irq Chunfeng Yun @ 2026-10-30 8:25 ` Chunfeng Yun 2026-10-30 8:50 ` Mika Westerberg 2026-10-30 8:54 ` sashiko-bot 2026-10-30 8:25 ` [PATCH 4/4] thunderbolt: prevent pcie tunnel activation/teardown warnings Chunfeng Yun 3 siblings, 2 replies; 13+ messages in thread From: Chunfeng Yun @ 2026-10-30 8:25 UTC (permalink / raw) To: Mika Westerberg Cc: Andreas Noever, Yehezkel Bernat, Matthias Brugger, Greg Kroah-Hartman, AngeloGioacchino Del Regno, linux-kernel, linux-usb, linux-arm-kernel, linux-mediatek, Chunfeng Yun when plug into a usb3 device into usb4 hub's DFP to trigger usb4 driver wakeup, encounter usb3 link up timeout issue, then cause usb3 hub disconnect: usb4_hi NVDA8100:00: 0:1: failed to reach state TB_PORT_UP. Ignoring port... usb4_hi NVDA8100:00: 0:1: lost during suspend, disconnecting usb4_hi NVDA8100:00: 0:4 <-> 1:20 (USB3): deactivating usb4_hi NVDA8100:00: deactivating USB3 Down path from 0:4 to 1:20 usb4_hi NVDA8100:00: deactivating USB3 Up path from 1:20 to 0:4 ... thunderbolt 0-1: device disconnected usb 2-1: USB disconnect, device number 10 Completes link training at ~1.48s and transitions to TB_PORT_UP after change the timeout to 2.5s: In the USB4 spec, Phase 5 (Link Equalization) alone is allowed up to 1000 ms. When add tPhase3 (100 ms), tPhase4 (100 ms), and retimer transaction delays (tCmdResponse = 50 ms), the physical link is allowed by spec to take 1200 ms to 1500 ms+ to complete initialization. here are two retimers in the link, if more retimers add it, need more time Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com> --- drivers/thunderbolt/switch.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/thunderbolt/switch.c b/drivers/thunderbolt/switch.c index d22d8db8f890..e7ee9639c54c 100644 --- a/drivers/thunderbolt/switch.c +++ b/drivers/thunderbolt/switch.c @@ -500,8 +500,9 @@ int tb_port_state(struct tb_port *port) */ int tb_wait_for_port(struct tb_port *port, bool wait_if_unplugged) { - int retries = 10; + int retries = 25; int state; + if (!port->cap_phy) { tb_port_WARN(port, "does not have PHY\n"); return -EINVAL; -- 2.43.0 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 3/4] thunderbolt: improve usb3 adapter link up timeout value 2026-10-30 8:25 ` [PATCH 3/4] thunderbolt: improve usb3 adapter link up timeout value Chunfeng Yun @ 2026-10-30 8:50 ` Mika Westerberg 2026-10-30 8:54 ` sashiko-bot 1 sibling, 0 replies; 13+ messages in thread From: Mika Westerberg @ 2026-10-30 8:50 UTC (permalink / raw) To: Chunfeng Yun Cc: Mika Westerberg, Andreas Noever, Yehezkel Bernat, Matthias Brugger, Greg Kroah-Hartman, AngeloGioacchino Del Regno, linux-kernel, linux-usb, linux-arm-kernel, linux-mediatek On Fri, Oct 30, 2026 at 04:25:38PM +0800, Chunfeng Yun wrote: > when plug into a usb3 device into usb4 hub's DFP to trigger usb4 USB 3 device into USB4 hub's downstream facing port to ... > driver wakeup, encounter usb3 link up timeout issue, then cause > usb3 hub disconnect: > > usb4_hi NVDA8100:00: 0:1: failed to reach state TB_PORT_UP. Ignoring port... > usb4_hi NVDA8100:00: 0:1: lost during suspend, disconnecting > usb4_hi NVDA8100:00: 0:4 <-> 1:20 (USB3): deactivating > usb4_hi NVDA8100:00: deactivating USB3 Down path from 0:4 to 1:20 > usb4_hi NVDA8100:00: deactivating USB3 Up path from 1:20 to 0:4 > ... > thunderbolt 0-1: device disconnected > usb 2-1: USB disconnect, device number 10 > > Completes link training at ~1.48s and transitions to TB_PORT_UP after > change the timeout to 2.5s: > > In the USB4 spec, Phase 5 (Link Equalization) alone is allowed up to > 1000 ms. When add tPhase3 (100 ms), tPhase4 (100 ms), and retimer > transaction delays (tCmdResponse = 50 ms), the physical link is > allowed by spec to take 1200 ms to 1500 ms+ to complete initialization. > > here are two retimers in the link, if more retimers add it, need more > time > > Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com> > --- > drivers/thunderbolt/switch.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/thunderbolt/switch.c b/drivers/thunderbolt/switch.c > index d22d8db8f890..e7ee9639c54c 100644 > --- a/drivers/thunderbolt/switch.c > +++ b/drivers/thunderbolt/switch.c > @@ -500,8 +500,9 @@ int tb_port_state(struct tb_port *port) > */ > int tb_wait_for_port(struct tb_port *port, bool wait_if_unplugged) > { > - int retries = 10; > + int retries = 25; > int state; > + This is unrelated change. > if (!port->cap_phy) { > tb_port_WARN(port, "does not have PHY\n"); > return -EINVAL; > -- > 2.43.0 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 3/4] thunderbolt: improve usb3 adapter link up timeout value 2026-10-30 8:25 ` [PATCH 3/4] thunderbolt: improve usb3 adapter link up timeout value Chunfeng Yun 2026-10-30 8:50 ` Mika Westerberg @ 2026-10-30 8:54 ` sashiko-bot 1 sibling, 0 replies; 13+ messages in thread From: sashiko-bot @ 2026-10-30 8:54 UTC (permalink / raw) To: Chunfeng Yun; +Cc: linux-usb Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [Low] thunderbolt: stale 1-second timeout in tb_wait_for_port() kernel-doc - [Low] thunderbolt: commit message misidentifies lane adapter timeout as USB3 -- commit bc67946f50d0699b97b8acb535ee59b1d04143ab Author: Chunfeng Yun <chunfeng.yun@mediatek.com> thunderbolt: improve usb3 adapter link up timeout value This commit increases the retry count in tb_wait_for_port() from 10 to 25 (raising the timeout from 1 second to 2.5 seconds) so that ports with slow link training do not time out and disconnect during resume. > thunderbolt: improve usb3 adapter link up timeout value > > when plug into a usb3 device into usb4 hub's DFP to trigger usb4 > driver wakeup, encounter usb3 link up timeout issue, then cause > usb3 hub disconnect: > > usb4_hi NVDA8100:00: 0:1: failed to reach state TB_PORT_UP. Ignoring port... > usb4_hi NVDA8100:00: 0:1: lost during suspend, disconnecting > usb4_hi NVDA8100:00: 0:4 <-> 1:20 (USB3): deactivating > usb4_hi NVDA8100:00: deactivating USB3 Down path from 0:4 to 1:20 > usb4_hi NVDA8100:00: deactivating USB3 Up path from 1:20 to 0:4 > ... > thunderbolt 0-1: device disconnected > usb 2-1: USB disconnect, device number 10 > > Completes link training at ~1.48s and transitions to TB_PORT_UP after > change the timeout to 2.5s: > > In the USB4 spec, Phase 5 (Link Equalization) alone is allowed up to > 1000 ms. When add tPhase3 (100 ms), tPhase4 (100 ms), and retimer > transaction delays (tCmdResponse = 50 ms), the physical link is > allowed by spec to take 1200 ms to 1500 ms+ to complete initialization. > > here are two retimers in the link, if more retimers add it, need more > time > > Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com> [Severity: Low] This isn't a bug in the code, but is the commit subject and description accurate in referring to a USB3 adapter link-up timeout? Looking at tb_wait_for_port() in drivers/thunderbolt/switch.c, it requires port->cap_phy and polls tb_port_state() for TB_PORT_UP (CL0). In tb_switch_resume(), tb_wait_for_port() is only called on physical lane adapters (checked via tb_port_is_null(port), which corresponds to port 0:1 in the log): drivers/thunderbolt/switch.c:tb_switch_resume() { ... tb_switch_for_each_port(sw, port) { if (!tb_port_is_null(port)) continue; if (!tb_port_resume(port)) continue; if (tb_wait_for_port(port, true) <= 0) { ... } USB3 protocol adapters (ports 0:4 and 1:20 in the log) do not have cap_phy and were only deactivated as a downstream consequence after lane adapter 0:1 timed out and the downstream router was marked unplugged. In addition, changing retries in tb_wait_for_port() affects all lane adapter readiness waits across tb_scan_port(), tb_switch_lane_bonding_enable(), and tb_xdomain_lane_bonding_enable(). Could the commit title and message be updated to clarify that this increases the physical lane adapter link-up timeout rather than a USB3 adapter timeout? > diff --git a/drivers/thunderbolt/switch.c b/drivers/thunderbolt/switch.c > index d22d8db8f8906..e7ee9639c54cd 100644 > --- a/drivers/thunderbolt/switch.c > +++ b/drivers/thunderbolt/switch.c > @@ -500,8 +500,9 @@ int tb_port_state(struct tb_port *port) > */ > int tb_wait_for_port(struct tb_port *port, bool wait_if_unplugged) > { > - int retries = 10; > + int retries = 25; [Severity: Low] This isn't a bug, but with retries increased to 25 (2.5 seconds at 100 ms per retry), should the kernel-doc comment immediately above tb_wait_for_port() in drivers/thunderbolt/switch.c also be updated where it still states a 1-second timeout? drivers/thunderbolt/switch.c: /** * tb_wait_for_port() - wait for a port to become ready * @port: Port to wait * @wait_if_unplugged: Wait also when port is unplugged * * Wait up to 1 second for a port to reach state TB_PORT_UP. If ... * Return: * * %0 - If the port is not connected or failed to reach * state %TB_PORT_UP within one second. -- Sashiko AI review · https://sashiko.dev/#/patchset/20261030082539.1208733-1-chunfeng.yun@mediatek.com?part=3 ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 4/4] thunderbolt: prevent pcie tunnel activation/teardown warnings 2026-10-30 8:25 [PATCH 0/4] add support platform nhi driver Chunfeng Yun ` (2 preceding siblings ...) 2026-10-30 8:25 ` [PATCH 3/4] thunderbolt: improve usb3 adapter link up timeout value Chunfeng Yun @ 2026-10-30 8:25 ` Chunfeng Yun 2026-10-30 8:56 ` Mika Westerberg 2026-10-30 9:01 ` sashiko-bot 3 siblings, 2 replies; 13+ messages in thread From: Chunfeng Yun @ 2026-10-30 8:25 UTC (permalink / raw) To: Mika Westerberg Cc: Andreas Noever, Yehezkel Bernat, Matthias Brugger, Greg Kroah-Hartman, AngeloGioacchino Del Regno, linux-kernel, linux-usb, linux-arm-kernel, linux-mediatek, Chunfeng Yun When usb4 enter sleep mode, echo 1 to authorized of sysfile, will wakeup usb4 driver, but fail to create pcie tunnel, cause the warning due to a race condition between switch disconnect during resume and user authorization When usb4 enter sleep mode, echo 0 to authorized of sysfile, will wakeup usb4 driver, but fail to disconnect pcie due to the PCIe tunnel had already been torn down and freed if the port is fail to resume: The check prevents a redundant lookup and eliminates the warning Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com> --- drivers/thunderbolt/switch.c | 5 +++++ drivers/thunderbolt/tb.c | 7 +++++++ 2 files changed, 12 insertions(+) diff --git a/drivers/thunderbolt/switch.c b/drivers/thunderbolt/switch.c index e7ee9639c54c..7902172f4e99 100644 --- a/drivers/thunderbolt/switch.c +++ b/drivers/thunderbolt/switch.c @@ -1849,6 +1849,11 @@ static int tb_switch_set_authorized(struct tb_switch *sw, unsigned int val) if (!!sw->authorized == !!val) goto unlock; + if (sw->is_unplugged) { + ret = -ENODEV; + goto unlock; + } + switch (val) { /* Disapprove switch */ case 0: diff --git a/drivers/thunderbolt/tb.c b/drivers/thunderbolt/tb.c index 57c11ba13459..69310622d786 100644 --- a/drivers/thunderbolt/tb.c +++ b/drivers/thunderbolt/tb.c @@ -2266,6 +2266,10 @@ static int tb_disconnect_pci(struct tb *tb, struct tb_switch *sw) struct tb_tunnel *tunnel; struct tb_port *up; + /* If the switch has already been unplugged, tunnels are already freed */ + if (sw->is_unplugged) + return 0; + up = tb_switch_find_port(sw, TB_TYPE_PCIE_UP); if (WARN_ON(!up)) return -ENODEV; @@ -2288,6 +2292,9 @@ static int tb_tunnel_pci(struct tb *tb, struct tb_switch *sw) struct tb_cm *tcm = tb_priv(tb); struct tb_tunnel *tunnel; + if (sw->is_unplugged) + return -ENODEV; + up = tb_switch_find_port(sw, TB_TYPE_PCIE_UP); if (!up) return 0; -- 2.43.0 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 4/4] thunderbolt: prevent pcie tunnel activation/teardown warnings 2026-10-30 8:25 ` [PATCH 4/4] thunderbolt: prevent pcie tunnel activation/teardown warnings Chunfeng Yun @ 2026-10-30 8:56 ` Mika Westerberg 2026-10-30 9:01 ` sashiko-bot 1 sibling, 0 replies; 13+ messages in thread From: Mika Westerberg @ 2026-10-30 8:56 UTC (permalink / raw) To: Chunfeng Yun Cc: Mika Westerberg, Andreas Noever, Yehezkel Bernat, Matthias Brugger, Greg Kroah-Hartman, AngeloGioacchino Del Regno, linux-kernel, linux-usb, linux-arm-kernel, linux-mediatek On Fri, Oct 30, 2026 at 04:25:39PM +0800, Chunfeng Yun wrote: > When usb4 enter sleep mode, echo 1 to authorized of sysfile, > will wakeup usb4 driver, but fail to create pcie tunnel, cause > the warning due to a race condition between switch disconnect It's router > during resume and user authorization > > When usb4 enter sleep mode, echo 0 to authorized of sysfile, > will wakeup usb4 driver, but fail to disconnect pcie due to > the PCIe tunnel had already been torn down and freed if the port > is fail to resume: So this is runtime resume when there is no PCIe tunnel yet, the device is unplugged and the userspace is at the same time writing to the "authorized" attribute? Then the PCIe tunnel creation fails as expected. What warning you are trying to silence? > > The check prevents a redundant lookup and eliminates the warning > > Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com> > --- > drivers/thunderbolt/switch.c | 5 +++++ > drivers/thunderbolt/tb.c | 7 +++++++ > 2 files changed, 12 insertions(+) > > diff --git a/drivers/thunderbolt/switch.c b/drivers/thunderbolt/switch.c > index e7ee9639c54c..7902172f4e99 100644 > --- a/drivers/thunderbolt/switch.c > +++ b/drivers/thunderbolt/switch.c > @@ -1849,6 +1849,11 @@ static int tb_switch_set_authorized(struct tb_switch *sw, unsigned int val) > if (!!sw->authorized == !!val) > goto unlock; > > + if (sw->is_unplugged) { > + ret = -ENODEV; > + goto unlock; > + } > + > switch (val) { > /* Disapprove switch */ > case 0: > diff --git a/drivers/thunderbolt/tb.c b/drivers/thunderbolt/tb.c > index 57c11ba13459..69310622d786 100644 > --- a/drivers/thunderbolt/tb.c > +++ b/drivers/thunderbolt/tb.c > @@ -2266,6 +2266,10 @@ static int tb_disconnect_pci(struct tb *tb, struct tb_switch *sw) > struct tb_tunnel *tunnel; > struct tb_port *up; > > + /* If the switch has already been unplugged, tunnels are already freed */ > + if (sw->is_unplugged) > + return 0; > + > up = tb_switch_find_port(sw, TB_TYPE_PCIE_UP); > if (WARN_ON(!up)) > return -ENODEV; > @@ -2288,6 +2292,9 @@ static int tb_tunnel_pci(struct tb *tb, struct tb_switch *sw) > struct tb_cm *tcm = tb_priv(tb); > struct tb_tunnel *tunnel; > > + if (sw->is_unplugged) > + return -ENODEV; > + > up = tb_switch_find_port(sw, TB_TYPE_PCIE_UP); > if (!up) > return 0; > -- > 2.43.0 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 4/4] thunderbolt: prevent pcie tunnel activation/teardown warnings 2026-10-30 8:25 ` [PATCH 4/4] thunderbolt: prevent pcie tunnel activation/teardown warnings Chunfeng Yun 2026-10-30 8:56 ` Mika Westerberg @ 2026-10-30 9:01 ` sashiko-bot 1 sibling, 0 replies; 13+ messages in thread From: sashiko-bot @ 2026-10-30 9:01 UTC (permalink / raw) To: Chunfeng Yun; +Cc: linux-usb > When usb4 enter sleep mode, echo 1 to authorized of sysfile, > will wakeup usb4 driver, but fail to create pcie tunnel, cause > the warning due to a race condition between switch disconnect > during resume and user authorization > > When usb4 enter sleep mode, echo 0 to authorized of sysfile, > will wakeup usb4 driver, but fail to disconnect pcie due to > the PCIe tunnel had already been torn down and freed if the port > is fail to resume: > > The check prevents a redundant lookup and eliminates the warning > > Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com> Sashiko has reviewed this patch and found no issues. -- Sashiko AI review · https://sashiko.dev/#/patchset/20261030082539.1208733-1-chunfeng.yun@mediatek.com?part=4 ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2026-10-30 9:01 UTC | newest] Thread overview: 13+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-10-30 8:25 [PATCH 0/4] add support platform nhi driver Chunfeng Yun 2026-10-30 8:25 ` [PATCH 1/4] thunderbolt: add " Chunfeng Yun 2026-10-30 8:40 ` sashiko-bot 2026-10-30 8:47 ` Mika Westerberg 2026-10-30 8:25 ` [PATCH 2/4] thunderbolt: add support optional dedicated wakeup irq Chunfeng Yun 2026-10-30 8:48 ` Mika Westerberg 2026-10-30 8:50 ` sashiko-bot 2026-10-30 8:25 ` [PATCH 3/4] thunderbolt: improve usb3 adapter link up timeout value Chunfeng Yun 2026-10-30 8:50 ` Mika Westerberg 2026-10-30 8:54 ` sashiko-bot 2026-10-30 8:25 ` [PATCH 4/4] thunderbolt: prevent pcie tunnel activation/teardown warnings Chunfeng Yun 2026-10-30 8:56 ` Mika Westerberg 2026-10-30 9:01 ` sashiko-bot
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.