linux-rockchip.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/7] Add Mule I2C multiplexer support
@ 2024-05-06 11:37 Farouk Bouabid
  2024-05-06 11:37 ` [PATCH v2 1/7] i2c: mux: add the ability to share mux core address with child nodes Farouk Bouabid
                   ` (7 more replies)
  0 siblings, 8 replies; 12+ messages in thread
From: Farouk Bouabid @ 2024-05-06 11:37 UTC (permalink / raw)
  To: Wolfram Sang, Peter Rosin, Andi Shyti, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Farouk Bouabid,
	Quentin Schulz, Heiko Stuebner, Quentin Schulz
  Cc: linux-i2c, linux-kernel, devicetree, linux-arm-kernel, linux-rockchip

Mule is an mcu that emulates a set of i2c devices which are reachable
through an i2c-mux.

The emulated devices share a single i2c address with the mux core itself
where the requested register is what determines which logic is executed
(muxing logic or device logic):

1- The devices on the mux core can be selected (muxing functionality) by
writing the appropriate device number to an i2c config register (0xff)
that is not used by any device logic.

2- Any access to a register other than the config register will be
handled by the previously selected device.

      +-------------------------------------------------------+
      |  Mule                                                 |
      |        +---------------+                              |
    ----+-(1)->|Config register|-----+                        |
      | |      +---------------+     |                        |
      | |                            V_                       |
      | |                            |  \          +--------+ |
      | |                            |   \-------->| dev #0 | |
      | |                            |   |         +--------+ |
      | |                            | M |-------->| dev #1 | |
      | +-----------(2)------------->| U |         +--------+ |
      |                              | X |-------->| dev #2 | |
      |                              |   |         +--------+ |
      |                              |   /-------->| dev #3 | |
      |                              |__/          +--------+ |
      +-------------------------------------------------------+

The current i2c-mux implementation does not allow the mux core to use the
i2c address of a child device. As a workaround, A new i2c-adapter quirk is
introduced to skip the check for conflict between a child device and the
mux core i2c address when adding the child device.

This patch-series adds support for this multiplexer. Mule is integrated
as part of rk3399-puma, px30-ringneck, rk3588-tiger and rk3588-jaguar
boards.

To: Wolfram Sang <wsa+renesas@sang-engineering.com>
To: Peter Rosin <peda@axentia.se>
To: Andi Shyti <andi.shyti@kernel.org>
To: Rob Herring <robh@kernel.org>
To: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
To: Conor Dooley <conor+dt@kernel.org>
To: Quentin Schulz <quentin.schulz@theobroma-systems.com>
To: Heiko Stuebner <heiko@sntech.de>
To: Quentin Schulz <quentin.schulz@cherry.de>
Cc: linux-i2c@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: devicetree@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-rockchip@lists.infradead.org
Signed-off-by: Farouk Bouabid <farouk.bouabid@cherry.de>

Changes in v2:
- Add i2c-adapter quirks to skip checking for conflict between the mux core
  and a child device address.
- Rename dt-binding to "tsd,mule-i2c-mux.yaml"
- Add Mule description to kconfig
- Fix indentation
- Move device table after probe

- Link to v1: https://lore.kernel.org/r/20240426-dev-mule-i2c-mux-v1-0-045a482f6ffb@theobroma-systems.com

---
Farouk Bouabid (7):
      i2c: mux: add the ability to share mux core address with child nodes
      dt-bindings: i2c: mux: mule: add dt-bindings for mule i2c multiplexer
      i2c: muxes: add support for mule i2c multiplexer
      arm64: dts: rockchip: add mule i2c mux (0x18) on rk3399-puma
      arm64: dts: rockchip: add mule i2c mux (0x18) on rk3588-tiger
      arm64: dts: rockchip: add mule i2c mux (0x18) on px30-ringneck
      arm64: dts: rockchip: add mule i2c mux (0x18) on rk3588-jaguar

 .../devicetree/bindings/i2c/tsd,mule-i2c-mux.yaml  |  80 +++++++++++
 arch/arm64/boot/dts/rockchip/px30-ringneck.dtsi    |  20 ++-
 arch/arm64/boot/dts/rockchip/rk3399-puma.dtsi      |  20 ++-
 arch/arm64/boot/dts/rockchip/rk3588-jaguar.dts     |  19 ++-
 arch/arm64/boot/dts/rockchip/rk3588-tiger.dtsi     |  19 ++-
 drivers/i2c/i2c-core-base.c                        |   6 +-
 drivers/i2c/i2c-mux.c                              |  25 +++-
 drivers/i2c/muxes/Kconfig                          |  18 +++
 drivers/i2c/muxes/Makefile                         |   1 +
 drivers/i2c/muxes/i2c-mux-mule.c                   | 157 +++++++++++++++++++++
 include/linux/i2c-mux.h                            |   1 +
 include/linux/i2c.h                                |   7 +
 12 files changed, 361 insertions(+), 12 deletions(-)
---
base-commit: 23918f4e52d072b96a4d909e91298b8dd6ad4325
change-id: 20240404-dev-mule-i2c-mux-9103cde07021

Best regards,
-- 
Farouk Bouabid <farouk.bouabid@cherry.de>


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

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

* [PATCH v2 1/7] i2c: mux: add the ability to share mux core address with child nodes
  2024-05-06 11:37 [PATCH v2 0/7] Add Mule I2C multiplexer support Farouk Bouabid
