[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CALNFmy0d2brVmu-z4mQwe=wyQqP7FBnmjufCjRyHhhPJmeSUQg@mail.gmail.com>
Date: Mon, 21 Aug 2023 08:20:15 +0200
From: Patrick Rudolph <patrick.rudolph@...ements.com>
To: Andi Shyti <andi.shyti@...nel.org>
Cc: Peter Rosin <peda@...ntia.se>, naresh.solanki@...ements.com,
linux-i2c@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v15 3/4] i2c: muxes: pca954x: Add MAX735x/MAX736x support
On Sun, Aug 20, 2023 at 1:23 PM Andi Shyti <andi.shyti@...nel.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@...ux.it>
> > * Copyright (c) 2008-2009 Eurotech S.p.A. <info@...otech.it>
> > + * Copyright (c) 2022 9elements GmbH <patrick.rudolph@...ements.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
> >
Powered by blists - more mailing lists