[PATCH 0/3] i3c: master: amd: Add IBI and hot-join support

[PATCH 0/3] i3c: master: amd: Add IBI and hot-join support

[Shubham Patil]

This series adds In-Band Interrupt (IBI) and Hot-Join support to the AMD
AXI I3C master controller driver.

An In-Band Interrupt (IBI) replaces the dedicated interrupt pin an I2C
slave would otherwise need. When the bus is idle, a target wanting
attention pulls SDA low to generate a START, then drives its own dynamic
address with RnW=1 during the arbitrated address header. The controller
either ACKs the request, accepting the interrupt and optionally reading a
Mandatory Data Byte and payload, or NACKs it. The interrupt therefore
travels in band on SDA/SCL, saving a wire per device.

Hot-Join (HJ) lets a target that was not present at boot join a running
bus, after being powered up later or physically attached. Having no
dynamic address yet, it arbitrates using the reserved Hot-Join address;
the controller ACKs that and runs dynamic address assignment to enumerate
it. The same hardware arbitrates and ACKs it as an IBI.

Both features are synthesis-time options in the IP, so patch 1 adds two
boolean device tree properties, "xlnx,ibi-capable" and "xlnx,hj-capable",
to describe what a design implements. Hot-Join requests are ACKed by the
IBI machinery, so hj-capable depends on ibi-capable and the binding
enforces that. The controller interrupt only carries IBI and Hot-Join
events, so it is required only for ibi-capable designs. The driver
assembles its ops at probe from the capability flags.

Patch 2 adds IBI support. The IBI ACK enable and its interrupt mask are
controller-wide rather than per-target, so they are armed on the first
target to enable IBIs and disarmed on the last to disable them, with
per-target control left on the bus via ENEC/DISEC. IBIs also share the
response and read FIFOs with normal transfers and are distinguished only
by the reserved TID 0x0F, so the transfer path identifies IBI responses
by TID and discards them along with their payload. A raced IBI is
dropped rather than deferred.

Patch 3 adds Hot-Join. ENTDAA needs bus traffic and can sleep, so it
cannot run in the hard IRQ handler; the Hot-Join event is therefore
deferred to a work item that runs the re-enumeration.

Shubham Patil (3):
  dt-bindings: i3c: xlnx: Add IBI and hot-join capability properties
  i3c: master: amd: Add support for in-band interrupts
  i3c: master: amd: Add hot-join support

 .../bindings/i3c/xlnx,axi-i3c-1.0.yaml        |  23 +
 drivers/i3c/master/amd-i3c-master.c           | 593 +++++++++++++++++-
 2 files changed, 596 insertions(+), 20 deletions(-)


base-commit: 320722812708d34c11c2d067a018fcbeec6b64aa
-- 
2.34.1

[PATCH 1/3] dt-bindings: i3c: xlnx: Add IBI and hot-join capability properties

[Shubham Patil]

In-Band Interrupt and Hot-Join are synthesis-time options of the AXI I3C
IP. Describe them with two boolean properties.

A Hot-Join request is acknowledged by the IBI machinery, so a hot-join
capable design is always IBI capable as well. Both events are reported
through the controller interrupt, which is therefore required whenever
the capability is present.

Signed-off-by: Shubham Patil <shubhamsanjay.patil@amd.com>
---
 .../bindings/i3c/xlnx,axi-i3c-1.0.yaml        | 23 +++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/Documentation/devicetree/bindings/i3c/xlnx,axi-i3c-1.0.yaml b/Documentation/devicetree/bindings/i3c/xlnx,axi-i3c-1.0.yaml
index 2caa245a8656..07e3d0b4d767 100644
--- a/Documentation/devicetree/bindings/i3c/xlnx,axi-i3c-1.0.yaml
+++ b/Documentation/devicetree/bindings/i3c/xlnx,axi-i3c-1.0.yaml
@@ -32,13 +32,34 @@ properties:
   interrupts:
     maxItems: 1
 
+  xlnx,ibi-capable:
+    type: boolean
+    description:
+      The IP is synthesized with In-Band Interrupt support. IBIs are reported
+      through the controller interrupt.
+
+  xlnx,hj-capable:
+    type: boolean
+    description:
+      The IP is synthesized with Hot-Join support. A Hot-Join request is
+      acknowledged by the same machinery as an In-Band Interrupt.
+
 required:
   - compatible
   - reg
   - clocks
 
+dependencies:
+  xlnx,hj-capable: ["xlnx,ibi-capable"]
+
 allOf:
   - $ref: i3c.yaml#
+  - if:
+      required:
+        - xlnx,ibi-capable
+    then:
+      required:
+        - interrupts
 
 unevaluatedProperties: false
 
@@ -54,5 +75,7 @@ examples:
         interrupts = <GIC_SPI 89 IRQ_TYPE_LEVEL_HIGH>;
         #address-cells = <3>;
         #size-cells = <0>;
+        xlnx,ibi-capable;
+        xlnx,hj-capable;
     };
 ...
-- 
2.34.1

[PATCH 2/3] i3c: master: amd: Add support for in-band interrupts

[Shubham Patil]

Add support for receiving and dequeueing I3C in-band interrupts.

IBI support is optional at synthesis time, so the ops are assembled at
probe from the base set plus the IBI callbacks, and the callbacks are
only registered when the "xlnx,ibi-capable" property is present. The
controller interrupt carries IBI events only, so it is likewise only
requested for such designs.

The IBI ACK enable and its interrupt mask are controller-wide rather
than per-target, so they are armed when the first target enables IBIs
and disarmed when the last one disables them. Per-target control stays
on the bus via ENEC/DISEC.

IBIs share the response and read FIFOs with normal transfers and are
tagged with the reserved TID 0x0F. Masking the interrupt for the
duration of a transfer keeps the handler from running, but does not
stop the controller from ACKing an IBI, so one taken in an idle moment
can still queue its response word and payload ahead of the transfer's
own. The polled transfer path therefore demultiplexes the response FIFO
by TID: an IBI response and its payload are discarded, and the
transfer's own response is claimed into the master state for the caller
to consume.

Signed-off-by: Shubham Patil <shubhamsanjay.patil@amd.com>
---
 drivers/i3c/master/amd-i3c-master.c | 528 ++++++++++++++++++++++++++--
 1 file changed, 508 insertions(+), 20 deletions(-)

diff --git a/drivers/i3c/master/amd-i3c-master.c b/drivers/i3c/master/amd-i3c-master.c
index ef5ad5abb788..d47b04326ea4 100644
--- a/drivers/i3c/master/amd-i3c-master.c
+++ b/drivers/i3c/master/amd-i3c-master.c
@@ -12,6 +12,7 @@
 #include <linux/delay.h>
 #include <linux/err.h>
 #include <linux/i3c/master.h>
+#include <linux/interrupt.h>
 #include <linux/io.h>
 #include <linux/iopoll.h>
 #include <linux/kernel.h>
@@ -19,7 +20,9 @@
 #include <linux/mutex.h>
 #include <linux/of.h>
 #include <linux/platform_device.h>
+#include <linux/property.h>
 #include <linux/slab.h>
+#include <linux/spinlock.h>
 #include <linux/time.h>
 #include <linux/unaligned.h>
 
@@ -28,6 +31,8 @@
 #define XI3C_CR_OFFSET				0x08	/* Control Register */
 #define XI3C_ADDRESS_OFFSET			0x0C	/* Target Address Register */
 #define XI3C_SR_OFFSET				0x10	/* Status Register */
+#define XI3C_INTR_STATUS_OFFSET			0x14	/* Interrupt status event (W1C) */
+#define XI3C_INTR_RE_OFFSET			0x18	/* Interrupt rising-edge enable mask */
 #define XI3C_CMD_FIFO_OFFSET			0x20	/* I3C Command FIFO Register */
 #define XI3C_WR_FIFO_OFFSET			0x24	/* I3C Write Data FIFO Register */
 #define XI3C_RD_FIFO_OFFSET			0x28	/* I3C Read Data FIFO Register */
