Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Tue, 9 Aug 2022 16:57:55 +0200
From:   Henning Schild <henning.schild@siemens.com>
To:     <linux-kernel@vger.kernel.org>, <linux-gpio@vger.kernel.org>,
        "Bartosz Golaszewski" <brgl@bgdev.pl>,
        Linus Walleij <linus.walleij@linaro.org>,
        Tasanakorn Phaipool <tasanakorn@gmail.com>
Cc:     Sheng-Yuan Huang <syhuang3@nuvoton.com>,
        Kuan-Wei Ho <cwho@nuvoton.com>,
        Andy Shevchenko <andy.shevchenko@gmail.com>
Subject: Re: [PATCH 1/4] gpio-f7188x: Add GPIO support for Nuvoton NCT6116
Message-ID: <20220809165755.252f34d0@md1za8fc.ad001.siemens.net>
In-Reply-To: <20220728155652.29516-2-henning.schild@siemens.com>
References: <20220728155652.29516-1-henning.schild@siemens.com>
        <20220728155652.29516-2-henning.schild@siemens.com>
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?O9p0qguYE6ZLQv5zLjivDJEB1nFKLd25MNjg6uRtJU73D1mCwrI/62rQWoq8?=
 =?us-ascii?Q?0WH2YR8+mfUrXeXWpuPlOq/IhjWH3Sc6fzfqbUjSQvufymBdNF1tCdNJH4oc?=
 =?us-ascii?Q?tiWhVZVNZVLGyi5qm4EdB2q769BB1osNoqHftB+KCoiDO5fZR4Dlx/83Y34p?=
 =?us-ascii?Q?LtJ/ctZjdQgFEkUhDOEfdiYKeBjdMxOJQzgFu7nHDcHsL/SYQadT4HnDGoe6?=
 =?us-ascii?Q?Bqncm4ZDP5EtoI2WRQSaX0H1Bg2mLgjZG0TSBPB36Bnl2Lu/a53NBo0d/HDY?=
 =?us-ascii?Q?/SVPTNtYAlIb7LPJ6s6uejqED7WlwNVgl7/2H0edtXVLqN+91OExBJIiYRFp?=
 =?us-ascii?Q?EmcwssuAmCgNfci26LZ7wmMMDKH7h/PsuHxmHigAZs8suNhywH+05NbSTw3U?=
 =?us-ascii?Q?Yg3Jt7r9sDquGrR/R4xqhsV2Xnq5sGqrD22iDWiBR/YW1C9jHqhztbW2OxOz?=
 =?us-ascii?Q?eGflCFrSK1HyacLYm6L8SZoYy4vXHl98KgLCo8tbfkaO6xPWiV1Hq9fkLjIf?=
 =?us-ascii?Q?Bc8efgV/MYY1NkWVAfWlM97l9CrBrQLNLHLDOSe8jztNxWBIxSaLcx8gySm8?=
 =?us-ascii?Q?OMWV/sMmHkBzPDb4RfQpnu0KCtqkVzcQjo6GuIxkOWMHQB+goKjc8wA3zPNu?=
 =?us-ascii?Q?uJZAbYrxHpqb6TdYK8Xr8E8rI75srz1M7yqlsdycYcdVtlnU9B5tE9J3kNLq?=
 =?us-ascii?Q?902SeX7Yvv00k3ANPJazz+MiBltNFKC2qOxWgaYhVwqILPj2m/TpEvNFE0RJ?=
 =?us-ascii?Q?5d39AY2pLyUvN+SVcMAS/dpLglK7Zl1n/yZuHEczIOb9XIo9v/VDZjcVGR02?=
 =?us-ascii?Q?ARzykkLR5VpEnMbZWHB2YVIWYQhinlfuZtD6QZ6Q9FMjXn7f3d5g6nFaIViG?=
 =?us-ascii?Q?Nb9hBj7JE07elJUg/I+poeqwjCtBKCkGgtTuQxy/2Zbvb3nXm7hh0MSlmwGi?=
 =?us-ascii?Q?g6+/iwp0Z1znk9LODrDvqS3mQSdpCqzIrCFRIkfNIipkV2APjuR9zzIVzllX?=
 =?us-ascii?Q?a/oe9CYAjozSjTyUhqcFtBu3qRPtJB/P4/3XVqpP6JeijIdHZiTXhfgEgab8?=
 =?us-ascii?Q?BGq0wObnk6HLlMoXTyZJKPNkipidI+sqhn/wZy45PSbVpGQAhSK7L/YzgHvg?=
 =?us-ascii?Q?K8x4Vguu9qGHb/9sMhNmvDp+EnLu9VKbFJDa6RhhvtejA54LyoBsYOQzacHr?=
 =?us-ascii?Q?iezsKFdZrfDkLfBMWhSvx2VuOyU9FIb/PAx+HcXH5fIX8tp3Blo4LFQe3SUE?=
 =?us-ascii?Q?GpWiauHqjoruLZ+bTClWLMfL6t0jOiR2fNhzk9tBrkE10W7WNoXx/3kXJvyo?=
 =?us-ascii?Q?yoczGNL0kqWOe37VI0HIzrnYiu6jyulXH5YAD2SERMoqn5SnxXZtrZzZt06w?=
 =?us-ascii?Q?hpVT3EB0TqRJnwEnZluyEjNwr8Jqq0jd0BoYVkQvSfWY4pcKpwjKq4oWU5Du?=
 =?us-ascii?Q?azqpXpugZKTbPDYxYz1a9rg4X7KMPfC+iS/Bid4m8jt9dqA8j9V0dMgkq6Xl?=
 =?us-ascii?Q?YgBG8iwVAy3LMKEOyyvQv8cRLyzXE90lhZMJPZiUoPlvDtH1re92x0yLuxSu?=
 =?us-ascii?Q?o7W526afD56BYAVQqIUszGoc3FzOm0TyURq6kWX4whrk8uzJpE/qKrb/G9nA?=
 =?us-ascii?Q?hg=3D=3D?=
