[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aEm3Xr7Oj1wjASUT@lizhi-Precision-Tower-5810>
Date: Wed, 11 Jun 2025 13:05:34 -0400
From: Frank Li <Frank.li@....com>
To: Billy Tsai <billy_tsai@...eedtech.com>
Cc: jk@...econstruct.com.au, alexandre.belloni@...tlin.com,
aniketmaurya@...gle.com, jarkko.nikula@...ux.intel.com,
Shyam-sundar.S-k@....com, wsa+renesas@...g-engineering.com,
linux-i3c@...ts.infradead.org, linux-kernel@...r.kernel.org,
BMC-SW@...eedtech.com, elbadrym@...gle.com, romlem@...gle.com
Subject: Re: [PATCH] i3c: ast2600: Generate T-bits to terminate IBI storm
On Wed, Jun 11, 2025 at 12:02:03PM +0800, Billy Tsai wrote:
> Under certain conditions, such as when an IBI interrupt is received and
> SDA remains high after the address phase,
Can you descript more clear?
Generally IBI happen at below two case
1. SDA pull down by target
2. Address arbitration happen
Then Host send out ACK, according to your descrpition, look like host
NACK target IBI request.
> the I3C controller will enter
> an infinite loop attempting to read data until a T-bit is detected.
BCR/DCR have indicate IBI mandatory data length. You should know how many
data need be read according to IBI won's target address. Why relay on T-bit,
which just is used for when target have less data than what expected.
> 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.
Add empty line here.
> 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.
why ineffective?
Frank
>
> Signed-off-by: Billy Tsai <billy_tsai@...eedtech.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);
> }
> 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,
> --
> 2.25.1
>
Powered by blists - more mailing lists