[PATCH v3 0/3] i3c: dw-i3c: Enable support for dw-i3c controller NACK retry sysfs and DAT restore fix

[PATCH v3 0/3] i3c: dw-i3c: Enable support for dw-i3c controller NACK retry sysfs and DAT restore fix

[adrianhoyin.ng]

From: Adrian Ng Ho Yin <adrianhoyin.ng@altera.com>

This patch series adds a controller-wide sysfs attribute
dev_nack_retry_count for the DesignWare I3C controller, allowing runtime
control of the automatic retry mechanism when a device NACKs. Some I3C
slave devices may temporarily be busy and unable to respond immediately;
automatic retries improve robustness in such cases. Writes are clamped to
the hardware maximum of 3, and the value is applied to all active DAT
entries.

The series also fixes dw_i3c_master_restore_addrs() to preserve existing
DAT entry bits, preventing overwrites during runtime PM resume.
---
changelog:
v2->v3
* Update commit message for better clarity
* Update function name to maintain consistency.
* Update store function to return error when retry value exceeds hw limit.
* Add lock to protect DAT to avoid concurrent during transfers.
* Clear the address field in the DAT entry before setting new values to
  ensure correct dynamic/static address configuration

v2 patch link:
https://lore.kernel.org/all/cover.1763703573.git.adrianhoyin.ng@altera.com/

v1->v2
* Drop dev_nack_retry_cnt binding and device tree changes.
* Update commit message for better clarity.
* Update to use controller wide sysfs attribute that configures
  dev_nack_retry_cnt during runtime.

v1 patch link:
https://lore.kernel.org/all/cover.1762245890.git.adrianhoyin.ng@altera.com/
---

Adrian Ng Ho Yin (3):
  i3c: add sysfs entry for Device NACK Retry count
  i3c: dw: Add sysfs support for Device NACK Retry count
  i3c: dw: Preserve DAT entry bits when restoring addresses

 Documentation/ABI/testing/sysfs-bus-i3c | 12 ++++
 drivers/i3c/master/dw-i3c-master.c      | 82 +++++++++++++++++++++++--
 drivers/i3c/master/dw-i3c-master.h      |  1 +
 3 files changed, 91 insertions(+), 4 deletions(-)

-- 
2.49.GIT

[PATCH v3 1/3] i3c: add sysfs entry for Device NACK Retry count

[adrianhoyin.ng]

From: Adrian Ng Ho Yin <adrianhoyin.ng@altera.com>

Document sysfs attribute dev_nack_retry_cnt that controls the number of
automatic retries performed by the I3C controller when a target device
returns a NACK

Signed-off-by: Adrian Ng Ho Yin <adrianhoyin.ng@altera.com>
---
 Documentation/ABI/testing/sysfs-bus-i3c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-bus-i3c b/Documentation/ABI/testing/sysfs-bus-i3c
index c812ab180ff4..0b8a0c223f4a 100644
--- a/Documentation/ABI/testing/sysfs-bus-i3c
+++ b/Documentation/ABI/testing/sysfs-bus-i3c
@@ -161,3 +161,15 @@ Contact:	linux-i3c@vger.kernel.org
 Description:
 		These directories are just symbolic links to
 		/sys/bus/i3c/devices/i3c-<bus-id>/<bus-id>-<device-pid>.
+
+What:		/sys/bus/i3c/devices/i3c-<bus-id>/<bus-id>-<device-pid>/dev_nack_retry_count
+KernelVersion:  6.18
+Contact:	linux-i3c@vger.kernel.org
+Description:
+		Expose the dev_nak_retry_count which controls the number of
+		automatic retries that will be performed by the controller when
+		the target device returns a NACK response. A value of 0 disables
+		the automatic retries. A max value of 3 can be configured. Exist
+		only when I3C constroller supports this retry on nack feature.
+
+		Valid values: 0-3
-- 
2.49.GIT

[PATCH v3 2/3] i3c: dw: Add sysfs support for Device NACK Retry count

[adrianhoyin.ng]

From: Adrian Ng Ho Yin <adrianhoyin.ng@altera.com>

