[PATCH v4 0/9] Add Mule MFD support

[PATCH v4 0/9] Add Mule MFD support

[Farouk Bouabid]

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

The mux and amc6821 combined make the Mule multi-function device (@0x18)

The devices on the mux can be selected by writing the appropriate
device number to an I2C config register (0xff) that is not used by
amc6821 logic.

The selected device on the mux can be accessed for reading and writing
at I2C address 0x6f.

      +--------+----------------+------------------------------+
      |  Mule         (MFD)                                    |
 0x18 |        +----------------+                              |
--------+----->|    amc6821     |                              |
      | |      +----------------+                              |
      | +----->| Mux config reg |-----+                        |
      |        +----------------+     |                        |
      |                               V__          +---------+ |
      |                              |   \-------->| isl1208 | |
      |                              |   |         +---------+ |
 0x6f |                              | M |-------->| dev #1  | |
------------------------------------>| U |         +---------+ |
      |                              | X |-------->| dev #2  | |
      |                              |   |         +---------+ |
      |                              |   /-------->| dev #3  | |
      |                              |__/          +---------+ |
      +--------------------------------------------------------+

This patch-series adds support for the Mule MFD and I2C multiplexer
as part of rk3399-puma, px30-ringneck, rk3588-tiger and rk3588-jaguar
boards.

Signed-off-by: Farouk Bouabid <farouk.bouabid@cherry.de>

Changes in v4:
- Drop the previously added i2c adapter quirks
- Add platform driver probe to amc6821.
- Change mule-i2c-mux driver to a platform driver
- Add dev_probe_err in mule-i2c-mux driver
- Add support for tsd,mule in simple-mfd-i2c
- Add tsd,mule mfd to supported dts

- Link to v3: https://lore.kernel.org/r/20240611-dev-mule-i2c-mux-v3-0-08d26a28e001@cherry.de

Changes in v3:
- Change "i2c" in comments/commit-logs to "I2C"
- Fix long line-length
- Warn when "share_addr_with_children" is set and the Mux is not an I2C device
- Fix/stop propagating "I2C_AQ_SKIP_ADDR_CHECK" flag if "share_addr_with_children"
  is not set.
- Fix "old_fw" variable is used to indicate the reversed meaning.

- Link to v2: https://lore.kernel.org/r/20240506-dev-mule-i2c-mux-v2-0-a91c954f65d7@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 (9):
      hwmon: (amc6821) add platform driver probe
      hwmon: (amc6821) dev_err using amc6821 device struct
      dt-bindings: mfd: add support for mule
      i2c: muxes: add support for mule i2c multiplexer
      mfd: simple-mfd-i2c: Add support for tsd,mule
      arm64: dts: rockchip: add mule mfd (0x18) on rk3588-jaguar
      arm64: dts: rockchip: add mule mfd (0x18) on rk3399-puma
      arm64: dts: rockchip: add mule mfd (0x18) on rk3588-tiger
      arm64: dts: rockchip: add mule mfd (0x18) on px30-ringneck

 .../devicetree/bindings/i2c/tsd,mule-i2c-mux.yaml  |  48 +++++++
 .../devicetree/bindings/mfd/tsd,mule.yaml          |  82 +++++++++++
 arch/arm64/boot/dts/rockchip/px30-ringneck.dtsi    |  33 ++++-
 arch/arm64/boot/dts/rockchip/rk3399-puma.dtsi      |  33 ++++-
 arch/arm64/boot/dts/rockchip/rk3588-jaguar.dts     |  34 ++++-
 arch/arm64/boot/dts/rockchip/rk3588-tiger.dtsi     |  32 ++++-
 drivers/hwmon/amc6821.c                            |  88 +++++++-----
 drivers/i2c/muxes/Kconfig                          |  17 +++
 drivers/i2c/muxes/Makefile                         |   1 +
 drivers/i2c/muxes/i2c-mux-mule.c                   | 151 +++++++++++++++++++++
 drivers/mfd/simple-mfd-i2c.c                       |   1 +
 11 files changed, 461 insertions(+), 59 deletions(-)
---
base-commit: 915c5c8d4bc3c72020b9526cb3cbbd8e14318fc4
change-id: 20240404-dev-mule-i2c-mux-9103cde07021

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

[PATCH v4 1/9] hwmon: (amc6821) add platform driver probe

[Farouk Bouabid]

Prepare for probing amc6821 in an I2C MFD device, by adding support to
probing amc6821 as a platform device.

Signed-off-by: Farouk Bouabid <farouk.bouabid@cherry.de>
---
 drivers/hwmon/amc6821.c | 26 ++++++++++++++++++++++++--
 1 file changed, 24 insertions(+), 2 deletions(-)

diff --git a/drivers/hwmon/amc6821.c b/drivers/hwmon/amc6821.c
index 9b02b304c2f5..464991fd7fc9 100644
--- a/drivers/hwmon/amc6821.c
+++ b/drivers/hwmon/amc6821.c
@@ -18,6 +18,7 @@
 #include <linux/hwmon-sysfs.h>
 #include <linux/err.h>
 #include <linux/mutex.h>
