| Message ID | 20230427152055.18380-2-henning.schild@siemens.com |
|---|---|
| State | Accepted |
| Commit | 3002b8642f016d7fe3ff56240dacea1075f6b877 |
| Headers | show |
| Series | fix pin count in gpio bank and chip name | expand |
On Thu, Apr 27, 2023 at 05:20:55PM +0200, Henning Schild wrote: > In fact the device with chip id 0xD283 is called NCT6126D, and that is > the chip id the Nuvoton code was written for. Correct that name to avoid > confusion, because a NCT6116D in fact exists as well but has another > chip id, and is currently not supported. > > The look at the spec also revealed that GPIO group7 in fact has 8 pins, > so correct the pin count in that group as well. > > Fixes: d0918a84aff0 ("gpio-f7188x: Add GPIO support for Nuvoton NCT6116") > Reported-by: Xing Tong Wu <xingtong.wu@siemens.com> > Signed-off-by: Henning Schild <henning.schild@siemens.com> Acked-by: Simon Guinot <simon.guinot@sequanux.org> > --- > drivers/gpio/Kconfig | 2 +- > drivers/gpio/gpio-f7188x.c | 28 ++++++++++++++-------------- > 2 files changed, 15 insertions(+), 15 deletions(-) > > diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig > index badbe0582318..14b655411aa0 100644 > --- a/drivers/gpio/Kconfig > +++ b/drivers/gpio/Kconfig > @@ -879,7 +879,7 @@ config GPIO_F7188X > help > This option enables support for GPIOs found on Fintek Super-I/O > chips F71869, F71869A, F71882FG, F71889F and F81866. > - As well as Nuvoton Super-I/O chip NCT6116D. > + As well as Nuvoton Super-I/O chip NCT6126D. > > To compile this driver as a module, choose M here: the module will > be called f7188x-gpio. > diff --git a/drivers/gpio/gpio-f7188x.c b/drivers/gpio/gpio-f7188x.c > index 9effa7769bef..f54ca5a1775e 100644 > --- a/drivers/gpio/gpio-f7188x.c > +++ b/drivers/gpio/gpio-f7188x.c > @@ -48,7 +48,7 @@ > /* > * Nuvoton devices. > */ > -#define SIO_NCT6116D_ID 0xD283 /* NCT6116D chipset ID */ > +#define SIO_NCT6126D_ID 0xD283 /* NCT6126D chipset ID */ > > #define SIO_LD_GPIO_NUVOTON 0x07 /* GPIO logical device */ > > @@ -62,7 +62,7 @@ enum chips { > f81866, > f81804, > f81865, > - nct6116d, > + nct6126d, > }; > > static const char * const f7188x_names[] = { > @@ -74,7 +74,7 @@ static const char * const f7188x_names[] = { > "f81866", > "f81804", > "f81865", > - "nct6116d", > + "nct6126d", > }; > > struct f7188x_sio { > @@ -187,8 +187,8 @@ static int f7188x_gpio_set_config(struct gpio_chip *chip, unsigned offset, > /* Output mode register (0:open drain 1:push-pull). */ > #define f7188x_gpio_out_mode(base) ((base) + 3) > > -#define f7188x_gpio_dir_invert(type) ((type) == nct6116d) > -#define f7188x_gpio_data_single(type) ((type) == nct6116d) > +#define f7188x_gpio_dir_invert(type) ((type) == nct6126d) > +#define f7188x_gpio_data_single(type) ((type) == nct6126d) > > static struct f7188x_gpio_bank f71869_gpio_bank[] = { > F7188X_GPIO_BANK(0, 6, 0xF0, DRVNAME "-0"), > @@ -274,7 +274,7 @@ static struct f7188x_gpio_bank f81865_gpio_bank[] = { > F7188X_GPIO_BANK(60, 5, 0x90, DRVNAME "-6"), > }; > > -static struct f7188x_gpio_bank nct6116d_gpio_bank[] = { > +static struct f7188x_gpio_bank nct6126d_gpio_bank[] = { > F7188X_GPIO_BANK(0, 8, 0xE0, DRVNAME "-0"), > F7188X_GPIO_BANK(10, 8, 0xE4, DRVNAME "-1"), > F7188X_GPIO_BANK(20, 8, 0xE8, DRVNAME "-2"), > @@ -282,7 +282,7 @@ static struct f7188x_gpio_bank nct6116d_gpio_bank[] = { > F7188X_GPIO_BANK(40, 8, 0xF0, DRVNAME "-4"), > F7188X_GPIO_BANK(50, 8, 0xF4, DRVNAME "-5"), > F7188X_GPIO_BANK(60, 8, 0xF8, DRVNAME "-6"), > - F7188X_GPIO_BANK(70, 1, 0xFC, DRVNAME "-7"), > + F7188X_GPIO_BANK(70, 8, 0xFC, DRVNAME "-7"), > }; > > static int f7188x_gpio_get_direction(struct gpio_chip *chip, unsigned offset) > @@ -490,9 +490,9 @@ static int f7188x_gpio_probe(struct platform_device *pdev) > data->nr_bank = ARRAY_SIZE(f81865_gpio_bank); > data->bank = f81865_gpio_bank; > break; > - case nct6116d: > - data->nr_bank = ARRAY_SIZE(nct6116d_gpio_bank); > - data->bank = nct6116d_gpio_bank; > + case nct6126d: > + data->nr_bank = ARRAY_SIZE(nct6126d_gpio_bank); > + data->bank = nct6126d_gpio_bank; > break; > default: > return -ENODEV; > @@ -559,9 +559,9 @@ static int __init f7188x_find(int addr, struct f7188x_sio *sio) > case SIO_F81865_ID: > sio->type = f81865; > break; > - case SIO_NCT6116D_ID: > + case SIO_NCT6126D_ID: > sio->device = SIO_LD_GPIO_NUVOTON; > - sio->type = nct6116d; > + sio->type = nct6126d; > break; > default: > pr_info("Unsupported Fintek device 0x%04x\n", devid); > @@ -569,7 +569,7 @@ static int __init f7188x_find(int addr, struct f7188x_sio *sio) > } > > /* double check manufacturer where possible */ > - if (sio->type != nct6116d) { > + if (sio->type != nct6126d) { > manid = superio_inw(addr, SIO_FINTEK_MANID); > if (manid != SIO_FINTEK_ID) { > pr_debug("Not a Fintek device at 0x%08x\n", addr); > @@ -581,7 +581,7 @@ static int __init f7188x_find(int addr, struct f7188x_sio *sio) > err = 0; > > pr_info("Found %s at %#x\n", f7188x_names[sio->type], (unsigned int)addr); > - if (sio->type != nct6116d) > + if (sio->type != nct6126d) > pr_info(" revision %d\n", superio_inb(addr, SIO_FINTEK_DEVREV)); > > err: > -- > 2.39.2
Hi all The chip id of NCT6116D is 0XD281, you can refer to NCT6116D_Datasheet_V1_0.pdf, Page 291 BRS Xing Tong Wu -----Original Message----- From: Simon Guinot <simon.guinot@sequanux.org> Sent: Friday, April 28, 2023 4:18 PM To: Schild, Henning (T CED SES-DE) <henning.schild@siemens.com> Cc: Linus Walleij <linus.walleij@linaro.org>; Bartosz Golaszewski <brgl@bgdev.pl>; linux-gpio@vger.kernel.org; linux-kernel@vger.kernel.org; Hans de Goede <hdegoede@redhat.com>; Andy Shevchenko <andriy.shevchenko@linux.intel.com>; Wu, Xing Tong (DI FA CTR IPC CN PRC4) <XingTong.Wu@siemens.com> Subject: Re: [PATCH v2 1/1] gpio-f7188x: fix chip name and pin count on Nuvoton chip On Thu, Apr 27, 2023 at 05:20:55PM +0200, Henning Schild wrote: > In fact the device with chip id 0xD283 is called NCT6126D, and that is > the chip id the Nuvoton code was written for. Correct that name to > avoid confusion, because a NCT6116D in fact exists as well but has > another chip id, and is currently not supported. > > The look at the spec also revealed that GPIO group7 in fact has 8 > pins, so correct the pin count in that group as well. > > Fixes: d0918a84aff0 ("gpio-f7188x: Add GPIO support for Nuvoton > NCT6116") > Reported-by: Xing Tong Wu <xingtong.wu@siemens.com> > Signed-off-by: Henning Schild <henning.schild@siemens.com> Acked-by: Simon Guinot <simon.guinot@sequanux.org> > --- > drivers/gpio/Kconfig | 2 +- > drivers/gpio/gpio-f7188x.c | 28 ++++++++++++++-------------- > 2 files changed, 15 insertions(+), 15 deletions(-) > > diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig index > badbe0582318..14b655411aa0 100644 > --- a/drivers/gpio/Kconfig > +++ b/drivers/gpio/Kconfig > @@ -879,7 +879,7 @@ config GPIO_F7188X > help > This option enables support for GPIOs found on Fintek Super-I/O > chips F71869, F71869A, F71882FG, F71889F and F81866. > - As well as Nuvoton Super-I/O chip NCT6116D. > + As well as Nuvoton Super-I/O chip NCT6126D. > > To compile this driver as a module, choose M here: the module will > be called f7188x-gpio. > diff --git a/drivers/gpio/gpio-f7188x.c b/drivers/gpio/gpio-f7188x.c > index 9effa7769bef..f54ca5a1775e 100644 > --- a/drivers/gpio/gpio-f7188x.c > +++ b/drivers/gpio/gpio-f7188x.c > @@ -48,7 +48,7 @@ > /* > * Nuvoton devices. > */ > -#define SIO_NCT6116D_ID 0xD283 /* NCT6116D chipset ID */ > +#define SIO_NCT6126D_ID 0xD283 /* NCT6126D chipset ID */ > > #define SIO_LD_GPIO_NUVOTON 0x07 /* GPIO logical device */ > > @@ -62,7 +62,7 @@ enum chips { > f81866, > f81804, > f81865, > - nct6116d, > + nct6126d, > }; > > static const char * const f7188x_names[] = { @@ -74,7 +74,7 @@ static > const char * const f7188x_names[] = { > "f81866", > "f81804", > "f81865", > - "nct6116d", > + "nct6126d", > }; > > struct f7188x_sio { > @@ -187,8 +187,8 @@ static int f7188x_gpio_set_config(struct gpio_chip > *chip, unsigned offset, > /* Output mode register (0:open drain 1:push-pull). */ #define > f7188x_gpio_out_mode(base) ((base) + 3) > > -#define f7188x_gpio_dir_invert(type) ((type) == nct6116d) > -#define f7188x_gpio_data_single(type) ((type) == nct6116d) > +#define f7188x_gpio_dir_invert(type) ((type) == nct6126d) > +#define f7188x_gpio_data_single(type) ((type) == nct6126d) > > static struct f7188x_gpio_bank f71869_gpio_bank[] = { > F7188X_GPIO_BANK(0, 6, 0xF0, DRVNAME "-0"), @@ -274,7 +274,7 @@ > static struct f7188x_gpio_bank f81865_gpio_bank[] = { > F7188X_GPIO_BANK(60, 5, 0x90, DRVNAME "-6"), }; > > -static struct f7188x_gpio_bank nct6116d_gpio_bank[] = { > +static struct f7188x_gpio_bank nct6126d_gpio_bank[] = { > F7188X_GPIO_BANK(0, 8, 0xE0, DRVNAME "-0"), > F7188X_GPIO_BANK(10, 8, 0xE4, DRVNAME "-1"), > F7188X_GPIO_BANK(20, 8, 0xE8, DRVNAME "-2"), @@ -282,7 +282,7 @@ > static struct f7188x_gpio_bank nct6116d_gpio_bank[] = { > F7188X_GPIO_BANK(40, 8, 0xF0, DRVNAME "-4"), > F7188X_GPIO_BANK(50, 8, 0xF4, DRVNAME "-5"), > F7188X_GPIO_BANK(60, 8, 0xF8, DRVNAME "-6"), > - F7188X_GPIO_BANK(70, 1, 0xFC, DRVNAME "-7"), > + F7188X_GPIO_BANK(70, 8, 0xFC, DRVNAME "-7"), > }; > > static int f7188x_gpio_get_direction(struct gpio_chip *chip, unsigned > offset) @@ -490,9 +490,9 @@ static int f7188x_gpio_probe(struct platform_device *pdev) > data->nr_bank = ARRAY_SIZE(f81865_gpio_bank); > data->bank = f81865_gpio_bank; > break; > - case nct6116d: > - data->nr_bank = ARRAY_SIZE(nct6116d_gpio_bank); > - data->bank = nct6116d_gpio_bank; > + case nct6126d: > + data->nr_bank = ARRAY_SIZE(nct6126d_gpio_bank); > + data->bank = nct6126d_gpio_bank; > break; > default: > return -ENODEV; > @@ -559,9 +559,9 @@ static int __init f7188x_find(int addr, struct f7188x_sio *sio) > case SIO_F81865_ID: > sio->type = f81865; > break; > - case SIO_NCT6116D_ID: > + case SIO_NCT6126D_ID: > sio->device = SIO_LD_GPIO_NUVOTON; > - sio->type = nct6116d; > + sio->type = nct6126d; > break; > default: > pr_info("Unsupported Fintek device 0x%04x\n", devid); @@ -569,7 > +569,7 @@ static int __init f7188x_find(int addr, struct f7188x_sio *sio) > } > > /* double check manufacturer where possible */ > - if (sio->type != nct6116d) { > + if (sio->type != nct6126d) { > manid = superio_inw(addr, SIO_FINTEK_MANID); > if (manid != SIO_FINTEK_ID) { > pr_debug("Not a Fintek device at 0x%08x\n", addr); @@ -581,7 > +581,7 @@ static int __init f7188x_find(int addr, struct f7188x_sio *sio) > err = 0; > > pr_info("Found %s at %#x\n", f7188x_names[sio->type], (unsigned int)addr); > - if (sio->type != nct6116d) > + if (sio->type != nct6126d) > pr_info(" revision %d\n", superio_inb(addr, SIO_FINTEK_DEVREV)); > > err: > -- > 2.39.2
Am Fri, 28 Apr 2023 10:26:18 +0200 schrieb "Wu, Xing Tong (DI FA CTR IPC CN PRC4)" <XingTong.Wu@siemens.com>: > Hi all > > The chip id of NCT6116D is 0XD281, you can refer to > NCT6116D_Datasheet_V1_0.pdf, Page 291 Thanks Xing Tong. I think we have come to agree that for now NCT6116D 0xD281 will not be supported in the kernel. Maybe until someone has access to that very chip and a use-case. But you managed to somehow get these datasheets, which are still not publicly available. Maybe you can use your contacts at Nuvoton to kindly ask them to publish those specs on their website for future reference. Some specs are there, but not all. That would help people to add more chips and avoid mistakes like they happened to me. Henning > BRS > Xing Tong Wu > > -----Original Message----- > From: Simon Guinot <simon.guinot@sequanux.org> > Sent: Friday, April 28, 2023 4:18 PM > To: Schild, Henning (T CED SES-DE) <henning.schild@siemens.com> > Cc: Linus Walleij <linus.walleij@linaro.org>; Bartosz Golaszewski > <brgl@bgdev.pl>; linux-gpio@vger.kernel.org; > linux-kernel@vger.kernel.org; Hans de Goede <hdegoede@redhat.com>; > Andy Shevchenko <andriy.shevchenko@linux.intel.com>; Wu, Xing Tong > (DI FA CTR IPC CN PRC4) <XingTong.Wu@siemens.com> Subject: Re: [PATCH > v2 1/1] gpio-f7188x: fix chip name and pin count on Nuvoton chip > > On Thu, Apr 27, 2023 at 05:20:55PM +0200, Henning Schild wrote: > > In fact the device with chip id 0xD283 is called NCT6126D, and that > > is the chip id the Nuvoton code was written for. Correct that name > > to avoid confusion, because a NCT6116D in fact exists as well but > > has another chip id, and is currently not supported. > > > > The look at the spec also revealed that GPIO group7 in fact has 8 > > pins, so correct the pin count in that group as well. > > > > Fixes: d0918a84aff0 ("gpio-f7188x: Add GPIO support for Nuvoton > > NCT6116") > > Reported-by: Xing Tong Wu <xingtong.wu@siemens.com> > > Signed-off-by: Henning Schild <henning.schild@siemens.com> > > Acked-by: Simon Guinot <simon.guinot@sequanux.org> > > > --- > > drivers/gpio/Kconfig | 2 +- > > drivers/gpio/gpio-f7188x.c | 28 ++++++++++++++-------------- > > 2 files changed, 15 insertions(+), 15 deletions(-) > > > > diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig index > > badbe0582318..14b655411aa0 100644 > > --- a/drivers/gpio/Kconfig > > +++ b/drivers/gpio/Kconfig > > @@ -879,7 +879,7 @@ config GPIO_F7188X > > help > > This option enables support for GPIOs found on Fintek Super-I/O > > chips F71869, F71869A, F71882FG, F71889F and F81866. > > - As well as Nuvoton Super-I/O chip NCT6116D. > > + As well as Nuvoton Super-I/O chip NCT6126D. > > > > To compile this driver as a module, choose M here: the module > > will be called f7188x-gpio. > > diff --git a/drivers/gpio/gpio-f7188x.c b/drivers/gpio/gpio-f7188x.c > > index 9effa7769bef..f54ca5a1775e 100644 > > --- a/drivers/gpio/gpio-f7188x.c > > +++ b/drivers/gpio/gpio-f7188x.c > > @@ -48,7 +48,7 @@ > > /* > > * Nuvoton devices. > > */ > > -#define SIO_NCT6116D_ID0xD283 /* NCT6116D chipset ID */ > > +#define SIO_NCT6126D_ID0xD283 /* NCT6126D chipset ID */ > > > > #define SIO_LD_GPIO_NUVOTON0x07/* GPIO logical device */ > > > > @@ -62,7 +62,7 @@ enum chips { > > f81866, > > f81804, > > f81865, > > -nct6116d, > > +nct6126d, > > }; > > > > static const char * const f7188x_names[] = { @@ -74,7 +74,7 @@ > > static const char * const f7188x_names[] = { > > "f81866", > > "f81804", > > "f81865", > > -"nct6116d", > > +"nct6126d", > > }; > > > > struct f7188x_sio { > > @@ -187,8 +187,8 @@ static int f7188x_gpio_set_config(struct > > gpio_chip *chip, unsigned offset, > > /* Output mode register (0:open drain 1:push-pull). */ #define > > f7188x_gpio_out_mode(base) ((base) + 3) > > > > -#define f7188x_gpio_dir_invert(type)((type) == nct6116d) > > -#define f7188x_gpio_data_single(type)((type) == nct6116d) > > +#define f7188x_gpio_dir_invert(type)((type) == nct6126d) > > +#define f7188x_gpio_data_single(type)((type) == nct6126d) > > > > static struct f7188x_gpio_bank f71869_gpio_bank[] = { > > F7188X_GPIO_BANK(0, 6, 0xF0, DRVNAME "-0"), @@ -274,7 +274,7 @@ > > static struct f7188x_gpio_bank f81865_gpio_bank[] = { > > F7188X_GPIO_BANK(60, 5, 0x90, DRVNAME "-6"), }; > > > > -static struct f7188x_gpio_bank nct6116d_gpio_bank[] = { > > +static struct f7188x_gpio_bank nct6126d_gpio_bank[] = { > > F7188X_GPIO_BANK(0, 8, 0xE0, DRVNAME "-0"), > > F7188X_GPIO_BANK(10, 8, 0xE4, DRVNAME "-1"), > > F7188X_GPIO_BANK(20, 8, 0xE8, DRVNAME "-2"), @@ -282,7 +282,7 @@ > > static struct f7188x_gpio_bank nct6116d_gpio_bank[] = { > > F7188X_GPIO_BANK(40, 8, 0xF0, DRVNAME "-4"), > > F7188X_GPIO_BANK(50, 8, 0xF4, DRVNAME "-5"), > > F7188X_GPIO_BANK(60, 8, 0xF8, DRVNAME "-6"), > > -F7188X_GPIO_BANK(70, 1, 0xFC, DRVNAME "-7"), > > +F7188X_GPIO_BANK(70, 8, 0xFC, DRVNAME "-7"), > > }; > > > > static int f7188x_gpio_get_direction(struct gpio_chip *chip, > > unsigned offset) @@ -490,9 +490,9 @@ static int > > f7188x_gpio_probe(struct platform_device *pdev) data->nr_bank = > > ARRAY_SIZE(f81865_gpio_bank); data->bank = f81865_gpio_bank; > > break; > > -case nct6116d: > > -data->nr_bank = ARRAY_SIZE(nct6116d_gpio_bank); > > -data->bank = nct6116d_gpio_bank; > > +case nct6126d: > > +data->nr_bank = ARRAY_SIZE(nct6126d_gpio_bank); > > +data->bank = nct6126d_gpio_bank; > > break; > > default: > > return -ENODEV; > > @@ -559,9 +559,9 @@ static int __init f7188x_find(int addr, struct > > f7188x_sio *sio) case SIO_F81865_ID: > > sio->type = f81865; > > break; > > -case SIO_NCT6116D_ID: > > +case SIO_NCT6126D_ID: > > sio->device = SIO_LD_GPIO_NUVOTON; > > -sio->type = nct6116d; > > +sio->type = nct6126d; > > break; > > default: > > pr_info("Unsupported Fintek device 0x%04x\n", devid); @@ -569,7 > > +569,7 @@ static int __init f7188x_find(int addr, struct f7188x_sio > > *sio) } > > > > /* double check manufacturer where possible */ > > -if (sio->type != nct6116d) { > > +if (sio->type != nct6126d) { > > manid = superio_inw(addr, SIO_FINTEK_MANID); > > if (manid != SIO_FINTEK_ID) { > > pr_debug("Not a Fintek device at 0x%08x\n", addr); @@ -581,7 > > +581,7 @@ static int __init f7188x_find(int addr, struct f7188x_sio > > *sio) err = 0; > > > > pr_info("Found %s at %#x\n", f7188x_names[sio->type], (unsigned > > int)addr); -if (sio->type != nct6116d) > > +if (sio->type != nct6126d) > > pr_info(" revision %d\n", superio_inb(addr, SIO_FINTEK_DEVREV)); > > > > err: > > -- > > 2.39.2
On Fri, Apr 28, 2023 at 11:33 AM Henning Schild <henning.schild@siemens.com> wrote: > > Am Fri, 28 Apr 2023 10:26:18 +0200 > schrieb "Wu, Xing Tong (DI FA CTR IPC CN PRC4)" > <XingTong.Wu@siemens.com>: > > > Hi all > > > > The chip id of NCT6116D is 0XD281, you can refer to > > NCT6116D_Datasheet_V1_0.pdf, Page 291 > > Thanks Xing Tong. I think we have come to agree that for now NCT6116D > 0xD281 will not be supported in the kernel. Maybe until someone has > access to that very chip and a use-case. > > But you managed to somehow get these datasheets, which are still not > publicly available. Maybe you can use your contacts at Nuvoton to > kindly ask them to publish those specs on their website for future > reference. Some specs are there, but not all. That would help people to > add more chips and avoid mistakes like they happened to me. > > Henning > Henning, do you plan to respin this with the ID corrected? Bart
Am Thu, 11 May 2023 14:52:17 +0200 schrieb Bartosz Golaszewski <brgl@bgdev.pl>: > On Fri, Apr 28, 2023 at 11:33 AM Henning Schild > <henning.schild@siemens.com> wrote: > > > > Am Fri, 28 Apr 2023 10:26:18 +0200 > > schrieb "Wu, Xing Tong (DI FA CTR IPC CN PRC4)" > > <XingTong.Wu@siemens.com>: > > > > > Hi all > > > > > > The chip id of NCT6116D is 0XD281, you can refer to > > > NCT6116D_Datasheet_V1_0.pdf, Page 291 > > > > Thanks Xing Tong. I think we have come to agree that for now > > NCT6116D 0xD281 will not be supported in the kernel. Maybe until > > someone has access to that very chip and a use-case. > > > > But you managed to somehow get these datasheets, which are still not > > publicly available. Maybe you can use your contacts at Nuvoton to > > kindly ask them to publish those specs on their website for future > > reference. Some specs are there, but not all. That would help > > people to add more chips and avoid mistakes like they happened to > > me. > > > > Henning > > > > Henning, do you plan to respin this with the ID corrected? Bart, no this one fixes the name of the chip i have at hand, and fixes the size of its last bank. So it is valid on its own and should IMHO be merged as is. Since i do not have a NCT6116D (0xD281) i could basically just guess (not too hard) but not test. And i do not really feel like contributing untested code for which there is no known user/tester. Henning > Bart
On Sat, May 13, 2023 at 11:53 AM Henning Schild <henning.schild@siemens.com> wrote: > > Am Thu, 11 May 2023 14:52:17 +0200 > schrieb Bartosz Golaszewski <brgl@bgdev.pl>: > > > On Fri, Apr 28, 2023 at 11:33 AM Henning Schild > > <henning.schild@siemens.com> wrote: > > > > > > Am Fri, 28 Apr 2023 10:26:18 +0200 > > > schrieb "Wu, Xing Tong (DI FA CTR IPC CN PRC4)" > > > <XingTong.Wu@siemens.com>: > > > > > > > Hi all > > > > > > > > The chip id of NCT6116D is 0XD281, you can refer to > > > > NCT6116D_Datasheet_V1_0.pdf, Page 291 > > > > > > Thanks Xing Tong. I think we have come to agree that for now > > > NCT6116D 0xD281 will not be supported in the kernel. Maybe until > > > someone has access to that very chip and a use-case. > > > > > > But you managed to somehow get these datasheets, which are still not > > > publicly available. Maybe you can use your contacts at Nuvoton to > > > kindly ask them to publish those specs on their website for future > > > reference. Some specs are there, but not all. That would help > > > people to add more chips and avoid mistakes like they happened to > > > me. > > > > > > Henning > > > > > > > Henning, do you plan to respin this with the ID corrected? > > Bart, no this one fixes the name of the chip i have at hand, and fixes > the size of its last bank. So it is valid on its own and should IMHO be > merged as is. > > Since i do not have a NCT6116D (0xD281) i could basically just guess > (not too hard) but not test. And i do not really feel like contributing > untested code for which there is no known user/tester. > > Henning > > > Bart > Fair enough, applied. Bart
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig index badbe0582318..14b655411aa0 100644 --- a/drivers/gpio/Kconfig +++ b/drivers/gpio/Kconfig @@ -879,7 +879,7 @@ config GPIO_F7188X help This option enables support for GPIOs found on Fintek Super-I/O chips F71869, F71869A, F71882FG, F71889F and F81866. - As well as Nuvoton Super-I/O chip NCT6116D. + As well as Nuvoton Super-I/O chip NCT6126D. To compile this driver as a module, choose M here: the module will be called f7188x-gpio. diff --git a/drivers/gpio/gpio-f7188x.c b/drivers/gpio/gpio-f7188x.c index 9effa7769bef..f54ca5a1775e 100644 --- a/drivers/gpio/gpio-f7188x.c +++ b/drivers/gpio/gpio-f7188x.c @@ -48,7 +48,7 @@ /* * Nuvoton devices. */ -#define SIO_NCT6116D_ID 0xD283 /* NCT6116D chipset ID */ +#define SIO_NCT6126D_ID 0xD283 /* NCT6126D chipset ID */ #define SIO_LD_GPIO_NUVOTON 0x07 /* GPIO logical device */ @@ -62,7 +62,7 @@ enum chips { f81866, f81804, f81865, - nct6116d, + nct6126d, }; static const char * const f7188x_names[] = { @@ -74,7 +74,7 @@ static const char * const f7188x_names[] = { "f81866", "f81804", "f81865", - "nct6116d", + "nct6126d", }; struct f7188x_sio { @@ -187,8 +187,8 @@ static int f7188x_gpio_set_config(struct gpio_chip *chip, unsigned offset, /* Output mode register (0:open drain 1:push-pull). */ #define f7188x_gpio_out_mode(base) ((base) + 3) -#define f7188x_gpio_dir_invert(type) ((type) == nct6116d) -#define f7188x_gpio_data_single(type) ((type) == nct6116d) +#define f7188x_gpio_dir_invert(type) ((type) == nct6126d) +#define f7188x_gpio_data_single(type) ((type) == nct6126d) static struct f7188x_gpio_bank f71869_gpio_bank[] = { F7188X_GPIO_BANK(0, 6, 0xF0, DRVNAME "-0"), @@ -274,7 +274,7 @@ static struct f7188x_gpio_bank f81865_gpio_bank[] = { F7188X_GPIO_BANK(60, 5, 0x90, DRVNAME "-6"), }; -static struct f7188x_gpio_bank nct6116d_gpio_bank[] = { +static struct f7188x_gpio_bank nct6126d_gpio_bank[] = { F7188X_GPIO_BANK(0, 8, 0xE0, DRVNAME "-0"), F7188X_GPIO_BANK(10, 8, 0xE4, DRVNAME "-1"), F7188X_GPIO_BANK(20, 8, 0xE8, DRVNAME "-2"), @@ -282,7 +282,7 @@ static struct f7188x_gpio_bank nct6116d_gpio_bank[] = { F7188X_GPIO_BANK(40, 8, 0xF0, DRVNAME "-4"), F7188X_GPIO_BANK(50, 8, 0xF4, DRVNAME "-5"), F7188X_GPIO_BANK(60, 8, 0xF8, DRVNAME "-6"), - F7188X_GPIO_BANK(70, 1, 0xFC, DRVNAME "-7"), + F7188X_GPIO_BANK(70, 8, 0xFC, DRVNAME "-7"), }; static int f7188x_gpio_get_direction(struct gpio_chip *chip, unsigned offset) @@ -490,9 +490,9 @@ static int f7188x_gpio_probe(struct platform_device *pdev) data->nr_bank = ARRAY_SIZE(f81865_gpio_bank); data->bank = f81865_gpio_bank; break; - case nct6116d: - data->nr_bank = ARRAY_SIZE(nct6116d_gpio_bank); - data->bank = nct6116d_gpio_bank; + case nct6126d: + data->nr_bank = ARRAY_SIZE(nct6126d_gpio_bank); + data->bank = nct6126d_gpio_bank; break; default: return -ENODEV; @@ -559,9 +559,9 @@ static int __init f7188x_find(int addr, struct f7188x_sio *sio) case SIO_F81865_ID: sio->type = f81865; break; - case SIO_NCT6116D_ID: + case SIO_NCT6126D_ID: sio->device = SIO_LD_GPIO_NUVOTON; - sio->type = nct6116d; + sio->type = nct6126d; break; default: pr_info("Unsupported Fintek device 0x%04x\n", devid); @@ -569,7 +569,7 @@ static int __init f7188x_find(int addr, struct f7188x_sio *sio) } /* double check manufacturer where possible */ - if (sio->type != nct6116d) { + if (sio->type != nct6126d) { manid = superio_inw(addr, SIO_FINTEK_MANID); if (manid != SIO_FINTEK_ID) { pr_debug("Not a Fintek device at 0x%08x\n", addr); @@ -581,7 +581,7 @@ static int __init f7188x_find(int addr, struct f7188x_sio *sio) err = 0; pr_info("Found %s at %#x\n", f7188x_names[sio->type], (unsigned int)addr); - if (sio->type != nct6116d) + if (sio->type != nct6126d) pr_info(" revision %d\n", superio_inb(addr, SIO_FINTEK_DEVREV)); err:
In fact the device with chip id 0xD283 is called NCT6126D, and that is the chip id the Nuvoton code was written for. Correct that name to avoid confusion, because a NCT6116D in fact exists as well but has another chip id, and is currently not supported. The look at the spec also revealed that GPIO group7 in fact has 8 pins, so correct the pin count in that group as well. Fixes: d0918a84aff0 ("gpio-f7188x: Add GPIO support for Nuvoton NCT6116") Reported-by: Xing Tong Wu <xingtong.wu@siemens.com> Signed-off-by: Henning Schild <henning.schild@siemens.com> --- drivers/gpio/Kconfig | 2 +- drivers/gpio/gpio-f7188x.c | 28 ++++++++++++++-------------- 2 files changed, 15 insertions(+), 15 deletions(-)