The DesignWare I3C controller supports automatically retrying transactions
when a device NACKs. This is useful for slave devices that may be
temporarily busy and not ready to respond immediately.

Adds a controller-wide sysfs attribute, dev_nack_retry_count, to read or
adjust the retry count at runtime. Returns error when value exceeds hw
specified limit, and the updated value is programmed into all active DAT
entries.

Signed-off-by: Adrian Ng Ho Yin <adrianhoyin.ng@altera.com>
---
 drivers/i3c/master/dw-i3c-master.c | 69 ++++++++++++++++++++++++++++++
 drivers/i3c/master/dw-i3c-master.h |  1 +
 2 files changed, 70 insertions(+)

diff --git a/drivers/i3c/master/dw-i3c-master.c b/drivers/i3c/master/dw-i3c-master.c
index 9ceedf09c3b6..e228c60840af 100644
--- a/drivers/i3c/master/dw-i3c-master.c
+++ b/drivers/i3c/master/dw-i3c-master.c
@@ -204,8 +204,10 @@
 #define EXTENDED_CAPABILITY		0xe8
 #define SLAVE_CONFIG			0xec
 
+#define DW_I3C_DEV_NACK_RETRY_CNT_MAX	0x3
 #define DEV_ADDR_TABLE_IBI_MDB		BIT(12)
 #define DEV_ADDR_TABLE_SIR_REJECT	BIT(13)
+#define DEV_ADDR_TABLE_DEV_NACK_RETRY_CNT(x)	(((x) << 29) & GENMASK(30, 29))
 #define DEV_ADDR_TABLE_LEGACY_I2C_DEV	BIT(31)
 #define DEV_ADDR_TABLE_DYNAMIC_ADDR(x)	(((x) << 16) & GENMASK(23, 16))
 #define DEV_ADDR_TABLE_STATIC_ADDR(x)	((x) & GENMASK(6, 0))
@@ -295,6 +297,64 @@ to_dw_i3c_master(struct i3c_master_controller *master)
 	return container_of(master, struct dw_i3c_master, base);
 }
 