+#include <linux/platform_device.h>
 
 /*
  * Addresses to scan.
@@ -900,9 +901,8 @@ static int amc6821_init_client(struct i2c_client *client)
 	return 0;
 }
 
-static int amc6821_probe(struct i2c_client *client)
+static int probe(struct device *dev, struct i2c_client *client)
 {
-	struct device *dev = &client->dev;
 	struct amc6821_data *data;
 	struct device *hwmon_dev;
 	int err;
@@ -927,6 +927,18 @@ static int amc6821_probe(struct i2c_client *client)
 	return PTR_ERR_OR_ZERO(hwmon_dev);
 }
 
+static int amc6821_probe(struct i2c_client *client)
+{
+	return probe(&client->dev, client);
+}
+
+static int amc6821_platform_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+
+	return probe(dev, to_i2c_client(dev->parent));
+}
+
 static const struct i2c_device_id amc6821_id[] = {
 	{ "amc6821", amc6821 },
 	{ }
@@ -958,6 +970,16 @@ static struct i2c_driver amc6821_driver = {
 
 module_i2c_driver(amc6821_driver);
 
+static struct platform_driver amc6821_platform_driver = {
+	.driver = {
+		.name	= "amc6821",
+		.of_match_table = of_match_ptr(amc6821_of_match),
+	},
+	.probe = amc6821_platform_probe,
+};
+
+module_platform_driver(amc6821_platform_driver);
+
 MODULE_LICENSE("GPL");
 MODULE_AUTHOR("T. Mertelj <tomaz.mertelj@guest.arnes.si>");
 MODULE_DESCRIPTION("Texas Instruments amc6821 hwmon driver");

-- 
2.34.1

[PATCH v4 2/9] hwmon: (amc6821) dev_err using amc6821 device struct

[Farouk Bouabid]

If probed as platform device, the i2c_client.dev is the parent device
(eg. MFD) and not the actual amc6821 device. Use the latter to log errors
instead.

Signed-off-by: Farouk Bouabid <farouk.bouabid@cherry.de>
---
 drivers/hwmon/amc6821.c | 62 +++++++++++++++++++++++++------------------------
 1 file changed, 32 insertions(+), 30 deletions(-)

diff --git a/drivers/hwmon/amc6821.c b/drivers/hwmon/amc6821.c
index 464991fd7fc9..17456d7c7cfe 100644
--- a/drivers/hwmon/amc6821.c
+++ b/drivers/hwmon/amc6821.c
@@ -290,7 +290,7 @@ static ssize_t temp_store(struct device *dev, struct device_attribute *attr,
 	mutex_lock(&data->update_lock);
 	data->temp[ix] = val;
 	if (i2c_smbus_write_byte_data(client, temp_reg[ix], data->temp[ix])) {
-		dev_err(&client->dev, "Register write error, aborting.\n");
+		dev_err(dev, "Register write error, aborting.\n");
 		count = -EIO;
 	}
 	mutex_unlock(&data->update_lock);
@@ -389,10 +389,10 @@ static ssize_t pwm1_enable_store(struct device *dev,
 	mutex_lock(&data->update_lock);
 	config = i2c_smbus_read_byte_data(client, AMC6821_REG_CONF1);
 	if (config < 0) {
-			dev_err(&client->dev,
+		dev_err(dev,
 			"Error reading configuration register, aborting.\n");
-			count = config;
-			goto unlock;
+		count = config;
+		goto unlock;
 	}
 
 	switch (val) {
@@ -413,9 +413,9 @@ static ssize_t pwm1_enable_store(struct device *dev,
 		goto unlock;
 	}
 	if (i2c_smbus_write_byte_data(client, AMC6821_REG_CONF1, config)) {
-			dev_err(&client->dev,
+		dev_err(dev,
 			"Configuration register write error, aborting.\n");
-			count = -EIO;
+		count = -EIO;
 	}
 unlock:
 	mutex_unlock(&data->update_lock);
@@ -459,11 +459,13 @@ static ssize_t pwm1_auto_point_pwm_show(struct device *dev,
 	return sprintf(buf, "%d\n", data->pwm1_auto_point_pwm[ix]);
 }
 
-static inline ssize_t set_slope_register(struct i2c_client *client,
+static inline ssize_t set_slope_register(struct device *dev,
 		u8 reg,
 		u8 dpwm,
 		u8 *ptemp)
 {
+	struct amc6821_data *data = dev_get_drvdata(dev);
+	struct i2c_client *client = data->client;
 	int dt;
 	u8 tmp;
 
@@ -475,7 +477,7 @@ static inline ssize_t set_slope_register(struct i2c_client *client,
 	tmp |= (ptemp[1] & 0x7C) << 1;
 	if (i2c_smbus_write_byte_data(client,
 			reg, tmp)) {
-		dev_err(&client->dev, "Register write error, aborting.\n");
+		dev_err(dev, "Register write error, aborting.\n");
 		return -EIO;
 	}
 	return 0;
@@ -525,7 +527,7 @@ static ssize_t temp_auto_point_temp_store(struct device *dev,
 					client,
 					AMC6821_REG_PSV_TEMP,
 					ptemp[0])) {
-				dev_err(&client->dev,
+				dev_err(dev,
 					"Register write error, aborting.\n");
 				count = -EIO;
 		}
@@ -544,7 +546,7 @@ static ssize_t temp_auto_point_temp_store(struct device *dev,
 		goto EXIT;
 	}
 	dpwm = data->pwm1_auto_point_pwm[2] - data->pwm1_auto_point_pwm[1];
-	if (set_slope_register(client, reg, dpwm, ptemp))
+	if (set_slope_register(dev, reg, dpwm, ptemp))
 		count = -EIO;
 
 EXIT:
@@ -568,17 +570,17 @@ static ssize_t pwm1_auto_point_pwm_store(struct device *dev,
 	data->pwm1_auto_point_pwm[1] = clamp_val(val, 0, 254);
 	if (i2c_smbus_write_byte_data(client, AMC6821_REG_DCY_LOW_TEMP,
 			data->pwm1_auto_point_pwm[1])) {
-		dev_err(&client->dev, "Register write error, aborting.\n");
+		dev_err(dev, "Register write error, aborting.\n");
 		count = -EIO;
 		goto EXIT;
 	}
 	dpwm = data->pwm1_auto_point_pwm[2] - data->pwm1_auto_point_pwm[1];
-	if (set_slope_register(client, AMC6821_REG_LTEMP_FAN_CTRL, dpwm,
+	if (set_slope_register(dev, AMC6821_REG_LTEMP_FAN_CTRL, dpwm,
 			data->temp1_auto_point_temp)) {
 		count = -EIO;
 		goto EXIT;
 	}
-	if (set_slope_register(client, AMC6821_REG_RTEMP_FAN_CTRL, dpwm,
+	if (set_slope_register(dev, AMC6821_REG_RTEMP_FAN_CTRL, dpwm,
 			data->temp2_auto_point_temp)) {
 		count = -EIO;
 		goto EXIT;
@@ -626,13 +628,13 @@ static ssize_t fan_store(struct device *dev, struct device_attribute *attr,
 	data->fan[ix] = (u16) clamp_val(val, 1, 0xFFFF);
 	if (i2c_smbus_write_byte_data(client, fan_reg_low[ix],
 			data->fan[ix] & 0xFF)) {
-		dev_err(&client->dev, "Register write error, aborting.\n");
+		dev_err(dev, "Register write error, aborting.\n");
 		count = -EIO;
 		goto EXIT;
 	}
 	if (i2c_smbus_write_byte_data(client,
 			fan_reg_hi[ix], data->fan[ix] >> 8)) {
-		dev_err(&client->dev, "Register write error, aborting.\n");
+		dev_err(dev, "Register write error, aborting.\n");
 		count = -EIO;
 	}
 EXIT:
@@ -661,7 +663,7 @@ static ssize_t fan1_div_store(struct device *dev,
 	mutex_lock(&data->update_lock);
 	config = i2c_smbus_read_byte_data(client, AMC6821_REG_CONF4);
 	if (config < 0) {
-		dev_err(&client->dev,
+		dev_err(dev,
 			"Error reading configuration register, aborting.\n");
 		count = config;
 		goto EXIT;
@@ -680,7 +682,7 @@ static ssize_t fan1_div_store(struct device *dev,
 		goto EXIT;
 	}
 	if (i2c_smbus_write_byte_data(client, AMC6821_REG_CONF4, config)) {
-		dev_err(&client->dev,
+		dev_err(dev,
 			"Configuration register write error, aborting.\n");
 		count = -EIO;
 	}
@@ -815,7 +817,7 @@ static int amc6821_detect(
 	return 0;
 }
 
-static int amc6821_init_client(struct i2c_client *client)
+static int amc6821_init_client(struct device *dev, struct i2c_client *client)
 {
 	int config;
 	int err = -EIO;
@@ -824,16 +826,16 @@ static int amc6821_init_client(struct i2c_client *client)
 		config = i2c_smbus_read_byte_data(client, AMC6821_REG_CONF4);
 
 		if (config < 0) {
-				dev_err(&client->dev,
-			"Error reading configuration register, aborting.\n");
-				return err;
+			dev_err(dev,
+				"Error reading configuration register, aborting.\n");
+			return err;
 		}
 
 		config |= AMC6821_CONF4_MODE;
 
 		if (i2c_smbus_write_byte_data(client, AMC6821_REG_CONF4,
 				config)) {
-			dev_err(&client->dev,
+			dev_err(dev,
 			"Configuration register write error, aborting.\n");
 			return err;
 		}
@@ -841,18 +843,18 @@ static int amc6821_init_client(struct i2c_client *client)
 		config = i2c_smbus_read_byte_data(client, AMC6821_REG_CONF3);
 
 		if (config < 0) {
-			dev_err(&client->dev,
+			dev_err(dev,
 			"Error reading configuration register, aborting.\n");
 			return err;
 		}
 
-		dev_info(&client->dev, "Revision %d\n", config & 0x0f);
+		dev_info(dev, "Revision %d\n", config & 0x0f);
 
 		config &= ~AMC6821_CONF3_THERM_FAN_EN;
 
 		if (i2c_smbus_write_byte_data(client, AMC6821_REG_CONF3,
 				config)) {
-			dev_err(&client->dev,
+			dev_err(dev,
 			"Configuration register write error, aborting.\n");
 			return err;
 		}
@@ -860,7 +862,7 @@ static int amc6821_init_client(struct i2c_client *client)
 		config = i2c_smbus_read_byte_data(client, AMC6821_REG_CONF2);
 
 		if (config < 0) {
-			dev_err(&client->dev,
+			dev_err(dev,
 			"Error reading configuration register, aborting.\n");
 			return err;
 		}
@@ -870,7 +872,7 @@ static int amc6821_init_client(struct i2c_client *client)
 		config &= ~AMC6821_CONF2_RTOIE;
 		if (i2c_smbus_write_byte_data(client,
 				AMC6821_REG_CONF2, config)) {
-			dev_err(&client->dev,
+			dev_err(dev,
 			"Configuration register write error, aborting.\n");
 			return err;
 		}
@@ -878,7 +880,7 @@ static int amc6821_init_client(struct i2c_client *client)
 		config = i2c_smbus_read_byte_data(client, AMC6821_REG_CONF1);
 
 		if (config < 0) {
-			dev_err(&client->dev,
+			dev_err(dev,
 			"Error reading configuration register, aborting.\n");
 			return err;
 		}
@@ -893,7 +895,7 @@ static int amc6821_init_client(struct i2c_client *client)
 
 		if (i2c_smbus_write_byte_data(
 				client, AMC6821_REG_CONF1, config)) {
-			dev_err(&client->dev,
+			dev_err(dev,
 			"Configuration register write error, aborting.\n");
 			return err;
 		}
@@ -917,7 +919,7 @@ static int probe(struct device *dev, struct i2c_client *client)
 	/*
 	 * Initialize the amc6821 chip
 	 */
