* [PATCH 0/4] add support for another simatic board @ 2022-07-28 15:56 Henning Schild 2022-07-28 15:56 ` [PATCH 1/4] gpio-f7188x: Add GPIO support for Nuvoton NCT6116 Henning Schild ` (5 more replies) 0 siblings, 6 replies; 13+ messages in thread From: Henning Schild @ 2022-07-28 15:56 UTC (permalink / raw) To: linux-kernel, linux-gpio, Bartosz Golaszewski, Linus Walleij, Tasanakorn Phaipool Cc: Sheng-Yuan Huang, Kuan-Wei Ho, Andy Shevchenko, Henning Schild This series first enables a SuperIO GPIO driver to support a chip from the vendor Nuvoton, the driver is for Fintek devices but those just are very similar. And in watchdog and hwmon subsystems these SuperIO drivers also share code and are sometimes called a family. In another step the individual banks receive a label to tell them apart, a step which potentially changes an interface to legacy users that might rely on all banks having the same label, or an exact label. But since a later patch wants to use GPIO_LOOKUP unique labels are needed and i decided to assign them for all supported chips. In a following patch the Simatic GPIO LED driver is extended to provide LEDs in case that SuperIO GPIO driver can be loaded. Last but not least the watchdog module of that same SuperIO gets loaded on a best effort basis. Note similar patches have appreared before as "[PATCH v3 0/1] add device driver for Nuvoton SIO gpio function" The main difference here is that i added chip support to an existing driver instead of creating a new one. And that i actually propose all patches and do not just have the LED patch for Simatic as an example. Also note that the patches are based on "[PATCH v6 00/12] platform/x86: introduce p2sb_bar() helper" Henning Schild (4): gpio-f7188x: Add GPIO support for Nuvoton NCT6116 gpio-f7188x: use unique labels for banks/chips leds: simatic-ipc-leds-gpio: add new model 227G platform/x86: simatic-ipc: enable watchdog for 227G drivers/gpio/gpio-f7188x.c | 192 +++++++++++------- drivers/leds/simple/simatic-ipc-leds-gpio.c | 42 +++- drivers/platform/x86/simatic-ipc.c | 7 +- .../platform_data/x86/simatic-ipc-base.h | 1 + include/linux/platform_data/x86/simatic-ipc.h | 1 + 5 files changed, 158 insertions(+), 85 deletions(-) -- 2.35.1 ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 1/4] gpio-f7188x: Add GPIO support for Nuvoton NCT6116 2022-07-28 15:56 [PATCH 0/4] add support for another simatic board Henning Schild @ 2022-07-28 15:56 ` Henning Schild 2022-07-28 21:33 ` Andy Shevchenko 2022-08-09 14:57 ` Henning Schild 2022-07-28 15:56 ` [PATCH 2/4] gpio-f7188x: use unique labels for banks/chips Henning Schild ` (4 subsequent siblings) 5 siblings, 2 replies; 13+ messages in thread From: Henning Schild @ 2022-07-28 15:56 UTC (permalink / raw) To: linux-kernel, linux-gpio, Bartosz Golaszewski, Linus Walleij, Tasanakorn Phaipool Cc: Sheng-Yuan Huang, Kuan-Wei Ho, Andy Shevchenko, Henning Schild 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. 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; -- 2.35.1 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 1/4] gpio-f7188x: Add GPIO support for Nuvoton NCT6116 2022-07-28 15:56 ` [PATCH 1/4] gpio-f7188x: Add GPIO support for Nuvoton NCT6116 Henning Schild @ 2022-07-28 21:33 ` Andy Shevchenko 2022-07-29 7:16 ` Henning Schild 2022-08-09 14:57 ` Henning Schild 1 sibling, 1 reply; 13+ messages in thread From: Andy Shevchenko @ 2022-07-28 21:33 UTC (permalink / raw) To: Henning Schild Cc: Linux Kernel Mailing List, open list:GPIO SUBSYSTEM, Bartosz Golaszewski, Linus Walleij, Tasanakorn Phaipool, Sheng-Yuan Huang, Kuan-Wei Ho On Thu, Jul 28, 2022 at 5:57 PM Henning Schild <henning.schild@siemens.com> wrote: > > 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. On the SuperIO > level the logical device is another one. ... > +#define SIO_GPIO_ENABLE 0x30 /* GPIO enable */ I don't see how it's being utilized... (But okay, it might be good to have as a hint for a reader who has no access to the documentation). ... > + if (sio->device == SIO_LD_GPIO_NUVOTON) { Everywhere else you use `device == SIO_LD_GPIO_FINTEK`, perhaps here for consistency? However, I would rather see a field that clearly states that it's an inverted value. Then you can use if (sio->dir_inv) ...do something... > + 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; -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/4] gpio-f7188x: Add GPIO support for Nuvoton NCT6116 2022-07-28 21:33 ` Andy Shevchenko @ 2022-07-29 7:16 ` Henning Schild 0 siblings, 0 replies; 13+ messages in thread From: Henning Schild @ 2022-07-29 7:16 UTC (permalink / raw) To: Andy Shevchenko Cc: Linux Kernel Mailing List, open list:GPIO SUBSYSTEM, Bartosz Golaszewski, Linus Walleij, Tasanakorn Phaipool, Sheng-Yuan Huang, Pavel Machek Am Thu, 28 Jul 2022 23:33:36 +0200 schrieb Andy Shevchenko <andy.shevchenko@gmail.com>: > On Thu, Jul 28, 2022 at 5:57 PM Henning Schild > <henning.schild@siemens.com> wrote: > > > > 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. On the > > SuperIO level the logical device is another one. > > ... > > > +#define SIO_GPIO_ENABLE 0x30 /* GPIO enable */ > > I don't see how it's being utilized... (But okay, it might be good to > have as a hint for a reader who has no access to the documentation). Good catch. That is a leftover from code that turned out to be not needed. Will drop. > ... > > > + if (sio->device == SIO_LD_GPIO_NUVOTON) { > > Everywhere else you use `device == SIO_LD_GPIO_FINTEK`, perhaps here > for consistency? However, I would rather see a field that clearly > states that it's an inverted value. Then you can use > > if (sio->dir_inv) > ...do something... Good idea, will look into that. Given we talk about a family of chips there might be more vendor ids that should be mapped onto "inv" in the future. Henning > > + 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; > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/4] gpio-f7188x: Add GPIO support for Nuvoton NCT6116 2022-07-28 15:56 ` [PATCH 1/4] gpio-f7188x: Add GPIO support for Nuvoton NCT6116 Henning Schild 2022-07-28 21:33 ` Andy Shevchenko @ 2022-08-09 14:57 ` Henning Schild 1 sibling, 0 replies; 13+ messages in thread From: Henning Schild @ 2022-08-09 14:57 UTC (permalink / raw) To: linux-kernel, linux-gpio, Bartosz Golaszewski, Linus Walleij, Tasanakorn Phaipool Cc: Sheng-Yuan Huang, Kuan-Wei Ho, Andy Shevchenko 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; ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 2/4] gpio-f7188x: use unique labels for banks/chips 2022-07-28 15:56 [PATCH 0/4] add support for another simatic board Henning Schild 2022-07-28 15:56 ` [PATCH 1/4] gpio-f7188x: Add GPIO support for Nuvoton NCT6116 Henning Schild @ 2022-07-28 15:56 ` Henning Schild 2022-08-22 7:17 ` Linus Walleij 2022-07-28 15:56 ` [PATCH 3/4] leds: simatic-ipc-leds-gpio: add new model 227G Henning Schild ` (3 subsequent siblings) 5 siblings, 1 reply; 13+ messages in thread From: Henning Schild @ 2022-07-28 15:56 UTC (permalink / raw) To: linux-kernel, linux-gpio, Bartosz Golaszewski, Linus Walleij, Tasanakorn Phaipool Cc: Sheng-Yuan Huang, Kuan-Wei Ho, Andy Shevchenko, Henning Schild So that drivers building on top can find those pins with GPIO_LOOKUP helpers. Signed-off-by: Henning Schild <henning.schild@siemens.com> --- drivers/gpio/gpio-f7188x.c | 138 ++++++++++++++++++------------------- 1 file changed, 69 insertions(+), 69 deletions(-) diff --git a/drivers/gpio/gpio-f7188x.c b/drivers/gpio/gpio-f7188x.c index 431ce2cda1d8..5f2f3c01231d 100644 --- a/drivers/gpio/gpio-f7188x.c +++ b/drivers/gpio/gpio-f7188x.c @@ -151,10 +151,10 @@ static void f7188x_gpio_set(struct gpio_chip *chip, unsigned offset, int value); static int f7188x_gpio_set_config(struct gpio_chip *chip, unsigned offset, unsigned long config); -#define F7188X_GPIO_BANK(_base, _ngpio, _regbase) \ +#define F7188X_GPIO_BANK(_base, _ngpio, _regbase, _label) \ { \ .chip = { \ - .label = DRVNAME, \ + .label = _label, \ .owner = THIS_MODULE, \ .get_direction = f7188x_gpio_get_direction, \ .direction_input = f7188x_gpio_direction_in, \ @@ -176,98 +176,98 @@ static int f7188x_gpio_set_config(struct gpio_chip *chip, unsigned offset, #define gpio_out_mode(base) (base + 3) static struct f7188x_gpio_bank f71869_gpio_bank[] = { - F7188X_GPIO_BANK(0, 6, 0xF0), - F7188X_GPIO_BANK(10, 8, 0xE0), - F7188X_GPIO_BANK(20, 8, 0xD0), - F7188X_GPIO_BANK(30, 8, 0xC0), - F7188X_GPIO_BANK(40, 8, 0xB0), - F7188X_GPIO_BANK(50, 5, 0xA0), - F7188X_GPIO_BANK(60, 6, 0x90), + F7188X_GPIO_BANK(0, 6, 0xF0, DRVNAME "-0"), + F7188X_GPIO_BANK(10, 8, 0xE0, DRVNAME "-1"), + F7188X_GPIO_BANK(20, 8, 0xD0, DRVNAME "-2"), + F7188X_GPIO_BANK(30, 8, 0xC0, DRVNAME "-3"), + F7188X_GPIO_BANK(40, 8, 0xB0, DRVNAME "-4"), + F7188X_GPIO_BANK(50, 5, 0xA0, DRVNAME "-5"), + F7188X_GPIO_BANK(60, 6, 0x90, DRVNAME "-6"), }; static struct f7188x_gpio_bank f71869a_gpio_bank[] = { - F7188X_GPIO_BANK(0, 6, 0xF0), - F7188X_GPIO_BANK(10, 8, 0xE0), - F7188X_GPIO_BANK(20, 8, 0xD0), - F7188X_GPIO_BANK(30, 8, 0xC0), - F7188X_GPIO_BANK(40, 8, 0xB0), - F7188X_GPIO_BANK(50, 5, 0xA0), - F7188X_GPIO_BANK(60, 8, 0x90), - F7188X_GPIO_BANK(70, 8, 0x80), + F7188X_GPIO_BANK(0, 6, 0xF0, DRVNAME "-0"), + F7188X_GPIO_BANK(10, 8, 0xE0, DRVNAME "-1"), + F7188X_GPIO_BANK(20, 8, 0xD0, DRVNAME "-2"), + F7188X_GPIO_BANK(30, 8, 0xC0, DRVNAME "-3"), + F7188X_GPIO_BANK(40, 8, 0xB0, DRVNAME "-4"), + F7188X_GPIO_BANK(50, 5, 0xA0, DRVNAME "-5"), + F7188X_GPIO_BANK(60, 8, 0x90, DRVNAME "-6"), + F7188X_GPIO_BANK(70, 8, 0x80, DRVNAME "-7"), }; static struct f7188x_gpio_bank f71882_gpio_bank[] = { - F7188X_GPIO_BANK(0, 8, 0xF0), - F7188X_GPIO_BANK(10, 8, 0xE0), - F7188X_GPIO_BANK(20, 8, 0xD0), - F7188X_GPIO_BANK(30, 4, 0xC0), - F7188X_GPIO_BANK(40, 4, 0xB0), + F7188X_GPIO_BANK(0, 8, 0xF0, DRVNAME "-0"), + F7188X_GPIO_BANK(10, 8, 0xE0, DRVNAME "-1"), + F7188X_GPIO_BANK(20, 8, 0xD0, DRVNAME "-2"), + F7188X_GPIO_BANK(30, 4, 0xC0, DRVNAME "-3"), + F7188X_GPIO_BANK(40, 4, 0xB0, DRVNAME "-4"), }; static struct f7188x_gpio_bank f71889a_gpio_bank[] = { - F7188X_GPIO_BANK(0, 7, 0xF0), - F7188X_GPIO_BANK(10, 7, 0xE0), - F7188X_GPIO_BANK(20, 8, 0xD0), - F7188X_GPIO_BANK(30, 8, 0xC0), - F7188X_GPIO_BANK(40, 8, 0xB0), - F7188X_GPIO_BANK(50, 5, 0xA0), - F7188X_GPIO_BANK(60, 8, 0x90), - F7188X_GPIO_BANK(70, 8, 0x80), + F7188X_GPIO_BANK(0, 7, 0xF0, DRVNAME "-0"), + F7188X_GPIO_BANK(10, 7, 0xE0, DRVNAME "-1"), + F7188X_GPIO_BANK(20, 8, 0xD0, DRVNAME "-2"), + F7188X_GPIO_BANK(30, 8, 0xC0, DRVNAME "-3"), + F7188X_GPIO_BANK(40, 8, 0xB0, DRVNAME "-4"), + F7188X_GPIO_BANK(50, 5, 0xA0, DRVNAME "-5"), + F7188X_GPIO_BANK(60, 8, 0x90, DRVNAME "-6"), + F7188X_GPIO_BANK(70, 8, 0x80, DRVNAME "-7"), }; static struct f7188x_gpio_bank f71889_gpio_bank[] = { - F7188X_GPIO_BANK(0, 7, 0xF0), - F7188X_GPIO_BANK(10, 7, 0xE0), - F7188X_GPIO_BANK(20, 8, 0xD0), - F7188X_GPIO_BANK(30, 8, 0xC0), - F7188X_GPIO_BANK(40, 8, 0xB0), - F7188X_GPIO_BANK(50, 5, 0xA0), - F7188X_GPIO_BANK(60, 8, 0x90), - F7188X_GPIO_BANK(70, 8, 0x80), + F7188X_GPIO_BANK(0, 7, 0xF0, DRVNAME "-0"), + F7188X_GPIO_BANK(10, 7, 0xE0, DRVNAME "-1"), + F7188X_GPIO_BANK(20, 8, 0xD0, DRVNAME "-2"), + F7188X_GPIO_BANK(30, 8, 0xC0, DRVNAME "-3"), + F7188X_GPIO_BANK(40, 8, 0xB0, DRVNAME "-4"), + F7188X_GPIO_BANK(50, 5, 0xA0, DRVNAME "-5"), + F7188X_GPIO_BANK(60, 8, 0x90, DRVNAME "-6"), + F7188X_GPIO_BANK(70, 8, 0x80, DRVNAME "-7"), }; static struct f7188x_gpio_bank f81866_gpio_bank[] = { - F7188X_GPIO_BANK(0, 8, 0xF0), - F7188X_GPIO_BANK(10, 8, 0xE0), - F7188X_GPIO_BANK(20, 8, 0xD0), - F7188X_GPIO_BANK(30, 8, 0xC0), - F7188X_GPIO_BANK(40, 8, 0xB0), - F7188X_GPIO_BANK(50, 8, 0xA0), - F7188X_GPIO_BANK(60, 8, 0x90), - F7188X_GPIO_BANK(70, 8, 0x80), - F7188X_GPIO_BANK(80, 8, 0x88), + F7188X_GPIO_BANK(0, 8, 0xF0, DRVNAME "-0"), + F7188X_GPIO_BANK(10, 8, 0xE0, DRVNAME "-1"), + F7188X_GPIO_BANK(20, 8, 0xD0, DRVNAME "-2"), + F7188X_GPIO_BANK(30, 8, 0xC0, DRVNAME "-3"), + F7188X_GPIO_BANK(40, 8, 0xB0, DRVNAME "-4"), + F7188X_GPIO_BANK(50, 8, 0xA0, DRVNAME "-5"), + F7188X_GPIO_BANK(60, 8, 0x90, DRVNAME "-6"), + F7188X_GPIO_BANK(70, 8, 0x80, DRVNAME "-7"), + F7188X_GPIO_BANK(80, 8, 0x88, DRVNAME "-8"), }; static struct f7188x_gpio_bank f81804_gpio_bank[] = { - F7188X_GPIO_BANK(0, 8, 0xF0), - F7188X_GPIO_BANK(10, 8, 0xE0), - F7188X_GPIO_BANK(20, 8, 0xD0), - F7188X_GPIO_BANK(50, 8, 0xA0), - F7188X_GPIO_BANK(60, 8, 0x90), - F7188X_GPIO_BANK(70, 8, 0x80), - F7188X_GPIO_BANK(90, 8, 0x98), + F7188X_GPIO_BANK(0, 8, 0xF0, DRVNAME "-0"), + F7188X_GPIO_BANK(10, 8, 0xE0, DRVNAME "-1"), + F7188X_GPIO_BANK(20, 8, 0xD0, DRVNAME "-2"), + F7188X_GPIO_BANK(50, 8, 0xA0, DRVNAME "-3"), + F7188X_GPIO_BANK(60, 8, 0x90, DRVNAME "-4"), + F7188X_GPIO_BANK(70, 8, 0x80, DRVNAME "-5"), + F7188X_GPIO_BANK(90, 8, 0x98, DRVNAME "-6"), }; static struct f7188x_gpio_bank f81865_gpio_bank[] = { - F7188X_GPIO_BANK(0, 8, 0xF0), - F7188X_GPIO_BANK(10, 8, 0xE0), - F7188X_GPIO_BANK(20, 8, 0xD0), - F7188X_GPIO_BANK(30, 8, 0xC0), - F7188X_GPIO_BANK(40, 8, 0xB0), - F7188X_GPIO_BANK(50, 8, 0xA0), - F7188X_GPIO_BANK(60, 5, 0x90), + F7188X_GPIO_BANK(0, 8, 0xF0, DRVNAME "-0"), + F7188X_GPIO_BANK(10, 8, 0xE0, DRVNAME "-1"), + F7188X_GPIO_BANK(20, 8, 0xD0, DRVNAME "-2"), + F7188X_GPIO_BANK(30, 8, 0xC0, DRVNAME "-3"), + F7188X_GPIO_BANK(40, 8, 0xB0, DRVNAME "-4"), + F7188X_GPIO_BANK(50, 8, 0xA0, DRVNAME "-5"), + F7188X_GPIO_BANK(60, 5, 0x90, DRVNAME "-6"), }; 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), + F7188X_GPIO_BANK(0, 8, 0xE0, DRVNAME "-0"), + F7188X_GPIO_BANK(10, 8, 0xE4, DRVNAME "-1"), + F7188X_GPIO_BANK(20, 8, 0xE8, DRVNAME "-2"), + F7188X_GPIO_BANK(30, 8, 0xEC, DRVNAME "-3"), + 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"), }; static int f7188x_gpio_get_direction(struct gpio_chip *chip, unsigned offset) -- 2.35.1 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 2/4] gpio-f7188x: use unique labels for banks/chips 2022-07-28 15:56 ` [PATCH 2/4] gpio-f7188x: use unique labels for banks/chips Henning Schild @ 2022-08-22 7:17 ` Linus Walleij 0 siblings, 0 replies; 13+ messages in thread From: Linus Walleij @ 2022-08-22 7:17 UTC (permalink / raw) To: Henning Schild Cc: linux-kernel, linux-gpio, Bartosz Golaszewski, Tasanakorn Phaipool, Sheng-Yuan Huang, Kuan-Wei Ho, Andy Shevchenko On Thu, Jul 28, 2022 at 5:57 PM Henning Schild <henning.schild@siemens.com> wrote: > So that drivers building on top can find those pins with GPIO_LOOKUP > helpers. > > Signed-off-by: Henning Schild <henning.schild@siemens.com> That's nice. Reviewed-by: Linus Walleij <linus.walleij@linaro.org> Yours, Linus Walleij ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 3/4] leds: simatic-ipc-leds-gpio: add new model 227G 2022-07-28 15:56 [PATCH 0/4] add support for another simatic board Henning Schild 2022-07-28 15:56 ` [PATCH 1/4] gpio-f7188x: Add GPIO support for Nuvoton NCT6116 Henning Schild 2022-07-28 15:56 ` [PATCH 2/4] gpio-f7188x: use unique labels for banks/chips Henning Schild @ 2022-07-28 15:56 ` Henning Schild 2022-07-28 21:41 ` Andy Shevchenko 2022-07-28 15:56 ` [PATCH 4/4] platform/x86: simatic-ipc: enable watchdog for 227G Henning Schild ` (2 subsequent siblings) 5 siblings, 1 reply; 13+ messages in thread From: Henning Schild @ 2022-07-28 15:56 UTC (permalink / raw) To: linux-kernel, linux-gpio, Bartosz Golaszewski, Linus Walleij, Tasanakorn Phaipool Cc: Sheng-Yuan Huang, Kuan-Wei Ho, Andy Shevchenko, Henning Schild This adds support of the Siemens Simatic IPC227G. Its LEDs are connected to GPIO pins provided by the gpio-f7188x module. We make sure that gets loaded, if not enabled in the kernel config no LED support will be available. Signed-off-by: Henning Schild <henning.schild@siemens.com> --- drivers/leds/simple/simatic-ipc-leds-gpio.c | 42 ++++++++++++++++--- drivers/platform/x86/simatic-ipc.c | 4 +- .../platform_data/x86/simatic-ipc-base.h | 1 + include/linux/platform_data/x86/simatic-ipc.h | 1 + 4 files changed, 42 insertions(+), 6 deletions(-) diff --git a/drivers/leds/simple/simatic-ipc-leds-gpio.c b/drivers/leds/simple/simatic-ipc-leds-gpio.c index 4c9e663a90ba..248882fb54e2 100644 --- a/drivers/leds/simple/simatic-ipc-leds-gpio.c +++ b/drivers/leds/simple/simatic-ipc-leds-gpio.c @@ -13,28 +13,45 @@ #include <linux/leds.h> #include <linux/module.h> #include <linux/platform_device.h> +#include <linux/platform_data/x86/simatic-ipc-base.h> -static struct gpiod_lookup_table simatic_ipc_led_gpio_table = { +struct gpiod_lookup_table *simatic_ipc_led_gpio_table; + +static struct gpiod_lookup_table simatic_ipc_led_gpio_table_127e = { .dev_id = "leds-gpio", .table = { - GPIO_LOOKUP_IDX("apollolake-pinctrl.0", 51, NULL, 0, GPIO_ACTIVE_LOW), GPIO_LOOKUP_IDX("apollolake-pinctrl.0", 52, NULL, 1, GPIO_ACTIVE_LOW), GPIO_LOOKUP_IDX("apollolake-pinctrl.0", 53, NULL, 2, GPIO_ACTIVE_LOW), GPIO_LOOKUP_IDX("apollolake-pinctrl.0", 57, NULL, 3, GPIO_ACTIVE_LOW), GPIO_LOOKUP_IDX("apollolake-pinctrl.0", 58, NULL, 4, GPIO_ACTIVE_LOW), GPIO_LOOKUP_IDX("apollolake-pinctrl.0", 60, NULL, 5, GPIO_ACTIVE_LOW), + GPIO_LOOKUP_IDX("apollolake-pinctrl.0", 51, NULL, 0, GPIO_ACTIVE_LOW), GPIO_LOOKUP_IDX("apollolake-pinctrl.0", 56, NULL, 6, GPIO_ACTIVE_LOW), GPIO_LOOKUP_IDX("apollolake-pinctrl.0", 59, NULL, 7, GPIO_ACTIVE_HIGH), }, }; +static struct gpiod_lookup_table simatic_ipc_led_gpio_table_227g = { + .dev_id = "leds-gpio", + .table = { + GPIO_LOOKUP_IDX("gpio-f7188x-2", 0, NULL, 0, GPIO_ACTIVE_LOW), + GPIO_LOOKUP_IDX("gpio-f7188x-2", 1, NULL, 1, GPIO_ACTIVE_LOW), + GPIO_LOOKUP_IDX("gpio-f7188x-2", 2, NULL, 2, GPIO_ACTIVE_LOW), + GPIO_LOOKUP_IDX("gpio-f7188x-2", 3, NULL, 3, GPIO_ACTIVE_LOW), + GPIO_LOOKUP_IDX("gpio-f7188x-2", 4, NULL, 4, GPIO_ACTIVE_LOW), + GPIO_LOOKUP_IDX("gpio-f7188x-2", 5, NULL, 5, GPIO_ACTIVE_LOW), + GPIO_LOOKUP_IDX("gpio-f7188x-2", 6, NULL, 6, GPIO_ACTIVE_LOW), + GPIO_LOOKUP_IDX("gpio-f7188x-3", 6, NULL, 7, GPIO_ACTIVE_HIGH), + } +}; + static const struct gpio_led simatic_ipc_gpio_leds[] = { - { .name = "green:" LED_FUNCTION_STATUS "-3" }, { .name = "red:" LED_FUNCTION_STATUS "-1" }, { .name = "green:" LED_FUNCTION_STATUS "-1" }, { .name = "red:" LED_FUNCTION_STATUS "-2" }, { .name = "green:" LED_FUNCTION_STATUS "-2" }, { .name = "red:" LED_FUNCTION_STATUS "-3" }, + { .name = "green:" LED_FUNCTION_STATUS "-3" }, }; static const struct gpio_led_platform_data simatic_ipc_gpio_leds_pdata = { @@ -46,7 +63,7 @@ static struct platform_device *simatic_leds_pdev; static int simatic_ipc_leds_gpio_remove(struct platform_device *pdev) { - gpiod_remove_lookup_table(&simatic_ipc_led_gpio_table); + gpiod_remove_lookup_table(simatic_ipc_led_gpio_table); platform_device_unregister(simatic_leds_pdev); return 0; @@ -54,10 +71,25 @@ static int simatic_ipc_leds_gpio_remove(struct platform_device *pdev) static int simatic_ipc_leds_gpio_probe(struct platform_device *pdev) { + const struct simatic_ipc_platform *plat = pdev->dev.platform_data; struct gpio_desc *gpiod; int err; - gpiod_add_lookup_table(&simatic_ipc_led_gpio_table); + switch (plat->devmode) { + case SIMATIC_IPC_DEVICE_127E: + simatic_ipc_led_gpio_table = &simatic_ipc_led_gpio_table_127e; + break; + case SIMATIC_IPC_DEVICE_227G: + if (!IS_ENABLED(CONFIG_GPIO_F7188X)) + return -ENODEV; + request_module("gpio-f7188x"); + simatic_ipc_led_gpio_table = &simatic_ipc_led_gpio_table_227g; + break; + default: + return -ENODEV; + } + + gpiod_add_lookup_table(simatic_ipc_led_gpio_table); simatic_leds_pdev = platform_device_register_resndata(NULL, "leds-gpio", PLATFORM_DEVID_NONE, NULL, 0, &simatic_ipc_gpio_leds_pdata, diff --git a/drivers/platform/x86/simatic-ipc.c b/drivers/platform/x86/simatic-ipc.c index ca3647b751d5..1825ef21a86d 100644 --- a/drivers/platform/x86/simatic-ipc.c +++ b/drivers/platform/x86/simatic-ipc.c @@ -41,6 +41,7 @@ static struct { {SIMATIC_IPC_IPC127E, SIMATIC_IPC_DEVICE_127E, SIMATIC_IPC_DEVICE_NONE}, {SIMATIC_IPC_IPC227D, SIMATIC_IPC_DEVICE_227D, SIMATIC_IPC_DEVICE_NONE}, {SIMATIC_IPC_IPC227E, SIMATIC_IPC_DEVICE_427E, SIMATIC_IPC_DEVICE_227E}, + {SIMATIC_IPC_IPC227G, SIMATIC_IPC_DEVICE_227G, SIMATIC_IPC_DEVICE_NONE}, {SIMATIC_IPC_IPC277E, SIMATIC_IPC_DEVICE_NONE, SIMATIC_IPC_DEVICE_227E}, {SIMATIC_IPC_IPC427D, SIMATIC_IPC_DEVICE_427E, SIMATIC_IPC_DEVICE_NONE}, {SIMATIC_IPC_IPC427E, SIMATIC_IPC_DEVICE_427E, SIMATIC_IPC_DEVICE_427E}, @@ -65,7 +66,8 @@ static int register_platform_devices(u32 station_id) } if (ledmode != SIMATIC_IPC_DEVICE_NONE) { - if (ledmode == SIMATIC_IPC_DEVICE_127E) + if (ledmode == SIMATIC_IPC_DEVICE_127E || + ledmode == SIMATIC_IPC_DEVICE_227G) pdevname = KBUILD_MODNAME "_leds_gpio"; platform_data.devmode = ledmode; ipc_led_platform_device = diff --git a/include/linux/platform_data/x86/simatic-ipc-base.h b/include/linux/platform_data/x86/simatic-ipc-base.h index 39fefd48cf4d..57d6a10dfc9e 100644 --- a/include/linux/platform_data/x86/simatic-ipc-base.h +++ b/include/linux/platform_data/x86/simatic-ipc-base.h @@ -19,6 +19,7 @@ #define SIMATIC_IPC_DEVICE_427E 2 #define SIMATIC_IPC_DEVICE_127E 3 #define SIMATIC_IPC_DEVICE_227E 4 +#define SIMATIC_IPC_DEVICE_227G 5 struct simatic_ipc_platform { u8 devmode; diff --git a/include/linux/platform_data/x86/simatic-ipc.h b/include/linux/platform_data/x86/simatic-ipc.h index f3b76b39776b..7a2e79f3be0b 100644 --- a/include/linux/platform_data/x86/simatic-ipc.h +++ b/include/linux/platform_data/x86/simatic-ipc.h @@ -31,6 +31,7 @@ enum simatic_ipc_station_ids { SIMATIC_IPC_IPC427E = 0x00000A01, SIMATIC_IPC_IPC477E = 0x00000A02, SIMATIC_IPC_IPC127E = 0x00000D01, + SIMATIC_IPC_IPC227G = 0x00000F01, }; static inline u32 simatic_ipc_get_station_id(u8 *data, int max_len) -- 2.35.1 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 3/4] leds: simatic-ipc-leds-gpio: add new model 227G 2022-07-28 15:56 ` [PATCH 3/4] leds: simatic-ipc-leds-gpio: add new model 227G Henning Schild @ 2022-07-28 21:41 ` Andy Shevchenko 2022-07-29 7:04 ` Henning Schild 0 siblings, 1 reply; 13+ messages in thread From: Andy Shevchenko @ 2022-07-28 21:41 UTC (permalink / raw) To: Henning Schild Cc: Linux Kernel Mailing List, open list:GPIO SUBSYSTEM, Bartosz Golaszewski, Linus Walleij, Tasanakorn Phaipool, Sheng-Yuan Huang, Kuan-Wei Ho On Thu, Jul 28, 2022 at 5:57 PM Henning Schild <henning.schild@siemens.com> wrote: > > This adds support of the Siemens Simatic IPC227G. Its LEDs are connected > to GPIO pins provided by the gpio-f7188x module. We make sure that > gets loaded, if not enabled in the kernel config no LED support will be > available. ... > + switch (plat->devmode) { > + case SIMATIC_IPC_DEVICE_127E: > + simatic_ipc_led_gpio_table = &simatic_ipc_led_gpio_table_127e; > + break; > + case SIMATIC_IPC_DEVICE_227G: > + if (!IS_ENABLED(CONFIG_GPIO_F7188X)) > + return -ENODEV; Hmm... What is the difference with the 127E model in the sense of the driver absence? Why do we need this check? > + request_module("gpio-f7188x"); I'm not sure why you need request_module(). > + simatic_ipc_led_gpio_table = &simatic_ipc_led_gpio_table_227g; > + break; > + default: > + return -ENODEV; > + } -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 3/4] leds: simatic-ipc-leds-gpio: add new model 227G 2022-07-28 21:41 ` Andy Shevchenko @ 2022-07-29 7:04 ` Henning Schild 0 siblings, 0 replies; 13+ messages in thread From: Henning Schild @ 2022-07-29 7:04 UTC (permalink / raw) To: Andy Shevchenko, Pavel Machek Cc: Linux Kernel Mailing List, open list:GPIO SUBSYSTEM, Bartosz Golaszewski, Linus Walleij, Tasanakorn Phaipool, Sheng-Yuan Huang Am Thu, 28 Jul 2022 23:41:31 +0200 schrieb Andy Shevchenko <andy.shevchenko@gmail.com>: > On Thu, Jul 28, 2022 at 5:57 PM Henning Schild > <henning.schild@siemens.com> wrote: > > > > This adds support of the Siemens Simatic IPC227G. Its LEDs are > > connected to GPIO pins provided by the gpio-f7188x module. We make > > sure that gets loaded, if not enabled in the kernel config no LED > > support will be available. > > ... > > > + switch (plat->devmode) { > > + case SIMATIC_IPC_DEVICE_127E: > > + simatic_ipc_led_gpio_table = > > &simatic_ipc_led_gpio_table_127e; > > + break; > > + case SIMATIC_IPC_DEVICE_227G: > > > + if (!IS_ENABLED(CONFIG_GPIO_F7188X)) > > + return -ENODEV; > > Hmm... What is the difference with the 127E model in the sense of the > driver absence? Why do we need this check? What happens when the GPIO_LOOKUP_IDX does not find anything is an endless printing of all missing lookups, actually pretty frequently so that the kernel log becomes useless. I did not look deeper but i guess "leds-gpio" will go into some sort of polling and wait for those pins. The debian configured kernels i used so far for my work always had PINCTRL_BROXTON, i guess if that was not available i would end up in the polling/printing loop as well. I never tried without, just found that polling thing when working on this model. In fact the 127E will likely also need to check for PINCTRL_BROXTON somehow. Or maybe the "leds-gpio" platform device needs to be registered in a way where it will not go into polling/printing. I will study the code to see if there is a way to go without polling but then i might need some sort of serialization so i go not try and register+fail before those pins exist. Pavel maybe you have an idea what to do here. > > + request_module("gpio-f7188x"); > > I'm not sure why you need request_module(). The difference to "pinctrl-broxton" is that it comes up on its own because it has autoloading support. And this one only probes on =m + modprobe or =y. Henning > > + simatic_ipc_led_gpio_table = > > &simatic_ipc_led_gpio_table_227g; > > + break; > > + default: > > + return -ENODEV; > > + } > ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 4/4] platform/x86: simatic-ipc: enable watchdog for 227G 2022-07-28 15:56 [PATCH 0/4] add support for another simatic board Henning Schild ` (2 preceding siblings ...) 2022-07-28 15:56 ` [PATCH 3/4] leds: simatic-ipc-leds-gpio: add new model 227G Henning Schild @ 2022-07-28 15:56 ` Henning Schild 2022-07-29 7:07 ` [PATCH 0/4] add support for another simatic board Henning Schild 2022-08-22 7:34 ` Linus Walleij 5 siblings, 0 replies; 13+ messages in thread From: Henning Schild @ 2022-07-28 15:56 UTC (permalink / raw) To: linux-kernel, linux-gpio, Bartosz Golaszewski, Linus Walleij, Tasanakorn Phaipool Cc: Sheng-Yuan Huang, Kuan-Wei Ho, Andy Shevchenko, Henning Schild Just load the watchdog module, after having identified that machine. That watchdog module does not have any autoloading support. Signed-off-by: Henning Schild <henning.schild@siemens.com> --- drivers/platform/x86/simatic-ipc.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/platform/x86/simatic-ipc.c b/drivers/platform/x86/simatic-ipc.c index 1825ef21a86d..8dd686d1c9f1 100644 --- a/drivers/platform/x86/simatic-ipc.c +++ b/drivers/platform/x86/simatic-ipc.c @@ -96,6 +96,9 @@ static int register_platform_devices(u32 station_id) ipc_wdt_platform_device->name); } + if (station_id == SIMATIC_IPC_IPC227G) + request_module("w83627hf_wdt"); + if (ledmode == SIMATIC_IPC_DEVICE_NONE && wdtmode == SIMATIC_IPC_DEVICE_NONE) { pr_warn("unsupported IPC detected, station id=%08x\n", -- 2.35.1 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 0/4] add support for another simatic board 2022-07-28 15:56 [PATCH 0/4] add support for another simatic board Henning Schild ` (3 preceding siblings ...) 2022-07-28 15:56 ` [PATCH 4/4] platform/x86: simatic-ipc: enable watchdog for 227G Henning Schild @ 2022-07-29 7:07 ` Henning Schild 2022-08-22 7:34 ` Linus Walleij 5 siblings, 0 replies; 13+ messages in thread From: Henning Schild @ 2022-07-29 7:07 UTC (permalink / raw) To: linux-kernel, linux-gpio, Bartosz Golaszewski, Linus Walleij, Tasanakorn Phaipool Cc: Sheng-Yuan Huang, Pavel Machek, Andy Shevchenko I wanted to send this series to a wider audience, somehow i messed that up and might have to send again. Maybe the v5, and if there is no changes i might send again as v4. Henning Am Thu, 28 Jul 2022 17:56:48 +0200 schrieb Henning Schild <henning.schild@siemens.com>: > This series first enables a SuperIO GPIO driver to support a chip from > the vendor Nuvoton, the driver is for Fintek devices but those just > are very similar. And in watchdog and hwmon subsystems these SuperIO > drivers also share code and are sometimes called a family. > > In another step the individual banks receive a label to tell them > apart, a step which potentially changes an interface to legacy users > that might rely on all banks having the same label, or an exact > label. But since a later patch wants to use GPIO_LOOKUP unique labels > are needed and i decided to assign them for all supported chips. > > In a following patch the Simatic GPIO LED driver is extended to > provide LEDs in case that SuperIO GPIO driver can be loaded. > > Last but not least the watchdog module of that same SuperIO gets > loaded on a best effort basis. > > Note similar patches have appreared before as > "[PATCH v3 0/1] add device driver for Nuvoton SIO gpio function" > The main difference here is that i added chip support to an existing > driver instead of creating a new one. And that i actually propose all > patches and do not just have the LED patch for Simatic as an example. > Also note that the patches are based on > "[PATCH v6 00/12] platform/x86: introduce p2sb_bar() helper" > > Henning Schild (4): > gpio-f7188x: Add GPIO support for Nuvoton NCT6116 > gpio-f7188x: use unique labels for banks/chips > leds: simatic-ipc-leds-gpio: add new model 227G > platform/x86: simatic-ipc: enable watchdog for 227G > > drivers/gpio/gpio-f7188x.c | 192 > +++++++++++------- drivers/leds/simple/simatic-ipc-leds-gpio.c | > 42 +++- drivers/platform/x86/simatic-ipc.c | 7 +- > .../platform_data/x86/simatic-ipc-base.h | 1 + > include/linux/platform_data/x86/simatic-ipc.h | 1 + > 5 files changed, 158 insertions(+), 85 deletions(-) > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 0/4] add support for another simatic board 2022-07-28 15:56 [PATCH 0/4] add support for another simatic board Henning Schild ` (4 preceding siblings ...) 2022-07-29 7:07 ` [PATCH 0/4] add support for another simatic board Henning Schild @ 2022-08-22 7:34 ` Linus Walleij 5 siblings, 0 replies; 13+ messages in thread From: Linus Walleij @ 2022-08-22 7:34 UTC (permalink / raw) To: Henning Schild Cc: linux-kernel, linux-gpio, Bartosz Golaszewski, Tasanakorn Phaipool, Sheng-Yuan Huang, Kuan-Wei Ho, Andy Shevchenko On Thu, Jul 28, 2022 at 5:57 PM Henning Schild <henning.schild@siemens.com> wrote: > In another step the individual banks receive a label to tell them apart, > a step which potentially changes an interface to legacy users that might > rely on all banks having the same label, or an exact label. But since a > later patch wants to use GPIO_LOOKUP unique labels are needed and i > decided to assign them for all supported chips. Since it is all in-kernel users, any "legacy users" should be in the kernel tree and you should then fix them in the patch. If they are not in the kernel tree, they are out-of-tree drivers and them we do not care, we can't fix the whole world. Yours, Linus Walleij ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2022-08-22 7:34 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-07-28 15:56 [PATCH 0/4] add support for another simatic board Henning Schild 2022-07-28 15:56 ` [PATCH 1/4] gpio-f7188x: Add GPIO support for Nuvoton NCT6116 Henning Schild 2022-07-28 21:33 ` Andy Shevchenko 2022-07-29 7:16 ` Henning Schild 2022-08-09 14:57 ` Henning Schild 2022-07-28 15:56 ` [PATCH 2/4] gpio-f7188x: use unique labels for banks/chips Henning Schild 2022-08-22 7:17 ` Linus Walleij 2022-07-28 15:56 ` [PATCH 3/4] leds: simatic-ipc-leds-gpio: add new model 227G Henning Schild 2022-07-28 21:41 ` Andy Shevchenko 2022-07-29 7:04 ` Henning Schild 2022-07-28 15:56 ` [PATCH 4/4] platform/x86: simatic-ipc: enable watchdog for 227G Henning Schild 2022-07-29 7:07 ` [PATCH 0/4] add support for another simatic board Henning Schild 2022-08-22 7:34 ` Linus Walleij
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).