| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <aSCb5Y4ypnEuWUYA@lizhi-Precision-Tower-5810>
Date: Fri, 21 Nov 2025 12:05:41 -0500
From: Frank Li <Frank.li@....com>
To: adrianhoyin.ng@...era.com
Cc: alexandre.belloni@...tlin.com, linux-i3c@...ts.infradead.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 2/3] i3c: dw: Add sysfs support for Device NACK Retry
count
On Fri, Nov 21, 2025 at 02:21:49PM +0800, adrianhoyin.ng@...era.com wrote:
> From: Adrian Ng Ho Yin <adrianhoyin.ng@...era.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. Writes are clamped to the hardware
> maximum of 3, and the updated value is programmed into all active DAT
> entries.
>
> Signed-off-by: Adrian Ng Ho Yin <adrianhoyin.ng@...era.com>
> ---
> drivers/i3c/master/dw-i3c-master.c | 64 ++++++++++++++++++++++++++++++
> drivers/i3c/master/dw-i3c-master.h | 1 +
> 2 files changed, 65 insertions(+)
>
> diff --git a/drivers/i3c/master/dw-i3c-master.c b/drivers/i3c/master/dw-i3c-master.c
> index 9ceedf09c3b6..c43eba615d8a 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,58 @@ to_dw_i3c_master(struct i3c_master_controller *master)
> return container_of(master, struct dw_i3c_master, base);
> }
>
> +static ssize_t dev_nack_retry_count_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
keep consistent with other function's naming. Maybe "dw_i3c_master" too
long, add "dw_" prefix should be fine.
> +{
> + struct dw_i3c_master *master = dev_get_drvdata(dev);
> +
> + return sysfs_emit(buf, "%u\n", master->dev_nack_retry_cnt);
> +}
> +
> +static ssize_t 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;
> + int ret, i;
> + u32 reg;
> +
> + ret = kstrtoul(buf, 0, &val);
> + if (ret)
> + return ret;
> +
> + if (val > DW_I3C_DEV_NACK_RETRY_CNT_MAX) {
> + dev_warn(dev, "Value %lu exceeds maximum %d, clamping to max\n",
> + val, DW_I3C_DEV_NACK_RETRY_CNT_MAX);
> + val = DW_I3C_DEV_NACK_RETRY_CNT_MAX;
return err.
> + }
> +
> + master->dev_nack_retry_cnt = val;
> +
> + /*
> + * 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));
do you need lock here? In case data is transferring.
> + }
> +
> + return count;
> +}
> +
> +static DEVICE_ATTR_RW(dev_nack_retry_count);
> +
> static void dw_i3c_master_disable(struct dw_i3c_master *master)
> {
> writel(readl(master->regs + DEVICE_CTRL) & ~DEV_CTRL_ENABLE,
> @@ -1032,6 +1086,7 @@ static int dw_i3c_master_attach_i3c_dev(struct i3c_dev_desc *dev)
> data->index = pos;
> master->devs[pos].addr = dev->info.dyn_addr ? : dev->info.static_addr;
> master->free_pos &= ~BIT(pos);
> + master->dev_nack_retry_cnt = 0;
suppose needn't init to 0 here again, it is already 0 when alloc master.
> writel(DEV_ADDR_TABLE_DYNAMIC_ADDR(master->devs[pos].addr),
> @@ -1598,6 +1653,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 +1678,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..c87a0e87bfd9 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;
just one space between u32 and dev_nack_retry_cnt
Frank
> /*
> * Per-device hardware data, used to manage the device address table
> * (DAT)
> --
> 2.49.GIT
>
Powered by blists - more mailing lists