-	err = amc6821_init_client(client);
+	err = amc6821_init_client(dev, client);
 	if (err)
 		return err;
 

-- 
2.34.1

[PATCH v4 3/9] dt-bindings: mfd: add support for mule

[Farouk Bouabid]

Mule is an MCU that emulates a set of I2C devices, among which an amc6821
device and an I2C mux that exposes more emulated devices. These two devices
share the same I2C address. Only the I2C mux uses register (0xff) as a
config register.

Add dt-binding support for the Mule I2C multi-function device.

Signed-off-by: Farouk Bouabid <farouk.bouabid@cherry.de>
---
 .../devicetree/bindings/i2c/tsd,mule-i2c-mux.yaml  | 48 +++++++++++++
 .../devicetree/bindings/mfd/tsd,mule.yaml          | 82 ++++++++++++++++++++++
 2 files changed, 130 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..ac9dfc936272
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/tsd,mule-i2c-mux.yaml
@@ -0,0 +1,48 @@
+# 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: |
+  This module is part of the Mule I2C multi-function device. For more
+  details see ../mfd/tsd,mule.yaml.
+
+  Mule I2C-mux configures the active device that can be accessed on address
+  0x6f through the config register.
+
+      +--------------------------------------------------+
+      | Mule                                             |
+  0x18|    +---------------+                             |
+  -------->|Config register|----+                        |
+      |    +---------------+    |                        |
+      |                         V_                       |
+      |                        |  \          +--------+  |
+      |                        |   \-------->| dev #0 |  |
+      |                        |   |         +--------+  |
+  0x6f|                        | M |-------->| dev #1 |  |
+  ---------------------------->| U |         +--------+  |
+      |                        | X |-------->| dev #2 |  |
+      |                        |   |         +--------+  |
+      |                        |   /-------->| dev #3 |  |
+      |                        |__/          +--------+  |
+      +--------------------------------------------------+
+
+
+allOf:
+  - $ref: /schemas/i2c/i2c-mux.yaml#
+
+properties:
+  compatible:
+    const: tsd,mule-i2c-mux
+
+required:
+  - compatible
+
+unevaluatedProperties: false
diff --git a/Documentation/devicetree/bindings/mfd/tsd,mule.yaml b/Documentation/devicetree/bindings/mfd/tsd,mule.yaml
new file mode 100644
index 000000000000..ab532340a17c
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/tsd,mule.yaml
@@ -0,0 +1,82 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mfd/tsd,mule.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Mule I2C multi function device
+
+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, among which an amc6821
+  device and an I2C mux that exposes more emulated devices. These two devices
+  share the same I2C address. Only the I2C mux uses register (0xff) as a config
+  register.
+
+properties:
+  compatible:
+    const: tsd,mule
+
+  reg:
+    maxItems: 1
+
+  "#address-cells":
+    const: 1
+
+  "#size-cells":
+    const: 0
+
+patternProperties:
+  "^fan(@[0-9a-f]+)?$":
+    $ref: /schemas/trivial-devices.yaml
+
+  "^i2c-mux(@[0-9a-f]+)?$":
+    $ref: /schemas/i2c/tsd,mule-i2c-mux.yaml
+
+required:
+  - "#address-cells"
+  - "#size-cells"
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        mule@18 {
+            compatible = "tsd,mule";
+            reg = <0x18>;
+            #address-cells = <1>;
+            #size-cells = <0>;
+
+            fan@18 {
+                compatible = "ti,amc6821";
+                reg = <0x18>;
+            };
+
+            i2c-mux {
+                compatible = "tsd,mule-i2c-mux";
+                #address-cells = <1>;
+                #size-cells = <0>;
+
+                i2c@0 {
+                    reg = <0x0>;
+                    #address-cells = <1>;
+                    #size-cells = <0>;
+
+                    rtc@6f {
+                        compatible = "isil,isl1208";
+                        reg = <0x6f>;
+                    };
+                };
+            };
+        };
+    };
+...

