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"
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 | 193 ++++++++++--------
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, 157 insertions(+), 87 deletions(-)
--
2.35.1
So that drivers building on top can find those pins with GPIO_LOOKUP
helpers.
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 7b05ecc611e9..45b466b04070 100644
--- a/drivers/gpio/gpio-f7188x.c
+++ b/drivers/gpio/gpio-f7188x.c
@@ -149,10 +149,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, \
@@ -177,98 +177,98 @@ static int f7188x_gpio_set_config(struct gpio_chip *chip, unsigned offset,
#define 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
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 <[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.
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
Am Thu, 11 Aug 2022 17:39:04 +0200
schrieb Henning Schild <[email protected]>:
> 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"
>
> 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"
patches 1 and 2 are independent of those other patches and they add
value on their own, i would be happy if they got picked while waiting
for these other ones.
Henning
>
> 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 | 193
> ++++++++++-------- 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, 157 insertions(+), 87 deletions(-)
>
Hi,
On 8/11/22 17:39, Henning Schild 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.
>
> Signed-off-by: Henning Schild <[email protected]>
Thanks, patch looks good to me:
Reviewed-by: Hans de Goede <[email protected]>
Regards,
Hans
> ---
> 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)
Hi,
On 8/11/22 17:39, Henning Schild wrote:
> Just load the watchdog module, after having identified that machine.
> That watchdog module does not have any autoloading support.
>
> Signed-off-by: Henning Schild <[email protected]>
Thank you for your patch, I've applied this patch to my review-hans
branch:
https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans
Note it will show up in my review-hans branch once I've pushed my
local branch there, which might take a while.
Patches which are added to review-hans now are intended for
the next rc1. This branch will get rebased to the next rc1 when
it is out and after the rebasing the contents of review-hans
will be pushed to the platform-drivers-x86/for-next branch.
Regards,
Hans
> ---
> 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",
Am Fri, 12 Aug 2022 10:39:08 +0200
schrieb Andy Shevchenko <[email protected]>:
> On Thursday, August 11, 2022, Henning Schild
> <[email protected]> wrote:
>
> > So that drivers building on top can find those pins with GPIO_LOOKUP
> > helpers.
>
>
>
> Missed given tag. Do we need to bother reviewing your patches?
Sorry but i have no idea what you are talking about, please help me
out. Whatever i did miss seems to be pretty relevant it seems.
Henning
>
> > 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 7b05ecc611e9..45b466b04070 100644
> > --- a/drivers/gpio/gpio-f7188x.c
> > +++ b/drivers/gpio/gpio-f7188x.c
> > @@ -149,10 +149,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, \ @@ -177,98 +177,98 @@ static int
> > f7188x_gpio_set_config(struct gpio_chip *chip, unsigned offset,
> > #define 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
> >
> >
>
On Mon, Aug 22, 2022 at 4:21 PM Henning Schild
<[email protected]> wrote:
> Am Fri, 12 Aug 2022 10:39:08 +0200
> schrieb Andy Shevchenko <[email protected]>:
> > On Thursday, August 11, 2022, Henning Schild
> > <[email protected]> wrote:
> >
> > > So that drivers building on top can find those pins with GPIO_LOOKUP
> > > helpers.
> >
> > Missed given tag. Do we need to bother reviewing your patches?
>
> Sorry but i have no idea what you are talking about, please help me
> out. Whatever i did miss seems to be pretty relevant it seems.
If I remember correctly somebody gave you an Acked-by (or
Reviewed-by?) tag in previous versions of the series. I don't see it
included.
--
With Best Regards,
Andy Shevchenko
On Mon, Aug 22, 2022 at 11:37 PM Andy Shevchenko
<[email protected]> wrote:
> On Mon, Aug 22, 2022 at 4:21 PM Henning Schild
> <[email protected]> wrote:
> > Am Fri, 12 Aug 2022 10:39:08 +0200
> > schrieb Andy Shevchenko <[email protected]>:
> > > On Thursday, August 11, 2022, Henning Schild
> > > <[email protected]> wrote:
> > >
> > > > So that drivers building on top can find those pins with GPIO_LOOKUP
> > > > helpers.
> > >
> > > Missed given tag. Do we need to bother reviewing your patches?
> >
> > Sorry but i have no idea what you are talking about, please help me
> > out. Whatever i did miss seems to be pretty relevant it seems.
>
> If I remember correctly somebody gave you an Acked-by (or
> Reviewed-by?) tag in previous versions of the series. I don't see it
> included.
I think I added a Reviewed-by but it came in probably after this
version was posted due to me being slow on processing my
inbox, so this one is likely on me.
Yours,
Linus Walleij