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

[NG, TZE YEE]

On 8/7/2026 11:01 pm, Frank Li wrote:
> On Wed, Jul 08, 2026 at 12:17:41AM -0700, tze.yee.ng@xxxxxxxxxx wrote:
>> From: Adrian Ng Ho Yin <adrian.ho.yin.ng@xxxxxxxxxx>
>>
>> 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 <adrian.ho.yin.ng@xxxxxxxxxx>
>> Signed-off-by: Tze Yee Ng <tze.yee.ng@xxxxxxxxxx>
>> ---
>> 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
> 

Hi Frank,

The retry count is not always 1. retries is 1 for GET and 0 for SET (rnw 
? I3C_CCC_RETRIES : 0). max_attempts = retries + 1 gives 2 attempts for 
Direct GET (spec §5.1.9.2.3) and 1 for side-effecting SET CCCs. This 
follows Adrian Hunter’s v4 suggestion in 
https://lore.kernel.org/all/382a7c2d-2e9e-4a83-94d1-b917921d91f1@xxxxxxxxx/; 
patch 4 is still required for validation and the central retry path, not 
only for the field itself.

Patch 4 is still needed for the central retry path, GET payload 
validation after a successful transfer, and resetting actual_len between 
attempts — not only for exposing retries in struct i3c_ccc_cmd.

Thanks,
Tze Yee

>>   		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
>>
>>