-- 
2.34.1

[PATCH v4 4/9] i2c: muxes: add support for mule i2c multiplexer

[Farouk Bouabid]

Mule is an MCU that emulates a set of I2C devices, among which devices
that are reachable through an I2C-mux. The devices on the mux can be
selected by writing the appropriate device number to an I2C config
register.

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

diff --git a/drivers/i2c/muxes/Kconfig b/drivers/i2c/muxes/Kconfig
index db1b9057612a..e535cd9ea712 100644
--- a/drivers/i2c/muxes/Kconfig
+++ b/drivers/i2c/muxes/Kconfig
@@ -119,4 +119,21 @@ 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
+	select MFD_SIMPLE_MFD_I2C
+	help
+	  Mule is an MCU that emulates a set of I2C devices, among which devices
+	  that are reachable through an I2C-mux. The devices on the mux can be
+	  selected by writing the appropriate device number to an I2C config
+	  register.
+
+	  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..530403adea50
--- /dev/null
+++ b/drivers/i2c/muxes/i2c-mux-mule.c
@@ -0,0 +1,151 @@
+// 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/platform_device.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 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 platform_device *pdev)
+{
+	struct device *mux_dev = &pdev->dev;
+	struct mule_i2c_reg_mux *priv;
+	struct i2c_client *client;
+	struct i2c_mux_core *muxc;
+	struct device_node *dev;
+	unsigned int readback;
+	int ndev, ret;
+	bool old_fw;
+
+	/* Count devices on the mux */
+	ndev = of_get_child_count(mux_dev->of_node);
+	dev_dbg(mux_dev, "%d devices on the mux\n", ndev);
+
+	client = to_i2c_client(mux_dev->parent);
+
+	muxc = i2c_mux_alloc(client->adapter, mux_dev, ndev, sizeof(*priv),
+			     I2C_MUX_LOCKED, mux_select, mux_deselect);
+	if (!muxc)
+		return -ENOMEM;
+
+	priv = i2c_mux_priv(muxc);
+
+	priv->regmap = dev_get_regmap(mux_dev->parent, NULL);
+	if (IS_ERR(priv->regmap))
+		return dev_err_probe(mux_dev, PTR_ERR(priv->regmap),
+				     "No parent i2c register map\n");
+
+	platform_set_drvdata(pdev, muxc);
+
+	/*
+	 * MUX_DEFAULT_DEV 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, MUX_DEFAULT_DEV);
+	if (ret)
+		return ret;
+
+	ret = regmap_read(priv->regmap, MUX_CONFIG_REG, &readback);
+	if (ret)
+		return ret;
+
+	old_fw = (readback != MUX_DEFAULT_DEV);
+
+	ret = devm_add_action_or_reset(mux_dev, mux_remove, muxc);
+	if (ret)
+		return ret;
+
+	/* Create device adapters */
+	for_each_child_of_node(mux_dev->of_node, dev) {
+		u32 reg;
+
+		ret = of_property_read_u32(dev, "reg", &reg);
+		if (ret)
+			return dev_err_probe(mux_dev, ret,
+					     "No reg property found for %s\n",
+					     of_node_full_name(dev));
+
+		if (old_fw && reg != 0) {
+			dev_warn(mux_dev,
+				 "Mux is not supported, please update Mule FW\n");
+			continue;
+		}
+
+		ret = mux_select(muxc, reg);
+		if (ret) {
+			dev_warn(mux_dev,
+				 "Device %d not supported, please update Mule FW\n", reg);
+			continue;
+		}
+
+		ret = i2c_mux_add_adapter(muxc, 0, reg);
+		if (ret)
+			return dev_err_probe(mux_dev, ret,
+					     "Failed to add i2c mux adapter %d\n", reg);
+	}
+
+	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 platform_driver mule_i2c_mux_driver = {
+	.driver		= {
+		.name	= "mule-i2c-mux",
+		.of_match_table = mule_i2c_mux_of_match,
+	},
+	.probe		= mule_i2c_mux_probe,
+};
+
+module_platform_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

[PATCH v4 5/9] mfd: simple-mfd-i2c: Add support for tsd,mule

[Farouk Bouabid]

Mule is an MCU that emulates a set of I2C devices, among which an amc6821
device and an I2C mux that exposes more emulated devices. These two devices
share the same I2C address. Only the I2C mux uses register (0xff) as a
config register.

Add a simple-mfd-i2c driver support for these two sub-devices.

Signed-off-by: Farouk Bouabid <farouk.bouabid@cherry.de>
---
 drivers/mfd/simple-mfd-i2c.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/mfd/simple-mfd-i2c.c b/drivers/mfd/simple-mfd-i2c.c
index 6eda79533208..1c5853868022 100644
--- a/drivers/mfd/simple-mfd-i2c.c
+++ b/drivers/mfd/simple-mfd-i2c.c
@@ -88,6 +88,7 @@ static const struct of_device_id simple_mfd_i2c_of_match[] = {
 	{ .compatible = "silergy,sy7636a", .data = &silergy_sy7636a},
 	{ .compatible = "maxim,max5970", .data = &maxim_max5970},
 	{ .compatible = "maxim,max5978", .data = &maxim_max5970},
+	{ .compatible = "tsd,mule" },
 	{}
 };
 MODULE_DEVICE_TABLE(of, simple_mfd_i2c_of_match);

-- 
2.34.1

[PATCH v4 6/9] arm64: dts: rockchip: add mule mfd (0x18) on rk3588-jaguar

[Farouk Bouabid]

Mule emulates amc6821 and an I2C mux (mfd:0x18). The isl1208 is exposed
behind this mux.

Add the mux node and isl1208 as a default device.

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

diff --git a/arch/arm64/boot/dts/rockchip/rk3588-jaguar.dts b/arch/arm64/boot/dts/rockchip/rk3588-jaguar.dts
index 31d2f8994f85..9c296d2116d4 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;
@@ -275,9 +276,33 @@ &i2c0 {
 	pinctrl-0 = <&i2c0m2_xfer>;
 	status = "okay";
 
-	fan@18 {
-		compatible = "ti,amc6821";
+	mule@18 {
+		compatible = "tsd,mule";
 		reg = <0x18>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		fan: fan@18 {
+			compatible = "ti,amc6821";
+			reg = <0x18>;
+		};
+
+		i2c-mux {
+			compatible = "tsd,mule-i2c-mux";
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			i2c10: i2c@0 {
+				reg = <0x0>;
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				rtc_twi: rtc@6f {
+					compatible = "isil,isl1208";
+					reg = <0x6f>;
+				};
+			};
+		};
 	};
 
 	vdd_npu_s0: regulator@42 {
@@ -313,11 +338,6 @@ regulator-state-mem {
 			regulator-off-in-suspend;
 		};
 	};
-
-	rtc_twi: rtc@6f {
-		compatible = "isil,isl1208";
-		reg = <0x6f>;
-	};
 };
 
 &i2c1 {

-- 
2.34.1

[PATCH v4 7/9] arm64: dts: rockchip: add mule mfd (0x18) on rk3399-puma

[Farouk Bouabid]

Mule emulates amc6821 and an I2C mux (mfd:0x18). The isl1208 is exposed
behind this mux.

Add the mux node and isl1208 as a default device.

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

diff --git a/arch/arm64/boot/dts/rockchip/rk3399-puma.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-puma.dtsi
index ccbe3a7a1d2c..052c01472c5a 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;
 	};
 
@@ -377,15 +378,33 @@ &i2c7 {
 	status = "okay";
 	clock-frequency = <400000>;
 
-	fan: fan@18 {
-		compatible = "ti,amc6821";
+	mule@18 {
+		compatible = "tsd,mule";
 		reg = <0x18>;
-		#cooling-cells = <2>;
-	};
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		fan: fan@18 {
+			compatible = "ti,amc6821";
+			reg = <0x18>;
+		};
+
+		i2c-mux {
+			compatible = "tsd,mule-i2c-mux";
+			#address-cells = <1>;
+			#size-cells = <0>;
 
-	rtc_twi: rtc@6f {
-		compatible = "isil,isl1208";
-		reg = <0x6f>;
+			i2c10: i2c@0 {
+				reg = <0x0>;
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				rtc_twi: rtc@6f {
+					compatible = "isil,isl1208";
+					reg = <0x6f>;
+				};
+			};
+		};
 	};
 };
 

-- 
2.34.1

[PATCH v4 8/9] arm64: dts: rockchip: add mule mfd (0x18) on rk3588-tiger

[Farouk Bouabid]

Mule emulates amc6821 and an I2C mux (mfd:0x18). The isl1208 is exposed
behind this mux.

Add the mux node and isl1208 as a default device.

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

diff --git a/arch/arm64/boot/dts/rockchip/rk3588-tiger.dtsi b/arch/arm64/boot/dts/rockchip/rk3588-tiger.dtsi
index aebe1fedd2d8..e6d23e6d95fc 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;
 	};
@@ -223,14 +224,33 @@ &i2c6 {
 	clock-frequency = <400000>;
 	status = "okay";
 
-	fan@18 {
-		compatible = "ti,amc6821";
+	mule@18 {
+		compatible = "tsd,mule";
 		reg = <0x18>;
-	};
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		fan: fan@18 {
+			compatible = "ti,amc6821";
+			reg = <0x18>;
+		};
+
+		i2c-mux {
+			compatible = "tsd,mule-i2c-mux";
+			#address-cells = <1>;
+			#size-cells = <0>;
 
-	rtc_twi: rtc@6f {
-		compatible = "isil,isl1208";
-		reg = <0x6f>;
+			i2c10: i2c@0 {
+				reg = <0x0>;
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				rtc_twi: rtc@6f {
+					compatible = "isil,isl1208";
+					reg = <0x6f>;
+				};
+			};
+		};
 	};
 };
 

-- 
2.34.1

[PATCH v4 9/9] arm64: dts: rockchip: add mule mfd (0x18) on px30-ringneck

[Farouk Bouabid]

Mule emulates amc6821 and an I2C mux (mfd:0x18). The isl1208 is exposed
behind this mux.

Add the mux node and isl1208 as a default device.

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

diff --git a/arch/arm64/boot/dts/rockchip/px30-ringneck.dtsi b/arch/arm64/boot/dts/rockchip/px30-ringneck.dtsi
index bb1aea82e666..cb84295f118d 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,15 +292,33 @@ &i2c1 {
 	/* SE05x is limited to Fast Mode */
 	clock-frequency = <400000>;
 
-	fan: fan@18 {
-		compatible = "ti,amc6821";
+	mule@18 {
+		compatible = "tsd,mule";
 		reg = <0x18>;
-		#cooling-cells = <2>;
-	};
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		fan: fan@18 {
+			compatible = "ti,amc6821";
+			reg = <0x18>;
+		};
+
+		i2c-mux {
+			compatible = "tsd,mule-i2c-mux";
+			#address-cells = <1>;
+			#size-cells = <0>;
 
-	rtc_twi: rtc@6f {
-		compatible = "isil,isl1208";
-		reg = <0x6f>;
+			i2c10: i2c@0 {
+				reg = <0x0>;
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				rtc_twi: rtc@6f {
+					compatible = "isil,isl1208";
+					reg = <0x6f>;
+				};
+			};
+		};
 	};
 };
 

-- 
2.34.1

Re: [PATCH v4 0/9] Add Mule MFD support

[Guenter Roeck]

On 6/18/24 09:06, Farouk Bouabid wrote:
> Mule is an MCU that emulates a set of I2C devices which are reachable
> through an I2C-mux.
> 
> The mux and amc6821 combined make the Mule multi-function device (@0x18)
> 

I don't think that is appropriate. Those devices should all have separate
devicetree entries and be modeled as individual i2c devices.

Guenter

Re: [PATCH v4 0/9] Add Mule MFD support

[Quentin Schulz]

Hi Guenter,

On 6/18/24 6:29 PM, Guenter Roeck wrote:
> On 6/18/24 09:06, Farouk Bouabid wrote:
>> Mule is an MCU that emulates a set of I2C devices which are reachable
>> through an I2C-mux.
>>
>> The mux and amc6821 combined make the Mule multi-function device (@0x18)
>>
> 
> I don't think that is appropriate. Those devices should all have separate
> devicetree entries and be modeled as individual i2c devices.
> 

I think there is a misunderstanding around the wording. They all have 
separate devicetree entries and they all are individual i2c devices 
(from the PoV of the kernel, they all are emulated within the same MCU).

- AMC6821 on address 0x18 for registers from 0x00 to 0xfe.
- Mux (paging, however you want to call it) on address 0x18 for register 
0xff.

Note that AMC6821 is **emulated** in the MCU so this is not some HW 
trickery here.

This MCU also emulates ISL1208 on 0x6f, as well as a PWM controller 
(merge request pending) and two small AT24 "protocol" EEPROMs, on that 
same address. Those are behind a paging/muxing mechanism. You access 
ISL1208 through page 0, PWM controller through page 1, etc...

So basically, the point is:
- 0x18 on i2c is now MFD Mule
   - two platform devices behind MFD = AMC6821 (reg 0x00 to 0xfe) + Mux 
(reg 0xff)
- 0x6f for devices "behind" the Mux
   - page 0 for device behind adapter 0
   - page 1 for device behind adapter 1
   - ...

All of the above are part of the same MCU.

Mule MFD is a simple-mfd-i2c device with its own devicetree entry.
Child nodes of the Mule MFD are AMC6821 as a platform device (but 
operates over i2c) and Mule Mux. That's what was meant as "The mux and 
amc6821 combined make the Mule multi-function device (@0x18)".

The Mule Mux then creates N i2c adapters representing the mux/pages, all 
of those being represented in DT. Each of those have one device on 
address 0x6f, all represented in DT as well.

Nothing hidden or hardcoded, everything in DT.

Did I miss something here?

Cheers,
Quentin

Re: [PATCH v4 0/9] Add Mule MFD support

[Guenter Roeck]

On 6/18/24 09:51, Quentin Schulz wrote:
> Hi Guenter,
> 
> On 6/18/24 6:29 PM, Guenter Roeck wrote:
>> On 6/18/24 09:06, Farouk Bouabid wrote:
>>> Mule is an MCU that emulates a set of I2C devices which are reachable
>>> through an I2C-mux.
>>>
>>> The mux and amc6821 combined make the Mule multi-function device (@0x18)
>>>
>>
>> I don't think that is appropriate. Those devices should all have separate
>> devicetree entries and be modeled as individual i2c devices.
>>
> 
> I think there is a misunderstanding around the wording. They all have separate devicetree entries and they all are individual i2c devices (from the PoV of the kernel, they all are emulated within the same MCU).
> 
> - AMC6821 on address 0x18 for registers from 0x00 to 0xfe.
> - Mux (paging, however you want to call it) on address 0x18 for register 0xff.
> 
> Note that AMC6821 is **emulated** in the MCU so this is not some HW trickery here.
> 
> This MCU also emulates ISL1208 on 0x6f, as well as a PWM controller (merge request pending) and two small AT24 "protocol" EEPROMs, on that same address. Those are behind a paging/muxing mechanism. You access ISL1208 through page 0, PWM controller through page 1, etc...
> 
> So basically, the point is:
> - 0x18 on i2c is now MFD Mule
>    - two platform devices behind MFD = AMC6821 (reg 0x00 to 0xfe) + Mux (reg 0xff)
> - 0x6f for devices "behind" the Mux
>    - page 0 for device behind adapter 0
>    - page 1 for device behind adapter 1
>    - ...
> 
> All of the above are part of the same MCU.
> 
> Mule MFD is a simple-mfd-i2c device with its own devicetree entry.
> Child nodes of the Mule MFD are AMC6821 as a platform device (but operates over i2c) and Mule Mux. That's what was meant as "The mux and amc6821 combined make the Mule multi-function device (@0x18)".
> 
> The Mule Mux then creates N i2c adapters representing the mux/pages, all of those being represented in DT. Each of those have one device on address 0x6f, all represented in DT as well.
> 
> Nothing hidden or hardcoded, everything in DT.
> 
> Did I miss something here?
> 

If it is properly defined in devicetree, the emulated AMC6821 should be
an i2c device, possibly sitting behind an i2c multiplexer, not a
platform device.

Guenter

Re: [PATCH v4 0/9] Add Mule MFD support

[Farouk Bouabid]

Hi Guenter,

On 18.06.24 19:30, Guenter Roeck wrote:
> On 6/18/24 09:51, Quentin Schulz wrote:
>> Hi Guenter,
>>
>> On 6/18/24 6:29 PM, Guenter Roeck wrote:
>>> On 6/18/24 09:06, Farouk Bouabid wrote:
>>>> Mule is an MCU that emulates a set of I2C devices which are reachable
>>>> through an I2C-mux.
>>>>
>>>> The mux and amc6821 combined make the Mule multi-function device 
>>>> (@0x18)
>>>>
>>>
>>> I don't think that is appropriate. Those devices should all have 
>>> separate
>>> devicetree entries and be modeled as individual i2c devices.
>>>
>>
>> I think there is a misunderstanding around the wording. They all have 
>> separate devicetree entries and they all are individual i2c devices 
>> (from the PoV of the kernel, they all are emulated within the same MCU).
>>
>> - AMC6821 on address 0x18 for registers from 0x00 to 0xfe.
>> - Mux (paging, however you want to call it) on address 0x18 for 
>> register 0xff.
>>
>> Note that AMC6821 is **emulated** in the MCU so this is not some HW 
>> trickery here.
>>
>> This MCU also emulates ISL1208 on 0x6f, as well as a PWM controller 
>> (merge request pending) and two small AT24 "protocol" EEPROMs, on 
>> that same address. Those are behind a paging/muxing mechanism. You 
>> access ISL1208 through page 0, PWM controller through page 1, etc...
>>
>> So basically, the point is:
>> - 0x18 on i2c is now MFD Mule
>>    - two platform devices behind MFD = AMC6821 (reg 0x00 to 0xfe) + 
>> Mux (reg 0xff)
>> - 0x6f for devices "behind" the Mux
>>    - page 0 for device behind adapter 0
>>    - page 1 for device behind adapter 1
>>    - ...
>>
>> All of the above are part of the same MCU.
>>
>> Mule MFD is a simple-mfd-i2c device with its own devicetree entry.
>> Child nodes of the Mule MFD are AMC6821 as a platform device (but 
>> operates over i2c) and Mule Mux. That's what was meant as "The mux 
>> and amc6821 combined make the Mule multi-function device (@0x18)".
>>
>> The Mule Mux then creates N i2c adapters representing the mux/pages, 
>> all of those being represented in DT. Each of those have one device 
>> on address 0x6f, all represented in DT as well.
>>
>> Nothing hidden or hardcoded, everything in DT.
>>
>> Did I miss something here?
>>
>
> If it is properly defined in devicetree, the emulated AMC6821 should be
> an i2c device, possibly sitting behind an i2c multiplexer, not a
> platform device.


The emulated AMC6821 and the Mule I2C mux are both reachable using I2C 
address (0x18), and hence the use of MFD as the mux only uses one I2C 
register that is not used by AMC6821.


Cheers,

Farouk

Re: [PATCH v4 0/9] Add Mule MFD support

[Guenter Roeck]

On 6/19/24 00:45, Farouk Bouabid wrote:

>>
>> If it is properly defined in devicetree, the emulated AMC6821 should be
>> an i2c device, possibly sitting behind an i2c multiplexer, not a
>> platform device.
> 
> 
> The emulated AMC6821 and the Mule I2C mux are both reachable using I2C address (0x18), and hence the use of MFD as the mux only uses one I2C register that is not used by AMC6821.
> 

Whatever you do, the amc chip is still an i2c driver and needs to remain one.
Modeling it as platform driver is simply wrong, and I won't accept those patches.

Guenter

Re: [PATCH v4 0/9] Add Mule MFD support

[Farouk Bouabid]

Hi Guenter,

On 19.06.24 15:31, Guenter Roeck wrote:
> On 6/19/24 00:45, Farouk Bouabid wrote:
>
>>>
>>> If it is properly defined in devicetree, the emulated AMC6821 should be
>>> an i2c device, possibly sitting behind an i2c multiplexer, not a
>>> platform device.
>>
>>
>> The emulated AMC6821 and the Mule I2C mux are both reachable using 
>> I2C address (0x18), and hence the use of MFD as the mux only uses one 
>> I2C register that is not used by AMC6821.
>>
>
> Whatever you do, the amc chip is still an i2c driver and needs to 
> remain one.
> Modeling it as platform driver is simply wrong, and I won't accept 
> those patches.
>

The issue that we have cannot be handled by an I2C mux because in that 
case both the mux and its child would have the same address which is not 
supported in the I2C subsystem:


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

                 };

         };

};


The I2C maintainer rejected supporting this use case and suggested that 
an MFD could probably be more suitable.


On one hand, the MFD looks indeed more appropriate and a lot of I2C 
devices are modeled through platform sub devices. On the other hand we 
are emulating the amc6821 in our device which requires us to have it 
modeled as platform:


       +--------+----------------+------------------------------+
       |  Mule         (MFD)                                    |
  0x18 |        +----------------+                              |
--------+----->|    amc6821     |                              |
       | |      +----------------+                              |
       | +----->|      Mux       |-----+                        |
       |        +----------------+     |                        |
       |                               V__          +---------+ |
       |                              |   \-------->| isl1208 | |
       |                              |   |         +---------+ |
  0x6f |                              | M |-------->| dev #1  | |
------------------------------------>| U |         +---------+ |
       |                              | X |-------->| dev #2  | |
       |                              |   |         +---------+ |
       |                              |   /-------->| dev #3  | |
       |                              |__/          +---------+ |
       +--------------------------------------------------------+


If we cannot proceed with that then we could add a compatible to the 
amc6821 driver to add the mux device (Basically the "tsd,mule" 
compatible in amc6821 compatible list would be a combo driver with mux 
logic + amc6821). Do you think that is more appropriate ?


Cheers,

Farouk

Re: [PATCH v4 3/9] dt-bindings: mfd: add support for mule

[Rob Herring]

On Tue, Jun 18, 2024 at 06:06:45PM +0200, Farouk Bouabid wrote:
> Mule is an MCU that emulates a set of I2C devices, among which an amc6821
> device and an I2C mux that exposes more emulated devices. These two devices
> share the same I2C address. Only the I2C mux uses register (0xff) as a
> config register.

Everywhere (or the first place in any section (subject, commit msg, 
title, description, etc.) you say 'mule' I think should be preceeded 
with the vendor because 'mule' has no meaning on its own unless we are 
talking animals.

> 
> Add dt-binding support for the Mule I2C multi-function device.
> 
> Signed-off-by: Farouk Bouabid <farouk.bouabid@cherry.de>
> ---
>  .../devicetree/bindings/i2c/tsd,mule-i2c-mux.yaml  | 48 +++++++++++++
>  .../devicetree/bindings/mfd/tsd,mule.yaml          | 82 ++++++++++++++++++++++
>  2 files changed, 130 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..ac9dfc936272
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/i2c/tsd,mule-i2c-mux.yaml
> @@ -0,0 +1,48 @@
> +# 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: |
> +  This module is part of the Mule I2C multi-function device. For more
> +  details see ../mfd/tsd,mule.yaml.
> +
> +  Mule I2C-mux configures the active device that can be accessed on address
> +  0x6f through the config register.
> +
> +      +--------------------------------------------------+
> +      | Mule                                             |
> +  0x18|    +---------------+                             |
> +  -------->|Config register|----+                        |
> +      |    +---------------+    |                        |
> +      |                         V_                       |
> +      |                        |  \          +--------+  |
> +      |                        |   \-------->| dev #0 |  |
> +      |                        |   |         +--------+  |
> +  0x6f|                        | M |-------->| dev #1 |  |
> +  ---------------------------->| U |         +--------+  |
> +      |                        | X |-------->| dev #2 |  |
> +      |                        |   |         +--------+  |
> +      |                        |   /-------->| dev #3 |  |
> +      |                        |__/          +--------+  |
> +      +--------------------------------------------------+
> +
> +
> +allOf:
> +  - $ref: /schemas/i2c/i2c-mux.yaml#
> +
> +properties:
> +  compatible:
> +    const: tsd,mule-i2c-mux
> +
> +required:
> +  - compatible
> +
> +unevaluatedProperties: false
> diff --git a/Documentation/devicetree/bindings/mfd/tsd,mule.yaml b/Documentation/devicetree/bindings/mfd/tsd,mule.yaml
> new file mode 100644
> index 000000000000..ab532340a17c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/tsd,mule.yaml
> @@ -0,0 +1,82 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mfd/tsd,mule.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Mule I2C multi function device
> +
> +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, among which an amc6821
> +  device and an I2C mux that exposes more emulated devices. These two devices
> +  share the same I2C address. Only the I2C mux uses register (0xff) as a config
> +  register.
> +
> +properties:
> +  compatible:
> +    const: tsd,mule
> +
> +  reg:
> +    maxItems: 1
> +
> +  "#address-cells":
> +    const: 1
> +
> +  "#size-cells":
> +    const: 0
> +
> +patternProperties:
> +  "^fan(@[0-9a-f]+)?$":
> +    $ref: /schemas/trivial-devices.yaml

That doesn't really define which device. You should do something like 
this:

additionalProperties: true
properties:
  compatible:
    contains:
      const: ti,amc6821

If the compatible is correct, then the schema for it will be applied 
separately.

> +
> +  "^i2c-mux(@[0-9a-f]+)?$":
> +    $ref: /schemas/i2c/tsd,mule-i2c-mux.yaml
> +
> +required:
> +  - "#address-cells"
> +  - "#size-cells"
> +  - compatible
> +  - reg
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        mule@18 {
> +            compatible = "tsd,mule";
> +            reg = <0x18>;
> +            #address-cells = <1>;
> +            #size-cells = <0>;
> +
> +            fan@18 {
> +                compatible = "ti,amc6821";
> +                reg = <0x18>;
> +            };
> +
> +            i2c-mux {
> +                compatible = "tsd,mule-i2c-mux";
> +                #address-cells = <1>;
> +                #size-cells = <0>;
> +
> +                i2c@0 {
> +                    reg = <0x0>;
> +                    #address-cells = <1>;
> +                    #size-cells = <0>;
> +
> +                    rtc@6f {
> +                        compatible = "isil,isl1208";
> +                        reg = <0x6f>;
> +                    };
> +                };
> +            };
> +        };
> +    };
> +...
> 
> -- 
> 2.34.1
> 

Re: [PATCH v4 0/9] Add Mule MFD support

[Guenter Roeck]

On 6/24/24 09:13, Farouk Bouabid wrote:
> Hi Guenter,
> 
> On 19.06.24 15:31, Guenter Roeck wrote:
>> On 6/19/24 00:45, Farouk Bouabid wrote:
>>
>>>>
>>>> If it is properly defined in devicetree, the emulated AMC6821 should be
>>>> an i2c device, possibly sitting behind an i2c multiplexer, not a
>>>> platform device.
>>>
>>>
>>> The emulated AMC6821 and the Mule I2C mux are both reachable using I2C address (0x18), and hence the use of MFD as the mux only uses one I2C register that is not used by AMC6821.
>>>
>>
>> Whatever you do, the amc chip is still an i2c driver and needs to remain one.
>> Modeling it as platform driver is simply wrong, and I won't accept those patches.
>>
> 
> The issue that we have cannot be handled by an I2C mux because in that case both the mux and its child would have the same address which is not supported in the I2C subsystem:
> 
> 
> 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>;
> 
>                  };
> 
>          };
> 
> };
> 
> 
> The I2C maintainer rejected supporting this use case and suggested that an MFD could probably be more suitable.
> 
> 
> On one hand, the MFD looks indeed more appropriate and a lot of I2C devices are modeled through platform sub devices. On the other hand we are emulating the amc6821 in our device which requires us to have it modeled as platform:
> 

