Re: [PATCH V4 08/17] i3c: mipi-i3c-hci: Avoid restarting DMA ring after aborting wrong transfer

[Frank Li <Frank.li@nxp.com>]

On Fri, May 15, 2026 at 07:26:12PM +0300, Adrian Hunter wrote:
> Software ABORT of the DMA ring is used to recover from transfer list
> timeouts, but it is inherently racy.  The intended transfer list may
> complete just before the ABORT takes effect, causing the subsequent
> transfer list to be aborted instead.
>
> In this case, an incomplete transfer list may remain in the ring and has
> not yet been processed by hci_dma_dequeue_xfer().  Restarting the DMA
> ring at that point can lead to unpredictable results.
>
> Detect when the next queued transfer is not the first entry of a transfer
> list and does not belong to the list currently being dequeued.  In that
> case, skip restarting the DMA ring and defer recovery until a subsequent
> call to hci_dma_dequeue_xfer(), which will safely restart the ring once
> the incomplete list is handled.
>
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.com>

>
>
> Changes in V3 and V4:
>
> 	None
>
> Changes in V2:
>
> 	Renamed completing_xfer to final_xfer
>
>
>  drivers/i3c/master/mipi-i3c-hci/dma.c | 15 +++++++++++++++
>  drivers/i3c/master/mipi-i3c-hci/hci.h |  1 +
>  2 files changed, 16 insertions(+)
>
> diff --git a/drivers/i3c/master/mipi-i3c-hci/dma.c b/drivers/i3c/master/mipi-i3c-hci/dma.c
> index 83b553e1ab0b..8e27fb6f18f5 100644
> --- a/drivers/i3c/master/mipi-i3c-hci/dma.c
> +++ b/drivers/i3c/master/mipi-i3c-hci/dma.c
> @@ -503,6 +503,7 @@ static int hci_dma_queue_xfer(struct i3c_hci *hci,
>  		u32 *ring_data = rh->xfer + rh->xfer_struct_sz * enqueue_ptr;
>
>  		xfer->final_xfer = xfer_list + n - 1;
> +		xfer->xfer_list_pos = i;
>
>  		/* store cmd descriptor */
>  		*ring_data++ = xfer->cmd_desc[0];
> @@ -669,6 +670,20 @@ static bool hci_dma_dequeue_xfer(struct i3c_hci *hci,
>  		}
>  	}
>
> +	/*
> +	 * A software ABORT may race with transfer completion and abort the next
> +	 * transfer list instead. Detect that case, and do not restart the ring.
> +	 * It will be handled by a subsequent dequeue.
> +	 */
> +	if (!did_unqueue) {
> +		struct hci_xfer *xfer = rh->src_xfers[rh->done_ptr];
> +
> +		if (xfer && xfer->xfer_list_pos && xfer->final_xfer != xfer_list->final_xfer) {
> +			spin_unlock_irq(&hci->lock);
> +			return false;
> +		}
> +	}
> +
>  	/* restart the ring */
>  	reinit_completion(&rh->op_done);
>  	mipi_i3c_hci_resume(hci);
> diff --git a/drivers/i3c/master/mipi-i3c-hci/hci.h b/drivers/i3c/master/mipi-i3c-hci/hci.h
> index f07fc627d4d2..83d4f13a68a3 100644
> --- a/drivers/i3c/master/mipi-i3c-hci/hci.h
> +++ b/drivers/i3c/master/mipi-i3c-hci/hci.h
> @@ -107,6 +107,7 @@ struct hci_xfer {
>  			struct hci_xfer *final_xfer;
>  			int ring_number;
>  			int ring_entry;
> +			int xfer_list_pos;
>  		};
>  	};
>  };
> --
> 2.51.0
>