@ 2024-05-06 11:37 ` Farouk Bouabid
  2024-05-06 21:26   ` Peter Rosin
  2024-05-06 11:37 ` [PATCH v2 2/7] dt-bindings: i2c: mux: mule: add dt-bindings for mule i2c multiplexer Farouk Bouabid
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 12+ messages in thread
From: Farouk Bouabid @ 2024-05-06 11:37 UTC (permalink / raw)
  To: Wolfram Sang, Peter Rosin, Andi Shyti, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Farouk Bouabid,
	Quentin Schulz, Heiko Stuebner, Quentin Schulz
  Cc: linux-i2c, linux-kernel, devicetree, linux-arm-kernel, linux-rockchip

Allow the mux core (if it's an i2c device) to have the same address as
a child device. This is useful when the mux core can only use an i2c
address that is used by a child device because no other addresses are
free to use. eg. the mux core can only use address 0x18 which is used by
amc6821 connected to the mux.

Signed-off-by: Farouk Bouabid <farouk.bouabid@cherry.de>
---
 drivers/i2c/i2c-core-base.c |  6 +++++-
 drivers/i2c/i2c-mux.c       | 25 ++++++++++++++++++++++++-
 include/linux/i2c-mux.h     |  1 +
 include/linux/i2c.h         |  7 +++++++
 4 files changed, 37 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
index ff5c486a1dbb..ce2425b0486d 100644
--- a/drivers/i2c/i2c-core-base.c
+++ b/drivers/i2c/i2c-core-base.c
@@ -821,9 +821,13 @@ static int i2c_check_mux_children(struct device *dev, void *addrp)
 static int i2c_check_addr_busy(struct i2c_adapter *adapter, int addr)
 {
 	struct i2c_adapter *parent = i2c_parent_is_i2c_adapter(adapter);
+	bool skip_check = false;
 	int result = 0;
 
-	if (parent)
+	if (adapter->quirks && adapter->quirks->flags & I2C_AQ_SKIP_ADDR_CHECK)
+		skip_check = adapter->quirks->skip_addr_in_parent == addr;
+
+	if (parent && !skip_check)
 		result = i2c_check_mux_parents(parent, addr);
 
 	if (!result)
diff --git a/drivers/i2c/i2c-mux.c b/drivers/i2c/i2c-mux.c
index 57ff09f18c37..bdb75a130cab 100644
--- a/drivers/i2c/i2c-mux.c
+++ b/drivers/i2c/i2c-mux.c
@@ -334,7 +334,30 @@ int i2c_mux_add_adapter(struct i2c_mux_core *muxc,
 	priv->adap.dev.parent = &parent->dev;
 	priv->adap.retries = parent->retries;
 	priv->adap.timeout = parent->timeout;
-	priv->adap.quirks = parent->quirks;
+	/*
+	 * When creating the adapter, the node devices are checked for i2c address
+	 * match with other devices on the parent adapter, among which is the mux core itself.
+	 * If a match is found the node device is not probed successfully.
+	 * Allow the mux to have the same address as a child device by skipping this check.
+	 */
+	if (muxc->share_addr_with_children && muxc->dev->type == &i2c_client_type) {
+		struct i2c_adapter_quirks *quirks = devm_kzalloc(muxc->dev,
+								 sizeof(*quirks), GFP_KERNEL);
+		struct i2c_client *client = to_i2c_client(muxc->dev);
+
+		if (!quirks)
+			return -ENOMEM;
+
+		if (parent->quirks)
+			memcpy(quirks, parent->quirks, sizeof(*quirks));
+
+		quirks->flags |= I2C_AQ_SKIP_ADDR_CHECK;
+		quirks->skip_addr_in_parent = client->addr;
+		priv->adap.quirks = quirks;
+	} else {
+		priv->adap.quirks = parent->quirks;
+	}
+
 	if (muxc->mux_locked)
 		priv->adap.lock_ops = &i2c_mux_lock_ops;
 	else
diff --git a/include/linux/i2c-mux.h b/include/linux/i2c-mux.h
index 98ef73b7c8fd..17ac68bf1703 100644
--- a/include/linux/i2c-mux.h
+++ b/include/linux/i2c-mux.h
@@ -21,6 +21,7 @@ struct i2c_mux_core {
 	unsigned int mux_locked:1;
 	unsigned int arbitrator:1;
 	unsigned int gate:1;
+	unsigned int share_addr_with_children:1;
 
 	void *priv;
 
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index 5e6cd43a6dbd..c3acbaaadae9 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -670,6 +670,7 @@ int i2c_generic_scl_recovery(struct i2c_adapter *adap);
  * @max_read_len: maximum length of a read message
  * @max_comb_1st_msg_len: maximum length of the first msg in a combined message
  * @max_comb_2nd_msg_len: maximum length of the second msg in a combined message
+ * @skip_addr_in_parent: No conflict check on parent adapter for a given address
  *
  * Note about combined messages: Some I2C controllers can only send one message
  * per transfer, plus something called combined message or write-then-read.
@@ -690,6 +691,7 @@ struct i2c_adapter_quirks {
 	u16 max_read_len;
 	u16 max_comb_1st_msg_len;
 	u16 max_comb_2nd_msg_len;
+	unsigned short skip_addr_in_parent;
 };
 
 /* enforce max_num_msgs = 2 and use max_comb_*_len for length checks */
@@ -711,6 +713,11 @@ struct i2c_adapter_quirks {
 #define I2C_AQ_NO_ZERO_LEN		(I2C_AQ_NO_ZERO_LEN_READ | I2C_AQ_NO_ZERO_LEN_WRITE)
 /* adapter cannot do repeated START */
 #define I2C_AQ_NO_REP_START		BIT(7)
+/**
+ * do not check for conflict on a given address
+ * used accordingly with "struct i2c_adapter_quirks.skip_addr_in_parent"
+ */
+#define I2C_AQ_SKIP_ADDR_CHECK	BIT(8)
 
 /*
  * i2c_adapter is the structure used to identify a physical i2c bus along

-- 
2.34.1


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

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

* [PATCH v2 2/7] dt-bindings: i2c: mux: mule: add dt-bindings for mule i2c multiplexer
  2024-05-06 11:37 [PATCH v2 0/7] Add Mule I2C multiplexer support Farouk Bouabid
  2024-05-06 11:37 ` [PATCH v2 1/7] i2c: mux: add the ability to share mux core address with child nodes Farouk Bouabid
