1. Patchew
  2. linux
  3. [PATCH v3 0/3] i3c: dw-i3c: Enable support for dw-i3c controller NACK retry sysfs and DAT restore fix
  4. View patch

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

adrianhoyin.ng@altera.com posted 3 patches 1 week, 3 days ago
There is a newer version of this series
[PATCH v3 2/3] i3c: dw: Add sysfs support for Device NACK Retry count
Posted by adrianhoyin.ng@altera.com 1 week, 3 days ago 
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
Re: [PATCH v3 2/3] i3c: dw: Add sysfs support for Device NACK Retry count
Posted by Frank Li 1 week ago 
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 2/3] i3c: dw: Add sysfs support for Device NACK Retry count
Posted by Ng, Adrian Ho Yin 6 days, 21 hours ago 
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
Posted by Adrian Hunter 6 days, 11 hours ago 
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
Posted by Frank Li 6 days, 7 hours ago 
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

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.