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

From: sashiko-bot@kernel.org
To: tze.yee.ng@altera.com
Cc: Frank.Li@kernel.org, linux-i3c@lists.infradead.org
Subject: Re: [PATCH v2 3/3] i3c: master: Validate GET CCC payload length and retry M0/M2 once
Date: Tue, 09 Jun 2026 10:30:29 +0000	[thread overview]
Message-ID: <20260609103029.994891F00893@smtp.kernel.org> (raw)
In-Reply-To: <352ed29363b12d1d145ffd44316564bd8e33444c.1780999511.git.tze.yee.ng@altera.com>

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@altera.com>

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?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1780999511.git.tze.yee.ng@altera.com?part=3

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c