Re: [PATCH 1/4] thunderbolt: add platform nhi driver

From: Chunfeng Yun (云春峰)

Date: Wed Nov 04 2026 - 02:34:33 EST

On Fri, 2026-10-30 at 09:47 +0100, Mika Westerberg wrote:
> 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@xxxxxxxxxxxx>
> > ---
> > 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
OK
>
> > 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?
if do not add this functions, there is an pm count overflow, it seems
happens on platform driver, I will re-check it.

>
> > 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@xxxxxxxxxxxx>
> > + */
> > +
> > +#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_
OK

>
> > +{
> > + /*
> > + * avoid device link fail for pcie tunnel
> > + */
>
> What's this?
explain why call these functions, without enable pm of nhi->dev,
tb_acpi_add_link() -> device_link_add() will fail

> And the comment style is not the expected.
will fix it.

>
> > + 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.
No, just enable the device's pm and get a count to link pcie/usb4's
device, and will release later.

>
> > +
> > + 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,... ?
still need a common one, and also add a mediatek,thunderbolt?

but the current platform is running on uefi/acpi but not based on dts,

should I add a dt-binding for it?

>
> > + { }
> > +};
> > +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?
see acpi spec release 6.5, section 5.6.7 Device Class-Specific Objects,
Table 5.205: ACPI Device IDs

ACPI0015
USB4 host interface device. See Links to ACPI-Related Documents under
the heading “USB4 Host Interface Specification”

>
> > + { }
> > +};
> > +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.
will fix it.

Thanks

>
> > struct work_struct interrupt_work;
> > u32 hop_count;
> > unsigned long quirks;
> > --
> > 2.43.0
>