[PATCH V2 12/14] i3c: mipi-i3c-hci: Fix race in DMA error handling in interrupt context

[Frank Li]

On Wed, Mar 04, 2026 at 08:17:03PM +0200, Adrian Hunter wrote:
> The DMA ring halts whenever a transfer encounters an error. The interrupt
> handler previously attempted to detect this situation and restart the ring
> if a transfer completed at the same time. However, this restart logic runs
> entirely in interrupt context and is inherently racy: it interacts with
> other paths manipulating the ring state, and fully serializing it within
> the interrupt handler is not practical.
>
> Move this error-recovery logic out of the interrupt handler and into the
> transfer-processing path (i3c_hci_process_xfer()), where serialization and
> state management are already controlled. Introduce a new optional I/O-ops
> callback, handle_error(), invoked when a completed transfer reports an
> error. For DMA operation, the implementation simply calls the existing
> dequeue function, which safely aborts and restarts the ring when needed.
>
> This removes the fragile ring-restart logic from the interrupt handler and
> centralizes error handling where proper sequencing can be ensured.
>
> Fixes: ccdb2e0e3b00d ("i3c: mipi-i3c-hci: Add Intel specific quirk to ring resuming")
> Cc: stable at vger.kernel.org
> Signed-off-by: Adrian Hunter <adrian.hunter at intel.com>
> ---

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

>
>
> Changes in V2:
>
> 	None
>
>
>  drivers/i3c/master/mipi-i3c-hci/core.c | 19 ++++++++++++----
>  drivers/i3c/master/mipi-i3c-hci/dma.c  | 31 +++++++-------------------
>  drivers/i3c/master/mipi-i3c-hci/hci.h  |  1 +
>  3 files changed, 24 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/i3c/master/mipi-i3c-hci/core.c b/drivers/i3c/master/mipi-i3c-hci/core.c
> index 4a80671536f0..b98952d12d7c 100644
> --- a/drivers/i3c/master/mipi-i3c-hci/core.c
> +++ b/drivers/i3c/master/mipi-i3c-hci/core.c
> @@ -223,10 +223,21 @@ int i3c_hci_process_xfer(struct i3c_hci *hci, struct hci_xfer *xfer, int n)
>  	if (ret)
>  		return ret;
>
> -	if (!wait_for_completion_timeout(done, timeout) &&
> -	    hci->io->dequeue_xfer(hci, xfer, n)) {
> -		dev_err(&hci->master.dev, "%s: timeout error\n", __func__);
> -		return -ETIMEDOUT;
> +	if (!wait_for_completion_timeout(done, timeout)) {
> +		if (hci->io->dequeue_xfer(hci, xfer, n)) {
> +			dev_err(&hci->master.dev, "%s: timeout error\n", __func__);
> +			return -ETIMEDOUT;
> +		}
> +		return 0;
> +	}
> +
> +	if (hci->io->handle_error) {
> +		bool error = false;
> +
> +		for (int i = 0; i < n && !error; i++)
> +			error = RESP_STATUS(xfer[i].response);
> +		if (error)
> +			return hci->io->handle_error(hci, xfer, n);
>  	}
>
>  	return 0;
> diff --git a/drivers/i3c/master/mipi-i3c-hci/dma.c b/drivers/i3c/master/mipi-i3c-hci/dma.c
> index 41b83f07fdab..e487ef52f6b4 100644
> --- a/drivers/i3c/master/mipi-i3c-hci/dma.c
> +++ b/drivers/i3c/master/mipi-i3c-hci/dma.c
> @@ -609,6 +609,11 @@ static bool hci_dma_dequeue_xfer(struct i3c_hci *hci,
>  	return did_unqueue;
>  }
>
> +static int hci_dma_handle_error(struct i3c_hci *hci, struct hci_xfer *xfer_list, int n)
> +{
> +	return hci_dma_dequeue_xfer(hci, xfer_list, n) ? -EIO : 0;
> +}
> +
>  static void hci_dma_xfer_done(struct i3c_hci *hci, struct hci_rh_data *rh)
>  {
>  	u32 op1_val, op2_val, resp, *ring_resp;
> @@ -870,29 +875,8 @@ static bool hci_dma_irq_handler(struct i3c_hci *hci)
>  			hci_dma_xfer_done(hci, rh);
>  		if (status & INTR_RING_OP)
>  			complete(&rh->op_done);
> -
> -		if (status & INTR_TRANSFER_ABORT) {
> -			u32 ring_status;
> -
> -			dev_notice_ratelimited(&hci->master.dev,
> -				"Ring %d: Transfer Aborted\n", i);
> -			mipi_i3c_hci_resume(hci);
> -			ring_status = rh_reg_read(RING_STATUS);
> -			if (!(ring_status & RING_STATUS_RUNNING) &&
> -			    status & INTR_TRANSFER_COMPLETION &&
> -			    status & INTR_TRANSFER_ERR) {
> -				/*
> -				 * Ring stop followed by run is an Intel
> -				 * specific required quirk after resuming the
> -				 * halted controller. Do it only when the ring
> -				 * is not in running state after a transfer
> -				 * error.
> -				 */
> -				rh_reg_write(RING_CONTROL, RING_CTRL_ENABLE);
> -				rh_reg_write(RING_CONTROL, RING_CTRL_ENABLE |
> -							   RING_CTRL_RUN_STOP);
> -			}
> -		}
> +		if (status & INTR_TRANSFER_ABORT)
> +			dev_dbg(&hci->master.dev, "Ring %d: Transfer Aborted\n", i);
>  		if (status & INTR_IBI_RING_FULL)
>  			dev_err_ratelimited(&hci->master.dev,
>  				"Ring %d: IBI Ring Full Condition\n", i);
> @@ -908,6 +892,7 @@ const struct hci_io_ops mipi_i3c_hci_dma = {
>  	.cleanup		= hci_dma_cleanup,
>  	.queue_xfer		= hci_dma_queue_xfer,
>  	.dequeue_xfer		= hci_dma_dequeue_xfer,
> +	.handle_error		= hci_dma_handle_error,
>  	.irq_handler		= hci_dma_irq_handler,
>  	.request_ibi		= hci_dma_request_ibi,
>  	.free_ibi		= hci_dma_free_ibi,
> diff --git a/drivers/i3c/master/mipi-i3c-hci/hci.h b/drivers/i3c/master/mipi-i3c-hci/hci.h
> index 850016e3d4fe..9ac9d0e342f4 100644
> --- a/drivers/i3c/master/mipi-i3c-hci/hci.h
> +++ b/drivers/i3c/master/mipi-i3c-hci/hci.h
> @@ -123,6 +123,7 @@ struct hci_io_ops {
>  	bool (*irq_handler)(struct i3c_hci *hci);
>  	int (*queue_xfer)(struct i3c_hci *hci, struct hci_xfer *xfer, int n);
>  	bool (*dequeue_xfer)(struct i3c_hci *hci, struct hci_xfer *xfer, int n);
> +	int (*handle_error)(struct i3c_hci *hci, struct hci_xfer *xfer, int n);
>  	int (*request_ibi)(struct i3c_hci *hci, struct i3c_dev_desc *dev,
>  			   const struct i3c_ibi_setup *req);
>  	void (*free_ibi)(struct i3c_hci *hci, struct i3c_dev_desc *dev);
> --
> 2.51.0
>
>
> --
> linux-i3c mailing list
> linux-i3c at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-i3c