[PATCH 06/12] i3c: mipi-i3c-hci: Correct RING_CTRL_ABORT handling in DMA dequeue

[Frank Li]

On Fri, Feb 27, 2026 at 04:11:43PM +0200, Adrian Hunter wrote:
> The logic used to abort the DMA ring contains several flaws:
>
>  1. The driver unconditionally issues a ring abort even when the ring has
>     already stopped.
>  2. The completion used to wait for abort completion is never
>     re-initialized, resulting in incorrect wait behavior.
>  3. The abort sequence unintentionally clears RING_CTRL_ENABLE, which
>     resets hardware ring pointers and disrupts the controller state.
>  4. If the ring is already stopped, the abort operation should be
>     considered successful without attempting further action.
>
> Fix the abort handling by checking whether the ring is running before
> issuing an abort, reinitializing the completion when needed, ensuring that
> RING_CTRL_ENABLE remains asserted during abort, and treating an already
> stopped ring as a successful condition.
>
> Fixes: 9ad9a52cce282 ("i3c/master: introduce the mipi-i3c-hci driver")
> Cc: stable at vger.kernel.org
> Signed-off-by: Adrian Hunter <adrian.hunter at intel.com>
> ---
>  drivers/i3c/master/mipi-i3c-hci/dma.c | 25 ++++++++++++++++---------
>  1 file changed, 16 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/i3c/master/mipi-i3c-hci/dma.c b/drivers/i3c/master/mipi-i3c-hci/dma.c
> index 8d5f808e03ea..dff96b84479e 100644
> --- a/drivers/i3c/master/mipi-i3c-hci/dma.c
> +++ b/drivers/i3c/master/mipi-i3c-hci/dma.c
> @@ -550,18 +550,25 @@ static bool hci_dma_dequeue_xfer(struct i3c_hci *hci,
>  	struct hci_rh_data *rh = &rings->headers[xfer_list[0].ring_number];
>  	unsigned int i;
>  	bool did_unqueue = false;
> +	u32 ring_status;
>
>  	guard(mutex)(&rh->control_mutex);
>
> -	/* stop the ring */
> -	rh_reg_write(RING_CONTROL, RING_CTRL_ABORT);
> -	if (wait_for_completion_timeout(&rh->op_done, HZ) == 0) {
> -		/*
> -		 * We're deep in it if ever this condition is ever met.
> -		 * Hardware might still be writing to memory, etc.
> -		 */
> -		dev_crit(&hci->master.dev, "unable to abort the ring\n");
> -		WARN_ON(1);
> +	ring_status = rh_reg_read(RING_STATUS);
> +	if (ring_status & RING_STATUS_RUNNING) {
> +		/* stop the ring */
> +		reinit_completion(&rh->op_done);
> +		rh_reg_write(RING_CONTROL, RING_CTRL_ENABLE | RING_CTRL_ABORT);
> +		wait_for_completion_timeout(&rh->op_done, HZ);
> +		ring_status = rh_reg_read(RING_STATUS);
> +		if (ring_status & RING_STATUS_RUNNING) {

Do you need readl_poll_timeout() here to make sure hardware actual stopped?

Frank

> +			/*
> +			 * We're deep in it if ever this condition is ever met.
> +			 * Hardware might still be writing to memory, etc.
> +			 */
> +			dev_crit(&hci->master.dev, "unable to abort the ring\n");
> +			WARN_ON(1);
> +		}
>  	}
>
>  	spin_lock_irq(&rh->lock);
> --
> 2.51.0
>
>
> --
> linux-i3c mailing list
> linux-i3c at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-i3c