[PATCH] i3c: ast2600: Generate T-bits to terminate IBI storm
Joshua Yeong
joshua.yeong at starfivetech.com
Sun Jun 15 19:31:59 PDT 2025
On 11-Jun-2025 12:02 PM, Billy Tsai wrote:
> Under certain conditions, such as when an IBI interrupt is received and
> SDA remains high after the address phase, the I3C controller will enter
> an infinite loop attempting to read data until a T-bit is detected.
> This commit addresses the issue by generating a fake T-bit to terminate
> the IBI storm when the received IBI data length exceeds the maximum
> allowable IBI payload.
> This issue cannot be resolved using the abort function, as it is
> ineffective when the I3C FSM is in the Servicing IBI Transfer (0xE) or
> Clock Extension (0x12) states.
>
> Signed-off-by: Billy Tsai <billy_tsai at aspeedtech.com>
> ---
> drivers/i3c/master/ast2600-i3c-master.c | 60 +++++++++++++++++++++++++
> drivers/i3c/master/dw-i3c-master.c | 14 ++++++
> drivers/i3c/master/dw-i3c-master.h | 9 ++++
> 3 files changed, 83 insertions(+)
>
> diff --git a/drivers/i3c/master/ast2600-i3c-master.c b/drivers/i3c/master/ast2600-i3c-master.c
> index e05e83812c71..6ac0122474d0 100644
> --- a/drivers/i3c/master/ast2600-i3c-master.c
> +++ b/drivers/i3c/master/ast2600-i3c-master.c
> @@ -33,11 +33,28 @@
> #define AST2600_I3CG_REG1_SA_EN BIT(15)
> #define AST2600_I3CG_REG1_INST_ID_MASK GENMASK(19, 16)
> #define AST2600_I3CG_REG1_INST_ID(x) (((x) << 16) & AST2600_I3CG_REG1_INST_ID_MASK)
> +#define AST2600_I3CG_REG1_SCL_SW_MODE_OE BIT(20)
> +#define AST2600_I3CG_REG1_SCL_OUT_SW_MODE_VAL BIT(21)
> +#define AST2600_I3CG_REG1_SCL_IN_SW_MODE_VAL BIT(23)
> +#define AST2600_I3CG_REG1_SDA_SW_MODE_OE BIT(24)
> +#define AST2600_I3CG_REG1_SDA_OUT_SW_MODE_VAL BIT(25)
> +#define AST2600_I3CG_REG1_SDA_IN_SW_MODE_VAL BIT(27)
> +#define AST2600_I3CG_REG1_SCL_IN_SW_MODE_EN BIT(28)
> +#define AST2600_I3CG_REG1_SDA_IN_SW_MODE_EN BIT(29)
> +#define AST2600_I3CG_REG1_SCL_OUT_SW_MODE_EN BIT(30)
> +#define AST2600_I3CG_REG1_SDA_OUT_SW_MODE_EN BIT(31)
>
> #define AST2600_DEFAULT_SDA_PULLUP_OHMS 2000
>
> #define DEV_ADDR_TABLE_IBI_PEC BIT(11)
>
> +#define IBI_QUEUE_STATUS 0x18
> +#define PRESENT_STATE 0x54
> +#define CM_TFR_STS GENMASK(13, 8)
> +#define CM_TFR_STS_MASTER_SERV_IBI 0xe
> +#define SDA_LINE_SIGNAL_LEVEL BIT(1)
> +#define SCL_LINE_SIGNAL_LEVEL BIT(0)
> +
> struct ast2600_i3c {
> struct dw_i3c_master dw;
> struct regmap *global_regs;
> @@ -117,9 +134,52 @@ static void ast2600_i3c_set_dat_ibi(struct dw_i3c_master *i3c,
> }
> }
>
> +static bool ast2600_i3c_fsm_exit_serv_ibi(struct dw_i3c_master *dw)
> +{
> + u32 state;
> +
> + /*
> + * Clear the IBI queue to enable the hardware to generate SCL and
> + * begin detecting the T-bit low to stop reading IBI data.
> + */
> + readl(dw->regs + IBI_QUEUE_STATUS);
> + state = FIELD_GET(CM_TFR_STS, readl(dw->regs + PRESENT_STATE));
> + if (state == CM_TFR_STS_MASTER_SERV_IBI)
> + return false;
> +
> + return true;
> +}
> +
> +static void ast2600_i3c_gen_tbits_in(struct dw_i3c_master *dw)
> +{
> + struct ast2600_i3c *i3c = to_ast2600_i3c(dw);
> + bool is_idle;
> + int ret;
> +
> + regmap_write_bits(i3c->global_regs, AST2600_I3CG_REG1(i3c->global_idx),
> + AST2600_I3CG_REG1_SDA_IN_SW_MODE_VAL,
> + AST2600_I3CG_REG1_SDA_IN_SW_MODE_VAL);
> + regmap_write_bits(i3c->global_regs, AST2600_I3CG_REG1(i3c->global_idx),
> + AST2600_I3CG_REG1_SDA_IN_SW_MODE_EN,
> + AST2600_I3CG_REG1_SDA_IN_SW_MODE_EN);
> +
> + regmap_write_bits(i3c->global_regs, AST2600_I3CG_REG1(i3c->global_idx),
> + AST2600_I3CG_REG1_SDA_IN_SW_MODE_VAL, 0);
> + ret = readx_poll_timeout_atomic(ast2600_i3c_fsm_exit_serv_ibi, dw,
> + is_idle, is_idle, 0, 2000000);
> + regmap_write_bits(i3c->global_regs, AST2600_I3CG_REG1(i3c->global_idx),
> + AST2600_I3CG_REG1_SDA_IN_SW_MODE_EN, 0);
> + if (ret)
> + dev_err(&dw->base.dev,
> + "Failed to exit the I3C fsm from %lx(MASTER_SERV_IBI): %d",
> + FIELD_GET(CM_TFR_STS, readl(dw->regs + PRESENT_STATE)),
> + ret);
> +}
> +
> static const struct dw_i3c_platform_ops ast2600_i3c_ops = {
> .init = ast2600_i3c_init,
> .set_dat_ibi = ast2600_i3c_set_dat_ibi,
> + .gen_tbits_in = ast2600_i3c_gen_tbits_in,
> };
>
> static int ast2600_i3c_probe(struct platform_device *pdev)
> diff --git a/drivers/i3c/master/dw-i3c-master.c b/drivers/i3c/master/dw-i3c-master.c
> index 611c22b72c15..380e6a29c7b8 100644
> --- a/drivers/i3c/master/dw-i3c-master.c
> +++ b/drivers/i3c/master/dw-i3c-master.c
> @@ -158,6 +158,14 @@
> #define DATA_BUFFER_STATUS_LEVEL_TX(x) ((x) & GENMASK(7, 0))
>
> #define PRESENT_STATE 0x54
> +#define CM_TFR_ST_STS GENMASK(21, 16)
> +#define CM_TFR_ST_STS_HALT 0x13
> +#define CM_TFR_STS GENMASK(13, 8)
> +#define CM_TFR_STS_MASTER_SERV_IBI 0xe
> +#define CM_TFR_STS_MASTER_HALT 0xf
> +#define CM_TFR_STS_SLAVE_HALT 0x6
> +#define SDA_LINE_SIGNAL_LEVEL BIT(1)
> +#define SCL_LINE_SIGNAL_LEVEL BIT(0)
> #define CCC_DEVICE_STATUS 0x58
> #define DEVICE_ADDR_TABLE_POINTER 0x5c
> #define DEVICE_ADDR_TABLE_DEPTH(x) (((x) & GENMASK(31, 16)) >> 16)
> @@ -1393,6 +1401,8 @@ static void dw_i3c_master_handle_ibi_sir(struct dw_i3c_master *master,
> unsigned long flags;
> u8 addr, len;
> int idx;
> + bool terminate_ibi = false;
> + u32 state;
>
> addr = IBI_QUEUE_IBI_ADDR(status);
> len = IBI_QUEUE_STATUS_DATA_LEN(status);
> @@ -1435,6 +1445,7 @@ static void dw_i3c_master_handle_ibi_sir(struct dw_i3c_master *master,
> dev_dbg_ratelimited(&master->base.dev,
> "IBI payload len %d greater than max %d\n",
> len, dev->ibi->max_payload_len);
> + terminate_ibi = true;
> goto err_drain;
> }
>
> @@ -1450,6 +1461,9 @@ static void dw_i3c_master_handle_ibi_sir(struct dw_i3c_master *master,
>
> err_drain:
> dw_i3c_master_drain_ibi_queue(master, len);
> + state = FIELD_GET(CM_TFR_STS, readl(master->regs + PRESENT_STATE));
> + if (terminate_ibi && state == CM_TFR_STS_MASTER_SERV_IBI)
> + master->platform_ops->gen_tbits_in(master);
>
> spin_unlock_irqrestore(&master->devs_lock, flags);
> }
Would it makes more sense to move `->gen_tbits_in(master);` logic to
`if (dev->ibi->max_payload_len < len) ` above instead?
The IBI queue would be properly flush at the end of function and don't need
additional `terminate_ibi ` variable. If there were IBI storm, there might be
IBI coming in right after `dw_i3c_master_drain_ibi_queue` here which will
trigger another round of interrupt anyway.
Thanks
Joshua
> diff --git a/drivers/i3c/master/dw-i3c-master.h b/drivers/i3c/master/dw-i3c-master.h
> index c5cb695c16ab..1da485e42e74 100644
> --- a/drivers/i3c/master/dw-i3c-master.h
> +++ b/drivers/i3c/master/dw-i3c-master.h
> @@ -89,6 +89,15 @@ struct dw_i3c_platform_ops {
> */
> void (*set_dat_ibi)(struct dw_i3c_master *i3c,
> struct i3c_dev_desc *dev, bool enable, u32 *reg);
> + /*
> + * Gerenating the fake t-bit (SDA low) to stop the IBI storm when the received
> + * data length of IBI is larger than the maximum IBI payload.
> + *
> + * When an IBI is received and SDA remains high after the address phase, the i3c
> + * controller may enter an infinite loop while trying to read data until the t-bit
> + * appears
> + */
> + void (*gen_tbits_in)(struct dw_i3c_master *i3c);
> };
>
> extern int dw_i3c_common_probe(struct dw_i3c_master *master,
More information about the linux-i3c
mailing list