changes since v5:
- adding patch to convert to pr_fmt
- adding patch to prefix macros with "f7188x_"
- rebased p1v4 to be p3v5 and added tag
changes since v4:
- remove int case from a printk in p1
- include tags into commit messages
changes since v3:
- update Kconfig as well
- drop chip names from comment in driver header
- add manufacturer check for Fintek again, Nuvoton not possible
- drop revision printing for Nuvoton
- restructure defines again
- add new model 427G
changes since v2: (p1 only)
- rename macros that change behavior
- use chip type not device id in the macros
- reorder defines a bit
changes since v1:
- remove unused define
- fix bug where (base + 2) was used as second data bit
- add macros for "inverted" and "single data bit"
The first two patches apply some style refactorings before actual
functional changes are made.
Later, This series 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.
The very last patch enables a second model of that same board type.
Henning Schild (7):
gpio-f7188x: switch over to using pr_fmt
gpio-f7188x: add a prefix to macros to keep gpio namespace clean
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
platform/x86: simatic-ipc: add new model 427G
drivers/gpio/Kconfig | 3 +-
drivers/gpio/gpio-f7188x.c | 275 +++++++++++-------
drivers/leds/simple/simatic-ipc-leds-gpio.c | 42 ++-
drivers/platform/x86/simatic-ipc.c | 10 +-
.../platform_data/x86/simatic-ipc-base.h | 1 +
include/linux/platform_data/x86/simatic-ipc.h | 2 +
6 files changed, 216 insertions(+), 117 deletions(-)
--
2.35.1
So that drivers building on top can find those pins with GPIO_LOOKUP
helpers.
Reviewed-by: Andy Shevchenko <[email protected]>
Reviewed-by: Linus Walleij <[email protected]>
Acked-by: Simon Guinot <[email protected]>
Signed-off-by: Henning Schild <[email protected]>
---
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 29c73ea96466..9effa7769bef 100644
--- a/drivers/gpio/gpio-f7188x.c
+++ b/drivers/gpio/gpio-f7188x.c
@@ -163,10 +163,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, \
@@ -191,98 +191,98 @@ static int f7188x_gpio_set_config(struct gpio_chip *chip, unsigned offset,
#define f7188x_gpio_data_single(type) ((type) == nct6116d)
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
Subsequent patches will touch that file, apply some nice to have style
changes before actually adding functional changes.
Signed-off-by: Henning Schild <[email protected]>
---
drivers/gpio/gpio-f7188x.c | 17 +++++++++--------
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/drivers/gpio/gpio-f7188x.c b/drivers/gpio/gpio-f7188x.c
index 18a3147f5a42..fef539bbc03a 100644
--- a/drivers/gpio/gpio-f7188x.c
+++ b/drivers/gpio/gpio-f7188x.c
@@ -7,6 +7,9 @@
* Author: Simon Guinot <[email protected]>
*/
+#define DRVNAME "gpio-f7188x"
+#define pr_fmt(fmt) DRVNAME ": " fmt
+
#include <linux/module.h>
#include <linux/init.h>
#include <linux/platform_device.h>
@@ -14,8 +17,6 @@
#include <linux/gpio/driver.h>
#include <linux/bitops.h>
-#define DRVNAME "gpio-f7188x"
-
/*
* Super-I/O registers
*/
@@ -110,7 +111,7 @@ static inline int superio_enter(int base)
{
/* Don't step on other drivers' I/O space by accident. */
if (!request_muxed_region(base, 2, DRVNAME)) {
- pr_err(DRVNAME "I/O address 0x%04x already in use\n", base);
+ pr_err("I/O address 0x%04x already in use\n", base);
return -EBUSY;
}
@@ -487,7 +488,7 @@ static int __init f7188x_find(int addr, struct f7188x_sio *sio)
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);
+ pr_debug("Not a Fintek device at 0x%08x\n", addr);
goto err;
}
@@ -518,13 +519,13 @@ static int __init f7188x_find(int addr, struct f7188x_sio *sio)
sio->type = f81865;
break;
default:
- pr_info(DRVNAME ": Unsupported Fintek device 0x%04x\n", devid);
+ pr_info("Unsupported Fintek device 0x%04x\n", devid);
goto err;
}
sio->addr = addr;
err = 0;
- pr_info(DRVNAME ": Found %s at %#x, revision %d\n",
+ pr_info("Found %s at %#x, revision %d\n",
f7188x_names[sio->type],
(unsigned int) addr,
(int) superio_inb(addr, SIO_DEVREV));
@@ -548,13 +549,13 @@ f7188x_gpio_device_add(const struct f7188x_sio *sio)
err = platform_device_add_data(f7188x_gpio_pdev,
sio, sizeof(*sio));
if (err) {
- pr_err(DRVNAME "Platform data allocation failed\n");
+ pr_err("Platform data allocation failed\n");
goto err;
}
err = platform_device_add(f7188x_gpio_pdev);
if (err) {
- pr_err(DRVNAME "Device addition failed\n");
+ pr_err("Device addition failed\n");
goto err;
}
--
2.35.1
This adds support for the Siemens Simatic IPC427G. A board which
basically works like the 227G added in a previous patch. So all there is
to do is to add the station_id and make it take all the 227G branches.
Reviewed-by: Andy Shevchenko <[email protected]>
Signed-off-by: Henning Schild <[email protected]>
---
drivers/platform/x86/simatic-ipc.c | 11 +++++++----
include/linux/platform_data/x86/simatic-ipc.h | 1 +
2 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/platform/x86/simatic-ipc.c b/drivers/platform/x86/simatic-ipc.c
index 8dd686d1c9f1..ca76076fc706 100644
--- a/drivers/platform/x86/simatic-ipc.c
+++ b/drivers/platform/x86/simatic-ipc.c
@@ -41,11 +41,12 @@ 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_IPC227G, SIMATIC_IPC_DEVICE_227G, SIMATIC_IPC_DEVICE_227G},
{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},
{SIMATIC_IPC_IPC477E, SIMATIC_IPC_DEVICE_NONE, SIMATIC_IPC_DEVICE_427E},
+ {SIMATIC_IPC_IPC427G, SIMATIC_IPC_DEVICE_227G, SIMATIC_IPC_DEVICE_227G},
};
static int register_platform_devices(u32 station_id)
@@ -82,6 +83,11 @@ static int register_platform_devices(u32 station_id)
ipc_led_platform_device->name);
}
+ if (wdtmode == SIMATIC_IPC_DEVICE_227G) {
+ request_module("w83627hf_wdt");
+ return 0;
+ }
+
if (wdtmode != SIMATIC_IPC_DEVICE_NONE) {
platform_data.devmode = wdtmode;
ipc_wdt_platform_device =
@@ -96,9 +102,6 @@ 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",
diff --git a/include/linux/platform_data/x86/simatic-ipc.h b/include/linux/platform_data/x86/simatic-ipc.h
index 7a2e79f3be0b..632320ec8f08 100644
--- a/include/linux/platform_data/x86/simatic-ipc.h
+++ b/include/linux/platform_data/x86/simatic-ipc.h
@@ -32,6 +32,7 @@ enum simatic_ipc_station_ids {
SIMATIC_IPC_IPC477E = 0x00000A02,
SIMATIC_IPC_IPC127E = 0x00000D01,
SIMATIC_IPC_IPC227G = 0x00000F01,
+ SIMATIC_IPC_IPC427G = 0x00001001,
};
static inline u32 simatic_ipc_get_station_id(u8 *data, int max_len)
--
2.35.1
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.
Reviewed-by: Andy Shevchenko <[email protected]>
Reviewed-by: Hans de Goede <[email protected]>
Signed-off-by: Henning Schild <[email protected]>
---
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..0d73dcbeec2d 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-3", 6, NULL, 6, GPIO_ACTIVE_HIGH),
+ GPIO_LOOKUP_IDX("gpio-f7188x-3", 7, 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
Just load the watchdog module, after having identified that machine.
That watchdog module does not have any autoloading support.
Reviewed-by: Andy Shevchenko <[email protected]>
Signed-off-by: Henning Schild <[email protected]>
---
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
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.
On a chip level we do not have a manufacturer ID to check and also no
revision.
Reviewed-by: Andy Shevchenko <[email protected]>
Acked-by: Simon Guinot <[email protected]>
Signed-off-by: Henning Schild <[email protected]>
---
drivers/gpio/Kconfig | 3 +-
drivers/gpio/gpio-f7188x.c | 104 ++++++++++++++++++++++++++++---------
2 files changed, 81 insertions(+), 26 deletions(-)
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 0642f579196f..3f64345fe40b 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -874,10 +874,11 @@ config GPIO_104_IDI_48
module parameter.
config GPIO_F7188X
- tristate "F71869, F71869A, F71882FG, F71889F and F81866 GPIO support"
+ tristate "Fintek and Nuvoton Super-I/O GPIO support"
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.
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 878458249833..29c73ea96466 100644
--- a/drivers/gpio/gpio-f7188x.c
+++ b/drivers/gpio/gpio-f7188x.c
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: GPL-2.0-or-later
/*
- * GPIO driver for Fintek Super-I/O F71869, F71869A, F71882, F71889 and F81866
+ * GPIO driver for Fintek and Nuvoton Super-I/O chips
*
* Copyright (C) 2010-2013 LaCie
*
@@ -22,23 +22,36 @@
*/
#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 */
+/*
+ * Fintek devices.
+ */
+#define SIO_FINTEK_DEVREV 0x22 /* Fintek Device revision */
+#define SIO_FINTEK_MANID 0x23 /* Fintek ID (2 bytes) */
+
+#define SIO_FINTEK_ID 0x1934 /* Manufacturer ID */
+
#define SIO_F71869_ID 0x0814 /* F71869 chipset ID */
#define SIO_F71869A_ID 0x1007 /* F71869A chipset ID */
#define SIO_F71882_ID 0x0541 /* F71882 chipset ID */
#define SIO_F71889_ID 0x0909 /* F71889 chipset ID */
#define SIO_F71889A_ID 0x1005 /* F71889A chipset ID */
#define SIO_F81866_ID 0x1010 /* F81866 chipset ID */
-#define SIO_F81804_ID 0x1502 /* F81804 chipset ID, same for f81966 */
+#define SIO_F81804_ID 0x1502 /* F81804 chipset ID, same for F81966 */
#define SIO_F81865_ID 0x0704 /* F81865 chipset ID */
+#define SIO_LD_GPIO_FINTEK 0x06 /* GPIO logical device */
+
+/*
+ * Nuvoton devices.
+ */
+#define SIO_NCT6116D_ID 0xD283 /* NCT6116D chipset ID */
+
+#define SIO_LD_GPIO_NUVOTON 0x07 /* GPIO logical device */
+
enum chips {
f71869,
@@ -49,6 +62,7 @@ enum chips {
f81866,
f81804,
f81865,
+ nct6116d,
};
static const char * const f7188x_names[] = {
@@ -60,10 +74,12 @@ static const char * const f7188x_names[] = {
"f81866",
"f81804",
"f81865",
+ "nct6116d",
};
struct f7188x_sio {
int addr;
+ int device;
enum chips type;
};
@@ -171,6 +187,9 @@ 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)
+
static struct f7188x_gpio_bank f71869_gpio_bank[] = {
F7188X_GPIO_BANK(0, 6, 0xF0),
F7188X_GPIO_BANK(10, 8, 0xE0),
@@ -255,6 +274,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;
@@ -265,13 +295,16 @@ 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, f7188x_gpio_dir(bank->regbase));
superio_exit(sio->addr);
- if (dir & 1 << offset)
+ if (f7188x_gpio_dir_invert(sio->type))
+ dir = ~dir;
+
+ if (dir & BIT(offset))
return GPIO_LINE_DIRECTION_OUT;
return GPIO_LINE_DIRECTION_IN;
@@ -287,10 +320,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, f7188x_gpio_dir(bank->regbase));
- dir &= ~BIT(offset);
+
+ if (f7188x_gpio_dir_invert(sio->type))
+ dir |= BIT(offset);
+ else
+ dir &= ~BIT(offset);
superio_outb(sio->addr, f7188x_gpio_dir(bank->regbase), dir);
superio_exit(sio->addr);
@@ -308,11 +345,11 @@ 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, f7188x_gpio_dir(bank->regbase));
dir = !!(dir & BIT(offset));
- if (dir)
+ if (f7188x_gpio_data_single(sio->type) || dir)
data = superio_inb(sio->addr, f7188x_gpio_data_out(bank->regbase));
else
data = superio_inb(sio->addr, f7188x_gpio_data_in(bank->regbase));
@@ -333,7 +370,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, f7188x_gpio_data_out(bank->regbase));
if (value)
@@ -343,7 +380,10 @@ static int f7188x_gpio_direction_out(struct gpio_chip *chip,
superio_outb(sio->addr, f7188x_gpio_data_out(bank->regbase), data_out);
dir = superio_inb(sio->addr, f7188x_gpio_dir(bank->regbase));
- dir |= BIT(offset);
+ if (f7188x_gpio_dir_invert(sio->type))
+ dir &= ~BIT(offset);
+ else
+ dir |= BIT(offset);
superio_outb(sio->addr, f7188x_gpio_dir(bank->regbase), dir);
superio_exit(sio->addr);
@@ -361,7 +401,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, f7188x_gpio_data_out(bank->regbase));
if (value)
@@ -389,7 +429,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, f7188x_gpio_out_mode(bank->regbase));
if (param == PIN_CONFIG_DRIVE_OPEN_DRAIN)
@@ -450,6 +490,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;
}
@@ -480,18 +524,15 @@ static int __init f7188x_find(int addr, struct f7188x_sio *sio)
{
int err;
u16 devid;
+ u16 manid;
err = superio_enter(addr);
if (err)
return err;
err = -ENODEV;
- devid = superio_inw(addr, SIO_MANID);
- if (devid != SIO_FINTEK_ID) {
- pr_debug("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:
@@ -518,17 +559,30 @@ 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("Unsupported Fintek device 0x%04x\n", devid);
goto err;
}
+
+ /* double check manufacturer where possible */
+ if (sio->type != nct6116d) {
+ manid = superio_inw(addr, SIO_FINTEK_MANID);
+ if (manid != SIO_FINTEK_ID) {
+ pr_debug("Not a Fintek device at 0x%08x\n", addr);
+ goto err;
+ }
+ }
+
sio->addr = addr;
err = 0;
- pr_info("Found %s at %#x, revision %d\n",
- f7188x_names[sio->type],
- (unsigned int) addr,
- (int) superio_inb(addr, SIO_DEVREV));
+ pr_info("Found %s at %#x\n", f7188x_names[sio->type], (unsigned int)addr);
+ if (sio->type != nct6116d)
+ pr_info(" revision %d\n", superio_inb(addr, SIO_FINTEK_DEVREV));
err:
superio_exit(addr);
--
2.35.1
On Thu, Aug 25, 2022 at 12:44:16PM +0200, Henning Schild wrote:
> Subsequent patches will touch that file, apply some nice to have style
> changes before actually adding functional changes.
>
> Signed-off-by: Henning Schild <[email protected]>
Acked-by: Simon Guinot <[email protected]>
> ---
> drivers/gpio/gpio-f7188x.c | 17 +++++++++--------
> 1 file changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpio/gpio-f7188x.c b/drivers/gpio/gpio-f7188x.c
> index 18a3147f5a42..fef539bbc03a 100644
> --- a/drivers/gpio/gpio-f7188x.c
> +++ b/drivers/gpio/gpio-f7188x.c
> @@ -7,6 +7,9 @@
> * Author: Simon Guinot <[email protected]>
> */
>
> +#define DRVNAME "gpio-f7188x"
> +#define pr_fmt(fmt) DRVNAME ": " fmt
> +
> #include <linux/module.h>
> #include <linux/init.h>
> #include <linux/platform_device.h>
> @@ -14,8 +17,6 @@
> #include <linux/gpio/driver.h>
> #include <linux/bitops.h>
>
> -#define DRVNAME "gpio-f7188x"
> -
> /*
> * Super-I/O registers
> */
> @@ -110,7 +111,7 @@ static inline int superio_enter(int base)
> {
> /* Don't step on other drivers' I/O space by accident. */
> if (!request_muxed_region(base, 2, DRVNAME)) {
> - pr_err(DRVNAME "I/O address 0x%04x already in use\n", base);
> + pr_err("I/O address 0x%04x already in use\n", base);
> return -EBUSY;
> }
>
> @@ -487,7 +488,7 @@ static int __init f7188x_find(int addr, struct f7188x_sio *sio)
> 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);
> + pr_debug("Not a Fintek device at 0x%08x\n", addr);
> goto err;
> }
>
> @@ -518,13 +519,13 @@ static int __init f7188x_find(int addr, struct f7188x_sio *sio)
> sio->type = f81865;
> break;
> default:
> - pr_info(DRVNAME ": Unsupported Fintek device 0x%04x\n", devid);
> + pr_info("Unsupported Fintek device 0x%04x\n", devid);
> goto err;
> }
> sio->addr = addr;
> err = 0;
>
> - pr_info(DRVNAME ": Found %s at %#x, revision %d\n",
> + pr_info("Found %s at %#x, revision %d\n",
> f7188x_names[sio->type],
> (unsigned int) addr,
> (int) superio_inb(addr, SIO_DEVREV));
> @@ -548,13 +549,13 @@ f7188x_gpio_device_add(const struct f7188x_sio *sio)
> err = platform_device_add_data(f7188x_gpio_pdev,
> sio, sizeof(*sio));
> if (err) {
> - pr_err(DRVNAME "Platform data allocation failed\n");
> + pr_err("Platform data allocation failed\n");
> goto err;
> }
>
> err = platform_device_add(f7188x_gpio_pdev);
> if (err) {
> - pr_err(DRVNAME "Device addition failed\n");
> + pr_err("Device addition failed\n");
> goto err;
> }
>
> --
> 2.35.1
Am Thu, 25 Aug 2022 16:25:49 +0200
schrieb Hans de Goede <[email protected]>:
> Hi,
>
> On 8/25/22 12:44, Henning Schild wrote:
> > changes since v5:
> > - adding patch to convert to pr_fmt
> > - adding patch to prefix macros with "f7188x_"
> > - rebased p1v4 to be p3v5 and added tag
> >
> > changes since v4:
> > - remove int case from a printk in p1
> > - include tags into commit messages
> >
> > changes since v3:
> > - update Kconfig as well
> > - drop chip names from comment in driver header
> > - add manufacturer check for Fintek again, Nuvoton not possible
> > - drop revision printing for Nuvoton
> > - restructure defines again
> > - add new model 427G
> >
> > changes since v2: (p1 only)
> > - rename macros that change behavior
> > - use chip type not device id in the macros
> > - reorder defines a bit
> >
> > changes since v1:
> > - remove unused define
> > - fix bug where (base + 2) was used as second data bit
> > - add macros for "inverted" and "single data bit"
> >
> > The first two patches apply some style refactorings before actual
> > functional changes are made.
> >
> > Later, This series 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.
> >
> > The very last patch enables a second model of that same board type.
> >
> > Henning Schild (7):
> > gpio-f7188x: switch over to using pr_fmt
> > gpio-f7188x: add a prefix to macros to keep gpio namespace clean
> > 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
> > platform/x86: simatic-ipc: add new model 427G
>
> So it looks like all these patches are ready for merging now,
> the only thing which is missing is an Ack from Pavel or
> one of the other LED people for patch 5/7.
>
> Pavel can have your ack for merging this through another tree
> please?
Would i need to send again and include the tags given on v6?
Henning
> So what is the plan for merging this?
>
> I see 2 options:
>
> Option a:
> 1. Merge the GPIO changes (patches 1-4) through the GPIO tree; and
> 2. Merge the leds + pdx86 changes through the pdx86 tree
>
> Option b:
> Merge everything through the pdx86 tree, and I will then provide
> an immutable branch + signed tag for other subsystems to pull
> (if they want to).
>
> Regards,
>
> Hans
>
Hi,
On 8/25/22 16:29, Henning Schild wrote:
> Am Thu, 25 Aug 2022 16:25:49 +0200
> schrieb Hans de Goede <[email protected]>:
>
>> Hi,
>>
>> On 8/25/22 12:44, Henning Schild wrote:
>>> changes since v5:
>>> - adding patch to convert to pr_fmt
>>> - adding patch to prefix macros with "f7188x_"
>>> - rebased p1v4 to be p3v5 and added tag
>>>
>>> changes since v4:
>>> - remove int case from a printk in p1
>>> - include tags into commit messages
>>>
>>> changes since v3:
>>> - update Kconfig as well
>>> - drop chip names from comment in driver header
>>> - add manufacturer check for Fintek again, Nuvoton not possible
>>> - drop revision printing for Nuvoton
>>> - restructure defines again
>>> - add new model 427G
>>>
>>> changes since v2: (p1 only)
>>> - rename macros that change behavior
>>> - use chip type not device id in the macros
>>> - reorder defines a bit
>>>
>>> changes since v1:
>>> - remove unused define
>>> - fix bug where (base + 2) was used as second data bit
>>> - add macros for "inverted" and "single data bit"
>>>
>>> The first two patches apply some style refactorings before actual
>>> functional changes are made.
>>>
>>> Later, This series 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.
>>>
>>> The very last patch enables a second model of that same board type.
>>>
>>> Henning Schild (7):
>>> gpio-f7188x: switch over to using pr_fmt
>>> gpio-f7188x: add a prefix to macros to keep gpio namespace clean
>>> 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
>>> platform/x86: simatic-ipc: add new model 427G
>>
>> So it looks like all these patches are ready for merging now,
>> the only thing which is missing is an Ack from Pavel or
>> one of the other LED people for patch 5/7.
>>
>> Pavel can have your ack for merging this through another tree
>> please?
>
> Would i need to send again and include the tags given on v6?
No that is not necessary. The only reason for sending a new
version would be if Pavel wants some changes to patch 5/7
Regards,
Hans
>
> Henning
>
>> So what is the plan for merging this?
>>
>> I see 2 options:
>>
>> Option a:
>> 1. Merge the GPIO changes (patches 1-4) through the GPIO tree; and
>> 2. Merge the leds + pdx86 changes through the pdx86 tree
>>
>> Option b:
>> Merge everything through the pdx86 tree, and I will then provide
>> an immutable branch + signed tag for other subsystems to pull
>> (if they want to).
>>
>> Regards,
>>
>> Hans
>>
>
Hi,
On 8/25/22 12:44, Henning Schild wrote:
> changes since v5:
> - adding patch to convert to pr_fmt
> - adding patch to prefix macros with "f7188x_"
> - rebased p1v4 to be p3v5 and added tag
>
> changes since v4:
> - remove int case from a printk in p1
> - include tags into commit messages
>
> changes since v3:
> - update Kconfig as well
> - drop chip names from comment in driver header
> - add manufacturer check for Fintek again, Nuvoton not possible
> - drop revision printing for Nuvoton
> - restructure defines again
> - add new model 427G
>
> changes since v2: (p1 only)
> - rename macros that change behavior
> - use chip type not device id in the macros
> - reorder defines a bit
>
> changes since v1:
> - remove unused define
> - fix bug where (base + 2) was used as second data bit
> - add macros for "inverted" and "single data bit"
>
> The first two patches apply some style refactorings before actual
> functional changes are made.
>
> Later, This series 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.
>
> The very last patch enables a second model of that same board type.
>
> Henning Schild (7):
> gpio-f7188x: switch over to using pr_fmt
> gpio-f7188x: add a prefix to macros to keep gpio namespace clean
> 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
> platform/x86: simatic-ipc: add new model 427G
So it looks like all these patches are ready for merging now,
the only thing which is missing is an Ack from Pavel or
one of the other LED people for patch 5/7.
Pavel can have your ack for merging this through another tree
please?
So what is the plan for merging this?
I see 2 options:
Option a:
1. Merge the GPIO changes (patches 1-4) through the GPIO tree; and
2. Merge the leds + pdx86 changes through the pdx86 tree
Option b:
Merge everything through the pdx86 tree, and I will then provide
an immutable branch + signed tag for other subsystems to pull
(if they want to).
Regards,
Hans
Am Thu, 25 Aug 2022 12:44:20 +0200
schrieb Henning Schild <[email protected]>:
> 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.
>
> Reviewed-by: Andy Shevchenko <[email protected]>
> Reviewed-by: Hans de Goede <[email protected]>
> Signed-off-by: Henning Schild <[email protected]>
> ---
> 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..0d73dcbeec2d 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-3", 6, NULL, 6,
> GPIO_ACTIVE_HIGH),
> + GPIO_LOOKUP_IDX("gpio-f7188x-3", 7, 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" },
> };
Concerning these two tables from above i have a question i need to find
an answer for for maintaining the out-of-tree modules of these drivers.
When getting the drivers into the kernel i had to rename the leds but
in out-of-tree i would like to keep the old names and just equip their
setters/getters with a deprecation warning. Just to give existing
users time to slowly adopt the upstream name change if i can.
In the open-coded way i just defined each LED twice and added a strcmp
+ pr_warn. With the "leds-gpio" version i still fail to find a solution
which does not get me into -EBUSY. So i already fail at the second
definition of the legacy name, not even had a chance to think about how
to smuggle in my deprecation warning.
I know out-of-tree is not a concern to people here, but someone might
have an answer anyhow.
Thanks,
Henning
> 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)
On Thu, Aug 25, 2022 at 8:51 PM Henning Schild
<[email protected]> wrote:
> Am Thu, 25 Aug 2022 12:44:20 +0200
> schrieb Henning Schild <[email protected]>:
...
> Concerning these two tables from above i have a question i need to find
> an answer for for maintaining the out-of-tree modules of these drivers.
>
> When getting the drivers into the kernel i had to rename the leds but
> in out-of-tree i would like to keep the old names and just equip their
> setters/getters with a deprecation warning. Just to give existing
> users time to slowly adopt the upstream name change if i can.
>
> In the open-coded way i just defined each LED twice and added a strcmp
> + pr_warn. With the "leds-gpio" version i still fail to find a solution
> which does not get me into -EBUSY. So i already fail at the second
> definition of the legacy name, not even had a chance to think about how
> to smuggle in my deprecation warning.
>
> I know out-of-tree is not a concern to people here, but someone might
> have an answer anyhow.
Yes, we (upstream) don't care about out-of-tree stuff. But I think
what you are asking for is kinda an alias. Maybe you simply can create
a module that will wait for the led appearing (by notify that adds a
device or alike) and create an alias by sysfs symlink (IIRC there are
kernel APIs for that)? It will be another out-of-tree module that you
may drop whenever is time.
--
With Best Regards,
Andy Shevchenko
Am Thu, 25 Aug 2022 21:03:53 +0300
schrieb Andy Shevchenko <[email protected]>:
> On Thu, Aug 25, 2022 at 8:51 PM Henning Schild
> <[email protected]> wrote:
> > Am Thu, 25 Aug 2022 12:44:20 +0200
> > schrieb Henning Schild <[email protected]>:
>
> ...
>
> > Concerning these two tables from above i have a question i need to
> > find an answer for for maintaining the out-of-tree modules of these
> > drivers.
> >
> > When getting the drivers into the kernel i had to rename the leds
> > but in out-of-tree i would like to keep the old names and just
> > equip their setters/getters with a deprecation warning. Just to
> > give existing users time to slowly adopt the upstream name change
> > if i can.
> >
> > In the open-coded way i just defined each LED twice and added a
> > strcmp
> > + pr_warn. With the "leds-gpio" version i still fail to find a
> > solution which does not get me into -EBUSY. So i already fail at
> > the second definition of the legacy name, not even had a chance to
> > think about how to smuggle in my deprecation warning.
> >
> > I know out-of-tree is not a concern to people here, but someone
> > might have an answer anyhow.
>
> Yes, we (upstream) don't care about out-of-tree stuff. But I think
> what you are asking for is kinda an alias. Maybe you simply can create
> a module that will wait for the led appearing (by notify that adds a
> device or alike) and create an alias by sysfs symlink (IIRC there are
> kernel APIs for that)? It will be another out-of-tree module that you
> may drop whenever is time.
Thanks! That sounds like a very good idea indeed. Would also keep the
code free of #ifdef and reduce the maint effort for the out-of-tree. I
was trying udev earlier but failed. Will give such a symlink driver a
try.
Might be hard to get the deprecation warning into each access, but will
have to resort to probe-time instead of access-time.
regards,
Henning
On Thu, Aug 25, 2022 at 12:44 PM Henning Schild
<[email protected]> wrote:
> Subsequent patches will touch that file, apply some nice to have style
> changes before actually adding functional changes.
>
> Signed-off-by: Henning Schild <[email protected]>
Reviewed-by: Linus Walleij <[email protected]>
Yours,
Linus Walleij
On Thu, Aug 25, 2022 at 4:25 PM Hans de Goede <[email protected]> wrote:
>
> Hi,
>
> On 8/25/22 12:44, Henning Schild wrote:
> > changes since v5:
> > - adding patch to convert to pr_fmt
> > - adding patch to prefix macros with "f7188x_"
> > - rebased p1v4 to be p3v5 and added tag
> >
> > changes since v4:
> > - remove int case from a printk in p1
> > - include tags into commit messages
> >
> > changes since v3:
> > - update Kconfig as well
> > - drop chip names from comment in driver header
> > - add manufacturer check for Fintek again, Nuvoton not possible
> > - drop revision printing for Nuvoton
> > - restructure defines again
> > - add new model 427G
> >
> > changes since v2: (p1 only)
> > - rename macros that change behavior
> > - use chip type not device id in the macros
> > - reorder defines a bit
> >
> > changes since v1:
> > - remove unused define
> > - fix bug where (base + 2) was used as second data bit
> > - add macros for "inverted" and "single data bit"
> >
> > The first two patches apply some style refactorings before actual
> > functional changes are made.
> >
> > Later, This series 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.
> >
> > The very last patch enables a second model of that same board type.
> >
> > Henning Schild (7):
> > gpio-f7188x: switch over to using pr_fmt
> > gpio-f7188x: add a prefix to macros to keep gpio namespace clean
> > 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
> > platform/x86: simatic-ipc: add new model 427G
>
> So it looks like all these patches are ready for merging now,
> the only thing which is missing is an Ack from Pavel or
> one of the other LED people for patch 5/7.
>
> Pavel can have your ack for merging this through another tree
> please?
>
> So what is the plan for merging this?
>
> I see 2 options:
>
> Option a:
> 1. Merge the GPIO changes (patches 1-4) through the GPIO tree; and
> 2. Merge the leds + pdx86 changes through the pdx86 tree
>
> Option b:
> Merge everything through the pdx86 tree, and I will then provide
> an immutable branch + signed tag for other subsystems to pull
> (if they want to).
>
Hey! Sorry for the delay, I've just come back from vacation. I'm fine
with option b and to that end:
Acked-by: Bartosz Golaszewski <[email protected]>
Hi All,
On 8/25/22 12:44, Henning Schild wrote:
> changes since v5:
> - adding patch to convert to pr_fmt
> - adding patch to prefix macros with "f7188x_"
> - rebased p1v4 to be p3v5 and added tag
>
> changes since v4:
> - remove int case from a printk in p1
> - include tags into commit messages
>
> changes since v3:
> - update Kconfig as well
> - drop chip names from comment in driver header
> - add manufacturer check for Fintek again, Nuvoton not possible
> - drop revision printing for Nuvoton
> - restructure defines again
> - add new model 427G
>
> changes since v2: (p1 only)
> - rename macros that change behavior
> - use chip type not device id in the macros
> - reorder defines a bit
>
> changes since v1:
> - remove unused define
> - fix bug where (base + 2) was used as second data bit
> - add macros for "inverted" and "single data bit"
>
> The first two patches apply some style refactorings before actual
> functional changes are made.
>
> Later, This series 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.
>
> The very last patch enables a second model of that same board type.
>
> Henning Schild (7):
> gpio-f7188x: switch over to using pr_fmt
> gpio-f7188x: add a prefix to macros to keep gpio namespace clean
> 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
> platform/x86: simatic-ipc: add new model 427G
Despite the "leds: simatic-ipc-leds-gpio: add new model 227G" missing
an ack from the LED subsys maintainers I have decided to move forward
with this.
The LED changes in that patch are quite small / trivial, it actually
touches both drivers/leds and drivers/platform/x86 and the latter
changes are bigger and it also has been Reviewed by both me and Andy.
So I'm going for the it is easier to ask for forgiveness then ...
route here wrt the missing LEDs subsys ack.
I've just pushed a new platform-drivers-x86-simatec branch which
contains 6.0-rc1 + these 7 patches.
Next I'm to send out a pull-req (based on a signed tag)
to the involved subsys maintainers and merge that signed tag into
my review-hans branch (and from there it will go to pdx86/for-next).
Regards,
Hans
>
> drivers/gpio/Kconfig | 3 +-
> drivers/gpio/gpio-f7188x.c | 275 +++++++++++-------
> drivers/leds/simple/simatic-ipc-leds-gpio.c | 42 ++-
> drivers/platform/x86/simatic-ipc.c | 10 +-
> .../platform_data/x86/simatic-ipc-base.h | 1 +
> include/linux/platform_data/x86/simatic-ipc.h | 2 +
> 6 files changed, 216 insertions(+), 117 deletions(-)
>
Dear GPIO and LED subsystem maintainers,
Here is a pull-request for v6.0-rc1 + the
"[PATCH v6 0/7] add support for another simatic board" series
for merging into the gpio and leds subsystems.
Regards,
Hans
The following changes since commit 568035b01cfb107af8d2e4bd2fb9aea22cf5b868:
Linux 6.0-rc1 (2022-08-14 15:50:18 -0700)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git tags/platform-drivers-x86-simatec-1
for you to fetch changes up to 8f5c9858c5db129359b5de2f60f5f034bf5d56c0:
platform/x86: simatic-ipc: add new model 427G (2022-09-01 16:15:03 +0200)
----------------------------------------------------------------
Tag (immutable branch) for:
v6.0-rc1 + "[PATCH v6 0/7] add support for another simatic board" series
for merging into the gpio, leds and pdx86 subsystems.
----------------------------------------------------------------
Henning Schild (7):
gpio-f7188x: switch over to using pr_fmt
gpio-f7188x: add a prefix to macros to keep gpio namespace clean
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
platform/x86: simatic-ipc: add new model 427G
drivers/gpio/Kconfig | 3 +-
drivers/gpio/gpio-f7188x.c | 275 ++++++++++++---------
drivers/leds/simple/simatic-ipc-leds-gpio.c | 42 +++-
drivers/platform/x86/simatic-ipc.c | 10 +-
include/linux/platform_data/x86/simatic-ipc-base.h | 1 +
include/linux/platform_data/x86/simatic-ipc.h | 2 +
6 files changed, 216 insertions(+), 117 deletions(-)
On Thu, Sep 1, 2022 at 4:53 PM Hans de Goede <[email protected]> wrote:
>
> Dear GPIO and LED subsystem maintainers,
>
> Here is a pull-request for v6.0-rc1 + the
> "[PATCH v6 0/7] add support for another simatic board" series
> for merging into the gpio and leds subsystems.
>
> Regards,
>
> Hans
>
>
> The following changes since commit 568035b01cfb107af8d2e4bd2fb9aea22cf5b868:
>
> Linux 6.0-rc1 (2022-08-14 15:50:18 -0700)
>
> are available in the Git repository at:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git tags/platform-drivers-x86-simatec-1
>
> for you to fetch changes up to 8f5c9858c5db129359b5de2f60f5f034bf5d56c0:
>
> platform/x86: simatic-ipc: add new model 427G (2022-09-01 16:15:03 +0200)
>
> ----------------------------------------------------------------
> Tag (immutable branch) for:
> v6.0-rc1 + "[PATCH v6 0/7] add support for another simatic board" series
> for merging into the gpio, leds and pdx86 subsystems.
>
> ----------------------------------------------------------------
> Henning Schild (7):
> gpio-f7188x: switch over to using pr_fmt
> gpio-f7188x: add a prefix to macros to keep gpio namespace clean
> 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
> platform/x86: simatic-ipc: add new model 427G
>
> drivers/gpio/Kconfig | 3 +-
> drivers/gpio/gpio-f7188x.c | 275 ++++++++++++---------
> drivers/leds/simple/simatic-ipc-leds-gpio.c | 42 +++-
> drivers/platform/x86/simatic-ipc.c | 10 +-
> include/linux/platform_data/x86/simatic-ipc-base.h | 1 +
> include/linux/platform_data/x86/simatic-ipc.h | 2 +
> 6 files changed, 216 insertions(+), 117 deletions(-)
>
Pulled, thanks!
Bart