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

["Chunfeng Yun (云春峰)" <Chunfeng.Yun@mediatek.com>]

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@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
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@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_
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
>