Re: [PATCH v5 3/5] i3c: use actual_len for read transfers

[Adrian Hunter <adrian.hunter@intel.com>]

On 22/09/2026 02:06, Sam Agazaryan wrote:
> Currently only the amd and svc I3C controller drivers populate
> i3c_xfer.actual_len on reads, while dw-i3c-master and mipi-i3c-hci
> overwrite i3c_xfer.len with the received byte count and leave actual_len
> as 0. Because of this inconsistency, mctp-i3c reads xfer.len instead of
> xfer.actual_len, making it fail on controllers that do not mutate len.
> 
> Unify read length reporting across the subsystem:
> - Clarify in <linux/i3c/device.h> that actual_len reflects the actual
>   number of bytes transferred on reads.
> - Update adi, cdns, dw, mipi-i3c-hci, and renesas controller drivers to
>   populate actual_len on read transfers without mutating len.
> - Update mctp-i3c to read xfer.actual_len instead of xfer.len.
> 
> Co-developed-by: Meagan Lloyd <meaganlloyd@linux.microsoft.com>
> Signed-off-by: Meagan Lloyd <meaganlloyd@linux.microsoft.com>
> Signed-off-by: Sam Agazaryan <samagazaryan@google.com>

Looks good.

1 issue from Sashiko:

	https://sashiko.dev/#/patchset/20260921230603.2518652-1-samagazaryan%40google.com?part=3

Also i3c_dev_do_xfers_locked() should probably clamp actual_len to len.

