[PATCH 0/2] i2c: mux: Add NXP PCA9641 I2C bus master arbiter

[PATCH 0/2] i2c: mux: Add NXP PCA9641 I2C bus master arbiter

[Shiv Prakash Gupta]

This series adds support 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 hardware lock/grant arbitration protocol. It provides
dedicated interrupt outputs (INT0/INT1) per upstream master and a 16-bit
inter-master mailbox.

The driver supports two arbitration modes:
- Interrupt mode: uses INT0/INT1 GPIO interrupt, with automatic fallback
  to polling when no IRQ is configured
- Polling mode: used when no interrupt is configured

Patch 1 adds the devicetree binding schema.
Patch 2 adds the driver with Kconfig, Makefile, and MAINTAINERS entries.

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           | 395 ++++++++++++++++++
 5 files changed, 527 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/i2c/nxp,pca9641.yaml
 create mode 100644 drivers/i2c/muxes/i2c-mux-pca9641.c

-- 
2.34.1

[PATCH 1/2] dt-bindings: i2c: Add NXP PCA9641 I2C bus master arbiter

[Shiv Prakash Gupta]

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>
---
 .../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

Re: [PATCH 1/2] dt-bindings: i2c: Add NXP PCA9641 I2C bus master arbiter

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The PCA9641 acts as an interrupt controller for downstream devices by propagating interrupts, but the binding omits the required `interrupt-controller` and `#interrupt-cells` properties.
--

commit f66b0c3d86e4ae950a20caea6a9eb2ad2243149c
Author: Shiv Prakash Gupta <shivprakash.gupta@nxp.com>
Subject: dt-bindings: i2c: Add NXP PCA9641 I2C bus master arbiter

This commit adds a device tree binding schema for the NXP PCA9641 2-to-1 I2C
bus master arbiter. The PCA9641 arbitrates between two upstream I2C masters
using a lock/grant model and supports an optional interrupts property for
interrupt-assisted arbitration.

diff --git a/Documentation/devicetree/bindings/i2c/nxp,pca9641.yaml b/Documentation/devicetree/bindings/i2c/nxp,pca9641.yaml
new file mode 100644
index 0000000000000..649a3f6d1776f
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/nxp,pca9641.yaml
[ ... ]
> properties:
>   compatible:
>     const: nxp,pca9641

[Severity: Medium]
Since the commit message notes the PCA9641 has an INT_IN interrupt input that
propagates downstream slave interrupts upstream, it appears to act as an
interrupt controller. 

Should the interrupt-controller and #interrupt-cells properties be defined here
to expose the hardware's capabilities to schema validation?

[ ... ]
> required:
>   - compatible
>   - reg
>   - i2c-arb
> 
> additionalProperties: false

[Severity: Medium]
If a downstream I2C device correctly specifies interrupt-parent = <&i2c_arbiter>
to route its interrupts through the PCA9641, would the device tree fail schema
validation because additionalProperties: false is set and the interrupt
controller properties are omitted?

Should the interrupt-controller and #interrupt-cells properties be included in
this required block to ensure nodes are correctly validated?

> 
> examples:
>   - |
>     /* Minimal example: polling mode (no interrupt wiring) */

[Severity: Medium]
If the interrupt controller properties are added to the required block, would
these examples also need to be updated to include them so the examples can
continue to pass dt_binding_check?

