Re: [PATCH 15/17] i3c: mipi-i3c-hci: Support configurable device NACK retries

[Frank Li]

On Mon, Sep 14, 2026 at 02:30:01PM +0300, Adrian Hunter wrote:
> Implement the .set_dev_nack_retry() master operation using the
> DAT_0_DEV_NACK_RETRY_CNT field in Device Address Table entries.
>
> Since the retry count is programmed per DAT entry, update all allocated
> entries when the setting changes and initialize newly allocated entries
> with the current value so that a consistent retry policy is applied
> across all devices.
>
> Add DAT helper operations to update individual entries and all allocated
> entries.  Return -ERANGE if the requested retry count exceeds the
> hardware field width and -EOPNOTSUPP when the active command descriptor
> model does not use DAT entries.
>
> In addition, default the retry count to 1.  The I3C specification
> mandates a retry when a Target NACKs its Dynamic Address.  HCI v1.1
> explicitly preserves that behaviour for Direct CCCs even when
> DEV_NACK_RETRY_CNT is programmed to 0, but HCI v1.0 defines the field
> only as a device-specific retry count and does not provide the same
> exception.  As a result, a v1.0 controller left at the reset value of 0
> may perform no retry whereas a v1.1 controller will retry once.
> Promoting 0 to 1 at probe time makes the behaviour consistent across
> controller versions.  Userspace can still select 0 retries explicitly
> via dev_nack_retry_count.
>
> Signed-off-by: Adrian Hunter <adrian.hunter@xxxxxxxxx>
> ---
>  drivers/i3c/master/mipi-i3c-hci/core.c   | 25 ++++++++++++++++++++++++
>  drivers/i3c/master/mipi-i3c-hci/dat.h    |  2 ++
>  drivers/i3c/master/mipi-i3c-hci/dat_v1.c | 25 ++++++++++++++++++++++++
>  3 files changed, 52 insertions(+)
>
> diff --git a/drivers/i3c/master/mipi-i3c-hci/core.c b/drivers/i3c/master/mipi-i3c-hci/core.c
> index 7a39be64c4e1..57d84d7682d7 100644
> --- a/drivers/i3c/master/mipi-i3c-hci/core.c
> +++ b/drivers/i3c/master/mipi-i3c-hci/core.c
> @@ -555,6 +555,11 @@ static int i3c_hci_i2c_xfers(struct i2c_dev_desc *dev,
>  	return ret;
>  }
>
> +static void i3c_hci_dat_v1_set_curr_nack_retry(struct i3c_hci *hci, unsigned int dat_idx)
> +{
> +	mipi_i3c_hci_dat_v1.set_nack_retry(hci, dat_idx, hci->master.dev_nack_retry_count);
> +}
> +
>  static int i3c_hci_attach_i3c_dev(struct i3c_dev_desc *dev)
>  {
>  	struct i3c_master_controller *m = i3c_dev_get_master(dev);
> @@ -573,6 +578,7 @@ static int i3c_hci_attach_i3c_dev(struct i3c_dev_desc *dev)
>  		}
>  		mipi_i3c_hci_dat_v1.set_dynamic_addr(hci, ret,
>  						     dev->info.dyn_addr ?: dev->info.static_addr);
> +		i3c_hci_dat_v1_set_curr_nack_retry(hci, ret);
>  		dev_data->dat_idx = ret;
>  	}
>  	i3c_dev_set_master_data(dev, dev_data);
> @@ -622,6 +628,7 @@ static int i3c_hci_attach_i2c_dev(struct i2c_dev_desc *dev)
>  	}
>  	mipi_i3c_hci_dat_v1.set_static_addr(hci, ret, dev->addr);
>  	mipi_i3c_hci_dat_v1.set_flags(hci, ret, DAT_0_I2C_DEVICE, 0);
> +	i3c_hci_dat_v1_set_curr_nack_retry(hci, ret);
>  	dev_data->dat_idx = ret;
>  	i2c_dev_set_master_data(dev, dev_data);
>  	return 0;
> @@ -733,6 +740,16 @@ static void i3c_hci_recycle_ibi_slot(struct i3c_dev_desc *dev,
>  	hci->io->recycle_ibi_slot(hci, dev, slot);
>  }
>
> +static int i3c_hci_set_dev_nack_retry(struct i3c_master_controller *m, unsigned int cnt)
> +{
> +	struct i3c_hci *hci = to_i3c_hci(m);
> +
> +	if (hci->cmd != &mipi_i3c_hci_cmd_v1)
> +		return -EOPNOTSUPP;
> +
> +	return mipi_i3c_hci_dat_v1.set_all_nack_retry(hci, cnt);
> +}
> +
>  static const struct i3c_master_controller_ops i3c_hci_ops = {
>  	.bus_init		= i3c_hci_bus_init,
>  	.bus_cleanup		= i3c_hci_bus_cleanup,
> @@ -752,6 +769,7 @@ static const struct i3c_master_controller_ops i3c_hci_ops = {
>  	.recycle_ibi_slot	= i3c_hci_recycle_ibi_slot,
>  	.enable_hotjoin		= i3c_hci_enable_hotjoin,
>  	.disable_hotjoin	= i3c_hci_disable_hotjoin,
> +	.set_dev_nack_retry	= i3c_hci_set_dev_nack_retry,
>  };
>
>  static irqreturn_t i3c_hci_irq_handler(int irq, void *dev_id)
> @@ -1186,6 +1204,13 @@ static int i3c_hci_probe(struct platform_device *pdev)
>  	if (device_can_wakeup(i3c_hci_sysdev(&pdev->dev)))
>  		hci->master.ibi_wakeup = true;
>
> +	/*
> +	 * HCI v1.1 onward does 1 retry for Direct CCCs anyway, so for v1.0 to
> +	 * be consistent, promote 0 to 1.
> +	 */
> +	if (!hci->master.dev_nack_retry_count)
> +		hci->master.dev_nack_retry_count = 1;
> +
>  	return i3c_master_register(&hci->master, &pdev->dev, &i3c_hci_ops, false);
>  }
>
> diff --git a/drivers/i3c/master/mipi-i3c-hci/dat.h b/drivers/i3c/master/mipi-i3c-hci/dat.h
> index 6881f19da77f..a07086bf717a 100644
> --- a/drivers/i3c/master/mipi-i3c-hci/dat.h
> +++ b/drivers/i3c/master/mipi-i3c-hci/dat.h
> @@ -25,6 +25,8 @@ struct hci_dat_ops {
>  	void (*clear_flags)(struct i3c_hci *hci, unsigned int dat_idx, u32 w0, u32 w1);
>  	int (*get_index)(struct i3c_hci *hci, u8 address);
>  	void (*restore)(struct i3c_hci *hci);
> +	void (*set_nack_retry)(struct i3c_hci *hci, unsigned int dat_idx, unsigned int cnt);
> +	int (*set_all_nack_retry)(struct i3c_hci *hci, unsigned int cnt);
>  };
>
>  extern const struct hci_dat_ops mipi_i3c_hci_dat_v1;
> diff --git a/drivers/i3c/master/mipi-i3c-hci/dat_v1.c b/drivers/i3c/master/mipi-i3c-hci/dat_v1.c
> index 852966aa20d9..cb9aa43e69fb 100644
> --- a/drivers/i3c/master/mipi-i3c-hci/dat_v1.c
> +++ b/drivers/i3c/master/mipi-i3c-hci/dat_v1.c
> @@ -189,6 +189,29 @@ static void hci_dat_v1_restore(struct i3c_hci *hci)
>  	}
>  }
>
> +static void hci_dat_v1_set_nack_retry(struct i3c_hci *hci, unsigned int dat_idx, unsigned int cnt)
> +{
> +	u32 dat_w0;
> +
> +	dat_w0 = dat_w0_read(dat_idx);
> +	dat_w0 &= ~DAT_0_DEV_NACK_RETRY_CNT;
> +	dat_w0 |= FIELD_PREP(DAT_0_DEV_NACK_RETRY_CNT, cnt);
> +	dat_w0_write(dat_idx, dat_w0);
> +}
> +
> +static int hci_dat_v1_set_all_nack_retry(struct i3c_hci *hci, unsigned int cnt)
> +{

do you have other implement for set_all_nack_retry()?

Frank
> +	unsigned int dat_idx;
> +
> +	if (cnt > FIELD_MAX(DAT_0_DEV_NACK_RETRY_CNT))
> +		return -ERANGE;
> +
> +	for_each_set_bit(dat_idx, hci->DAT_data, hci->DAT_entries)
> +		hci_dat_v1_set_nack_retry(hci, dat_idx, cnt);
> +
> +	return 0;
> +}
> +
>  const struct hci_dat_ops mipi_i3c_hci_dat_v1 = {
>  	.init			= hci_dat_v1_init,
>  	.alloc_entry		= hci_dat_v1_alloc_entry,
> @@ -199,4 +222,6 @@ const struct hci_dat_ops mipi_i3c_hci_dat_v1 = {
>  	.clear_flags		= hci_dat_v1_clear_flags,
>  	.get_index		= hci_dat_v1_get_index,
>  	.restore		= hci_dat_v1_restore,
> +	.set_nack_retry		= hci_dat_v1_set_nack_retry,
> +	.set_all_nack_retry	= hci_dat_v1_set_all_nack_retry,
>  };
> --
> 2.53.0
>