Add the Maxim MAX735x as supported chip to PCA954x and add an
example how to use it.
Signed-off-by: Patrick Rudolph <[email protected]>
---
.../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
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 <[email protected]>
---
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
Add a regulator called vcc and update the example.
Signed-off-by: Patrick Rudolph <[email protected]>
---
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
Add an optional vcc regulator and enable it when found for devices
that are powered off by default.
Signed-off-by: Patrick Rudolph <[email protected]>
---
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
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 <[email protected]>
> ---
> .../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
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 <[email protected]>
> ---
> 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 <[email protected]>
> +
> 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
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/[email protected]/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 <[email protected]>
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/[email protected]
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 <[email protected]>
>> ---
>> .../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>;
>
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 <[email protected]> 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 <[email protected]>
> >> ---
> >> .../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>;
> >
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 <[email protected]>
> ---
> .../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
>
>
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 <[email protected]> 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 <[email protected]>
> > >> ---
> > >> .../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