@@ -42,13 +47,17 @@
 #define XI3C_TSU_STOP_OFFSET			0x50	/* I3C STOP Setup Register */
 #define XI3C_OD_SCL_HIGH_TIME_OFFSET		0x54	/* I3C OD SCL HIGH Register */
 #define XI3C_OD_SCL_LOW_TIME_OFFSET		0x58	/* I3C OD SCL LOW Register */
+#define XI3C_IBI_TARGET_ADDR_OFFSET		0x5C	/* IBI source address register */
+#define XI3C_TARGET_ADDR_BCR_OFFSET		0x60	/* Per-target {DA, BCR} SPRAM */
 #define XI3C_PID0_OFFSET			0x6C	/* LSB 4 bytes of the PID */
 #define XI3C_PID1_BCR_DCR			0x70	/* MSB 2 bytes of the PID, BCR and DCR */
 
 #define XI3C_CR_EN_MASK				BIT(0)	/* Core Enable */
 #define XI3C_CR_RESUME_MASK			BIT(2)	/* Core Resume */
+#define XI3C_CR_IBI_MASK			BIT(3)	/* IBI ACK enable */
 #define XI3C_SR_RESP_NOT_EMPTY_MASK		BIT(4)	/* Resp Fifo not empty status mask */
 #define XI3C_RD_FIFO_NOT_EMPTY_MASK		BIT(15)	/* Read Fifo not empty status mask */
+#define XI3C_INTR_IBI_MASK			BIT(7)	/* IBI event (INTR status/enable) */
 
 #define XI3C_BCR_MASK				GENMASK(23, 16)
 #define XI3C_DCR_MASK				GENMASK(31, 24)
@@ -57,6 +66,7 @@
 #define XI3C_REV_NUM_MASK			GENMASK(15, 8)
 #define XI3C_PID1_MASK				GENMASK(15, 0)
 #define XI3C_FIFO_LEVEL_MASK			GENMASK(15, 0)
+#define XI3C_RESP_FIFO_LEVEL_MASK		GENMASK(31, 16)
 #define XI3C_RESP_CODE_MASK			GENMASK(8, 5)
 
 /* Controller response codes; PG439 page 34, Table 46 */
@@ -69,7 +79,9 @@
 #define XI3C_XFER_SHORT_READ			1
 
 #define XI3C_RESP_BYTES_MASK			GENMASK(20, 9)	/* NUM_BYTES processed */
+#define XI3C_RESP_TID_MASK			GENMASK(3, 0)	/* response transfer ID */
 #define XI3C_ADDR_MASK				GENMASK(6, 0)
+#define XI3C_TGT_BCR_MASK			GENMASK(15, 8)	/* TARGET_ADDR_BCR: BCR field */
 #define XI3C_FIFOS_RST_MASK			GENMASK(4, 1)
 
 /* Command FIFO word layout (bit ranges encoded in the GENMASK/BIT args) */
@@ -121,9 +133,15 @@
 #define XI3C_POLL_INTERVAL_US			10
 
 #define XI3C_I2C_MODE				0
-#define XI3C_I2C_TID				0
 #define XI3C_SDR_MODE				1
+
+/*
+ * TID tags: I2C/SDR are driver-chosen for normal transfers;
+ * 0x0F is the spec-reserved IBI response TID.
+ */
+#define XI3C_I2C_TID				0
 #define XI3C_SDR_TID				1
+#define XI3C_IBI_RESP_TID			0x0F
 
 #define XI3C_WORD_LEN				4
 
@@ -132,6 +150,9 @@
 /* Software guard: 1 s (ms, for msecs_to_jiffies) to bail out if a transfer never completes */
 #define XI3C_XFER_TIMEOUT_MS			1000
 
