linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/4] dt-bindings: i2c Update PCA954x
@ 2021-12-14  9:50 Patrick Rudolph
  2021-12-14  9:50 ` [PATCH 2/4] drivers/i2c/mux: Add MAX735x support to PCA954x Patrick Rudolph
                   ` (4 more replies)
  0 siblings, 5 replies; 11+ messages in thread
From: Patrick Rudolph @ 2021-12-14  9:50 UTC (permalink / raw)
  To: Peter Rosin, Rob Herring, Laurent Pinchart
  Cc: Patrick Rudolph, linux-i2c, devicetree, linux-kernel

Add the Maxim MAX735x as supported chip to PCA954x and add an
example how to use it.

Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
---
 .../bindings/i2c/i2c-mux-pca954x.yaml         | 40 +++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml
index 9f1726d0356b..bd794cb80c11 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml
+++ b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml
@@ -11,6 +11,7 @@ maintainers:
 
 description:
   The binding supports NXP PCA954x and PCA984x I2C mux/switch devices.
+  Compatible with Maxim MAX7356 - MAX7358 I2C mux/switch devices.
 
 allOf:
   - $ref: /schemas/i2c/i2c-mux.yaml#
@@ -19,6 +20,9 @@ properties:
   compatible:
     oneOf:
       - enum:
+          - maxim,max7356
+          - maxim,max7357
+          - maxim,max7358
           - nxp,pca9540
           - nxp,pca9542
           - nxp,pca9543
@@ -40,6 +44,7 @@ properties:
 
   interrupts:
     maxItems: 1
+    description: Only supported on NXP devices. Unsupported on Maxim MAX735x.
 
   "#interrupt-cells":
     const: 2
@@ -100,6 +105,41 @@ examples:
                 #size-cells = <0>;
                 reg = <4>;
 
