[PATCH] i3c: mipi-i3c-hci: fix atomic updates to RING_OPERATION1 register

Billy Tsai posted 1 patch 20 hours ago
drivers/i3c/master/mipi-i3c-hci/dma.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
[PATCH] i3c: mipi-i3c-hci: fix atomic updates to RING_OPERATION1 register
Posted by Billy Tsai 20 hours ago
The RING_OPERATION1 register contains multiple bitfields (enqueue,
software dequeue, and IBI dequeue pointers) that are updated from
different contexts. Because these updates are performed via
read-modify-write sequences, concurrent access from process and IRQ
contexts can lead to lost updates.

Example:
CPU 0 (hci_dma_queue_xfer): reads RING_OPERATION1 (enq=5, deq=2)
CPU 1 (hci_dma_xfer_done): reads RING_OPERATION1 (enq=5, deq=2)
CPU 0: updates enq to 6, writes back (enq=6, deq=2)
CPU 1: updates deq to 3, writes back (enq=5, deq=3) <--Pointer 6 is LOST!

Fix this by wrapping all accesses to RING_OPERATION1 and associated
ring state variables (xfer_space, done_ptr, ibi_chunk_ptr) in the unified
hci->lock. This ensures that the read-modify-write sequence is atomic
across enqueue, dequeue, and completion paths.

Specifically:
- Add hci->lock protection to hci_dma_xfer_done() and
hci_dma_process_ibi().
- Ensure hci_dma_queue_xfer() and hci_dma_dequeue_xfer() use the unified
  lock consistently.
- Ensure the software-maintained ring pointers are synchronized with
  the hardware pointer updates.

Fixes: 9ad9a52cce28 ("i3c/master: introduce the mipi-i3c-hci driver")
Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com>
---
 drivers/i3c/master/mipi-i3c-hci/dma.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/i3c/master/mipi-i3c-hci/dma.c b/drivers/i3c/master/mipi-i3c-hci/dma.c
index e487ef52f6b4..3d967157411e 100644
--- a/drivers/i3c/master/mipi-i3c-hci/dma.c
+++ b/drivers/i3c/master/mipi-i3c-hci/dma.c
@@ -617,10 +617,12 @@ static int hci_dma_handle_error(struct i3c_hci *hci, struct hci_xfer *xfer_list,
 static void hci_dma_xfer_done(struct i3c_hci *hci, struct hci_rh_data *rh)
 {
 	u32 op1_val, op2_val, resp, *ring_resp;
-	unsigned int tid, done_ptr = rh->done_ptr;
+	unsigned int tid, done_ptr;
 	unsigned int done_cnt = 0;
 	struct hci_xfer *xfer;
 
+	spin_lock(&hci->lock);
+	done_ptr = rh->done_ptr;
 	for (;;) {
 		op2_val = rh_reg_read(RING_OPERATION2);
 		if (done_ptr == FIELD_GET(RING_OP2_CR_DEQ_PTR, op2_val))
@@ -659,6 +661,7 @@ static void hci_dma_xfer_done(struct i3c_hci *hci, struct hci_rh_data *rh)
 	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,
@@ -716,6 +719,7 @@ static void hci_dma_process_ibi(struct i3c_hci *hci, struct hci_rh_data *rh)
 	void *ring_ibi_data;
 	dma_addr_t ring_ibi_data_dma;
 
+	spin_lock(&hci->lock);
 	op1_val = rh_reg_read(RING_OPERATION1);
 	deq_ptr = FIELD_GET(RING_OP1_IBI_DEQ_PTR, op1_val);
 
@@ -767,6 +771,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);
+		spin_unlock(&hci->lock);
 		return;
 	}
 	deq_ptr = last_ptr + 1;
@@ -849,6 +854,7 @@ 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);
+	spin_unlock(&hci->lock);
 }
 
 static bool hci_dma_irq_handler(struct i3c_hci *hci)

---
base-commit: f311a05784634febd299f03476b80f3f18489767
change-id: 20260331-i3c-hci-dma-lock-a700a140c845

Best regards,
-- 
Billy Tsai <billy_tsai@aspeedtech.com>
Re: [PATCH] i3c: mipi-i3c-hci: fix atomic updates to RING_OPERATION1 register
Posted by Frank Li 17 hours ago
On Tue, Mar 31, 2026 at 08:12:23PM +0800, Billy Tsai wrote:
> The RING_OPERATION1 register contains multiple bitfields (enqueue,
> software dequeue, and IBI dequeue pointers) that are updated from
> different contexts. Because these updates are performed via
> read-modify-write sequences, concurrent access from process and IRQ
> contexts can lead to lost updates.
>
> Example:
> CPU 0 (hci_dma_queue_xfer): reads RING_OPERATION1 (enq=5, deq=2)
> CPU 1 (hci_dma_xfer_done): reads RING_OPERATION1 (enq=5, deq=2)

Add Adrian Hunter <adrian.hunter@intel.com>, who add lock at equeue.

https://lore.kernel.org/linux-i3c/20260306072451.11131-6-adrian.hunter@intel.com/

Dose above patch fix your problem ?

Frank