+/* IBI response wait in hard-IRQ context; software-chosen safety cap. */
+#define XI3C_IBI_RESP_TIMEOUT_US		1000
+
 struct xi3c_cmd {
 	const void *tx_buf;
 	void *rx_buf;
@@ -160,9 +181,30 @@ struct xi3c_xfer {
  * @membase: Memory base of the HW registers.
  * @pclk: Input clock driving the controller.
  * @lock: Serializes transfers and CCC submission.
+ * @reg_lock: IRQ-safe lock serializing read-modify-write of the shared
+ *	      control (XI3C_CR_OFFSET) and interrupt-enable
+ *	      (XI3C_INTR_RE_OFFSET) registers.
  * @daa: ENTDAA enumeration state.
  * @daa.addrs: Dynamic addresses assigned in enumeration order.
  * @daa.index: Number of responders enumerated so far.
+ * @xfer_resp: Response word claimed for the transfer in flight; valid while
+ *	       @xfer_resp_valid is set. Guarded by @lock.
+ * @xfer_resp_valid: True once the in-flight transfer's own response word has
+ *		     been taken from the shared response FIFO. Guarded by @lock.
+ * @irq: Controller interrupt line, used for IBI events. Only valid when
+ *	 @ibi_capable is set.
+ * @ibi_capable: True when the IP was synthesized with In-Band Interrupt
+ *		 support ("xlnx,ibi-capable"); also the condition for the
+ *		 controller interrupt being present.
+ * @ops: Controller ops handed to the framework, assembled at probe time from
+ *	 the base ops plus the callbacks the design actually supports.
+ * @ibi: In-Band Interrupt slot tracking.
+ * @ibi.lock: Protects @ibi.slots against the IBI handler.
+ * @ibi.slots: Per-device IBI registration, indexed by slot.
+ * @ibi.enabled_count: Number of devices with IBI currently enabled; the
+ *		       controller-wide IBI ACK/interrupt is armed on the
+ *		       first enable and disarmed on the last disable. Guarded
+ *		       by @reg_lock.
  */
 struct xi3c_master {
 	struct i3c_master_controller base;
@@ -170,10 +212,31 @@ struct xi3c_master {
 	void __iomem *membase;
 	struct clk *pclk;
 	struct mutex lock; /* serializes transfers and CCC submission */
+	spinlock_t reg_lock;
 	struct {
 		u8 addrs[XI3C_MAX_DEVS];
 		u8 index;
 	} daa;
+	u32 xfer_resp;
+	bool xfer_resp_valid;
+	int irq;
+	bool ibi_capable;
+	struct i3c_master_controller_ops ops;
+	struct {
+		spinlock_t lock; /* protects slots[] against the IBI handler */
+		struct i3c_dev_desc *slots[XI3C_MAX_DEVS];
+		unsigned int enabled_count;
+	} ibi;
+};
+
+/**
+ * struct xi3c_i3c_dev_data - Per-device controller state.
+ * @ibi_pool: Generic IBI slot pool backing this device's IBIs.
+ * @ibi_slot: Index into &xi3c_master.ibi.slots, or -1 when unregistered.
+ */
+struct xi3c_i3c_dev_data {
+	struct i3c_generic_ibi_pool *ibi_pool;
+	s16 ibi_slot;
 };
 
 static inline struct xi3c_master *
@@ -206,23 +269,79 @@ static inline bool xi3c_is_resp_available(struct xi3c_master *master)
 			 ioread32(master->membase + XI3C_SR_OFFSET));
 }
 
+static inline u16 xi3c_resp_fifo_level(struct xi3c_master *master)
+{
+	return FIELD_GET(XI3C_RESP_FIFO_LEVEL_MASK,
+			 ioread32(master->membase + XI3C_FIFO_LVL_STATUS_1_OFFSET));
+}
+
+/*
+ * Discard an unconsumed IBI payload from the RX FIFO, keeping the FIFO
+ * aligned for the next transfer.
+ */
+static void xi3c_master_drain_ibi_fifo(struct xi3c_master *master, u16 len)
+{
+	unsigned int words = DIV_ROUND_UP(len, XI3C_WORD_LEN);
+
+	while (words--)
+		ioread32(master->membase + XI3C_RD_FIFO_OFFSET);
+}
+
+/*
+ * Tell whether the transfer in flight has had its response posted, claiming
+ * that response into @master->xfer_resp when it has.
+ *
+ * The response and read FIFOs are shared with IBIs, and masking the interrupt
+ * for the duration of a transfer does not stop the controller from ACKing one:
+ * an IBI taken in an idle moment queues its response word and its payload
+ * ahead of ours. Drop such an IBI here, so a transfer cannot mistake the IBI's
+ * response code and byte count for its own, nor the IBI's payload for read
+ * data.
+ */
+static bool xi3c_xfer_resp_available(struct xi3c_master *master)
+{
+	u32 resp;
+	u16 len;
+
+	if (master->xfer_resp_valid)
+		return true;
+
+	while (xi3c_is_resp_available(master)) {
+		resp = ioread32(master->membase + XI3C_RESP_STATUS_FIFO_OFFSET);
+
+		if (FIELD_GET(XI3C_RESP_TID_MASK, resp) != XI3C_IBI_RESP_TID) {
+			master->xfer_resp = resp;
+			master->xfer_resp_valid = true;
+			return true;
+		}
+
+		len = FIELD_GET(XI3C_RESP_BYTES_MASK, resp);
+		dev_dbg_ratelimited(master->dev,
+				    "IBI raced with transfer, dropping %u bytes\n",
+				    len);
+		xi3c_master_drain_ibi_fifo(master, len);
+	}
+
+	return false;
+}
+
 static int xi3c_get_response(struct xi3c_master *master, struct xi3c_cmd *cmd)
 {
 	u32 response_data;
-	u32 resp_reg;
+	bool available;
 	u8 code;
 	int ret;
 
-	ret = readl_poll_timeout(master->membase + XI3C_SR_OFFSET,
-				 resp_reg,
-				 resp_reg & XI3C_SR_RESP_NOT_EMPTY_MASK,
-				 XI3C_POLL_INTERVAL_US, XI3C_RESP_TIMEOUT_US);
+	ret = read_poll_timeout(xi3c_xfer_resp_available, available, available,
+				XI3C_POLL_INTERVAL_US, XI3C_RESP_TIMEOUT_US,
+				false, master);
 	if (ret) {
 		dev_err(master->dev, "XI3C response timeout\n");
 		return ret;
 	}
 
-	response_data = ioread32(master->membase + XI3C_RESP_STATUS_FIFO_OFFSET);
+	response_data = master->xfer_resp;
+	master->xfer_resp_valid = false;
 	code = FIELD_GET(XI3C_RESP_CODE_MASK, response_data);
 
 	switch (code) {
@@ -247,6 +366,19 @@ static int xi3c_get_response(struct xi3c_master *master, struct xi3c_cmd *cmd)
 	}
 }
 
+/*
+ * Wait for a response in hard-IRQ context (IBI path). Uses the atomic
+ * poll variant since the IBI handler must not sleep.
+ */
+static int xi3c_wait_resp_atomic(struct xi3c_master *master)
+{
+	u32 sr;
+
+	return readl_poll_timeout_atomic(master->membase + XI3C_SR_OFFSET, sr,
+					 sr & XI3C_SR_RESP_NOT_EMPTY_MASK,
+					 0, XI3C_IBI_RESP_TIMEOUT_US);
+}
+
 static inline void xi3c_writesl_be(void __iomem *addr, const void *buffer,
 				   unsigned int count)
 {
@@ -331,6 +463,7 @@ static inline void xi3c_master_disable(struct xi3c_master *master)
 
 static inline void xi3c_master_resume(struct xi3c_master *master)
 {
+	guard(spinlock_irqsave)(&master->reg_lock);
 	iowrite32(ioread32(master->membase + XI3C_CR_OFFSET) |
 		  XI3C_CR_RESUME_MASK, master->membase + XI3C_CR_OFFSET);
 }
@@ -356,6 +489,11 @@ static void xi3c_master_reset_fifos(struct xi3c_master *master)
 
 static inline void xi3c_master_init(struct xi3c_master *master)
 {
+	/* Mask all interrupt sources and clear any stale latched events. */
+	iowrite32(0, master->membase + XI3C_INTR_RE_OFFSET);
+	iowrite32(ioread32(master->membase + XI3C_INTR_STATUS_OFFSET),
+		  master->membase + XI3C_INTR_STATUS_OFFSET);
+
 	/* Reset fifos */
 	xi3c_master_reset_fifos(master);
 
@@ -437,13 +575,19 @@ static int xi3c_master_read(struct xi3c_master *master, struct xi3c_cmd *cmd)
 		return ret;
 	}
 
-	if (!(status_reg & XI3C_RD_FIFO_NOT_EMPTY_MASK))
+	/*
+	 * No data queued means this read produced none, unless what is queued
+	 * is an IBI response that raced with it; xi3c_xfer_resp_available()
+	 * discards that case so the wait for our own data can continue.
+	 */
+	if (!(status_reg & XI3C_RD_FIFO_NOT_EMPTY_MASK) &&
+	    xi3c_xfer_resp_available(master))
 		return 0;
 
 	timeout = jiffies + msecs_to_jiffies(XI3C_XFER_TIMEOUT_MS);
 
 	/* Read data from rx fifo */
-	while (cmd->rx_len > 0 && !xi3c_is_resp_available(master)) {
+	while (cmd->rx_len > 0 && !xi3c_xfer_resp_available(master)) {
 		if (time_after(jiffies, timeout)) {
 			dev_err(master->dev, "XI3C read timeout\n");
 			return -EIO;
@@ -503,7 +647,7 @@ static int xi3c_master_write(struct xi3c_master *master, struct xi3c_cmd *cmd)
 
 	timeout = jiffies + msecs_to_jiffies(XI3C_XFER_TIMEOUT_MS);
 	/* Fill if any remaining data to tx fifo */
-	while (cmd->tx_len > 0 && !xi3c_is_resp_available(master)) {
+	while (cmd->tx_len > 0 && !xi3c_xfer_resp_available(master)) {
 		if (time_after(jiffies, timeout)) {
 			dev_err(master->dev, "XI3C write timeout\n");
 			return -EIO;
@@ -547,22 +691,33 @@ static int xi3c_master_common_xfer(struct xi3c_master *master,
 				   struct xi3c_xfer *xfer)
 {
 	unsigned int i;
-	int ret;
+	int ret = 0;
 
 	guard(mutex)(&master->lock);
 
+	/*
+	 * IBIs share the response/read FIFOs; mask the IRQ so the handler
+	 * cannot take this transfer's response.
+	 */
+	if (master->ibi_capable)
+		disable_irq(master->irq);
+
+	/* Discard any response claimed but not consumed by an aborted transfer. */
+	master->xfer_resp_valid = false;
+
 	for (i = 0; i < xfer->ncmds; i++) {
 		ret = xi3c_master_xfer(master, &xfer->cmds[i]);
-		if (ret) {
-			/* Count commands sent on the bus; the rest never ran */
-			xfer->nissued = i + 1;
-			return ret;
-		}
+		if (ret)
+			break;
 	}
 
-	xfer->nissued = xfer->ncmds;
+	if (master->ibi_capable)
+		enable_irq(master->irq);
 
-	return 0;
+	/* On failure @i is the command that failed; the rest never ran. */
+	xfer->nissued = (i < xfer->ncmds) ? i + 1 : xfer->ncmds;
+
+	return ret;
 }
 
 static int xi3c_master_do_daa(struct i3c_master_controller *m)
@@ -1052,9 +1207,296 @@ static void xi3c_master_bus_cleanup(struct i3c_master_controller *m)
 {
 	struct xi3c_master *master = to_xi3c_master(m);
 
+	/*
+	 * Disarm all interrupt sources and the IBI ACK so the controller can't
+	 * assert once disabled; reset the refcount that tracks them.
+	 */
+	scoped_guard(spinlock_irqsave, &master->reg_lock) {
+		iowrite32(0, master->membase + XI3C_INTR_RE_OFFSET);
+		iowrite32(ioread32(master->membase + XI3C_CR_OFFSET) &
+			  ~XI3C_CR_IBI_MASK, master->membase + XI3C_CR_OFFSET);
+		master->ibi.enabled_count = 0;
+	}
+
 	xi3c_master_disable(master);
 }
 
+static int xi3c_master_request_ibi(struct i3c_dev_desc *dev,
+				   const struct i3c_ibi_setup *req)
+{
+	struct i3c_master_controller *m = i3c_dev_get_master(dev);
+	struct xi3c_master *master = to_xi3c_master(m);
+	struct xi3c_i3c_dev_data *data;
+	unsigned long flags;
+	unsigned int i;
+
+	data = kzalloc_obj(*data, GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	data->ibi_slot = -1;
+	data->ibi_pool = i3c_generic_ibi_alloc_pool(dev, req);
+	if (IS_ERR(data->ibi_pool)) {
+		int ret = PTR_ERR(data->ibi_pool);
+
+		kfree(data);
+		return ret;
+	}
+
+	spin_lock_irqsave(&master->ibi.lock, flags);
+	for (i = 0; i < ARRAY_SIZE(master->ibi.slots); i++) {
+		if (!master->ibi.slots[i]) {
+			data->ibi_slot = i;
+			master->ibi.slots[i] = dev;
+			break;
+		}
+	}
+	spin_unlock_irqrestore(&master->ibi.lock, flags);
+
+	if (data->ibi_slot < 0) {
+		dev_err(master->dev, "IBI: no free slot for addr 0x%02x\n",
+			dev->info.dyn_addr);
+		i3c_generic_ibi_free_pool(data->ibi_pool);
+		kfree(data);
+		return -ENOSPC;
+	}
+
+	i3c_dev_set_master_data(dev, data);
+
+	return 0;
+}
+
+static void xi3c_master_free_ibi(struct i3c_dev_desc *dev)
+{
+	struct xi3c_i3c_dev_data *data = i3c_dev_get_master_data(dev);
+	struct i3c_master_controller *m = i3c_dev_get_master(dev);
+	struct xi3c_master *master = to_xi3c_master(m);
+	unsigned long flags;
+
+	spin_lock_irqsave(&master->ibi.lock, flags);
+	master->ibi.slots[data->ibi_slot] = NULL;
+	spin_unlock_irqrestore(&master->ibi.lock, flags);
+
+	i3c_generic_ibi_free_pool(data->ibi_pool);
+	i3c_dev_set_master_data(dev, NULL);
+	kfree(data);
+}
+
+/*
+ * Arm the controller-wide IBI ACK enable and event interrupt on the first
+ * device to enable IBIs. Per-device SIR control is still done on the bus via
+ * ENEC/DISEC; this only gates whether the controller reacts to IBIs at all.
+ */
+static void xi3c_master_ibi_arm(struct xi3c_master *master)
+{
+	guard(spinlock_irqsave)(&master->reg_lock);
+
+	if (master->ibi.enabled_count++)
+		return;
+
+	iowrite32(ioread32(master->membase + XI3C_CR_OFFSET) | XI3C_CR_IBI_MASK,
+		  master->membase + XI3C_CR_OFFSET);
+	iowrite32(ioread32(master->membase + XI3C_INTR_RE_OFFSET) |
+		  XI3C_INTR_IBI_MASK, master->membase + XI3C_INTR_RE_OFFSET);
+}
+
+/* Disarm the controller-wide IBI enable once the last device disables IBIs. */
+static void xi3c_master_ibi_disarm(struct xi3c_master *master)
+{
+	guard(spinlock_irqsave)(&master->reg_lock);
+
+	if (WARN_ON(!master->ibi.enabled_count))
+		return;
+
+	if (--master->ibi.enabled_count)
+		return;
+
+	iowrite32(ioread32(master->membase + XI3C_INTR_RE_OFFSET) &
+		  ~XI3C_INTR_IBI_MASK, master->membase + XI3C_INTR_RE_OFFSET);
+	iowrite32(ioread32(master->membase + XI3C_CR_OFFSET) & ~XI3C_CR_IBI_MASK,
+		  master->membase + XI3C_CR_OFFSET);
+}
+
+static int xi3c_master_enable_ibi(struct i3c_dev_desc *dev)
+{
+	struct i3c_master_controller *m = i3c_dev_get_master(dev);
+	struct xi3c_master *master = to_xi3c_master(m);
+	u32 val;
+	int ret;
+
+	/*
+	 * Program {DA, BCR} into the IBI SPRAM; the DA field self-selects the
+	 * row, so a plain write keeps the entry fresh after DAA reassigns the
+	 * address.
+	 */
+	val = FIELD_PREP(XI3C_ADDR_MASK, dev->info.dyn_addr) |
+	      FIELD_PREP(XI3C_TGT_BCR_MASK, dev->info.bcr);
+	iowrite32(val, master->membase + XI3C_TARGET_ADDR_BCR_OFFSET);
+
+	/* Arm the controller IBI path before allowing this device to SIR. */
+	xi3c_master_ibi_arm(master);
+
+	ret = i3c_master_enec_locked(m, dev->info.dyn_addr, I3C_CCC_EVENT_SIR);
+	if (ret) {
+		dev_err(master->dev, "IBI: ENEC failed addr 0x%02x (%d)\n",
+			dev->info.dyn_addr, ret);
+		xi3c_master_ibi_disarm(master);
+	}
+
+	return ret;
+}
+
+static int xi3c_master_disable_ibi(struct i3c_dev_desc *dev)
+{
+	struct i3c_master_controller *m = i3c_dev_get_master(dev);
+	struct xi3c_master *master = to_xi3c_master(m);
+	int ret;
+
+	ret = i3c_master_disec_locked(m, dev->info.dyn_addr, I3C_CCC_EVENT_SIR);
+
+	xi3c_master_ibi_disarm(master);
+
+	return ret;
+}
+
+static void xi3c_master_recycle_ibi_slot(struct i3c_dev_desc *dev,
+					 struct i3c_ibi_slot *slot)
+{
+	struct xi3c_i3c_dev_data *data = i3c_dev_get_master_data(dev);
+
+	i3c_generic_ibi_recycle_slot(data->ibi_pool, slot);
+}
+
+static void xi3c_master_handle_ibi(struct xi3c_master *master)
+{
+	struct xi3c_i3c_dev_data *data;
+	struct i3c_ibi_slot *slot;
+	struct i3c_dev_desc *dev;
+	u32 ibi_reg, resp;
+	unsigned int id;
+	u8 da, code;
+	u16 len;
+
+	ibi_reg = ioread32(master->membase + XI3C_IBI_TARGET_ADDR_OFFSET);
+	da = FIELD_GET(XI3C_ADDR_MASK, ibi_reg);
+
+	if (xi3c_wait_resp_atomic(master)) {
+		dev_err_ratelimited(master->dev, "XI3C IBI response timeout\n");
+		/* Controller parks in STOP on failure; RESUME to recover (PG439). */
+		xi3c_master_resume(master);
+		return;
+	}
+
+	resp = ioread32(master->membase + XI3C_RESP_STATUS_FIFO_OFFSET);
+	code = FIELD_GET(XI3C_RESP_CODE_MASK, resp);
+	len = FIELD_GET(XI3C_RESP_BYTES_MASK, resp);
+
+	/*
+	 * TID 0x0F marks an IBI response. A transfer masks this interrupt and
+	 * demuxes the response FIFO itself, so its response must never reach
+	 * here; draining one would eat that transfer's read data.
+	 */
+	if (FIELD_GET(XI3C_RESP_TID_MASK, resp) != XI3C_IBI_RESP_TID) {
+		WARN_ONCE(1, "XI3C: transfer response in IBI handler, dropping %u bytes\n",
+			  len);
+		xi3c_master_drain_ibi_fifo(master, len);
+		/* A non-success stray response parks the controller in STOP (PG439). */
+		if (code != XI3C_RESP_CODE_SUCCESS)
+			xi3c_master_resume(master);
+		return;
+	}
+
+	if (code != XI3C_RESP_CODE_SUCCESS) {
+		dev_dbg_ratelimited(master->dev,
+				    "IBI: non-success code %u, dropping %u bytes\n",
+				    code, len);
+		xi3c_master_drain_ibi_fifo(master, len);
+		xi3c_master_resume(master);
+		return;
+	}
+
+	spin_lock(&master->ibi.lock);
+
+	for (id = 0; id < ARRAY_SIZE(master->ibi.slots); id++)
+		if (master->ibi.slots[id] &&
+		    master->ibi.slots[id]->info.dyn_addr == da)
+			break;
+
+	if (id == ARRAY_SIZE(master->ibi.slots)) {
+		dev_dbg_ratelimited(master->dev,
+				    "IBI: no registered device for addr 0x%02x\n",
+				    da);
+		goto err_drain;
+	}
+
+	dev = master->ibi.slots[id];
+
+	if (!dev->ibi) {
+		dev_dbg_ratelimited(master->dev,
+				    "IBI: addr 0x%02x not fully set up\n", da);
+		goto err_drain;
+	}
+
+	if (len > dev->ibi->max_payload_len) {
+		dev_dbg_ratelimited(master->dev,
+				    "IBI: payload %u > max %u for addr 0x%02x\n",
+				    len, dev->ibi->max_payload_len, da);
+		goto err_drain;
+	}
+
+	data = i3c_dev_get_master_data(dev);
+	slot = i3c_generic_ibi_get_free_slot(data->ibi_pool);
+	if (!slot) {
+		dev_dbg_ratelimited(master->dev,
+				    "IBI: no free pool slot for addr 0x%02x\n",
+				    da);
+		goto err_drain;
+	}
+
+	slot->len = 0;
+	if (len) {
+		xi3c_readl_fifo(master->membase + XI3C_RD_FIFO_OFFSET,
+				slot->data, len);
+		slot->len = len;
+	}
+
+	i3c_master_queue_ibi(dev, slot);
+	spin_unlock(&master->ibi.lock);
+
+	return;
+
+err_drain:
+	/*
+	 * Reached only with code == SUCCESS, so the controller is not parked in
+	 * STOP; no resume needed (PG439).
+	 */
+	spin_unlock(&master->ibi.lock);
+	xi3c_master_drain_ibi_fifo(master, len);
+}
+
+static irqreturn_t xi3c_master_irq_handler(int irq, void *dev_id)
+{
+	struct xi3c_master *master = dev_id;
+	u32 status;
+
+	status = ioread32(master->membase + XI3C_INTR_STATUS_OFFSET);
+	if (!status)
+		return IRQ_NONE;
+
+	/* Write-1-to-clear the latched events before servicing them. */
+	iowrite32(status, master->membase + XI3C_INTR_STATUS_OFFSET);
+
+	/* Event latches once; drain the queued IBI responses (TID checked per entry). */
+	if (status & XI3C_INTR_IBI_MASK) {
+		u16 pending = xi3c_resp_fifo_level(master);
+
+		while (pending-- && xi3c_is_resp_available(master))
+			xi3c_master_handle_ibi(master);
+	}
+
+	return IRQ_HANDLED;
+}
+
 static const struct i3c_master_controller_ops xi3c_master_ops = {
 	.bus_init = xi3c_master_bus_init,
 	.bus_cleanup = xi3c_master_bus_cleanup,
@@ -1065,6 +1507,15 @@ static const struct i3c_master_controller_ops xi3c_master_ops = {
 	.i2c_xfers = xi3c_master_i2c_xfers,
 };
 
+static void xi3c_master_init_ibi_ops(struct xi3c_master *master)
+{
+	master->ops.request_ibi = xi3c_master_request_ibi;
+	master->ops.free_ibi = xi3c_master_free_ibi;
+	master->ops.enable_ibi = xi3c_master_enable_ibi;
+	master->ops.disable_ibi = xi3c_master_disable_ibi;
+	master->ops.recycle_ibi_slot = xi3c_master_recycle_ibi_slot;
+}
+
 static int xi3c_master_probe(struct platform_device *pdev)
 {
 	struct xi3c_master *master;
@@ -1090,16 +1541,53 @@ static int xi3c_master_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
+	master->ops = xi3c_master_ops;
+
+	spin_lock_init(&master->ibi.lock);
+	spin_lock_init(&master->reg_lock);
+
+	master->ibi_capable = device_property_read_bool(master->dev,
+							"xlnx,ibi-capable");
+
+	/*
+	 * The interrupt only carries IBI events, so it is only described for
+	 * designs synthesized with that feature.
+	 */
+	if (master->ibi_capable) {
+		xi3c_master_init_ibi_ops(master);
+
+		master->irq = platform_get_irq(pdev, 0);
+		if (master->irq < 0)
+			return master->irq;
+
+		ret = devm_request_irq(master->dev, master->irq,
+				       xi3c_master_irq_handler, IRQF_NO_AUTOEN,
+				       dev_name(master->dev), master);
+		if (ret)
+			return dev_err_probe(master->dev, ret,
+					     "Failed to request IRQ\n");
+	}
+
 	platform_set_drvdata(pdev, master);
 
-	return i3c_master_register(&master->base, master->dev,
-				   &xi3c_master_ops, false);
+	ret = i3c_master_register(&master->base, master->dev, &master->ops,
+				  false);
+	if (ret)
+		return ret;
+
+	if (master->ibi_capable)
+		enable_irq(master->irq);
+
+	return 0;
 }
 
 static void xi3c_master_remove(struct platform_device *pdev)
 {
 	struct xi3c_master *master = platform_get_drvdata(pdev);
 
+	if (master->ibi_capable)
+		disable_irq(master->irq);
+
 	i3c_master_unregister(&master->base);
 }
 
-- 
2.34.1

[PATCH 3/3] i3c: master: amd: Add hot-join support

[Shubham Patil]

Add hot-join support for the AMD AXI I3C master controller.
By default, the hot-join acknowledgment is disabled. Users can use the
sysfs entry to enable it. A hot-join event is serviced by re-running DAA
from a work item so the i3c core enumerates the new device.

Signed-off-by: Shubham Patil <shubhamsanjay.patil@amd.com>
---
 drivers/i3c/master/amd-i3c-master.c | 77 ++++++++++++++++++++++++++---
 1 file changed, 71 insertions(+), 6 deletions(-)

diff --git a/drivers/i3c/master/amd-i3c-master.c b/drivers/i3c/master/amd-i3c-master.c
index d47b04326ea4..62221b138bfe 100644
--- a/drivers/i3c/master/amd-i3c-master.c
+++ b/drivers/i3c/master/amd-i3c-master.c
@@ -25,6 +25,7 @@
 #include <linux/spinlock.h>
 #include <linux/time.h>
 #include <linux/unaligned.h>
+#include <linux/workqueue.h>
 
 #define XI3C_VERSION_OFFSET			0x00	/* Version Register */
 #define XI3C_RESET_OFFSET			0x04	/* Soft Reset Register */
@@ -55,9 +56,11 @@
 #define XI3C_CR_EN_MASK				BIT(0)	/* Core Enable */
 #define XI3C_CR_RESUME_MASK			BIT(2)	/* Core Resume */
 #define XI3C_CR_IBI_MASK			BIT(3)	/* IBI ACK enable */
+#define XI3C_CR_HJ_MASK				BIT(4)	/* Hot-Join ACK enable */
 #define XI3C_SR_RESP_NOT_EMPTY_MASK		BIT(4)	/* Resp Fifo not empty status mask */
 #define XI3C_RD_FIFO_NOT_EMPTY_MASK		BIT(15)	/* Read Fifo not empty status mask */
 #define XI3C_INTR_IBI_MASK			BIT(7)	/* IBI event (INTR status/enable) */
+#define XI3C_INTR_HJ_MASK			BIT(8)	/* Hot-Join event */
 
 #define XI3C_BCR_MASK				GENMASK(23, 16)
 #define XI3C_DCR_MASK				GENMASK(31, 24)
@@ -191,11 +194,14 @@ struct xi3c_xfer {
  *	       @xfer_resp_valid is set. Guarded by @lock.
  * @xfer_resp_valid: True once the in-flight transfer's own response word has
  *		     been taken from the shared response FIFO. Guarded by @lock.
- * @irq: Controller interrupt line, used for IBI events. Only valid when
- *	 @ibi_capable is set.
+ * @irq: Controller interrupt line, used for IBI/Hot-Join events. Only valid
+ *	 when @ibi_capable is set.
  * @ibi_capable: True when the IP was synthesized with In-Band Interrupt
- *		 support ("xlnx,ibi-capable"); also the condition for the
- *		 controller interrupt being present.
+ *		 support ("xlnx,ibi-capable"). Since Hot-Join requests are
+ *		 ACKed by the IBI machinery, this is also the condition for
+ *		 the controller interrupt being present at all.
+ * @hj_capable: True when the IP was synthesized with Hot-Join support
+ *		("xlnx,hj-capable"); implies @ibi_capable.
  * @ops: Controller ops handed to the framework, assembled at probe time from
  *	 the base ops plus the callbacks the design actually supports.
  * @ibi: In-Band Interrupt slot tracking.
@@ -205,6 +211,7 @@ struct xi3c_xfer {
  *		       controller-wide IBI ACK/interrupt is armed on the
  *		       first enable and disarmed on the last disable. Guarded
  *		       by @reg_lock.
+ * @hj_work: Deferred re-DAA triggered from the Hot-Join interrupt.
  */
 struct xi3c_master {
 	struct i3c_master_controller base;
@@ -221,12 +228,14 @@ struct xi3c_master {
 	bool xfer_resp_valid;
 	int irq;
 	bool ibi_capable;
+	bool hj_capable;
 	struct i3c_master_controller_ops ops;
 	struct {
 		spinlock_t lock; /* protects slots[] against the IBI handler */
 		struct i3c_dev_desc *slots[XI3C_MAX_DEVS];
 		unsigned int enabled_count;
 	} ibi;
+	struct work_struct hj_work;
 };
 
 /**
@@ -1474,6 +1483,40 @@ static void xi3c_master_handle_ibi(struct xi3c_master *master)
 	xi3c_master_drain_ibi_fifo(master, len);
 }
 
+static int xi3c_master_enable_hotjoin(struct i3c_master_controller *m)
+{
+	struct xi3c_master *master = to_xi3c_master(m);
+
+	guard(spinlock_irqsave)(&master->reg_lock);
+	iowrite32(ioread32(master->membase + XI3C_CR_OFFSET) | XI3C_CR_HJ_MASK,
+		  master->membase + XI3C_CR_OFFSET);
+	iowrite32(ioread32(master->membase + XI3C_INTR_RE_OFFSET) |
+		  XI3C_INTR_HJ_MASK, master->membase + XI3C_INTR_RE_OFFSET);
+
+	return 0;
+}
+
+static int xi3c_master_disable_hotjoin(struct i3c_master_controller *m)
+{
+	struct xi3c_master *master = to_xi3c_master(m);
+
+	guard(spinlock_irqsave)(&master->reg_lock);
+	iowrite32(ioread32(master->membase + XI3C_INTR_RE_OFFSET) &
+		  ~XI3C_INTR_HJ_MASK, master->membase + XI3C_INTR_RE_OFFSET);
+	iowrite32(ioread32(master->membase + XI3C_CR_OFFSET) & ~XI3C_CR_HJ_MASK,
+		  master->membase + XI3C_CR_OFFSET);
+
+	return 0;
+}
+
+static void xi3c_master_hj_work(struct work_struct *work)
+{
+	struct xi3c_master *master = container_of(work, struct xi3c_master,
+						  hj_work);
+
+	i3c_master_do_daa(&master->base);
+}
+
 static irqreturn_t xi3c_master_irq_handler(int irq, void *dev_id)
 {
 	struct xi3c_master *master = dev_id;
@@ -1494,6 +1537,9 @@ static irqreturn_t xi3c_master_irq_handler(int irq, void *dev_id)
 			xi3c_master_handle_ibi(master);
 	}
 
+	if (status & XI3C_INTR_HJ_MASK)
+		queue_work(master->base.wq, &master->hj_work);
+
 	return IRQ_HANDLED;
 }
 
@@ -1514,6 +1560,11 @@ static void xi3c_master_init_ibi_ops(struct xi3c_master *master)
 	master->ops.enable_ibi = xi3c_master_enable_ibi;
 	master->ops.disable_ibi = xi3c_master_disable_ibi;
 	master->ops.recycle_ibi_slot = xi3c_master_recycle_ibi_slot;
+
+	if (master->hj_capable) {
+		master->ops.enable_hotjoin = xi3c_master_enable_hotjoin;
+		master->ops.disable_hotjoin = xi3c_master_disable_hotjoin;
+	}
 }
 
 static int xi3c_master_probe(struct platform_device *pdev)
@@ -1545,13 +1596,21 @@ static int xi3c_master_probe(struct platform_device *pdev)
 
 	spin_lock_init(&master->ibi.lock);
 	spin_lock_init(&master->reg_lock);
+	INIT_WORK(&master->hj_work, xi3c_master_hj_work);
 
 	master->ibi_capable = device_property_read_bool(master->dev,
 							"xlnx,ibi-capable");
+	master->hj_capable = device_property_read_bool(master->dev,
+						       "xlnx,hj-capable");
+
+	/* Hot-Join requests are ACKed by the IBI machinery. */
+	if (master->hj_capable && !master->ibi_capable)
+		return dev_err_probe(master->dev, -EINVAL,
+				     "xlnx,hj-capable requires xlnx,ibi-capable\n");
 
 	/*
-	 * The interrupt only carries IBI events, so it is only described for
-	 * designs synthesized with that feature.
+	 * The interrupt only carries IBI and Hot-Join events, so it is only
+	 * described for designs synthesized with those features.
 	 */
 	if (master->ibi_capable) {
 		xi3c_master_init_ibi_ops(master);
@@ -1588,6 +1647,12 @@ static void xi3c_master_remove(struct platform_device *pdev)
 	if (master->ibi_capable)
 		disable_irq(master->irq);
 
+	if (master->hj_capable) {
+		/* Stop new Hot-Join IRQs before flushing, so the work can't be re-queued. */
+		xi3c_master_disable_hotjoin(&master->base);
+		cancel_work_sync(&master->hj_work);
+	}
+
 	i3c_master_unregister(&master->base);
 }
 
-- 
2.34.1

Re: [PATCH 3/3] i3c: master: amd: Add hot-join support

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The driver duplicates the I3C core framework's Hot-Join work mechanism but conditionally cancels it, leading to a Use-After-Free during driver unbind.
--

--- Patch [3]: [PATCH 3/3] i3c: master: amd: Add hot-join support ---
commit 06fad25ac0b3ab8838ac7e9c3c0f9c33f86244dc
Author: Shubham Patil <shubhamsanjay.patil@amd.com>

i3c: master: amd: Add hot-join support

This patch adds hot-join support for the AMD AXI I3C master controller.
By default, the hot-join acknowledgment is disabled, but it can be enabled
via sysfs. A hot-join event is serviced by re-running DAA from a work
item so the i3c core enumerates the new device.

> diff --git a/drivers/i3c/master/amd-i3c-master.c b/drivers/i3c/master/amd-i3c-master.c
> --- a/drivers/i3c/master/amd-i3c-master.c
> +++ b/drivers/i3c/master/amd-i3c-master.c
[ ... ]
> @@ -221,12 +228,14 @@ struct xi3c_master {
>  	bool xfer_resp_valid;
>  	int irq;
>  	bool ibi_capable;
> +	bool hj_capable;
>  	struct i3c_master_controller_ops ops;
>  	struct {
>  		spinlock_t lock; /* protects slots[] against the IBI handler */
>  		struct i3c_dev_desc *slots[XI3C_MAX_DEVS];
>  		unsigned int enabled_count;
>  	} ibi;
> +	struct work_struct hj_work;
>  };

[Severity: High]
Could this custom hj_work bypass the safety checks in the core framework?

The I3C core provides i3c_master_queue_hotjoin() which safely handles
queuing, checks the shutting_down flag, and guarantees unconditional
cancellation during i3c_master_unregister(). Defining a custom work item
here bypasses those safeguards.

[ ... ]
> @@ -1474,6 +1483,40 @@ static void xi3c_master_handle_ibi(struct xi3c_master *master)
[ ... ]
> +static void xi3c_master_hj_work(struct work_struct *work)
> +{
> +	struct xi3c_master *master = container_of(work, struct xi3c_master,
> +						  hj_work);
> +
> +	i3c_master_do_daa(&master->base);
> +}
> +
>  static irqreturn_t xi3c_master_irq_handler(int irq, void *dev_id)
>  {
>  	struct xi3c_master *master = dev_id;
> @@ -1494,6 +1537,9 @@ static irqreturn_t xi3c_master_irq_handler(int irq, void *dev_id)
>  			xi3c_master_handle_ibi(master);
>  	}
>  
> +	if (status & XI3C_INTR_HJ_MASK)
> +		queue_work(master->base.wq, &master->hj_work);
> +
>  	return IRQ_HANDLED;
>  }

[Severity: High]
Can this unconditionally queue the work even if hj_capable is false?

If the device tree lacks Hot-Join support (hj_capable is false), but the
hardware status register still records a Hot-Join event (for instance, from
an I3C target issuing a request while an IBI interrupt triggers the handler),
this will queue hj_work since master->hj_capable is not checked here.

[ ... ]
> @@ -1588,6 +1647,12 @@ static void xi3c_master_remove(struct platform_device *pdev)
>  	if (master->ibi_capable)
>  		disable_irq(master->irq);
>  
> +	if (master->hj_capable) {
> +		/* Stop new Hot-Join IRQs before flushing, so the work can't be re-queued. */
> +		xi3c_master_disable_hotjoin(&master->base);
> +		cancel_work_sync(&master->hj_work);
> +	}
> +
>  	i3c_master_unregister(&master->base);
>  }

[Severity: High]
Could this lead to a use-after-free during driver unbind?

If the work was queued while hj_capable is false, this conditional block is
skipped and the work is never cancelled. The devres framework will free the
master structure, but the work remains on the workqueue and will access freed
memory when it executes.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260814115155.3974988-1-shubhamsanjay.patil@amd.com?part=3

Re: [PATCH 2/3] i3c: master: amd: Add support for in-band interrupts

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] ACKed In-Band Interrupts (IBIs) are permanently discarded when they race with normal transfers.
- [High] Eager polling of the shared RX FIFO corrupts read transfers with IBI payload data if an IBI wins arbitration.
- [High] Race condition in IBI teardown allows late IBIs to be queued after the generic core's completion check, leading to a Use-After-Free.
- [Medium] The controller-wide IBI enable refcount is decremented even if disabling the target on the bus fails.
--

--- Patch [2]: [PATCH 2/3] i3c: master: amd: Add support for in-band interrupts ---
commit 49feb531b9ade85c295172ceec9457d5f98c1d0d
Author: Shubham Patil <shubhamsanjay.patil@amd.com>
Subject: i3c: master: amd: Add support for in-band interrupts

This commit adds support for receiving and dequeueing I3C in-band
interrupts (IBIs). The controller interrupt carries IBI events only, and
the IBI ACK enable and its interrupt mask are controller-wide. It handles
demultiplexing shared response and read FIFOs when IBIs race with normal
transfers.

> diff --git a/drivers/i3c/master/amd-i3c-master.c b/drivers/i3c/master/amd-i3c-master.c
> index ef5ad5abb788c..d47b04326ea46 100644
> --- a/drivers/i3c/master/amd-i3c-master.c
> +++ b/drivers/i3c/master/amd-i3c-master.c
[ ... ]
> @@ -206,23 +269,79 @@ static inline bool xi3c_is_resp_available(struct xi3c_master *master)
[ ... ]
> +static bool xi3c_xfer_resp_available(struct xi3c_master *master)
> +{
> +	u32 resp;
> +	u16 len;
> +
> +	if (master->xfer_resp_valid)
> +		return true;
> +
> +	while (xi3c_is_resp_available(master)) {
> +		resp = ioread32(master->membase + XI3C_RESP_STATUS_FIFO_OFFSET);
> +
> +		if (FIELD_GET(XI3C_RESP_TID_MASK, resp) != XI3C_IBI_RESP_TID) {
> +			master->xfer_resp = resp;
> +			master->xfer_resp_valid = true;
> +			return true;
> +		}
> +
> +		len = FIELD_GET(XI3C_RESP_BYTES_MASK, resp);
> +		dev_dbg_ratelimited(master->dev,
> +				    "IBI raced with transfer, dropping %u bytes\n",
> +				    len);
> +		xi3c_master_drain_ibi_fifo(master, len);
> +	}
> +
> +	return false;
> +}

[Severity: High]
Since the hardware has already ACKed the IBI on the bus, doesn't draining
and discarding the IBI response here mean it is permanently lost? 

If an IBI arrives while the driver is executing a normal transfer, the
driver masks the IBI interrupt, but the hardware still ACKs the IBI on the
bus. The target device won't resend the IBI once it is ACKed. Could this
cause essential target events like sensor data ready or hot-join requests to
be dropped entirely?

[ ... ]
> @@ -437,13 +575,19 @@ static int xi3c_master_read(struct xi3c_master *master, struct xi3c_cmd *cmd)
[ ... ]
>  	/* Read data from rx fifo */
> -	while (cmd->rx_len > 0 && !xi3c_is_resp_available(master)) {
> +	while (cmd->rx_len > 0 && !xi3c_xfer_resp_available(master)) {
>  		if (time_after(jiffies, timeout)) {
>  			dev_err(master->dev, "XI3C read timeout\n");
>  			return -EIO;

[Severity: High]
Is it safe to eagerly poll the shared RX FIFO before validating the
transfer response?

If an IBI wins arbitration concurrently, its payload might enter the shared
RX FIFO first. Because xi3c_master_rd_from_rx_fifo() is called within this
loop before the transfer's own response word arrives, it seems the driver
might mistakenly read the IBI payload bytes directly into the normal read
transfer's buffer. Could this corrupt normal read transfers with IBI
payload data?

[ ... ]
> @@ -1052,9 +1207,296 @@ static void xi3c_master_bus_cleanup(struct i3c_master_controller *m)
[ ... ]
> +static void xi3c_master_free_ibi(struct i3c_dev_desc *dev)
> +{
> +	struct xi3c_i3c_dev_data *data = i3c_dev_get_master_data(dev);
> +	struct i3c_master_controller *m = i3c_dev_get_master(dev);
> +	struct xi3c_master *master = to_xi3c_master(m);
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&master->ibi.lock, flags);
> +	master->ibi.slots[data->ibi_slot] = NULL;
> +	spin_unlock_irqrestore(&master->ibi.lock, flags);
> +
> +	i3c_generic_ibi_free_pool(data->ibi_pool);
> +	i3c_dev_set_master_data(dev, NULL);
> +	kfree(data);
> +}
[ ... ]
> +static int xi3c_master_disable_ibi(struct i3c_dev_desc *dev)
> +{
> +	struct i3c_master_controller *m = i3c_dev_get_master(dev);
> +	struct xi3c_master *master = to_xi3c_master(m);
> +	int ret;
> +
> +	ret = i3c_master_disec_locked(m, dev->info.dyn_addr, I3C_CCC_EVENT_SIR);
> +
> +	xi3c_master_ibi_disarm(master);
> +
> +	return ret;
> +}

[Severity: Medium]
If i3c_master_disec_locked() fails (for example, if the target NACKs the
DISEC command), the device is still considered enabled by the core. Does
unconditionally calling xi3c_master_ibi_disarm() here risk dropping the
master->ibi.enabled_count prematurely? This could unintentionally mask the
global IBI interrupt.

[Severity: High]
Does this teardown sequence need a synchronization barrier (like
synchronize_irq()) to ensure the hard IRQ handler has finished processing
events?

If a late IBI fires just before the DISEC command, the IRQ might be pending
on another CPU. Because xi3c_master_disable_ibi() completes without waiting
for the IRQ, the core might see no pending IBIs and proceed to call
xi3c_master_free_ibi(), freeing the data->ibi_pool.

Could the concurrent IRQ handler then execute, copy the payload, and queue
a work item that accesses the now-freed IBI pool?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260814115155.3974988-1-shubhamsanjay.patil@amd.com?part=2

Re: [PATCH 1/3] dt-bindings: i3c: xlnx: Add IBI and hot-join capability properties

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 2322 bytes --]

On Fri, Aug 14, 2026 at 05:21:53PM +0530, Shubham Patil wrote:
> In-Band Interrupt and Hot-Join are synthesis-time options of the AXI I3C
> IP. Describe them with two boolean properties.
> 
> A Hot-Join request is acknowledged by the IBI machinery, so a hot-join
> capable design is always IBI capable as well. Both events are reported
> through the controller interrupt, which is therefore required whenever
> the capability is present.
> 
> Signed-off-by: Shubham Patil <shubhamsanjay.patil@amd.com>
> ---
>  .../bindings/i3c/xlnx,axi-i3c-1.0.yaml        | 23 +++++++++++++++++++
>  1 file changed, 23 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/i3c/xlnx,axi-i3c-1.0.yaml b/Documentation/devicetree/bindings/i3c/xlnx,axi-i3c-1.0.yaml
> index 2caa245a8656..07e3d0b4d767 100644
> --- a/Documentation/devicetree/bindings/i3c/xlnx,axi-i3c-1.0.yaml
> +++ b/Documentation/devicetree/bindings/i3c/xlnx,axi-i3c-1.0.yaml
> @@ -32,13 +32,34 @@ properties:
>    interrupts:
>      maxItems: 1
>  
> +  xlnx,ibi-capable:
> +    type: boolean
> +    description:
> +      The IP is synthesized with In-Band Interrupt support. IBIs are reported
> +      through the controller interrupt.
> +
> +  xlnx,hj-capable:

tbh, I'd prefer if these were spelt out a little bit more, characters
are cheap. Not a big deal though.

> +    type: boolean
> +    description:
> +      The IP is synthesized with Hot-Join support. A Hot-Join request is
> +      acknowledged by the same machinery as an In-Band Interrupt.
> +
>  required:
>    - compatible
>    - reg
>    - clocks
>  
> +dependencies:
> +  xlnx,hj-capable: ["xlnx,ibi-capable"]
> +
>  allOf:
>    - $ref: i3c.yaml#
> +  - if:
> +      required:
> +        - xlnx,ibi-capable
> +    then:
> +      required:
> +        - interrupts

Why is one dependency expressed this way, and the other expressed using
the dependencies property? Can the interrupts property be handled that
way too?

Cheers,
Conor.

>  
>  unevaluatedProperties: false
>  
> @@ -54,5 +75,7 @@ examples:
>          interrupts = <GIC_SPI 89 IRQ_TYPE_LEVEL_HIGH>;
>          #address-cells = <3>;
>          #size-cells = <0>;
> +        xlnx,ibi-capable;
> +        xlnx,hj-capable;
>      };
>  ...
> -- 
> 2.34.1
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 1/3] dt-bindings: i3c: xlnx: Add IBI and hot-join capability properties

[Frank Li]

On Fri, Aug 14, 2026 at 05:21:53PM +0530, Shubham Patil wrote:
> In-Band Interrupt and Hot-Join are synthesis-time options of the AXI I3C
> IP. Describe them with two boolean properties.
>
> A Hot-Join request is acknowledged by the IBI machinery, so a hot-join
> capable design is always IBI capable as well. Both events are reported
> through the controller interrupt, which is therefore required whenever
> the capability is present.
>
> Signed-off-by: Shubham Patil <shubhamsanjay.patil@amd.com>
> ---
>  .../bindings/i3c/xlnx,axi-i3c-1.0.yaml        | 23 +++++++++++++++++++
>  1 file changed, 23 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/i3c/xlnx,axi-i3c-1.0.yaml b/Documentation/devicetree/bindings/i3c/xlnx,axi-i3c-1.0.yaml
> index 2caa245a8656..07e3d0b4d767 100644
> --- a/Documentation/devicetree/bindings/i3c/xlnx,axi-i3c-1.0.yaml
> +++ b/Documentation/devicetree/bindings/i3c/xlnx,axi-i3c-1.0.yaml
> @@ -32,13 +32,34 @@ properties:
>    interrupts:
>      maxItems: 1
>
> +  xlnx,ibi-capable:
> +    type: boolean
> +    description:
> +      The IP is synthesized with In-Band Interrupt support. IBIs are reported
> +      through the controller interrupt.
> +
> +  xlnx,hj-capable:
> +    type: boolean
> +    description:
> +      The IP is synthesized with Hot-Join support. A Hot-Join request is
> +      acknowledged by the same machinery as an In-Band Interrupt.
> +

The compatiblity xlnx,axi-i3c-1.0 already indicates such cap. Do you have
variance. such as

	one Soc have two instances.
	instance 1  have hj and ibi cap
	instance 2 no hj and ibi cap

If new version support HJ and IBI, need use new compatible string.

Frank

>  required:
>    - compatible
>    - reg
>    - clocks
>
> +dependencies:
> +  xlnx,hj-capable: ["xlnx,ibi-capable"]
> +
>  allOf:
>    - $ref: i3c.yaml#
> +  - if:
> +      required:
> +        - xlnx,ibi-capable
> +    then:
> +      required:
> +        - interrupts
>
>  unevaluatedProperties: false
>
> @@ -54,5 +75,7 @@ examples:
>          interrupts = <GIC_SPI 89 IRQ_TYPE_LEVEL_HIGH>;
>          #address-cells = <3>;
>          #size-cells = <0>;
> +        xlnx,ibi-capable;
> +        xlnx,hj-capable;
>      };
>  ...
> --
> 2.34.1
>