[PATCH 15/17] i3c: mipi-i3c-hci: Support configurable device NACK retries
Adrian Hunter
adrian.hunter at intel.com
Tue Sep 15 02:41:10 PDT 2026
On 14/09/2026 21:21, Frank Li wrote:
> 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 at intel.com>
>> ---
>> 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()?
If you mean the naming "dat_v1", that is just a common name for the
DAT implementation, presumably assuming there might be a dat_v2 one
day.
If you mean adding the function to struct hci_dat_ops, that is just
for consistency with how all other DAT functions are accessed.
>
> 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
>>
More information about the linux-i3c
mailing list