* [PATCH] i3c: dw: reduce do_daa time if there's no client
@ 2026-08-03 14:03 Jisheng Zhang
2026-08-03 14:36 ` sashiko-bot
2026-08-05 20:05 ` Frank Li
0 siblings, 2 replies; 3+ messages in thread
From: Jisheng Zhang @ 2026-08-03 14:03 UTC (permalink / raw)
To: Alexandre Belloni, Frank Li; +Cc: linux-i3c, linux-kernel
If there's no client, wait for completion will timeout, then there's no
need to add i3c dev at all. This can save boot time by ~6s if there's
no client connected to the i3c interface.
Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
---
drivers/i3c/master/dw-i3c-master.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/i3c/master/dw-i3c-master.c b/drivers/i3c/master/dw-i3c-master.c
index 0625587f1527..18ebf304029a 100644
--- a/drivers/i3c/master/dw-i3c-master.c
+++ b/drivers/i3c/master/dw-i3c-master.c
@@ -885,8 +885,10 @@ static int dw_i3c_master_daa(struct i3c_master_controller *m)
COMMAND_PORT_ROC;
dw_i3c_master_enqueue_xfer(master, xfer);
- if (!wait_for_completion_timeout(&xfer->comp, XFER_TIMEOUT))
+ if (!wait_for_completion_timeout(&xfer->comp, XFER_TIMEOUT)) {
dw_i3c_master_dequeue_xfer(master, xfer);
+ goto rpm_out;
+ }
newdevs = GENMASK(master->maxdevs - cmd->rx_len - 1, 0);
newdevs &= ~olddevs;
--
2.53.0
--
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] i3c: dw: reduce do_daa time if there's no client
2026-08-03 14:03 [PATCH] i3c: dw: reduce do_daa time if there's no client Jisheng Zhang
@ 2026-08-03 14:36 ` sashiko-bot
2026-08-05 20:05 ` Frank Li
1 sibling, 0 replies; 3+ messages in thread
From: sashiko-bot @ 2026-08-03 14:36 UTC (permalink / raw)
To: Jisheng Zhang; +Cc: Frank.Li, Alexandre Belloni, linux-i3c
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
Pre-existing issues:
- [High] A late hardware response from a timed-out transfer can erroneously complete a newly enqueued transfer due to insufficient hardware abortion.
- [High] Ignoring `xfer->ret` in `dw_i3c_master_daa` leads to Undefined Behavior in `GENMASK` and phantom device registration if the hardware completes with an error.
- [High] Queue stall in `dw_i3c_master_dequeue_xfer_locked` due to failure to start the next transfer after a timeout.
--
--- Patch [1]: [PATCH] i3c: dw: reduce do_daa time if there's no client ---
commit 68e06554d2c19f165bbc91e6eef6e5857d1b346a
Author: Jisheng Zhang <jszhang@kernel.org>
i3c: dw: reduce do_daa time if there's no client
If there's no client, wait for completion will timeout, then there's no
need to add i3c dev at all. This can save boot time by ~6s if there's
no client connected to the i3c interface.
> diff --git a/drivers/i3c/master/dw-i3c-master.c b/drivers/i3c/master/dw-i3c-master.c
> index 2f8c0c4683e06..ae4ac37cc08c3 100644
> --- a/drivers/i3c/master/dw-i3c-master.c
> +++ b/drivers/i3c/master/dw-i3c-master.c
> @@ -885,8 +885,10 @@ static int dw_i3c_master_daa(struct i3c_master_controller *m)
> COMMAND_PORT_ROC;
>
> dw_i3c_master_enqueue_xfer(master, xfer);
> - if (!wait_for_completion_timeout(&xfer->comp, XFER_TIMEOUT))
> + if (!wait_for_completion_timeout(&xfer->comp, XFER_TIMEOUT)) {
> dw_i3c_master_dequeue_xfer(master, xfer);
[Severity: High]
This is a pre-existing issue, but does this dequeue logic leave the hardware
susceptible to late responses?
Looking at dw_i3c_master_dequeue_xfer_locked(), it resets the software queues:
dw_i3c_master_dequeue_xfer_locked() {
...
writel(RESET_CTRL_RX_FIFO | RESET_CTRL_TX_FIFO |
RESET_CTRL_RESP_QUEUE | RESET_CTRL_CMD_QUEUE,
master->regs + RESET_CTRL);
...
}
However, this does not halt or soft-reset the underlying hardware state
machine (using RESET_CTRL_SOFT). If the transaction is merely delayed on the
bus and finishes later, won't a response be pushed to the newly cleared
RESP_QUEUE? Since a subsequent single-command transfer (like another DAA)
will use a Transaction ID (TID) of 0, could the IRQ handler mistakenly apply
the late response to the new transfer, leading to state mismatch and data
corruption?
[Severity: High]
This is also a pre-existing issue, but does dw_i3c_master_dequeue_xfer_locked()
strand other pending transfers when a timeout occurs?
When setting the current transfer to NULL on timeout:
dw_i3c_master_dequeue_xfer_locked() {
...
master->xferqueue.cur = NULL;
...
}
The driver fails to pull and start the next transfer from master->xferqueue.list.
Because cur is NULL, the next time dw_i3c_master_enqueue_xfer() is called,
it will bypass the queue list entirely, set the new transfer as cur, and
start it immediately:
dw_i3c_master_enqueue_xfer() {
...
if (master->xferqueue.cur) {
list_add_tail(&xfer->node, &master->xferqueue.list);
} else {
master->xferqueue.cur = xfer;
dw_i3c_master_start_xfer_locked(master);
}
}
Will this cause older queued transfers to languish in the list forever and
inevitably fail via timeout?
> + goto rpm_out;
> + }
>
> newdevs = GENMASK(master->maxdevs - cmd->rx_len - 1, 0);
[Severity: High]
This isn't a bug introduced by this patch, but if the transfer completes
with a hardware error (like a parity error or NACK), is xfer->ret ignored?
If the completion succeeds but xfer->ret indicates an error occurred before
any devices were assigned, cmd->rx_len will still equal the requested count
(master->maxdevs). This makes the calculation:
master->maxdevs - cmd->rx_len - 1
evaluate to -1.
Doesn't GENMASK(-1, 0) trigger C undefined behavior by shifting by the
type's width, resulting in ~0UL? This looks like it would cause the driver
to erroneously register all free slots as phantom devices, which could lead
to a system panic when communicating with them later.
> newdevs &= ~olddevs;
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260803140307.12665-1-jszhang@kernel.org?part=1
--
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] i3c: dw: reduce do_daa time if there's no client
2026-08-03 14:03 [PATCH] i3c: dw: reduce do_daa time if there's no client Jisheng Zhang
2026-08-03 14:36 ` sashiko-bot
@ 2026-08-05 20:05 ` Frank Li
1 sibling, 0 replies; 3+ messages in thread
From: Frank Li @ 2026-08-05 20:05 UTC (permalink / raw)
To: Jisheng Zhang; +Cc: Alexandre Belloni, Frank Li, linux-i3c, linux-kernel
On Mon, Aug 03, 2026 at 10:03:07PM +0800, Jisheng Zhang wrote:
> If there's no client, wait for completion will timeout, then there's no
> need to add i3c dev at all. This can save boot time by ~6s if there's
> no client connected to the i3c interface.
>
> Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
> ---
> drivers/i3c/master/dw-i3c-master.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/i3c/master/dw-i3c-master.c b/drivers/i3c/master/dw-i3c-master.c
> index 0625587f1527..18ebf304029a 100644
> --- a/drivers/i3c/master/dw-i3c-master.c
> +++ b/drivers/i3c/master/dw-i3c-master.c
> @@ -885,8 +885,10 @@ static int dw_i3c_master_daa(struct i3c_master_controller *m)
> COMMAND_PORT_ROC;
>
> dw_i3c_master_enqueue_xfer(master, xfer);
> - if (!wait_for_completion_timeout(&xfer->comp, XFER_TIMEOUT))
> + if (!wait_for_completion_timeout(&xfer->comp, XFER_TIMEOUT)) {
> dw_i3c_master_dequeue_xfer(master, xfer);
> + goto rpm_out;
not sure why save 6s. timeout already happen, does register i3c device
takes 6s?
And if do_daa find 3 devices, then timeout, you skip register found 3
devices.
Frank
> + }
>
> newdevs = GENMASK(master->maxdevs - cmd->rx_len - 1, 0);
> newdevs &= ~olddevs;
> --
> 2.53.0
>
--
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-08-05 20:05 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-03 14:03 [PATCH] i3c: dw: reduce do_daa time if there's no client Jisheng Zhang
2026-08-03 14:36 ` sashiko-bot
2026-08-05 20:05 ` Frank Li
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).