Re: [PATCH 09/17] i3c: mipi-i3c-hci: Process multiple IBIs per interrupt

[Frank Li]

On Mon, Sep 14, 2026 at 02:29:55PM +0300, Adrian Hunter wrote:
> INTR_IBI_READY indicates that one or more IBI Status Descriptors are
> present in the IBI ring.  After clearing interrupt status, the interrupt
> handler processes only a single IBI even though additional IBIs may
> already be queued.
>
> The controller does not reassert INTR_IBI_READY solely because entries
> remain in the ring after interrupt status has been cleared.  Consequently,
> the remaining IBIs are not processed until some later interrupt occurs,
> and can accumulate if IBIs arrive more often than the interrupt handler
> runs.
>
> Process all IBIs that are pending when the handler runs: capture the IBI
> enqueue pointer up front and continue until the dequeue pointer reaches
> that position.
>
> Fixes: 9ad9a52cce28 ("i3c/master: introduce the mipi-i3c-hci driver")
> Signed-off-by: Adrian Hunter <adrian.hunter@xxxxxxxxx>
> ---
>  drivers/i3c/master/mipi-i3c-hci/dma.c | 39 +++++++++++++++++----------
>  1 file changed, 25 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/i3c/master/mipi-i3c-hci/dma.c b/drivers/i3c/master/mipi-i3c-hci/dma.c
> index 5b195978f376..a798b0648922 100644
> --- a/drivers/i3c/master/mipi-i3c-hci/dma.c
> +++ b/drivers/i3c/master/mipi-i3c-hci/dma.c
> @@ -868,25 +868,24 @@ static void hci_dma_recycle_ibi_slot(struct i3c_hci *hci,
>  	i3c_generic_ibi_recycle_slot(dev_ibi->pool, slot);
>  }
>
> -static void hci_dma_process_ibi(struct i3c_hci *hci, struct hci_rh_data *rh)
> +static bool hci_dma_process_ibi(struct i3c_hci *hci, struct hci_rh_data *rh,
> +				u32 *op1_val, unsigned int enq_ptr)
>  {
>  	struct hci_rings_data *rings = hci->io_data;
>  	struct i3c_dev_desc *dev;
>  	struct i3c_hci_dev_data *dev_data;
>  	struct hci_dma_dev_ibi_data *dev_ibi;
>  	struct i3c_ibi_slot *slot;
> -	u32 op1_val, op2_val, ibi_status_error;
> -	unsigned int ptr, enq_ptr, deq_ptr;
> +	u32 ibi_status_error;
> +	unsigned int ptr, deq_ptr;
>  	unsigned int ibi_size, ibi_chunks, ibi_data_offset, first_part;
>  	int ibi_addr, last_ptr;
>  	void *ring_ibi_data;
>  	dma_addr_t ring_ibi_data_dma;
>
> -	op1_val = rh_reg_read(RING_OPERATION1);
> -	deq_ptr = FIELD_GET(RING_OP1_IBI_DEQ_PTR, op1_val);
> -
> -	op2_val = rh_reg_read(RING_OPERATION2);
> -	enq_ptr = FIELD_GET(RING_OP2_IBI_ENQ_PTR, op2_val);
> +	deq_ptr = FIELD_GET(RING_OP1_IBI_DEQ_PTR, *op1_val);
> +	if (deq_ptr == enq_ptr)
> +		return false;
>
>  	ibi_status_error = 0;
>  	ibi_addr = -1;
> @@ -936,7 +935,7 @@ static void hci_dma_process_ibi(struct i3c_hci *hci, struct hci_rh_data *rh)
>  		dev_dbg(&hci->master.dev,
>  			"no LAST_STATUS available (e=%d d=%d)",
>  			enq_ptr, deq_ptr);
> -		return;
> +		return false;
>  	}
>  	deq_ptr = last_ptr + 1;
>  	deq_ptr %= rh->ibi_status_entries;
> @@ -1015,10 +1014,9 @@ static void hci_dma_process_ibi(struct i3c_hci *hci, struct hci_rh_data *rh)
>  	i3c_master_queue_ibi(dev, slot);
>
>  done:
> -	op1_val = rh_reg_read(RING_OPERATION1);
> -	op1_val &= ~RING_OP1_IBI_DEQ_PTR;
> -	op1_val |= FIELD_PREP(RING_OP1_IBI_DEQ_PTR, deq_ptr);
> -	rh_reg_write(RING_OPERATION1, op1_val);
> +	*op1_val &= ~RING_OP1_IBI_DEQ_PTR;
> +	*op1_val |= FIELD_PREP(RING_OP1_IBI_DEQ_PTR, deq_ptr);
> +	rh_reg_write(RING_OPERATION1, *op1_val);
>
>  	/* update the chunk pointer */
>  	rh->ibi_chunk_ptr += ibi_chunks;
> @@ -1026,6 +1024,19 @@ static void hci_dma_process_ibi(struct i3c_hci *hci, struct hci_rh_data *rh)
>
>  	/* and tell the hardware about freed chunks */
>  	rh_reg_write(CHUNK_CONTROL, rh_reg_read(CHUNK_CONTROL) + ibi_chunks);
> +
> +	return true;
> +}
> +
> +static void hci_dma_drain_ibi_ring(struct i3c_hci *hci, struct hci_rh_data *rh)
> +{
> +	u32 op1_val = rh_reg_read(RING_OPERATION1);
> +	u32 op2_val = rh_reg_read(RING_OPERATION2);
> +	unsigned int enq_ptr = FIELD_GET(RING_OP2_IBI_ENQ_PTR, op2_val);
> +
> +	/* Loop is bounded by enq_ptr. Further IBIs will re-assert INTR_IBI_READY */
> +	while (hci_dma_process_ibi(hci, rh, &op1_val, enq_ptr))
> +		;

You just read once RING_OPERATION1, I think you can read in RING_OPERATION1
loop, utils hardware not pending irqs.

And I am not sure if op2_val is always mach op1_val by two 32bit read.
is it possible op1_value changed, when read op2_val?

A new irq status may be trigger later, but it become empty ops at irq
handle.

Frank

>  }
>
>  static bool hci_dma_irq_handler(struct i3c_hci *hci)
> @@ -1047,7 +1058,7 @@ static bool hci_dma_irq_handler(struct i3c_hci *hci)
>  		rh_reg_write(INTR_STATUS, status);
>
>  		if (status & INTR_IBI_READY)
> -			hci_dma_process_ibi(hci, rh);
> +			hci_dma_drain_ibi_ring(hci, rh);
>  		if (status & (INTR_TRANSFER_COMPLETION | INTR_TRANSFER_ERR))
>  			hci_dma_xfer_done(hci, rh);
>  		if (status & INTR_RING_OP)
> --
> 2.53.0
>