Re: [PATCH] i3c: dw: Prevent premature transfer completion on IBI

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [High] i3c: dw: invalid Signed-off-by trailer using handle rui05.wang
It also noticed 1 related pre-existing issue this patch does not
introduce:
- [High] i3c: dw-i3c-master: queue stall in
  dw_i3c_master_dequeue_xfer_locked() [1]
--

commit a91019b468ed66eec75160248bee93e6d03bfe83
Author: rui05.wang <wr574332525@163.com>

> i3c: dw: Prevent premature transfer completion on IBI
>
> An IBI threshold interrupt can arrive while a transfer is pending and
> before its command responses are ready. The IRQ handler calls
> dw_i3c_master_end_xfer_locked() even for an IBI-only interrupt. If the
> response queue is empty, the function completes the transfer with a
> return value of zero. Later responses can remain unconsumed and keep the
> response-ready interrupt asserted.
>
> Only call the transfer completion handler when INTR_MASTER_MASK
> indicates that responses are ready or a transfer error has occurred.
> An IBI-only interrupt leaves the transfer pending.
>
> The fix was tested successfully on FPGA hardware.
>
> Fixes: e389b1d72a624 ("i3c: dw: Add support for in-band interrupts")
> Signed-off-by: rui05.wang <wr574332525@163.com>

[Severity: High]
The Author header and Signed-off-by trailer use an account handle
(rui05.wang) rather than a real human name, which does not meet the
Developer's Certificate of Origin requirements in
Documentation/process/submitting-patches.rst.

> diff --git a/drivers/i3c/master/dw-i3c-master.c b/drivers/i3c/master/dw-i3c-master.c
> index 34aa8f8e1a0a2..19735ec3db042 100644
> --- a/drivers/i3c/master/dw-i3c-master.c
> +++ b/drivers/i3c/master/dw-i3c-master.c
> @@ -1527,7 +1527,8 @@ static irqreturn_t dw_i3c_master_irq_handler(int irq, void *dev_id)
>  	}
>  
>  	spin_lock(&master->xferqueue.lock);
> -	dw_i3c_master_end_xfer_locked(master, status);
> +	if (status & INTR_MASTER_MASK)
> +		dw_i3c_master_end_xfer_locked(master, status);
>  	if (status & INTR_TRANSFER_ERR_STAT)
>  		writel(INTR_TRANSFER_ERR_STAT, master->regs + INTR_STATUS);
>  	spin_unlock(&master->xferqueue.lock);

[Severity: High]
This problem wasn't introduced by this patch, but can
dw_i3c_master_dequeue_xfer_locked() stall master->xferqueue.list when a
transfer times out [1]?

drivers/i3c/master/dw-i3c-master.c:dw_i3c_master_dequeue_xfer_locked() {
	if (master->xferqueue.cur == xfer) {
		u32 status;

		master->xferqueue.cur = NULL;

		writel(RESET_CTRL_RX_FIFO | RESET_CTRL_TX_FIFO |
		       RESET_CTRL_RESP_QUEUE | RESET_CTRL_CMD_QUEUE,
		       master->regs + RESET_CTRL);

		readl_poll_timeout_atomic(master->regs + RESET_CTRL, status,
					  !status, 10, 1000000);
	} else {
		list_del_init(&xfer->node);
	}
}

When the active transfer times out, dw_i3c_master_dequeue_xfer_locked()
resets the hardware FIFOs and sets master->xferqueue.cur to NULL without
popping and starting the next transfer from master->xferqueue.list.  Any
transfers already queued in master->xferqueue.list remain stranded, and a
later dw_i3c_master_enqueue_xfer() call sees master->xferqueue.cur == NULL
and starts the new transfer immediately ahead of them.

[1] https://sashiko.dev/#/bug/linux-14289fee-c593-4b6f-a612-d3df728e5dc7

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261112084056.1147-1-wr574332525@163.com?part=1

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