@ 2024-05-06 11:37 ` Farouk Bouabid
  2024-05-06 20:26   ` Rob Herring (Arm)
  2024-05-06 11:37 ` [PATCH v2 3/7] i2c: muxes: add support " Farouk Bouabid
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 12+ messages in thread
From: Farouk Bouabid @ 2024-05-06 11:37 UTC (permalink / raw)
  To: Wolfram Sang, Peter Rosin, Andi Shyti, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Farouk Bouabid,
	Quentin Schulz, Heiko Stuebner, Quentin Schulz
  Cc: linux-i2c, linux-kernel, devicetree, linux-arm-kernel, linux-rockchip

Mule is an MCU that emulates a set of i2c devices which are reachable
through an i2c-mux.

The emulated devices share a single i2c address with the mux core itself
where the requested register is what determines which logic is executed
(mux logic or device logic).

Add support for the Mule I2C multiplexer bindings.

Signed-off-by: Farouk Bouabid <farouk.bouabid@cherry.de>
---
 .../devicetree/bindings/i2c/tsd,mule-i2c-mux.yaml  | 80 ++++++++++++++++++++++
 1 file changed, 80 insertions(+)

diff --git a/Documentation/devicetree/bindings/i2c/tsd,mule-i2c-mux.yaml b/Documentation/devicetree/bindings/i2c/tsd,mule-i2c-mux.yaml
new file mode 100644
index 000000000000..50915183bd43
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/tsd,mule-i2c-mux.yaml
@@ -0,0 +1,80 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/i2c/tsd,mule-i2c-mux.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Mule I2C multiplexer
+
+maintainers:
+  - Farouk Bouabid <farouk.bouabid@cherry.de>
+  - Quentin Schulz <quentin.schulz@cherry.de>
+
+description: |
+  Mule is an MCU that emulates a set of i2c devices which are reachable
+  through an i2c-mux.
+
+  The emulated devices share a single i2c address with the mux core itself
+  where the requested register is what determines which logic is executed
+  (mux logic or device logic)
+
+      +--------------------------------------------------+
+      | Mule                                             |
+      |    +---------------+                             |
+  ------+->|Config register|----+                        |
+      | |  +---------------+    |                        |
+      | |                       V_                       |
+      | |                      |  \          +--------+  |
+      | |                      |   \-------->| dev #0 |  |
+      | |                      |   |         +--------+  |
+      | |                      | M |-------->| dev #1 |  |
+      | +--------------------->| U |         +--------+  |
+      |                        | X |-------->| dev #2 |  |
+      |                        |   |         +--------+  |
+      |                        |   /-------->| dev #3 |  |
+      |                        |__/          +--------+  |
+      +--------------------------------------------------+
+
+
+allOf:
+  - $ref: /schemas/i2c/i2c-mux.yaml#
+
+properties:
+  compatible:
+    const: tsd,mule-i2c-mux
+
+  reg:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        i2c-mux@18 {
+            compatible = "tsd,mule-i2c-mux";
+            reg = <0x18>;
+            #address-cells = <1>;
+            #size-cells = <0>;
+
+            i2c@0 {
+                reg = <0x0>;
+                #address-cells = <1>;
+                #size-cells = <0>;
+
+                fan: fan@18 {
+                  compatible = "ti,amc6821";
+                  reg = <0x18>;
+                  #cooling-cells = <2>;
+                };
+            };
+        };
+    };
+...

-- 
2.34.1


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

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

* [PATCH v2 3/7] i2c: muxes: add support for mule i2c multiplexer
  2024-05-06 11:37 [PATCH v2 0/7] Add Mule I2C multiplexer support Farouk Bouabid
  2024-05-06 11:37 ` [PATCH v2 1/7] i2c: mux: add the ability to share mux core address with child nodes Farouk Bouabid
  2024-05-06 11:37 ` [PATCH v2 2/7] dt-bindings: i2c: mux: mule: add dt-bindings for mule i2c multiplexer Farouk Bouabid
@ 2024-05-06 11:37 ` Farouk Bouabid
  2024-05-06 11:37 ` [PATCH v2 4/7] arm64: dts: rockchip: add mule i2c mux (0x18) on rk3399-puma Farouk Bouabid
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 12+ messages in thread
From: Farouk Bouabid @ 2024-05-06 11:37 UTC (permalink / raw)
  To: Wolfram Sang, Peter Rosin, Andi Shyti, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Farouk Bouabid,
	Quentin Schulz, Heiko Stuebner, Quentin Schulz
  Cc: linux-i2c, linux-kernel, devicetree, linux-arm-kernel, linux-rockchip

Mule is an MCU that emulates a set of i2c devices which are reachable
through an i2c-mux.

The emulated devices share a single i2c address with the mux core itself
where the requested register is what determines which logic is executed
(mux logic or device logic):

1- The devices on the mux can be selected (mux function) by writing the
appropriate device number to an i2c config register (0xff) that is not
used by any device logic.

2- Any access to a register other than the config register will be
handled by the previously selected device.

Signed-off-by: Farouk Bouabid <farouk.bouabid@cherry.de>
---
 drivers/i2c/muxes/Kconfig        |  18 +++++
 drivers/i2c/muxes/Makefile       |   1 +
 drivers/i2c/muxes/i2c-mux-mule.c | 157 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 176 insertions(+)

diff --git a/drivers/i2c/muxes/Kconfig b/drivers/i2c/muxes/Kconfig
index db1b9057612a..45d3c5997a4f 100644
--- a/drivers/i2c/muxes/Kconfig
+++ b/drivers/i2c/muxes/Kconfig
@@ -119,4 +119,22 @@ config I2C_MUX_MLXCPLD
 	  This driver can also be built as a module.  If so, the module
 	  will be called i2c-mux-mlxcpld.
 
+config I2C_MUX_MULE
+	tristate "Mule I2C device multiplexer"
+	depends on OF
+	help
+	  Mule is an MCU that emulates a set of i2c devices which are reachable
+	  through an i2c-mux.
+
+	  The emulated devices share a single i2c address with the mux core itself
+	  where the requested register is what determines which logic is executed
+	  (mux logic or device logic).
+
+	  If you say yes to this option, support will be included for a
+	  Mule I2C device multiplexer. This driver provides access to
+	  I2C devices connected on the Mule I2C mux.
+
+	  This driver can also be built as a module.  If so, the module
+	  will be called i2c-mux-mule.
+
 endmenu
diff --git a/drivers/i2c/muxes/Makefile b/drivers/i2c/muxes/Makefile
index 6d9d865e8518..4b24f49515a7 100644
--- a/drivers/i2c/muxes/Makefile
+++ b/drivers/i2c/muxes/Makefile
@@ -10,6 +10,7 @@ obj-$(CONFIG_I2C_MUX_GPIO)	+= i2c-mux-gpio.o
 obj-$(CONFIG_I2C_MUX_GPMUX)	+= i2c-mux-gpmux.o
 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_PCA954x)	+= i2c-mux-pca954x.o
 obj-$(CONFIG_I2C_MUX_PINCTRL)	+= i2c-mux-pinctrl.o
diff --git a/drivers/i2c/muxes/i2c-mux-mule.c b/drivers/i2c/muxes/i2c-mux-mule.c
new file mode 100644
index 000000000000..781c978276cd
--- /dev/null
+++ b/drivers/i2c/muxes/i2c-mux-mule.c
@@ -0,0 +1,157 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Mule I2C device multiplexer
+ *
+ * Copyright (C) 2024 Theobroma Systems Design und Consulting GmbH
+ */
+
+#include <linux/i2c-mux.h>
+#include <linux/i2c.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/property.h>
+#include <linux/regmap.h>
+
+#define MUX_CONFIG_REG  0xff
+#define MUX_DEFAULT_DEV 0x0
+
+struct mule_i2c_reg_mux {
+	struct regmap *regmap;
+};
+
+static const struct regmap_config mule_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+};
+
+static inline int __mux_select(struct regmap *regmap, u32 dev)
+{
+	return regmap_write(regmap, MUX_CONFIG_REG, dev);
+}
+
+static int mux_select(struct i2c_mux_core *muxc, u32 dev)
+{
+	struct mule_i2c_reg_mux *mux = muxc->priv;
+
+	return __mux_select(mux->regmap, dev);
+}
+
+static int mux_deselect(struct i2c_mux_core *muxc, u32 dev)
+{
+	return mux_select(muxc, MUX_DEFAULT_DEV);
+}
+
+static void mux_remove(void *data)
+{
+	struct i2c_mux_core *muxc = data;
+
+	i2c_mux_del_adapters(muxc);
+
+	mux_deselect(muxc, MUX_DEFAULT_DEV);
+}
+
+static int mule_i2c_mux_probe(struct i2c_client *client)
+{
+	struct i2c_adapter *adap = client->adapter;
+	struct mule_i2c_reg_mux *priv;
+	struct i2c_mux_core *muxc;
+	struct device_node *dev;
+	unsigned int readback;
+	bool old_fw;
+	int ndev, ret;
+
+	/* Count devices on the mux */
+	ndev = of_get_child_count(client->dev.of_node);
+	dev_dbg(&client->dev, "%u devices on the mux\n", ndev);
+
+	muxc = i2c_mux_alloc(adap, &client->dev,
+			     ndev, sizeof(*priv),
+			     I2C_MUX_LOCKED,
+			     mux_select, mux_deselect);
+	if (!muxc)
+		return -ENOMEM;
+
+	muxc->share_addr_with_children = 1;
+	priv = i2c_mux_priv(muxc);
+
+	priv->regmap = devm_regmap_init_i2c(client, &mule_regmap_config);
+	if (IS_ERR(priv->regmap))
+		return dev_err_probe(&client->dev, PTR_ERR(priv->regmap),
+				     "Failed to allocate i2c register map\n");
+
+	i2c_set_clientdata(client, muxc);
+
+	/*
+	 * Mux 0 is guaranteed to exist on all old and new mule fw.
+	 * mule fw without mux support will accept write ops to the
+	 * config register, but readback returns 0xff (register not updated).
+	 */
+	ret = mux_select(muxc, 0);
+	if (ret)
+		return ret;
+
+	ret = regmap_read(priv->regmap, MUX_CONFIG_REG, &readback);
+	if (ret)
+		return ret;
+
+	old_fw = (readback == 0);
+
+	ret = devm_add_action_or_reset(&client->dev, mux_remove, muxc);
+	if (ret)
+		return ret;
+
+	/* Create device adapters */
+	for_each_child_of_node(client->dev.of_node, dev) {
+		u32 reg;
+
+		ret = of_property_read_u32(dev, "reg", &reg);
+		if (ret) {
+			dev_err(&client->dev, "No reg property found for %s: %d\n",
+				of_node_full_name(dev), ret);
+			return ret;
+		}
+
+		if (!old_fw && reg != 0) {
+			dev_warn(&client->dev,
+				 "Mux %d not supported, please update Mule FW\n", reg);
+			continue;
+		}
+
+		ret = mux_select(muxc, reg);
+		if (ret) {
+			dev_warn(&client->dev,
+				 "Mux %d not supported, please update Mule FW\n", reg);
+			continue;
+		}
+
+		ret = i2c_mux_add_adapter(muxc, 0, reg, 0);
+		if (ret) {
+			dev_err(&client->dev, "Failed to add i2c mux adapter %d: %d\n", reg, ret);
+			return ret;
+		}
+	}
+
+	mux_deselect(muxc, MUX_DEFAULT_DEV);
+
+	return 0;
+}
+
+static const struct of_device_id mule_i2c_mux_of_match[] = {
+	{.compatible = "tsd,mule-i2c-mux",},
+	{},
+};
+MODULE_DEVICE_TABLE(of, mule_i2c_mux_of_match);
+
+static struct i2c_driver mule_i2c_mux_driver = {
+	.driver		= {
+		.name	= "mule-i2c-mux",
+		.of_match_table = mule_i2c_mux_of_match,
+	},
+	.probe		= mule_i2c_mux_probe,
+};
+
+module_i2c_driver(mule_i2c_mux_driver);
+
+MODULE_AUTHOR("Farouk Bouabid <farouk.bouabid@cherry.de>");
+MODULE_DESCRIPTION("I2C mux driver for Mule");
+MODULE_LICENSE("GPL");

-- 
2.34.1


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

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

* [PATCH v2 4/7] arm64: dts: rockchip: add mule i2c mux (0x18) on rk3399-puma
  2024-05-06 11:37 [PATCH v2 0/7] Add Mule I2C multiplexer support Farouk Bouabid
                   ` (2 preceding siblings ...)
  2024-05-06 11:37 ` [PATCH v2 3/7] i2c: muxes: add support " Farouk Bouabid
@ 2024-05-06 11:37 ` Farouk Bouabid
  2024-05-06 11:37 ` [PATCH v2 5/7] arm64: dts: rockchip: add mule i2c mux (0x18) on rk3588-tiger Farouk Bouabid
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 12+ messages in thread
From: Farouk Bouabid @ 2024-05-06 11:37 UTC (permalink / raw)
  To: Wolfram Sang, Peter Rosin, Andi Shyti, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Farouk Bouabid,
	Quentin Schulz, Heiko Stuebner, Quentin Schulz
  Cc: linux-i2c, linux-kernel, devicetree, linux-arm-kernel, linux-rockchip

Mule emulates an i2c mux (address 0x18). The amc6821 is exposed behind
this bus.

Add the mux node and amc6821 as a default device.

Signed-off-by: Farouk Bouabid <farouk.bouabid@cherry.de>
---
 arch/arm64/boot/dts/rockchip/rk3399-puma.dtsi | 20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399-puma.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-puma.dtsi
index c08e69391c01..e7313be24c1a 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-puma.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399-puma.dtsi
@@ -10,6 +10,7 @@
 / {
 	aliases {
 		ethernet0 = &gmac;
+		i2c10 = &i2c10;
 		mmc0 = &sdhci;
 	};
 
@@ -357,10 +358,23 @@ &i2c7 {
 	status = "okay";
 	clock-frequency = <400000>;
 
-	fan: fan@18 {
-		compatible = "ti,amc6821";
+	i2c-mux@18 {
+		compatible = "tsd,mule-i2c-mux";
 		reg = <0x18>;
-		#cooling-cells = <2>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		i2c10: i2c@0 {
+			reg = <0x0>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			fan: fan@18 {
+				compatible = "ti,amc6821";
+				reg = <0x18>;
+				#cooling-cells = <2>;
+			};
+		};
 	};
 
 	rtc_twi: rtc@6f {

-- 
2.34.1


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

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

* [PATCH v2 5/7] arm64: dts: rockchip: add mule i2c mux (0x18) on rk3588-tiger
  2024-05-06 11:37 [PATCH v2 0/7] Add Mule I2C multiplexer support Farouk Bouabid
                   ` (3 preceding siblings ...)
  2024-05-06 11:37 ` [PATCH v2 4/7] arm64: dts: rockchip: add mule i2c mux (0x18) on rk3399-puma Farouk Bouabid
@ 2024-05-06 11:37 ` Farouk Bouabid
  2024-05-06 11:37 ` [PATCH v2 6/7] arm64: dts: rockchip: add mule i2c mux (0x18) on px30-ringneck Farouk Bouabid
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 12+ messages in thread
From: Farouk Bouabid @ 2024-05-06 11:37 UTC (permalink / raw)
  To: Wolfram Sang, Peter Rosin, Andi Shyti, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Farouk Bouabid,
	Quentin Schulz, Heiko Stuebner, Quentin Schulz
  Cc: linux-i2c, linux-kernel, devicetree, linux-arm-kernel, linux-rockchip

Mule emulates an i2c mux (address 0x18). The amc6821 is exposed behind
this bus.

Add the mux node and amc6821 as a default device.

Signed-off-by: Farouk Bouabid <farouk.bouabid@cherry.de>
---
 arch/arm64/boot/dts/rockchip/rk3588-tiger.dtsi | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3588-tiger.dtsi b/arch/arm64/boot/dts/rockchip/rk3588-tiger.dtsi
index 1eb2543a5fde..5ed7d51717bb 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588-tiger.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3588-tiger.dtsi
@@ -12,6 +12,7 @@ / {
 	compatible = "tsd,rk3588-tiger", "rockchip,rk3588";
 
 	aliases {
+		i2c10 = &i2c10;
 		mmc0 = &sdhci;
 		rtc0 = &rtc_twi;
 	};
@@ -210,9 +211,23 @@ &i2c6 {
 	clock-frequency = <400000>;
 	status = "okay";
 
-	fan@18 {
-		compatible = "ti,amc6821";
+	i2c-mux@18 {
+		compatible = "tsd,mule-i2c-mux";
 		reg = <0x18>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		i2c10: i2c@0 {
+			reg = <0x0>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			fan: fan@18 {
+				compatible = "ti,amc6821";
+				reg = <0x18>;
+				#cooling-cells = <2>;
+			};
+		};
 	};
 
 	rtc_twi: rtc@6f {

-- 
2.34.1


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

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

* [PATCH v2 6/7] arm64: dts: rockchip: add mule i2c mux (0x18) on px30-ringneck
  2024-05-06 11:37 [PATCH v2 0/7] Add Mule I2C multiplexer support Farouk Bouabid
                   ` (4 preceding siblings ...)
  2024-05-06 11:37 ` [PATCH v2 5/7] arm64: dts: rockchip: add mule i2c mux (0x18) on rk3588-tiger Farouk Bouabid
@ 2024-05-06 11:37 ` Farouk Bouabid
  2024-05-06 11:37 ` [PATCH v2 7/7] arm64: dts: rockchip: add mule i2c mux (0x18) on rk3588-jaguar Farouk Bouabid
  2024-05-06 20:40 ` [PATCH v2 0/7] Add Mule I2C multiplexer support Rob Herring (Arm)
  7 siblings, 0 replies; 12+ messages in thread
