[PATCH V3 08/16] i3c: mipi-i3c-hci: Avoid restarting DMA ring after aborting wrong transfer

[Adrian Hunter]

On 12/05/2026 19:50, Frank Li wrote:
> On Mon, May 04, 2026 at 02:33:44PM +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 at intel.com>
>> ---
>>
>>
>> Changes in V3:
>>
>> 	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 899fdf6555a8..268f54b32101 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);
> 
> Is it possible to use auto cleanup to handle lock()?

No, there are 2 paths that do not return directly after unlocking
and do different things.

> 
> Frank
>> +			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
>>