Re: [PATCH v6 5/5] i3c: master: Add optional_bytes for variable-length GET CCC validation

On Wed, Jul 08, 2026 at 12:17:41AM -0700, [email protected] wrote:
> From: Adrian Ng Ho Yin <[email protected]>
>
> Add optional_bytes to struct i3c_ccc_cmd_payload so callers describe
> variable-length GET CCC responses. GETMRL and GETMXDS set optional_bytes
> at the call site. Extend i3c_ccc_validate_payload_len() to honour it.
>
> Signed-off-by: Adrian Ng Ho Yin <[email protected]>
> Signed-off-by: Tze Yee Ng <[email protected]>
> ---
> Changes in v5:
> - Split from v4 patch 3/3: optional_bytes and variable-length GET rules only.
> - Simplify validation: drop redundant exact-length check when
>   optional_bytes == 0 (per review).
> - Clarify GETMXDS fallback: with optional_bytes = 3, a 2-byte success on
>   the first attempt no longer needs the fallback; it remains for when the
>   5-byte request fails rather than short-reading.
> ---
>  drivers/i3c/master.c    | 29 +++++++++++++++++++++++------
>  include/linux/i3c/ccc.h |  2 ++
>  2 files changed, 25 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> index 3b6d3ecd9d12..12ca7c929ee5 100644
> --- a/drivers/i3c/master.c
> +++ b/drivers/i3c/master.c
> @@ -902,6 +902,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;
> +	dest->payload.optional_bytes = 0;
>  	if (payloadlen)
>  		dest->payload.data = kzalloc(payloadlen, GFP_KERNEL);
>  	else
> @@ -944,11 +945,19 @@ static int i3c_ccc_validate_payload_len(struct i3c_ccc_cmd *cmd)
>
>  	for (i = 0; i < cmd->ndests; i++) {
>  		struct i3c_ccc_cmd_payload *p = &cmd->dests[i].payload;
> +		u16 min_len;
> +
> +		if (p->optional_bytes > p->len)
> +			return -EINVAL;
>
>  		if (p->actual_len > p->len)
>  			return -EIO;
>
> -		if (p->len && p->actual_len != p->len)
> +		if (!p->len)
> +			continue;
> +
> +		min_len = p->len - p->optional_bytes;
> +		if (p->actual_len < min_len)
>  			return -EIO;
>  	}
>
> @@ -1342,10 +1351,14 @@ static int i3c_master_getmrl_locked(struct i3c_master_controller *master,
>  		return -ENOMEM;
>
>  	/*
> -	 * When the device does not have IBI payload GETMRL only returns 2
> -	 * bytes of data.
> +	 * GETMRL returns 2 bytes (max read length) when the device does not
> +	 * advertise IBI payload, or 2 or 3 bytes when it does (the optional
> +	 * third byte is max IBI length). Use optional_bytes to allow either
> +	 * length when IBI payload is supported.
>  	 */
> -	if (!(info->bcr & I3C_BCR_IBI_PAYLOAD))
> +	if (info->bcr & I3C_BCR_IBI_PAYLOAD)
> +		dest.payload.optional_bytes = 1;
> +	else
>  		dest.payload.len -= 1;
>
>  	i3c_ccc_cmd_init(&cmd, true, I3C_CCC_GETMRL, &dest, 1);
> @@ -1414,14 +1427,18 @@ static int i3c_master_getmxds_locked(struct i3c_master_controller *master,
>  	if (!getmaxds)
>  		return -ENOMEM;
>
> +	dest.payload.optional_bytes = 3;
> +
>  	i3c_ccc_cmd_init(&cmd, true, I3C_CCC_GETMXDS, &dest, 1);
>  	ret = i3c_master_send_ccc_cmd_locked(master, &cmd);
>  	if (ret) {
>  		/*
> -		 * Retry when the device does not support max read turnaround
> -		 * while expecting shorter length from this CCC command.
> +		 * optional_bytes = 3 accepts a 2-byte response on the first
> +		 * attempt, so this fallback runs only when the 5-byte request
> +		 * fails rather than returning a short read.
>  		 */
>  		dest.payload.len -= 3;
> +		dest.payload.optional_bytes = 0;
>  		i3c_ccc_cmd_init(&cmd, true, I3C_CCC_GETMXDS, &dest, 1);

where use retry != 1 case ? if all is 1, patch 4 is not necessary.

Frank

>  		ret = i3c_master_send_ccc_cmd_locked(master, &cmd);
>  		if (ret)
> diff --git a/include/linux/i3c/ccc.h b/include/linux/i3c/ccc.h
> index 2506d83b8255..7ad677baf761 100644
> --- a/include/linux/i3c/ccc.h
> +++ b/include/linux/i3c/ccc.h
> @@ -347,11 +347,13 @@ struct i3c_ccc_getxtime {
>   *
>   * @len: requested payload length
>   * @actual_len: number of bytes received on a GET CCC (filled by the driver)
> + * @optional_bytes: GET CCCs may return up to this many fewer bytes than @len
>   * @data: payload data. This buffer must be DMA-able
>   */
>  struct i3c_ccc_cmd_payload {
>  	u16 len;
>  	u16 actual_len;
> +	u16 optional_bytes;
>  	void *data;
>  };
>
> --
> 2.43.7
>
>

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