Re: [PATCH v4 03/16] i3c: renesas: Follow the reset deassert order used in probe

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] In `renesas_i3c_i2c_xfers`, unhandled timeouts cause the loop to proceed, risking data corruption and silent failures.
- [High] In `renesas_i3c_i3c_xfers`, transfer statuses and errors are completely ignored, unconditionally returning 0.
- [High] Missing hardware abort sequence on transfer timeout risks controller lockup.
--

commit 85523970e99a91763d831b25e5f1d70eaf5d402f
Author: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

i3c: renesas: Follow the reset deassert order used in probe

Use the same reset deassert order in the resume and probe paths to avoid
potential failures due to ordering differences.

> diff --git a/drivers/i3c/master/renesas-i3c.c b/drivers/i3c/master/renesas-i3c.c
> index e70db3a17bc72..7d448936b74c1 100644
> --- a/drivers/i3c/master/renesas-i3c.c
> +++ b/drivers/i3c/master/renesas-i3c.c
> @@ -1457,17 +1457,17 @@ static int renesas_i3c_resume_noirq(struct device *dev)
>  	struct renesas_i3c *i3c = dev_get_drvdata(dev);
>  	int i, ret;
>  
> -	ret = reset_control_deassert(i3c->presetn);
> +	ret = reset_control_deassert(i3c->tresetn);
>  	if (ret)
>  		return ret;

[ ... ]

While reviewing this file, I noticed a few pre-existing issues that might need
your attention:

[Severity: High]
This is a pre-existing issue, but does this code properly handle timeouts
during I2C transfers?

drivers/i3c/master/renesas-i3c.c:renesas_i3c_i2c_xfers() {
...
		renesas_set_bit(i3c->regs, NTSTE, NTSTE_TDBEE0);

		wait_for_completion_timeout(&xfer->comp, m->i2c.timeout);

		if (cmd->err)
			break;

		start_bit = CNDCTL_SRCND;
...
}

If wait_for_completion_timeout() times out, its return value is discarded and
cmd->err remains 0. The loop condition if (cmd->err) break; does not trigger,
and the loop proceeds to the next iteration.

Can this overwrite shared state and issue a new START condition while the
hardware might still be processing the previous request? This could lead to
silent failures and controller desynchronization since the function eventually
returns success despite the timeout.

[Severity: High]
This is a pre-existing issue, but does renesas_i3c_i3c_xfers() ignore hardware
errors and timeouts?

drivers/i3c/master/renesas-i3c.c:renesas_i3c_i3c_xfers() {
...
		if (!i3c_xfers[i].rnw && i3c_xfers[i].len > 4) {
			i3c_writel_fifo(i3c->regs + NTDTBP0, cmd->tx_buf, cmd->len);
			if (cmd->len > NTDTBP0_DEPTH * sizeof(u32))
				renesas_set_bit(i3c->regs, NTIE, NTIE_TDBEIE0);
		}

		renesas_i3c_wait_xfer(i3c, xfer);
	}

	return 0;
}

The loop iterates over i3c_nxfers and waits for each transfer using
renesas_i3c_wait_xfer(), but completely ignores xfer->ret which is populated
by the ISR with actual hardware error statuses like NACK or parity errors.

The function then unconditionally returns 0. Could masking all hardware errors
and timeouts from the upper layers lead to subsequent silent failures?

[Severity: High]
This is a pre-existing issue, but is there a missing hardware abort sequence
when a transfer times out?

drivers/i3c/master/renesas-i3c.c:renesas_i3c_wait_xfer() {
...
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);
}
...
}

When a transfer times out, the software state is dequeued via
renesas_i3c_dequeue_xfer(). However, it does not appear to set the BCTL_ABT
bit or reset the controller, leaving the hardware in an indeterminate busy
state.

When the next transfer is enqueued, renesas_i3c_start_xfer_locked()
indiscriminately writes new commands to the hardware command queue (NCMDQP).
Could this cause a bus collision or controller lockup?

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

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