1. Patchew
  2. linux
  3. [PATCH v3 0/2] i3c: dw-i3c-master: scoped spinlock guards and SIR reject fix
  4. View patch

[PATCH v3 2/2] i3c: dw-i3c-master: fix SIR reject bit mapping for dynamic addresses

adrianhoyin.ng@altera.com posted 2 patches 1 week, 6 days ago
There is a newer version of this series
[PATCH v3 2/2] i3c: dw-i3c-master: fix SIR reject bit mapping for dynamic addresses
Posted by adrianhoyin.ng@altera.com 1 week, 6 days ago 
From: Adrian Ng Ho Yin <adrianhoyin.ng@altera.com>

The IBI_SIR_REQ_REJECT register is a 32-bit bitmap indexed by the
dynamic address of each I3C slave. The DesignWare controller derives
the bit index by folding the 7-bit dynamic address into a 5-bit value,
using the sum of the lower 5 bits and the upper 2 bits, modulo 32.

The current implementation incorrectly uses the device table index
when updating the SIR reject mask, which can result in rejecting or
accepting IBIs for the wrong device.

Compute the SIR reject bit index directly from the dynamic address,
as defined by the controller specification, and use it consistently
when updating the reject mask.

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

diff --git a/drivers/i3c/master/dw-i3c-master.c b/drivers/i3c/master/dw-i3c-master.c
index ef8a38620e0e..77e0edd1c660 100644
--- a/drivers/i3c/master/dw-i3c-master.c
+++ b/drivers/i3c/master/dw-i3c-master.c
@@ -205,6 +205,11 @@
 #define EXTENDED_CAPABILITY		0xe8
 #define SLAVE_CONFIG			0xec
 
+#define DYN_ADDR_LO_BITS		5
+#define DYN_ADDR_LO_MASK GENMASK(4, 0)
+#define DYN_ADDR_HI_MASK GENMASK(6, 5)
+#define IBI_SIR_BIT_MOD 32       /* 32-bit vector */
+
 #define DW_I3C_DEV_NACK_RETRY_CNT_MAX	0x3
 #define DEV_ADDR_TABLE_DEV_NACK_RETRY_MASK   GENMASK(30, 29)
 #define DEV_ADDR_TABLE_DYNAMIC_MASK		GENMASK(23, 16)
@@ -217,6 +222,7 @@
 #define DEV_ADDR_TABLE_DYNAMIC_ADDR(x) FIELD_PREP(DEV_ADDR_TABLE_DYNAMIC_MASK, x)
 #define DEV_ADDR_TABLE_STATIC_ADDR(x) FIELD_PREP(DEV_ADDR_TABLE_STATIC_MASK, x)
 #define DEV_ADDR_TABLE_LOC(start, idx)	((start) + ((idx) << 2))
+#define DEV_ADDR_TABLE_GET_DYNAMIC_ADDR(x) FIELD_GET(DEV_ADDR_TABLE_DYNAMIC_MASK, x)
 
 #define I3C_BUS_SDR1_SCL_RATE		8000000
 #define I3C_BUS_SDR2_SCL_RATE		6000000
@@ -264,6 +270,14 @@ struct dw_i3c_drvdata {
 	u32 flags;
 };
 
