2023-05-01 09:17:02

by Patrick Rudolph

[permalink] [raw]
Subject: [PATCH v14 0/4] Add support for Maxim MAX735x/MAX736x variants

v14:
- Added comment for interrupt support

v13:
- Fix dt-binding
- Fixed nits

v12:
- Add separate patch correcting interrupt support in dt-binding
- Fix typo in commit message
- Make vdd-supply non optional

v11:
- Fix dt-binding example

v10:
- Small updates to dt-bindings
- Make vdd-supply optional
- Drop MAX7357 enhanced mode configuration

v9:
- Fix 'then' not aligned with 'if' in dt-bindings
- Split enhanced mode configuration into separate patch
- Add MAX7357/MAX7358 register definitions
- Rename config register defines
- Update comments and explain non default config being applied on MAX7357
- Check for I2C_FUNC_SMBUS_WRITE_BYTE_DATA functionality

v8:
- Move allOf in dt-binding and use double negation

v7:
- Reworked the commit message, comments and renamed a struct
field. No functional change.

v6:
- Fix typo in dt-bindings

v5:
- Remove optional and make vdd-supply mandatory

v4:
- Add missing maxitems dt-bindings property

v3:
- Merge dt-bindings into i2c-mux-pca954x.yaml

v2:
- Move dt-bindings to separate file
- Added support for MAX736x as they are very similar
- Fixed an issue found by kernel test robot
- Dropped max735x property and custom IRQ check
- Added MAX7357 config register defines instead of magic values
- Renamed vcc-supply to vdd-supply

Patrick Rudolph (4):
dt-bindings: i2c: pca954x: Correct interrupt support
dt-bindings: i2c: Add Maxim MAX735x/MAX736x variants
i2c: muxes: pca954x: Add MAX735x/MAX736x support
i2c: muxes: pca954x: Add regulator support

.../bindings/i2c/i2c-mux-pca954x.yaml | 45 +++++++--
drivers/i2c/muxes/Kconfig | 6 +-
drivers/i2c/muxes/i2c-mux-pca954x.c | 92 +++++++++++++++++--
3 files changed, 128 insertions(+), 15 deletions(-)

--
2.39.2


2023-05-01 09:17:19

by Patrick Rudolph

[permalink] [raw]
Subject: [PATCH v14 1/4] dt-bindings: i2c: pca954x: Correct interrupt support

Only some of the PCA954x compatible ICs have interrupt
capability, but the binding advertises it on all ICs.

Sync the dt-binding with the driver and only advertise it on:
- nxp,pca9542
- nxp,pca9543
- nxp,pca9544
- nxp,pca9545

Signed-off-by: Patrick Rudolph <[email protected]>
Reviewed-by: Krzysztof Kozlowski <[email protected]>
---
.../bindings/i2c/i2c-mux-pca954x.yaml | 23 +++++++++++++++----
1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml
index 9f1726d0356b..e5c1070903ef 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml
+++ b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml
@@ -12,9 +12,6 @@ maintainers:
description:
The binding supports NXP PCA954x and PCA984x I2C mux/switch devices.

-allOf:
- - $ref: /schemas/i2c/i2c-mux.yaml#
-
properties:
compatible:
oneOf:
@@ -63,6 +60,24 @@ required:
- compatible
- reg

+allOf:
+ - $ref: /schemas/i2c/i2c-mux.yaml#
+ - if:
+ not:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - nxp,pca9542
+ - nxp,pca9543
+ - nxp,pca9544
+ - nxp,pca9545
+ then:
+ properties:
+ interrupts: false
+ "#interrupt-cells": false
+ interrupt-controller: false
+
unevaluatedProperties: false

examples:
@@ -74,7 +89,7 @@ examples:
#size-cells = <0>;

