Re: [PATCH V3 00/16] i3c: mipi-i3c-hci: DMA abort, recovery and related improvements

Re: [PATCH V3 00/16] i3c: mipi-i3c-hci: DMA abort, recovery and related improvements

[Adrian Hunter]

On 04/05/2026 14:33, Adrian Hunter wrote:
> Hi
> 
> This series improves the robustness of the MIPI I3C HCI DMA mode driver,
> addressing issues observed during error handling and recovery.

Any comments on this?

> 
> Patch 1 ensures suspend always invokes io->suspend.
> 
> Patches 2-4 fix issues in the existing DMA abort path: preserving the RUN
> bit during abort per the MIPI specification, blocking enqueue during
> abort/error, and waiting for ring restart completion.
> 
> Patches 5-8 improve how partially completed transfer lists are handled
> during dequeue: moving hci_dma_xfer_done() earlier so completed
> responses are processed before NoOp replacement, completing transfer
> lists immediately on error rather than deferring, and detecting when an
> abort races with transfer completion to avoid restarting the wrong
> transfer list.
> 
> Patches 9-10 add Intel-specific quirks for DMA ring abort: a PIO queue
> reset after abort, and an HC_CONTROL ABORT before the ring-level abort.
> 
> Patch 11 factors out a reset-and-restore helper from the suspend path
> for reuse.
> 
> Patch 12 adds a full DMA recovery path for internal controller errors.
> When the hardware reports a TID mismatch or the ring becomes stuck, the
> driver now resets and restores the controller, terminating all in-flight
> transfers with an error status.
> 
> Patch 13 makes NoOp command handling observable: instead of discarding
> NoOp responses, the driver now waits for them to complete and triggers
> recovery if they fail.
> 
> Patch 14 adjusts transfer timeout accounting to start from when a
> transfer actually begins execution rather than when it was queued,
> preventing premature timeouts behind slow predecessors.
> 
> Patches 15-16 are minor optimizations: consolidating the DMA command and
> response ring into a single coherent allocation, and increasing the ring
> size to the maximum 255 entries to avoid ring-space exhaustion.
> 
> 
> Changes in V3:
> 
>   i3c: mipi-i3c-hci: Fix suspend behavior when bus disable falls back to software reset
>   i3c: mipi-i3c-hci: Preserve RUN bit when aborting DMA ring
> 	Add Frank's rev'd-by
> 
>   i3c: mipi-i3c-hci: Add DMA-mode recovery for internal controller errors
> 	When erroring out transfers, ensure the final transfer of a
> 	transfer list is processed last
> 
> 
> Changes in V2:
> 
>   i3c: mipi-i3c-hci: Fix suspend behavior when bus disable falls back to software reset
> 	Always return 0 from suspend callback
> 	Amend commit message
> 
>   i3c: mipi-i3c-hci: Preserve RUN bit when aborting DMA ring
> 	Improve commit message
> 
>   i3c: mipi-i3c-hci: Prevent DMA enqueue while ring is aborting or in error
> 	Improve commit message
> 
>   i3c: mipi-i3c-hci: Wait for DMA ring restart to complete
> 	None
> 
>   i3c: mipi-i3c-hci: Move hci_dma_xfer_done() definition
> 	Add Frank's Rev'd-by
> 
>   i3c: mipi-i3c-hci: Call hci_dma_xfer_done() from dequeue path
> 	Add Frank's Rev'd-by
> 
>   i3c: mipi-i3c-hci: Complete transfer lists immediately on error
> 	Rename completing_xfer to final_xfer
> 
>   i3c: mipi-i3c-hci: Avoid restarting DMA ring after aborting wrong transfer
> 	Rename completing_xfer to final_xfer
> 
>   i3c: mipi-i3c-hci: Add DMA ring abort/reset quirk for Intel controllers
> 	None
> 
>   i3c: mipi-i3c-hci: Add DMA ring abort quirk for Intel controllers
> 	None
> 
>   i3c: mipi-i3c-hci: Factor out reset-and-restore helper
> 	Drop redundant i3c_hci_sync_irq_inactive(hci)
> 	from i3c_hci_reset_and_restore() because it is called by
> 	hci->io->suspend() anyway
> 
>   i3c: mipi-i3c-hci: Add DMA-mode recovery for internal controller errors
> 	Rename completing_xfer to final_xfer
> 	Add hci_dma_xfer_done() before checking for an already complete
> 	transfer
> 	Improve commit message
> 
>   i3c: mipi-i3c-hci: Wait for NoOp commands to complete
> 	Rename completing_xfer to final_xfer
> 	Add missing reinit_completion()
> 
>   i3c: mipi-i3c-hci: Base timeouts on actual transfer start time
> 	Do not flag the next transfer as started when there is an error
> 	which halts the controller
> 	Instead flag it started at the end of hci_dma_dequeue_xfer()
> 	Use hci_start_xfer() in pio.c
> 
>   i3c: mipi-i3c-hci: Consolidate DMA ring allocation
> 	Check for failed allocation before assignments to avoid doing
> 	arithmetic with NULL pointers
> 
>   i3c: mipi-i3c-hci: Increase DMA transfer ring size to maximum
> 	None
> 
> 
> Adrian Hunter (16):
>   i3c: mipi-i3c-hci: Fix suspend behavior when bus disable falls back to software reset
>   i3c: mipi-i3c-hci: Preserve RUN bit when aborting DMA ring
>   i3c: mipi-i3c-hci: Prevent DMA enqueue while ring is aborting or in error
>   i3c: mipi-i3c-hci: Wait for DMA ring restart to complete
>   i3c: mipi-i3c-hci: Move hci_dma_xfer_done() definition
>   i3c: mipi-i3c-hci: Call hci_dma_xfer_done() from dequeue path
>   i3c: mipi-i3c-hci: Complete transfer lists immediately on error
>   i3c: mipi-i3c-hci: Avoid restarting DMA ring after aborting wrong transfer
>   i3c: mipi-i3c-hci: Add DMA ring abort/reset quirk for Intel controllers
>   i3c: mipi-i3c-hci: Add DMA ring abort quirk for Intel controllers
>   i3c: mipi-i3c-hci: Factor out reset-and-restore helper
>   i3c: mipi-i3c-hci: Add DMA-mode recovery for internal controller errors
>   i3c: mipi-i3c-hci: Wait for NoOp commands to complete
>   i3c: mipi-i3c-hci: Base timeouts on actual transfer start time
>   i3c: mipi-i3c-hci: Consolidate DMA ring allocation
>   i3c: mipi-i3c-hci: Increase DMA transfer ring size to maximum
> 
>  drivers/i3c/master/mipi-i3c-hci/cmd.h  |   6 +
>  drivers/i3c/master/mipi-i3c-hci/core.c |  82 ++++++--
>  drivers/i3c/master/mipi-i3c-hci/dma.c  | 344 +++++++++++++++++++++++++--------
>  drivers/i3c/master/mipi-i3c-hci/hci.h  |  22 +++
>  drivers/i3c/master/mipi-i3c-hci/pio.c  |   1 +
>  5 files changed, 365 insertions(+), 90 deletions(-)
> 
> 
> Regards
> Adrian

