Re: [PATCH v2 3/3] i3c: master: Validate GET CCC payload length and retry M0/M2 once

[NG, TZE YEE]

On 9/6/2026 6:30 pm, sashiko-bot@xxxxxxxxxx wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Address NACKs are incorrectly mapped to -EINVAL instead of a hardware error code (-EIO or -ENXIO), breaking standard subsystem error reporting conventions for standard transfers.
> --
> 
> commit f0f1354839be36f55ac51ec9d8d9c182653a6497
> Author: Adrian Ng Ho Yin <adrian.ho.yin.ng@xxxxxxxxxx>
> 
> i3c: master: Validate GET CCC payload length and retry M0/M2 once
> 
> Validate GET CCC payload length after a successful transfer. Treat a
> short read as I3C_ERROR_M0 and return -EIO. Retry GET CCCs once on
> retriable errors.
> 
> [ ... ]
>> diff --git a/drivers/i3c/master/dw-i3c-master.c b/drivers/i3c/master/dw-i3c-master.c
>> index c4004f87c6857..a8b5ebabd3508 100644
>> --- a/drivers/i3c/master/dw-i3c-master.c
>> +++ b/drivers/i3c/master/dw-i3c-master.c
>> @@ -493,7 +493,6 @@ static void dw_i3c_master_end_xfer_locked(struct dw_i3c_master *master, u32 isr)
>>   			break;
>>   		case RESPONSE_ERROR_PARITY:
>>   		case RESPONSE_ERROR_IBA_NACK:
>> -		case RESPONSE_ERROR_ADDRESS_NACK:
>>   		case RESPONSE_ERROR_TRANSF_ABORT:
>>   		case RESPONSE_ERROR_CRC:
>>   		case RESPONSE_ERROR_FRAME:
>> @@ -503,6 +502,7 @@ static void dw_i3c_master_end_xfer_locked(struct dw_i3c_master *master, u32 isr)
>>   			ret = -ENOSPC;
>>   			break;
>>   		case RESPONSE_ERROR_I2C_W_NACK_ERR:
>> +		case RESPONSE_ERROR_ADDRESS_NACK:
>>   		default:
>>   			ret = -EINVAL;
>>   			break;
> 
> [Severity: High]
> Does moving RESPONSE_ERROR_ADDRESS_NACK to the default case incorrectly map
> address NACKs to -EINVAL instead of a hardware error code like -EIO or
> -ENXIO?
> 
> This function serves as the generic completion handler for all transfers,
> including standard I3C and I2C private reads and writes. An address NACK
> usually indicates an absent, disconnected, or busy device.
> 
> Returning -EINVAL for a hardware NACK during a data transfer diverges from
> subsystem conventions. Can this cause userspace tools like i2cdetect or
> core probing logic to misinterpret an absent device as a fatal invalid
> parameter error, rather than allowing graceful fallbacks?
> 
I will submit v3 to move RESPONSE_ERROR_ADDRESS_NACK to return -EIO 
(alongside IBA_NACK) in patch 2/3 and drop the accidental revert in 
patch 3/3.

Regards,
Tze Yee