Devicetree
 help / color / mirror / Atom feed
* [PATCH v2 0/2] Add NXP PCA9641 I2C bus master arbiter support
@ 2026-09-17  8:58 Shiv Prakash Gupta
  2026-09-17  8:58 ` [PATCH v2 1/2] dt-bindings: i2c: Add NXP PCA9641 I2C bus master arbiter Shiv Prakash Gupta
  2026-09-17  8:58 ` [PATCH v2 2/2] i2c: mux: Add NXP PCA9641 I2C bus master arbiter driver Shiv Prakash Gupta
  0 siblings, 2 replies; 4+ messages in thread
From: Shiv Prakash Gupta @ 2026-09-17  8:58 UTC (permalink / raw)
  To: andi.shyti, robh, krzk+dt, conor+dt, peda, linux-i2c, devicetree,
	linux-kernel
  Cc: vikash.bansal, priyanka.jain, Shiv Prakash Gupta

This series adds device tree bindings and an I2C mux driver for the NXP
PCA9641 2-to-1 I2C bus master arbiter. The PCA9641 lets two upstream I2C
masters share a single downstream slave bus using a lock/grant protocol:
a master asserts LOCK_REQ, waits for LOCK_GRANT, sets BUS_CONNECT to open
the switch, performs its transactions, and then clears LOCK_REQ to release
the bus. The driver plugs into the i2c-mux core as an I2C_MUX_ARBITRATOR
and supports interrupt-assisted arbitration (via INT0/INT1 wired to a
GPIO) with a fallback to polling when no interrupt is configured.

Changes in v2:
- Binding: add Acked-by from Conor Dooley (no functional changes).
- Driver: drop IRQF_SHARED from the threaded IRQ request.
- Driver: clear the latched LOCK_REQ on every timeout path in
  select_chan() so a master that gives up does not block the peer.
- Driver: reorder probe interrupt setup - mask all sources and clear
  stale INT_STATUS before requesting the IRQ, then unmask LOCK_GRANT
  and BUS_LOST.
- Driver: re-mask interrupts if i2c_mux_add_adapter() fails.
- Driver: unregister the child adapter before masking interrupts in
  remove().
- Driver: use a named initializer for the i2c_device_id table.

Shiv Prakash Gupta (2):
  dt-bindings: i2c: Add NXP PCA9641 I2C bus master arbiter
  i2c: mux: Add NXP PCA9641 I2C bus master arbiter driver

 .../devicetree/bindings/i2c/nxp,pca9641.yaml  | 109 +++++
 MAINTAINERS                                   |   7 +
 drivers/i2c/muxes/Kconfig                     |  15 +
 drivers/i2c/muxes/Makefile                    |   1 +
 drivers/i2c/muxes/i2c-mux-pca9641.c           | 420 ++++++++++++++++++
 5 files changed, 552 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/i2c/nxp,pca9641.yaml
 create mode 100644 drivers/i2c/muxes/i2c-mux-pca9641.c

-- 
2.34.1


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH v2 1/2] dt-bindings: i2c: Add NXP PCA9641 I2C bus master arbiter
  2026-09-17  8:58 [PATCH v2 0/2] Add NXP PCA9641 I2C bus master arbiter support Shiv Prakash Gupta
@ 2026-09-17  8:58 ` Shiv Prakash Gupta
  2026-09-17  8:58 ` [PATCH v2 2/2] i2c: mux: Add NXP PCA9641 I2C bus master arbiter driver Shiv Prakash Gupta
  1 sibling, 0 replies; 4+ messages in thread
From: Shiv Prakash Gupta @ 2026-09-17  8:58 UTC (permalink / raw)
  To: andi.shyti, robh, krzk+dt, conor+dt, peda, linux-i2c, devicetree,
	linux-kernel
  Cc: vikash.bansal, priyanka.jain, Shiv Prakash Gupta, Conor Dooley

Add device tree binding schema for the NXP PCA9641 2-to-1 I2C bus
master arbiter.

The PCA9641 arbitrates between two upstream I2C masters competing for a
single downstream slave bus using a lock/grant ownership model. The
binding supports an optional 'interrupts' property for interrupt-assisted
arbitration.

Signed-off-by: Shiv Prakash Gupta <shivprakash.gupta@nxp.com>
Acked-by: Conor Dooley <conor.dooley@microchip.com>
---
Changes in v2:
- Add Acked-by from Conor Dooley (no functional changes to the binding).

 .../devicetree/bindings/i2c/nxp,pca9641.yaml  | 109 ++++++++++++++++++
 1 file changed, 109 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/i2c/nxp,pca9641.yaml