>     i2c {
>         #address-cells = <1>;
[ ... ]

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

Re: [PATCH 1/2] dt-bindings: i2c: Add NXP PCA9641 I2C bus master arbiter

[Conor Dooley]

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

On Tue, Sep 08, 2026 at 06:01:45PM +0530, Shiv Prakash Gupta wrote:
> 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>
> ---
>  .../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.

Why does this node need to exist? Can't the i2c-arbiter node itself
contain the child devices?

Thanks,
Conor.

> +
> +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
> 

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

Re: [PATCH 1/2] dt-bindings: i2c: Add NXP PCA9641 I2C bus master arbiter

[Peter Rosin]

Den Tue, Sep 08, 2026 at 06:47:01PM +0100, skrev Conor Dooley:
> On Tue, Sep 08, 2026 at 06:01:45PM +0530, Shiv Prakash Gupta wrote:
> > 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>
> > ---

...

> > +  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.
> 
> Why does this node need to exist? Can't the i2c-arbiter node itself
> contain the child devices?

For symmetry with the pre-existing I2C arbs, which all have the extra
node for historical implementation reasons? I don't know if that's
considered a valid reason? I prefer to have all arbs follow the same
structure...

The "documentation" describing common I2C arbitration details went missing
here:

https://lore.kernel.org/all/20230731163833.319258-1-krzysztof.kozlowski@linaro.org/

Cheers,
Peter

Re: [PATCH 1/2] dt-bindings: i2c: Add NXP PCA9641 I2C bus master arbiter

[Conor Dooley]

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

On Wed, Sep 09, 2026 at 07:25:38AM +0200, Peter Rosin wrote:
> Den Tue, Sep 08, 2026 at 06:47:01PM +0100, skrev Conor Dooley:
> > On Tue, Sep 08, 2026 at 06:01:45PM +0530, Shiv Prakash Gupta wrote:
> > > 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>
> > > ---
> 
> ...
> 
> > > +  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.
> > 
> > Why does this node need to exist? Can't the i2c-arbiter node itself
> > contain the child devices?
> 
> For symmetry with the pre-existing I2C arbs, which all have the extra
> node for historical implementation reasons? I don't know if that's
> considered a valid reason? I prefer to have all arbs follow the same
> structure...

If this is just how these devices are described in general, I think
that's probably fine.

Acked-by: Conor Dooley <conor.dooley@microchip.com>
pw-bot: not-applicable

(Also, good to see you back active)

Cheers.,
Conor.

> 
> The "documentation" describing common I2C arbitration details went missing
> here:
> 
> https://lore.kernel.org/all/20230731163833.319258-1-krzysztof.kozlowski@linaro.org/
> 
> Cheers,
> Peter

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

[PATCH 2/2] i2c: mux: Add NXP PCA9641 I2C bus master arbiter driver

[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>
---
 MAINTAINERS                         |   7 +
 drivers/i2c/muxes/Kconfig           |  15 ++
 drivers/i2c/muxes/Makefile          |   1 +
 drivers/i2c/muxes/i2c-mux-pca9641.c | 395 ++++++++++++++++++++++++++++
 4 files changed, 418 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..c12aa88dcf56
--- /dev/null
+++ b/drivers/i2c/muxes/i2c-mux-pca9641.c
@@ -0,0 +1,395 @@
+// 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_RT          0x03  /* Reserve Time register (R/W) */
+#define PCA9641_INT_STATUS  0x04  /* Interrupt Status register (R/W, sticky) */
+#define PCA9641_INT_MSK     0x05  /* Interrupt Mask register (R/W) */
+#define PCA9641_MB_LO       0x06  /* Mailbox low byte (R/W) */
+#define PCA9641_MB_HI       0x07  /* Mailbox high byte (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 */
+#define PCA9641_CTL_BUS_INIT        BIT(3)  /* Trigger bus init/recovery (9 clocks) */
+#define PCA9641_CTL_SMBUS_SWRST     BIT(4)  /* Assert SCL LOW 35 ms after soft reset */
+#define PCA9641_CTL_IDLE_TIMER_DIS  BIT(5)  /* Disconnect after 100 ms bus idle */
+#define PCA9641_CTL_SMBUS_DIS       BIT(6)  /* Disconnect on SMBus timeout detection */
+#define PCA9641_CTL_PRIORITY        BIT(7)  /* Tie-break priority when both request */
+
+/* STATUS register (0x02) bit definitions (datasheet Section 8.3) */
+#define PCA9641_STA_OTHER_LOCK      BIT(0)  /* Other master currently holds the lock (R only) */
+#define PCA9641_STA_SCL_IO          BIT(6)  /* Manual SCL I/O (LOCK_GRANT=1, BUS_CONNECT=0) */
+#define PCA9641_STA_SDA_IO          BIT(7)  /* Manual SDA I/O (LOCK_GRANT=1, BUS_CONNECT=0) */
+
+/* INT_STATUS register (0x04) bit definitions (sticky, clear by writing 1) */
+#define PCA9641_INTS_INT_IN         BIT(0)  /* Interrupt received on INT_IN pin */
+#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 */
+#define PCA9641_INTS_TEST_INT       BIT(3)  /* Self-test interrupt triggered */
+#define PCA9641_INTS_MBOX_EMPTY     BIT(4)  /* Sent mailbox data was read by other master */
+#define PCA9641_INTS_MBOX_FULL      BIT(5)  /* New mailbox data received from other master */
+#define PCA9641_INTS_BUS_HUNG       BIT(6)  /* Bus hung condition detected */
+
+/* INT_MSK register (0x05) bit definitions (1 = masked/disabled, POR = 0x7F) */
+#define PCA9641_MSK_INT_IN          BIT(0)
+#define PCA9641_MSK_BUS_LOST        BIT(1)
+#define PCA9641_MSK_LOCK_GRANT      BIT(2)
+#define PCA9641_MSK_TEST_INT        BIT(3)
+#define PCA9641_MSK_MBOX_EMPTY      BIT(4)
+#define PCA9641_MSK_MBOX_FULL       BIT(5)
+#define PCA9641_MSK_BUS_HUNG        BIT(6)
+
+/* 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))
+
+/* 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[] = {
+	{ "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. */
+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");
+				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");
+			return -ETIMEDOUT;
+		}
+
+set_bus_connect:
+		(void)pca9641_reg_write(client, PCA9641_INT_STATUS,
+					PCA9641_INTS_LOCK_GRANT | PCA9641_INTS_BUS_LOST);
+
+		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");
+	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) {
+		ret = i2c_smbus_write_byte_data(client, PCA9641_INT_MSK,
+						PCA9641_INT_MSK_ARB);
+		if (ret < 0) {
+			dev_warn(&client->dev,
+				 "Failed to set interrupt mask (%d); using polling\n",
+				 ret);
+			goto add_adapter;
+		}
+
+		ret = devm_request_threaded_irq(&client->dev, client->irq,
+						NULL, pca9641_irq_handler,
+						IRQF_ONESHOT | IRQF_SHARED,
+						dev_name(&client->dev), data);
+		if (ret < 0) {
+			dev_warn(&client->dev,
+				 "Failed to request IRQ %d (%d); using polling\n",
+				 client->irq, ret);
+			i2c_smbus_write_byte_data(client, PCA9641_INT_MSK,
+						  PCA9641_INT_MSK_ALL);
+		} else {
+			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)
+		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);
+
+	if (data->irq > 0)
+		(void)i2c_smbus_write_byte_data(client, PCA9641_INT_MSK,
+						PCA9641_INT_MSK_ALL);
+
+	i2c_mux_del_adapters(muxc);
+}
+
+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