X-OriginatorOrg: siemens.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 97237726-bafb-4e86-f019-08da7a178ee8
X-MS-Exchange-CrossTenant-AuthSource: PA4PR10MB5780.EURPRD10.PROD.OUTLOOK.COM
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 09 Aug 2022 14:58:02.8844
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 38ae3bcd-9579-4fd4-adda-b42e1495d55a
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: 42MqGZWSKaoaz+xNXyQMoxbVDv5rrTim5jCHue9yR1dOzWu4NP+7r6R/dIctegaEPkoDMQjruy2JQkBuDP1w6gpR7n5JqcNz6p31bMnH6Dg=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DB4PR10MB6166
X-Spam-Status: No, score=-1.1 required=5.0 tests=BAYES_00,DKIMWL_WL_MED,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FORGED_SPF_HELO,
        RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_NONE,
        T_SCC_BODY_TEXT_LINE autolearn=no autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Am Thu, 28 Jul 2022 17:56:49 +0200
schrieb Henning Schild <henning.schild@siemens.com>:

> Add GPIO support for Nuvoton NCT6116 chip. Nuvoton SuperIO chips are
> very similar to the ones from Fintek. In other subsystems they also
> share drivers and are called a family of drivers.
> 
> For the GPIO subsystem the only difference is that the direction bit
> is reversed and that there is only one data bit per pin. 

In fact the modification of f7188x_gpio_get is missing in this patch,
the function needs to be modified for this chip variant, where the
given bit has another meaning. (value invert, not used in the driver)

Will send a fixed version

Henning