diff --git a/Documentation/devicetree/bindings/i2c/nxp,pca9641.yaml b/Documentation/devicetree/bindings/i2c/nxp,pca9641.yaml
new file mode 100644
index 000000000000..649a3f6d1776
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/nxp,pca9641.yaml
@@ -0,0 +1,109 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/i2c/nxp,pca9641.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: NXP PCA9641 2-to-1 I2C bus master arbiter
+
+maintainers:
+  - Shiv Prakash Gupta <shivprakash.gupta@nxp.com>
+
+description: |
+  The PCA9641 is a 2-to-1 I2C bus master arbiter that manages two upstream
+  I2C masters competing for a single downstream slave bus. It uses a
+  lock/grant ownership model: a master requests the downstream bus by setting
+  LOCK_REQ, waits for the arbiter to assert LOCK_GRANT, then explicitly
+  connects to the bus via BUS_CONNECT before issuing transactions.
+
+  Key features compared to the PCA9541:
+    - Lock/grant ownership model (LOCK_REQ + LOCK_GRANT bits in CONTR register)
+    - BUS_CONNECT bit must be set explicitly after receiving LOCK_GRANT
+    - Reserve Time register (RT): guarantees bus ownership for 1-255 ms
+    - INT0 and INT1 interrupt outputs (one per upstream master) and INT_IN
+      interrupt input that propagates downstream slave interrupts upstream
+    - 16-bit shared mailbox (MB_LO + MB_HI) for inter-master communication
+    - ID register (read-only, value 0x38) to distinguish from PCA9541
+    - Four address pins (AD0-AD3) allowing up to 112 unique I2C addresses
+
+properties:
+  compatible:
+    const: nxp,pca9641
+
+  reg:
+    maxItems: 1
+    description:
+      7-bit I2C slave address of the PCA9641 on the upstream bus. The address
+      is set by hardware pins AD0-AD3 at power-on or hardware reset.
+
+  interrupts:
+    maxItems: 1
+    description:
+      Optional interrupt from the INT0 or INT1 output pin. When provided the
+      driver uses interrupt-assisted arbitration (waits on LOCK_GRANT interrupt)
+      instead of polling the CONTR register. Either INT0 or INT1 can be
+      connected depending on which upstream master port is used.
+
+  i2c-arb:
+    type: object
+    $ref: /schemas/i2c/i2c-controller.yaml
+    unevaluatedProperties: false
+    description:
+      I2C bus node representing the downstream slave bus controlled by the
+      PCA9641. Downstream slave devices are declared as child nodes here.
+
+required:
+  - compatible
+  - reg
+  - i2c-arb
+
+additionalProperties: false
+
+examples:
+  - |
+    /* Minimal example: polling mode (no interrupt wiring) */
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        i2c-arbiter@74 {
+            compatible = "nxp,pca9641";
+            reg = <0x74>;
+
+            i2c-arb {
+                #address-cells = <1>;
+                #size-cells = <0>;
+
+                eeprom@50 {
+                    compatible = "atmel,24c32";
+                    reg = <0x50>;
+                };
+            };
+        };
+    };
+
+  - |
+    /* Interrupt mode: INT0 wired to SoC GPIO */
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        i2c-arbiter@70 {
+            compatible = "nxp,pca9641";
+            reg = <0x70>;
+            interrupt-parent = <&gpio1>;
+            interrupts = <5 IRQ_TYPE_EDGE_FALLING>;
+
+            i2c-arb {
+                #address-cells = <1>;
+                #size-cells = <0>;
+
+                temperature-sensor@48 {
+                    compatible = "national,lm75";
+                    reg = <0x48>;
+                };
+            };
+        };
+    };
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH v2 2/2] i2c: mux: Add NXP PCA9641 I2C bus master arbiter driver
  2026-09-17  8:58 [PATCH v2 0/2] Add NXP PCA9641 I2C bus master arbiter support Shiv Prakash Gupta
  2026-09-17  8:58 ` [PATCH v2 1/2] dt-bindings: i2c: Add NXP PCA9641 I2C bus master arbiter Shiv Prakash Gupta
@ 2026-09-17  8:58 ` Shiv Prakash Gupta
  2026-09-17  9:10   ` sashiko-bot
  1 sibling, 1 reply; 4+ messages in thread
From: Shiv Prakash Gupta @ 2026-09-17  8:58 UTC (permalink / raw)
  To: andi.shyti, robh, krzk+dt, conor+dt, peda, linux-i2c, devicetree,
	linux-kernel
  Cc: vikash.bansal, priyanka.jain, Shiv Prakash Gupta

Add a driver for the NXP PCA9641 2-to-1 I2C bus master arbiter.

The PCA9641 allows two upstream I2C masters to share a single downstream
slave bus using a lock/grant protocol: a master asserts LOCK_REQ, waits
for LOCK_GRANT, sets BUS_CONNECT to open the switch, performs transactions,
then clears LOCK_REQ to release the bus.

The driver supports interrupt-assisted arbitration when INT0/INT1 is
connected to a GPIO. A threaded IRQ handler signals a completion variable
that select_chan() waits on, avoiding busy-polling. Polling mode is used
when no interrupt is configured.

Signed-off-by: Shiv Prakash Gupta <shivprakash.gupta@nxp.com>
---
Changes in v2:
- Drop IRQF_SHARED from the threaded IRQ request.
- Clear the latched LOCK_REQ on every timeout path in
  pca9641_select_chan() so a master that gives up does not block the
  peer master.
- Reorder probe interrupt setup: mask all sources and clear stale
  INT_STATUS before requesting the IRQ, then unmask LOCK_GRANT and
  BUS_LOST.
- Re-mask interrupts if i2c_mux_add_adapter() fails.
- Unregister the child adapter before masking interrupts in
  pca9641_remove().
- Use a named initializer for the i2c_device_id table.

 MAINTAINERS                         |   7 +
 drivers/i2c/muxes/Kconfig           |  15 +
 drivers/i2c/muxes/Makefile          |   1 +
 drivers/i2c/muxes/i2c-mux-pca9641.c | 420 ++++++++++++++++++++++++++++
 4 files changed, 443 insertions(+)
 create mode 100644 drivers/i2c/muxes/i2c-mux-pca9641.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 6215fcb07770..2b02090edc9e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -12250,6 +12250,13 @@ S:	Maintained
 F:	Documentation/i2c/busses/i2c-nvidia-gpu.rst
 F:	drivers/i2c/busses/i2c-nvidia-gpu.c
 
+NXP PCA9641 I2C BUS MASTER ARBITER DRIVER
+M:	Shiv Prakash Gupta <shivprakash.gupta@nxp.com>
+L:	linux-i2c@vger.kernel.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/i2c/nxp,pca9641.yaml
+F:	drivers/i2c/muxes/i2c-mux-pca9641.c
+
 I2C MUXES
 M:	Peter Rosin <peda@lysator.liu.se>
 L:	linux-i2c@vger.kernel.org
diff --git a/drivers/i2c/muxes/Kconfig b/drivers/i2c/muxes/Kconfig
index 6d2f66810cdc..0caff9592466 100644
--- a/drivers/i2c/muxes/Kconfig
+++ b/drivers/i2c/muxes/Kconfig
@@ -64,6 +64,21 @@ config I2C_MUX_PCA9541
 	  This driver can also be built as a module.  If so, the module
 	  will be called i2c-mux-pca9541.
 
+config I2C_MUX_PCA9641
+	tristate "NXP PCA9641 I2C Master Arbiter"
+	help
+	  If you say yes here you get support for the NXP PCA9641
+	  2-to-1 I2C bus master arbiter.
+
+	  The PCA9641 arbitrates between two upstream I2C masters competing
+	  for a single downstream slave bus. It implements a lock/grant
+	  ownership model with an optional reserve time window, hardware
+	  interrupt outputs (INT0/INT1), and a 16-bit shared mailbox for
+	  inter-master communication.
+
+	  This driver can also be built as a module.  If so, the module
+	  will be called i2c-mux-pca9641.
+
 config I2C_MUX_PCA954x
 	tristate "NXP PCA954x/PCA984x and Maxim MAX735x/MAX736x I2C Mux/switches"
 	depends on GPIOLIB || COMPILE_TEST
diff --git a/drivers/i2c/muxes/Makefile b/drivers/i2c/muxes/Makefile
index 4b24f49515a7..a50df4013224 100644
--- a/drivers/i2c/muxes/Makefile
+++ b/drivers/i2c/muxes/Makefile
@@ -12,6 +12,7 @@ obj-$(CONFIG_I2C_MUX_LTC4306)	+= i2c-mux-ltc4306.o
 obj-$(CONFIG_I2C_MUX_MLXCPLD)	+= i2c-mux-mlxcpld.o
 obj-$(CONFIG_I2C_MUX_MULE)	+= i2c-mux-mule.o
 obj-$(CONFIG_I2C_MUX_PCA9541)	+= i2c-mux-pca9541.o
+obj-$(CONFIG_I2C_MUX_PCA9641)	+= i2c-mux-pca9641.o
 obj-$(CONFIG_I2C_MUX_PCA954x)	+= i2c-mux-pca954x.o
 obj-$(CONFIG_I2C_MUX_PINCTRL)	+= i2c-mux-pinctrl.o
 obj-$(CONFIG_I2C_MUX_REG)	+= i2c-mux-reg.o
diff --git a/drivers/i2c/muxes/i2c-mux-pca9641.c b/drivers/i2c/muxes/i2c-mux-pca9641.c
new file mode 100644
index 000000000000..a37820cf6c2e
--- /dev/null
+++ b/drivers/i2c/muxes/i2c-mux-pca9641.c
@@ -0,0 +1,420 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * I2C multiplexer driver for PCA9641 2-to-1 I2C bus master arbiter
+ *
+ * Copyright (C) 2026 Shiv Prakash Gupta <shivprakash.gupta@nxp.com>
+ *
+ * Based on i2c-mux-pca9541.c by Guenter Roeck <linux@roeck-us.net>
+ *
+ * Datasheet: https://www.nxp.com/docs/en/data-sheet/PCA9641.pdf
+ */
+
+#include <linux/bitops.h>
+#include <linux/completion.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/i2c.h>
+#include <linux/i2c-mux.h>
+#include <linux/interrupt.h>
+#include <linux/jiffies.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+
+/* Register addresses, selected via the 3 LSBs of the command code byte */
+#define PCA9641_ID          0x00  /* Device ID register (R only, value 0x38) */
+#define PCA9641_CONTR       0x01  /* Control register (R/W) */
+#define PCA9641_STATUS      0x02  /* Status register (R/W) */
+#define PCA9641_INT_STATUS  0x04  /* Interrupt Status register (R/W, sticky) */
+#define PCA9641_INT_MSK     0x05  /* Interrupt Mask register (R/W) */
+
+/* CONTR register (0x01) bit definitions */
+#define PCA9641_CTL_LOCK_REQ        BIT(0)  /* Request downstream bus ownership */
+#define PCA9641_CTL_LOCK_GRANT      BIT(1)  /* Downstream bus granted (R only) */
+#define PCA9641_CTL_BUS_CONNECT     BIT(2)  /* Connect master to downstream bus */
+
+/* STATUS register (0x02) bit definitions */
+#define PCA9641_STA_OTHER_LOCK      BIT(0)  /* Other master currently holds the lock (R only) */
+
+/* INT_STATUS register (0x04) bit definitions (sticky, clear by writing 1) */
+#define PCA9641_INTS_BUS_LOST       BIT(1)  /* This master involuntarily lost the bus */
+#define PCA9641_INTS_LOCK_GRANT     BIT(2)  /* This master was granted the bus */
+
+/* INT_MSK register (0x05) bit definitions (1 = masked/disabled, POR = 0x7F) */
+#define PCA9641_MSK_BUS_LOST        BIT(1)
+#define PCA9641_MSK_LOCK_GRANT      BIT(2)
+
+/* POR value of INT_MSK: all interrupt sources masked */
+#define PCA9641_INT_MSK_ALL         0x7Fu
+
+/*
+ * Interrupt mask value that enables only LOCK_GRANT and BUS_LOST.
+ * All other sources remain masked (bit = 1 means masked).
+ */
+#define PCA9641_INT_MSK_ARB \
+	(PCA9641_INT_MSK_ALL & ~(PCA9641_MSK_LOCK_GRANT | PCA9641_MSK_BUS_LOST))
+
+/* Sticky INT_STATUS bits used by the arbitration logic (write 1 to clear) */
+#define PCA9641_INTS_ARB \
+	(PCA9641_INTS_LOCK_GRANT | PCA9641_INTS_BUS_LOST)
+
+/* Value in ID register that uniquely identifies PCA9641 (vs. PCA9541) */
+#define PCA9641_ID_MAGIC            0x38u
+
+/* Arbitration retry delays (microseconds) */
+#define PCA9641_DELAY_SHORT         50u
+#define PCA9641_DELAY_LONG          1000u
+
+/**
+ * struct pca9641 - per-device driver state
+ * @client:          I2C client for the arbiter device
+ * @select_timeout:  Current polling retry delay in microseconds
+ * @irq:             Linux IRQ number for INT0/INT1 GPIO, or -1 if not used
+ * @lock_grant_comp: Completion signaled from the IRQ handler on LOCK_GRANT
+ */
+struct pca9641 {
+	struct i2c_client *client;
+	unsigned long select_timeout;
+	int irq;
+	struct completion lock_grant_comp;
+};
+
+static const struct i2c_device_id pca9641_id[] = {
+	{ .name = "pca9641" },
+	{}
+};
+MODULE_DEVICE_TABLE(i2c, pca9641_id);
+
+static const struct of_device_id pca9641_of_match[] = {
+	{ .compatible = "nxp,pca9641" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, pca9641_of_match);
+
+/*
+ * Write to chip register. Don't use i2c_transfer()/i2c_smbus_xfer()
+ * as they will try to lock the adapter a second time.
+ */
+static int pca9641_reg_write(struct i2c_client *client, u8 reg, u8 val)
+{
+	union i2c_smbus_data data = { .byte = val };
+
+	return __i2c_smbus_xfer(client->adapter, client->addr, client->flags,
+				I2C_SMBUS_WRITE, reg,
+				I2C_SMBUS_BYTE_DATA, &data);
+}
+
+/*
+ * Read from chip register. Don't use i2c_transfer()/i2c_smbus_xfer()
+ * as they will try to lock adapter a second time.
+ */
+static int pca9641_reg_read(struct i2c_client *client, u8 reg)
+{
+	union i2c_smbus_data data;
+	int ret;
+
+	ret = __i2c_smbus_xfer(client->adapter, client->addr, client->flags,
+			       I2C_SMBUS_READ, reg,
+			       I2C_SMBUS_BYTE_DATA, &data);
+
+	return ret ? ret : (int)data.byte;
+}
+
+/*
+ * Release bus ownership. Writing 0 clears LOCK_REQ (and BUS_CONNECT),
+ * which lets the arbiter grant the downstream bus to the other master.
+ */
+static void pca9641_release_bus(struct i2c_client *client)
+{
+	pca9641_reg_write(client, PCA9641_CONTR, 0x00);
+}
+
+/*
+ * Threaded IRQ handler for INT0/INT1. Signals the completion waited on by
+ * pca9641_select_chan(). No I2C access here to avoid deadlock on the adapter
+ * mutex held by select_chan().
+ */
+static irqreturn_t pca9641_irq_handler(int irq, void *dev_id)
+{
+	struct pca9641 *data = dev_id;
+
+	complete(&data->lock_grant_comp);
+	return IRQ_HANDLED;
+}
+
+/*
+ * Arbitration management. Asserts LOCK_REQ and checks LOCK_GRANT to acquire
+ * the downstream bus. Returns 1 when acquired, 0 to retry, <0 on error.
+ */
+static int pca9641_arbitrate(struct i2c_client *client)
+{
+	struct i2c_mux_core *muxc = i2c_get_clientdata(client);
+	struct pca9641 *data = i2c_mux_priv(muxc);
+	int ctrl, status, ret;
+
+	ctrl = pca9641_reg_read(client, PCA9641_CONTR);
+	if (ctrl < 0)
+		return ctrl;
+
+	if (ctrl & PCA9641_CTL_LOCK_GRANT) {
+		if (!(ctrl & PCA9641_CTL_BUS_CONNECT)) {
+			ret = pca9641_reg_write(client, PCA9641_CONTR,
+						(u8)(ctrl | PCA9641_CTL_BUS_CONNECT));
+			if (ret < 0)
+				return ret;
+		}
+		return 1;
+	}
+
+	if (!(ctrl & PCA9641_CTL_LOCK_REQ)) {
+		ret = pca9641_reg_write(client, PCA9641_CONTR,
+					(u8)(ctrl | PCA9641_CTL_LOCK_REQ));
+		if (ret < 0)
+			return ret;
+		data->select_timeout = PCA9641_DELAY_SHORT;
+		return 0;
+	}
+
+	status = pca9641_reg_read(client, PCA9641_STATUS);
+	if (status < 0)
+		return status;
+
+	data->select_timeout = (status & PCA9641_STA_OTHER_LOCK) ?
+				PCA9641_DELAY_LONG : PCA9641_DELAY_SHORT;
+
+	return 0;
+}
+
+/*
+ * Acquire the downstream bus before a transaction. Uses interrupt mode
+ * if IRQ is available, polling otherwise.
+ */
+static int pca9641_select_chan(struct i2c_mux_core *muxc, u32 chan)
+{
+	struct pca9641 *data = i2c_mux_priv(muxc);
+	struct i2c_client *client = data->client;
+	unsigned long timeout = jiffies + 2 * client->adapter->timeout;
+	int ctrl, ret;
+
+	if (data->irq > 0) {
+		reinit_completion(&data->lock_grant_comp);
+
+		ctrl = pca9641_reg_read(client, PCA9641_CONTR);
+		if (ctrl < 0)
+			return ctrl;
+
+		if (ctrl & PCA9641_CTL_LOCK_GRANT)
+			goto set_bus_connect;
+
+		ret = pca9641_reg_write(client, PCA9641_CONTR,
+					(u8)((ctrl & ~PCA9641_CTL_BUS_CONNECT) |
+					     PCA9641_CTL_LOCK_REQ));
+		if (ret < 0)
+			return ret;
+
+		if (!wait_for_completion_timeout(&data->lock_grant_comp,
+						 client->adapter->timeout)) {
+			ctrl = pca9641_reg_read(client, PCA9641_CONTR);
+			if (ctrl < 0)
+				return ctrl;
+			if (!(ctrl & PCA9641_CTL_LOCK_GRANT)) {
+				dev_warn(&client->dev,
+					 "Timed out waiting for bus grant\n");
+				/*
+				 * Clear our latched LOCK_REQ so the arbiter is
+				 * not left holding a request for a master that
+				 * has given up, which would block the peer.
+				 */
+				pca9641_release_bus(client);
+				return -ETIMEDOUT;
+			}
+			goto set_bus_connect;
+		}
+
+		ctrl = pca9641_reg_read(client, PCA9641_CONTR);
+		if (ctrl < 0)
+			return ctrl;
+
+		if (!(ctrl & PCA9641_CTL_LOCK_GRANT)) {
+			dev_warn(&client->dev,
+				 "Interrupt fired but LOCK_GRANT not set\n");
+			pca9641_release_bus(client);
+			return -ETIMEDOUT;
+		}
+
+set_bus_connect:
+		(void)pca9641_reg_write(client, PCA9641_INT_STATUS,
+					PCA9641_INTS_ARB);
+
+		if (!(ctrl & PCA9641_CTL_BUS_CONNECT)) {
+			ret = pca9641_reg_write(client, PCA9641_CONTR,
+						(u8)(ctrl | PCA9641_CTL_BUS_CONNECT));
+			if (ret < 0)
+				return ret;
+		}
+		return 0;
+	}
+
+	do {
+		ret = pca9641_arbitrate(client);
+		if (ret)
+			return ret < 0 ? ret : 0;
+
+		if (data->select_timeout <= PCA9641_DELAY_SHORT)
+			udelay(data->select_timeout);
+		else
+			msleep(data->select_timeout / 1000);
+	} while (time_is_after_eq_jiffies(timeout));
+
+	dev_warn(&client->dev, "Failed to acquire I2C bus, timed out\n");
+	/* Drop our latched LOCK_REQ so the peer master is not blocked. */
+	pca9641_release_bus(client);
+	return -ETIMEDOUT;
+}
+
+/* Release the downstream bus after a transaction completes. */
+static int pca9641_release_chan(struct i2c_mux_core *muxc, u32 chan)
+{
+	struct pca9641 *data = i2c_mux_priv(muxc);
+
+	pca9641_release_bus(data->client);
+	return 0;
+}
+
+static int pca9641_probe(struct i2c_client *client)
+{
+	struct i2c_adapter *adap = client->adapter;
+	struct i2c_mux_core *muxc;
+	struct pca9641 *data;
+	int id, ret;
+
+	if (!i2c_check_functionality(adap, I2C_FUNC_SMBUS_BYTE_DATA))
+		return -ENODEV;
+
+	id = i2c_smbus_read_byte_data(client, PCA9641_ID);
+	if (id < 0) {
+		dev_err(&client->dev, "Failed to read device ID: %d\n", id);
+		return id;
+	}
+	if ((u8)id != PCA9641_ID_MAGIC) {
+		dev_err(&client->dev,
+			"Unexpected device ID 0x%02x (expected 0x%02x for PCA9641)\n",
+			(u8)id, PCA9641_ID_MAGIC);
+		return -ENODEV;
+	}
+
+	/* Clear any stale bus ownership from a previous crash. */
+	i2c_lock_bus(adap, I2C_LOCK_SEGMENT);
+	pca9641_release_bus(client);
+	i2c_unlock_bus(adap, I2C_LOCK_SEGMENT);
+
+	muxc = i2c_mux_alloc(adap, &client->dev, 1, sizeof(*data),
+			     I2C_MUX_ARBITRATOR,
+			     pca9641_select_chan, pca9641_release_chan);
+	if (!muxc)
+		return -ENOMEM;
+
+	data = i2c_mux_priv(muxc);
+	data->client = client;
+	data->irq = -1;
+	init_completion(&data->lock_grant_comp);
+
+	i2c_set_clientdata(client, muxc);
+
+	/* Optional interrupt mode; fall back to polling on failure. */
+	if (client->irq > 0) {
+		/*
+		 * Start from a known state: mask all sources and clear any
+		 * stale sticky status before the handler is registered.
+		 */
+		ret = i2c_smbus_write_byte_data(client, PCA9641_INT_MSK,
+						PCA9641_INT_MSK_ALL);
+		if (ret < 0) {
+			dev_warn(&client->dev,
+				 "Failed to mask interrupts (%d); using polling\n",
+				 ret);
+			goto add_adapter;
+		}
+		i2c_smbus_write_byte_data(client, PCA9641_INT_STATUS,
+					  PCA9641_INTS_ARB);
+
+		/*
+		 * Register the handler before unmasking the device so a
+		 * pending interrupt cannot fire into an unregistered line.
+		 */
+		ret = devm_request_threaded_irq(&client->dev, client->irq,
+						NULL, pca9641_irq_handler,
+						IRQF_ONESHOT,
+						dev_name(&client->dev), data);
+		if (ret < 0) {
+			dev_warn(&client->dev,
+				 "Failed to request IRQ %d (%d); using polling\n",
+				 client->irq, ret);
+			goto add_adapter;
+		}
+
+		/* Handler is ready; now enable LOCK_GRANT + BUS_LOST. */
+		ret = i2c_smbus_write_byte_data(client, PCA9641_INT_MSK,
+						PCA9641_INT_MSK_ARB);
+		if (ret < 0) {
+			dev_warn(&client->dev,
+				 "Failed to enable interrupts (%d); using polling\n",
+				 ret);
+			goto add_adapter;
+		}
+
+		data->irq = client->irq;
+		dev_dbg(&client->dev,
+			"Interrupt mode enabled on IRQ %d\n",
+			client->irq);
+	}
+
+add_adapter:
+	ret = i2c_mux_add_adapter(muxc, 0, 0);
+	if (ret) {
+		/* Re-mask so a left-enabled source cannot storm the line. */
+		if (data->irq > 0)
+			i2c_smbus_write_byte_data(client, PCA9641_INT_MSK,
+						  PCA9641_INT_MSK_ALL);
+		return ret;
+	}
+
+	dev_info(&client->dev,
+		 "PCA9641 I2C master arbiter registered (I2C bus %s, %s mode)\n",
+		 client->name,
+		 data->irq > 0 ? "interrupt" : "polling");
+
+	return 0;
+}
+
+static void pca9641_remove(struct i2c_client *client)
+{
+	struct i2c_mux_core *muxc = i2c_get_clientdata(client);
+	struct pca9641 *data = i2c_mux_priv(muxc);
+
+	/*
+	 * Unregister the child adapter first so any in-flight downstream
+	 * transaction can finish arbitration before the interrupt that
+	 * drives it is masked.
+	 */
+	i2c_mux_del_adapters(muxc);
+
+	if (data->irq > 0)
+		(void)i2c_smbus_write_byte_data(client, PCA9641_INT_MSK,
+						PCA9641_INT_MSK_ALL);
+}
+
+static struct i2c_driver pca9641_driver = {
+	.driver = {
+		.name		= "pca9641",
+		.of_match_table	= pca9641_of_match,
+	},
+	.probe		= pca9641_probe,
+	.remove		= pca9641_remove,
+	.id_table	= pca9641_id,
+};
+
+module_i2c_driver(pca9641_driver);
+
+MODULE_AUTHOR("Shiv Prakash Gupta <shivprakash.gupta@nxp.com>");
+MODULE_DESCRIPTION("PCA9641 2-to-1 I2C bus master arbiter driver");
+MODULE_LICENSE("GPL");
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v2 2/2] i2c: mux: Add NXP PCA9641 I2C bus master arbiter driver
  2026-09-17  8:58 ` [PATCH v2 2/2] i2c: mux: Add NXP PCA9641 I2C bus master arbiter driver Shiv Prakash Gupta
