[PATCH v2 0/1] fix pin count in gpio bank and chip name

[PATCH v2 0/1] fix pin count in gpio bank and chip name

[Henning Schild]

change to v1:
 - rename the chip in the code

When first writing that code i looked at a spec that was only describing
a similar chip. Now i have access to the specs and found that i got the
name of the chip for my given chip id wrong, and also the pin count on
the 7th bank.

I added a "Fixes" in the hope that this can be backported as a bugfix.

Henning Schild (1):
  gpio-f7188x: fix chip name and pin count on Nuvoton chip

 drivers/gpio/Kconfig       |  2 +-
 drivers/gpio/gpio-f7188x.c | 28 ++++++++++++++--------------
 2 files changed, 15 insertions(+), 15 deletions(-)

-- 
2.39.2

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

[Henning Schild]

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(-)

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

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

[Simon Guinot]

[-- Attachment #1: Type: text/plain, Size: 5206 bytes --]

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

[Wu, Xing Tong]

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

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

[Henning Schild]

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  

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

[Bartosz Golaszewski]

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

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

[Henning Schild]

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

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

[Bartosz Golaszewski]

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