Re: [PATCH v3 15/17] i3c: renesas: Use the "dev_name:irq_name" format for the interrupt name

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Hardware transfers are not aborted on timeout, leading to late-interrupt races that prematurely complete subsequent transfers, and the driver unconditionally returns success (0) on timeout.
- [High] In `renesas_i3c_i2c_xfers()`, the return value of `wait_for_completion_timeout()` is ignored, causing the driver to blindly proceed to the next message when a timeout occurs.
--

commit 6bee3adf98ffbd95bda0355a68d579fc811fc62e
Author: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

i3c: renesas: Use the "dev_name:irq_name" format for the interrupt name

Use the "dev_name:irq_name" format for the interrupt names. This makes it
easier to identify interrupts in systems where multiple devices may request
interrupts with the same name.

> diff --git a/drivers/i3c/master/renesas-i3c.c b/drivers/i3c/master/renesas-i3c.c
> index 103c938d14553..6572934a1837e 100644
> --- a/drivers/i3c/master/renesas-i3c.c
> +++ b/drivers/i3c/master/renesas-i3c.c

[ ... ]

> static void renesas_i3c_wait_xfer(struct renesas_i3c *i3c, struct renesas_i3c_xfer *xfer)
> {
> 	unsigned long time_left;
> 
> 	renesas_i3c_enqueue_xfer(i3c, xfer);
> 
> 	time_left = wait_for_completion_timeout(&xfer->comp, msecs_to_jiffies(1000));
> 	if (!time_left)
> 		renesas_i3c_dequeue_xfer(i3c, xfer);
> }

[Severity: High]
This is a pre-existing issue, but hardware transfers do not appear to be
aborted on timeout here.

The timeout dequeues the software object but does not abort the hardware,
setting up a late-interrupt race condition.

When looking at renesas_i3c_i3c_xfers():

drivers/i3c/master/renesas-i3c.c:renesas_i3c_i3c_xfers() {
...
		renesas_i3c_wait_xfer(i3c, xfer);
	}

	return 0;
}

Can this cause the loop to continue without breaking after a timeout?

It looks like the function proceeds to enqueue the next message in the
transfer sequence using the same xfer object and sends it to the hardware,
even though the hardware hasn't been properly aborted or reset for the
previous command.

If the hardware eventually completes the original timed-out message, could
it fire an interrupt where the ISR (renesas_i3c_resp_isr) sees the xfer
object and calls complete(&xfer->comp), causing the
wait_for_completion_timeout() for the second message to return prematurely?

Could this false success also cause the caller to use uninitialized read
buffers or assume writes succeeded? Does this same omission of timeout error
propagation also apply to renesas_i3c_send_ccc_cmd() and renesas_i3c_daa()?

[ ... ]

> static int renesas_i3c_i2c_xfers(struct i2c_dev_desc *dev,
> 				 struct i2c_msg *i2c_xfers,
> 				 int i2c_nxfers)
> {
[ ... ]
> 		wait_for_completion_timeout(&xfer->comp, m->i2c.timeout);
> 
> 		if (cmd->err)
> 			break;
> 
> 		start_bit = CNDCTL_SRCND;
> 	}
> 
> 	return cmd->err;
> }

[Severity: High]
This is a pre-existing issue, but does this loop fail to break on a timeout?

The return value of wait_for_completion_timeout() is ignored here. If a
timeout occurs, cmd->err is never updated and remains 0. The loop condition
if (cmd->err) evaluates to false, so the driver continues to the next I2C
message.

Does this cause the driver to issue a new start condition to the hardware
while it is still hung processing the timed-out message, reusing the
un-reinitialized completion object?

Since it eventually returns cmd->err (0), could this falsely report success
to the I2C subsystem for a transfer sequence that failed or timed out?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260608201543.804902-1-claudiu.beznea@kernel.org?part=15

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