Re: [PATCH v2 1/1] gpio-f7188x: fix chip name and pin count on Nuvoton chip

From: Henning Schild
Date: Fri Apr 28 2023 - 05:33:53 EST


Am Fri, 28 Apr 2023 10:26:18 +0200
schrieb "Wu, Xing Tong (DI FA CTR IPC CN PRC4)"
<XingTong.Wu@xxxxxxxxxxx>:

> 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@xxxxxxxxxxxx>
> Sent: Friday, April 28, 2023 4:18 PM
> To: Schild, Henning (T CED SES-DE) <henning.schild@xxxxxxxxxxx>
> Cc: Linus Walleij <linus.walleij@xxxxxxxxxx>; Bartosz Golaszewski
> <brgl@xxxxxxxx>; linux-gpio@xxxxxxxxxxxxxxx;
> linux-kernel@xxxxxxxxxxxxxxx; Hans de Goede <hdegoede@xxxxxxxxxx>;
> Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>; Wu, Xing Tong
> (DI FA CTR IPC CN PRC4) <XingTong.Wu@xxxxxxxxxxx> 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@xxxxxxxxxxx>
> > Signed-off-by: Henning Schild <henning.schild@xxxxxxxxxxx>
>
> Acked-by: Simon Guinot <simon.guinot@xxxxxxxxxxxx>
>
> > ---
> > 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