i3c: mipi-i3c-hci: Fixes for v7.0

PATCH Applied Adrian Hunter 2 participants 16 messages Mar 6 – Mar 11 I3C

lore

Share & tools

lore

Adrian Hunter

author

2026-03-06 07:25 UTC #0 lore raw

v3 cover i3c: mipi-i3c-hci: Fixes for v7.0

Please note, all patches now have Frank's Reviewed-by.

Here are some fixes to the MIPI I3C HCI driver. Please consider them as fixes for v7.0.

They are mostly related to error recovery and a lack of serialization.

There is a minor conflict with the "i3c: mipi-i3c-hci-pci: Enable IBI while runtime suspended for Intel controllers" patch set. So I will re-send that patch set in in due course.

Changes in V3:

i3c: mipi-i3c-hci: Factor out DMA mapping from queuing path
i3c: mipi-i3c-hci: Consolidate spinlocks
i3c: mipi-i3c-hci: Fix race in DMA ring enqueue for parallel xfers
i3c: mipi-i3c-hci: Fix race in DMA ring dequeue
i3c: mipi-i3c-hci: Fix race between DMA ring dequeue and interrupt handler
i3c: mipi-i3c-hci: Fix race in DMA error handling in interrupt context
i3c: mipi-i3c-hci: Fix handling of shared IRQs during early initialization
Add Frank's Rev'd-by

Changes in V2:

i3c: mipi-i3c-hci: Use ETIMEDOUT instead of ETIME for timeout errors
Added Frank's Rev'd-by

i3c: mipi-i3c-hci: Fix Hot-Join NACK
Added Frank's Rev'd-by

i3c: mipi-i3c-hci: Factor out DMA mapping from queuing path
New patch split out from "i3c: mipi-i3c-hci: Fix race in DMA ring enqueue for parallel xfers"

i3c: mipi-i3c-hci: Consolidate spinlocks
New patch

i3c: mipi-i3c-hci: Fix race in DMA ring enqueue for parallel xfers
Refactor of hci_dma_queue_xfer() to do all DMA mapping first, is now a separate earlier patch: "i3c: mipi-i3c-hci: Factor out DMA mapping from queuing path"

i3c: mipi-i3c-hci: Fix race in DMA ring dequeue
Mutex now defined in struct i3c_hci instead of struct hci_rh_data

