Re: [PATCH v6 2/5] i3c: master: Report actual GET CCC payload length on success

[Frank Li <Frank.li@oss.nxp.com>]

On Wed, Jul 08, 2026 at 12:17:38AM -0700, tze.yee.ng@altera.com wrote:
> From: Adrian Ng Ho Yin <adrian.ho.yin.ng@altera.com>
>
> Set dests[].payload.actual_len on successful GET CCC transfers in I3C
> master drivers so the core can distinguish requested and received
> buffer lengths. Switch core GET helpers to use actual_len instead of
> len when interpreting GET CCC results.
>
> Signed-off-by: Adrian Ng Ho Yin <adrian.ho.yin.ng@altera.com>
> Signed-off-by: Tze Yee Ng <tze.yee.ng@altera.com>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.com>

> Changes in v6:
> - Fix ADI and Cadence GET actual_len: store hardware-reported xfer bytes
>   in cmd->rx_len and set actual_len from that, not from payload.len.
> - Fix Renesas GET actual_len: drain the response FIFO at rx_buf + rx_count
>   in resp_isr, clamp rx_count to data_len before subtracting (avoids
>   unsigned underflow when rx_isr reads round up to 32-bit words), and
>   report actual_len from cmd->rx_count.
>
> Changes in v5:
> - New patch scope: all I3C master drivers (adi, dw, cdns, renesas, svc,
>   mipi-hci) set actual_len on successful GET transfers.
> - dw_i3c_ccc_get() sets actual_len from cmd->rx_len; dw_i3c_ccc_set()
>   does not touch actual_len (fixes v4 regression).
> - Initialise actual_len to 0 in i3c_ccc_cmd_dest_init().
> - Switch core GET helpers (getmrl, getmwl, getmxds, gethdrcap) to use
>   actual_len instead of len when interpreting GET results.
>
> Changes in v4:
> - Refactor dw_i3c_ccc_map_err() to return enum i3c_error_code; callers
>   assign ccc->err.
> - Map I3C_ERROR_M2 only for RESPONSE_ERROR_IBA_NACK (broadcast address
>   7'h7E not ACKed). Do not report target-address NACK as M2.
> - Keep RESPONSE_ERROR_ADDRESS_NACK returning -EIO in
>   dw_i3c_master_end_xfer_locked().
> ---
>  drivers/i3c/master.c                   | 11 ++++++-----
>  drivers/i3c/master/adi-i3c-master.c    |  3 +++
>  drivers/i3c/master/dw-i3c-master.c     |  5 ++++-
>  drivers/i3c/master/i3c-master-cdns.c   |  3 +++
>  drivers/i3c/master/mipi-i3c-hci/core.c |  5 +++--
>  drivers/i3c/master/renesas-i3c.c       | 16 ++++++++++++----
>  drivers/i3c/master/svc-i3c-master.c    |  4 ++--
>  7 files changed, 33 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> index 5cd4e5da2233..037b3b980717 100644
> --- a/drivers/i3c/master.c
> +++ b/drivers/i3c/master.c
> @@ -901,6 +901,7 @@ static void *i3c_ccc_cmd_dest_init(struct i3c_ccc_cmd_dest *dest, u8 addr,
>  {
>  	dest->addr = addr;
>  	dest->payload.len = payloadlen;
> +	dest->payload.actual_len = 0;
>  	if (payloadlen)
>  		dest->payload.data = kzalloc(payloadlen, GFP_KERNEL);
>  	else
> @@ -1302,7 +1303,7 @@ static int i3c_master_getmrl_locked(struct i3c_master_controller *master,
>  	if (ret)
>  		goto out;
>
> -	switch (dest.payload.len) {
> +	switch (dest.payload.actual_len) {
>  	case 3:
>  		info->max_ibi_len = mrl->ibi_len;
>  		fallthrough;
> @@ -1337,7 +1338,7 @@ static int i3c_master_getmwl_locked(struct i3c_master_controller *master,
>  	if (ret)
>  		goto out;
>
> -	if (dest.payload.len != sizeof(*mwl)) {
> +	if (dest.payload.actual_len != sizeof(*mwl)) {
>  		ret = -EIO;
>  		goto out;
>  	}
> @@ -1376,14 +1377,14 @@ static int i3c_master_getmxds_locked(struct i3c_master_controller *master,
>  			goto out;
>  	}
>
> -	if (dest.payload.len != 2 && dest.payload.len != 5) {
> +	if (dest.payload.actual_len != 2 && dest.payload.actual_len != 5) {
>  		ret = -EIO;
>  		goto out;
>  	}
>
>  	info->max_read_ds = getmaxds->maxrd;
>  	info->max_write_ds = getmaxds->maxwr;
> -	if (dest.payload.len == 5)
> +	if (dest.payload.actual_len == 5)
>  		info->max_read_turnaround = getmaxds->maxrdturn[0] |
>  					    ((u32)getmaxds->maxrdturn[1] << 8) |
>  					    ((u32)getmaxds->maxrdturn[2] << 16);
> @@ -1412,7 +1413,7 @@ static int i3c_master_gethdrcap_locked(struct i3c_master_controller *master,
>  	if (ret)
>  		goto out;
>
> -	if (dest.payload.len != 1) {
> +	if (dest.payload.actual_len != 1) {
>  		ret = -EIO;
>  		goto out;
>  	}
> diff --git a/drivers/i3c/master/adi-i3c-master.c b/drivers/i3c/master/adi-i3c-master.c
> index 047081c9f064..b5ab2e4e5d57 100644
> --- a/drivers/i3c/master/adi-i3c-master.c
> +++ b/drivers/i3c/master/adi-i3c-master.c
> @@ -246,6 +246,7 @@ static void adi_i3c_master_end_xfer_locked(struct adi_i3c_master *master,
>  		if (cmd->cmd0 & REG_CMD_FIFO_0_RNW) {
>  			rx_len = min_t(u32, REG_CMDR_FIFO_XFER_BYTES(cmdr), cmd->rx_len);
>  			adi_i3c_master_rd_from_rx_fifo(master, cmd->rx_buf, rx_len);
> +			cmd->rx_len = rx_len;
>  		}
>  		cmd->error = REG_CMDR_FIFO_ERROR(cmdr);
>  	}
> @@ -360,6 +361,8 @@ static int adi_i3c_master_send_ccc_cmd(struct i3c_master_controller *m,
>  		adi_i3c_master_unqueue_xfer(master, xfer);
>
>  	cmd->err = adi_i3c_cmd_get_err(&xfer->cmds[0]);
> +	if (!xfer->ret && cmd->rnw)
> +		cmd->dests[0].payload.actual_len = ccmd->rx_len;
>
>  	return xfer->ret;
>  }
> diff --git a/drivers/i3c/master/dw-i3c-master.c b/drivers/i3c/master/dw-i3c-master.c
> index 655693a2187e..6998ffd15f5f 100644
> --- a/drivers/i3c/master/dw-i3c-master.c
> +++ b/drivers/i3c/master/dw-i3c-master.c
> @@ -780,7 +780,10 @@ 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)
> +	cmd = &xfer->cmds[0];
> +	if (!ret)
> +		ccc->dests[0].payload.actual_len = cmd->rx_len;
> +	if (cmd->error == RESPONSE_ERROR_IBA_NACK)
>  		ccc->err = I3C_ERROR_M2;
>
>  	return ret;
> diff --git a/drivers/i3c/master/i3c-master-cdns.c b/drivers/i3c/master/i3c-master-cdns.c
> index 5cfec6761494..cd6adbcecc26 100644
> --- a/drivers/i3c/master/i3c-master-cdns.c
> +++ b/drivers/i3c/master/i3c-master-cdns.c
> @@ -574,6 +574,7 @@ static void cdns_i3c_master_end_xfer_locked(struct cdns_i3c_master *master,
>  		cmd = &xfer->cmds[CMDR_CMDID(cmdr)];
>  		rx_len = min_t(u32, CMDR_XFER_BYTES(cmdr), cmd->rx_len);
>  		cdns_i3c_master_rd_from_rx_fifo(master, cmd->rx_buf, rx_len);
> +		cmd->rx_len = rx_len;
>  		cmd->error = CMDR_ERROR(cmdr);
>  	}
>
> @@ -715,6 +716,8 @@ static int cdns_i3c_master_send_ccc_cmd(struct i3c_master_controller *m,
>
>  	ret = xfer->ret;
>  	cmd->err = cdns_i3c_cmd_get_err(&xfer->cmds[0]);
> +	if (!ret && cmd->rnw)
> +		cmd->dests[0].payload.actual_len = ccmd->rx_len;
>  	cdns_i3c_master_free_xfer(xfer);
>
>  	return ret;
> diff --git a/drivers/i3c/master/mipi-i3c-hci/core.c b/drivers/i3c/master/mipi-i3c-hci/core.c
> index b781dbed2165..2b215658e093 100644
> --- a/drivers/i3c/master/mipi-i3c-hci/core.c
> +++ b/drivers/i3c/master/mipi-i3c-hci/core.c
> @@ -326,7 +326,7 @@ static int i3c_hci_send_ccc_cmd(struct i3c_master_controller *m,
>  		goto out;
>  	for (i = prefixed; i < nxfers; i++) {
>  		if (ccc->rnw)
> -			ccc->dests[i - prefixed].payload.len =
> +			ccc->dests[i - prefixed].payload.actual_len =
>  				RESP_DATA_LENGTH(xfer[i].response);
>  		switch (RESP_STATUS(xfer[i].response)) {
>  		case RESP_SUCCESS:
> @@ -343,7 +343,8 @@ static int i3c_hci_send_ccc_cmd(struct i3c_master_controller *m,
>
>  	if (ccc->rnw)
>  		dev_dbg(&hci->master.dev, "got: %*ph",
> -			ccc->dests[0].payload.len, ccc->dests[0].payload.data);
> +			ccc->dests[0].payload.actual_len,
> +			ccc->dests[0].payload.data);
>
>  out:
>  	hci_free_xfer(xfer, nxfers);
> diff --git a/drivers/i3c/master/renesas-i3c.c b/drivers/i3c/master/renesas-i3c.c
> index f39c449922ca..25a2b2ed618e 100644
> --- a/drivers/i3c/master/renesas-i3c.c
> +++ b/drivers/i3c/master/renesas-i3c.c
> @@ -805,6 +805,8 @@ static int renesas_i3c_send_ccc_cmd(struct i3c_master_controller *m,
>  	ret = xfer->ret;
>  	if (ret)
>  		ccc->err = I3C_ERROR_M2;
> +	else if (ccc->rnw)
> +		ccc->dests[0].payload.actual_len = cmd->rx_count;
>
>  	return ret;
>  }
> @@ -1072,10 +1074,16 @@ static irqreturn_t renesas_i3c_resp_isr(int irq, void *data)
>  			break;
>  		case I3C_INTERNAL_STATE_CONTROLLER_READ:
>  		case I3C_INTERNAL_STATE_CONTROLLER_COMMAND_READ:
> -			if (NDBSTLV0_RDBLV(renesas_readl(i3c->regs, NDBSTLV0)) && !cmd->err)
> -				bytes_remaining = data_len - cmd->rx_count;
> -
> -			i3c_readl_fifo(i3c->regs + NTDTBP0, cmd->rx_buf, bytes_remaining);
> +			if (!cmd->err) {
> +				u32 rx_count = min(cmd->rx_count, data_len);
> +
> +				bytes_remaining = data_len - rx_count;
> +				if (bytes_remaining)
> +					i3c_readl_fifo(i3c->regs + NTDTBP0,
> +						       cmd->rx_buf + rx_count,
> +						       bytes_remaining);
> +				cmd->rx_count = data_len;
> +			}
>  			renesas_clear_bit(i3c->regs, NTIE, NTIE_RDBFIE0);
>  			break;
>  		default:
> diff --git a/drivers/i3c/master/svc-i3c-master.c b/drivers/i3c/master/svc-i3c-master.c
> index e2d99a3ac07d..c92d96e6b4d8 100644
> --- a/drivers/i3c/master/svc-i3c-master.c
> +++ b/drivers/i3c/master/svc-i3c-master.c
> @@ -1706,8 +1706,8 @@ static int svc_i3c_master_send_direct_ccc_cmd(struct svc_i3c_master *master,
>  		svc_i3c_master_dequeue_xfer(master, xfer);
>  	mutex_unlock(&master->lock);
>
> -	if (cmd->actual_len != xfer_len)
> -		ccc->dests[0].payload.len = cmd->actual_len;
> +	if (ccc->rnw)
> +		ccc->dests[0].payload.actual_len = cmd->actual_len;
>
>  	ret = xfer->ret;
>  	svc_i3c_master_free_xfer(xfer);
> --
> 2.43.7
>
>