+                rtc@51 {
+                    compatible = "nxp,pcf8563";
+                    reg = <0x51>;
+                };
+            };
+        };
+    };
+
+  - |
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        i2c-mux@74 {
+            compatible = "maxim,max7357";
+            #address-cells = <1>;
+            #size-cells = <0>;
+            reg = <0x74>;
+
+            i2c@1 {
+                #address-cells = <1>;
+                #size-cells = <0>;
+                reg = <1>;
+
+                eeprom@54 {
+                    compatible = "atmel,24c08";
+                    reg = <0x54>;
+                };
+            };
+
+            i2c@7 {
+                #address-cells = <1>;
+                #size-cells = <0>;
+                reg = <7>;
+
                 rtc@51 {
                     compatible = "nxp,pcf8563";
                     reg = <0x51>;
-- 
2.33.1


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

* [PATCH 2/4] drivers/i2c/mux: Add MAX735x support to PCA954x
  2021-12-14  9:50 [PATCH 1/4] dt-bindings: i2c Update PCA954x Patrick Rudolph
@ 2021-12-14  9:50 ` Patrick Rudolph
  2021-12-14  9:50 ` [PATCH 3/4] dt-bindings: i2c Add regulator to pca954x Patrick Rudolph
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 11+ messages in thread
From: Patrick Rudolph @ 2021-12-14  9:50 UTC (permalink / raw)
  To: Peter Rosin; +Cc: Patrick Rudolph, linux-i2c, linux-kernel

Add support for MAX7356, MAX7357 and MAX7358 using the existing driver.
All added Maxim chips behave like the PCA9848, where a single SMBUS byte
write selects up to 8 channels to be bridged to the primary bus.

The MAX7357 exposes 6 additional registers at Power-On-Reset and is
configured to enable additional features:
 - Disabled interrupts on bus locked up detection
 - Enable bus locked-up clearing
 - Disconnect only locked bus instead of all channels

The MAX7357/MAX7358 IRQs aren't supported and must not be enabled.

Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
---
 drivers/i2c/muxes/Kconfig           |  4 +-
 drivers/i2c/muxes/i2c-mux-pca954x.c | 63 +++++++++++++++++++++++++++--
 2 files changed, 61 insertions(+), 6 deletions(-)

diff --git a/drivers/i2c/muxes/Kconfig b/drivers/i2c/muxes/Kconfig
index 1708b1a82da2..621c8a5314f6 100644
--- a/drivers/i2c/muxes/Kconfig
+++ b/drivers/i2c/muxes/Kconfig
@@ -65,11 +65,11 @@ config I2C_MUX_PCA9541
 	  will be called i2c-mux-pca9541.
 
 config I2C_MUX_PCA954x
-	tristate "NXP PCA954x and PCA984x I2C Mux/switches"
+	tristate "NXP PCA954x and PCA984x and Maxim MAX735x I2C Mux/switches"
 	depends on GPIOLIB || COMPILE_TEST
 	help
 	  If you say yes here you get support for the NXP PCA954x
-	  and PCA984x I2C mux/switch devices.
+	  and PCA984x and Maxim MAX735x I2C mux/switch devices.
 
 	  This driver can also be built as a module.  If so, the module
 	  will be called i2c-mux-pca954x.
diff --git a/drivers/i2c/muxes/i2c-mux-pca954x.c b/drivers/i2c/muxes/i2c-mux-pca954x.c
index 4ad665757dd8..23e0f24bab04 100644
--- a/drivers/i2c/muxes/i2c-mux-pca954x.c
+++ b/drivers/i2c/muxes/i2c-mux-pca954x.c
@@ -11,6 +11,12 @@
  *	 PCA9540, PCA9542, PCA9543, PCA9544, PCA9545, PCA9546, PCA9547,
  *	 PCA9548, PCA9846, PCA9847, PCA9848 and PCA9849.
  *
+ * It's also compatible to Maxims MAX735x I2C switch chips, which is controlled
+ * as the NXP PCA9548.
+ *
+ * This includes the:
+ *	 MAX7356, MAX7357 and MAX7358.
+ *
  * These chips are all controlled via the I2C bus itself, and all have a
  * single 8-bit register. The upstream "parent" bus fans out to two,
  * four, or eight downstream busses or channels; which of these
@@ -50,7 +56,12 @@
 
 #define PCA954X_IRQ_OFFSET 4
 
+#define MAX7357_REG_CONFIG_DEFAULTS 0x16
+
 enum pca_type {
+	max_7356,
+	max_7357,
+	max_7358,
 	pca_9540,
 	pca_9542,
 	pca_9543,
@@ -69,6 +80,8 @@ struct chip_desc {
 	u8 nchans;
 	u8 enable;	/* used for muxes only */
 	u8 has_irq;
+	u8 max7357;
+	u8 max735x;
 	enum muxtype {
 		pca954x_ismux = 0,
 		pca954x_isswi
@@ -90,8 +103,27 @@ struct pca954x {
 	raw_spinlock_t lock;
 };
 
-/* Provide specs for the PCA954x types we know about */
+/* Provide specs for the PCA954x and MAX735x types we know about */
 static const struct chip_desc chips[] = {
+	[max_7356] = {
+		.nchans = 8,
+		.muxtype = pca954x_isswi,
+		.max735x = 1,
+		.id = { .manufacturer_id = I2C_DEVICE_ID_NONE },
+	},
+	[max_7357] = {
+		.nchans = 8,
+		.muxtype = pca954x_isswi,
+		.max735x = 1,
+		.max7357 = 1,
+		.id = { .manufacturer_id = I2C_DEVICE_ID_NONE },
+	},
+	[max_7358] = {
+		.nchans = 8,
+		.muxtype = pca954x_isswi,
+		.max735x = 1,
+		.id = { .manufacturer_id = I2C_DEVICE_ID_NONE },
+	},
 	[pca_9540] = {
 		.nchans = 2,
 		.enable = 0x4,
@@ -177,6 +209,9 @@ static const struct chip_desc chips[] = {
 };
 
 static const struct i2c_device_id pca954x_id[] = {
+	{ "max7356", max_7356 },
+	{ "max7357", max_7357 },
+	{ "max7358", max_7358 },
 	{ "pca9540", pca_9540 },
 	{ "pca9542", pca_9542 },
 	{ "pca9543", pca_9543 },
@@ -194,6 +229,9 @@ static const struct i2c_device_id pca954x_id[] = {
 MODULE_DEVICE_TABLE(i2c, pca954x_id);
 
 static const struct of_device_id pca954x_of_match[] = {
+	{ .compatible = "maxim,max7356", .data = &chips[max_7356] },
+	{ .compatible = "maxim,max7357", .data = &chips[max_7357] },
+	{ .compatible = "maxim,max7358", .data = &chips[max_7358] },
 	{ .compatible = "nxp,pca9540", .data = &chips[pca_9540] },
 	{ .compatible = "nxp,pca9542", .data = &chips[pca_9542] },
 	{ .compatible = "nxp,pca9543", .data = &chips[pca_9543] },
@@ -355,6 +393,11 @@ static int pca954x_irq_setup(struct i2c_mux_core *muxc)
 	if (!data->chip->has_irq || client->irq <= 0)
 		return 0;
 
+	if (data->chip->max735x) {
+		dev_err(&client->dev, "IRQs not supported for MAX735x\n");
+		return -EOPNOTSUPP;
+	}
+
 	raw_spin_lock_init(&data->lock);
 
 	data->irq = irq_domain_add_linear(client->dev.of_node,
@@ -401,9 +444,21 @@ static int pca954x_init(struct i2c_client *client, struct pca954x *data)
 	else
 		data->last_chan = 0; /* Disconnect multiplexer */
 
-	ret = i2c_smbus_write_byte(client, data->last_chan);
-	if (ret < 0)
-		data->last_chan = 0;
+	/*
+	 * MAX7357 exposes 7 register on POR which allows to configure additional
+	 * features. Disable interrupts, enable bus locked-up clearing,
+	 * disconnect only locked bus instead of all channels.
+	 */
+	if (data->chip->max7357) {
+		ret = i2c_smbus_write_byte_data(client, data->last_chan,
+						MAX7357_REG_CONFIG_DEFAULTS);
+		if (ret < 0)
+			data->last_chan = 0;
+	} else {
+		ret = i2c_smbus_write_byte(client, data->last_chan);
+		if (ret < 0)
+			data->last_chan = 0;
+	}
 
 	return ret;
 }
-- 
2.33.1


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

* [PATCH 3/4] dt-bindings: i2c Add regulator to pca954x
  2021-12-14  9:50 [PATCH 1/4] dt-bindings: i2c Update PCA954x Patrick Rudolph
  2021-12-14  9:50 ` [PATCH 2/4] drivers/i2c/mux: Add MAX735x support to PCA954x Patrick Rudolph
@ 2021-12-14  9:50 ` Patrick Rudolph
  2021-12-14 11:37   ` Laurent Pinchart
  2021-12-14  9:50 ` [PATCH 4/4] i2c-mux-pca954x: Add regulator support Patrick Rudolph
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 11+ messages in thread
From: Patrick Rudolph @ 2021-12-14  9:50 UTC (permalink / raw)
  To: Peter Rosin, Rob Herring, Laurent Pinchart
  Cc: Patrick Rudolph, linux-i2c, devicetree, linux-kernel

Add a regulator called vcc and update the example.

Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
---
 Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml
index bd794cb80c11..5add7db02c0c 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml
+++ b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml
@@ -64,6 +64,9 @@ properties:
     description: if present, overrides i2c-mux-idle-disconnect
     $ref: /schemas/mux/mux-controller.yaml#/properties/idle-state
 
+  vcc-supply:
+    description: An optional voltage regulator supplying power to the chip.
+
 required:
   - compatible
   - reg
@@ -84,6 +87,8 @@ examples:
             #size-cells = <0>;
             reg = <0x74>;
 
+            vcc-supply = <&p3v3>;
+
             interrupt-parent = <&ipic>;
             interrupts = <17 IRQ_TYPE_LEVEL_LOW>;
             interrupt-controller;
-- 
2.33.1


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

* [PATCH 4/4] i2c-mux-pca954x: Add regulator support
  2021-12-14  9:50 [PATCH 1/4] dt-bindings: i2c Update PCA954x Patrick Rudolph
  2021-12-14  9:50 ` [PATCH 2/4] drivers/i2c/mux: Add MAX735x support to PCA954x Patrick Rudolph
  2021-12-14  9:50 ` [PATCH 3/4] dt-bindings: i2c Add regulator to pca954x Patrick Rudolph
@ 2021-12-14  9:50 ` Patrick Rudolph
  2021-12-14 13:12   ` kernel test robot
  2021-12-14 11:13 ` [PATCH 1/4] dt-bindings: i2c Update PCA954x Laurent Pinchart
  2021-12-15 20:33 ` Rob Herring
  4 siblings, 1 reply; 11+ messages in thread
From: Patrick Rudolph @ 2021-12-14  9:50 UTC (permalink / raw)
  To: Peter Rosin; +Cc: Patrick Rudolph, linux-i2c, linux-kernel

Add an optional vcc regulator and enable it when found for devices
that are powered off by default.

Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
---
 drivers/i2c/muxes/i2c-mux-pca954x.c | 33 ++++++++++++++++++++++++-----
 1 file changed, 28 insertions(+), 5 deletions(-)

diff --git a/drivers/i2c/muxes/i2c-mux-pca954x.c b/drivers/i2c/muxes/i2c-mux-pca954x.c
index 23e0f24bab04..5fa266cb02c0 100644
--- a/drivers/i2c/muxes/i2c-mux-pca954x.c
+++ b/drivers/i2c/muxes/i2c-mux-pca954x.c
@@ -48,6 +48,7 @@
 #include <linux/module.h>
 #include <linux/pm.h>
 #include <linux/property.h>
+#include <linux/regulator/consumer.h>
 #include <linux/slab.h>
 #include <linux/spinlock.h>
 #include <dt-bindings/mux/mux.h>
@@ -101,6 +102,7 @@ struct pca954x {
 	struct irq_domain *irq;
 	unsigned int irq_mask;
 	raw_spinlock_t lock;
+	struct regulator *supply;
 };
 
 /* Provide specs for the PCA954x and MAX735x types we know about */
@@ -425,6 +427,9 @@ static void pca954x_cleanup(struct i2c_mux_core *muxc)
 	struct pca954x *data = i2c_mux_priv(muxc);
 	int c, irq;
 
+	if (!IS_ERR_OR_NULL(data->supply))
+		regulator_disable(data->supply);
+
 	if (data->irq) {
 		for (c = 0; c < data->chip->nchans; c++) {
 			irq = irq_find_mapping(data->irq, c);
@@ -484,15 +489,31 @@ static int pca954x_probe(struct i2c_client *client,
 			     pca954x_select_chan, pca954x_deselect_mux);
 	if (!muxc)
 		return -ENOMEM;
+
 	data = i2c_mux_priv(muxc);
 
 	i2c_set_clientdata(client, muxc);
 	data->client = client;
 
+	data->supply = devm_regulator_get(dev, "vcc");
+	if (IS_ERR(data->supply)) {
+		if ((PTR_ERR(data->supply) == -EPROBE_DEFER))
+			return -EPROBE_DEFER;
+		dev_warn(dev, "Failed to get regulator for vcc: %d\n", ret);
+	} else {
+		ret = regulator_enable(data->supply);
+		if (ret) {
+			dev_err(dev, "Failed to enable regulator vcc\n");
+			return ret;
+		}
+	}
+
 	/* Reset the mux if a reset GPIO is specified. */
 	gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
-	if (IS_ERR(gpio))
-		return PTR_ERR(gpio);
+	if (IS_ERR(gpio)) {
+		ret = PTR_ERR(gpio);
+		goto fail_cleanup;
+	}
 	if (gpio) {
 		udelay(1);
 		gpiod_set_value_cansleep(gpio, 0);
@@ -509,7 +530,7 @@ static int pca954x_probe(struct i2c_client *client,
 
 		ret = i2c_get_device_id(client, &id);
 		if (ret && ret != -EOPNOTSUPP)
-			return ret;
+			goto fail_cleanup;
 
 		if (!ret &&
 		    (id.manufacturer_id != data->chip->id.manufacturer_id ||
@@ -517,7 +538,8 @@ static int pca954x_probe(struct i2c_client *client,
 			dev_warn(dev, "unexpected device id %03x-%03x-%x\n",
 				 id.manufacturer_id, id.part_id,
 				 id.die_revision);
-			return -ENODEV;
+			ret = -ENODEV;
+			goto fail_cleanup;
 		}
 	}
 
@@ -536,7 +558,8 @@ static int pca954x_probe(struct i2c_client *client,
 	ret = pca954x_init(client, data);
 	if (ret < 0) {
 		dev_warn(dev, "probe failed\n");
-		return -ENODEV;
+		ret = -ENODEV;
+		goto fail_cleanup;
 	}
 
 	ret = pca954x_irq_setup(muxc);
-- 
2.33.1


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

* Re: [PATCH 1/4] dt-bindings: i2c Update PCA954x
  2021-12-14  9:50 [PATCH 1/4] dt-bindings: i2c Update PCA954x Patrick Rudolph
                   ` (2 preceding siblings ...)
  2021-12-14  9:50 ` [PATCH 4/4] i2c-mux-pca954x: Add regulator support Patrick Rudolph
@ 2021-12-14 11:13 ` Laurent Pinchart
  2021-12-15 12:42   ` Peter Rosin
  2021-12-15 20:33 ` Rob Herring
  4 siblings, 1 reply; 11+ messages in thread
From: Laurent Pinchart @ 2021-12-14 11:13 UTC (permalink / raw)
  To: Patrick Rudolph
  Cc: Peter Rosin, Rob Herring, linux-i2c, devicetree, linux-kernel

Hi Patrick,

Thank you for the patch.

On Tue, Dec 14, 2021 at 10:50:18AM +0100, Patrick Rudolph wrote:
> Add the Maxim MAX735x as supported chip to PCA954x and add an
> example how to use it.
> 
> Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
> ---
>  .../bindings/i2c/i2c-mux-pca954x.yaml         | 40 +++++++++++++++++++
>  1 file changed, 40 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml
> index 9f1726d0356b..bd794cb80c11 100644
> --- a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml
> +++ b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml
> @@ -11,6 +11,7 @@ maintainers:
>  
>  description:
>    The binding supports NXP PCA954x and PCA984x I2C mux/switch devices.
> +  Compatible with Maxim MAX7356 - MAX7358 I2C mux/switch devices.
>  
>  allOf:
>    - $ref: /schemas/i2c/i2c-mux.yaml#
> @@ -19,6 +20,9 @@ properties:
>    compatible:
>      oneOf:
>        - enum:
> +          - maxim,max7356
> +          - maxim,max7357
> +          - maxim,max7358
>            - nxp,pca9540
>            - nxp,pca9542
>            - nxp,pca9543
> @@ -40,6 +44,7 @@ properties:
>  
>    interrupts:
>      maxItems: 1
> +    description: Only supported on NXP devices. Unsupported on Maxim MAX735x.

Could this be modelled by a YAML schema instead ? Something like

allOf:
  - if:
      properties:
        compatible:
	  contains:
	    enum:
              - maxim,max7356
              - maxim,max7357
              - maxim,max7358
    then:
      properties:
        interrupts: false

(untested, it would be nice to use a pattern check for the compatible
property if possible)

>  
>    "#interrupt-cells":
>      const: 2
> @@ -100,6 +105,41 @@ examples:
>                  #size-cells = <0>;
>                  reg = <4>;
>  
> +                rtc@51 {
> +                    compatible = "nxp,pcf8563";
> +                    reg = <0x51>;
> +                };
> +            };
> +        };
> +    };
> +
> +  - |
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        i2c-mux@74 {
> +            compatible = "maxim,max7357";
> +            #address-cells = <1>;
> +            #size-cells = <0>;
> +            reg = <0x74>;
> +
> +            i2c@1 {
> +                #address-cells = <1>;
> +                #size-cells = <0>;
> +                reg = <1>;
> +
> +                eeprom@54 {
> +                    compatible = "atmel,24c08";
> +                    reg = <0x54>;
> +                };
> +            };
> +
> +            i2c@7 {
> +                #address-cells = <1>;
> +                #size-cells = <0>;
> +                reg = <7>;
> +
>                  rtc@51 {
>                      compatible = "nxp,pcf8563";
>                      reg = <0x51>;

-- 
Regards,

Laurent Pinchart

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

* Re: [PATCH 3/4] dt-bindings: i2c Add regulator to pca954x
  2021-12-14  9:50 ` [PATCH 3/4] dt-bindings: i2c Add regulator to pca954x Patrick Rudolph
@ 2021-12-14 11:37   ` Laurent Pinchart
  0 siblings, 0 replies; 11+ messages in thread
From: Laurent Pinchart @ 2021-12-14 11:37 UTC (permalink / raw)
  To: Patrick Rudolph
  Cc: Peter Rosin, Rob Herring, linux-i2c, devicetree, linux-kernel

Hi Patrick,

Thank you for the patch.

On Tue, Dec 14, 2021 at 10:50:20AM +0100, Patrick Rudolph wrote:
> Add a regulator called vcc and update the example.
> 
> Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
> ---
>  Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml
> index bd794cb80c11..5add7db02c0c 100644
> --- a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml
> +++ b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml
> @@ -64,6 +64,9 @@ properties:
>      description: if present, overrides i2c-mux-idle-disconnect
>      $ref: /schemas/mux/mux-controller.yaml#/properties/idle-state
>  
> +  vcc-supply:
> +    description: An optional voltage regulator supplying power to the chip.

The NXP datasheet names the supply VDD, could we use vdd-supply here ? I
also wouldn't call it ooptional (even if it effectively is from a DT
point of view as the property isn't listed as required), given that the
power supply isn't optional for the chip to function. How about the
following ?

  vdd-supply:
    description: The voltage regulator powering to the VDD supply.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> +
>  required:
>    - compatible
>    - reg
> @@ -84,6 +87,8 @@ examples:
>              #size-cells = <0>;
>              reg = <0x74>;
>  
> +            vcc-supply = <&p3v3>;
> +
>              interrupt-parent = <&ipic>;
>              interrupts = <17 IRQ_TYPE_LEVEL_LOW>;
>              interrupt-controller;

-- 
Regards,

Laurent Pinchart

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

* Re: [PATCH 4/4] i2c-mux-pca954x: Add regulator support
  2021-12-14  9:50 ` [PATCH 4/4] i2c-mux-pca954x: Add regulator support Patrick Rudolph
@ 2021-12-14 13:12   ` kernel test robot
  0 siblings, 0 replies; 11+ messages in thread
From: kernel test robot @ 2021-12-14 13:12 UTC (permalink / raw)
  To: Patrick Rudolph, Peter Rosin
  Cc: llvm, kbuild-all, Patrick Rudolph, linux-i2c, linux-kernel

Hi Patrick,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on wsa/i2c/for-next]
[also build test WARNING on robh/for-next linux/master linus/master v5.16-rc5]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Patrick-Rudolph/dt-bindings-i2c-Update-PCA954x/20211214-175258
base:   https://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git i2c/for-next
config: riscv-randconfig-r042-20211214 (https://download.01.org/0day-ci/archive/20211214/202112142101.s4i5cHhd-lkp@intel.com/config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project b6a2ddb6c8ac29412b1361810972e15221fa021c)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install riscv cross compiling tool for clang build
        # apt-get install binutils-riscv64-linux-gnu
        # https://github.com/0day-ci/linux/commit/3498c52eb6aec09c78a3f07cdcb042897960f8ef
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Patrick-Rudolph/dt-bindings-i2c-Update-PCA954x/20211214-175258
        git checkout 3498c52eb6aec09c78a3f07cdcb042897960f8ef
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=riscv SHELL=/bin/bash drivers/i2c/muxes/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/i2c/muxes/i2c-mux-pca954x.c:502:58: warning: variable 'ret' is uninitialized when used here [-Wuninitialized]
                   dev_warn(dev, "Failed to get regulator for vcc: %d\n", ret);
                                                                          ^~~
   include/linux/dev_printk.h:146:70: note: expanded from macro 'dev_warn'
           dev_printk_index_wrap(_dev_warn, KERN_WARNING, dev, dev_fmt(fmt), ##__VA_ARGS__)
                                                                               ^~~~~~~~~~~
   include/linux/dev_printk.h:110:23: note: expanded from macro 'dev_printk_index_wrap'
                   _p_func(dev, fmt, ##__VA_ARGS__);                       \
                                       ^~~~~~~~~~~
   drivers/i2c/muxes/i2c-mux-pca954x.c:483:9: note: initialize the variable 'ret' to silence this warning
           int ret;
                  ^
                   = 0
   1 warning generated.


vim +/ret +502 drivers/i2c/muxes/i2c-mux-pca954x.c

   470	
   471	/*
   472	 * I2C init/probing/exit functions
   473	 */
   474	static int pca954x_probe(struct i2c_client *client,
   475				 const struct i2c_device_id *id)
   476	{
   477		struct i2c_adapter *adap = client->adapter;
   478		struct device *dev = &client->dev;
   479		struct gpio_desc *gpio;
   480		struct i2c_mux_core *muxc;
   481		struct pca954x *data;
   482		int num;
   483		int ret;
   484	
   485		if (!i2c_check_functionality(adap, I2C_FUNC_SMBUS_BYTE))
   486			return -ENODEV;
   487	
   488		muxc = i2c_mux_alloc(adap, dev, PCA954X_MAX_NCHANS, sizeof(*data), 0,
   489				     pca954x_select_chan, pca954x_deselect_mux);
   490		if (!muxc)
   491			return -ENOMEM;
   492	
   493		data = i2c_mux_priv(muxc);
   494	
   495		i2c_set_clientdata(client, muxc);
   496		data->client = client;
   497	
   498		data->supply = devm_regulator_get(dev, "vcc");
   499		if (IS_ERR(data->supply)) {
   500			if ((PTR_ERR(data->supply) == -EPROBE_DEFER))
   501				return -EPROBE_DEFER;
 > 502			dev_warn(dev, "Failed to get regulator for vcc: %d\n", ret);
   503		} else {
   504			ret = regulator_enable(data->supply);
   505			if (ret) {
   506				dev_err(dev, "Failed to enable regulator vcc\n");
   507				return ret;
   508			}
   509		}
   510	
   511		/* Reset the mux if a reset GPIO is specified. */
   512		gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
   513		if (IS_ERR(gpio)) {
   514			ret = PTR_ERR(gpio);
   515			goto fail_cleanup;
   516		}
   517		if (gpio) {
   518			udelay(1);
   519			gpiod_set_value_cansleep(gpio, 0);
   520			/* Give the chip some time to recover. */
   521			udelay(1);
   522		}
   523	
   524		data->chip = device_get_match_data(dev);
   525		if (!data->chip)
   526			data->chip = &chips[id->driver_data];
   527	
   528		if (data->chip->id.manufacturer_id != I2C_DEVICE_ID_NONE) {
   529			struct i2c_device_identity id;
   530	
   531			ret = i2c_get_device_id(client, &id);
   532			if (ret && ret != -EOPNOTSUPP)
   533				goto fail_cleanup;
   534	
   535			if (!ret &&
   536			    (id.manufacturer_id != data->chip->id.manufacturer_id ||
   537			     id.part_id != data->chip->id.part_id)) {
   538				dev_warn(dev, "unexpected device id %03x-%03x-%x\n",
   539					 id.manufacturer_id, id.part_id,
   540					 id.die_revision);
   541				ret = -ENODEV;
   542				goto fail_cleanup;
   543			}
   544		}
   545	
   546		data->idle_state = MUX_IDLE_AS_IS;
   547		if (device_property_read_u32(dev, "idle-state", &data->idle_state)) {
   548			if (device_property_read_bool(dev, "i2c-mux-idle-disconnect"))
   549				data->idle_state = MUX_IDLE_DISCONNECT;
   550		}
   551	
   552		/*
   553		 * Write the mux register at addr to verify
   554		 * that the mux is in fact present. This also
   555		 * initializes the mux to a channel
   556		 * or disconnected state.
   557		 */
   558		ret = pca954x_init(client, data);
   559		if (ret < 0) {
   560			dev_warn(dev, "probe failed\n");
   561			ret = -ENODEV;
   562			goto fail_cleanup;
   563		}
   564	
   565		ret = pca954x_irq_setup(muxc);
   566		if (ret)
   567			goto fail_cleanup;
   568	
   569		/* Now create an adapter for each channel */
   570		for (num = 0; num < data->chip->nchans; num++) {
   571			ret = i2c_mux_add_adapter(muxc, 0, num, 0);
   572			if (ret)
   573				goto fail_cleanup;
   574		}
   575	
   576		if (data->irq) {
   577			ret = devm_request_threaded_irq(dev, data->client->irq,
   578							NULL, pca954x_irq_handler,
   579							IRQF_ONESHOT | IRQF_SHARED,
   580							"pca954x", data);
   581			if (ret)
   582				goto fail_cleanup;
   583		}
   584	
   585		/*
   586		 * The attr probably isn't going to be needed in most cases,
   587		 * so don't fail completely on error.
   588		 */
   589		device_create_file(dev, &dev_attr_idle_state);
   590	
   591		dev_info(dev, "registered %d multiplexed busses for I2C %s %s\n",
   592			 num, data->chip->muxtype == pca954x_ismux
   593					? "mux" : "switch", client->name);
   594	
   595		return 0;
   596	
   597	fail_cleanup:
   598		pca954x_cleanup(muxc);
   599		return ret;
   600	}
   601	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

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

* Re: [PATCH 1/4] dt-bindings: i2c Update PCA954x
  2021-12-14 11:13 ` [PATCH 1/4] dt-bindings: i2c Update PCA954x Laurent Pinchart
@ 2021-12-15 12:42   ` Peter Rosin
  2021-12-15 14:19     ` Patrick Rudolph
  0 siblings, 1 reply; 11+ messages in thread
From: Peter Rosin @ 2021-12-15 12:42 UTC (permalink / raw)
  To: Laurent Pinchart, Patrick Rudolph
  Cc: Rob Herring, linux-i2c, devicetree, linux-kernel

Hi!

On 2021-12-14 12:13, Laurent Pinchart wrote:
> Hi Patrick,
> 
> Thank you for the patch.
> 
> On Tue, Dec 14, 2021 at 10:50:18AM +0100, Patrick Rudolph wrote:
>> Add the Maxim MAX735x as supported chip to PCA954x and add an
>> example how to use it.
>>
>> Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
>> ---
>>  .../bindings/i2c/i2c-mux-pca954x.yaml         | 40 +++++++++++++++++++
>>  1 file changed, 40 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml
>> index 9f1726d0356b..bd794cb80c11 100644
>> --- a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml
>> +++ b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml
>> @@ -11,6 +11,7 @@ maintainers:
>>  
>>  description:
>>    The binding supports NXP PCA954x and PCA984x I2C mux/switch devices.
>> +  Compatible with Maxim MAX7356 - MAX7358 I2C mux/switch devices.
>>  
>>  allOf:
>>    - $ref: /schemas/i2c/i2c-mux.yaml#
>> @@ -19,6 +20,9 @@ properties:
>>    compatible:
>>      oneOf:
>>        - enum:
>> +          - maxim,max7356
>> +          - maxim,max7357
>> +          - maxim,max7358
>>            - nxp,pca9540
>>            - nxp,pca9542
>>            - nxp,pca9543
>> @@ -40,6 +44,7 @@ properties:
>>  
>>    interrupts:
>>      maxItems: 1
>> +    description: Only supported on NXP devices. Unsupported on Maxim MAX735x.
> 
> Could this be modelled by a YAML schema instead ? Something like
> 
> allOf:
>   - if:
>       properties:
>         compatible:
> 	  contains:
> 	    enum:
>               - maxim,max7356
>               - maxim,max7357
>               - maxim,max7358
>     then:
>       properties:
>         interrupts: false
> 
> (untested, it would be nice to use a pattern check for the compatible
> property if possible)

Some of the existing NXP chips do not support interrupts; we should
probably treat these new chips the same as the older ones. Either by
disallowing interrupts on both kinds or by continuing to ignore the
situation.

That said, I'm slightly in favor of the latter, since these new chips
do have interrupts, just not the same flavor as the NXP chips. What
the Maxim chips do not have is support for being an
	interrupt-controller;
At least that's how I read it...

I don't know how this situation is supposed to be described? Maybe this
new kind of interrupt should be indicated with a bus-error-interrupts
property (or bikeshed along those lines)? Maybe there should be two
entries in the existing interrupts property? Maybe these new chips
should be described in a new binding specific to maxim,max7356-7358
(could still be handled by the pca954x driver of course) to keep the
yaml simpler to read?

However, there is also maxim,max7367-7369 to consider. They seem to
have interrupts of the style described by the NXP binding (haven't
checked if the registers work the same, but since they reuse the
0x70 address-range the are in all likelihood also compatible).

Cheers,
Peter

>>  
>>    "#interrupt-cells":
>>      const: 2
>> @@ -100,6 +105,41 @@ examples:
>>                  #size-cells = <0>;
>>                  reg = <4>;
>>  
>> +                rtc@51 {
>> +                    compatible = "nxp,pcf8563";
>> +                    reg = <0x51>;
>> +                };
>> +            };
>> +        };
>> +    };
>> +
>> +  - |
>> +    i2c {
>> +        #address-cells = <1>;
>> +        #size-cells = <0>;
>> +
>> +        i2c-mux@74 {
>> +            compatible = "maxim,max7357";
>> +            #address-cells = <1>;
>> +            #size-cells = <0>;
>> +            reg = <0x74>;
>> +
>> +            i2c@1 {
>> +                #address-cells = <1>;
>> +                #size-cells = <0>;
>> +                reg = <1>;
>> +
>> +                eeprom@54 {
>> +                    compatible = "atmel,24c08";
>> +                    reg = <0x54>;
>> +                };
>> +            };
>> +
>> +            i2c@7 {
>> +                #address-cells = <1>;
>> +                #size-cells = <0>;
>> +                reg = <7>;
>> +
>>                  rtc@51 {
>>                      compatible = "nxp,pcf8563";
>>                      reg = <0x51>;
> 

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

* Re: [PATCH 1/4] dt-bindings: i2c Update PCA954x
  2021-12-15 12:42   ` Peter Rosin
@ 2021-12-15 14:19     ` Patrick Rudolph
  2021-12-15 21:22       ` Laurent Pinchart
  0 siblings, 1 reply; 11+ messages in thread
From: Patrick Rudolph @ 2021-12-15 14:19 UTC (permalink / raw)
  To: Peter Rosin
  Cc: Laurent Pinchart, Rob Herring, linux-i2c, devicetree, linux-kernel

Hi Peter,
thanks for your feedback.
You are right, the added Maxim chips should not have set the
interrupt-controller; flag.

The reason I decided to not handle that interrupt is that I don't know
where to pass that bus error to.
It looks like only the I2C master can signal bus errors by returning
-EIO, however there's no API for I2C clients
to pass such errors to the master. However any attempt to access the
stuck and isolated bus will fail and
the address will be NACKed, so I don't think that this a big issue as
in the end the bus stall will be detected.

Is there a mapping between devicetree bindings and driver file names?
If not I'll use
maxim,max7356 as devicetree binding to make it easier to read and
mention that interrupts
are not supported for those maxim devices.

Regards,
Patrick

On Wed, Dec 15, 2021 at 1:42 PM Peter Rosin <peda@axentia.se> wrote:
>
> Hi!
>
> On 2021-12-14 12:13, Laurent Pinchart wrote:
> > Hi Patrick,
> >
> > Thank you for the patch.
> >
> > On Tue, Dec 14, 2021 at 10:50:18AM +0100, Patrick Rudolph wrote:
> >> Add the Maxim MAX735x as supported chip to PCA954x and add an
> >> example how to use it.
> >>
> >> Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
> >> ---
> >>  .../bindings/i2c/i2c-mux-pca954x.yaml         | 40 +++++++++++++++++++
> >>  1 file changed, 40 insertions(+)
> >>
> >> diff --git a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml
> >> index 9f1726d0356b..bd794cb80c11 100644
> >> --- a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml
> >> +++ b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml
> >> @@ -11,6 +11,7 @@ maintainers:
> >>
> >>  description:
> >>    The binding supports NXP PCA954x and PCA984x I2C mux/switch devices.
> >> +  Compatible with Maxim MAX7356 - MAX7358 I2C mux/switch devices.
> >>
> >>  allOf:
> >>    - $ref: /schemas/i2c/i2c-mux.yaml#
> >> @@ -19,6 +20,9 @@ properties:
> >>    compatible:
> >>      oneOf:
> >>        - enum:
> >> +          - maxim,max7356
> >> +          - maxim,max7357
> >> +          - maxim,max7358
> >>            - nxp,pca9540
> >>            - nxp,pca9542
> >>            - nxp,pca9543
> >> @@ -40,6 +44,7 @@ properties:
> >>
> >>    interrupts:
> >>      maxItems: 1
> >> +    description: Only supported on NXP devices. Unsupported on Maxim MAX735x.
> >
> > Could this be modelled by a YAML schema instead ? Something like
> >
> > allOf:
> >   - if:
> >       properties:
> >         compatible:
> >         contains:
> >           enum:
> >               - maxim,max7356
> >               - maxim,max7357
> >               - maxim,max7358
> >     then:
> >       properties:
> >         interrupts: false
> >
> > (untested, it would be nice to use a pattern check for the compatible
> > property if possible)
>
> Some of the existing NXP chips do not support interrupts; we should
> probably treat these new chips the same as the older ones. Either by
> disallowing interrupts on both kinds or by continuing to ignore the
> situation.
>
> That said, I'm slightly in favor of the latter, since these new chips
> do have interrupts, just not the same flavor as the NXP chips. What
> the Maxim chips do not have is support for being an
>         interrupt-controller;
> At least that's how I read it...
>
> I don't know how this situation is supposed to be described? Maybe this
> new kind of interrupt should be indicated with a bus-error-interrupts
> property (or bikeshed along those lines)? Maybe there should be two
> entries in the existing interrupts property? Maybe these new chips
> should be described in a new binding specific to maxim,max7356-7358
> (could still be handled by the pca954x driver of course) to keep the
> yaml simpler to read?
>
> However, there is also maxim,max7367-7369 to consider. They seem to
> have interrupts of the style described by the NXP binding (haven't
> checked if the registers work the same, but since they reuse the
> 0x70 address-range the are in all likelihood also compatible).
>
> Cheers,
> Peter
>
> >>
> >>    "#interrupt-cells":
> >>      const: 2
> >> @@ -100,6 +105,41 @@ examples:
> >>                  #size-cells = <0>;
> >>                  reg = <4>;
> >>
> >> +                rtc@51 {
> >> +                    compatible = "nxp,pcf8563";
> >> +                    reg = <0x51>;
> >> +                };
> >> +            };
> >> +        };
> >> +    };
> >> +
> >> +  - |
> >> +    i2c {
> >> +        #address-cells = <1>;
> >> +        #size-cells = <0>;
> >> +
> >> +        i2c-mux@74 {
> >> +            compatible = "maxim,max7357";
> >> +            #address-cells = <1>;
> >> +            #size-cells = <0>;
> >> +            reg = <0x74>;
> >> +
> >> +            i2c@1 {
> >> +                #address-cells = <1>;
> >> +                #size-cells = <0>;
> >> +                reg = <1>;
> >> +
> >> +                eeprom@54 {
> >> +                    compatible = "atmel,24c08";
> >> +                    reg = <0x54>;
> >> +                };
> >> +            };
> >> +
> >> +            i2c@7 {
> >> +                #address-cells = <1>;
> >> +                #size-cells = <0>;
> >> +                reg = <7>;
> >> +
> >>                  rtc@51 {
> >>                      compatible = "nxp,pcf8563";
> >>                      reg = <0x51>;
> >

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

* Re: [PATCH 1/4] dt-bindings: i2c Update PCA954x
  2021-12-14  9:50 [PATCH 1/4] dt-bindings: i2c Update PCA954x Patrick Rudolph
                   ` (3 preceding siblings ...)
  2021-12-14 11:13 ` [PATCH 1/4] dt-bindings: i2c Update PCA954x Laurent Pinchart
@ 2021-12-15 20:33 ` Rob Herring
  4 siblings, 0 replies; 11+ messages in thread
From: Rob Herring @ 2021-12-15 20:33 UTC (permalink / raw)
  To: Patrick Rudolph
  Cc: Peter Rosin, Laurent Pinchart, linux-i2c, devicetree, linux-kernel

On Tue, Dec 14, 2021 at 10:50:18AM +0100, Patrick Rudolph wrote:
> Add the Maxim MAX735x as supported chip to PCA954x and add an
> example how to use it.

The subject needs some work. Every change is an 'update' and you should 
say something about Maxim. 'Add Maxim MAX735x variants' or something.

> 
> Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
> ---
>  .../bindings/i2c/i2c-mux-pca954x.yaml         | 40 +++++++++++++++++++
>  1 file changed, 40 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml
> index 9f1726d0356b..bd794cb80c11 100644
> --- a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml
> +++ b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml
> @@ -11,6 +11,7 @@ maintainers:
>  
>  description:
>    The binding supports NXP PCA954x and PCA984x I2C mux/switch devices.
> +  Compatible with Maxim MAX7356 - MAX7358 I2C mux/switch devices.
>  
>  allOf:
>    - $ref: /schemas/i2c/i2c-mux.yaml#
> @@ -19,6 +20,9 @@ properties:
>    compatible:
>      oneOf:
>        - enum:
> +          - maxim,max7356
> +          - maxim,max7357
> +          - maxim,max7358
>            - nxp,pca9540
>            - nxp,pca9542
>            - nxp,pca9543
> @@ -40,6 +44,7 @@ properties:
>  
>    interrupts:
>      maxItems: 1
> +    description: Only supported on NXP devices. Unsupported on Maxim MAX735x.

You can express that as an if/then schema.

Just 'interrupts: false' for maxim compatibles. There's lots of 
examples in the tree.

>  
>    "#interrupt-cells":
>      const: 2
> @@ -100,6 +105,41 @@ examples:
>                  #size-cells = <0>;
>                  reg = <4>;
>  
> +                rtc@51 {
> +                    compatible = "nxp,pcf8563";
> +                    reg = <0x51>;
> +                };

Unrelated change.

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

Really need another example?

> +
> +        i2c-mux@74 {
> +            compatible = "maxim,max7357";
> +            #address-cells = <1>;
> +            #size-cells = <0>;
> +            reg = <0x74>;
> +
> +            i2c@1 {
> +                #address-cells = <1>;
> +                #size-cells = <0>;
> +                reg = <1>;
> +
> +                eeprom@54 {
> +                    compatible = "atmel,24c08";
> +                    reg = <0x54>;
> +                };
> +            };
> +
> +            i2c@7 {
> +                #address-cells = <1>;
> +                #size-cells = <0>;
> +                reg = <7>;
> +
>                  rtc@51 {
>                      compatible = "nxp,pcf8563";
>                      reg = <0x51>;
> -- 
> 2.33.1
> 
> 

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

* Re: [PATCH 1/4] dt-bindings: i2c Update PCA954x
  2021-12-15 14:19     ` Patrick Rudolph
@ 2021-12-15 21:22       ` Laurent Pinchart
  0 siblings, 0 replies; 11+ messages in thread
From: Laurent Pinchart @ 2021-12-15 21:22 UTC (permalink / raw)
  To: Patrick Rudolph
  Cc: Peter Rosin, Rob Herring, linux-i2c, devicetree, linux-kernel

Hi Patrick,

On Wed, Dec 15, 2021 at 03:19:37PM +0100, Patrick Rudolph wrote:
> Hi Peter,
> thanks for your feedback.
> You are right, the added Maxim chips should not have set the interrupt-controller; flag.
> 
> The reason I decided to not handle that interrupt is that I don't know where to pass that bus error to.
> It looks like only the I2C master can signal bus errors by returning -EIO, however there's no API for I2C clients
> to pass such errors to the master. However any attempt to access the stuck and isolated bus will fail and
> the address will be NACKed, so I don't think that this a big issue as in the end the bus stall will be detected.

You don't have to handle interrupts in the driver in order to declare
the interrupts property in the bindings. If the device has an interrupt
output that is meant to be connected to an interrupt input of the SoC,
then an interrupt property should describe how that signal is connected.
You can upstream support for those devices in the driver without
handlign the interrupt, it can be added later.

> Is there a mapping between devicetree bindings and driver file names? If not I'll use
> maxim,max7356 as devicetree binding to make it easier to read and mention that interrupts
> are not supported for those maxim devices.

The bindings and drivers file names are usually related, as they support
the same device, but there's no specific rule that requires that.

> On Wed, Dec 15, 2021 at 1:42 PM Peter Rosin <peda@axentia.se> wrote:
> > On 2021-12-14 12:13, Laurent Pinchart wrote:
> > > Hi Patrick,
> > >
> > > Thank you for the patch.
> > >
> > > On Tue, Dec 14, 2021 at 10:50:18AM +0100, Patrick Rudolph wrote:
> > >> Add the Maxim MAX735x as supported chip to PCA954x and add an
> > >> example how to use it.
> > >>
> > >> Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
> > >> ---
> > >>  .../bindings/i2c/i2c-mux-pca954x.yaml         | 40 +++++++++++++++++++
> > >>  1 file changed, 40 insertions(+)
> > >>
> > >> diff --git a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml
> > >> index 9f1726d0356b..bd794cb80c11 100644
> > >> --- a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml
> > >> +++ b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml
> > >> @@ -11,6 +11,7 @@ maintainers:
> > >>
> > >>  description:
> > >>    The binding supports NXP PCA954x and PCA984x I2C mux/switch devices.
> > >> +  Compatible with Maxim MAX7356 - MAX7358 I2C mux/switch devices.
> > >>
> > >>  allOf:
> > >>    - $ref: /schemas/i2c/i2c-mux.yaml#
> > >> @@ -19,6 +20,9 @@ properties:
> > >>    compatible:
> > >>      oneOf:
> > >>        - enum:
> > >> +          - maxim,max7356
> > >> +          - maxim,max7357
> > >> +          - maxim,max7358
> > >>            - nxp,pca9540
> > >>            - nxp,pca9542
> > >>            - nxp,pca9543
> > >> @@ -40,6 +44,7 @@ properties:
> > >>
> > >>    interrupts:
> > >>      maxItems: 1
> > >> +    description: Only supported on NXP devices. Unsupported on Maxim MAX735x.
> > >
> > > Could this be modelled by a YAML schema instead ? Something like
> > >
> > > allOf:
> > >   - if:
> > >       properties:
> > >         compatible:
> > >         contains:
> > >           enum:
> > >               - maxim,max7356
> > >               - maxim,max7357
> > >               - maxim,max7358
> > >     then:
> > >       properties:
> > >         interrupts: false
> > >
> > > (untested, it would be nice to use a pattern check for the compatible
> > > property if possible)
> >
> > Some of the existing NXP chips do not support interrupts; we should
> > probably treat these new chips the same as the older ones. Either by
> > disallowing interrupts on both kinds or by continuing to ignore the
> > situation.
> >
> > That said, I'm slightly in favor of the latter, since these new chips
> > do have interrupts, just not the same flavor as the NXP chips. What
> > the Maxim chips do not have is support for being an
> >         interrupt-controller;
> > At least that's how I read it...
> >
> > I don't know how this situation is supposed to be described? Maybe this
> > new kind of interrupt should be indicated with a bus-error-interrupts
> > property (or bikeshed along those lines)? Maybe there should be two
> > entries in the existing interrupts property? Maybe these new chips
> > should be described in a new binding specific to maxim,max7356-7358
> > (could still be handled by the pca954x driver of course) to keep the
> > yaml simpler to read?
> >
> > However, there is also maxim,max7367-7369 to consider. They seem to
> > have interrupts of the style described by the NXP binding (haven't
> > checked if the registers work the same, but since they reuse the
> > 0x70 address-range the are in all likelihood also compatible).
> >
> > >>    "#interrupt-cells":
> > >>      const: 2
> > >> @@ -100,6 +105,41 @@ examples:
> > >>                  #size-cells = <0>;
> > >>                  reg = <4>;
> > >>
> > >> +                rtc@51 {
> > >> +                    compatible = "nxp,pcf8563";
> > >> +                    reg = <0x51>;
> > >> +                };
> > >> +            };
> > >> +        };
> > >> +    };
> > >> +
> > >> +  - |
> > >> +    i2c {
> > >> +        #address-cells = <1>;
> > >> +        #size-cells = <0>;
> > >> +
> > >> +        i2c-mux@74 {
> > >> +            compatible = "maxim,max7357";
> > >> +            #address-cells = <1>;
> > >> +            #size-cells = <0>;
> > >> +            reg = <0x74>;
> > >> +
> > >> +            i2c@1 {
> > >> +                #address-cells = <1>;
> > >> +                #size-cells = <0>;
> > >> +                reg = <1>;
> > >> +
> > >> +                eeprom@54 {
> > >> +                    compatible = "atmel,24c08";
> > >> +                    reg = <0x54>;
> > >> +                };
> > >> +            };
> > >> +
> > >> +            i2c@7 {
> > >> +                #address-cells = <1>;
> > >> +                #size-cells = <0>;
> > >> +                reg = <7>;
> > >> +
> > >>                  rtc@51 {
> > >>                      compatible = "nxp,pcf8563";
> > >>                      reg = <0x51>;

-- 
Regards,

Laurent Pinchart

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

end of thread, other threads:[~2021-12-15 21:22 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-14  9:50 [PATCH 1/4] dt-bindings: i2c Update PCA954x Patrick Rudolph
2021-12-14  9:50 ` [PATCH 2/4] drivers/i2c/mux: Add MAX735x support to PCA954x Patrick Rudolph
2021-12-14  9:50 ` [PATCH 3/4] dt-bindings: i2c Add regulator to pca954x Patrick Rudolph
2021-12-14 11:37   ` Laurent Pinchart
2021-12-14  9:50 ` [PATCH 4/4] i2c-mux-pca954x: Add regulator support Patrick Rudolph
2021-12-14 13:12   ` kernel test robot
2021-12-14 11:13 ` [PATCH 1/4] dt-bindings: i2c Update PCA954x Laurent Pinchart
2021-12-15 12:42   ` Peter Rosin
2021-12-15 14:19     ` Patrick Rudolph
2021-12-15 21:22       ` Laurent Pinchart
2021-12-15 20:33 ` Rob Herring

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