Re: [PATCH 2/2] i2c: mux: Add NXP PCA9641 I2C bus master arbiter driver

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:
- [High] The interrupt implementation is fatally flawed: it requests a shared IRQ with a NULL primary handler (which fails unconditionally), fails to clear the hardware interrupt source (causing an infinite IRQ storm), blindly consumes shared interrupts (stealing from other devices), and instantly aborts I2C transfers upon spurious wakeups.
- [High] Missing clearing of LOCK_REQ on error paths leads to permanent bus lockout.
- [High] pca9641_probe fails to re-mask interrupts if i2c_mux_add_adapter fails, leading to an unhandled IRQ storm.
- [Medium] Incorrect teardown order in `pca9641_remove()` masks hardware interrupts before unregistering child adapters, causing concurrent transactions to stall.
- [Medium] The driver unmasks hardware interrupts before registering the IRQ handler, risking immediate spurious interrupts and disabled IRQ lines.
- [Low] The `i2c_device_id` array fails to use named initializers.
--

commit 2d4a239fba3344246dc50ddca835d1ac1501417b
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 supports interrupt-assisted arbitration when INT0/INT1 is connected to a
GPIO, and falls back to polling mode when an interrupt is 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..c12aa88dcf560
> --- /dev/null
> +++ b/drivers/i2c/muxes/i2c-mux-pca9641.c

