Re: [PATCH] i3c: master: dw: Report CCC M0/M2 errors and retry on failure

On 03/06/2026 10:25, [email protected] wrote:
> From: Adrian Ng Ho Yin <[email protected]>
> 
> Improve CCC reliability on the DesignWare master by reporting
> standard I3C error codes to the core and retrying the command once
> when the failure looks transient.
> 
> Map hardware status to ccc->err as follows:
> - I3C_ERROR_M2: IBA NACK or address NACK

That sounds like it duplicates dw_i3c_master_set_dev_nack_retry().

> - I3C_ERROR_M0: frame error, or an incomplete GET/SET payload when
>   the controller reports a successful transfer (short read vs
>   requested length; GETMRL uses GET_MRL_MIN_LEN if the buffer is
>   larger than struct i3c_ccc_mrl)

Need to consider what would be better handled by I3C core.
Anything related to the protocol, that every controller driver
needs, should be handled there.

> 
> On a successful GET CCC, set dests[0].payload.len to the number of
> bytes actually received.

Is that a bug fix?  Maybe it needs a separate patch?

> 
> Reset ccc->err before each attempt. Retry the CCC get or set once
> if the transfer fails with I3C_ERROR_M0 or I3C_ERROR_M2.

Again, please consider what would be better handled in
drivers/i3c/master.c