i2c-mux@74 {
- compatible = "nxp,pca9548";
+ compatible = "nxp,pca9545";
#address-cells = <1>;
#size-cells = <0>;
reg = <0x74>;
--
2.39.2

2023-05-01 09:17:29

by Patrick Rudolph

[permalink] [raw]
Subject: [PATCH v14 4/4] i2c: muxes: pca954x: Add regulator support

Add a vdd regulator and enable it for boards that have the
mux powered off by default.

Signed-off-by: Patrick Rudolph <[email protected]>
Reviewed-by: Andi Shyti <[email protected]>
---
drivers/i2c/muxes/i2c-mux-pca954x.c | 28 +++++++++++++++++++++++-----
1 file changed, 23 insertions(+), 5 deletions(-)

diff --git a/drivers/i2c/muxes/i2c-mux-pca954x.c b/drivers/i2c/muxes/i2c-mux-pca954x.c
index 968111442625..b863f7b18190 100644
--- a/drivers/i2c/muxes/i2c-mux-pca954x.c
+++ b/drivers/i2c/muxes/i2c-mux-pca954x.c
@@ -49,6 +49,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 MAX735x, PCA954x and PCA984x types we know about */
@@ -444,6 +446,8 @@ static void pca954x_cleanup(struct i2c_mux_core *muxc)
struct pca954x *data = i2c_mux_priv(muxc);
int c, irq;

+ regulator_disable(data->supply);
+
if (data->irq) {
for (c = 0; c < data->chip->nchans; c++) {
irq = irq_find_mapping(data->irq, c);
@@ -496,10 +500,22 @@ static int pca954x_probe(struct i2c_client *client)
i2c_set_clientdata(client, muxc);
data->client = client;

+ data->supply = devm_regulator_get(dev, "vdd");
+ if (IS_ERR(data->supply))
+ return dev_err_probe(dev, PTR_ERR(data->supply),
+ "Failed to request regulator\n");
+
+ ret = regulator_enable(data->supply);
+ if (ret)
+ return dev_err_probe(dev, ret,
+ "Failed to enable vdd supply\n");
+
/* 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);
@@ -516,7 +532,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 ||
@@ -524,7 +540,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;
}
}

@@ -543,7 +560,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.39.2

2023-05-01 09:17:30

by Patrick Rudolph

[permalink] [raw]
Subject: [PATCH v14 3/4] i2c: muxes: pca954x: Add MAX735x/MAX736x support

Add support for the following Maxim chips using the existing PCA954x
driver:
- MAX7356
- MAX7357
- MAX7358
- MAX7367
- MAX7368
- MAX7369

All added Maxim chips behave like the PCA954x, where a single SMBUS byte
write selects up to 8 channels to be bridged to the primary bus.

While the MAX7357/MAX7358 have interrupt support, they don't act as
interrupt controller like the PCA9545 does. Thus don't enable IRQ support
and handle them like the PCA9548.

Tested using the MAX7357.

Signed-off-by: Patrick Rudolph <[email protected]>
---
drivers/i2c/muxes/Kconfig | 6 +--
drivers/i2c/muxes/i2c-mux-pca954x.c | 64 ++++++++++++++++++++++++++++-
2 files changed, 66 insertions(+), 4 deletions(-)

diff --git a/drivers/i2c/muxes/Kconfig b/drivers/i2c/muxes/Kconfig
index ea838dbae32e..db1b9057612a 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/PCA984x and Maxim MAX735x/MAX736x 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.
+ If you say yes here you get support for NXP PCA954x/PCA984x
+ and Maxim MAX735x/MAX736x 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 0ccee2ae5720..968111442625 100644
--- a/drivers/i2c/muxes/i2c-mux-pca954x.c
+++ b/drivers/i2c/muxes/i2c-mux-pca954x.c
@@ -4,6 +4,7 @@
*
* Copyright (c) 2008-2009 Rodolfo Giometti <[email protected]>
* Copyright (c) 2008-2009 Eurotech S.p.A. <[email protected]>
+ * Copyright (c) 2022 9elements GmbH <[email protected]>
*
* This module supports the PCA954x and PCA984x series of I2C multiplexer/switch
* chips made by NXP Semiconductors.
@@ -11,6 +12,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 are controlled
+ * as the NXP PCA9548 and the MAX736x chips that act like the PCA9544.
+ *
+ * This includes the:
+ * MAX7356, MAX7357, MAX7358, MAX7367, MAX7368 and MAX7369
+ *
* 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
@@ -51,6 +58,12 @@
#define PCA954X_IRQ_OFFSET 4

enum pca_type {
+ max_7356,
+ max_7357,
+ max_7358,
+ max_7367,
+ max_7368,
+ max_7369,
pca_9540,
pca_9542,
pca_9543,
@@ -90,8 +103,45 @@ struct pca954x {
raw_spinlock_t lock;
};

-/* Provide specs for the PCA954x types we know about */
+/* Provide specs for the MAX735x, PCA954x and PCA984x types we know about */
static const struct chip_desc chips[] = {
+ [max_7356] = {
+ .nchans = 8,
+ .muxtype = pca954x_isswi,
+ .id = { .manufacturer_id = I2C_DEVICE_ID_NONE },
+ },
+ [max_7357] = {
+ .nchans = 8,
+ .muxtype = pca954x_isswi,
+ .id = { .manufacturer_id = I2C_DEVICE_ID_NONE },
+ /* No interrupt controller support.
+ The interrupt provides information about stuck channels. */
+ },
+ [max_7358] = {
+ .nchans = 8,
+ .muxtype = pca954x_isswi,
+ .id = { .manufacturer_id = I2C_DEVICE_ID_NONE },
+ /* No interrupt controller support.
+ The interrupt provides information about stuck channels. */
+ },
+ [max_7367] = {
+ .nchans = 4,
+ .muxtype = pca954x_isswi,
+ .has_irq = 1,
+ .id = { .manufacturer_id = I2C_DEVICE_ID_NONE },
+ },
+ [max_7368] = {
+ .nchans = 4,
+ .muxtype = pca954x_isswi,
+ .id = { .manufacturer_id = I2C_DEVICE_ID_NONE },
+ },
+ [max_7369] = {
+ .nchans = 4,
+ .enable = 0x4,
+ .muxtype = pca954x_ismux,
+ .has_irq = 1,
+ .id = { .manufacturer_id = I2C_DEVICE_ID_NONE },
+ },
[pca_9540] = {
.nchans = 2,
.enable = 0x4,
@@ -177,6 +227,12 @@ static const struct chip_desc chips[] = {
};

static const struct i2c_device_id pca954x_id[] = {
+ { "max7356", max_7356 },
+ { "max7357", max_7357 },
+ { "max7358", max_7358 },
+ { "max7367", max_7367 },
+ { "max7368", max_7368 },
+ { "max7369", max_7369 },
{ "pca9540", pca_9540 },
{ "pca9542", pca_9542 },
{ "pca9543", pca_9543 },
@@ -194,6 +250,12 @@ 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 = "maxim,max7367", .data = &chips[max_7367] },
+ { .compatible = "maxim,max7368", .data = &chips[max_7368] },
+ { .compatible = "maxim,max7369", .data = &chips[max_7369] },
{ .compatible = "nxp,pca9540", .data = &chips[pca_9540] },
{ .compatible = "nxp,pca9542", .data = &chips[pca_9542] },
{ .compatible = "nxp,pca9543", .data = &chips[pca_9543] },
--
2.39.2

2023-05-01 09:18:00

by Patrick Rudolph

[permalink] [raw]
Subject: [PATCH v14 2/4] dt-bindings: i2c: Add Maxim MAX735x/MAX736x variants

Update the pca954x bindings to add support for the Maxim MAX735x/MAX736x
chips. The functionality will be provided by the existing pca954x driver.

For chips that are powered off by default add a regulator called vdd-supply.

Signed-off-by: Patrick Rudolph <[email protected]>
Reviewed-by: Krzysztof Kozlowski <[email protected]>
---
.../bindings/i2c/i2c-mux-pca954x.yaml | 22 +++++++++++++++++--
1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml
index e5c1070903ef..6fed6eae9c9b 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml
+++ b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml
@@ -4,18 +4,29 @@
$id: http://devicetree.org/schemas/i2c/i2c-mux-pca954x.yaml#
$schema: http://devicetree.org/meta-schemas/core.yaml#

-title: NXP PCA954x I2C bus switch
+title: NXP PCA954x I2C and compatible bus switches

maintainers:
- Laurent Pinchart <[email protected]>

description:
- The binding supports NXP PCA954x and PCA984x I2C mux/switch devices.
+ The NXP PCA954x and compatible devices are I2C bus
+ multiplexer/switches that share the same functionality
+ and register layout.
+ The devices usually have 4 or 8 child buses, which are
+ attached to the parent bus by using the SMBus "Send Byte"
+ command.

properties:
compatible:
oneOf:
- enum:
+ - maxim,max7356
+ - maxim,max7357
+ - maxim,max7358
+ - maxim,max7367
+ - maxim,max7368
+ - maxim,max7369
- nxp,pca9540
- nxp,pca9542
- nxp,pca9543
@@ -56,6 +67,9 @@ properties:
description: if present, overrides i2c-mux-idle-disconnect
$ref: /schemas/mux/mux-controller.yaml#/properties/idle-state

+ vdd-supply:
+ description: A voltage regulator supplying power to the chip.
+
required:
- compatible
- reg
@@ -68,6 +82,8 @@ allOf:
compatible:
contains:
enum:
+ - maxim,max7367
+ - maxim,max7369
- nxp,pca9542
- nxp,pca9543
- nxp,pca9544
@@ -94,6 +110,8 @@ examples:
#size-cells = <0>;
reg = <0x74>;

+ vdd-supply = <&p3v3>;
+
interrupt-parent = <&ipic>;
interrupts = <17 IRQ_TYPE_LEVEL_LOW>;
interrupt-controller;
--
2.39.2

2023-05-02 06:15:51

by Peter Rosin

[permalink] [raw]
Subject: Re: [PATCH v14 2/4] dt-bindings: i2c: Add Maxim MAX735x/MAX736x variants

Hi!

2023-05-01 at 11:15, Patrick Rudolph wrote:
> Update the pca954x bindings to add support for the Maxim MAX735x/MAX736x
> chips. The functionality will be provided by the existing pca954x driver.
>
> For chips that are powered off by default add a regulator called vdd-supply.
>
> Signed-off-by: Patrick Rudolph <[email protected]>
> Reviewed-by: Krzysztof Kozlowski <[email protected]>
> ---
> .../bindings/i2c/i2c-mux-pca954x.yaml | 22 +++++++++++++++++--
> 1 file changed, 20 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml
> index e5c1070903ef..6fed6eae9c9b 100644
> --- a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml
> +++ b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml
> @@ -4,18 +4,29 @@
> $id: http://devicetree.org/schemas/i2c/i2c-mux-pca954x.yaml#
> $schema: http://devicetree.org/meta-schemas/core.yaml#
>
> -title: NXP PCA954x I2C bus switch
> +title: NXP PCA954x I2C and compatible bus switches
>
> maintainers:
> - Laurent Pinchart <[email protected]>
>
> description:
> - The binding supports NXP PCA954x and PCA984x I2C mux/switch devices.
> + The NXP PCA954x and compatible devices are I2C bus
> + multiplexer/switches that share the same functionality
> + and register layout.
> + The devices usually have 4 or 8 child buses, which are
> + attached to the parent bus by using the SMBus "Send Byte"
> + command.
>
> properties:
> compatible:
> oneOf:
> - enum:
> + - maxim,max7356
> + - maxim,max7357
> + - maxim,max7358
> + - maxim,max7367
> + - maxim,max7368
> + - maxim,max7369
> - nxp,pca9540
> - nxp,pca9542
> - nxp,pca9543
> @@ -56,6 +67,9 @@ properties:
> description: if present, overrides i2c-mux-idle-disconnect
> $ref: /schemas/mux/mux-controller.yaml#/properties/idle-state
>
> + vdd-supply:
> + description: A voltage regulator supplying power to the chip.
> +

The pca9846-9 chips do not have one VDD, they have separate supplies for
"low level" (VDD1), and "core logic" (VDD2). I don't know how such a
situation is normally reflected in bindings, but could it not cause
headaches later if use of unqualified VDD is spreading for those chips?
Possibly with different semantics depending on if vdd-supply is tied to
VDD1, VDD2 or both?

Cheers,
Peter

> required:
> - compatible
> - reg
> @@ -68,6 +82,8 @@ allOf:
> compatible:
> contains:
> enum:
> + - maxim,max7367
> + - maxim,max7369
> - nxp,pca9542
> - nxp,pca9543
> - nxp,pca9544
> @@ -94,6 +110,8 @@ examples:
> #size-cells = <0>;
> reg = <0x74>;
>
> + vdd-supply = <&p3v3>;
> +
> interrupt-parent = <&ipic>;
> interrupts = <17 IRQ_TYPE_LEVEL_LOW>;
> interrupt-controller;

2023-05-02 06:54:35

by Patrick Rudolph

[permalink] [raw]
Subject: Re: [PATCH v14 2/4] dt-bindings: i2c: Add Maxim MAX735x/MAX736x variants

Hi Peter,
it could indeed cause problems when VDD1 != VDD2 and at both needs to
be enabled.
The pca9846 datasheet seems to refer to VDD1 as VDD. Thus I could add
an optional "vdd2" regulator to the binding and driver.

Please let me know if that's what you had in mind.
Regards,
Patrick

On Tue, May 2, 2023 at 8:03 AM Peter Rosin <[email protected]> wrote:
>
> Hi!
>
> 2023-05-01 at 11:15, Patrick Rudolph wrote:
> > Update the pca954x bindings to add support for the Maxim MAX735x/MAX736x
> > chips. The functionality will be provided by the existing pca954x driver.
> >
> > For chips that are powered off by default add a regulator called vdd-supply.
> >
> > Signed-off-by: Patrick Rudolph <[email protected]>
> > Reviewed-by: Krzysztof Kozlowski <[email protected]>
> > ---
> > .../bindings/i2c/i2c-mux-pca954x.yaml | 22 +++++++++++++++++--
> > 1 file changed, 20 insertions(+), 2 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml
> > index e5c1070903ef..6fed6eae9c9b 100644
> > --- a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml
> > +++ b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml
> > @@ -4,18 +4,29 @@
> > $id: http://devicetree.org/schemas/i2c/i2c-mux-pca954x.yaml#
> > $schema: http://devicetree.org/meta-schemas/core.yaml#
> >
> > -title: NXP PCA954x I2C bus switch
> > +title: NXP PCA954x I2C and compatible bus switches
> >
> > maintainers:
> > - Laurent Pinchart <[email protected]>
> >
> > description:
> > - The binding supports NXP PCA954x and PCA984x I2C mux/switch devices.
> > + The NXP PCA954x and compatible devices are I2C bus
> > + multiplexer/switches that share the same functionality
> > + and register layout.
> > + The devices usually have 4 or 8 child buses, which are
> > + attached to the parent bus by using the SMBus "Send Byte"
> > + command.
> >
> > properties:
> > compatible:
> > oneOf:
> > - enum:
> > + - maxim,max7356
> > + - maxim,max7357
> > + - maxim,max7358
> > + - maxim,max7367
> > + - maxim,max7368
> > + - maxim,max7369
> > - nxp,pca9540
> > - nxp,pca9542
> > - nxp,pca9543
> > @@ -56,6 +67,9 @@ properties:
> > description: if present, overrides i2c-mux-idle-disconnect
> > $ref: /schemas/mux/mux-controller.yaml#/properties/idle-state
> >
> > + vdd-supply:
> > + description: A voltage regulator supplying power to the chip.
> > +
>
> The pca9846-9 chips do not have one VDD, they have separate supplies for
> "low level" (VDD1), and "core logic" (VDD2). I don't know how such a
> situation is normally reflected in bindings, but could it not cause
> headaches later if use of unqualified VDD is spreading for those chips?
> Possibly with different semantics depending on if vdd-supply is tied to
> VDD1, VDD2 or both?
>
> Cheers,
> Peter
>
> > required:
> > - compatible
> > - reg
> > @@ -68,6 +82,8 @@ allOf:
> > compatible:
> > contains:
> > enum:
> > + - maxim,max7367
> > + - maxim,max7369
> > - nxp,pca9542
> > - nxp,pca9543
> > - nxp,pca9544
> > @@ -94,6 +110,8 @@ examples:
> > #size-cells = <0>;
> > reg = <0x74>;
> >
> > + vdd-supply = <&p3v3>;
> > +
> > interrupt-parent = <&ipic>;
> > interrupts = <17 IRQ_TYPE_LEVEL_LOW>;
> > interrupt-controller;

2023-05-02 08:49:31

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v14 2/4] dt-bindings: i2c: Add Maxim MAX735x/MAX736x variants

On 02/05/2023 08:52, Patrick Rudolph wrote:
> Hi Peter,
> it could indeed cause problems when VDD1 != VDD2 and at both needs to
> be enabled.
> The pca9846 datasheet seems to refer to VDD1 as VDD. Thus I could add
> an optional "vdd2" regulator to the binding and driver.
>
> Please let me know if that's what you had in mind.

Don't top post.

In such case vdd-supply should not be used for VDD2.

Best regards,
Krzysztof

2023-05-02 10:41:41

by Peter Rosin

[permalink] [raw]
Subject: Re: [PATCH v14 2/4] dt-bindings: i2c: Add Maxim MAX735x/MAX736x variants

Hi!

2023-05-02 at 10:46, Krzysztof Kozlowski wrote:
> On 02/05/2023 08:52, Patrick Rudolph wrote:
>> Hi Peter,
>> it could indeed cause problems when VDD1 != VDD2 and at both needs to
>> be enabled.
>> The pca9846 datasheet seems to refer to VDD1 as VDD. Thus I could add
>> an optional "vdd2" regulator to the binding and driver.
>>
>> Please let me know if that's what you had in mind.
>
> Don't top post.
>
> In such case vdd-supply should not be used for VDD2.

When reading the data sheet [1], I get the feeling that the instances
of VDD are either copy-paste errors from data sheets from chip with a
single VDD, or a reference to either of VDD1 or VDD2. It is thus not
super clear to me that VDD should be the same thing as VDD1.

Sure, there is section 6.5 "Power-on reset", which mentions VDD and
VDD2 (but not VDD1), but that seems like a simply typo and that it
should really have been VDD1 instead of an unqualified VDD.

There are also various timings "glitch supply voltage difference"
(delta VDD(gl)) and "supply voltage glitch pulse width" (t w(gl)VDD)
with notes that refer to VDD2, which *could* indicate that the
glitch in VDD is about a glitch VDD1. But it could also mean glitches
on any of VDD1 and VDD2?

The general description of the chip indicates that VDD1 is there
mainly to allow different bus voltages on each of the channels.
Which is not at all the function of VDD on the other chips. Meanwhile
VDD2 "is the core logic supply from which most of the PCA9846
circuitry runs", and seems like it is a better match for plain VDD?

Maybe one can find out more by reading the spec more carefully, but
as I said, it is not clear to me that either of VDD1 or VDD2 can be
matched to VDD.

Perhaps it is best to not mix things at all?

[1] https://www.nxp.com/docs/en/data-sheet/PCA9846.pdf

2023-07-31 16:36:58

by Naresh Solanki

[permalink] [raw]
Subject: Re: [PATCH v14 2/4] dt-bindings: i2c: Add Maxim MAX735x/MAX736x variants

Hi Peter,

On 02-05-2023 16:06, Peter Rosin wrote:
> Hi!
>
> 2023-05-02 at 10:46, Krzysztof Kozlowski wrote:
>> On 02/05/2023 08:52, Patrick Rudolph wrote:
>>> Hi Peter,
>>> it could indeed cause problems when VDD1 != VDD2 and at both needs to
>>> be enabled.
>>> The pca9846 datasheet seems to refer to VDD1 as VDD. Thus I could add
>>> an optional "vdd2" regulator to the binding and driver.
>>>
>>> Please let me know if that's what you had in mind.
>> Don't top post.
>>
>> In such case vdd-supply should not be used for VDD2.
> When reading the data sheet [1], I get the feeling that the instances
> of VDD are either copy-paste errors from data sheets from chip with a
> single VDD, or a reference to either of VDD1 or VDD2. It is thus not
> super clear to me that VDD should be the same thing as VDD1.
>
> Sure, there is section 6.5 "Power-on reset", which mentions VDD and
> VDD2 (but not VDD1), but that seems like a simply typo and that it
> should really have been VDD1 instead of an unqualified VDD.
>
> There are also various timings "glitch supply voltage difference"
> (delta VDD(gl)) and "supply voltage glitch pulse width" (t w(gl)VDD)
> with notes that refer to VDD2, which *could* indicate that the
> glitch in VDD is about a glitch VDD1. But it could also mean glitches
> on any of VDD1 and VDD2?
>
> The general description of the chip indicates that VDD1 is there
> mainly to allow different bus voltages on each of the channels.
> Which is not at all the function of VDD on the other chips. Meanwhile
> VDD2 "is the core logic supply from which most of the PCA9846
> circuitry runs", and seems like it is a better match for plain VDD?
Yes, based on Figure 14 in datasheet, VDD2 seems to be better match
for plain VDD.
Also VDD1 is I2C bus voltage on micro-controller side so the best
match I can think of is VBUS.
>
> Maybe one can find out more by reading the spec more carefully, but
> as I said, it is not clear to me that either of VDD1 or VDD2 can be
> matched to VDD.
>
> Perhaps it is best to not mix things at all?
Yes. For designs with same voltage rails, "VDD" can serve the purpose.
For designs with different voltage rail, VBUS would be needed to
identify micro-controller side bus supply.
Let me know your thoughts.

Regards,
Naresh
>
> [1] https://www.nxp.com/docs/en/data-sheet/PCA9846.pdf
>