linux-kernel - Re: [PATCH 3/4] i3c: dw: Add support for Device NACK Retry count

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
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <aQpTxBGY8vbAQBmI@lizhi-Precision-Tower-5810>
Date: Tue, 4 Nov 2025 14:28:04 -0500
From: Frank Li <Frank.li@....com>
To: adrianhoyin.ng@...era.com
Cc: alexandre.belloni@...tlin.com, wsa+renesas@...g-engineering.com,
	robh@...nel.org, krzk+dt@...nel.org, conor+dt@...nel.org,
	dinguyen@...nel.org, linux-i3c@...ts.infradead.org,
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 3/4] i3c: dw: Add support for Device NACK Retry count

On Tue, Nov 04, 2025 at 04:51:10PM +0800, adrianhoyin.ng@...era.com wrote:
> From: Adrian Ng Ho Yin <adrianhoyin.ng@...era.com>
>
> Some I3C slave devices may temporarily NACK transactions while they are
> busy or not ready to respond. To improve bus reliability, the DesignWare
> I3C controller provides a mechanism to automatically retry a transaction
> when a device issues a NACK.
>
> Add support for configuring the Device NACK Retry count in the Synopsys
> DesignWare I3C master driver. The retry count is obtained from the
> 'snps,dev-nack-retry-cnt' device tree property and written into the
> Device Address Table (DAT) entry for each I3C device.

Most like it should come from sys/debugfs interface. Or just fixed value
should be enough.

Frank
>
> If the property is not defined, the driver defaults to zero retries.
> This behavior is consistent across both Device Tree and ACPI-based
> systems, where the value is only applied if the corresponding property
> is explicitly provided.
>
> The value is clamped to a maximum of 3 (hardware-defined limit), and a
> warning is issued if a higher value is specified.
>
> Signed-off-by: Adrian Ng Ho Yin <adrianhoyin.ng@...era.com>
> ---
>  drivers/i3c/master/dw-i3c-master.c | 34 ++++++++++++++++++++++++++++++
>  drivers/i3c/master/dw-i3c-master.h |  1 +
>  2 files changed, 35 insertions(+)
>
> diff --git a/drivers/i3c/master/dw-i3c-master.c b/drivers/i3c/master/dw-i3c-master.c
> index 9ceedf09c3b6..12ee4c4afedf 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))
> @@ -989,6 +991,7 @@ static int dw_i3c_master_reattach_i3c_dev(struct i3c_dev_desc *dev,
>  	struct i3c_master_controller *m = i3c_dev_get_master(dev);
>  	struct dw_i3c_master *master = to_dw_i3c_master(m);
>  	int pos;
> +	u32 reg;
>
>  	pos = dw_i3c_master_get_free_pos(master);
>
> @@ -1009,6 +1012,15 @@ static int dw_i3c_master_reattach_i3c_dev(struct i3c_dev_desc *dev,
>  	       master->regs +
>  	       DEV_ADDR_TABLE_LOC(master->datstartaddr, data->index));
>
> +	if (master->dev_nack_retry_cnt) {
> +		reg = readl(master->regs +
> +			DEV_ADDR_TABLE_LOC(master->datstartaddr, data->index));
> +		reg |= DEV_ADDR_TABLE_DEV_NACK_RETRY_CNT(master->dev_nack_retry_cnt) |
> +				DEV_ADDR_TABLE_SIR_REJECT;
> +		writel(reg, master->regs +
> +			DEV_ADDR_TABLE_LOC(master->datstartaddr, data->index));
> +	}
> +
>  	master->devs[data->index].addr = dev->info.dyn_addr;
>
>  	return 0;
> @@ -1020,6 +1032,7 @@ static int dw_i3c_master_attach_i3c_dev(struct i3c_dev_desc *dev)
>  	struct dw_i3c_master *master = to_dw_i3c_master(m);
>  	struct dw_i3c_i2c_dev_data *data;
>  	int pos;
> +	u32 reg;
>
>  	pos = dw_i3c_master_get_free_pos(master);
>  	if (pos < 0)
> @@ -1038,6 +1051,15 @@ static int dw_i3c_master_attach_i3c_dev(struct i3c_dev_desc *dev)
>  	       master->regs +
>  	       DEV_ADDR_TABLE_LOC(master->datstartaddr, data->index));
>
> +	if (master->dev_nack_retry_cnt) {
> +		reg = readl(master->regs +
> +			DEV_ADDR_TABLE_LOC(master->datstartaddr, data->index));
> +		reg |= DEV_ADDR_TABLE_DEV_NACK_RETRY_CNT(master->dev_nack_retry_cnt) |
> +				DEV_ADDR_TABLE_SIR_REJECT;
> +		writel(reg, master->regs +
> +			DEV_ADDR_TABLE_LOC(master->datstartaddr, data->index));
> +	}
> +
>  	return 0;
>  }
>
> @@ -1592,6 +1614,18 @@ int dw_i3c_common_probe(struct dw_i3c_master *master,
>
>  	master->quirks = (unsigned long)device_get_match_data(&pdev->dev);
>
> +	ret = device_property_read_u32(&pdev->dev, "snps,dev-nack-retry-cnt",
> +				       &master->dev_nack_retry_cnt);
> +	if (ret) {
> +		master->dev_nack_retry_cnt = 0;
> +	} else if (master->dev_nack_retry_cnt > DW_I3C_DEV_NACK_RETRY_CNT_MAX) {
> +		dev_warn(&pdev->dev,
> +			 "dev_nack_retry_cnt (%u) exceeds max (%u), clamping to %u\n",
> +			 master->dev_nack_retry_cnt, DW_I3C_DEV_NACK_RETRY_CNT_MAX,
> +			 DW_I3C_DEV_NACK_RETRY_CNT_MAX);
> +		master->dev_nack_retry_cnt = DW_I3C_DEV_NACK_RETRY_CNT_MAX;
> +	}
> +
>  	INIT_WORK(&master->hj_work, dw_i3c_hj_work);
>  	ret = i3c_master_register(&master->base, &pdev->dev,
>  				  &dw_mipi_i3c_ops, false);
> 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
>