[PATCH V2 07/14] i3c: mipi-i3c-hci: Fix race between DMA ring dequeue and interrupt handler
Frank Li
Frank.li at nxp.com
Wed Mar 4 12:05:42 PST 2026
On Wed, Mar 04, 2026 at 08:16:58PM +0200, Adrian Hunter wrote:
> The DMA ring bookkeeping in the MIPI I3C HCI driver is updated from two
> contexts: the DMA ring dequeue path (hci_dma_dequeue_xfer()) and the
> interrupt handler (hci_dma_xfer_done()). Both modify the ring's
> in-flight transfer state - specifically rh->src_xfers[] and
> xfer->ring_entry - but without any serialization. This allows the two
> paths to race, potentially leading to inconsistent ring state.
>
> Serialize access to the shared ring state by extending the existing
> spinlock to cover the DMA dequeue path and the entire interrupt handler.
> Since the core IRQ handler now holds this lock, remove the per-function
> locking from the PIO and DMA sub-handlers.
>
> Additionally, clear the completed entry in rh->src_xfers[] in
> hci_dma_xfer_done() so it cannot be matched or completed again.
>
> Finally, place the ring restart sequence under the same lock in
> hci_dma_dequeue_xfer() to avoid concurrent enqueue or completion
> operations while the ring state is being modified.
>
> 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>
> ---
Reviewed-by: Frank Li <Frank.Li at nxp.com>
>
>
> Changes in V2:
>
> Now uses unified spinlock (as per earlier patch "i3c: mipi-i3c-hci:
> Consolidate spinlocks") and extends coverage to the entire interrupt
> handler
>
>
> drivers/i3c/master/mipi-i3c-hci/core.c | 2 ++
> drivers/i3c/master/mipi-i3c-hci/dma.c | 11 +++++------
> drivers/i3c/master/mipi-i3c-hci/pio.c | 6 +-----
> 3 files changed, 8 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/i3c/master/mipi-i3c-hci/core.c b/drivers/i3c/master/mipi-i3c-hci/core.c
> index 061e84a5c412..adf35b7fa498 100644
> --- a/drivers/i3c/master/mipi-i3c-hci/core.c
> +++ b/drivers/i3c/master/mipi-i3c-hci/core.c
> @@ -567,6 +567,8 @@ static irqreturn_t i3c_hci_irq_handler(int irq, void *dev_id)
> irqreturn_t result = IRQ_NONE;
> u32 val;
>
> + guard(spinlock)(&hci->lock);
> +
> /*
> * The IRQ can be shared, so the handler may be called when the IRQ is
> * due to a different device. That could happen when runtime suspended,
> diff --git a/drivers/i3c/master/mipi-i3c-hci/dma.c b/drivers/i3c/master/mipi-i3c-hci/dma.c
> index f7d411e5e11f..d7840ff69e59 100644
> --- a/drivers/i3c/master/mipi-i3c-hci/dma.c
> +++ b/drivers/i3c/master/mipi-i3c-hci/dma.c
> @@ -560,6 +560,8 @@ static bool hci_dma_dequeue_xfer(struct i3c_hci *hci,
> WARN_ON(1);
> }
>
> + spin_lock_irq(&hci->lock);
> +
> for (i = 0; i < n; i++) {
> struct hci_xfer *xfer = xfer_list + i;
> int idx = xfer->ring_entry;
> @@ -593,6 +595,8 @@ static bool hci_dma_dequeue_xfer(struct i3c_hci *hci,
> /* restart the ring */
> rh_reg_write(RING_CONTROL, RING_CTRL_ENABLE);
>
> + spin_unlock_irq(&hci->lock);
> +
> return did_unqueue;
> }
>
> @@ -618,6 +622,7 @@ static void hci_dma_xfer_done(struct i3c_hci *hci, struct hci_rh_data *rh)
> dev_dbg(&hci->master.dev, "orphaned ring entry");
> } else {
> hci_dma_unmap_xfer(hci, xfer, 1);
> + rh->src_xfers[done_ptr] = NULL;
> xfer->ring_entry = -1;
> xfer->response = resp;
> if (tid != xfer->cmd_tid) {
> @@ -635,14 +640,11 @@ static void hci_dma_xfer_done(struct i3c_hci *hci, struct hci_rh_data *rh)
> done_cnt += 1;
> }
>
> - /* take care to update the software dequeue pointer atomically */
> - spin_lock(&hci->lock);
> rh->xfer_space += done_cnt;
> op1_val = rh_reg_read(RING_OPERATION1);
> op1_val &= ~RING_OP1_CR_SW_DEQ_PTR;
> op1_val |= FIELD_PREP(RING_OP1_CR_SW_DEQ_PTR, done_ptr);
> rh_reg_write(RING_OPERATION1, op1_val);
> - spin_unlock(&hci->lock);
> }
>
> static int hci_dma_request_ibi(struct i3c_hci *hci, struct i3c_dev_desc *dev,
> @@ -822,13 +824,10 @@ static void hci_dma_process_ibi(struct i3c_hci *hci, struct hci_rh_data *rh)
> i3c_master_queue_ibi(dev, slot);
>
> done:
> - /* take care to update the ibi dequeue pointer atomically */
> - spin_lock(&hci->lock);
> 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);
> - spin_unlock(&hci->lock);
>
> /* update the chunk pointer */
> rh->ibi_chunk_ptr += ibi_chunks;
> diff --git a/drivers/i3c/master/mipi-i3c-hci/pio.c b/drivers/i3c/master/mipi-i3c-hci/pio.c
> index 02866c2237fa..8f48a81e65ab 100644
> --- a/drivers/i3c/master/mipi-i3c-hci/pio.c
> +++ b/drivers/i3c/master/mipi-i3c-hci/pio.c
> @@ -1014,15 +1014,12 @@ static bool hci_pio_irq_handler(struct i3c_hci *hci)
> struct hci_pio_data *pio = hci->io_data;
> u32 status;
>
> - spin_lock(&hci->lock);
> status = pio_reg_read(INTR_STATUS);
> dev_dbg(&hci->master.dev, "PIO_INTR_STATUS %#x/%#x",
> status, pio->enabled_irqs);
> status &= pio->enabled_irqs | STAT_LATENCY_WARNINGS;
> - if (!status) {
> - spin_unlock(&hci->lock);
> + if (!status)
> return false;
> - }
>
> if (status & STAT_IBI_STATUS_THLD)
> hci_pio_process_ibi(hci, pio);
> @@ -1056,7 +1053,6 @@ static bool hci_pio_irq_handler(struct i3c_hci *hci)
> pio_reg_write(INTR_SIGNAL_ENABLE, pio->enabled_irqs);
> dev_dbg(&hci->master.dev, "PIO_INTR_STATUS %#x/%#x",
> pio_reg_read(INTR_STATUS), pio_reg_read(INTR_SIGNAL_ENABLE));
> - spin_unlock(&hci->lock);
> return true;
> }
>
> --
> 2.51.0
>
>
> --
> linux-i3c mailing list
> linux-i3c at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-i3c
More information about the linux-i3c
mailing list