+static ssize_t dw_dev_nack_retry_count_show(struct device *dev,
+					    struct device_attribute *attr,
+					    char *buf)
+{
+	struct dw_i3c_master *master = dev_get_drvdata(dev);
+
+	return sysfs_emit(buf, "%u\n", master->dev_nack_retry_cnt);
+}
+
+static ssize_t dw_dev_nack_retry_count_store(struct device *dev,
+					     struct device_attribute *attr,
+					     const char *buf, size_t count)
+{
+	struct dw_i3c_master *master = dev_get_drvdata(dev);
+	unsigned long val, flags;
+	int ret, i;
+	u32 reg;
+
+	ret = kstrtoul(buf, 0, &val);
+	if (ret)
+		return ret;
+
+	if (val > DW_I3C_DEV_NACK_RETRY_CNT_MAX) {
+		dev_err(dev,
+			"Value %lu exceeds maximum %d\n",
+			val, DW_I3C_DEV_NACK_RETRY_CNT_MAX);
+		return -ERANGE;
+	}
+
+	master->dev_nack_retry_cnt = val;
+
+	spin_lock_irqsave(&master->devs_lock, flags);
+	/*
+	 * Update DAT entries for all currently attached devices.
+	 * We directly iterate through the master's device array.
+	 */
+	for (i = 0; i < master->maxdevs; i++) {
+		/* Skip free/empty slots */
+		if (master->free_pos & BIT(i))
+			continue;
+
+		reg = readl(master->regs +
+				DEV_ADDR_TABLE_LOC(master->datstartaddr, i));
+		reg &= ~GENMASK(30, 29);
+		reg |= DEV_ADDR_TABLE_DEV_NACK_RETRY_CNT(val);
+		writel(reg, master->regs +
+			DEV_ADDR_TABLE_LOC(master->datstartaddr, i));
+	}
+	spin_unlock_irqrestore(&master->devs_lock, flags);
+
+	return count;
+}
+
+static struct device_attribute dev_attr_dev_nack_retry_count =
+	__ATTR(dev_nack_retry_count, 0644,
+	       dw_dev_nack_retry_count_show,
+	       dw_dev_nack_retry_count_store);
+
 static void dw_i3c_master_disable(struct dw_i3c_master *master)
 {
 	writel(readl(master->regs + DEVICE_CTRL) & ~DEV_CTRL_ENABLE,
@@ -1598,6 +1658,12 @@ int dw_i3c_common_probe(struct dw_i3c_master *master,
 	if (ret)
 		goto err_disable_pm;
 
+	dev_set_drvdata(&master->base.dev, master);
+	ret = device_create_file(&master->base.dev, &dev_attr_dev_nack_retry_count);
+	if (ret)
+		dev_warn(&master->base.dev,
+			 "Failed to create dev_nack_retry_count sysfs: %d\n", ret);
+
 	return 0;
 
 err_disable_pm:
@@ -1617,6 +1683,9 @@ void dw_i3c_common_remove(struct dw_i3c_master *master)
 	cancel_work_sync(&master->hj_work);
 	i3c_master_unregister(&master->base);
 
+	device_remove_file(&master->base.dev, &dev_attr_dev_nack_retry_count);
+	dev_set_drvdata(&master->base.dev, NULL);
+
 	pm_runtime_disable(master->dev);
 	pm_runtime_set_suspended(master->dev);
 	pm_runtime_dont_use_autosuspend(master->dev);
diff --git a/drivers/i3c/master/dw-i3c-master.h b/drivers/i3c/master/dw-i3c-master.h
index c5cb695c16ab..45fc1774724a 100644
--- a/drivers/i3c/master/dw-i3c-master.h
+++ b/drivers/i3c/master/dw-i3c-master.h
@@ -51,6 +51,7 @@ struct dw_i3c_master {
 	u32 i2c_fm_timing;
 	u32 i2c_fmp_timing;
 	u32 quirks;
+	u32 dev_nack_retry_cnt;
 	/*
 	 * Per-device hardware data, used to manage the device address table
 	 * (DAT)
-- 
2.49.GIT

[PATCH v3 3/3] i3c: dw: Preserve DAT entry bits when restoring addresses

[adrianhoyin.ng]

From: Adrian Ng Ho Yin <adrianhoyin.ng@altera.com>

Update dw_i3c_master_restore_addrs() to preserve existing bits in each
Device Address Table (DAT) entry when restoring addresses. This prevents
overwriting configuration bits during PM runtime resumes.

Signed-off-by: Adrian Ng Ho Yin <adrianhoyin.ng@altera.com>
---
 drivers/i3c/master/dw-i3c-master.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/i3c/master/dw-i3c-master.c b/drivers/i3c/master/dw-i3c-master.c
index e228c60840af..37275f7b6c61 100644
--- a/drivers/i3c/master/dw-i3c-master.c
+++ b/drivers/i3c/master/dw-i3c-master.c
@@ -1723,11 +1723,16 @@ static void dw_i3c_master_restore_addrs(struct dw_i3c_master *master)
 		if (master->free_pos & BIT(pos))
 			continue;
 
-		if (master->devs[pos].is_i2c_addr)
-			reg_val = DEV_ADDR_TABLE_LEGACY_I2C_DEV |
+		reg_val = readl(master->regs + DEV_ADDR_TABLE_LOC(master->datstartaddr, pos));
+
+		if (master->devs[pos].is_i2c_addr) {
+			reg_val &= ~(GENMASK(6, 0));
+			reg_val |= DEV_ADDR_TABLE_LEGACY_I2C_DEV |
 			       DEV_ADDR_TABLE_STATIC_ADDR(master->devs[pos].addr);
-		else
-			reg_val = DEV_ADDR_TABLE_DYNAMIC_ADDR(master->devs[pos].addr);
+		} else {
+			reg_val &= ~(GENMASK(23, 16));
+			reg_val |= DEV_ADDR_TABLE_DYNAMIC_ADDR(master->devs[pos].addr);
+		}
 
 		writel(reg_val, master->regs + DEV_ADDR_TABLE_LOC(master->datstartaddr, pos));
 	}
-- 
2.49.GIT

Re: [PATCH v3 1/3] i3c: add sysfs entry for Device NACK Retry count

[Frank Li]

On Sat, Nov 22, 2025 at 02:00:39AM +0800, adrianhoyin.ng@altera.com wrote:
> From: Adrian Ng Ho Yin <adrianhoyin.ng@altera.com>
>
> Document sysfs attribute dev_nack_retry_cnt that controls the number of
> automatic retries performed by the I3C controller when a target device
> returns a NACK
>
> Signed-off-by: Adrian Ng Ho Yin <adrianhoyin.ng@altera.com>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.com>

>  Documentation/ABI/testing/sysfs-bus-i3c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
>
> diff --git a/Documentation/ABI/testing/sysfs-bus-i3c b/Documentation/ABI/testing/sysfs-bus-i3c
> index c812ab180ff4..0b8a0c223f4a 100644
> --- a/Documentation/ABI/testing/sysfs-bus-i3c
> +++ b/Documentation/ABI/testing/sysfs-bus-i3c
> @@ -161,3 +161,15 @@ Contact:	linux-i3c@vger.kernel.org
>  Description:
>  		These directories are just symbolic links to
>  		/sys/bus/i3c/devices/i3c-<bus-id>/<bus-id>-<device-pid>.
> +
> +What:		/sys/bus/i3c/devices/i3c-<bus-id>/<bus-id>-<device-pid>/dev_nack_retry_count
> +KernelVersion:  6.18
> +Contact:	linux-i3c@vger.kernel.org
> +Description:
> +		Expose the dev_nak_retry_count which controls the number of
> +		automatic retries that will be performed by the controller when
> +		the target device returns a NACK response. A value of 0 disables
> +		the automatic retries. A max value of 3 can be configured. Exist
> +		only when I3C constroller supports this retry on nack feature.
> +
> +		Valid values: 0-3
> --
> 2.49.GIT
>

Re: [PATCH v3 2/3] i3c: dw: Add sysfs support for Device NACK Retry count

[Frank Li]

On Sat, Nov 22, 2025 at 02:00:40AM +0800, adrianhoyin.ng@altera.com wrote:
> From: Adrian Ng Ho Yin <adrianhoyin.ng@altera.com>
>
> The DesignWare I3C controller supports automatically retrying transactions
> when a device NACKs. This is useful for slave devices that may be
> temporarily busy and not ready to respond immediately.
>
> Adds a controller-wide sysfs attribute, dev_nack_retry_count, to read or
> adjust the retry count at runtime. Returns error when value exceeds hw
> specified limit, and the updated value is programmed into all active DAT
> entries.
>
> Signed-off-by: Adrian Ng Ho Yin <adrianhoyin.ng@altera.com>
> ---
>  drivers/i3c/master/dw-i3c-master.c | 69 ++++++++++++++++++++++++++++++
>  drivers/i3c/master/dw-i3c-master.h |  1 +
>  2 files changed, 70 insertions(+)
>
> diff --git a/drivers/i3c/master/dw-i3c-master.c b/drivers/i3c/master/dw-i3c-master.c
> index 9ceedf09c3b6..e228c60840af 100644
> --- a/drivers/i3c/master/dw-i3c-master.c
> +++ b/drivers/i3c/master/dw-i3c-master.c
> @@ -204,8 +204,10 @@
>  #define EXTENDED_CAPABILITY		0xe8
>  #define SLAVE_CONFIG			0xec
>
> +#define DW_I3C_DEV_NACK_RETRY_CNT_MAX	0x3
>  #define DEV_ADDR_TABLE_IBI_MDB		BIT(12)
>  #define DEV_ADDR_TABLE_SIR_REJECT	BIT(13)
> +#define DEV_ADDR_TABLE_DEV_NACK_RETRY_CNT(x)	(((x) << 29) & GENMASK(30, 29))
>  #define DEV_ADDR_TABLE_LEGACY_I2C_DEV	BIT(31)
>  #define DEV_ADDR_TABLE_DYNAMIC_ADDR(x)	(((x) << 16) & GENMASK(23, 16))
>  #define DEV_ADDR_TABLE_STATIC_ADDR(x)	((x) & GENMASK(6, 0))
> @@ -295,6 +297,64 @@ to_dw_i3c_master(struct i3c_master_controller *master)
>  	return container_of(master, struct dw_i3c_master, base);
>  }
>
...
> +static ssize_t dw_dev_nack_retry_count_store(struct device *dev,
> +					     struct device_attribute *attr,
> +					     const char *buf, size_t count)
> +{
> +	struct dw_i3c_master *master = dev_get_drvdata(dev);
> +	unsigned long val, flags;
> +	int ret, i;
> +	u32 reg;
> +
> +	ret = kstrtoul(buf, 0, &val);
> +	if (ret)
> +		return ret;
> +
> +	if (val > DW_I3C_DEV_NACK_RETRY_CNT_MAX) {
> +		dev_err(dev,
> +			"Value %lu exceeds maximum %d\n",
> +			val, DW_I3C_DEV_NACK_RETRY_CNT_MAX);
> +		return -ERANGE;
> +	}
> +
> +	master->dev_nack_retry_cnt = val;
> +
> +	spin_lock_irqsave(&master->devs_lock, flags);

I think you'd better to hold i3c_bus_maintenance_lock() lock to make
sure not transfer on going.

Frank
> +	/*
> +	 * Update DAT entries for all currently attached devices.
> +	 * We directly iterate through the master's device array.
> +	 */
> +	for (i = 0; i < master->maxdevs; i++) {
> +		/* Skip free/empty slots */
> +		if (master->free_pos & BIT(i))
> +			continue;
> +
> +		reg = readl(master->regs +
> +				DEV_ADDR_TABLE_LOC(master->datstartaddr, i));
> +		reg &= ~GENMASK(30, 29);
> +		reg |= DEV_ADDR_TABLE_DEV_NACK_RETRY_CNT(val);
> +		writel(reg, master->regs +
> +			DEV_ADDR_TABLE_LOC(master->datstartaddr, i));
> +	}
> +	spin_unlock_irqrestore(&master->devs_lock, flags);
> +
> +	return count;
> +}
> +
...
> --
> 2.49.GIT
>

Re: [PATCH v3 3/3] i3c: dw: Preserve DAT entry bits when restoring addresses

[Frank Li]

On Sat, Nov 22, 2025 at 02:00:41AM +0800, adrianhoyin.ng@altera.com wrote:
> From: Adrian Ng Ho Yin <adrianhoyin.ng@altera.com>
>
> Update dw_i3c_master_restore_addrs() to preserve existing bits in each
> Device Address Table (DAT) entry when restoring addresses. This prevents
> overwriting configuration bits during PM runtime resumes.
>
> Signed-off-by: Adrian Ng Ho Yin <adrianhoyin.ng@altera.com>
> ---
>  drivers/i3c/master/dw-i3c-master.c | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/i3c/master/dw-i3c-master.c b/drivers/i3c/master/dw-i3c-master.c
> index e228c60840af..37275f7b6c61 100644
> --- a/drivers/i3c/master/dw-i3c-master.c
> +++ b/drivers/i3c/master/dw-i3c-master.c
> @@ -1723,11 +1723,16 @@ static void dw_i3c_master_restore_addrs(struct dw_i3c_master *master)
>  		if (master->free_pos & BIT(pos))
>  			continue;
>
> -		if (master->devs[pos].is_i2c_addr)
> -			reg_val = DEV_ADDR_TABLE_LEGACY_I2C_DEV |
> +		reg_val = readl(master->regs + DEV_ADDR_TABLE_LOC(master->datstartaddr, pos));
> +
> +		if (master->devs[pos].is_i2c_addr) {
> +			reg_val &= ~(GENMASK(6, 0));

#define at MASK for it

#define DEV_ADDR_TABLE_DYNAMIC_MASK	GENMASK(23, 16);
#define DEV_ADDR_TABLE_DYNAMIC_ADDR(x)  FIELD_PREP(DEV_ADDR_TABLE_DYNAMIC_MASK, x) //(((x) << 16) & GENMASK(23, 16))

Create a patch to just change it. And use DEV_ADDR_TABLE_DYNAMIC_MASK at
this patch.

Frank

> +			reg_val |= DEV_ADDR_TABLE_LEGACY_I2C_DEV |
>  			       DEV_ADDR_TABLE_STATIC_ADDR(master->devs[pos].addr);
> -		else
> -			reg_val = DEV_ADDR_TABLE_DYNAMIC_ADDR(master->devs[pos].addr);
> +		} else {
> +			reg_val &= ~(GENMASK(23, 16));
> +			reg_val |= DEV_ADDR_TABLE_DYNAMIC_ADDR(master->devs[pos].addr);
> +		}
>
>  		writel(reg_val, master->regs + DEV_ADDR_TABLE_LOC(master->datstartaddr, pos));
>  	}
> --
> 2.49.GIT
>

Re: [PATCH v3 2/3] i3c: dw: Add sysfs support for Device NACK Retry count

[Ng, Adrian Ho Yin]

On 11/25/2025 1:59 AM, Frank Li wrote:

>> +
>> +	master->dev_nack_retry_cnt = val;
>> +
>> +	spin_lock_irqsave(&master->devs_lock, flags);
> 
> I think you'd better to hold i3c_bus_maintenance_lock() lock to make
> sure not transfer on going.
> 
> Frank
Hi Frank

i3c_bus_maintenance_lock is a static function in master.c. So should i 
update it and expose a public helper to take the maintenance lock or 
continue using the existing spin lock to protect the DAT updates?

Thank You
Adrian
>> +	/*
>> +	 * Update DAT entries for all currently attached devices.
>> +	 * We directly iterate through the master's device array.
>> +	 */
>> +	for (i = 0; i < master->maxdevs; i++) {
>> +		/* Skip free/empty slots */

Re: [PATCH v3 2/3] i3c: dw: Add sysfs support for Device NACK Retry count

[Adrian Hunter]

On 25/11/2025 05:19, Ng, Adrian Ho Yin wrote:
> On 11/25/2025 1:59 AM, Frank Li wrote:
> 
>>> +
>>> +    master->dev_nack_retry_cnt = val;
>>> +
>>> +    spin_lock_irqsave(&master->devs_lock, flags);
>>
>> I think you'd better to hold i3c_bus_maintenance_lock() lock to make
>> sure not transfer on going.
>>
>> Frank
> Hi Frank
> 
> i3c_bus_maintenance_lock is a static function in master.c. So should i update it and expose a public helper to take the maintenance lock or continue using the existing spin lock to protect the DAT updates?
> 
> Thank You
> Adrian

Other controllers (e.g. MIPI I3C) support dev_nack_retry_cnt.

Seems to me this should be in master.c anyway, so it can be shared.

Re: [PATCH v3 2/3] i3c: dw: Add sysfs support for Device NACK Retry count

[Frank Li]

On Tue, Nov 25, 2025 at 04:05:53PM +0200, Adrian Hunter wrote:
> On 25/11/2025 05:19, Ng, Adrian Ho Yin wrote:
> > On 11/25/2025 1:59 AM, Frank Li wrote:
> >
> >>> +
> >>> +    master->dev_nack_retry_cnt = val;
> >>> +
> >>> +    spin_lock_irqsave(&master->devs_lock, flags);
> >>
> >> I think you'd better to hold i3c_bus_maintenance_lock() lock to make
> >> sure not transfer on going.
> >>
> >> Frank
> > Hi Frank
> >
> > i3c_bus_maintenance_lock is a static function in master.c. So should i update it and expose a public helper to take the maintenance lock or continue using the existing spin lock to protect the DAT updates?
> >
> > Thank You
> > Adrian
>
> Other controllers (e.g. MIPI I3C) support dev_nack_retry_cnt.
>
> Seems to me this should be in master.c anyway, so it can be shared.
>

Agree.

Frank