> On the
> SuperIO level the logical device is another one.
> 
> Signed-off-by: Henning Schild <henning.schild@siemens.com>
> ---
>  drivers/gpio/gpio-f7188x.c | 70
> ++++++++++++++++++++++++++++---------- 1 file changed, 52
> insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/gpio/gpio-f7188x.c b/drivers/gpio/gpio-f7188x.c
> index 18a3147f5a42..431ce2cda1d8 100644
> --- a/drivers/gpio/gpio-f7188x.c
> +++ b/drivers/gpio/gpio-f7188x.c
> @@ -1,6 +1,7 @@
>  // SPDX-License-Identifier: GPL-2.0-or-later
>  /*
>   * GPIO driver for Fintek Super-I/O F71869, F71869A, F71882, F71889
> and F81866
> + * and Nuvoton Super-I/O NCT6116D
>   *
>   * Copyright (C) 2010-2013 LaCie
>   *
> @@ -22,13 +23,11 @@
>  #define SIO_LDSEL		0x07	/* Logical device
> select */ #define SIO_DEVID		0x20	/* Device ID
> (2 bytes) */ #define SIO_DEVREV		0x22	/* Device
> revision */ -#define SIO_MANID		0x23	/* Fintek
> ID (2 bytes) */ 
> -#define SIO_LD_GPIO		0x06	/* GPIO logical
> device */ #define SIO_UNLOCK_KEY		0x87	/* Key to
> enable Super-I/O */ #define SIO_LOCK_KEY		0xAA
> /* Key to disable Super-I/O */ 
> -#define SIO_FINTEK_ID		0x1934	/* Manufacturer
> ID */ +#define SIO_LD_GPIO_FINTEK	0x06	/* GPIO logical
> device */ #define SIO_F71869_ID		0x0814	/*
> F71869 chipset ID */ #define SIO_F71869A_ID
> 0x1007	/* F71869A chipset ID */ #define SIO_F71882_ID
> 	0x0541	/* F71882 chipset ID */ @@ -38,6 +37,9 @@
>  #define SIO_F81804_ID		0x1502  /* F81804 chipset ID,
> same for f81966 */ #define SIO_F81865_ID		0x0704
> /* F81865 chipset ID */ 
> +#define SIO_LD_GPIO_NUVOTON	0x07	/* GPIO logical
> device */ +#define SIO_NCT6116D_ID		0xD283  /* NCT6116D
> chipset ID */ +#define SIO_GPIO_ENABLE		0x30	/*
> GPIO enable */ 
>  enum chips {
>  	f71869,
> @@ -48,6 +50,7 @@ enum chips {
>  	f81866,
>  	f81804,
>  	f81865,
> +	nct6116d,
>  };
>  
>  static const char * const f7188x_names[] = {
> @@ -59,10 +62,12 @@ static const char * const f7188x_names[] = {
>  	"f81866",
>  	"f81804",
>  	"f81865",
> +	"nct6116d",
>  };
>  
>  struct f7188x_sio {
>  	int addr;
> +	int device;
>  	enum chips type;
>  };
>  
> @@ -254,6 +259,17 @@ static struct f7188x_gpio_bank
> f81865_gpio_bank[] = { F7188X_GPIO_BANK(60, 5, 0x90),
>  };
>  
> +static struct f7188x_gpio_bank nct6116d_gpio_bank[] = {
> +	F7188X_GPIO_BANK(0, 8, 0xE0),
> +	F7188X_GPIO_BANK(10, 8, 0xE4),
> +	F7188X_GPIO_BANK(20, 8, 0xE8),
> +	F7188X_GPIO_BANK(30, 8, 0xEC),
> +	F7188X_GPIO_BANK(40, 8, 0xF0),
> +	F7188X_GPIO_BANK(50, 8, 0xF4),
> +	F7188X_GPIO_BANK(60, 8, 0xF8),
> +	F7188X_GPIO_BANK(70, 1, 0xFC),
> +};
> +
>  static int f7188x_gpio_get_direction(struct gpio_chip *chip,
> unsigned offset) {
>  	int err;
> @@ -264,13 +280,20 @@ static int f7188x_gpio_get_direction(struct
> gpio_chip *chip, unsigned offset) err = superio_enter(sio->addr);
>  	if (err)
>  		return err;
> -	superio_select(sio->addr, SIO_LD_GPIO);
> +	superio_select(sio->addr, sio->device);
>  
>  	dir = superio_inb(sio->addr, gpio_dir(bank->regbase));
>  
>  	superio_exit(sio->addr);
>  
> -	if (dir & 1 << offset)
> +	if (sio->device == SIO_LD_GPIO_NUVOTON) {
> +		if (dir & BIT(offset))
> +			return GPIO_LINE_DIRECTION_IN;
> +
> +		return GPIO_LINE_DIRECTION_OUT;
> +	}
> +
> +	if (dir & BIT(offset))
>  		return GPIO_LINE_DIRECTION_OUT;
>  
>  	return GPIO_LINE_DIRECTION_IN;
> @@ -286,10 +309,14 @@ static int f7188x_gpio_direction_in(struct
> gpio_chip *chip, unsigned offset) err = superio_enter(sio->addr);
>  	if (err)
>  		return err;
> -	superio_select(sio->addr, SIO_LD_GPIO);
> +	superio_select(sio->addr, sio->device);
>  
>  	dir = superio_inb(sio->addr, gpio_dir(bank->regbase));
> -	dir &= ~BIT(offset);
> +
> +	if (sio->device == SIO_LD_GPIO_FINTEK)
> +		dir &= ~BIT(offset);
> +	else
> +		dir |= BIT(offset);
>  	superio_outb(sio->addr, gpio_dir(bank->regbase), dir);
>  
>  	superio_exit(sio->addr);
> @@ -307,7 +334,7 @@ static int f7188x_gpio_get(struct gpio_chip
> *chip, unsigned offset) err = superio_enter(sio->addr);
>  	if (err)
>  		return err;
> -	superio_select(sio->addr, SIO_LD_GPIO);
> +	superio_select(sio->addr, sio->device);
>  
>  	dir = superio_inb(sio->addr, gpio_dir(bank->regbase));
>  	dir = !!(dir & BIT(offset));
> @@ -332,7 +359,7 @@ static int f7188x_gpio_direction_out(struct
> gpio_chip *chip, err = superio_enter(sio->addr);
>  	if (err)
>  		return err;
> -	superio_select(sio->addr, SIO_LD_GPIO);
> +	superio_select(sio->addr, sio->device);
>  
>  	data_out = superio_inb(sio->addr,
> gpio_data_out(bank->regbase)); if (value)
> @@ -342,7 +369,10 @@ static int f7188x_gpio_direction_out(struct
> gpio_chip *chip, superio_outb(sio->addr,
> gpio_data_out(bank->regbase), data_out); 
>  	dir = superio_inb(sio->addr, gpio_dir(bank->regbase));
> -	dir |= BIT(offset);
> +	if (sio->device == SIO_LD_GPIO_FINTEK)
> +		dir |= BIT(offset);
> +	else
> +		dir &= ~BIT(offset);
>  	superio_outb(sio->addr, gpio_dir(bank->regbase), dir);
>  
>  	superio_exit(sio->addr);
> @@ -360,7 +390,7 @@ static void f7188x_gpio_set(struct gpio_chip
> *chip, unsigned offset, int value) err = superio_enter(sio->addr);
>  	if (err)
>  		return;
> -	superio_select(sio->addr, SIO_LD_GPIO);
> +	superio_select(sio->addr, sio->device);
>  
>  	data_out = superio_inb(sio->addr,
> gpio_data_out(bank->regbase)); if (value)
> @@ -388,7 +418,7 @@ static int f7188x_gpio_set_config(struct
> gpio_chip *chip, unsigned offset, err = superio_enter(sio->addr);
>  	if (err)
>  		return err;
> -	superio_select(sio->addr, SIO_LD_GPIO);
> +	superio_select(sio->addr, sio->device);
>  
>  	data = superio_inb(sio->addr, gpio_out_mode(bank->regbase));
>  	if (param == PIN_CONFIG_DRIVE_OPEN_DRAIN)
> @@ -449,6 +479,10 @@ 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;
> +		break;
>  	default:
>  		return -ENODEV;
>  	}
> @@ -485,12 +519,8 @@ static int __init f7188x_find(int addr, struct
> f7188x_sio *sio) return err;
>  
>  	err = -ENODEV;
> -	devid = superio_inw(addr, SIO_MANID);
> -	if (devid != SIO_FINTEK_ID) {
> -		pr_debug(DRVNAME ": Not a Fintek device at
> 0x%08x\n", addr);
> -		goto err;
> -	}
>  
> +	sio->device = SIO_LD_GPIO_FINTEK;
>  	devid = superio_inw(addr, SIO_DEVID);
>  	switch (devid) {
>  	case SIO_F71869_ID:
> @@ -517,8 +547,12 @@ static int __init f7188x_find(int addr, struct
> f7188x_sio *sio) case SIO_F81865_ID:
>  		sio->type = f81865;
>  		break;
> +	case SIO_NCT6116D_ID:
> +		sio->device = SIO_LD_GPIO_NUVOTON;
> +		sio->type = nct6116d;
> +		break;
>  	default:
> -		pr_info(DRVNAME ": Unsupported Fintek device
> 0x%04x\n", devid);
> +		pr_info(DRVNAME ": Unsupported device 0x%04x\n",
> devid); goto err;
>  	}
>  	sio->addr = addr;