> ---
>  drivers/i3c/master/adi-i3c-master.c    |  5 ++++-
>  drivers/i3c/master/dw-i3c-master.c     |  2 +-
>  drivers/i3c/master/i3c-master-cdns.c   |  5 ++++-
>  drivers/i3c/master/mipi-i3c-hci/core.c |  2 +-
>  drivers/i3c/master/renesas-i3c.c       |  3 +++
>  drivers/net/mctp/mctp-i3c.c            | 10 +++++-----
>  include/linux/i3c/device.h             |  2 +-
>  7 files changed, 19 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/i3c/master/adi-i3c-master.c b/drivers/i3c/master/adi-i3c-master.c
> index b35386260350..d033bfd4ecc2 100644
> --- a/drivers/i3c/master/adi-i3c-master.c
> +++ b/drivers/i3c/master/adi-i3c-master.c
> @@ -412,8 +412,11 @@ static int adi_i3c_master_i3c_xfers(struct i3c_dev_desc *dev,
>  
>  	ret = xfer->ret;
>  
> -	for (i = 0; i < nxfers; i++)
> +	for (i = 0; i < nxfers; i++) {
>  		xfers[i].err = adi_i3c_cmd_get_err(&xfer->cmds[i]);
> +		if (xfers[i].rnw)
> +			xfers[i].actual_len = xfer->cmds[i].rx_len;
> +	}
>  
>  	return ret;
>  }
> diff --git a/drivers/i3c/master/dw-i3c-master.c b/drivers/i3c/master/dw-i3c-master.c
> index 4563d8761ba0..28e8eb293747 100644
> --- a/drivers/i3c/master/dw-i3c-master.c
> +++ b/drivers/i3c/master/dw-i3c-master.c
> @@ -1029,7 +1029,7 @@ static int dw_i3c_master_i3c_xfers(struct i3c_dev_desc *dev,
>  		struct dw_i3c_cmd *cmd = &xfer->cmds[i];
>  
>  		if (i3c_xfers[i].rnw)
> -			i3c_xfers[i].len = cmd->rx_len;
> +			i3c_xfers[i].actual_len = cmd->rx_len;
>  	}
>  
>  	ret = xfer->ret;
> diff --git a/drivers/i3c/master/i3c-master-cdns.c b/drivers/i3c/master/i3c-master-cdns.c
> index 2d98c1ce9b12..331062c10b31 100644
> --- a/drivers/i3c/master/i3c-master-cdns.c
> +++ b/drivers/i3c/master/i3c-master-cdns.c
> @@ -795,8 +795,11 @@ static int cdns_i3c_master_i3c_xfers(struct i3c_dev_desc *dev,
>  
>  	ret = cdns_xfer->ret;
>  
> -	for (i = 0; i < nxfers; i++)
> +	for (i = 0; i < nxfers; i++) {
>  		xfers[i].err = cdns_i3c_cmd_get_err(&cdns_xfer->cmds[i]);
> +		if (xfers[i].rnw)
> +			xfers[i].actual_len = cdns_xfer->cmds[i].rx_len;
> +	}
>  
>  	cdns_i3c_master_free_xfer(cdns_xfer);
>  
> diff --git a/drivers/i3c/master/mipi-i3c-hci/core.c b/drivers/i3c/master/mipi-i3c-hci/core.c
> index dadf049bd4b5..f74c02fab9e3 100644
> --- a/drivers/i3c/master/mipi-i3c-hci/core.c
> +++ b/drivers/i3c/master/mipi-i3c-hci/core.c
> @@ -509,7 +509,7 @@ static int i3c_hci_i3c_xfers(struct i3c_dev_desc *dev,
>  		goto out;
>  	for (i = 0; i < nxfers; i++) {
>  		if (i3c_xfers[i].rnw)
> -			i3c_xfers[i].len = RESP_DATA_LENGTH(xfer[i].response);
> +			i3c_xfers[i].actual_len = RESP_DATA_LENGTH(xfer[i].response);
>  		if (RESP_STATUS(xfer[i].response) != RESP_SUCCESS) {
>  			ret = -EIO;
>  			goto out;
> diff --git a/drivers/i3c/master/renesas-i3c.c b/drivers/i3c/master/renesas-i3c.c
> index 28c0927a0179..a3dcb4cd9b0e 100644
> --- a/drivers/i3c/master/renesas-i3c.c
> +++ b/drivers/i3c/master/renesas-i3c.c
> @@ -918,6 +918,9 @@ static int renesas_i3c_i3c_xfers(struct i3c_dev_desc *dev, struct i3c_xfer *i3c_
>  		time_left = renesas_i3c_wait_xfer(i3c, xfer);
>  		if (!time_left)
>  			xfer_failed = true;
> +
> +		if (i3c_xfers[i].rnw)
> +			i3c_xfers[i].actual_len = cmd->rx_count;
>  	}
>  
>  	if (xfer_failed)
> diff --git a/drivers/net/mctp/mctp-i3c.c b/drivers/net/mctp/mctp-i3c.c
> index 6d2bbae7477b..5f94c11a1712 100644
> --- a/drivers/net/mctp/mctp-i3c.c
> +++ b/drivers/net/mctp/mctp-i3c.c
> @@ -131,12 +131,12 @@ static int mctp_i3c_read(struct mctp_i3c_device *mi)
>  	if (rc < 0)
>  		goto err;
>  
> -	if (WARN_ON_ONCE(xfer.len > mi->mrl)) {
> +	if (WARN_ON_ONCE(xfer.actual_len > mi->mrl)) {
>  		/* Bad i3c bus driver */
>  		rc = -EIO;
>  		goto err;
>  	}
> -	if (xfer.len < MCTP_I3C_MINLEN) {
> +	if (xfer.actual_len < MCTP_I3C_MINLEN) {
>  		stats->rx_length_errors++;
>  		rc = -EIO;
>  		goto err;
> @@ -145,15 +145,15 @@ static int mctp_i3c_read(struct mctp_i3c_device *mi)
>  	/* check PEC, including address byte */
>  	addr = mi->addr << 1 | 1;
>  	pec = i2c_smbus_pec(0, &addr, 1);
> -	pec = i2c_smbus_pec(pec, xfer.data.in, xfer.len - 1);
> -	if (pec != ((u8 *)xfer.data.in)[xfer.len - 1]) {
> +	pec = i2c_smbus_pec(pec, xfer.data.in, xfer.actual_len - 1);
> +	if (pec != ((u8 *)xfer.data.in)[xfer.actual_len - 1]) {
>  		stats->rx_crc_errors++;
>  		rc = -EINVAL;
>  		goto err;
>  	}
>  
>  	/* Remove PEC */
> -	skb_trim(skb, xfer.len - 1);
> +	skb_trim(skb, xfer.actual_len - 1);
>  
>  	cb = __mctp_cb(skb);
>  	cb->halen = PID_SIZE;
> diff --git a/include/linux/i3c/device.h b/include/linux/i3c/device.h
> index 971d53349b6f..f868a7bf2bc9 100644
> --- a/include/linux/i3c/device.h
> +++ b/include/linux/i3c/device.h
> @@ -59,7 +59,7 @@ enum i3c_xfer_mode {
>   * @rnw: encodes the transfer direction. true for a read, false for a write
>   * @cmd: Read/Write command in HDR mode, read: 0x80 - 0xff, write: 0x00 - 0x7f
>   * @len: transfer length in bytes of the transfer
> - * @actual_len: actual length in bytes are transferred by the controller
> + * @actual_len: actual length in bytes transferred by the controller on read
>   * @data: input/output buffer
>   * @data.in: input buffer. Must point to a DMA-able buffer
>   * @data.out: output buffer. Must point to a DMA-able buffer