Re: [PATCH v2 2/7] thunderbolt: Move ICM mailbox functions to icm.c and internalize interrupt work

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

On Tue, Nov 10, 2026 at 07:48:50PM +0800, Chunfeng Yun wrote:
> In preparation for modularizing PCIe NHI and ICM support as a separate
> driver, clean up internal helper functions so that they do not need to
> be exported:
> 
> nhi_mailbox_cmd() and nhi_mailbox_mode() are only used by the Intel
> firmware Connection Manager (icm.c) to exchange mailbox commands with
> firmware running on Intel PCIe NHIs. The Thunderbolt core framework,
> software connection manager, and non-PCIe platform controllers do
> not use this mailbox interface. Move both functions into icm.c and
> mark them static.
> 
> nhi_interrupt_work was previously initialized in pci.c within
> nhi_pci_init_msi(), requiring it to be visible across files. Move
> INIT_WORK(&nhi->interrupt_work, ...) into tb_nhi_probe() so that
> the workqueue item is initialized by the NHI core and
> nhi_interrupt_work() can be marked static in nhi.c.
> 
> Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> ---
> new patch
> ---
>  drivers/thunderbolt/icm.c | 59 ++++++++++++++++++++++++++++++++++++
>  drivers/thunderbolt/nhi.c | 64 ++-------------------------------------
>  drivers/thunderbolt/nhi.h |  3 --
>  drivers/thunderbolt/pci.c |  2 --
>  4 files changed, 62 insertions(+), 66 deletions(-)
> 
> diff --git a/drivers/thunderbolt/icm.c b/drivers/thunderbolt/icm.c
> index a295a5e6cc9c..736f551cd567 100644
> --- a/drivers/thunderbolt/icm.c
> +++ b/drivers/thunderbolt/icm.c
> @@ -45,6 +45,7 @@
>  #define ICM_RETRIES			3
>  #define ICM_APPROVE_TIMEOUT		10000	/* ms */
>  #define ICM_MAX_LINK			4
> +#define ICM_MAILBOX_TIMEOUT		500 /* ms */
>  
>  static bool start_icm;
>  module_param(start_icm, bool, 0444);
> @@ -269,6 +270,64 @@ static int pcie2cio_write(struct icm *icm, enum tb_cfg_space cs,
>  	return pci2cio_wait_completion(icm, 5000);
>  }
>  
> +/**
> + * nhi_mailbox_cmd() - Send a command through NHI mailbox
> + * @nhi: Pointer to the NHI structure
> + * @cmd: Command to send
> + * @data: Data to be send with the command
> + *
> + * Sends mailbox command to the firmware running on NHI.
> + *
> + * Return: %0 on success, negative errno otherwise.
> + */
> +static int nhi_mailbox_cmd(struct tb_nhi *nhi, enum nhi_mailbox_cmd cmd, u32 data)
> +{
> +	ktime_t timeout;
> +	u32 val;
> +
> +	iowrite32(data, nhi->iobase + REG_INMAIL_DATA);
> +
> +	val = ioread32(nhi->iobase + REG_INMAIL_CMD);
> +	val &= ~(REG_INMAIL_CMD_MASK | REG_INMAIL_ERROR);
> +	val |= REG_INMAIL_OP_REQUEST | cmd;
> +	iowrite32(val, nhi->iobase + REG_INMAIL_CMD);
> +
> +	timeout = ktime_add_ms(ktime_get(), ICM_MAILBOX_TIMEOUT);
> +	do {
> +		val = ioread32(nhi->iobase + REG_INMAIL_CMD);
> +		if (!(val & REG_INMAIL_OP_REQUEST))
> +			break;
> +		usleep_range(10, 20);
> +	} while (ktime_before(ktime_get(), timeout));
> +
> +	if (val & REG_INMAIL_OP_REQUEST)
> +		return -ETIMEDOUT;
> +	if (val & REG_INMAIL_ERROR)
> +		return -EIO;
> +
> +	return 0;
> +}
> +
> +/**
> + * nhi_mailbox_mode() - Return current firmware operation mode
> + * @nhi: Pointer to the NHI structure
> + *
> + * The function reads current firmware operation mode using NHI mailbox
> + * registers and returns it to the caller.
> + *
> + * Return: &enum nhi_fw_mode.
> + */
> +static enum nhi_fw_mode nhi_mailbox_mode(struct tb_nhi *nhi)
> +{
> +	u32 val;
> +
> +	val = ioread32(nhi->iobase + REG_OUTMAIL_CMD);
> +	val &= REG_OUTMAIL_CMD_OPMODE_MASK;
> +	val >>= REG_OUTMAIL_CMD_OPMODE_SHIFT;
> +
> +	return (enum nhi_fw_mode)val;
> +}
> +
>  static bool icm_match(const struct tb_cfg_request *req,
>  		      const struct ctl_pkg *pkg)
>  {
> diff --git a/drivers/thunderbolt/nhi.c b/drivers/thunderbolt/nhi.c
> index c574c336f25b..1e3a58636d3d 100644
> --- a/drivers/thunderbolt/nhi.c
> +++ b/drivers/thunderbolt/nhi.c
> @@ -35,8 +35,6 @@
>   */
>  #define RING_E2E_RESERVED_HOPID	RING_FIRST_USABLE_HOPID
>  
> -#define NHI_MAILBOX_TIMEOUT	500 /* ms */
> -
>  static bool host_reset = true;
>  module_param(host_reset, bool, 0444);
>  MODULE_PARM_DESC(host_reset, "reset USB4 host router (default: true)");
> @@ -964,65 +962,7 @@ int tb_ring_throttling(struct tb_ring *ring, unsigned int interval_nsec)
>  }
>  EXPORT_SYMBOL_GPL(tb_ring_throttling);
>  
> -/**
> - * nhi_mailbox_cmd() - Send a command through NHI mailbox
> - * @nhi: Pointer to the NHI structure
> - * @cmd: Command to send
> - * @data: Data to be send with the command
> - *
> - * Sends mailbox command to the firmware running on NHI.
> - *
> - * Return: %0 on success, negative errno otherwise.
> - */
> -int nhi_mailbox_cmd(struct tb_nhi *nhi, enum nhi_mailbox_cmd cmd, u32 data)
> -{
> -	ktime_t timeout;
> -	u32 val;
> -
> -	iowrite32(data, nhi->iobase + REG_INMAIL_DATA);
> -
> -	val = ioread32(nhi->iobase + REG_INMAIL_CMD);
> -	val &= ~(REG_INMAIL_CMD_MASK | REG_INMAIL_ERROR);
> -	val |= REG_INMAIL_OP_REQUEST | cmd;
> -	iowrite32(val, nhi->iobase + REG_INMAIL_CMD);
> -
> -	timeout = ktime_add_ms(ktime_get(), NHI_MAILBOX_TIMEOUT);
> -	do {
> -		val = ioread32(nhi->iobase + REG_INMAIL_CMD);
> -		if (!(val & REG_INMAIL_OP_REQUEST))
> -			break;
> -		usleep_range(10, 20);
> -	} while (ktime_before(ktime_get(), timeout));
> -
> -	if (val & REG_INMAIL_OP_REQUEST)
> -		return -ETIMEDOUT;
> -	if (val & REG_INMAIL_ERROR)
> -		return -EIO;
> -
> -	return 0;
> -}
> -
> -/**
> - * nhi_mailbox_mode() - Return current firmware operation mode
> - * @nhi: Pointer to the NHI structure
> - *
> - * The function reads current firmware operation mode using NHI mailbox
> - * registers and returns it to the caller.
> - *
> - * Return: &enum nhi_fw_mode.
> - */
> -enum nhi_fw_mode nhi_mailbox_mode(struct tb_nhi *nhi)
> -{
> -	u32 val;
> -
> -	val = ioread32(nhi->iobase + REG_OUTMAIL_CMD);
> -	val &= REG_OUTMAIL_CMD_OPMODE_MASK;
> -	val >>= REG_OUTMAIL_CMD_OPMODE_SHIFT;
> -
> -	return (enum nhi_fw_mode)val;
> -}
> -
> -void nhi_interrupt_work(struct work_struct *work)
> +static void nhi_interrupt_work(struct work_struct *work)
>  {
>  	struct tb_nhi *nhi = container_of(work, typeof(*nhi), interrupt_work);
>  	int value = 0; /* Suppress uninitialized usage warning. */
> @@ -1331,6 +1271,8 @@ int tb_nhi_probe(struct tb_nhi *nhi)
>  	if (!nhi->tx_rings || !nhi->rx_rings || !nhi->interrupt_mask)
>  		return -ENOMEM;
>  
> +    INIT_WORK(&nhi->interrupt_work, nhi_interrupt_work);
> +

This has weird indent.

>  	nhi_reset(nhi);
>  
>  	/* In case someone left them on. */
> diff --git a/drivers/thunderbolt/nhi.h b/drivers/thunderbolt/nhi.h
> index a36c62c49a5c..3dd6ace6150d 100644
> --- a/drivers/thunderbolt/nhi.h
> +++ b/drivers/thunderbolt/nhi.h
> @@ -27,11 +27,8 @@ enum nhi_mailbox_cmd {
>  	NHI_MAILBOX_ALLOW_ALL_DEVS = 0x23,
>  };
>  
> -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 tb_nhi_isr(int irq, void *data);
>  irqreturn_t tb_ring_msix(int irq, void *data);
>  int tb_nhi_probe(struct tb_nhi *nhi);
> diff --git a/drivers/thunderbolt/pci.c b/drivers/thunderbolt/pci.c
> index 2f0abf4d27a3..275389903090 100644
> --- a/drivers/thunderbolt/pci.c
> +++ b/drivers/thunderbolt/pci.c
> @@ -206,8 +206,6 @@ static int nhi_pci_init_msi(struct tb_nhi *nhi)
>  		if (nvec < 0)
>  			return nvec;
>  
> -		INIT_WORK(&nhi->interrupt_work, nhi_interrupt_work);
> -
>  		irq = pci_irq_vector(pdev, 0);
>  		if (irq < 0)
>  			return irq;
> -- 
> 2.45.2