[ ... ]

> +static const struct i2c_device_id pca9641_id[] = {
> +	{ "pca9641" },
> +	{}
> +};

[Severity: Low]
Is it possible to use named initializers here? I2C subsystem guidelines
typically expect initialized arrays of type struct i2c_device_id to use
named initializers.

[ ... ]

> +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 handler need to clear the hardware interrupt source before
returning IRQ_HANDLED? Since this is a shared interrupt, claiming the
interrupt without checking or clearing the hardware source could lead to
an IRQ storm, and might consume interrupts meant for other devices sharing
the same line.

[ ... ]

> +		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");
> +				return -ETIMEDOUT;
> +			}
> +			goto set_bus_connect;
> +		}

[Severity: High]
If this wait times out, do we need to clear the PCA9641_CTL_LOCK_REQ bit in
the control register before returning? Bailing out here without clearing it
seems like it might permanently lock out the downstream I2C bus for the
other master.

> +
> +		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");
> +			return -ETIMEDOUT;
> +		}

[Severity: High]
Since this IRQ is requested as shared, could a spurious wakeup or an
interrupt from another device cause us to take this path? If so, this would
instantly fail the transfer instead of safely resuming the wait.

[ ... ]

> +	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");
> +	return -ETIMEDOUT;

[Severity: High]
Similar to the interrupt wait timeout, if this polling loop times out,
should we clear the PCA9641_CTL_LOCK_REQ bit before returning -ETIMEDOUT?
Otherwise, the lock might be held indefinitely.

[ ... ]

> +	/* Optional interrupt mode; fall back to polling on failure. */
> +	if (client->irq > 0) {
> +		ret = i2c_smbus_write_byte_data(client, PCA9641_INT_MSK,
> +						PCA9641_INT_MSK_ARB);

[Severity: Medium]
Are hardware interrupts unmasked here before the IRQ handler is successfully
registered? If the device asserts an interrupt before the OS handler is
ready, could the kernel permanently disable the shared IRQ line?

> +		if (ret < 0) {
> +			dev_warn(&client->dev,
> +				 "Failed to set interrupt mask (%d); using polling\n",
> +				 ret);
> +			goto add_adapter;
> +		}
> +
> +		ret = devm_request_threaded_irq(&client->dev, client->irq,
> +						NULL, pca9641_irq_handler,
> +						IRQF_ONESHOT | IRQF_SHARED,
> +						dev_name(&client->dev), data);

[Severity: High]
Will this request succeed? Requesting a threaded IRQ with IRQF_SHARED and a
NULL primary handler usually fails unconditionally with -EINVAL, which would
prevent interrupt mode from working entirely.

[ ... ]

> +add_adapter:
> +	ret = i2c_mux_add_adapter(muxc, 0, 0);
> +	if (ret)
> +		return ret;

[Severity: High]
If i2c_mux_add_adapter fails here, should the error path re-mask the
interrupts? Returning an error while hardware interrupts are left unmasked
could lead to an unhandled IRQ storm.

[ ... ]

> +static void pca9641_remove(struct i2c_client *client)
> +{
> +	struct i2c_mux_core *muxc = i2c_get_clientdata(client);
> +	struct pca9641 *data = i2c_mux_priv(muxc);
> +
> +	if (data->irq > 0)
> +		(void)i2c_smbus_write_byte_data(client, PCA9641_INT_MSK,
> +						PCA9641_INT_MSK_ALL);
> +
> +	i2c_mux_del_adapters(muxc);
> +}

[Severity: Medium]
Could this teardown order stall concurrent transactions? Masking hardware
interrupts before unregistering the child adapters via i2c_mux_del_adapters
means that if a downstream transaction is waiting for a bus grant during
removal, the completion might simply time out instead of finishing.

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