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

[Frank Li]

On Mon, Mar 02, 2026 at 10:45:11AM +0200, Adrian Hunter wrote:
> On 27/02/2026 18:25, Frank Li wrote:
> > 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?
>
> No the completion already waits for op_done

Okay, suppose check return value of wait_for_completion_timeout().  read
register also work.

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

Frank
>
> >
> > 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
>
>
> --
> linux-i3c mailing list
> linux-i3c at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-i3c