From: Farouk Bouabid @ 2024-05-06 11:37 UTC (permalink / raw)
  To: Wolfram Sang, Peter Rosin, Andi Shyti, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Farouk Bouabid,
	Quentin Schulz, Heiko Stuebner, Quentin Schulz
  Cc: linux-i2c, linux-kernel, devicetree, linux-arm-kernel, linux-rockchip

Mule emulates an i2c mux (address 0x18). The amc6821 is exposed behind
this bus.

Add the mux node and amc6821 as a default device.

Signed-off-by: Farouk Bouabid <farouk.bouabid@cherry.de>
---
 arch/arm64/boot/dts/rockchip/px30-ringneck.dtsi | 20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/boot/dts/rockchip/px30-ringneck.dtsi b/arch/arm64/boot/dts/rockchip/px30-ringneck.dtsi
index bb1aea82e666..eea906379983 100644
--- a/arch/arm64/boot/dts/rockchip/px30-ringneck.dtsi
+++ b/arch/arm64/boot/dts/rockchip/px30-ringneck.dtsi
@@ -9,6 +9,7 @@
 
 / {
 	aliases {
+		i2c10 = &i2c10;
 		mmc0 = &emmc;
 		mmc1 = &sdio;
 		rtc0 = &rtc_twi;
@@ -291,10 +292,23 @@ &i2c1 {
 	/* SE05x is limited to Fast Mode */
 	clock-frequency = <400000>;
 
-	fan: fan@18 {
-		compatible = "ti,amc6821";
+	i2c-mux@18 {
+		compatible = "tsd,mule-i2c-mux";
 		reg = <0x18>;
-		#cooling-cells = <2>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		i2c10: i2c@0 {
+			reg = <0x0>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			fan: fan@18 {
+				compatible = "ti,amc6821";
+				reg = <0x18>;
+				#cooling-cells = <2>;
+			};
+		};
 	};
 
 	rtc_twi: rtc@6f {

-- 
2.34.1


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

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

* [PATCH v2 7/7] arm64: dts: rockchip: add mule i2c mux (0x18) on rk3588-jaguar
  2024-05-06 11:37 [PATCH v2 0/7] Add Mule I2C multiplexer support Farouk Bouabid
                   ` (5 preceding siblings ...)
  2024-05-06 11:37 ` [PATCH v2 6/7] arm64: dts: rockchip: add mule i2c mux (0x18) on px30-ringneck Farouk Bouabid
@ 2024-05-06 11:37 ` Farouk Bouabid
  2024-05-06 20:40 ` [PATCH v2 0/7] Add Mule I2C multiplexer support Rob Herring (Arm)
  7 siblings, 0 replies; 12+ messages in thread
From: Farouk Bouabid @ 2024-05-06 11:37 UTC (permalink / raw)
  To: Wolfram Sang, Peter Rosin, Andi Shyti, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Farouk Bouabid,
	Quentin Schulz, Heiko Stuebner, Quentin Schulz
  Cc: linux-i2c, linux-kernel, devicetree, linux-arm-kernel, linux-rockchip

Mule emulates an i2c mux (address 0x18). The amc6821 is exposed behind
this bus.

Add the mux node and amc6821 as a default device.

Signed-off-by: Farouk Bouabid <farouk.bouabid@cherry.de>
---
 arch/arm64/boot/dts/rockchip/rk3588-jaguar.dts | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3588-jaguar.dts b/arch/arm64/boot/dts/rockchip/rk3588-jaguar.dts
index 39d65002add1..14f1322c162f 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588-jaguar.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3588-jaguar.dts
@@ -32,6 +32,7 @@ button-bios-disable {
 
 	aliases {
 		ethernet0 = &gmac0;
+		i2c10 = &i2c10;
 		mmc0 = &sdhci;
 		mmc1 = &sdmmc;
 		rtc0 = &rtc_twi;
@@ -249,9 +250,23 @@ &i2c0 {
 	pinctrl-0 = <&i2c0m2_xfer>;
 	status = "okay";
 
-	fan@18 {
-		compatible = "ti,amc6821";
+	i2c-mux@18 {
+		compatible = "tsd,mule-i2c-mux";
 		reg = <0x18>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		i2c10: i2c@0 {
+			reg = <0x0>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			fan: fan@18 {
+				compatible = "ti,amc6821";
+				reg = <0x18>;
+				#cooling-cells = <2>;
+			};
+		};
 	};
 
 	vdd_npu_s0: regulator@42 {

-- 
2.34.1


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

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

* Re: [PATCH v2 2/7] dt-bindings: i2c: mux: mule: add dt-bindings for mule i2c multiplexer
  2024-05-06 11:37 ` [PATCH v2 2/7] dt-bindings: i2c: mux: mule: add dt-bindings for mule i2c multiplexer Farouk Bouabid
@ 2024-05-06 20:26   ` Rob Herring (Arm)
  0 siblings, 0 replies; 12+ messages in thread
From: Rob Herring (Arm) @ 2024-05-06 20:26 UTC (permalink / raw)
  To: Farouk Bouabid
  Cc: devicetree, linux-rockchip, linux-arm-kernel, linux-i2c,
	Quentin Schulz, Wolfram Sang, Heiko Stuebner, Andi Shyti,
	Conor Dooley, linux-kernel, Quentin Schulz, Peter Rosin,
	Krzysztof Kozlowski


On Mon, 06 May 2024 13:37:53 +0200, Farouk Bouabid wrote:
> Mule is an MCU that emulates a set of i2c devices which are reachable
> through an i2c-mux.
> 
> The emulated devices share a single i2c address with the mux core itself
> where the requested register is what determines which logic is executed
> (mux logic or device logic).
> 
> Add support for the Mule I2C multiplexer bindings.
> 
> Signed-off-by: Farouk Bouabid <farouk.bouabid@cherry.de>
> ---
>  .../devicetree/bindings/i2c/tsd,mule-i2c-mux.yaml  | 80 ++++++++++++++++++++++
>  1 file changed, 80 insertions(+)
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/i2c/tsd,mule-i2c-mux.example.dtb: fan@18: '#cooling-cells' does not match any of the regexes: 'pinctrl-[0-9]+'
	from schema $id: http://devicetree.org/schemas/trivial-devices.yaml#

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20240506-dev-mule-i2c-mux-v2-2-a91c954f65d7@cherry.de

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

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

* Re: [PATCH v2 0/7] Add Mule I2C multiplexer support
  2024-05-06 11:37 [PATCH v2 0/7] Add Mule I2C multiplexer support Farouk Bouabid
                   ` (6 preceding siblings ...)
  2024-05-06 11:37 ` [PATCH v2 7/7] arm64: dts: rockchip: add mule i2c mux (0x18) on rk3588-jaguar Farouk Bouabid
@ 2024-05-06 20:40 ` Rob Herring (Arm)
  7 siblings, 0 replies; 12+ messages in thread
From: Rob Herring (Arm) @ 2024-05-06 20:40 UTC (permalink / raw)
  To: Farouk Bouabid
  Cc: linux-rockchip, linux-i2c, Heiko Stuebner, Andi Shyti,
	Quentin Schulz, linux-kernel, Wolfram Sang, Conor Dooley,
	Krzysztof Kozlowski, linux-arm-kernel, Peter Rosin,
	Quentin Schulz, devicetree


On Mon, 06 May 2024 13:37:51 +0200, Farouk Bouabid wrote:
> Mule is an mcu that emulates a set of i2c devices which are reachable
> through an i2c-mux.
> 
> The emulated devices share a single i2c address with the mux core itself
> where the requested register is what determines which logic is executed
> (muxing logic or device logic):
> 
> 1- The devices on the mux core can be selected (muxing functionality) by
> writing the appropriate device number to an i2c config register (0xff)
> that is not used by any device logic.
> 
> 2- Any access to a register other than the config register will be
> handled by the previously selected device.
> 
>       +-------------------------------------------------------+
>       |  Mule                                                 |
>       |        +---------------+                              |
>     ----+-(1)->|Config register|-----+                        |
>       | |      +---------------+     |                        |
>       | |                            V_                       |
>       | |                            |  \          +--------+ |
>       | |                            |   \-------->| dev #0 | |
>       | |                            |   |         +--------+ |
>       | |                            | M |-------->| dev #1 | |
>       | +-----------(2)------------->| U |         +--------+ |
>       |                              | X |-------->| dev #2 | |
>       |                              |   |         +--------+ |
>       |                              |   /-------->| dev #3 | |
>       |                              |__/          +--------+ |
>       +-------------------------------------------------------+
> 
> The current i2c-mux implementation does not allow the mux core to use the
> i2c address of a child device. As a workaround, A new i2c-adapter quirk is
> introduced to skip the check for conflict between a child device and the
> mux core i2c address when adding the child device.
> 
> This patch-series adds support for this multiplexer. Mule is integrated
> as part of rk3399-puma, px30-ringneck, rk3588-tiger and rk3588-jaguar
> boards.
> 
> To: Wolfram Sang <wsa+renesas@sang-engineering.com>
> To: Peter Rosin <peda@axentia.se>
> To: Andi Shyti <andi.shyti@kernel.org>
> To: Rob Herring <robh@kernel.org>
> To: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
> To: Conor Dooley <conor+dt@kernel.org>
> To: Quentin Schulz <quentin.schulz@theobroma-systems.com>
> To: Heiko Stuebner <heiko@sntech.de>
> To: Quentin Schulz <quentin.schulz@cherry.de>
> Cc: linux-i2c@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Cc: devicetree@vger.kernel.org
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-rockchip@lists.infradead.org
> Signed-off-by: Farouk Bouabid <farouk.bouabid@cherry.de>
> 
> Changes in v2:
> - Add i2c-adapter quirks to skip checking for conflict between the mux core
>   and a child device address.
> - Rename dt-binding to "tsd,mule-i2c-mux.yaml"
> - Add Mule description to kconfig
> - Fix indentation
> - Move device table after probe
> 
> - Link to v1: https://lore.kernel.org/r/20240426-dev-mule-i2c-mux-v1-0-045a482f6ffb@theobroma-systems.com
> 
> ---
> Farouk Bouabid (7):
>       i2c: mux: add the ability to share mux core address with child nodes
>       dt-bindings: i2c: mux: mule: add dt-bindings for mule i2c multiplexer
>       i2c: muxes: add support for mule i2c multiplexer
>       arm64: dts: rockchip: add mule i2c mux (0x18) on rk3399-puma
>       arm64: dts: rockchip: add mule i2c mux (0x18) on rk3588-tiger
>       arm64: dts: rockchip: add mule i2c mux (0x18) on px30-ringneck
>       arm64: dts: rockchip: add mule i2c mux (0x18) on rk3588-jaguar
> 
>  .../devicetree/bindings/i2c/tsd,mule-i2c-mux.yaml  |  80 +++++++++++
>  arch/arm64/boot/dts/rockchip/px30-ringneck.dtsi    |  20 ++-
>  arch/arm64/boot/dts/rockchip/rk3399-puma.dtsi      |  20 ++-
>  arch/arm64/boot/dts/rockchip/rk3588-jaguar.dts     |  19 ++-
>  arch/arm64/boot/dts/rockchip/rk3588-tiger.dtsi     |  19 ++-
>  drivers/i2c/i2c-core-base.c                        |   6 +-
>  drivers/i2c/i2c-mux.c                              |  25 +++-
>  drivers/i2c/muxes/Kconfig                          |  18 +++
>  drivers/i2c/muxes/Makefile                         |   1 +
>  drivers/i2c/muxes/i2c-mux-mule.c                   | 157 +++++++++++++++++++++
>  include/linux/i2c-mux.h                            |   1 +
>  include/linux/i2c.h                                |   7 +
>  12 files changed, 361 insertions(+), 12 deletions(-)
> ---
> base-commit: 23918f4e52d072b96a4d909e91298b8dd6ad4325
> change-id: 20240404-dev-mule-i2c-mux-9103cde07021
> 
> Best regards,
> --
> Farouk Bouabid <farouk.bouabid@cherry.de>
> 
> 
> 


My bot found new DTB warnings on the .dts files added or changed in this
series.

Some warnings may be from an existing SoC .dtsi. Or perhaps the warnings
are fixed by another series. Ultimately, it is up to the platform
maintainer whether these warnings are acceptable or not. No need to reply
unless the platform maintainer has comments.

If you already ran DT checks and didn't see these error(s), then
make sure dt-schema is up to date:

  pip3 install dtschema --upgrade


New warnings running 'make CHECK_DTBS=y rockchip/rk3588-jaguar.dtb' for 20240506-dev-mule-i2c-mux-v2-0-a91c954f65d7@cherry.de:

arch/arm64/boot/dts/rockchip/rk3588-jaguar.dtb: fan@18: '#cooling-cells' does not match any of the regexes: 'pinctrl-[0-9]+'
	from schema $id: http://devicetree.org/schemas/trivial-devices.yaml#






_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

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

* Re: [PATCH v2 1/7] i2c: mux: add the ability to share mux core address with child nodes
  2024-05-06 11:37 ` [PATCH v2 1/7] i2c: mux: add the ability to share mux core address with child nodes Farouk Bouabid
@ 2024-05-06 21:26   ` Peter Rosin
  2024-05-07  9:48     ` Quentin Schulz
  0 siblings, 1 reply; 12+ messages in thread
From: Peter Rosin @ 2024-05-06 21:26 UTC (permalink / raw)
  To: Farouk Bouabid, Wolfram Sang, Andi Shyti, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Quentin Schulz,
	Heiko Stuebner, Quentin Schulz
  Cc: linux-i2c, linux-kernel, devicetree, linux-arm-kernel, linux-rockchip

Hi!

Regarding the subject (and elsewhere) I think of "mux core" as roughly
the code in the i2c-mux.c file. So, for me, the "mux core" does not have
an address, it is a mux "driver instance" or "device" that sits on the
I2C address that you need to share.

2024-05-06 at 13:37, Farouk Bouabid wrote:
> Allow the mux core (if it's an i2c device) to have the same address as
> a child device. This is useful when the mux core can only use an i2c
> address that is used by a child device because no other addresses are
> free to use. eg. the mux core can only use address 0x18 which is used by
> amc6821 connected to the mux.

Use I2C in text, not i2c (applies to other patches as well).

> 
> Signed-off-by: Farouk Bouabid <farouk.bouabid@cherry.de>
> ---
>  drivers/i2c/i2c-core-base.c |  6 +++++-
>  drivers/i2c/i2c-mux.c       | 25 ++++++++++++++++++++++++-
>  include/linux/i2c-mux.h     |  1 +
>  include/linux/i2c.h         |  7 +++++++
>  4 files changed, 37 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
> index ff5c486a1dbb..ce2425b0486d 100644
> --- a/drivers/i2c/i2c-core-base.c
> +++ b/drivers/i2c/i2c-core-base.c
> @@ -821,9 +821,13 @@ static int i2c_check_mux_children(struct device *dev, void *addrp)
>  static int i2c_check_addr_busy(struct i2c_adapter *adapter, int addr)
>  {
>  	struct i2c_adapter *parent = i2c_parent_is_i2c_adapter(adapter);
> +	bool skip_check = false;
>  	int result = 0;
>  
> -	if (parent)
> +	if (adapter->quirks && adapter->quirks->flags & I2C_AQ_SKIP_ADDR_CHECK)
> +		skip_check = adapter->quirks->skip_addr_in_parent == addr;
> +
> +	if (parent && !skip_check)
>  		result = i2c_check_mux_parents(parent, addr);
>  
>  	if (!result)
> diff --git a/drivers/i2c/i2c-mux.c b/drivers/i2c/i2c-mux.c
> index 57ff09f18c37..bdb75a130cab 100644
> --- a/drivers/i2c/i2c-mux.c
> +++ b/drivers/i2c/i2c-mux.c
> @@ -334,7 +334,30 @@ int i2c_mux_add_adapter(struct i2c_mux_core *muxc,
>  	priv->adap.dev.parent = &parent->dev;
>  	priv->adap.retries = parent->retries;
>  	priv->adap.timeout = parent->timeout;
> -	priv->adap.quirks = parent->quirks;
> +	/*
> +	 * When creating the adapter, the node devices are checked for i2c address
> +	 * match with other devices on the parent adapter, among which is the mux core itself.
> +	 * If a match is found the node device is not probed successfully.
> +	 * Allow the mux to have the same address as a child device by skipping this check.
> +	 */
> +	if (muxc->share_addr_with_children && muxc->dev->type == &i2c_client_type) {
> +		struct i2c_adapter_quirks *quirks = devm_kzalloc(muxc->dev,
> +								 sizeof(*quirks), GFP_KERNEL);
> +		struct i2c_client *client = to_i2c_client(muxc->dev);

The above section has overly long lines without good reason. Please
rewrite as something like this:

	/*
	 * When creating the adapter, the node devices are checked for I2C
	 * address match with other devices on the parent adapter, among
	 * which is the mux device itself. If a match is found the node
	 * device is not probed successfully. Allow the mux to have the
	 * same address as a child device by skipping this check.
	 */
	if (muxc->share_addr_with_children &&
	    muxc->dev->type == &i2c_client_type)
	{
		struct i2c_client *client = to_i2c_client(muxc->dev);
		struct i2c_adapter_quirks *quirks;

		quirks = devm_kzalloc(muxc->dev, sizeof(*quirks), GFP_KERNEL);

I also wonder if that second condition (...->type == &i2c_client_type) should
be a WARN_ON_ONCE? I don't see how the flag can be set sanely on an adapter
that is not itself an I2C client. Can it?

> +
> +		if (!quirks)
> +			return -ENOMEM;
> +
> +		if (parent->quirks)
> +			memcpy(quirks, parent->quirks, sizeof(*quirks));
> +
> +		quirks->flags |= I2C_AQ_SKIP_ADDR_CHECK;
> +		quirks->skip_addr_in_parent = client->addr;
> +		priv->adap.quirks = quirks;

The I2C_AQ_SKIP_ADDR_CHECK flag should probably not be propagated?

	} else if (priv->adap.quirks &&
		   priv->adap.quirks->flags & I2C_AQ_SKIP_ADDR_CHECK)
	{
		struct i2c_adapter_quirks *quirks;

		quirks = devm_kzalloc(muxc->dev, sizeof(*quirks), GFP_KERNEL);
		if (!quirks)
			return -ENOMEM;

		memcpy(quirks, parent->quirks, sizeof(*quirks));
		quirks->flags &= ~I2C_AQ_SKIP_ADDR_CHECK;
		priv->adap.quirks = quirks;

(My code written directly in the mua, there's probably something silly in
there...)

Cheers,
Peter

> +	} else {
> +		priv->adap.quirks = parent->quirks;
> +	}
> +
>  	if (muxc->mux_locked)
>  		priv->adap.lock_ops = &i2c_mux_lock_ops;
>  	else
> diff --git a/include/linux/i2c-mux.h b/include/linux/i2c-mux.h
> index 98ef73b7c8fd..17ac68bf1703 100644
> --- a/include/linux/i2c-mux.h
> +++ b/include/linux/i2c-mux.h
> @@ -21,6 +21,7 @@ struct i2c_mux_core {
>  	unsigned int mux_locked:1;
>  	unsigned int arbitrator:1;
>  	unsigned int gate:1;
> +	unsigned int share_addr_with_children:1;
>  
>  	void *priv;
>  
> diff --git a/include/linux/i2c.h b/include/linux/i2c.h
> index 5e6cd43a6dbd..c3acbaaadae9 100644
> --- a/include/linux/i2c.h
> +++ b/include/linux/i2c.h
> @@ -670,6 +670,7 @@ int i2c_generic_scl_recovery(struct i2c_adapter *adap);
>   * @max_read_len: maximum length of a read message
>   * @max_comb_1st_msg_len: maximum length of the first msg in a combined message
>   * @max_comb_2nd_msg_len: maximum length of the second msg in a combined message
> + * @skip_addr_in_parent: No conflict check on parent adapter for a given address
>   *
>   * Note about combined messages: Some I2C controllers can only send one message
>   * per transfer, plus something called combined message or write-then-read.
> @@ -690,6 +691,7 @@ struct i2c_adapter_quirks {
>  	u16 max_read_len;
>  	u16 max_comb_1st_msg_len;
>  	u16 max_comb_2nd_msg_len;
> +	unsigned short skip_addr_in_parent;
>  };
>  
>  /* enforce max_num_msgs = 2 and use max_comb_*_len for length checks */
> @@ -711,6 +713,11 @@ struct i2c_adapter_quirks {
>  #define I2C_AQ_NO_ZERO_LEN		(I2C_AQ_NO_ZERO_LEN_READ | I2C_AQ_NO_ZERO_LEN_WRITE)
>  /* adapter cannot do repeated START */
>  #define I2C_AQ_NO_REP_START		BIT(7)
> +/**
> + * do not check for conflict on a given address
> + * used accordingly with "struct i2c_adapter_quirks.skip_addr_in_parent"
> + */
> +#define I2C_AQ_SKIP_ADDR_CHECK	BIT(8)
>  
>  /*
>   * i2c_adapter is the structure used to identify a physical i2c bus along
> 

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

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

* Re: [PATCH v2 1/7] i2c: mux: add the ability to share mux core address with child nodes
  2024-05-06 21:26   ` Peter Rosin
@ 2024-05-07  9:48     ` Quentin Schulz
  0 siblings, 0 replies; 12+ messages in thread
From: Quentin Schulz @ 2024-05-07  9:48 UTC (permalink / raw)
  To: Peter Rosin, Farouk Bouabid, Wolfram Sang, Andi Shyti,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Heiko Stuebner,
	Quentin Schulz
  Cc: linux-i2c, linux-kernel, devicetree, linux-arm-kernel, linux-rockchip

Hi Peter,

On 5/6/24 11:26 PM, Peter Rosin wrote:
> Hi!
> 
> Regarding the subject (and elsewhere) I think of "mux core" as roughly
> the code in the i2c-mux.c file. So, for me, the "mux core" does not have
> an address, it is a mux "driver instance" or "device" that sits on the
> I2C address that you need to share.
> 

I'm the one who suggested mux core here (privately) :)

The issue is that in my head a mux device is the i2c adapter/bus (from 
i2c-mux.yaml dt binding example):

"""
     i2c {
         #address-cells = <1>;
         #size-cells = <0>;

         i2c-mux@70 {
             compatible = "nxp,pca9548";
             reg = <0x70>;
             #address-cells = <1>;
             #size-cells = <0>;

             i2c@3 {
                 #address-cells = <1>;
                 #size-cells = <0>;
                 reg = <3>;

                 gpio@20 {
                     compatible = "nxp,pca9555";
                     gpio-controller;
                     #gpio-cells = <2>;
                     reg = <0x20>;
                 };
             };
             i2c@4 {
                 #address-cells = <1>;
                 #size-cells = <0>;
                 reg = <4>;

                 gpio@20 {
                     compatible = "nxp,pca9555";
                     gpio-controller;
                     #gpio-cells = <2>;
                     reg = <0x20>;
                 };
             };
         };
     };
"""

"mux core" here would refer to i2c-mux@70, "mux device"/"mux" i2c@3 or 
i2c@4. E.g. when I'm saying "in mux 3", I'm talking about i2c@3 here.

For me a driver instance is a device, so "mux driver instance" would be 
a "mux device". Ah... naming is hard. Anyway, up to you, I just wanted 
to make sure we're talking about the same thing and there's no confusion 
here.

[...]
> I also wonder if that second condition (...->type == &i2c_client_type) should
> be a WARN_ON_ONCE? I don't see how the flag can be set sanely on an adapter
> that is not itself an I2C client. Can it?
> 

Agreed, good suggestion here... Though... 
https://lwn.net/Articles/969923/ it seems new additions of WARN_ON are 
now discouraged? Not looking to start a discussion here about whether 
WARN_ON is good or bad, merely pointing at this if it was missed somehow.

>> +
>> +		if (!quirks)
>> +			return -ENOMEM;
>> +
>> +		if (parent->quirks)
>> +			memcpy(quirks, parent->quirks, sizeof(*quirks));
>> +
>> +		quirks->flags |= I2C_AQ_SKIP_ADDR_CHECK;
>> +		quirks->skip_addr_in_parent = client->addr;
>> +		priv->adap.quirks = quirks;
> 
> The I2C_AQ_SKIP_ADDR_CHECK flag should probably not be propagated?
> 

Oh... you mean if we have a mux on an i2c adapter of a mux and the 
adapters handled by the parent mux have SKIP_ADDR set and we don't want 
the adapters handled by the leaf mux to have this flag as well? Is that 
what you meant?

Cheers,
Quentin

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

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

end of thread, other threads:[~2024-05-07  9:49 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-05-06 11:37 [PATCH v2 0/7] Add Mule I2C multiplexer support Farouk Bouabid
2024-05-06 11:37 ` [PATCH v2 1/7] i2c: mux: add the ability to share mux core address with child nodes Farouk Bouabid
2024-05-06 21:26   ` Peter Rosin
2024-05-07  9:48     ` Quentin Schulz
2024-05-06 11:37 ` [PATCH v2 2/7] dt-bindings: i2c: mux: mule: add dt-bindings for mule i2c multiplexer Farouk Bouabid
2024-05-06 20:26   ` Rob Herring (Arm)
2024-05-06 11:37 ` [PATCH v2 3/7] i2c: muxes: add support " Farouk Bouabid
2024-05-06 11:37 ` [PATCH v2 4/7] arm64: dts: rockchip: add mule i2c mux (0x18) on rk3399-puma Farouk Bouabid
2024-05-06 11:37 ` [PATCH v2 5/7] arm64: dts: rockchip: add mule i2c mux (0x18) on rk3588-tiger Farouk Bouabid
2024-05-06 11:37 ` [PATCH v2 6/7] arm64: dts: rockchip: add mule i2c mux (0x18) on px30-ringneck Farouk Bouabid
2024-05-06 11:37 ` [PATCH v2 7/7] arm64: dts: rockchip: add mule i2c mux (0x18) on rk3588-jaguar Farouk Bouabid
2024-05-06 20:40 ` [PATCH v2 0/7] Add Mule I2C multiplexer support Rob Herring (Arm)

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).