Re: [PATCH V3 03/16] i3c: mipi-i3c-hci: Prevent DMA enqueue while ring is aborting or in error

[Frank Li]

On Mon, May 04, 2026 at 02:33:39PM +0300, Adrian Hunter wrote:
> Block the DMA enqueue path while a Ring abort is in progress or after an
> error condition has been detected.
>
> Previously, new transfers could be enqueued while the DMA Ring was being
> aborted or while error handling was underway. This allowed enqueue and
> error-recovery paths to run concurrently, potentially interfering with
> each other and corrupting Ring state.
>
> Introduce explicit enqueue blocking and a wait queue to serialize access:
> enqueue operations now wait until abort or error handling has completed
> before proceeding. Enqueue is unblocked once the Ring is safely restarted.
>
> Note, there is only 1 ring bundle configured, and a transfer error causes
> the controller to halt ring (bundle) operation, so there is only ever 1
> outstanding error at a time.  Furthermore, a later patch ensures that only
> the currently active transfer list can time out.  Consequently, the DMA
> queue will not be unblocked while there are outstanding transfer errors or
> timeouts.
>
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> ---

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

>
>
> Changes in V3:
>
> 	None
>
> Changes in V2:
>
> 	Improve commit message
>
>
>  drivers/i3c/master/mipi-i3c-hci/core.c |  1 +
>  drivers/i3c/master/mipi-i3c-hci/dma.c  | 25 +++++++++++++++++++++++--
>  drivers/i3c/master/mipi-i3c-hci/hci.h  |  2 ++
>  3 files changed, 26 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/i3c/master/mipi-i3c-hci/core.c b/drivers/i3c/master/mipi-i3c-hci/core.c
> index afb0764b5e1f..44617eb3a3f1 100644
> --- a/drivers/i3c/master/mipi-i3c-hci/core.c
> +++ b/drivers/i3c/master/mipi-i3c-hci/core.c
> @@ -973,6 +973,7 @@ static int i3c_hci_probe(struct platform_device *pdev)
>
>  	spin_lock_init(&hci->lock);
>  	mutex_init(&hci->control_mutex);
> +	init_waitqueue_head(&hci->enqueue_wait_queue);
>
>  	/*
>  	 * Multi-bus instances share the same MMIO address range, but not
> diff --git a/drivers/i3c/master/mipi-i3c-hci/dma.c b/drivers/i3c/master/mipi-i3c-hci/dma.c
> index 4cd32e3afa7b..314635e6e190 100644
> --- a/drivers/i3c/master/mipi-i3c-hci/dma.c
> +++ b/drivers/i3c/master/mipi-i3c-hci/dma.c
> @@ -484,6 +484,12 @@ static int hci_dma_queue_xfer(struct i3c_hci *hci,
>
>  	spin_lock_irq(&hci->lock);
>
> +	while (unlikely(hci->enqueue_blocked)) {
> +		spin_unlock_irq(&hci->lock);
> +		wait_event(hci->enqueue_wait_queue, !READ_ONCE(hci->enqueue_blocked));
> +		spin_lock_irq(&hci->lock);
> +	}
> +
>  	if (n > rh->xfer_space) {
>  		spin_unlock_irq(&hci->lock);
>  		hci_dma_unmap_xfer(hci, xfer_list, n);
> @@ -539,6 +545,14 @@ static int hci_dma_queue_xfer(struct i3c_hci *hci,
>  	return 0;
>  }
>
> +static void hci_dma_unblock_enqueue(struct i3c_hci *hci)
> +{
> +	if (hci->enqueue_blocked) {
> +		hci->enqueue_blocked = false;
> +		wake_up_all(&hci->enqueue_wait_queue);
> +	}
> +}
> +
>  static bool hci_dma_dequeue_xfer(struct i3c_hci *hci,
>  				 struct hci_xfer *xfer_list, int n)
>  {
> @@ -550,12 +564,17 @@ static bool hci_dma_dequeue_xfer(struct i3c_hci *hci,
>
>  	guard(mutex)(&hci->control_mutex);
>
> +	spin_lock_irq(&hci->lock);
> +
>  	ring_status = rh_reg_read(RING_STATUS);
>  	if (ring_status & RING_STATUS_RUNNING) {
> +		hci->enqueue_blocked = true;
> +		spin_unlock_irq(&hci->lock);
>  		/* stop the ring */
>  		reinit_completion(&rh->op_done);
>  		rh_reg_write(RING_CONTROL, rh_reg_read(RING_CONTROL) | RING_CTRL_ABORT);
>  		wait_for_completion_timeout(&rh->op_done, HZ);
> +		spin_lock_irq(&hci->lock);
>  		ring_status = rh_reg_read(RING_STATUS);
>  		if (ring_status & RING_STATUS_RUNNING) {
>  			/*
> @@ -567,8 +586,6 @@ static bool hci_dma_dequeue_xfer(struct i3c_hci *hci,
>  		}
>  	}
>
> -	spin_lock_irq(&hci->lock);
> -
>  	for (i = 0; i < n; i++) {
>  		struct hci_xfer *xfer = xfer_list + i;
>  		int idx = xfer->ring_entry;
> @@ -604,6 +621,8 @@ static bool hci_dma_dequeue_xfer(struct i3c_hci *hci,
>  	rh_reg_write(RING_CONTROL, RING_CTRL_ENABLE);
>  	rh_reg_write(RING_CONTROL, RING_CTRL_ENABLE | RING_CTRL_RUN_STOP);
>
> +	hci_dma_unblock_enqueue(hci);
> +
>  	spin_unlock_irq(&hci->lock);
>
>  	return did_unqueue;
> @@ -647,6 +666,8 @@ static void hci_dma_xfer_done(struct i3c_hci *hci, struct hci_rh_data *rh)
>  			}
>  			if (xfer->completion)
>  				complete(xfer->completion);
> +			if (RESP_STATUS(resp))
> +				hci->enqueue_blocked = true;
>  		}
>
>  		done_ptr = (done_ptr + 1) % rh->xfer_entries;
> diff --git a/drivers/i3c/master/mipi-i3c-hci/hci.h b/drivers/i3c/master/mipi-i3c-hci/hci.h
> index f17f43494c1b..d630400ec945 100644
> --- a/drivers/i3c/master/mipi-i3c-hci/hci.h
> +++ b/drivers/i3c/master/mipi-i3c-hci/hci.h
> @@ -54,6 +54,8 @@ struct i3c_hci {
>  	struct mutex control_mutex;
>  	atomic_t next_cmd_tid;
>  	bool irq_inactive;
> +	bool enqueue_blocked;
> +	wait_queue_head_t enqueue_wait_queue;
>  	u32 caps;
>  	unsigned int quirks;
>  	unsigned int DAT_entries;
> --
> 2.51.0
>

Re: [PATCH V3 04/16] i3c: mipi-i3c-hci: Wait for DMA ring restart to complete

[Frank Li]

On Mon, May 04, 2026 at 02:33:40PM +0300, Adrian Hunter wrote:
> Although hci_dma_dequeue_xfer() is serialized against itself via
> control_mutex, this does not guarantee that a DMA ring restart
> triggered by a previous invocation has fully completed.
>
> When the function is called again in rapid succession, the DMA ring may
> still be transitioning back to the running state, which may confound or
> disrupt further state changes.
>
> Address this by waiting for the DMA ring restart to complete before
> continuing.
>
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> ---

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

>
>
> Changes in V2 and V3:
>
> 	None
>
>
>  drivers/i3c/master/mipi-i3c-hci/dma.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/drivers/i3c/master/mipi-i3c-hci/dma.c b/drivers/i3c/master/mipi-i3c-hci/dma.c
> index 314635e6e190..28614fdbf558 100644
> --- a/drivers/i3c/master/mipi-i3c-hci/dma.c
> +++ b/drivers/i3c/master/mipi-i3c-hci/dma.c
> @@ -617,6 +617,7 @@ static bool hci_dma_dequeue_xfer(struct i3c_hci *hci,
>  	}
>
>  	/* restart the ring */
> +	reinit_completion(&rh->op_done);
>  	mipi_i3c_hci_resume(hci);
>  	rh_reg_write(RING_CONTROL, RING_CTRL_ENABLE);
>  	rh_reg_write(RING_CONTROL, RING_CTRL_ENABLE | RING_CTRL_RUN_STOP);
> @@ -625,6 +626,8 @@ static bool hci_dma_dequeue_xfer(struct i3c_hci *hci,
>
>  	spin_unlock_irq(&hci->lock);
>
> +	wait_for_completion_timeout(&rh->op_done, HZ);
> +
>  	return did_unqueue;
>  }
>
> --
> 2.51.0
>

Re: [PATCH V3 07/16] i3c: mipi-i3c-hci: Complete transfer lists immediately on error

[Frank Li]

On Mon, May 04, 2026 at 02:33:43PM +0300, Adrian Hunter wrote:
> In DMA mode, transfer lists are currently completed only when the final
> transfer in the list completes. If an earlier transfer fails, the list is
> left incomplete and callers wait until timeout.
>
> There is no need to wait for a timeout, as the completion path in
> i3c_hci_process_xfer() already checks for error status. Complete the
> transfer list as soon as any transfer in the list reports an error.
>
> This avoids unnecessary delays and spurious timeouts on error.
>
> Complete a transfer list completion immediately there is an error.
>
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> ---

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

>
>
> Changes in V3:
>
> 	None
>
> Changes in V2:
>
> 	Renamed completing_xfer to final_xfer
>
>
>  drivers/i3c/master/mipi-i3c-hci/dma.c | 6 ++++--
>  drivers/i3c/master/mipi-i3c-hci/hci.h | 1 +
>  2 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/i3c/master/mipi-i3c-hci/dma.c b/drivers/i3c/master/mipi-i3c-hci/dma.c
> index 28e4d38f55d3..899fdf6555a8 100644
> --- a/drivers/i3c/master/mipi-i3c-hci/dma.c
> +++ b/drivers/i3c/master/mipi-i3c-hci/dma.c
> @@ -502,6 +502,8 @@ static int hci_dma_queue_xfer(struct i3c_hci *hci,
>  		struct hci_xfer *xfer = xfer_list + i;
>  		u32 *ring_data = rh->xfer + rh->xfer_struct_sz * enqueue_ptr;
>
> +		xfer->final_xfer = xfer_list + n - 1;
> +
>  		/* store cmd descriptor */
>  		*ring_data++ = xfer->cmd_desc[0];
>  		*ring_data++ = xfer->cmd_desc[1];
> @@ -576,8 +578,8 @@ static void hci_dma_xfer_done(struct i3c_hci *hci, struct hci_rh_data *rh)
>  					tid, xfer->cmd_tid);
>  				/* TODO: do something about it? */
>  			}
> -			if (xfer->completion)
> -				complete(xfer->completion);
> +			if (xfer == xfer->final_xfer || RESP_STATUS(resp))
> +				complete(xfer->final_xfer->completion);
>  			if (RESP_STATUS(resp))
>  				hci->enqueue_blocked = true;
>  		}
> diff --git a/drivers/i3c/master/mipi-i3c-hci/hci.h b/drivers/i3c/master/mipi-i3c-hci/hci.h
> index d630400ec945..f07fc627d4d2 100644
> --- a/drivers/i3c/master/mipi-i3c-hci/hci.h
> +++ b/drivers/i3c/master/mipi-i3c-hci/hci.h
> @@ -104,6 +104,7 @@ struct hci_xfer {
>  		struct {
>  			/* DMA specific */
>  			struct i3c_dma *dma;
> +			struct hci_xfer *final_xfer;
>  			int ring_number;
>  			int ring_entry;
>  		};
> --
> 2.51.0
>

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

[Frank Li]

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@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()?

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
>

Re: [PATCH V3 09/16] i3c: mipi-i3c-hci: Add DMA ring abort/reset quirk for Intel controllers

[Frank Li]

On Mon, May 04, 2026 at 02:33:45PM +0300, Adrian Hunter wrote:
> Some Intel I3C HCI controllers cannot reliably restart a DMA ring after an
> ABORT.  Additional queue resets are required to recover, and must be
> performed using PIO reset bits even while operating in DMA mode.
>
> This behavior is non-standard.  Introduce a controller quirk to opt into
> the required PIO queue resets after a DMA ring abort, and enable it for
> Intel LPSS I3C controllers.
>
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> ---
>
>
> Changes in V2 and V3:
>
> 	None
>
>
>  drivers/i3c/master/mipi-i3c-hci/core.c | 15 ++++++++++++++-
>  drivers/i3c/master/mipi-i3c-hci/dma.c  |  9 +++++++++
>  drivers/i3c/master/mipi-i3c-hci/hci.h  |  2 ++
>  3 files changed, 25 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/i3c/master/mipi-i3c-hci/core.c b/drivers/i3c/master/mipi-i3c-hci/core.c
> index 44617eb3a3f1..770235ad6b25 100644
> --- a/drivers/i3c/master/mipi-i3c-hci/core.c
> +++ b/drivers/i3c/master/mipi-i3c-hci/core.c
> @@ -240,6 +240,18 @@ void mipi_i3c_hci_pio_reset(struct i3c_hci *hci)
>  	reg_write(RESET_CONTROL, RX_FIFO_RST | TX_FIFO_RST | RESP_QUEUE_RST);
>  }
>
> +#define ALL_QUEUES_RST (CMD_QUEUE_RST | RESP_QUEUE_RST | RX_FIFO_RST | TX_FIFO_RST | IBI_QUEUE_RST)
> +
> +void mipi_i3c_hci_pio_reset_all_queues(struct i3c_hci *hci)
> +{
> +	u32 regval;
> +
> +	reg_write(RESET_CONTROL, ALL_QUEUES_RST);
> +	if (readx_poll_timeout_atomic(reg_read, RESET_CONTROL, regval,
> +				      !(regval & ALL_QUEUES_RST), 0, 20))
> +		dev_err(&hci->master.dev, "%s: Reset queues failed\n", __func__);
> +}
> +
>  /* located here rather than dct.c because needed bits are in core reg space */
>  void mipi_i3c_hci_dct_index_reset(struct i3c_hci *hci)
>  {
> @@ -1040,7 +1052,8 @@ MODULE_DEVICE_TABLE(acpi, i3c_hci_acpi_match);
>  static const struct platform_device_id i3c_hci_driver_ids[] = {
>  	{ .name = "intel-lpss-i3c", HCI_QUIRK_RPM_ALLOWED |
>  				    HCI_QUIRK_RPM_IBI_ALLOWED |
> -				    HCI_QUIRK_RPM_PARENT_MANAGED },
> +				    HCI_QUIRK_RPM_PARENT_MANAGED |
> +				    HCI_QUIRK_DMA_ABORT_REQUIRES_PIO_RESET },
>  	{ /* sentinel */ }
>  };
>  MODULE_DEVICE_TABLE(platform, i3c_hci_driver_ids);
> diff --git a/drivers/i3c/master/mipi-i3c-hci/dma.c b/drivers/i3c/master/mipi-i3c-hci/dma.c
> index 268f54b32101..699c6d523eed 100644
> --- a/drivers/i3c/master/mipi-i3c-hci/dma.c
> +++ b/drivers/i3c/master/mipi-i3c-hci/dma.c
> @@ -597,6 +597,13 @@ static void hci_dma_xfer_done(struct i3c_hci *hci, struct hci_rh_data *rh)
>  	rh_reg_write(RING_OPERATION1, op1_val);
>  }
>
> +static void hci_dma_abort_requires_pio_reset_quirk(struct i3c_hci *hci, struct hci_rh_data *rh)
> +{
> +	if ((hci->quirks & HCI_QUIRK_DMA_ABORT_REQUIRES_PIO_RESET) &&
> +	    (rh_reg_read(RING_STATUS) & RING_STATUS_ABORTED))
> +		mipi_i3c_hci_pio_reset_all_queues(hci);
> +}
> +
>  static void hci_dma_unblock_enqueue(struct i3c_hci *hci)
>  {
>  	if (hci->enqueue_blocked) {
> @@ -638,6 +645,8 @@ static bool hci_dma_dequeue_xfer(struct i3c_hci *hci,
>  		}
>  	}
>
> +	hci_dma_abort_requires_pio_reset_quirk(hci, rh);
> +

If only use once, needn't helper function since this helper function is
simple

Frank
>  	hci_dma_xfer_done(hci, rh);
>
>  	for (i = 0; i < n; i++) {
> diff --git a/drivers/i3c/master/mipi-i3c-hci/hci.h b/drivers/i3c/master/mipi-i3c-hci/hci.h
> index 83d4f13a68a3..01237b12d32e 100644
> --- a/drivers/i3c/master/mipi-i3c-hci/hci.h
> +++ b/drivers/i3c/master/mipi-i3c-hci/hci.h
> @@ -156,10 +156,12 @@ struct i3c_hci_dev_data {
>  #define HCI_QUIRK_RPM_ALLOWED		BIT(5)  /* Runtime PM allowed */
>  #define HCI_QUIRK_RPM_IBI_ALLOWED	BIT(6)  /* IBI and Hot-Join allowed while runtime suspended */
>  #define HCI_QUIRK_RPM_PARENT_MANAGED	BIT(7)  /* Runtime PM managed by parent device */
> +#define HCI_QUIRK_DMA_ABORT_REQUIRES_PIO_RESET	BIT(8)  /* Do PIO queue SW resets after DMA abort */
>
>  /* global functions */
>  void mipi_i3c_hci_resume(struct i3c_hci *hci);
>  void mipi_i3c_hci_pio_reset(struct i3c_hci *hci);
> +void mipi_i3c_hci_pio_reset_all_queues(struct i3c_hci *hci);
>  void mipi_i3c_hci_dct_index_reset(struct i3c_hci *hci);
>  void amd_set_od_pp_timing(struct i3c_hci *hci);
>  void amd_set_resp_buf_thld(struct i3c_hci *hci);
> --
> 2.51.0
>

Re: [PATCH V3 10/16] i3c: mipi-i3c-hci: Add DMA ring abort quirk for Intel controllers

[Frank Li]

On Mon, May 04, 2026 at 02:33:46PM +0300, Adrian Hunter wrote:
> DMA rings can be aborted either per-ring via RING_CONTROL or globally
> via HC_CONTROL_ABORT. The driver currently relies on the per-ring
> mechanism.
>
> Some Intel I3C HCI controllers require HC_CONTROL_ABORT to be asserted
> before a DMA ring abort is effective.  This behavior is non-standard.
> Introduce a controller quirk to select the required abort method and
> enable it for Intel LPSS I3C controllers.
>
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> ---
>
>
> Changes in V2 and V3:
>
> 	None
>
>
>  drivers/i3c/master/mipi-i3c-hci/core.c | 18 +++++++++++++++--
>  drivers/i3c/master/mipi-i3c-hci/dma.c  | 27 +++++++++++++++++++++++---
>  drivers/i3c/master/mipi-i3c-hci/hci.h  |  2 ++
>  3 files changed, 42 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/i3c/master/mipi-i3c-hci/core.c b/drivers/i3c/master/mipi-i3c-hci/core.c
> index 770235ad6b25..8274c84b16be 100644
> --- a/drivers/i3c/master/mipi-i3c-hci/core.c
> +++ b/drivers/i3c/master/mipi-i3c-hci/core.c
> @@ -231,7 +231,20 @@ static void i3c_hci_bus_cleanup(struct i3c_master_controller *m)
>
>  void mipi_i3c_hci_resume(struct i3c_hci *hci)
>  {
> -	reg_set(HC_CONTROL, HC_CONTROL_RESUME);
> +	u32 reg = reg_read(HC_CONTROL);
> +
> +	reg |= HC_CONTROL_RESUME;
> +	reg &= ~HC_CONTROL_ABORT;
> +	reg_write(HC_CONTROL, reg);
> +}
> +
> +void mipi_i3c_hci_abort(struct i3c_hci *hci)
> +{
> +	u32 reg = reg_read(HC_CONTROL);
> +
> +	reg &= ~HC_CONTROL_RESUME; /* Do not set resume */
> +	reg |= HC_CONTROL_ABORT;
> +	reg_write(HC_CONTROL, reg);
>  }
>
>  /* located here rather than pio.c because needed bits are in core reg space */
> @@ -1053,7 +1066,8 @@ static const struct platform_device_id i3c_hci_driver_ids[] = {
>  	{ .name = "intel-lpss-i3c", HCI_QUIRK_RPM_ALLOWED |
>  				    HCI_QUIRK_RPM_IBI_ALLOWED |
>  				    HCI_QUIRK_RPM_PARENT_MANAGED |
> -				    HCI_QUIRK_DMA_ABORT_REQUIRES_PIO_RESET },
> +				    HCI_QUIRK_DMA_ABORT_REQUIRES_PIO_RESET |
> +				    HCI_QUIRK_DMA_REQUIRES_HC_ABORT },
>  	{ /* sentinel */ }
>  };
>  MODULE_DEVICE_TABLE(platform, i3c_hci_driver_ids);
> diff --git a/drivers/i3c/master/mipi-i3c-hci/dma.c b/drivers/i3c/master/mipi-i3c-hci/dma.c
> index 699c6d523eed..41bbd912df7f 100644
> --- a/drivers/i3c/master/mipi-i3c-hci/dma.c
> +++ b/drivers/i3c/master/mipi-i3c-hci/dma.c
> @@ -597,6 +597,29 @@ static void hci_dma_xfer_done(struct i3c_hci *hci, struct hci_rh_data *rh)
>  	rh_reg_write(RING_OPERATION1, op1_val);
>  }
>
> +static bool hci_dma_requires_hc_abort_quirk(struct i3c_hci *hci, struct hci_rh_data *rh)
> +{
> +	if (!(hci->quirks & HCI_QUIRK_DMA_REQUIRES_HC_ABORT))
> +		return false;
> +
> +	reinit_completion(&rh->op_done);
> +	mipi_i3c_hci_abort(hci);
> +	wait_for_completion_timeout(&rh->op_done, HZ);
> +	rh_reg_write(RING_CONTROL, rh_reg_read(RING_CONTROL) | RING_CTRL_ABORT);
> +
> +	return true;
> +}
> +
> +static void hci_dma_abort(struct i3c_hci *hci, struct hci_rh_data *rh)
> +{
> +	if (hci_dma_requires_hc_abort_quirk(hci, rh))
> +		return;

Move check logic here to make overall logic simple

	if (hci->quirks & HCI_QUIRK_DMA_REQUIRES_HC_ABORT) {
		hci_dma_requires_hc_abort_quirk()
		return;
	}

> +
> +	reinit_completion(&rh->op_done);
> +	rh_reg_write(RING_CONTROL, rh_reg_read(RING_CONTROL) | RING_CTRL_ABORT);
> +	wait_for_completion_timeout(&rh->op_done, HZ);

move these codes reorg into new patch.

Frank
> +}
> +
>  static void hci_dma_abort_requires_pio_reset_quirk(struct i3c_hci *hci, struct hci_rh_data *rh)
>  {
>  	if ((hci->quirks & HCI_QUIRK_DMA_ABORT_REQUIRES_PIO_RESET) &&
> @@ -630,9 +653,7 @@ static bool hci_dma_dequeue_xfer(struct i3c_hci *hci,
>  		hci->enqueue_blocked = true;
>  		spin_unlock_irq(&hci->lock);
>  		/* stop the ring */
> -		reinit_completion(&rh->op_done);
> -		rh_reg_write(RING_CONTROL, rh_reg_read(RING_CONTROL) | RING_CTRL_ABORT);
> -		wait_for_completion_timeout(&rh->op_done, HZ);
> +		hci_dma_abort(hci, rh);
>  		spin_lock_irq(&hci->lock);
>  		ring_status = rh_reg_read(RING_STATUS);
>  		if (ring_status & RING_STATUS_RUNNING) {
> diff --git a/drivers/i3c/master/mipi-i3c-hci/hci.h b/drivers/i3c/master/mipi-i3c-hci/hci.h
> index 01237b12d32e..97c31a315a6e 100644
> --- a/drivers/i3c/master/mipi-i3c-hci/hci.h
> +++ b/drivers/i3c/master/mipi-i3c-hci/hci.h
> @@ -157,9 +157,11 @@ struct i3c_hci_dev_data {
>  #define HCI_QUIRK_RPM_IBI_ALLOWED	BIT(6)  /* IBI and Hot-Join allowed while runtime suspended */
>  #define HCI_QUIRK_RPM_PARENT_MANAGED	BIT(7)  /* Runtime PM managed by parent device */
>  #define HCI_QUIRK_DMA_ABORT_REQUIRES_PIO_RESET	BIT(8)  /* Do PIO queue SW resets after DMA abort */
> +#define HCI_QUIRK_DMA_REQUIRES_HC_ABORT		BIT(9)  /* Use HC_CONTROL ABORT to abort DMA */
>
>  /* global functions */
>  void mipi_i3c_hci_resume(struct i3c_hci *hci);
> +void mipi_i3c_hci_abort(struct i3c_hci *hci);
>  void mipi_i3c_hci_pio_reset(struct i3c_hci *hci);
>  void mipi_i3c_hci_pio_reset_all_queues(struct i3c_hci *hci);
>  void mipi_i3c_hci_dct_index_reset(struct i3c_hci *hci);
> --
> 2.51.0
>

Re: [PATCH V3 11/16] i3c: mipi-i3c-hci: Factor out reset-and-restore helper

[Frank Li]

On Mon, May 04, 2026 at 02:33:47PM +0300, Adrian Hunter wrote:
> Factor the reset-and-restore sequence out of i3c_hci_rpm_resume() into
> a separate helper.
>
> This allows the same logic to be reused for recovery paths in subsequent
> changes without duplicating suspend/resume handling.
>
> No functional change.
>
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> ---
Reviewed-by: Frank Li <Frank.Li@nxp.com>
>
>
> Changes in V3:
>
> 	None
>
> Changes in V2:
>
> 	Drop redundant i3c_hci_sync_irq_inactive(hci)
> 	from i3c_hci_reset_and_restore() because it is called by
> 	hci->io->suspend() anyway
>
>
>  drivers/i3c/master/mipi-i3c-hci/core.c | 19 +++++++++++++++++--
>  drivers/i3c/master/mipi-i3c-hci/hci.h  |  2 ++
>  2 files changed, 19 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/i3c/master/mipi-i3c-hci/core.c b/drivers/i3c/master/mipi-i3c-hci/core.c
> index 8274c84b16be..12a0122fb709 100644
> --- a/drivers/i3c/master/mipi-i3c-hci/core.c
> +++ b/drivers/i3c/master/mipi-i3c-hci/core.c
> @@ -798,9 +798,8 @@ int i3c_hci_rpm_suspend(struct device *dev)
>  }
>  EXPORT_SYMBOL_GPL(i3c_hci_rpm_suspend);
>
> -int i3c_hci_rpm_resume(struct device *dev)
> +static int i3c_hci_do_reset_and_restore(struct i3c_hci *hci)
>  {
> -	struct i3c_hci *hci = dev_get_drvdata(dev);
>  	int ret;
>
>  	ret = i3c_hci_reset_and_init(hci);
> @@ -821,6 +820,22 @@ int i3c_hci_rpm_resume(struct device *dev)
>
>  	return 0;
>  }
> +
> +int i3c_hci_reset_and_restore(struct i3c_hci *hci)
> +{
> +	i3c_hci_bus_disable(hci);
> +
> +	hci->io->suspend(hci);
> +
> +	return i3c_hci_do_reset_and_restore(hci);
> +}
> +
> +int i3c_hci_rpm_resume(struct device *dev)
> +{
> +	struct i3c_hci *hci = dev_get_drvdata(dev);
> +
> +	return i3c_hci_do_reset_and_restore(hci);
> +}
>  EXPORT_SYMBOL_GPL(i3c_hci_rpm_resume);
>
>  static int i3c_hci_runtime_suspend(struct device *dev)
> diff --git a/drivers/i3c/master/mipi-i3c-hci/hci.h b/drivers/i3c/master/mipi-i3c-hci/hci.h
> index 97c31a315a6e..a3151c26827e 100644
> --- a/drivers/i3c/master/mipi-i3c-hci/hci.h
> +++ b/drivers/i3c/master/mipi-i3c-hci/hci.h
> @@ -175,4 +175,6 @@ int i3c_hci_process_xfer(struct i3c_hci *hci, struct hci_xfer *xfer, int n);
>  int i3c_hci_rpm_suspend(struct device *dev);
>  int i3c_hci_rpm_resume(struct device *dev);
>
> +int i3c_hci_reset_and_restore(struct i3c_hci *hci);
> +
>  #endif
> --
> 2.51.0
>

Re: [PATCH V3 13/16] i3c: mipi-i3c-hci: Wait for NoOp commands to complete

[Frank Li]

On Mon, May 04, 2026 at 02:33:49PM +0300, Adrian Hunter wrote:
> When a transfer list is only partially completed due to an error,
> hci_dma_dequeue_xfer() overwrites the remaining DMA ring entries with
> NoOp commands and restarts the ring to flush them out.
>
> While NoOp commands are expected to complete successfully, they may still
> fail to complete if the DMA ring is stuck.  Explicitly wait for the NoOp
> commands to finish, and trigger controller recovery if they do not
> complete or report an error.
>
> This ensures that partially completed transfer lists are reliably
> resolved and that a stuck ring is recovered promptly.
>
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.com>
>
>
> Changes in V3:
>
> 	None
>
> Changes in V2:
>
> 	Rename completing_xfer to final_xfer
> 	Add missing reinit_completion()
>
>
>  drivers/i3c/master/mipi-i3c-hci/dma.c | 39 ++++++++++++++++++++++-----
>  1 file changed, 33 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/i3c/master/mipi-i3c-hci/dma.c b/drivers/i3c/master/mipi-i3c-hci/dma.c
> index 376062c0fcbf..90fa621c9d56 100644
> --- a/drivers/i3c/master/mipi-i3c-hci/dma.c
> +++ b/drivers/i3c/master/mipi-i3c-hci/dma.c
> @@ -696,11 +696,33 @@ static void hci_dma_recovery(struct i3c_hci *hci)
>  	dev_err(&hci->master.dev, "Recovery %s\n", ret ? "failed!" : "done");
>  }
>
> +static bool hci_dma_wait_for_noop(struct i3c_hci *hci, struct hci_xfer *xfer_list, int n,
> +				  int noop_pos)
> +{
> +	struct completion *done = xfer_list->final_xfer->completion;
> +	bool timeout = !wait_for_completion_timeout(done, HZ);
> +	u32 error = timeout;
> +
> +	for (int i = noop_pos; i < n && !error; i++)
> +		error = RESP_STATUS(xfer_list[i].response);
> +
> +	if (!error)
> +		return true;
> +
> +	if (timeout)
> +		dev_err(&hci->master.dev, "NoOp timeout error\n");
> +	else
> +		dev_err(&hci->master.dev, "NoOp error %u\n", error);
> +
> +	return false;
> +}
> +
>  static bool hci_dma_dequeue_xfer(struct i3c_hci *hci,
>  				 struct hci_xfer *xfer_list, int n)
>  {
>  	struct hci_rings_data *rings = hci->io_data;
>  	struct hci_rh_data *rh = &rings->headers[xfer_list[0].ring_number];
> +	int noop_pos = -1;
>  	unsigned int i;
>  	bool did_unqueue = false;
>  	u32 ring_status;
> @@ -708,7 +730,7 @@ static bool hci_dma_dequeue_xfer(struct i3c_hci *hci,
>  	guard(mutex)(&hci->control_mutex);
>
>  	spin_lock_irq(&hci->lock);
> -
> +restart:
>  	ring_status = rh_reg_read(RING_STATUS);
>  	if (ring_status & RING_STATUS_RUNNING) {
>  		/*
> @@ -765,11 +787,10 @@ static bool hci_dma_dequeue_xfer(struct i3c_hci *hci,
>  				*ring_data++ = 0;
>  			}
>
> -			/* disassociate this xfer struct */
> -			rh->src_xfers[idx] = NULL;
> -
> -			/* and unmap it */
> -			hci_dma_unmap_xfer(hci, xfer, 1);
> +			if (noop_pos < 0) {
> +				reinit_completion(xfer->final_xfer->completion);
> +				noop_pos = i;
> +			}
>
>  			did_unqueue = true;
>  		}
> @@ -801,6 +822,12 @@ static bool hci_dma_dequeue_xfer(struct i3c_hci *hci,
>
>  	wait_for_completion_timeout(&rh->op_done, HZ);
>
> +	if (did_unqueue && !hci_dma_wait_for_noop(hci, xfer_list, n, noop_pos)) {
> +		spin_lock_irq(&hci->lock);
> +		hci->recovery_needed = true;
> +		goto restart;
> +	}
> +
>  	return did_unqueue;
>  }
>
> --
> 2.51.0
>

Re: [PATCH V3 14/16] i3c: mipi-i3c-hci: Base timeouts on actual transfer start time

[Frank Li]

On Mon, May 04, 2026 at 02:33:50PM +0300, Adrian Hunter wrote:
> Transfer timeouts are currently measured from the point where a transfer
> list is queued to the controller.  This can cause transfers to time out
> before they have actually started, if earlier queued transfers consume
> the timeout interval.
>
> Fix this by recording when a transfer reaches the head of the queue and
> adjusting the timeout calculation to start from that point.  The existing
> low-overhead completion-based timeout mechanism is preserved, but care is
> taken to ensure the transfer start time is consistently recorded for both
> PIO and DMA paths.
>
> This prevents premature timeouts while retaining efficient timeout
> handling.
>
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> ---
>
>
> Changes in V3:
>
> 	None
>
> Changes in V2:
> 	Do not flag the next transfer as started when there is an error
> 	which halts the controller
> 	Instead flag it started at the end of hci_dma_dequeue_xfer()
> 	Use hci_start_xfer() in pio.c
>
>
>  drivers/i3c/master/mipi-i3c-hci/core.c | 19 ++++++++++++++++++-
>  drivers/i3c/master/mipi-i3c-hci/dma.c  | 19 ++++++++++++++++++-
>  drivers/i3c/master/mipi-i3c-hci/hci.h  | 11 +++++++++++
>  drivers/i3c/master/mipi-i3c-hci/pio.c  |  1 +
>  4 files changed, 48 insertions(+), 2 deletions(-)
>
...
>
>  #include <linux/io.h>
> +#include <linux/jiffies.h>
>
>  /* 32-bit word aware bit and mask macros */
>  #define W0_MASK(h, l)  GENMASK((h) - 0,  (l) - 0)
> @@ -88,11 +89,13 @@ struct hci_xfer {
>  	u32 cmd_desc[4];
>  	u32 response;
>  	bool rnw;
> +	bool started;
>  	void *data;
>  	unsigned int data_len;
>  	unsigned int cmd_tid;
>  	struct completion *completion;
>  	unsigned long timeout;
> +	unsigned long start_time;

it'd better to add unit for start_time

Frank
>  	union {
>  		struct {
>  			/* PIO specific */
> @@ -123,6 +126,14 @@ static inline void hci_free_xfer(struct hci_xfer *xfer, unsigned int n)
>  	kfree(xfer);
>  }
>
> +static inline void hci_start_xfer(struct hci_xfer *xfer)
> +{
> +	if (!xfer->started) {
> +		xfer->started = true;
> +		xfer->start_time = jiffies;
> +	}
> +}
> +
>  /* This abstracts PIO vs DMA operations */
>  struct hci_io_ops {
>  	bool (*irq_handler)(struct i3c_hci *hci);
> diff --git a/drivers/i3c/master/mipi-i3c-hci/pio.c b/drivers/i3c/master/mipi-i3c-hci/pio.c
> index 8f48a81e65ab..6b8cc5f2b4d2 100644
> --- a/drivers/i3c/master/mipi-i3c-hci/pio.c
> +++ b/drivers/i3c/master/mipi-i3c-hci/pio.c
> @@ -605,6 +605,7 @@ static bool hci_pio_process_cmd(struct i3c_hci *hci, struct hci_pio_data *pio)
>  		 * Finally send the command.
>  		 */
>  		hci_pio_write_cmd(hci, pio->curr_xfer);
> +		hci_start_xfer(pio->curr_xfer);
>  		/*
>  		 * And move on.
>  		 */
> --
> 2.51.0
>

Re: [PATCH V3 15/16] i3c: mipi-i3c-hci: Consolidate DMA ring allocation

[Frank Li]

On Mon, May 04, 2026 at 02:33:51PM +0300, Adrian Hunter wrote:
> dma_alloc_coherent() allocates memory in whole pages, which can waste
> space when command and response queues are allocated separately.
>
> Allocate the DMA command and response queues from a single coherent
> allocation instead, while preserving the required 4-byte alignment.
>
> This reduces memory overhead without changing behavior.
>
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> ---
>
>
> Changes in V3:
>
> 	None
>
> Changes in V2:
>
> 	Check for failed allocation before assignments to avoid doing
> 	arithmetic with NULL pointers
>
>
>  drivers/i3c/master/mipi-i3c-hci/dma.c | 24 +++++++++++-------------
>  1 file changed, 11 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/i3c/master/mipi-i3c-hci/dma.c b/drivers/i3c/master/mipi-i3c-hci/dma.c
> index 6440302c63ca..4029d4d9e784 100644
> --- a/drivers/i3c/master/mipi-i3c-hci/dma.c
> +++ b/drivers/i3c/master/mipi-i3c-hci/dma.c
> @@ -186,14 +186,12 @@ static void hci_dma_free(void *data)
>  	for (int i = 0; i < rings->total; i++) {
>  		rh = &rings->headers[i];
>
> -		if (rh->xfer)
> -			dma_free_coherent(rings->sysdev,
> -					  rh->xfer_struct_sz * rh->xfer_entries,
> -					  rh->xfer, rh->xfer_dma);
> -		if (rh->resp)
> -			dma_free_coherent(rings->sysdev,
> -					  rh->resp_struct_sz * rh->xfer_entries,
> -					  rh->resp, rh->resp_dma);
> +		if (rh->xfer) {
> +			size_t sz = round_up(rh->xfer_struct_sz * rh->xfer_entries, 4);
> +
> +			sz += rh->resp_struct_sz * rh->xfer_entries;
> +			dma_free_coherent(rings->sysdev, sz, rh->xfer, rh->xfer_dma);

Good cleanup. can you save sz into rh when alloc it?

Frank

> +		}
>  		kfree(rh->src_xfers);
>  		if (rh->ibi_status)
>  			dma_free_coherent(rings->sysdev,
> @@ -359,18 +357,18 @@ static int hci_dma_init(struct i3c_hci *hci)
>  		dev_dbg(&hci->master.dev,
>  			"xfer_struct_sz = %d, resp_struct_sz = %d",
>  			rh->xfer_struct_sz, rh->resp_struct_sz);
> -		xfers_sz = rh->xfer_struct_sz * rh->xfer_entries;
> +		xfers_sz = round_up(rh->xfer_struct_sz * rh->xfer_entries, 4);
>  		resps_sz = rh->resp_struct_sz * rh->xfer_entries;
>
> -		rh->xfer = dma_alloc_coherent(rings->sysdev, xfers_sz,
> +		rh->xfer = dma_alloc_coherent(rings->sysdev, xfers_sz + resps_sz,
>  					      &rh->xfer_dma, GFP_KERNEL);
> -		rh->resp = dma_alloc_coherent(rings->sysdev, resps_sz,
> -					      &rh->resp_dma, GFP_KERNEL);
>  		rh->src_xfers =
>  			kzalloc_objs(*rh->src_xfers, rh->xfer_entries);
>  		ret = -ENOMEM;
> -		if (!rh->xfer || !rh->resp || !rh->src_xfers)
> +		if (!rh->xfer || !rh->src_xfers)
>  			goto err_out;
> +		rh->resp = rh->xfer + xfers_sz;
> +		rh->resp_dma = rh->xfer_dma + xfers_sz;
>
>  		/* IBIs */
>
> --
> 2.51.0
>

Re: [PATCH V3 16/16] i3c: mipi-i3c-hci: Increase DMA transfer ring size to maximum

[Frank Li]

On Mon, May 04, 2026 at 02:33:52PM +0300, Adrian Hunter wrote:
> The DMA transfer ring is currently limited to 16 entries, despite the
> MIPI I3C HCI supporting up to 32 devices.  When the ring lacks space for a
> new transfer list, the driver returns -EBUSY, which can be unexpected
> for clients.
>
> Increase the DMA transfer ring size to the maximum supported value of
> 255 entries.  This effectively eliminates ring-space exhaustion in
> practice and avoids the complexity of adding secondary queuing
> mechanisms.
>
> Even at the maximum size, the memory overhead remains small
> (approximately 24 bytes per entry by default).
>
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> ---
Reviewed-by: Frank Li <Frank.Li@nxp.com>
>
>
> Changes in V2 and V3:
>
> 	None
>
>
>  drivers/i3c/master/mipi-i3c-hci/dma.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/i3c/master/mipi-i3c-hci/dma.c b/drivers/i3c/master/mipi-i3c-hci/dma.c
> index 4029d4d9e784..9549d98add4b 100644
> --- a/drivers/i3c/master/mipi-i3c-hci/dma.c
> +++ b/drivers/i3c/master/mipi-i3c-hci/dma.c
> @@ -27,7 +27,7 @@
>   */
>
>  #define XFER_RINGS			1	/* max: 8 */
> -#define XFER_RING_ENTRIES		16	/* max: 255 */
> +#define XFER_RING_ENTRIES		255	/* max: 255 */
>
>  #define IBI_RINGS			1	/* max: 8 */
>  #define IBI_STATUS_RING_ENTRIES		32	/* max: 255 */
> --
> 2.51.0
>