+static inline u32 get_ibi_sir_bit_index(u8 addr)
+{
+	u32 lo = FIELD_GET(DYN_ADDR_LO_MASK, addr);
+	u32 hi = FIELD_GET(DYN_ADDR_HI_MASK, addr);
+
+	return (lo + hi) % IBI_SIR_BIT_MOD;
+}
+
 static bool dw_i3c_master_supports_ccc_cmd(struct i3c_master_controller *m,
 					   const struct i3c_ccc_cmd *cmd)
 {
@@ -1236,11 +1250,19 @@ static void dw_i3c_master_set_sir_enabled(struct dw_i3c_master *master,
 {
 	u32 dat_entry, reg;
 	bool global;
+	u8 dynamic_addr;
 
 	dat_entry = DEV_ADDR_TABLE_LOC(master->datstartaddr, idx);
 
 	guard(spinlock_irqsave)(&master->devs_lock);
 	reg = readl(master->regs + dat_entry);
+	dynamic_addr = DEV_ADDR_TABLE_GET_DYNAMIC_ADDR(reg);
+
+	if (!dynamic_addr)
+		dev_warn(master->dev,
+			 "<%s> unassigned slave device, dynamic addr:%x\n",
+			 __func__, dynamic_addr);
+
 	if (enable) {
 		reg &= ~DEV_ADDR_TABLE_SIR_REJECT;
 		if (dev->info.bcr & I3C_BCR_IBI_PAYLOAD)
@@ -1253,11 +1275,11 @@ static void dw_i3c_master_set_sir_enabled(struct dw_i3c_master *master,
 
 	if (enable) {
 		global = (master->sir_rej_mask == IBI_REQ_REJECT_ALL);
-		master->sir_rej_mask &= ~BIT(idx);
+		master->sir_rej_mask &= ~BIT(get_ibi_sir_bit_index(dynamic_addr));
 	} else {
 		bool hj_rejected = !!(readl(master->regs + DEVICE_CTRL) & DEV_CTRL_HOT_JOIN_NACK);
 
-		master->sir_rej_mask |= BIT(idx);
+		master->sir_rej_mask |= BIT(get_ibi_sir_bit_index(dynamic_addr));
 		global = (master->sir_rej_mask == IBI_REQ_REJECT_ALL) && hj_rejected;
 	}
 	writel(master->sir_rej_mask, master->regs + IBI_SIR_REQ_REJECT);
-- 
2.49.GIT
Re: [PATCH v3 2/2] i3c: dw-i3c-master: fix SIR reject bit mapping for dynamic addresses
Posted by Frank Li 1 week, 6 days ago 
On Mon, Jan 26, 2026 at 01:42:23PM +0800, adrianhoyin.ng@altera.com wrote:
> From: Adrian Ng Ho Yin <adrianhoyin.ng@altera.com>
>
> The IBI_SIR_REQ_REJECT register is a 32-bit bitmap indexed by the
> dynamic address of each I3C slave. The DesignWare controller derives
> the bit index by folding the 7-bit dynamic address into a 5-bit value,
> using the sum of the lower 5 bits and the upper 2 bits, modulo 32.
>
> The current implementation incorrectly uses the device table index
> when updating the SIR reject mask, which can result in rejecting or
> accepting IBIs for the wrong device.
>
> Compute the SIR reject bit index directly from the dynamic address,
> as defined by the controller specification, and use it consistently
> when updating the reject mask.
>
> Signed-off-by: Adrian Ng Ho Yin <adrianhoyin.ng@altera.com>
> ---
>  drivers/i3c/master/dw-i3c-master.c | 26 ++++++++++++++++++++++++--
>  1 file changed, 24 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/i3c/master/dw-i3c-master.c b/drivers/i3c/master/dw-i3c-master.c
> index ef8a38620e0e..77e0edd1c660 100644
> --- a/drivers/i3c/master/dw-i3c-master.c
> +++ b/drivers/i3c/master/dw-i3c-master.c
> @@ -205,6 +205,11 @@
>  #define EXTENDED_CAPABILITY		0xe8
>  #define SLAVE_CONFIG			0xec
>
> +#define DYN_ADDR_LO_BITS		5

Not used,

> +#define DYN_ADDR_LO_MASK GENMASK(4, 0)
> +#define DYN_ADDR_HI_MASK GENMASK(6, 5)
> +#define IBI_SIR_BIT_MOD 32       /* 32-bit vector */
> +
>  #define DW_I3C_DEV_NACK_RETRY_CNT_MAX	0x3
>  #define DEV_ADDR_TABLE_DEV_NACK_RETRY_MASK   GENMASK(30, 29)
>  #define DEV_ADDR_TABLE_DYNAMIC_MASK		GENMASK(23, 16)
> @@ -217,6 +222,7 @@
>  #define DEV_ADDR_TABLE_DYNAMIC_ADDR(x) FIELD_PREP(DEV_ADDR_TABLE_DYNAMIC_MASK, x)
>  #define DEV_ADDR_TABLE_STATIC_ADDR(x) FIELD_PREP(DEV_ADDR_TABLE_STATIC_MASK, x)
>  #define DEV_ADDR_TABLE_LOC(start, idx)	((start) + ((idx) << 2))
> +#define DEV_ADDR_TABLE_GET_DYNAMIC_ADDR(x) FIELD_GET(DEV_ADDR_TABLE_DYNAMIC_MASK, x)
>
>  #define I3C_BUS_SDR1_SCL_RATE		8000000
>  #define I3C_BUS_SDR2_SCL_RATE		6000000
> @@ -264,6 +270,14 @@ struct dw_i3c_drvdata {
>  	u32 flags;
>  };
>
> +static inline u32 get_ibi_sir_bit_index(u8 addr)
> +{
> +	u32 lo = FIELD_GET(DYN_ADDR_LO_MASK, addr);
> +	u32 hi = FIELD_GET(DYN_ADDR_HI_MASK, addr);
> +
> +	return (lo + hi) % IBI_SIR_BIT_MOD;
> +}
> +
>  static bool dw_i3c_master_supports_ccc_cmd(struct i3c_master_controller *m,
>  					   const struct i3c_ccc_cmd *cmd)
>  {
> @@ -1236,11 +1250,19 @@ static void dw_i3c_master_set_sir_enabled(struct dw_i3c_master *master,
>  {
>  	u32 dat_entry, reg;
>  	bool global;
> +	u8 dynamic_addr;
>
>  	dat_entry = DEV_ADDR_TABLE_LOC(master->datstartaddr, idx);
>
>  	guard(spinlock_irqsave)(&master->devs_lock);
>  	reg = readl(master->regs + dat_entry);
> +	dynamic_addr = DEV_ADDR_TABLE_GET_DYNAMIC_ADDR(reg);
> +
> +	if (!dynamic_addr)
> +		dev_warn(master->dev,
> +			 "<%s> unassigned slave device, dynamic addr:%x\n",
> +			 __func__, dynamic_addr);
> +
>  	if (enable) {
>  		reg &= ~DEV_ADDR_TABLE_SIR_REJECT;
>  		if (dev->info.bcr & I3C_BCR_IBI_PAYLOAD)
> @@ -1253,11 +1275,11 @@ static void dw_i3c_master_set_sir_enabled(struct dw_i3c_master *master,
>

if use
	idx = get_ibi_sir_bit_index(dynamic_addr);

code will be simpler

Frank
>  	if (enable) {
>  		global = (master->sir_rej_mask == IBI_REQ_REJECT_ALL);
> -		master->sir_rej_mask &= ~BIT(idx);
> +		master->sir_rej_mask &= ~BIT(get_ibi_sir_bit_index(dynamic_addr));
>  	} else {
>  		bool hj_rejected = !!(readl(master->regs + DEVICE_CTRL) & DEV_CTRL_HOT_JOIN_NACK);
>
> -		master->sir_rej_mask |= BIT(idx);
> +		master->sir_rej_mask |= BIT(get_ibi_sir_bit_index(dynamic_addr));
>  		global = (master->sir_rej_mask == IBI_REQ_REJECT_ALL) && hj_rejected;
>  	}
>  	writel(master->sir_rej_mask, master->regs + IBI_SIR_REQ_REJECT);
> --
> 2.49.GIT
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.