The difference is that those other i2c devices are real multi-function
devices.

> 
>        +--------+----------------+------------------------------+
>        |  Mule         (MFD)                                    |
>   0x18 |        +----------------+                              |
> --------+----->|    amc6821     |                              |
>        | |      +----------------+                              |
>        | +----->|      Mux       |-----+                        |
>        |        +----------------+     |                        |
>        |                               V__          +---------+ |
>        |                              |   \-------->| isl1208 | |
>        |                              |   |         +---------+ |
>   0x6f |                              | M |-------->| dev #1  | |
> ------------------------------------>| U |         +---------+ |
>        |                              | X |-------->| dev #2  | |
>        |                              |   |         +---------+ |
>        |                              |   /-------->| dev #3  | |
>        |                              |__/          +---------+ |
>        +--------------------------------------------------------+
> 
> 

It would have been much more appropriate to use a different I2C address for the mux.

> If we cannot proceed with that then we could add a compatible to the amc6821 driver to add the mux device (Basically the "tsd,mule" compatible in amc6821 compatible list would be a combo driver with mux logic + amc6821). Do you think that is more appropriate ?
> 

Implement the mux as part of the amc6821 driver ? No. We could discuss
instantiating the i2c mux driver from the amc6821 driver.

Guenter

Re: [PATCH v4 0/9] Add Mule MFD support