i3c: mipi-i3c-hci: Fix race between DMA ring dequeue and interrupt handler
Now uses unified spinlock (as per earlier patch "i3c: mipi-i3c-hci:
Consolidate spinlocks") and extends coverage to the entire interrupt handler

i3c: mipi-i3c-hci: Correct RING_CTRL_ABORT handling in DMA dequeue
Add Frank's Rev'd-by
Mutex is now defined in struct i3c_hci

i3c: mipi-i3c-hci: Add missing TID field to no-op command descriptor
Add Frank's Rev'd-by

i3c: mipi-i3c-hci: Restart DMA ring correctly after dequeue abort
Add Frank's Rev'd-by

i3c: mipi-i3c-hci: Consolidate common xfer processing logic
Add Frank's Rev'd-by

i3c: mipi-i3c-hci: Fix race in DMA error handling in interrupt context
None

i3c: mipi-i3c-hci: Fix handling of shared IRQs during early initialization
Place irq_inactive under the protection of the now unified spinlock, eliminating the need for explict memory barriers. Correctly position the update of irq_inactive in i3c_hci_sync_irq_inactive() to after synchronize_irq()

i3c: mipi-i3c-hci: Fallback to software reset when bus disable fails
Add Frank's Rev'd-by

Adrian Hunter (14):
i3c: mipi-i3c-hci: Use ETIMEDOUT instead of ETIME for timeout errors
i3c: mipi-i3c-hci: Fix Hot-Join NACK
i3c: mipi-i3c-hci: Factor out DMA mapping from queuing path
i3c: mipi-i3c-hci: Consolidate spinlocks
i3c: mipi-i3c-hci: Fix race in DMA ring enqueue for parallel xfers
i3c: mipi-i3c-hci: Fix race in DMA ring dequeue
i3c: mipi-i3c-hci: Fix race between DMA ring dequeue and interrupt handler
i3c: mipi-i3c-hci: Correct RING_CTRL_ABORT handling in DMA dequeue
i3c: mipi-i3c-hci: Add missing TID field to no-op command descriptor
i3c: mipi-i3c-hci: Restart DMA ring correctly after dequeue abort
i3c: mipi-i3c-hci: Consolidate common xfer processing logic
i3c: mipi-i3c-hci: Fix race in DMA error handling in interrupt context
i3c: mipi-i3c-hci: Fix handling of shared IRQs during early initialization
i3c: mipi-i3c-hci: Fallback to software reset when bus disable fails

 drivers/i3c/master/mipi-i3c-hci/cmd.h    |   1 +
 drivers/i3c/master/mipi-i3c-hci/cmd_v1.c |   8 +-
 drivers/i3c/master/mipi-i3c-hci/cmd_v2.c |   8 +-
 drivers/i3c/master/mipi-i3c-hci/core.c   | 143 +++++++++++++++++-----------
 drivers/i3c/master/mipi-i3c-hci/dma.c    | 156 ++++++++++++++++---------------
 drivers/i3c/master/mipi-i3c-hci/hci.h    |   5 +
 drivers/i3c/master/mipi-i3c-hci/pio.c    |  16 +---
 7 files changed, 190 insertions(+), 147 deletions(-)

Regards
Adrian

Signature · 3 lines

linux-i3c mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-i3c

Adrian Hunter

author

2026-03-06 07:25 UTC #1 lore raw

PATCH v3 1/14 i3c: mipi-i3c-hci: Use ETIMEDOUT instead of ETIME for timeout errors

The MIPI I3C HCI driver currently returns -ETIME for various timeout conditions, while other I3C master drivers consistently use -ETIMEDOUT for the same class of errors. Align the HCI driver with the rest of the subsystem by replacing all uses of -ETIME with -ETIMEDOUT.

Fixes 9ad9a52cce282 ("i3c/master: introduce the mipi-i3c-hci driver")

Cc [email protected]

Signed-off-by Adrian Hunter <[email protected]>

Reviewed-by Frank Li <[email protected]>

---

Changes in V3:

None

Changes in V2:

Added Frank's Rev'd-by

 drivers/i3c/master/mipi-i3c-hci/cmd_v1.c | 2 +-
 drivers/i3c/master/mipi-i3c-hci/cmd_v2.c | 2 +-
 drivers/i3c/master/mipi-i3c-hci/core.c   | 6 +++---
 3 files changed, 5 insertions(+), 5 deletions(-)

drivers/i3c/master/mipi-i3c-hci/cmd_v1.c +1 -1

index fe260461e7e6..831a261f6c56 100644
@@ -334,7 +334,7 @@ static int hci_cmd_v1_daa(struct i3c_hci *hci)
 		hci->io->queue_xfer(hci, xfer, 1);
 		if (!wait_for_completion_timeout(&done, HZ) &&
 		    hci->io->dequeue_xfer(hci, xfer, 1)) {
-			ret = -ETIME;
+			ret = -ETIMEDOUT;
 			break;
 		}
 		if ((RESP_STATUS(xfer->response) == RESP_ERR_ADDR_HEADER ||

drivers/i3c/master/mipi-i3c-hci/cmd_v2.c +1 -1

index 3729e6419581..054beee36da5 100644
@@ -275,7 +275,7 @@ static int hci_cmd_v2_daa(struct i3c_hci *hci)
 		hci->io->queue_xfer(hci, xfer, 2);
 		if (!wait_for_completion_timeout(&done, HZ) &&
 		    hci->io->dequeue_xfer(hci, xfer, 2)) {
-			ret = -ETIME;
+			ret = -ETIMEDOUT;
 			break;
 		}
 		if (RESP_STATUS(xfer[0].response) != RESP_SUCCESS) {

drivers/i3c/master/mipi-i3c-hci/core.c +3 -3

index 5879bba78164..dbe93df0c70e 100644
@@ -261,7 +261,7 @@ static int i3c_hci_send_ccc_cmd(struct i3c_master_controller *m,
 		goto out;
 	if (!wait_for_completion_timeout(&done, HZ) &&
 	    hci->io->dequeue_xfer(hci, xfer, nxfers)) {
-		ret = -ETIME;
+		ret = -ETIMEDOUT;
 		goto out;
 	}
 	for (i = prefixed; i < nxfers; i++) {

@@ -340,7 +340,7 @@ static int i3c_hci_i3c_xfers(struct i3c_dev_desc *dev,
 		goto out;
 	if (!wait_for_completion_timeout(&done, HZ) &&
 	    hci->io->dequeue_xfer(hci, xfer, nxfers)) {
-		ret = -ETIME;
+		ret = -ETIMEDOUT;
 		goto out;
 	}
 	for (i = 0; i < nxfers; i++) {

@@ -388,7 +388,7 @@ static int i3c_hci_i2c_xfers(struct i2c_dev_desc *dev,
 		goto out;
 	if (!wait_for_completion_timeout(&done, m->i2c.timeout) &&
 	    hci->io->dequeue_xfer(hci, xfer, nxfers)) {
-		ret = -ETIME;
+		ret = -ETIMEDOUT;
 		goto out;
 	}
 	for (i = 0; i < nxfers; i++) {

Signature · 7 lines

2.51.0


-- 
linux-i3c mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-i3c

Adrian Hunter

author

2026-03-06 07:25 UTC #2 lore raw

PATCH v3 2/14 i3c: mipi-i3c-hci: Fix Hot-Join NACK

In reply to Adrian Hunter

The MIPI I3C HCI host controller driver does not implement Hot-Join handling, yet Hot-Join response control defaults to allowing devices to Hot-Join the bus. Configure HC_CONTROL_HOT_JOIN_CTRL to NACK all Hot-Join attempts.

Fixes 9ad9a52cce282 ("i3c/master: introduce the mipi-i3c-hci driver")

Cc [email protected]

Signed-off-by Adrian Hunter <[email protected]>

Reviewed-by Frank Li <[email protected]>

---

Changes in V3:

None

Changes in V2:

Added Frank's Rev'd-by

 drivers/i3c/master/mipi-i3c-hci/core.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

drivers/i3c/master/mipi-i3c-hci/core.c +4 -2

index dbe93df0c70e..4877a321edf9 100644
@@ -152,7 +152,8 @@ static int i3c_hci_bus_init(struct i3c_master_controller *m)
 	if (hci->quirks & HCI_QUIRK_RESP_BUF_THLD)
 		amd_set_resp_buf_thld(hci);
 
-	reg_set(HC_CONTROL, HC_CONTROL_BUS_ENABLE);
+	/* Enable bus with Hot-Join disabled */
+	reg_set(HC_CONTROL, HC_CONTROL_BUS_ENABLE | HC_CONTROL_HOT_JOIN_CTRL);
 	dev_dbg(&hci->master.dev, "HC_CONTROL = %#x", reg_read(HC_CONTROL));
 
 	return 0;

@@ -764,7 +765,8 @@ static int i3c_hci_runtime_resume(struct device *dev)
 
 	hci->io->resume(hci);
 
-	reg_set(HC_CONTROL, HC_CONTROL_BUS_ENABLE);
+	/* Enable bus with Hot-Join disabled */
+	reg_set(HC_CONTROL, HC_CONTROL_BUS_ENABLE | HC_CONTROL_HOT_JOIN_CTRL);
 
 	return 0;
 }

Signature · 7 lines

2.51.0


-- 
linux-i3c mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-i3c

Adrian Hunter

author

2026-03-06 07:25 UTC #3 lore raw

PATCH v3 3/14 i3c: mipi-i3c-hci: Factor out DMA mapping from queuing path

In reply to Adrian Hunter

Prepare for fixing a race in the DMA ring enqueue path when handling parallel transfers. Move all DMA mapping out of hci_dma_queue_xfer() and into a new helper that performs the mapping up front.

This refactoring allows the upcoming fix to extend the spinlock coverage around the enqueue operation without performing DMA mapping under the spinlock.

No functional change is intended in this patch.

Fixes 9ad9a52cce282 ("i3c/master: introduce the mipi-i3c-hci driver")

Cc [email protected]

Signed-off-by Adrian Hunter <[email protected]>

Reviewed-by Frank Li <[email protected]>

---

Changes in V3:

Add Frank's Rev'd-by

Changes in V2:

New patch split out from "i3c: mipi-i3c-hci: Fix race in DMA ring enqueue for parallel xfers"

 drivers/i3c/master/mipi-i3c-hci/dma.c | 49 ++++++++++++++++++---------
 1 file changed, 33 insertions(+), 16 deletions(-)

drivers/i3c/master/mipi-i3c-hci/dma.c +33 ~~-16~~

index b903a2da1fd1..ba451f026386 100644
@@ -439,6 +439,33 @@ static void hci_dma_unmap_xfer(struct i3c_hci *hci,
 	}
 }
 
+static struct i3c_dma *hci_dma_map_xfer(struct device *dev, struct hci_xfer *xfer)
+{
+	enum dma_data_direction dir = xfer->rnw ? DMA_FROM_DEVICE : DMA_TO_DEVICE;
+	bool need_bounce = device_iommu_mapped(dev) && xfer->rnw && (xfer->data_len & 3);
+
+	return i3c_master_dma_map_single(dev, xfer->data, xfer->data_len, need_bounce, dir);
+}
+
+static int hci_dma_map_xfer_list(struct i3c_hci *hci, struct device *dev,
+				 struct hci_xfer *xfer_list, int n)
+{
+	for (int i = 0; i < n; i++) {
+		struct hci_xfer *xfer = xfer_list + i;
+
+		if (!xfer->data)
+			continue;
+
+		xfer->dma = hci_dma_map_xfer(dev, xfer);
+		if (!xfer->dma) {
+			hci_dma_unmap_xfer(hci, xfer_list, i);
+			return -ENOMEM;
+		}
+	}
+
+	return 0;
+}
+
 static int hci_dma_queue_xfer(struct i3c_hci *hci,
 			      struct hci_xfer *xfer_list, int n)
 {

@@ -446,6 +473,11 @@ static int hci_dma_queue_xfer(struct i3c_hci *hci,
 	struct hci_rh_data *rh;
 	unsigned int i, ring, enqueue_ptr;
 	u32 op1_val, op2_val;
+	int ret;
+
+	ret = hci_dma_map_xfer_list(hci, rings->sysdev, xfer_list, n);
+	if (ret)
+		return ret;
 
 	/* For now we only use ring 0 */
 	ring = 0;

@@ -456,9 +488,6 @@ static int hci_dma_queue_xfer(struct i3c_hci *hci,
 	for (i = 0; i < n; i++) {
 		struct hci_xfer *xfer = xfer_list + i;
 		u32 *ring_data = rh->xfer + rh->xfer_struct_sz * enqueue_ptr;
-		enum dma_data_direction dir = xfer->rnw ? DMA_FROM_DEVICE :
-							  DMA_TO_DEVICE;
-		bool need_bounce;
 
 		/* store cmd descriptor */
 		*ring_data++ = xfer->cmd_desc[0];

@@ -477,18 +506,6 @@ static int hci_dma_queue_xfer(struct i3c_hci *hci,
 
 		/* 2nd and 3rd words of Data Buffer Descriptor Structure */
 		if (xfer->data) {
-			need_bounce = device_iommu_mapped(rings->sysdev) &&
-				      xfer->rnw &&
-				      xfer->data_len != ALIGN(xfer->data_len, 4);
-			xfer->dma = i3c_master_dma_map_single(rings->sysdev,
-							      xfer->data,
-							      xfer->data_len,
-							      need_bounce,
-							      dir);
-			if (!xfer->dma) {
-				hci_dma_unmap_xfer(hci, xfer_list, i);
-				return -ENOMEM;
-			}
 			*ring_data++ = lower_32_bits(xfer->dma->addr);
 			*ring_data++ = upper_32_bits(xfer->dma->addr);
 		} else {

@@ -511,7 +528,7 @@ static int hci_dma_queue_xfer(struct i3c_hci *hci,
 		op2_val = rh_reg_read(RING_OPERATION2);
 		if (enqueue_ptr == FIELD_GET(RING_OP2_CR_DEQ_PTR, op2_val)) {
 			/* the ring is full */
-			hci_dma_unmap_xfer(hci, xfer_list, i + 1);
+			hci_dma_unmap_xfer(hci, xfer_list, n);
 			return -EBUSY;
 		}
 	}

Signature · 7 lines

2.51.0


-- 
linux-i3c mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-i3c

Adrian Hunter

author

2026-03-06 07:25 UTC #4 lore raw

PATCH v3 4/14 i3c: mipi-i3c-hci: Consolidate spinlocks

In reply to Adrian Hunter

The MIPI I3C HCI driver currently uses separate spinlocks for different contexts (PIO vs. DMA rings). This split is unnecessary and complicates upcoming fixes. The driver does not support concurrent PIO and DMA operation, and it only supports a single DMA ring, so a single lock is sufficient for all paths.

Introduce a unified spinlock in struct i3c_hci, switch both PIO and DMA code to use it, and remove the per-context locks.

No functional change is intended in this patch.

Fixes 9ad9a52cce282 ("i3c/master: introduce the mipi-i3c-hci driver")

Cc [email protected]

Signed-off-by Adrian Hunter <[email protected]>

Reviewed-by Frank Li <[email protected]>

---

Changes in V3:

Add Frank's Rev'd-by

Changes in V2:

New patch

 drivers/i3c/master/mipi-i3c-hci/core.c |  2 ++
 drivers/i3c/master/mipi-i3c-hci/dma.c  | 14 ++++++--------
 drivers/i3c/master/mipi-i3c-hci/hci.h  |  1 +
 drivers/i3c/master/mipi-i3c-hci/pio.c  | 16 +++++++---------
 4 files changed, 16 insertions(+), 17 deletions(-)

drivers/i3c/master/mipi-i3c-hci/core.c +2 -0

index 4877a321edf9..faf5eae2409f 100644
@@ -926,6 +926,8 @@ static int i3c_hci_probe(struct platform_device *pdev)
 	if (!hci)
 		return -ENOMEM;
 
+	spin_lock_init(&hci->lock);
+
 	/*
 	 * Multi-bus instances share the same MMIO address range, but not
 	 * necessarily in separate contiguous sub-ranges. To avoid overlapping

drivers/i3c/master/mipi-i3c-hci/dma.c +6 -8

index ba451f026386..2442cedd5c2a 100644
@@ -131,7 +131,6 @@ struct hci_rh_data {
 	unsigned int xfer_struct_sz, resp_struct_sz, ibi_status_sz, ibi_chunk_sz;
 	unsigned int done_ptr, ibi_chunk_ptr;
 	struct hci_xfer **src_xfers;
-	spinlock_t lock;
 	struct completion op_done;
 };

@@ -344,7 +343,6 @@ static int hci_dma_init(struct i3c_hci *hci)
 			goto err_out;
 		rh = &rings->headers[i];
 		rh->regs = hci->base_regs + offset;
-		spin_lock_init(&rh->lock);
 		init_completion(&rh->op_done);
 
 		rh->xfer_entries = XFER_RING_ENTRIES;

@@ -534,12 +532,12 @@ static int hci_dma_queue_xfer(struct i3c_hci *hci,
 	}
 
 	/* take care to update the hardware enqueue pointer atomically */
-	spin_lock_irq(&rh->lock);
+	spin_lock_irq(&hci->lock);
 	op1_val = rh_reg_read(RING_OPERATION1);
 	op1_val &= ~RING_OP1_CR_ENQ_PTR;
 	op1_val |= FIELD_PREP(RING_OP1_CR_ENQ_PTR, enqueue_ptr);
 	rh_reg_write(RING_OPERATION1, op1_val);
-	spin_unlock_irq(&rh->lock);
+	spin_unlock_irq(&hci->lock);
 
 	return 0;
 }

@@ -637,12 +635,12 @@ static void hci_dma_xfer_done(struct i3c_hci *hci, struct hci_rh_data *rh)
 	}
 
 	/* take care to update the software dequeue pointer atomically */
-	spin_lock(&rh->lock);
+	spin_lock(&hci->lock);
 	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(&rh->lock);
+	spin_unlock(&hci->lock);
 }
 
 static int hci_dma_request_ibi(struct i3c_hci *hci, struct i3c_dev_desc *dev,

@@ -823,12 +821,12 @@ static void hci_dma_process_ibi(struct i3c_hci *hci, struct hci_rh_data *rh)
 
 done:
 	/* take care to update the ibi dequeue pointer atomically */
-	spin_lock(&rh->lock);
+	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(&rh->lock);
+	spin_unlock(&hci->lock);
 
 	/* update the chunk pointer */
 	rh->ibi_chunk_ptr += ibi_chunks;

drivers/i3c/master/mipi-i3c-hci/hci.h +1 -0

index 337b7ab1cb06..f1dd502c071f 100644
@@ -50,6 +50,7 @@ struct i3c_hci {
 	const struct hci_io_ops *io;
 	void *io_data;
 	const struct hci_cmd_ops *cmd;
+	spinlock_t lock;
 	atomic_t next_cmd_tid;
 	bool irq_inactive;
 	u32 caps;

drivers/i3c/master/mipi-i3c-hci/pio.c +7 -9

index f8825ac81408..02866c2237fa 100644
@@ -123,7 +123,6 @@ struct hci_pio_ibi_data {
 };
 
 struct hci_pio_data {
-	spinlock_t lock;
 	struct hci_xfer *curr_xfer, *xfer_queue;
 	struct hci_xfer *curr_rx, *rx_queue;
 	struct hci_xfer *curr_tx, *tx_queue;

@@ -212,7 +211,6 @@ static int hci_pio_init(struct i3c_hci *hci)
 		return -ENOMEM;
 
 	hci->io_data = pio;
-	spin_lock_init(&pio->lock);
 
 	__hci_pio_init(hci, &size_val);

@@ -631,7 +629,7 @@ static int hci_pio_queue_xfer(struct i3c_hci *hci, struct hci_xfer *xfer, int n)
 		xfer[i].data_left = xfer[i].data_len;
 	}
 
-	spin_lock_irq(&pio->lock);
+	spin_lock_irq(&hci->lock);
 	prev_queue_tail = pio->xfer_queue;
 	pio->xfer_queue = &xfer[n - 1];
 	if (pio->curr_xfer) {

@@ -645,7 +643,7 @@ static int hci_pio_queue_xfer(struct i3c_hci *hci, struct hci_xfer *xfer, int n)
 			pio_reg_read(INTR_STATUS),
 			pio_reg_read(INTR_SIGNAL_ENABLE));
 	}
-	spin_unlock_irq(&pio->lock);
+	spin_unlock_irq(&hci->lock);
 	return 0;
 }

@@ -716,14 +714,14 @@ static bool hci_pio_dequeue_xfer(struct i3c_hci *hci, struct hci_xfer *xfer, int
 	struct hci_pio_data *pio = hci->io_data;
 	int ret;
 
-	spin_lock_irq(&pio->lock);
+	spin_lock_irq(&hci->lock);
 	dev_dbg(&hci->master.dev, "n=%d status=%#x/%#x", n,
 		pio_reg_read(INTR_STATUS), pio_reg_read(INTR_SIGNAL_ENABLE));
 	dev_dbg(&hci->master.dev, "main_status = %#x/%#x",
 		readl(hci->base_regs + 0x20), readl(hci->base_regs + 0x28));
 
 	ret = hci_pio_dequeue_xfer_common(hci, pio, xfer, n);
-	spin_unlock_irq(&pio->lock);
+	spin_unlock_irq(&hci->lock);
 	return ret;
 }

@@ -1016,13 +1014,13 @@ static bool hci_pio_irq_handler(struct i3c_hci *hci)
 	struct hci_pio_data *pio = hci->io_data;
 	u32 status;
 
-	spin_lock(&pio->lock);
+	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(&pio->lock);
+		spin_unlock(&hci->lock);
 		return false;
 	}

@@ -1058,7 +1056,7 @@ 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(&pio->lock);
+	spin_unlock(&hci->lock);
 	return true;
 }

Signature · 7 lines

2.51.0


-- 
linux-i3c mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-i3c

Adrian Hunter

author

2026-03-06 07:25 UTC #5 lore raw

PATCH v3 5/14 i3c: mipi-i3c-hci: Fix race in DMA ring enqueue for parallel xfers

In reply to Adrian Hunter

The I3C subsystem allows multiple transfers to be queued concurrently. However, the MIPI I3C HCI driver's DMA enqueue path, hci_dma_queue_xfer(), lacks sufficient serialization.

In particular, the allocation of the enqueue_ptr and its subsequent update in the RING_OPERATION1 register, must be done atomically. Otherwise, for example, it would be possible for 2 transfers to be allocated the same enqueue_ptr.

Extend the use of the existing spinlock for that purpose. Keep a count of the number of xfers enqueued so that it is easy to determine if the ring has enough space.

Fixes 9ad9a52cce282 ("i3c/master: introduce the mipi-i3c-hci driver")

Cc [email protected]

Signed-off-by Adrian Hunter <[email protected]>

Reviewed-by Frank Li <[email protected]>

---

Changes in V3:

Add Frank's Rev'd-by

Changes in V2:

Refactor of hci_dma_queue_xfer() to do all DMA mapping first, is now a separate earlier patch: "i3c: mipi-i3c-hci: Factor out DMA mapping from queuing path"

 drivers/i3c/master/mipi-i3c-hci/dma.c | 32 +++++++++++++--------------
 1 file changed, 16 insertions(+), 16 deletions(-)

drivers/i3c/master/mipi-i3c-hci/dma.c +16 ~~-16~~

index 2442cedd5c2a..74b255ad6d0f 100644
@@ -129,7 +129,7 @@ struct hci_rh_data {
 	dma_addr_t xfer_dma, resp_dma, ibi_status_dma, ibi_data_dma;
 	unsigned int xfer_entries, ibi_status_entries, ibi_chunks_total;
 	unsigned int xfer_struct_sz, resp_struct_sz, ibi_status_sz, ibi_chunk_sz;
-	unsigned int done_ptr, ibi_chunk_ptr;
+	unsigned int done_ptr, ibi_chunk_ptr, xfer_space;
 	struct hci_xfer **src_xfers;
 	struct completion op_done;
 };

@@ -260,6 +260,7 @@ static void hci_dma_init_rh(struct i3c_hci *hci, struct hci_rh_data *rh, int i)
 
 	rh->done_ptr = 0;
 	rh->ibi_chunk_ptr = 0;
+	rh->xfer_space = rh->xfer_entries;
 }
 
 static void hci_dma_init_rings(struct i3c_hci *hci)

@@ -470,7 +471,7 @@ static int hci_dma_queue_xfer(struct i3c_hci *hci,
 	struct hci_rings_data *rings = hci->io_data;
 	struct hci_rh_data *rh;
 	unsigned int i, ring, enqueue_ptr;
-	u32 op1_val, op2_val;
+	u32 op1_val;
 	int ret;
 
 	ret = hci_dma_map_xfer_list(hci, rings->sysdev, xfer_list, n);

@@ -481,6 +482,14 @@ static int hci_dma_queue_xfer(struct i3c_hci *hci,
 	ring = 0;
 	rh = &rings->headers[ring];
 
+	spin_lock_irq(&hci->lock);
+
+	if (n > rh->xfer_space) {
+		spin_unlock_irq(&hci->lock);
+		hci_dma_unmap_xfer(hci, xfer_list, n);
+		return -EBUSY;
+	}
+
 	op1_val = rh_reg_read(RING_OPERATION1);
 	enqueue_ptr = FIELD_GET(RING_OP1_CR_ENQ_PTR, op1_val);
 	for (i = 0; i < n; i++) {

@@ -518,22 +527,10 @@ static int hci_dma_queue_xfer(struct i3c_hci *hci,
 		xfer->ring_entry = enqueue_ptr;
 
 		enqueue_ptr = (enqueue_ptr + 1) % rh->xfer_entries;
-
-		/*
-		 * We may update the hardware view of the enqueue pointer
-		 * only if we didn't reach its dequeue pointer.
-		 */
-		op2_val = rh_reg_read(RING_OPERATION2);
-		if (enqueue_ptr == FIELD_GET(RING_OP2_CR_DEQ_PTR, op2_val)) {
-			/* the ring is full */
-			hci_dma_unmap_xfer(hci, xfer_list, n);
-			return -EBUSY;
-		}
 	}
 
-	/* take care to update the hardware enqueue pointer atomically */
-	spin_lock_irq(&hci->lock);
-	op1_val = rh_reg_read(RING_OPERATION1);
+	rh->xfer_space -= n;
+
 	op1_val &= ~RING_OP1_CR_ENQ_PTR;
 	op1_val |= FIELD_PREP(RING_OP1_CR_ENQ_PTR, enqueue_ptr);
 	rh_reg_write(RING_OPERATION1, op1_val);

@@ -601,6 +598,7 @@ 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 done_cnt = 0;
 	struct hci_xfer *xfer;
 
 	for (;;) {

@@ -632,10 +630,12 @@ static void hci_dma_xfer_done(struct i3c_hci *hci, struct hci_rh_data *rh)
 
 		done_ptr = (done_ptr + 1) % rh->xfer_entries;
 		rh->done_ptr = done_ptr;
+		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);

Signature · 7 lines

2.51.0


-- 
linux-i3c mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-i3c

Adrian Hunter

author

2026-03-06 07:25 UTC #6 lore raw

PATCH v3 6/14 i3c: mipi-i3c-hci: Fix race in DMA ring dequeue

In reply to Adrian Hunter

The HCI DMA dequeue path (hci_dma_dequeue_xfer()) may be invoked for multiple transfers that timeout around the same time. However, the function is not serialized and can race with itself.

When a timeout occurs, hci_dma_dequeue_xfer() stops the ring, processes incomplete transfers, and then restarts the ring. If another timeout triggers a parallel call into the same function, the two instances may interfere with each other - stopping or restarting the ring at unexpected times.

Add a mutex so that hci_dma_dequeue_xfer() is serialized with respect to itself.

Fixes 9ad9a52cce282 ("i3c/master: introduce the mipi-i3c-hci driver")

Cc [email protected]

Signed-off-by Adrian Hunter <[email protected]>

Reviewed-by Frank Li <[email protected]>

---

Changes in V3:

Add Frank's Rev'd-by

Changes in V2:

Mutex now defined in struct i3c_hci instead of struct hci_rh_data

 drivers/i3c/master/mipi-i3c-hci/core.c | 1 +
 drivers/i3c/master/mipi-i3c-hci/dma.c  | 2 ++
 drivers/i3c/master/mipi-i3c-hci/hci.h  | 1 +
 3 files changed, 4 insertions(+)

drivers/i3c/master/mipi-i3c-hci/core.c +1 -0

index faf5eae2409f..061e84a5c412 100644
@@ -927,6 +927,7 @@ static int i3c_hci_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	spin_lock_init(&hci->lock);
+	mutex_init(&hci->control_mutex);
 
 	/*
 	 * Multi-bus instances share the same MMIO address range, but not

drivers/i3c/master/mipi-i3c-hci/dma.c +2 -0

index 74b255ad6d0f..f7d411e5e11f 100644
@@ -547,6 +547,8 @@ static bool hci_dma_dequeue_xfer(struct i3c_hci *hci,
 	unsigned int i;
 	bool did_unqueue = false;
 
+	guard(mutex)(&hci->control_mutex);
+
 	/* stop the ring */
 	rh_reg_write(RING_CONTROL, RING_CTRL_ABORT);
 	if (wait_for_completion_timeout(&rh->op_done, HZ) == 0) {

drivers/i3c/master/mipi-i3c-hci/hci.h +1 -0

index f1dd502c071f..9c63d80f7fc4 100644
@@ -51,6 +51,7 @@ struct i3c_hci {
 	void *io_data;
 	const struct hci_cmd_ops *cmd;
 	spinlock_t lock;
+	struct mutex control_mutex;
 	atomic_t next_cmd_tid;
 	bool irq_inactive;
 	u32 caps;

Signature · 7 lines

2.51.0


-- 
linux-i3c mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-i3c

Adrian Hunter

author

2026-03-06 07:25 UTC #7 lore raw

PATCH v3 7/14 i3c: mipi-i3c-hci: Fix race between DMA ring dequeue and interrupt handler

In reply to Adrian Hunter

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 [email protected]

Signed-off-by Adrian Hunter <[email protected]>

Reviewed-by Frank Li <[email protected]>

---

Changes in V3:

Add Frank's Rev'd-by

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

drivers/i3c/master/mipi-i3c-hci/core.c +2 -0

index 061e84a5c412..adf35b7fa498 100644
@@ -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,

drivers/i3c/master/mipi-i3c-hci/dma.c +5 -6

index f7d411e5e11f..d7840ff69e59 100644
@@ -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;

drivers/i3c/master/mipi-i3c-hci/pio.c +1 -5

index 02866c2237fa..8f48a81e65ab 100644
@@ -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;
 }

Signature · 7 lines

2.51.0


-- 
linux-i3c mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-i3c

Adrian Hunter

author

2026-03-06 07:25 UTC #8 lore raw

PATCH v3 8/14 i3c: mipi-i3c-hci: Correct RING_CTRL_ABORT handling in DMA dequeue

In reply to Adrian Hunter

The logic used to abort the DMA ring contains several flaws:

1. The driver unconditionally issues a ring abort even when the ring has already stopped.
2. The completion used to wait for abort completion is never re-initialized, resulting in incorrect wait behavior.
3. The abort sequence unintentionally clears RING_CTRL_ENABLE, which resets hardware ring pointers and disrupts the controller state.
4. If the ring is already stopped, the abort operation should be considered successful without attempting further action.

Fix the abort handling by checking whether the ring is running before issuing an abort, re-initializing the completion when needed, ensuring that RING_CTRL_ENABLE remains asserted during abort, and treating an already stopped ring as a successful condition.

Fixes 9ad9a52cce282 ("i3c/master: introduce the mipi-i3c-hci driver")

Cc [email protected]

Signed-off-by Adrian Hunter <[email protected]>

Reviewed-by Frank Li <[email protected]>

---

Changes in V3:

None

Changes in V2:

Add Frank's Rev'd-by
Mutex is now defined in struct i3c_hci

 drivers/i3c/master/mipi-i3c-hci/dma.c | 25 ++++++++++++++++---------
 1 file changed, 16 insertions(+), 9 deletions(-)

drivers/i3c/master/mipi-i3c-hci/dma.c +16 -9

index d7840ff69e59..a3e8e01a35c9 100644
@@ -546,18 +546,25 @@ static bool hci_dma_dequeue_xfer(struct i3c_hci *hci,
 	struct hci_rh_data *rh = &rings->headers[xfer_list[0].ring_number];
 	unsigned int i;
 	bool did_unqueue = false;
+	u32 ring_status;
 
 	guard(mutex)(&hci->control_mutex);
 
-	/* stop the ring */
-	rh_reg_write(RING_CONTROL, RING_CTRL_ABORT);
-	if (wait_for_completion_timeout(&rh->op_done, HZ) == 0) {
-		/*
-		 * We're deep in it if ever this condition is ever met.
-		 * Hardware might still be writing to memory, etc.
-		 */
-		dev_crit(&hci->master.dev, "unable to abort the ring\n");
-		WARN_ON(1);
+	ring_status = rh_reg_read(RING_STATUS);
+	if (ring_status & RING_STATUS_RUNNING) {
+		/* stop the ring */
+		reinit_completion(&rh->op_done);
+		rh_reg_write(RING_CONTROL, RING_CTRL_ENABLE | RING_CTRL_ABORT);
+		wait_for_completion_timeout(&rh->op_done, HZ);
+		ring_status = rh_reg_read(RING_STATUS);
+		if (ring_status & RING_STATUS_RUNNING) {
+			/*
+			 * We're deep in it if ever this condition is ever met.
+			 * Hardware might still be writing to memory, etc.
+			 */
+			dev_crit(&hci->master.dev, "unable to abort the ring\n");
+			WARN_ON(1);
+		}
 	}
 
 	spin_lock_irq(&hci->lock);

Signature · 7 lines

2.51.0


-- 
linux-i3c mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-i3c

Adrian Hunter

author

2026-03-06 07:25 UTC #9 lore raw

PATCH v3 9/14 i3c: mipi-i3c-hci: Add missing TID field to no-op command descriptor

In reply to Adrian Hunter

The internal control command descriptor used for no-op commands includes a Transaction ID (TID) field, but the no-op command constructed in hci_dma_dequeue_xfer() omitted it. As a result, the hardware receives a no-op descriptor without the expected TID.

This bug has gone unnoticed because the TID is currently not validated in the no-op completion path, but the descriptor format requires it to be present.

Add the missing TID field when generating a no-op descriptor so that its layout matches the defined command structure.

Fixes 9ad9a52cce282 ("i3c/master: introduce the mipi-i3c-hci driver")

Cc [email protected]

Signed-off-by Adrian Hunter <[email protected]>

Reviewed-by Frank Li <[email protected]>

---

Changes in V3:

None

Changes in V2:

Add Frank's Rev'd-by

 drivers/i3c/master/mipi-i3c-hci/cmd.h | 1 +
 drivers/i3c/master/mipi-i3c-hci/dma.c | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

drivers/i3c/master/mipi-i3c-hci/cmd.h +1 -0

index 1d6dd2c5d01a..b1bf87daa651 100644
@@ -17,6 +17,7 @@
 #define CMD_0_TOC			W0_BIT_(31)
 #define CMD_0_ROC			W0_BIT_(30)
 #define CMD_0_ATTR			W0_MASK(2, 0)
+#define CMD_0_TID			W0_MASK(6, 3)
 
 /*
  * Response Descriptor Structure

drivers/i3c/master/mipi-i3c-hci/dma.c +1 -1

index a3e8e01a35c9..239a195df7c7 100644
@@ -582,7 +582,7 @@ static bool hci_dma_dequeue_xfer(struct i3c_hci *hci,
 			u32 *ring_data = rh->xfer + rh->xfer_struct_sz * idx;
 
 			/* store no-op cmd descriptor */
-			*ring_data++ = FIELD_PREP(CMD_0_ATTR, 0x7);
+			*ring_data++ = FIELD_PREP(CMD_0_ATTR, 0x7) | FIELD_PREP(CMD_0_TID, xfer->cmd_tid);
 			*ring_data++ = 0;
 			if (hci->cmd == &mipi_i3c_hci_cmd_v2) {
 				*ring_data++ = 0;

Signature · 7 lines

2.51.0


-- 
linux-i3c mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-i3c

Adrian Hunter

author

2026-03-06 07:25 UTC #10 lore raw

PATCH v3 10/14 i3c: mipi-i3c-hci: Restart DMA ring correctly after dequeue abort

In reply to Adrian Hunter

The DMA dequeue path attempts to restart the ring after aborting an in-flight transfer, but the current sequence is incomplete. The controller must be brought out of the aborted state and the ring control registers must be programmed in the correct order: first clearing ABORT, then re-enabling the ring and asserting RUN_STOP to resume operation.

Add the missing controller resume step and update the ring control writes so that the ring is restarted using the proper sequence.

Fixes 9ad9a52cce282 ("i3c/master: introduce the mipi-i3c-hci driver")

Cc [email protected]

Signed-off-by Adrian Hunter <[email protected]>

Reviewed-by Frank Li <[email protected]>

---

Changes in V3:

None

Changes in V2:

Add Frank's Rev'd-by

 drivers/i3c/master/mipi-i3c-hci/dma.c | 2 ++
 1 file changed, 2 insertions(+)

drivers/i3c/master/mipi-i3c-hci/dma.c +2 -0

index 239a195df7c7..41b83f07fdab 100644
@@ -600,7 +600,9 @@ static bool hci_dma_dequeue_xfer(struct i3c_hci *hci,
 	}
 
 	/* restart the ring */
+	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);
 
 	spin_unlock_irq(&hci->lock);

Signature · 7 lines

2.51.0


-- 
linux-i3c mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-i3c

Adrian Hunter

author

2026-03-06 07:25 UTC #11 lore raw

PATCH v3 11/14 i3c: mipi-i3c-hci: Consolidate common xfer processing logic

In reply to Adrian Hunter

Several parts of the MIPI I3C HCI driver duplicate the same sequence for queuing a transfer, waiting for completion, and handling timeouts. This logic appears in five separate locations and will be affected by an upcoming fix.

Refactor the repeated code into a new helper, i3c_hci_process_xfer(), and store the timeout value in the hci_xfer structure so that callers do not need to pass it as a separate parameter.

Fixes 9ad9a52cce282 ("i3c/master: introduce the mipi-i3c-hci driver")

Cc [email protected]

Signed-off-by Adrian Hunter <[email protected]>

Reviewed-by Frank Li <[email protected]>

---

Changes in V3:

None

Changes in V2:

Add Frank's Rev'd-by

 drivers/i3c/master/mipi-i3c-hci/cmd_v1.c |  8 ++---
 drivers/i3c/master/mipi-i3c-hci/cmd_v2.c |  8 ++---
 drivers/i3c/master/mipi-i3c-hci/core.c   | 43 ++++++++++++++----------
 drivers/i3c/master/mipi-i3c-hci/hci.h    |  2 ++
 4 files changed, 33 insertions(+), 28 deletions(-)

drivers/i3c/master/mipi-i3c-hci/cmd_v1.c +3 -5

index 831a261f6c56..75d452d7f6af 100644
@@ -331,12 +331,10 @@ static int hci_cmd_v1_daa(struct i3c_hci *hci)
 			CMD_A0_ROC | CMD_A0_TOC;
 		xfer->cmd_desc[1] = 0;
 		xfer->completion = &done;
-		hci->io->queue_xfer(hci, xfer, 1);
-		if (!wait_for_completion_timeout(&done, HZ) &&
-		    hci->io->dequeue_xfer(hci, xfer, 1)) {
-			ret = -ETIMEDOUT;
+		xfer->timeout = HZ;
+		ret = i3c_hci_process_xfer(hci, xfer, 1);
+		if (ret)
 			break;
-		}
 		if ((RESP_STATUS(xfer->response) == RESP_ERR_ADDR_HEADER ||
 		     RESP_STATUS(xfer->response) == RESP_ERR_NACK) &&
 		    RESP_DATA_LENGTH(xfer->response) == 1) {

drivers/i3c/master/mipi-i3c-hci/cmd_v2.c +3 -5

index 054beee36da5..39eec26a363c 100644
@@ -253,6 +253,7 @@ static int hci_cmd_v2_daa(struct i3c_hci *hci)
 	xfer[0].rnw = true;
 	xfer[0].cmd_desc[1] = CMD_A1_DATA_LENGTH(8);
 	xfer[1].completion = &done;
+	xfer[1].timeout = HZ;
 
 	for (;;) {
 		ret = i3c_master_get_free_addr(&hci->master, next_addr);

@@ -272,12 +273,9 @@ static int hci_cmd_v2_daa(struct i3c_hci *hci)
 			CMD_A0_ASSIGN_ADDRESS(next_addr) |
 			CMD_A0_ROC |
 			CMD_A0_TOC;
-		hci->io->queue_xfer(hci, xfer, 2);
-		if (!wait_for_completion_timeout(&done, HZ) &&
-		    hci->io->dequeue_xfer(hci, xfer, 2)) {
-			ret = -ETIMEDOUT;
+		ret = i3c_hci_process_xfer(hci, xfer, 2);
+		if (ret)
 			break;
-		}
 		if (RESP_STATUS(xfer[0].response) != RESP_SUCCESS) {
 			ret = 0;  /* no more devices to be assigned */
 			break;

drivers/i3c/master/mipi-i3c-hci/core.c +25 ~~-18~~

79 lines not on this page · raw message

drivers/i3c/master/mipi-i3c-hci/hci.h +2 -0

index 9c63d80f7fc4..850016e3d4fe 100644
@@ -89,6 +89,7 @@ struct hci_xfer {
 	unsigned int data_len;
 	unsigned int cmd_tid;
 	struct completion *completion;
+	unsigned long timeout;
 	union {
 		struct {
 			/* PIO specific */

@@ -156,5 +157,6 @@ 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);
 void i3c_hci_sync_irq_inactive(struct i3c_hci *hci);
+int i3c_hci_process_xfer(struct i3c_hci *hci, struct hci_xfer *xfer, int n);
 
 #endif

Signature · 7 lines

2.51.0


-- 
linux-i3c mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-i3c

Adrian Hunter

author

2026-03-06 07:25 UTC #12 lore raw

PATCH v3 12/14 i3c: mipi-i3c-hci: Fix race in DMA error handling in interrupt context

In reply to Adrian Hunter

The DMA ring halts whenever a transfer encounters an error. The interrupt handler previously attempted to detect this situation and restart the ring if a transfer completed at the same time. However, this restart logic runs entirely in interrupt context and is inherently racy: it interacts with other paths manipulating the ring state, and fully serializing it within the interrupt handler is not practical.

Move this error-recovery logic out of the interrupt handler and into the transfer-processing path (i3c_hci_process_xfer()), where serialization and state management are already controlled. Introduce a new optional I/O-ops callback, handle_error(), invoked when a completed transfer reports an error. For DMA operation, the implementation simply calls the existing dequeue function, which safely aborts and restarts the ring when needed.

This removes the fragile ring-restart logic from the interrupt handler and centralizes error handling where proper sequencing can be ensured.

Fixes ccdb2e0e3b00d ("i3c: mipi-i3c-hci: Add Intel specific quirk to ring resuming")

Cc [email protected]

Signed-off-by Adrian Hunter <[email protected]>

Reviewed-by Frank Li <[email protected]>

---

Changes in V3:

Add Frank's Rev'd-by

Changes in V2:

None

 drivers/i3c/master/mipi-i3c-hci/core.c | 19 ++++++++++++----
 drivers/i3c/master/mipi-i3c-hci/dma.c  | 31 +++++++-------------------
 drivers/i3c/master/mipi-i3c-hci/hci.h  |  1 +
 3 files changed, 24 insertions(+), 27 deletions(-)

drivers/i3c/master/mipi-i3c-hci/core.c +15 -4

index 4a80671536f0..b98952d12d7c 100644
@@ -223,10 +223,21 @@ int i3c_hci_process_xfer(struct i3c_hci *hci, struct hci_xfer *xfer, int n)
 	if (ret)
 		return ret;
 
-	if (!wait_for_completion_timeout(done, timeout) &&
-	    hci->io->dequeue_xfer(hci, xfer, n)) {
-		dev_err(&hci->master.dev, "%s: timeout error\n", __func__);
-		return -ETIMEDOUT;
+	if (!wait_for_completion_timeout(done, timeout)) {
+		if (hci->io->dequeue_xfer(hci, xfer, n)) {
+			dev_err(&hci->master.dev, "%s: timeout error\n", __func__);
+			return -ETIMEDOUT;
+		}
+		return 0;
+	}
+
+	if (hci->io->handle_error) {
+		bool error = false;
+
+		for (int i = 0; i < n && !error; i++)
+			error = RESP_STATUS(xfer[i].response);
+		if (error)
+			return hci->io->handle_error(hci, xfer, n);
 	}
 
 	return 0;

drivers/i3c/master/mipi-i3c-hci/dma.c +8 ~~-23~~

index 41b83f07fdab..e487ef52f6b4 100644
@@ -609,6 +609,11 @@ static bool hci_dma_dequeue_xfer(struct i3c_hci *hci,
 	return did_unqueue;
 }
 
+static int hci_dma_handle_error(struct i3c_hci *hci, struct hci_xfer *xfer_list, int n)
+{
+	return hci_dma_dequeue_xfer(hci, xfer_list, n) ? -EIO : 0;
+}
+
 static void hci_dma_xfer_done(struct i3c_hci *hci, struct hci_rh_data *rh)
 {
 	u32 op1_val, op2_val, resp, *ring_resp;

@@ -870,29 +875,8 @@ static bool hci_dma_irq_handler(struct i3c_hci *hci)
 			hci_dma_xfer_done(hci, rh);
 		if (status & INTR_RING_OP)
 			complete(&rh->op_done);
-
-		if (status & INTR_TRANSFER_ABORT) {
-			u32 ring_status;
-
-			dev_notice_ratelimited(&hci->master.dev,
-				"Ring %d: Transfer Aborted\n", i);
-			mipi_i3c_hci_resume(hci);
-			ring_status = rh_reg_read(RING_STATUS);
-			if (!(ring_status & RING_STATUS_RUNNING) &&
-			    status & INTR_TRANSFER_COMPLETION &&
-			    status & INTR_TRANSFER_ERR) {
-				/*
-				 * Ring stop followed by run is an Intel
-				 * specific required quirk after resuming the
-				 * halted controller. Do it only when the ring
-				 * is not in running state after a transfer
-				 * error.
-				 */
-				rh_reg_write(RING_CONTROL, RING_CTRL_ENABLE);
-				rh_reg_write(RING_CONTROL, RING_CTRL_ENABLE |
-							   RING_CTRL_RUN_STOP);
-			}
-		}
+		if (status & INTR_TRANSFER_ABORT)
+			dev_dbg(&hci->master.dev, "Ring %d: Transfer Aborted\n", i);
 		if (status & INTR_IBI_RING_FULL)
 			dev_err_ratelimited(&hci->master.dev,
 				"Ring %d: IBI Ring Full Condition\n", i);

@@ -908,6 +892,7 @@ const struct hci_io_ops mipi_i3c_hci_dma = {
 	.cleanup		= hci_dma_cleanup,
 	.queue_xfer		= hci_dma_queue_xfer,
 	.dequeue_xfer		= hci_dma_dequeue_xfer,
+	.handle_error		= hci_dma_handle_error,
 	.irq_handler		= hci_dma_irq_handler,
 	.request_ibi		= hci_dma_request_ibi,
 	.free_ibi		= hci_dma_free_ibi,

drivers/i3c/master/mipi-i3c-hci/hci.h +1 -0

index 850016e3d4fe..9ac9d0e342f4 100644
@@ -123,6 +123,7 @@ struct hci_io_ops {
 	bool (*irq_handler)(struct i3c_hci *hci);
 	int (*queue_xfer)(struct i3c_hci *hci, struct hci_xfer *xfer, int n);
 	bool (*dequeue_xfer)(struct i3c_hci *hci, struct hci_xfer *xfer, int n);
+	int (*handle_error)(struct i3c_hci *hci, struct hci_xfer *xfer, int n);
 	int (*request_ibi)(struct i3c_hci *hci, struct i3c_dev_desc *dev,
 			   const struct i3c_ibi_setup *req);
 	void (*free_ibi)(struct i3c_hci *hci, struct i3c_dev_desc *dev);

Signature · 7 lines

2.51.0


-- 
linux-i3c mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-i3c

Adrian Hunter

author

2026-03-06 07:25 UTC #13 lore raw

PATCH v3 13/14 i3c: mipi-i3c-hci: Fix handling of shared IRQs during early initialization

In reply to Adrian Hunter

Shared interrupts may fire unexpectedly, including during periods when the controller is not yet fully initialized. Commit b9a15012a1452 ("i3c: mipi-i3c-hci: Add optional Runtime PM support") addressed this issue for the runtime-suspended state, but the same problem can also occur before the bus is enabled for the first time.

Ensure the IRQ handler ignores interrupts until initialization is complete by making consistent use of the existing irq_inactive flag. The flag is now set to false immediately before enabling the bus.

To guarantee correct ordering with respect to the IRQ handler, protect all transitions of irq_inactive with the same spinlock used inside the handler.

Fixes b8460480f62e1 ("i3c: mipi-i3c-hci: Allow for Multi-Bus Instances")

Cc [email protected]

Signed-off-by Adrian Hunter <[email protected]>

Reviewed-by Frank Li <[email protected]>

---

Changes in V3:

Add Frank's Rev'd-by

Changes in V2:

Place irq_inactive under the protection of the now unified spinlock, eliminating the need for explict memory barriers. Correctly position the update of irq_inactive in i3c_hci_sync_irq_inactive() to after synchronize_irq()

 drivers/i3c/master/mipi-i3c-hci/core.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

drivers/i3c/master/mipi-i3c-hci/core.c +10 -3

index b98952d12d7c..d5e91af7d569 100644
@@ -152,6 +152,9 @@ static int i3c_hci_bus_init(struct i3c_master_controller *m)
 	if (hci->quirks & HCI_QUIRK_RESP_BUF_THLD)
 		amd_set_resp_buf_thld(hci);
 
+	scoped_guard(spinlock_irqsave, &hci->lock)
+		hci->irq_inactive = false;
+
 	/* Enable bus with Hot-Join disabled */
 	reg_set(HC_CONTROL, HC_CONTROL_BUS_ENABLE | HC_CONTROL_HOT_JOIN_CTRL);
 	dev_dbg(&hci->master.dev, "HC_CONTROL = %#x", reg_read(HC_CONTROL));

@@ -184,8 +187,9 @@ void i3c_hci_sync_irq_inactive(struct i3c_hci *hci)
 	int irq = platform_get_irq(pdev, 0);
 
 	reg_write(INTR_SIGNAL_ENABLE, 0x0);
-	hci->irq_inactive = true;
 	synchronize_irq(irq);
+	scoped_guard(spinlock_irqsave, &hci->lock)
+		hci->irq_inactive = true;
 }
 
 static void i3c_hci_bus_cleanup(struct i3c_master_controller *m)

@@ -781,10 +785,11 @@ static int i3c_hci_runtime_resume(struct device *dev)
 
 	mipi_i3c_hci_dat_v1.restore(hci);
 
-	hci->irq_inactive = false;
-
 	hci->io->resume(hci);
 
+	scoped_guard(spinlock_irqsave, &hci->lock)
+		hci->irq_inactive = false;
+
 	/* Enable bus with Hot-Join disabled */
 	reg_set(HC_CONTROL, HC_CONTROL_BUS_ENABLE | HC_CONTROL_HOT_JOIN_CTRL);

@@ -975,6 +980,8 @@ static int i3c_hci_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
+	hci->irq_inactive = true;
+
 	irq = platform_get_irq(pdev, 0);
 	ret = devm_request_irq(&pdev->dev, irq, i3c_hci_irq_handler,
 			       IRQF_SHARED, NULL, hci);

Signature · 7 lines

2.51.0


-- 
linux-i3c mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-i3c

Adrian Hunter

author

2026-03-06 07:25 UTC #14 lore raw

PATCH v3 14/14 i3c: mipi-i3c-hci: Fallback to software reset when bus disable fails

In reply to Adrian Hunter

Disruption of the MIPI I3C HCI controller's internal state can cause i3c_hci_bus_disable() to fail when attempting to shut down the bus.

In the code paths where bus disable is invoked - bus clean-up and runtime suspend - the controller does not need to remain operational afterward, so a full controller reset is a safe recovery mechanism.

Add a fallback to issue a software reset when disabling the bus fails. This ensures the bus is reliably halted even if the controller's state machine is stuck or unresponsive.

The fallback is used both during bus clean-up and in the runtime suspend path. In the latter case, ensure interrupts are quiesced after reset.

Fixes 9ad9a52cce282 ("i3c/master: introduce the mipi-i3c-hci driver")

Cc [email protected]

Signed-off-by Adrian Hunter <[email protected]>

Reviewed-by Frank Li <[email protected]>

---

Changes in V3:

None

Changes in V2:

Add Frank's Rev'd-by

 drivers/i3c/master/mipi-i3c-hci/core.c | 65 ++++++++++++++------------
 1 file changed, 35 insertions(+), 30 deletions(-)

drivers/i3c/master/mipi-i3c-hci/core.c +35 ~~-30~~

index d5e91af7d569..284f3ed7af8c 100644
@@ -181,6 +181,34 @@ static int i3c_hci_bus_disable(struct i3c_hci *hci)
 	return ret;
 }
 
+static int i3c_hci_software_reset(struct i3c_hci *hci)
+{
+	u32 regval;
+	int ret;
+
+	/*
+	 * SOFT_RST must be clear before we write to it.
+	 * Then we must wait until it clears again.
+	 */
+	ret = readx_poll_timeout(reg_read, RESET_CONTROL, regval,
+				 !(regval & SOFT_RST), 0, 10 * USEC_PER_MSEC);
+	if (ret) {
+		dev_err(&hci->master.dev, "%s: Software reset stuck\n", __func__);
+		return ret;
+	}
+
+	reg_write(RESET_CONTROL, SOFT_RST);
+
+	ret = readx_poll_timeout(reg_read, RESET_CONTROL, regval,
+				 !(regval & SOFT_RST), 0, 10 * USEC_PER_MSEC);
+	if (ret) {
+		dev_err(&hci->master.dev, "%s: Software reset failed\n", __func__);
+		return ret;
+	}
+
+	return 0;
+}
+
 void i3c_hci_sync_irq_inactive(struct i3c_hci *hci)
 {
 	struct platform_device *pdev = to_platform_device(hci->master.dev.parent);

@@ -196,7 +224,8 @@ static void i3c_hci_bus_cleanup(struct i3c_master_controller *m)
 {
 	struct i3c_hci *hci = to_i3c_hci(m);
 
-	i3c_hci_bus_disable(hci);
+	if (i3c_hci_bus_disable(hci))
+		i3c_hci_software_reset(hci);
 	hci->io->cleanup(hci);
 }

@@ -626,34 +655,6 @@ static irqreturn_t i3c_hci_irq_handler(int irq, void *dev_id)
 	return result;
 }
 
-static int i3c_hci_software_reset(struct i3c_hci *hci)
-{
-	u32 regval;
-	int ret;
-
-	/*
-	 * SOFT_RST must be clear before we write to it.
-	 * Then we must wait until it clears again.
-	 */
-	ret = readx_poll_timeout(reg_read, RESET_CONTROL, regval,
-				 !(regval & SOFT_RST), 0, 10 * USEC_PER_MSEC);
-	if (ret) {
-		dev_err(&hci->master.dev, "%s: Software reset stuck\n", __func__);
-		return ret;
-	}
-
-	reg_write(RESET_CONTROL, SOFT_RST);
-
-	ret = readx_poll_timeout(reg_read, RESET_CONTROL, regval,
-				 !(regval & SOFT_RST), 0, 10 * USEC_PER_MSEC);
-	if (ret) {
-		dev_err(&hci->master.dev, "%s: Software reset failed\n", __func__);
-		return ret;
-	}
-
-	return 0;
-}
-
 static inline bool is_version_1_1_or_newer(struct i3c_hci *hci)
 {
 	return hci->version_major > 1 || (hci->version_major == 1 && hci->version_minor > 0);

@@ -764,8 +765,12 @@ static int i3c_hci_runtime_suspend(struct device *dev)
 	int ret;
 
 	ret = i3c_hci_bus_disable(hci);
-	if (ret)
+	if (ret) {
+		/* Fall back to software reset to disable the bus */
+		ret = i3c_hci_software_reset(hci);
+		i3c_hci_sync_irq_inactive(hci);
 		return ret;
+	}
 
 	hci->io->suspend(hci);

Signature · 7 lines

2.51.0


-- 
linux-i3c mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-i3c

Alexandre Belloni

maintainer Applied

2026-03-11 21:13 UTC #15 lore raw

In reply to Adrian Hunter

Quoted 9 lines · Adrian Hunter

Please note, all patches now have Frank's Reviewed-by.
Here are some fixes to the MIPI I3C HCI driver. Please consider them as fixes for v7.0.
They are mostly related to error recovery and a lack of serialization.
[...]

Applied, thanks!

[01/14] i3c: mipi-i3c-hci: Use ETIMEDOUT instead of ETIME for timeout errors

        https://git.kernel.org/abelloni/c/4167b8914463
[02/14] i3c: mipi-i3c-hci: Fix Hot-Join NACK
        https://git.kernel.org/abelloni/c/fa9586bd77ad
[03/14] i3c: mipi-i3c-hci: Factor out DMA mapping from queuing path
        https://git.kernel.org/abelloni/c/f3bcbfe1b8b0
[04/14] i3c: mipi-i3c-hci: Consolidate spinlocks
        https://git.kernel.org/abelloni/c/fa12bb903bc3
[05/14] i3c: mipi-i3c-hci: Fix race in DMA ring enqueue for parallel xfers
        https://git.kernel.org/abelloni/c/4decbbc8a8cf
[06/14] i3c: mipi-i3c-hci: Fix race in DMA ring dequeue
        https://git.kernel.org/abelloni/c/1dca8aee80ee
[07/14] i3c: mipi-i3c-hci: Fix race between DMA ring dequeue and interrupt handler
        https://git.kernel.org/abelloni/c/f0b5159637ca
[08/14] i3c: mipi-i3c-hci: Correct RING_CTRL_ABORT handling in DMA dequeue
        https://git.kernel.org/abelloni/c/b795e68bf307
[09/14] i3c: mipi-i3c-hci: Add missing TID field to no-op command descriptor
        https://git.kernel.org/abelloni/c/ec3cfd835f7c
[10/14] i3c: mipi-i3c-hci: Restart DMA ring correctly after dequeue abort
        https://git.kernel.org/abelloni/c/b6d586431ae2
[11/14] i3c: mipi-i3c-hci: Consolidate common xfer processing logic
        https://git.kernel.org/abelloni/c/7ac45bc68f08
[12/14] i3c: mipi-i3c-hci: Fix race in DMA error handling in interrupt context
        https://git.kernel.org/abelloni/c/e44d2719225e
[13/14] i3c: mipi-i3c-hci: Fix handling of shared IRQs during early initialization
        https://git.kernel.org/abelloni/c/c6396b835a5e
[14/14] i3c: mipi-i3c-hci: Fallback to software reset when bus disable fails
        https://git.kernel.org/abelloni/c/9a258d1336f7

Best regards,

Signature · 8 lines

Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

-- 
linux-i3c mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-i3c