@ 2026-09-17  9:10   ` sashiko-bot
  0 siblings, 0 replies; 4+ messages in thread
From: sashiko-bot @ 2026-09-17  9:10 UTC (permalink / raw)
  To: Shiv Prakash Gupta; +Cc: robh, conor+dt, devicetree

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] i2c: pca9641: missing interrupt clear in handler causes IRQ storm
- [High] i2c: pca9641: probe error path leaves IRQ active during polling fallback
- [High] i2c: pca9641: select_chan() illegally sleeps in atomic context
--

commit 1eafadafa36c19cfd93cd95319667d5c31725ee1
Author: Shiv Prakash Gupta <shivprakash.gupta@nxp.com>

i2c: mux: Add NXP PCA9641 I2C bus master arbiter driver

This patch adds a driver for the NXP PCA9641 2-to-1 I2C bus master arbiter.
It introduces support for interrupt-assisted arbitration to avoid busy-polling,
as well as a fallback to polling mode when interrupts are not configured.

> diff --git a/drivers/i2c/muxes/i2c-mux-pca9641.c b/drivers/i2c/muxes/i2c-mux-pca9641.c
> new file mode 100644
> index 0000000000000..6e6f29fdd6d49
> --- /dev/null
> +++ b/drivers/i2c/muxes/i2c-mux-pca9641.c
[ ... ]
> +static irqreturn_t pca9641_irq_handler(int irq, void *dev_id)
> +{
> +	struct pca9641 *data = dev_id;
> +
> +	complete(&data->lock_grant_comp);
> +	return IRQ_HANDLED;
> +}

[Severity: High]
Does this threaded handler need to clear the device-level sticky interrupt
status before returning IRQ_HANDLED?

If the physical line remains asserted because the device-level interrupt
status isn't cleared here, returning IRQ_HANDLED will cause the generic IRQ
core to unmask the line, immediately triggering another hardware interrupt.
This would set up an infinite loop of RT-priority threaded handler executions,
starving the normal-priority task running pca9641_select_chan() and
potentially livelocking the system.

[ ... ]
> +static int pca9641_select_chan(struct i2c_mux_core *muxc, u32 chan)
> +{
> +	struct pca9641 *data = i2c_mux_priv(muxc);
> +	struct i2c_client *client = data->client;
> +	unsigned long timeout = jiffies + 2 * client->adapter->timeout;
> +	int ctrl, ret;
> +
> +	if (data->irq > 0) {
> +		reinit_completion(&data->lock_grant_comp);
> +
> +		ctrl = pca9641_reg_read(client, PCA9641_CONTR);
> +		if (ctrl < 0)
> +			return ctrl;
> +
> +		if (ctrl & PCA9641_CTL_LOCK_GRANT)
> +			goto set_bus_connect;
> +
> +		ret = pca9641_reg_write(client, PCA9641_CONTR,
> +					(u8)((ctrl & ~PCA9641_CTL_BUS_CONNECT) |
> +					     PCA9641_CTL_LOCK_REQ));
> +		if (ret < 0)
> +			return ret;
> +
> +		if (!wait_for_completion_timeout(&data->lock_grant_comp,
> +						 client->adapter->timeout)) {

[Severity: High]
Can this unconditionally call a sleeping synchronization primitive in atomic
context?

When an atomic I2C transfer is requested, the i2c-mux core executes in atomic
context with interrupts disabled and calls pca9641_select_chan(). Calling
wait_for_completion_timeout() here invokes schedule(), which explicitly
sleeps. This violates context constraints and would trigger a scheduling while
atomic bug.

> +			ctrl = pca9641_reg_read(client, PCA9641_CONTR);
> +			if (ctrl < 0)
> +				return ctrl;
> +			if (!(ctrl & PCA9641_CTL_LOCK_GRANT)) {
> +				dev_warn(&client->dev,
> +					 "Timed out waiting for bus grant\n");
> +				/*
> +				 * Clear our latched LOCK_REQ so the arbiter is
> +				 * not left holding a request for a master that
> +				 * has given up, which would block the peer.
> +				 */
> +				pca9641_release_bus(client);
> +				return -ETIMEDOUT;
> +			}

[Severity: High]
Does this error path return without clearing the pending interrupt status?

If so, it would leave the unmasked line asserted, which also guarantees an
immediate IRQ storm.

[ ... ]
> +	do {
> +		ret = pca9641_arbitrate(client);
> +		if (ret)
> +			return ret < 0 ? ret : 0;
> +
> +		if (data->select_timeout <= PCA9641_DELAY_SHORT)
> +			udelay(data->select_timeout);
> +		else
> +			msleep(data->select_timeout / 1000);

[Severity: High]
Is it safe to unconditionally call msleep() here?

During an atomic I2C transfer, calling msleep() would sleep while interrupts
are disabled, violating context constraints. Additionally, this polling loop
relies on time_is_after_eq_jiffies(), but jiffies do not advance when timer
interrupts are disabled, leading to an infinite spin loop.

Could the driver check i2c_in_atomic_xfer_mode() and implement a safe delay
loop using udelay() for atomic context?

> +	} while (time_is_after_eq_jiffies(timeout));
> +
> +	dev_warn(&client->dev, "Failed to acquire I2C bus, timed out\n");
> +	/* Drop our latched LOCK_REQ so the peer master is not blocked. */
> +	pca9641_release_bus(client);
> +	return -ETIMEDOUT;
> +}
[ ... ]
> +static int pca9641_probe(struct i2c_client *client)
> +{
[ ... ]
> +		/* Handler is ready; now enable LOCK_GRANT + BUS_LOST. */
> +		ret = i2c_smbus_write_byte_data(client, PCA9641_INT_MSK,
> +						PCA9641_INT_MSK_ARB);

[Severity: High]
Does this unmask the BUS_LOST interrupt, which doesn't seem to be handled
anywhere in the driver?

If BUS_LOST fires, returning from the interrupt handler without clearing it
would guarantee an immediate and permanent IRQ storm.

> +		if (ret < 0) {
> +			dev_warn(&client->dev,
> +				 "Failed to enable interrupts (%d); using polling\n",
> +				 ret);
> +			goto add_adapter;
> +		}

[Severity: High]
Does this error path fall back to polling mode without freeing the previously
requested threaded IRQ handler?

If the hardware later asserts the IRQ line (e.g. from spurious noise or
because it was already enabled prior to probe), the active handler will execute
and return IRQ_HANDLED. Because pca9641_select_chan() operates in polling mode
(data->irq == -1) and never waits on the completion, the interrupt status is
never cleared, triggering a permanent IRQ storm and system lockup.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260917085852.552079-1-shivprakash.gupta@nxp.com?part=2

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-09-17  9:10 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-17  8:58 [PATCH v2 0/2] Add NXP PCA9641 I2C bus master arbiter support Shiv Prakash Gupta
2026-09-17  8:58 ` [PATCH v2 1/2] dt-bindings: i2c: Add NXP PCA9641 I2C bus master arbiter Shiv Prakash Gupta
2026-09-17  8:58 ` [PATCH v2 2/2] i2c: mux: Add NXP PCA9641 I2C bus master arbiter driver Shiv Prakash Gupta
2026-09-17  9:10   ` sashiko-bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox