Message ID | 20230817071056.2125679-1-patrick.rudolph@9elements.com |
---|---|
Headers | show |
Series | Add support for Maxim MAX735x/MAX736x variants | expand |
Hi! 2023-08-17 at 09:10, Patrick Rudolph wrote: > 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 <patrick.rudolph@9elements.com> > Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> Acked-by: Peter Rosin <peda@axentia.se> Cheers, Peter
Hi! 2023-08-17 at 09:10, Patrick Rudolph wrote: > 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 <patrick.rudolph@9elements.com> Acked-by: Peter Rosin <peda@axentia.se> /* * However, if you find yourself doing a v16 for some reason, please * update new multi-line comments to match the preferred coding style. */ Cheers, Peter > --- > 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 <giometti@linux.it> > * Copyright (c) 2008-2009 Eurotech S.p.A. <info@eurotech.it> > + * Copyright (c) 2022 9elements GmbH <patrick.rudolph@9elements.com> > * > * 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] },
Hi Patrick, > 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 <giometti@linux.it> > * Copyright (c) 2008-2009 Eurotech S.p.A. <info@eurotech.it> > + * Copyright (c) 2022 9elements GmbH <patrick.rudolph@9elements.com> 2023? One question, why are you adding yourself in the copyright? > * 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. */ I'm sorry, Peter already commented on this, can you please fix the commenting? /* * No interrupt controller support. * The interrupt provides information * about stuck channels. */ Or /* * No interrupt controller support. The interrupt * provides information * about stuck channels. */ to save one line. (I prefer the latter, your choice) Rest looks good. Andi > + }, > + [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.41.0 >
On Sun, Aug 20, 2023 at 1:23 PM Andi Shyti <andi.shyti@kernel.org> wrote: > > Hi Patrick, > > > 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 <giometti@linux.it> > > * Copyright (c) 2008-2009 Eurotech S.p.A. <info@eurotech.it> > > + * Copyright (c) 2022 9elements GmbH <patrick.rudolph@9elements.com> > > 2023? > > One question, why are you adding yourself in the copyright? Can't remember the reason. It's been a while since I created the first version. Will drop the line. > > > * 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. */ > > I'm sorry, Peter already commented on this, can you please fix > the commenting? > > /* > * No interrupt controller support. > * The interrupt provides information > * about stuck channels. > */ > > Or > > /* > * No interrupt controller support. The interrupt > * provides information * about stuck channels. > */ > > to save one line. (I prefer the latter, your choice) > I'm sorry, I overlooked this one. Will send a new version. > Rest looks good. > > Andi > > > + }, > > + [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.41.0 > >