> 
> Signed-off-by: Adrian Ng Ho Yin <[email protected]>
> Signed-off-by: Tze Yee Ng <[email protected]>
> ---
>  drivers/i3c/master/dw-i3c-master.c | 79 ++++++++++++++++++++++++++----
>  1 file changed, 69 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/i3c/master/dw-i3c-master.c b/drivers/i3c/master/dw-i3c-master.c
> index 655693a2187e..85ea0af57717 100644
> --- a/drivers/i3c/master/dw-i3c-master.c
> +++ b/drivers/i3c/master/dw-i3c-master.c
> @@ -243,6 +243,12 @@
>  #define AMD_I3C_OD_PP_TIMING		BIT(1)
>  #define DW_I3C_DISABLE_RUNTIME_PM_QUIRK	BIT(2)
>  
> +/* Minimum GETMRL payload (read_len only; no IBI byte). */
> +#define GET_MRL_MIN_LEN 2
> +
> +/* Maximum number of retries for CCC commands */
> +#define DW_I3C_CCC_MAX_RETRIES 2
> +
>  struct dw_i3c_cmd {
>  	u32 cmd_lo;
>  	u32 cmd_hi;
> @@ -708,12 +714,47 @@ static void dw_i3c_master_bus_cleanup(struct i3c_master_controller *m)
>  	dw_i3c_master_disable(master);
>  }
>  
> +static bool dw_i3c_ccc_get_len_mismatch(u8 ccc_id, u16 req_len, u16 rx_len)
> +{
> +	if (ccc_id != I3C_CCC_GETMRL)
> +		return rx_len < req_len;
> +
> +	/*
> +	 * GETMRL returns 2 or 3 bytes; core sets req_len accordingly.
> +	 * If the buffer is larger, only enforce the minimum valid size.
> +	 */
> +	if (req_len <= sizeof(struct i3c_ccc_mrl))
> +		return rx_len < req_len;
> +
> +	return rx_len < GET_MRL_MIN_LEN;
> +}
> +
> +static void dw_i3c_ccc_map_err(struct i3c_ccc_cmd *ccc,
> +			       struct dw_i3c_cmd *cmd,
> +			       bool data_len_mismatch, int *ret)
> +{
> +	u8 err = cmd->error;
> +
> +	if (err == RESPONSE_ERROR_IBA_NACK ||
> +	    err == RESPONSE_ERROR_ADDRESS_NACK) {
> +		ccc->err = I3C_ERROR_M2;
> +	} else if (err == RESPONSE_ERROR_FRAME) {
> +		ccc->err = I3C_ERROR_M0;
> +	} else if (data_len_mismatch && err == RESPONSE_NO_ERROR && !*ret) {
> +		ccc->err = I3C_ERROR_M0;
> +		*ret = -EIO;
> +	}
> +}
> +
>  static int dw_i3c_ccc_set(struct dw_i3c_master *master,
>  			  struct i3c_ccc_cmd *ccc)
>  {
>  	struct dw_i3c_cmd *cmd;
> +	bool data_len_mismatch;
>  	int ret, pos = 0;
>  
> +	ccc->err = I3C_ERROR_UNKNOWN;
> +
>  	if (ccc->id & I3C_CCC_DIRECT) {
>  		pos = dw_i3c_master_get_addr_pos(master, ccc->dests[0].addr);
>  		if (pos < 0)
> @@ -742,8 +783,14 @@ static int dw_i3c_ccc_set(struct dw_i3c_master *master,
>  		dw_i3c_master_dequeue_xfer(master, xfer);
>  
>  	ret = xfer->ret;
> -	if (xfer->cmds[0].error == RESPONSE_ERROR_IBA_NACK)
> -		ccc->err = I3C_ERROR_M2;
> +	cmd = &xfer->cmds[0];
> +	/*
> +	 * RESPONSE_PORT_DATA_LEN reports bytes transferred; on SET CCCs
> +	 * this reflects the write count (stored in cmd->rx_len).
> +	 */
> +	data_len_mismatch = ccc->dests[0].payload.len &&
> +			    cmd->rx_len < ccc->dests[0].payload.len;
> +	dw_i3c_ccc_map_err(ccc, cmd, data_len_mismatch, &ret);
>  
>  	return ret;
>  }
> @@ -751,21 +798,27 @@ static int dw_i3c_ccc_set(struct dw_i3c_master *master,
>  static int dw_i3c_ccc_get(struct dw_i3c_master *master, struct i3c_ccc_cmd *ccc)
>  {
>  	struct dw_i3c_cmd *cmd;
> +	u16 req_len;
> +	bool rx_len_mismatch;
>  	int ret, pos;
>  
> +	ccc->err = I3C_ERROR_UNKNOWN;
> +
>  	pos = dw_i3c_master_get_addr_pos(master, ccc->dests[0].addr);
>  	if (pos < 0)
>  		return pos;
>  
> +	req_len = ccc->dests[0].payload.len;
> +
>  	struct dw_i3c_xfer *xfer __free(kfree) = dw_i3c_master_alloc_xfer(master, 1);
>  	if (!xfer)
>  		return -ENOMEM;
>  
>  	cmd = xfer->cmds;
>  	cmd->rx_buf = ccc->dests[0].payload.data;
> -	cmd->rx_len = ccc->dests[0].payload.len;
> +	cmd->rx_len = req_len;
>  
> -	cmd->cmd_hi = COMMAND_PORT_ARG_DATA_LEN(ccc->dests[0].payload.len) |
> +	cmd->cmd_hi = COMMAND_PORT_ARG_DATA_LEN(req_len) |
>  		      COMMAND_PORT_TRANSFER_ARG;
>  
>  	cmd->cmd_lo = COMMAND_PORT_READ_TRANSFER |
> @@ -780,8 +833,12 @@ static int dw_i3c_ccc_get(struct dw_i3c_master *master, struct i3c_ccc_cmd *ccc)
>  		dw_i3c_master_dequeue_xfer(master, xfer);
>  
>  	ret = xfer->ret;
> -	if (xfer->cmds[0].error == RESPONSE_ERROR_IBA_NACK)
> -		ccc->err = I3C_ERROR_M2;
> +	cmd = &xfer->cmds[0];
> +	rx_len_mismatch = dw_i3c_ccc_get_len_mismatch(ccc->id, req_len,
> +						      cmd->rx_len);
> +	dw_i3c_ccc_map_err(ccc, cmd, rx_len_mismatch, &ret);
> +	if (!ret)
> +		ccc->dests[0].payload.len = cmd->rx_len;
>  
>  	return ret;
>  }
> @@ -796,6 +853,7 @@ static int dw_i3c_master_send_ccc_cmd(struct i3c_master_controller *m,
>  				      struct i3c_ccc_cmd *ccc)
>  {
>  	struct dw_i3c_master *master = to_dw_i3c_master(m);
> +	int retries = DW_I3C_CCC_MAX_RETRIES;
>  	int ret = 0;
>  
>  	if (ccc->id == I3C_CCC_ENTDAA)
> @@ -816,10 +874,11 @@ static int dw_i3c_master_send_ccc_cmd(struct i3c_master_controller *m,
>  		return ret;
>  	}
>  
> -	if (ccc->rnw)
> -		ret = dw_i3c_ccc_get(master, ccc);
> -	else
> -		ret = dw_i3c_ccc_set(master, ccc);
> +	do {
> +		ret = ccc->rnw ? dw_i3c_ccc_get(master, ccc)
> +					   : dw_i3c_ccc_set(master, ccc);
> +	} while (--retries &&
> +			 ret && (ccc->err == I3C_ERROR_M0 || ccc->err == I3C_ERROR_M2));
>  
>  	pm_runtime_put_autosuspend(master->dev);
>  	return ret;


-- 
linux-i3c mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-i3c