[Farouk Bouabid]

Hi Guenter,

On 28.06.24 02:05, Guenter Roeck wrote:
> On 6/24/24 09:13, Farouk Bouabid wrote:
>> Hi Guenter,
>>
>> On 19.06.24 15:31, Guenter Roeck wrote:
>>> On 6/19/24 00:45, Farouk Bouabid wrote:


[...]


>
>> If we cannot proceed with that then we could add a compatible to the 
>> amc6821 driver to add the mux device (Basically the "tsd,mule" 
>> compatible in amc6821 compatible list would be a combo driver with 
>> mux logic + amc6821). Do you think that is more appropriate ?
>>
>
> Implement the mux as part of the amc6821 driver ? No. We could discuss
> instantiating the i2c mux driver from the amc6821 driver.


If I understand correctly we would have the amc6821 for "tsd,mule" 
compatible probe the mule-i2c-mux as platform_device. Possible DT 
representation would be something like:


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

     fan@18 {
         compatible = "tsd,mule", "ti,amc6821";
         reg = <0x18>;

         i2c-mux {
             compatible = "tsd,mule-i2c-mux";
             #address-cells = <1>;
             #size-cells = <0>;

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

                 rtc@6f {
                     compatible = "isil,isl1208";
                     reg = <0x6f>;
                 };

             };
         };
     };
};


Cheers,

Farouk

Re: [PATCH v4 0/9] Add Mule MFD support

[Guenter Roeck]

On 6/28/24 02:03, Farouk Bouabid wrote:
> Hi Guenter,
> 
> On 28.06.24 02:05, Guenter Roeck wrote:
>> On 6/24/24 09:13, Farouk Bouabid wrote:
>>> Hi Guenter,
>>>
>>> On 19.06.24 15:31, Guenter Roeck wrote:
>>>> On 6/19/24 00:45, Farouk Bouabid wrote:
> 
> 
> [...]
> 
> 
>>
>>> If we cannot proceed with that then we could add a compatible to the amc6821 driver to add the mux device (Basically the "tsd,mule" compatible in amc6821 compatible list would be a combo driver with mux logic + amc6821). Do you think that is more appropriate ?
>>>
>>
>> Implement the mux as part of the amc6821 driver ? No. We could discuss
>> instantiating the i2c mux driver from the amc6821 driver.
> 
> 
> If I understand correctly we would have the amc6821 for "tsd,mule" compatible probe the mule-i2c-mux as platform_device. Possible DT representation would be something like:
> 

Yes, something like that. I have a patch series ready which converts the amc6821
to use regmap, so the i2c-mux driver might work as-is with that patch series in place.

Thanks,
Guenter