[PATCH v3 0/4] simatic-ipc additions to p2sb apl lake gpio

[PATCH v3 0/4] simatic-ipc additions to p2sb apl lake gpio

[Henning Schild]

changed since v2:
 - put GPIO led module under same config switch
 - some style changes from review
 - init gpios with gpiod_get_index and drop gpiod_set_value
 - drop bogus "if" from watchdog Kconfig

changed since v1:
 - rebased
 - split p1 into p1-3

This switches the simatic-ipc modules to using the upcoming p2sb 
interface introduced by Andy with 
"platform/x86: introduce p2sb_bar() helper".
The series relies on these p2sb changes already applied and Andy will
likely take them into his series at some point.

It also switches to one apollo lake device to using gpio leds.

Henning Schild (4):
  leds: simatic-ipc-leds: convert to use P2SB accessor
  watchdog: simatic-ipc-wdt: convert to use P2SB accessor
  platform/x86: simatic-ipc: drop custom P2SB bar code
  leds: simatic-ipc-leds-gpio: add GPIO version of Siemens driver

 drivers/leds/simple/Kconfig                   |   6 +-
 drivers/leds/simple/Makefile                  |   1 +
 drivers/leds/simple/simatic-ipc-leds-gpio.c   | 105 ++++++++++++++++++
 drivers/leds/simple/simatic-ipc-leds.c        |  80 +------------
 drivers/platform/x86/simatic-ipc.c            |  43 +------
 drivers/watchdog/Kconfig                      |   1 +
 drivers/watchdog/simatic-ipc-wdt.c            |  15 +--
 .../platform_data/x86/simatic-ipc-base.h      |   2 -
 8 files changed, 126 insertions(+), 127 deletions(-)
 create mode 100644 drivers/leds/simple/simatic-ipc-leds-gpio.c

-- 
2.35.1

[PATCH v3 1/4] leds: simatic-ipc-leds: convert to use P2SB accessor

[Henning Schild]

Since we have a common P2SB accessor in tree we may use it instead of
open coded variants.

Replace custom code by p2sb_bar() call.

Signed-off-by: Henning Schild <henning.schild@siemens.com>
---
 drivers/leds/simple/Kconfig            |  1 +
 drivers/leds/simple/simatic-ipc-leds.c | 14 +++++++-------
 2 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/leds/simple/Kconfig b/drivers/leds/simple/Kconfig
index 9f6a68336659..bbf8cff3c3f6 100644
--- a/drivers/leds/simple/Kconfig
+++ b/drivers/leds/simple/Kconfig
@@ -3,6 +3,7 @@ config LEDS_SIEMENS_SIMATIC_IPC
 	tristate "LED driver for Siemens Simatic IPCs"
 	depends on LEDS_CLASS
 	depends on SIEMENS_SIMATIC_IPC
+	select P2SB
 	help
 	  This option enables support for the LEDs of several Industrial PCs
 	  from Siemens.
diff --git a/drivers/leds/simple/simatic-ipc-leds.c b/drivers/leds/simple/simatic-ipc-leds.c
index 078d43f5ba38..2e7597c143d8 100644
--- a/drivers/leds/simple/simatic-ipc-leds.c
+++ b/drivers/leds/simple/simatic-ipc-leds.c
@@ -15,6 +15,7 @@
 #include <linux/leds.h>
 #include <linux/module.h>
 #include <linux/pci.h>
+#include <linux/platform_data/x86/p2sb.h>
 #include <linux/platform_data/x86/simatic-ipc-base.h>
 #include <linux/platform_device.h>
 #include <linux/sizes.h>
@@ -38,8 +39,8 @@ static struct simatic_ipc_led simatic_ipc_leds_io[] = {
 	{ }
 };
 
-/* the actual start will be discovered with PCI, 0 is a placeholder */
-static struct resource simatic_ipc_led_mem_res = DEFINE_RES_MEM_NAMED(0, SZ_4K, KBUILD_MODNAME);
+/* the actual start will be discovered with p2sb, 0 is a placeholder */
+static struct resource simatic_ipc_led_mem_res = DEFINE_RES_MEM_NAMED(0, 0, KBUILD_MODNAME);
 
 static void __iomem *simatic_ipc_led_memory;
 
@@ -145,14 +146,13 @@ static int simatic_ipc_leds_probe(struct platform_device *pdev)
 		ipcled = simatic_ipc_leds_mem;
 		type = IORESOURCE_MEM;
 
-		/* get GPIO base from PCI */
-		res->start = simatic_ipc_get_membase0(PCI_DEVFN(13, 0));
-		if (res->start == 0)
-			return -ENODEV;
+		err = p2sb_bar(NULL, 0, res);
+		if (err)
+			return err;
 
 		/* do the final address calculation */
 		res->start = res->start + (0xC5 << 16);
-		res->end += res->start;
+		res->end = res->start + SZ_4K - 1;
 
 		simatic_ipc_led_memory = devm_ioremap_resource(dev, res);
 		if (IS_ERR(simatic_ipc_led_memory))
-- 
2.35.1

[PATCH v3 2/4] watchdog: simatic-ipc-wdt: convert to use P2SB accessor

[Henning Schild]

Since we have a common P2SB accessor in tree we may use it instead of
open coded variants.

Replace custom code by p2sb_bar() call.

Signed-off-by: Henning Schild <henning.schild@siemens.com>
---
 drivers/watchdog/Kconfig           |  1 +
 drivers/watchdog/simatic-ipc-wdt.c | 15 ++++++++-------
 2 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index a7cd3ef5b3d8..0097fbb455ab 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -1639,6 +1639,7 @@ config SIEMENS_SIMATIC_IPC_WDT
 	tristate "Siemens Simatic IPC Watchdog"
 	depends on SIEMENS_SIMATIC_IPC
 	select WATCHDOG_CORE
+	select P2SB
 	help
 	  This driver adds support for several watchdogs found in Industrial
 	  PCs from Siemens.
diff --git a/drivers/watchdog/simatic-ipc-wdt.c b/drivers/watchdog/simatic-ipc-wdt.c
index 8bac793c63fb..6599695dc672 100644
--- a/drivers/watchdog/simatic-ipc-wdt.c
+++ b/drivers/watchdog/simatic-ipc-wdt.c
@@ -16,6 +16,7 @@
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/pci.h>
+#include <linux/platform_data/x86/p2sb.h>
 #include <linux/platform_data/x86/simatic-ipc-base.h>
 #include <linux/platform_device.h>
 #include <linux/sizes.h>
@@ -54,9 +55,9 @@ static struct resource io_resource_trigger =
 	DEFINE_RES_IO_NAMED(WD_TRIGGER_IOADR, SZ_1,
 			    KBUILD_MODNAME " WD_TRIGGER_IOADR");
 
-/* the actual start will be discovered with pci, 0 is a placeholder */
+/* the actual start will be discovered with p2sb, 0 is a placeholder */
 static struct resource mem_resource =
-	DEFINE_RES_MEM_NAMED(0, SZ_4, "WD_RESET_BASE_ADR");
+	DEFINE_RES_MEM_NAMED(0, 0, "WD_RESET_BASE_ADR");
 
 static u32 wd_timeout_table[] = {2, 4, 6, 8, 16, 32, 48, 64 };
 static void __iomem *wd_reset_base_addr;
@@ -150,6 +151,7 @@ static int simatic_ipc_wdt_probe(struct platform_device *pdev)
 	struct simatic_ipc_platform *plat = pdev->dev.platform_data;
 	struct device *dev = &pdev->dev;
 	struct resource *res;
+	int ret;
 
 	switch (plat->devmode) {
 	case SIMATIC_IPC_DEVICE_227E:
@@ -190,15 +192,14 @@ static int simatic_ipc_wdt_probe(struct platform_device *pdev)
 	if (plat->devmode == SIMATIC_IPC_DEVICE_427E) {
 		res = &mem_resource;
 
-		/* get GPIO base from PCI */
-		res->start = simatic_ipc_get_membase0(PCI_DEVFN(0x1f, 1));
-		if (res->start == 0)
-			return -ENODEV;
+		ret = p2sb_bar(NULL, 0, res);
+		if (ret)
+			return ret;
 
 		/* do the final address calculation */
 		res->start = res->start + (GPIO_COMMUNITY0_PORT_ID << 16) +
 			     PAD_CFG_DW0_GPP_A_23;
-		res->end += res->start;
+		res->end = res->start + SZ_4 - 1;
 
 		wd_reset_base_addr = devm_ioremap_resource(dev, res);
 		if (IS_ERR(wd_reset_base_addr))
-- 
2.35.1

Re: [PATCH v3 2/4] watchdog: simatic-ipc-wdt: convert to use P2SB accessor

[Guenter Roeck]

On 5/13/22 01:36, Henning Schild wrote:
> Since we have a common P2SB accessor in tree we may use it instead of
> open coded variants.
> 
> Replace custom code by p2sb_bar() call.
> 
> Signed-off-by: Henning Schild <henning.schild@siemens.com>

Acked-by: Guenter Roeck <linux@roeck-us.net>

> ---
>   drivers/watchdog/Kconfig           |  1 +
>   drivers/watchdog/simatic-ipc-wdt.c | 15 ++++++++-------
>   2 files changed, 9 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index a7cd3ef5b3d8..0097fbb455ab 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -1639,6 +1639,7 @@ config SIEMENS_SIMATIC_IPC_WDT
>   	tristate "Siemens Simatic IPC Watchdog"
>   	depends on SIEMENS_SIMATIC_IPC
>   	select WATCHDOG_CORE
> +	select P2SB
>   	help
>   	  This driver adds support for several watchdogs found in Industrial
>   	  PCs from Siemens.
> diff --git a/drivers/watchdog/simatic-ipc-wdt.c b/drivers/watchdog/simatic-ipc-wdt.c
> index 8bac793c63fb..6599695dc672 100644
> --- a/drivers/watchdog/simatic-ipc-wdt.c
> +++ b/drivers/watchdog/simatic-ipc-wdt.c
> @@ -16,6 +16,7 @@
>   #include <linux/kernel.h>
>   #include <linux/module.h>
>   #include <linux/pci.h>
> +#include <linux/platform_data/x86/p2sb.h>
>   #include <linux/platform_data/x86/simatic-ipc-base.h>
>   #include <linux/platform_device.h>
>   #include <linux/sizes.h>
> @@ -54,9 +55,9 @@ static struct resource io_resource_trigger =
>   	DEFINE_RES_IO_NAMED(WD_TRIGGER_IOADR, SZ_1,
>   			    KBUILD_MODNAME " WD_TRIGGER_IOADR");
>   
> -/* the actual start will be discovered with pci, 0 is a placeholder */
> +/* the actual start will be discovered with p2sb, 0 is a placeholder */
>   static struct resource mem_resource =
> -	DEFINE_RES_MEM_NAMED(0, SZ_4, "WD_RESET_BASE_ADR");
> +	DEFINE_RES_MEM_NAMED(0, 0, "WD_RESET_BASE_ADR");
>   
>   static u32 wd_timeout_table[] = {2, 4, 6, 8, 16, 32, 48, 64 };
>   static void __iomem *wd_reset_base_addr;
> @@ -150,6 +151,7 @@ static int simatic_ipc_wdt_probe(struct platform_device *pdev)
>   	struct simatic_ipc_platform *plat = pdev->dev.platform_data;
>   	struct device *dev = &pdev->dev;
>   	struct resource *res;
> +	int ret;
>   
>   	switch (plat->devmode) {
>   	case SIMATIC_IPC_DEVICE_227E:
> @@ -190,15 +192,14 @@ static int simatic_ipc_wdt_probe(struct platform_device *pdev)
>   	if (plat->devmode == SIMATIC_IPC_DEVICE_427E) {
>   		res = &mem_resource;
>   
> -		/* get GPIO base from PCI */
> -		res->start = simatic_ipc_get_membase0(PCI_DEVFN(0x1f, 1));
> -		if (res->start == 0)
> -			return -ENODEV;
> +		ret = p2sb_bar(NULL, 0, res);
> +		if (ret)
> +			return ret;
>   
>   		/* do the final address calculation */
>   		res->start = res->start + (GPIO_COMMUNITY0_PORT_ID << 16) +
>   			     PAD_CFG_DW0_GPP_A_23;
> -		res->end += res->start;
> +		res->end = res->start + SZ_4 - 1;
>   
>   		wd_reset_base_addr = devm_ioremap_resource(dev, res);
>   		if (IS_ERR(wd_reset_base_addr))

[PATCH v3 3/4] platform/x86: simatic-ipc: drop custom P2SB bar code

[Henning Schild]

The two drivers that used to use this have been switched over to the
common P2SB accessor, so this code is not needed any longer.

Signed-off-by: Henning Schild <henning.schild@siemens.com>
---
 drivers/platform/x86/simatic-ipc.c            | 38 -------------------
 .../platform_data/x86/simatic-ipc-base.h      |  2 -
 2 files changed, 40 deletions(-)

diff --git a/drivers/platform/x86/simatic-ipc.c b/drivers/platform/x86/simatic-ipc.c
index b599cda5ba3c..26c35e1660cb 100644
--- a/drivers/platform/x86/simatic-ipc.c
+++ b/drivers/platform/x86/simatic-ipc.c
@@ -101,44 +101,6 @@ static int register_platform_devices(u32 station_id)
 	return 0;
 }
 
-/* FIXME: this should eventually be done with generic P2SB discovery code
- * the individual drivers for watchdogs and LEDs access memory that implements
- * GPIO, but pinctrl will not come up because of missing ACPI entries
- *
- * While there is no conflict a cleaner solution would be to somehow bring up
- * pinctrl even with these ACPI entries missing, and base the drivers on pinctrl.
- * After which the following function could be dropped, together with the code
- * poking the memory.
- */
-/*
- * Get membase address from PCI, used in leds and wdt module. Here we read
- * the bar0. The final address calculation is done in the appropriate modules
- */
-u32 simatic_ipc_get_membase0(unsigned int p2sb)
-{
-	struct pci_bus *bus;
-	u32 bar0 = 0;
-	/*
-	 * The GPIO memory is in bar0 of the hidden P2SB device.
-	 * Unhide the device to have a quick look at it, before we hide it
-	 * again.
-	 * Also grab the pci rescan lock so that device does not get discovered
-	 * and remapped while it is visible.
-	 * This code is inspired by drivers/mfd/lpc_ich.c
-	 */
-	bus = pci_find_bus(0, 0);
-	pci_lock_rescan_remove();
-	pci_bus_write_config_byte(bus, p2sb, 0xE1, 0x0);
-	pci_bus_read_config_dword(bus, p2sb, PCI_BASE_ADDRESS_0, &bar0);
-
-	bar0 &= ~0xf;
-	pci_bus_write_config_byte(bus, p2sb, 0xE1, 0x1);
-	pci_unlock_rescan_remove();
-
-	return bar0;
-}
-EXPORT_SYMBOL(simatic_ipc_get_membase0);
-
 static int __init simatic_ipc_init_module(void)
 {
 	const struct dmi_system_id *match;
diff --git a/include/linux/platform_data/x86/simatic-ipc-base.h b/include/linux/platform_data/x86/simatic-ipc-base.h
index 62d2bc774067..39fefd48cf4d 100644
--- a/include/linux/platform_data/x86/simatic-ipc-base.h
+++ b/include/linux/platform_data/x86/simatic-ipc-base.h
@@ -24,6 +24,4 @@ struct simatic_ipc_platform {
 	u8	devmode;
 };
 
-u32 simatic_ipc_get_membase0(unsigned int p2sb);
-
 #endif /* __PLATFORM_DATA_X86_SIMATIC_IPC_BASE_H */
-- 
2.35.1

Re: [PATCH v3 3/4] platform/x86: simatic-ipc: drop custom P2SB bar code

[Andy Shevchenko]

On Fri, May 13, 2022 at 10:36:51AM +0200, Henning Schild wrote:
> The two drivers that used to use this have been switched over to the
> common P2SB accessor, so this code is not needed any longer.

Hans, in order to minimize the risk of not getting this into next cycle, I
would like to collect your tag (if you have no objection) and then, when I send
a new version of the P2SB series with this series together, we would everything
ready to be taken by Lee.

-- 
With Best Regards,
Andy Shevchenko

[PATCH v3 4/4] leds: simatic-ipc-leds-gpio: add GPIO version of Siemens driver

[Henning Schild]

On Apollo Lake the pinctrl drivers will now come up without ACPI. Use
that instead of open coding it.
Create a new driver for that which can later be filled with more GPIO
based models, and which has different dependencies.

Signed-off-by: Henning Schild <henning.schild@siemens.com>
---
 drivers/leds/simple/Kconfig                 |   7 +-
 drivers/leds/simple/Makefile                |   1 +
 drivers/leds/simple/simatic-ipc-leds-gpio.c | 105 ++++++++++++++++++++
 drivers/leds/simple/simatic-ipc-leds.c      |  80 +--------------
 drivers/platform/x86/simatic-ipc.c          |   5 +-
 5 files changed, 117 insertions(+), 81 deletions(-)
 create mode 100644 drivers/leds/simple/simatic-ipc-leds-gpio.c

diff --git a/drivers/leds/simple/Kconfig b/drivers/leds/simple/Kconfig
index bbf8cff3c3f6..fd2b8225d926 100644
--- a/drivers/leds/simple/Kconfig
+++ b/drivers/leds/simple/Kconfig
@@ -1,12 +1,11 @@
 # SPDX-License-Identifier: GPL-2.0-only
 config LEDS_SIEMENS_SIMATIC_IPC
 	tristate "LED driver for Siemens Simatic IPCs"
-	depends on LEDS_CLASS
+	depends on LEDS_GPIO
 	depends on SIEMENS_SIMATIC_IPC
-	select P2SB
 	help
 	  This option enables support for the LEDs of several Industrial PCs
 	  from Siemens.
 
-	  To compile this driver as a module, choose M here: the module
-	  will be called simatic-ipc-leds.
+	  To compile this driver as a module, choose M here: the modules
+	  will be called simatic-ipc-leds and simatic-ipc-leds-gpio.
diff --git a/drivers/leds/simple/Makefile b/drivers/leds/simple/Makefile
index 8481f1e9e360..1c7ef5e1324b 100644
--- a/drivers/leds/simple/Makefile
+++ b/drivers/leds/simple/Makefile
@@ -1,2 +1,3 @@
 # SPDX-License-Identifier: GPL-2.0
 obj-$(CONFIG_LEDS_SIEMENS_SIMATIC_IPC)	+= simatic-ipc-leds.o
+obj-$(CONFIG_LEDS_SIEMENS_SIMATIC_IPC)	+= simatic-ipc-leds-gpio.o
diff --git a/drivers/leds/simple/simatic-ipc-leds-gpio.c b/drivers/leds/simple/simatic-ipc-leds-gpio.c
new file mode 100644
index 000000000000..4c9e663a90ba
--- /dev/null
+++ b/drivers/leds/simple/simatic-ipc-leds-gpio.c
@@ -0,0 +1,105 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Siemens SIMATIC IPC driver for GPIO based LEDs
+ *
+ * Copyright (c) Siemens AG, 2022
+ *
+ * Authors:
+ *  Henning Schild <henning.schild@siemens.com>
+ */
+
+#include <linux/gpio/machine.h>
+#include <linux/gpio/consumer.h>
+#include <linux/leds.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+
+static struct gpiod_lookup_table simatic_ipc_led_gpio_table = {
+	.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", 56, NULL, 6, GPIO_ACTIVE_LOW),
+		GPIO_LOOKUP_IDX("apollolake-pinctrl.0", 59, 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" },
+};
+
+static const struct gpio_led_platform_data simatic_ipc_gpio_leds_pdata = {
+	.num_leds	= ARRAY_SIZE(simatic_ipc_gpio_leds),
+	.leds		= simatic_ipc_gpio_leds,
+};
+
+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);
+	platform_device_unregister(simatic_leds_pdev);
+
+	return 0;
+}
+
+static int simatic_ipc_leds_gpio_probe(struct platform_device *pdev)
+{
+	struct gpio_desc *gpiod;
+	int err;
+
+	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,
+		sizeof(simatic_ipc_gpio_leds_pdata));
+	if (IS_ERR(simatic_leds_pdev)) {
+		err = PTR_ERR(simatic_leds_pdev);
+		goto out;
+	}
+
+	/* PM_BIOS_BOOT_N */
+	gpiod = gpiod_get_index(&simatic_leds_pdev->dev, NULL, 6, GPIOD_OUT_LOW);
+	if (IS_ERR(gpiod)) {
+		err = PTR_ERR(gpiod);
+		goto out;
+	}
+	gpiod_put(gpiod);
+
+	/* PM_WDT_OUT */
+	gpiod = gpiod_get_index(&simatic_leds_pdev->dev, NULL, 7, GPIOD_OUT_LOW);
+	if (IS_ERR(gpiod)) {
+		err = PTR_ERR(gpiod);
+		goto out;
+	}
+	gpiod_put(gpiod);
+
+	return 0;
+out:
+	simatic_ipc_leds_gpio_remove(pdev);
+
+	return err;
+}
+
+static struct platform_driver simatic_ipc_led_gpio_driver = {
+	.probe = simatic_ipc_leds_gpio_probe,
+	.remove = simatic_ipc_leds_gpio_remove,
+	.driver = {
+		.name = KBUILD_MODNAME,
+	}
+};
+module_platform_driver(simatic_ipc_led_gpio_driver);
+
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:" KBUILD_MODNAME);
+MODULE_SOFTDEP("pre: platform:leds-gpio");
+MODULE_AUTHOR("Henning Schild <henning.schild@siemens.com>");
diff --git a/drivers/leds/simple/simatic-ipc-leds.c b/drivers/leds/simple/simatic-ipc-leds.c
index 2e7597c143d8..4894c228c165 100644
--- a/drivers/leds/simple/simatic-ipc-leds.c
+++ b/drivers/leds/simple/simatic-ipc-leds.c
@@ -15,7 +15,6 @@
 #include <linux/leds.h>
 #include <linux/module.h>
 #include <linux/pci.h>
-#include <linux/platform_data/x86/p2sb.h>
 #include <linux/platform_data/x86/simatic-ipc-base.h>
 #include <linux/platform_device.h>
 #include <linux/sizes.h>
@@ -24,7 +23,7 @@
 #define SIMATIC_IPC_LED_PORT_BASE	0x404E
 
 struct simatic_ipc_led {
-	unsigned int value; /* mask for io and offset for mem */
+	unsigned int value; /* mask for io */
 	char *name;
 	struct led_classdev cdev;
 };
@@ -39,21 +38,6 @@ static struct simatic_ipc_led simatic_ipc_leds_io[] = {
 	{ }
 };
 
-/* the actual start will be discovered with p2sb, 0 is a placeholder */
-static struct resource simatic_ipc_led_mem_res = DEFINE_RES_MEM_NAMED(0, 0, KBUILD_MODNAME);
-
-static void __iomem *simatic_ipc_led_memory;
-
-static struct simatic_ipc_led simatic_ipc_leds_mem[] = {
-	{0x500 + 0x1A0, "red:" LED_FUNCTION_STATUS "-1"},
-	{0x500 + 0x1A8, "green:" LED_FUNCTION_STATUS "-1"},
-	{0x500 + 0x1C8, "red:" LED_FUNCTION_STATUS "-2"},
-	{0x500 + 0x1D0, "green:" LED_FUNCTION_STATUS "-2"},
-	{0x500 + 0x1E0, "red:" LED_FUNCTION_STATUS "-3"},
-	{0x500 + 0x198, "green:" LED_FUNCTION_STATUS "-3"},
-	{ }
-};
-
 static struct resource simatic_ipc_led_io_res =
 	DEFINE_RES_IO_NAMED(SIMATIC_IPC_LED_PORT_BASE, SZ_2, KBUILD_MODNAME);
 
@@ -89,28 +73,6 @@ static enum led_brightness simatic_ipc_led_get_io(struct led_classdev *led_cd)
 	return inw(SIMATIC_IPC_LED_PORT_BASE) & led->value ? LED_OFF : led_cd->max_brightness;
 }
 
-static void simatic_ipc_led_set_mem(struct led_classdev *led_cd,
-				    enum led_brightness brightness)
-{
-	struct simatic_ipc_led *led = cdev_to_led(led_cd);
-	void __iomem *reg = simatic_ipc_led_memory + led->value;
-	u32 val;
-
-	val = readl(reg);
-	val = (val & ~1) | (brightness == LED_OFF);
-	writel(val, reg);
-}
-
-static enum led_brightness simatic_ipc_led_get_mem(struct led_classdev *led_cd)
-{
-	struct simatic_ipc_led *led = cdev_to_led(led_cd);
-	void __iomem *reg = simatic_ipc_led_memory + led->value;
-	u32 val;
-
-	val = readl(reg);
-	return (val & 1) ? LED_OFF : led_cd->max_brightness;
-}
-
 static int simatic_ipc_leds_probe(struct platform_device *pdev)
 {
 	const struct simatic_ipc_platform *plat = pdev->dev.platform_data;
@@ -118,9 +80,7 @@ static int simatic_ipc_leds_probe(struct platform_device *pdev)
 	struct simatic_ipc_led *ipcled;
 	struct led_classdev *cdev;
 	struct resource *res;
-	void __iomem *reg;
-	int err, type;
-	u32 val;
+	int err;
 
 	switch (plat->devmode) {
 	case SIMATIC_IPC_DEVICE_227D:
@@ -135,51 +95,19 @@ static int simatic_ipc_leds_probe(struct platform_device *pdev)
 			}
 			ipcled = simatic_ipc_leds_io;
 		}
-		type = IORESOURCE_IO;
 		if (!devm_request_region(dev, res->start, resource_size(res), KBUILD_MODNAME)) {
 			dev_err(dev, "Unable to register IO resource at %pR\n", res);
 			return -EBUSY;
 		}
 		break;
-	case SIMATIC_IPC_DEVICE_127E:
-		res = &simatic_ipc_led_mem_res;
-		ipcled = simatic_ipc_leds_mem;
-		type = IORESOURCE_MEM;
-
-		err = p2sb_bar(NULL, 0, res);
-		if (err)
-			return err;
-
-		/* do the final address calculation */
-		res->start = res->start + (0xC5 << 16);
-		res->end = res->start + SZ_4K - 1;
-
-		simatic_ipc_led_memory = devm_ioremap_resource(dev, res);
-		if (IS_ERR(simatic_ipc_led_memory))
-			return PTR_ERR(simatic_ipc_led_memory);
-
-		/* initialize power/watchdog LED */
-		reg = simatic_ipc_led_memory + 0x500 + 0x1D8; /* PM_WDT_OUT */
-		val = readl(reg);
-		writel(val & ~1, reg);
-
-		reg = simatic_ipc_led_memory + 0x500 + 0x1C0; /* PM_BIOS_BOOT_N */
-		val = readl(reg);
-		writel(val | 1, reg);
-		break;
 	default:
 		return -ENODEV;
 	}
 
 	while (ipcled->value) {
 		cdev = &ipcled->cdev;
-		if (type == IORESOURCE_MEM) {
-			cdev->brightness_set = simatic_ipc_led_set_mem;
-			cdev->brightness_get = simatic_ipc_led_get_mem;
-		} else {
-			cdev->brightness_set = simatic_ipc_led_set_io;
-			cdev->brightness_get = simatic_ipc_led_get_io;
-		}
+		cdev->brightness_set = simatic_ipc_led_set_io;
+		cdev->brightness_get = simatic_ipc_led_get_io;
 		cdev->max_brightness = LED_ON;
 		cdev->name = ipcled->name;
 
diff --git a/drivers/platform/x86/simatic-ipc.c b/drivers/platform/x86/simatic-ipc.c
index 26c35e1660cb..ca3647b751d5 100644
--- a/drivers/platform/x86/simatic-ipc.c
+++ b/drivers/platform/x86/simatic-ipc.c
@@ -51,6 +51,7 @@ static int register_platform_devices(u32 station_id)
 {
 	u8 ledmode = SIMATIC_IPC_DEVICE_NONE;
 	u8 wdtmode = SIMATIC_IPC_DEVICE_NONE;
+	char *pdevname = KBUILD_MODNAME "_leds";
 	int i;
 
 	platform_data.devmode = SIMATIC_IPC_DEVICE_NONE;
@@ -64,10 +65,12 @@ static int register_platform_devices(u32 station_id)
 	}
 
 	if (ledmode != SIMATIC_IPC_DEVICE_NONE) {
+		if (ledmode == SIMATIC_IPC_DEVICE_127E)
+			pdevname = KBUILD_MODNAME "_leds_gpio";
 		platform_data.devmode = ledmode;
 		ipc_led_platform_device =
 			platform_device_register_data(NULL,
-				KBUILD_MODNAME "_leds", PLATFORM_DEVID_NONE,
+				pdevname, PLATFORM_DEVID_NONE,
 				&platform_data,
 				sizeof(struct simatic_ipc_platform));
 		if (IS_ERR(ipc_led_platform_device))
-- 
2.35.1

Re: [PATCH v3 0/4] simatic-ipc additions to p2sb apl lake gpio

[Andy Shevchenko]

On Fri, May 13, 2022 at 10:36:48AM +0200, Henning Schild wrote:
> changed since v2:
>  - put GPIO led module under same config switch
>  - some style changes from review
>  - init gpios with gpiod_get_index and drop gpiod_set_value
>  - drop bogus "if" from watchdog Kconfig
> 
> changed since v1:
>  - rebased
>  - split p1 into p1-3
> 
> This switches the simatic-ipc modules to using the upcoming p2sb 
> interface introduced by Andy with 
> "platform/x86: introduce p2sb_bar() helper".
> The series relies on these p2sb changes already applied and Andy will
> likely take them into his series at some point.
> 
> It also switches to one apollo lake device to using gpio leds.

Hmm... This series misses LEDS maintainer. Okay, I will include them when
sending v6 after v5.19-rc1.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 0/4] simatic-ipc additions to p2sb apl lake gpio

[Andy Shevchenko]

On Mon, May 23, 2022 at 07:24:08PM +0300, Andy Shevchenko wrote:
> On Fri, May 13, 2022 at 10:36:48AM +0200, Henning Schild wrote:
> > changed since v2:
> >  - put GPIO led module under same config switch
> >  - some style changes from review
> >  - init gpios with gpiod_get_index and drop gpiod_set_value
> >  - drop bogus "if" from watchdog Kconfig
> > 
> > changed since v1:
> >  - rebased
> >  - split p1 into p1-3
> > 
> > This switches the simatic-ipc modules to using the upcoming p2sb 
> > interface introduced by Andy with 
> > "platform/x86: introduce p2sb_bar() helper".
> > The series relies on these p2sb changes already applied and Andy will
> > likely take them into his series at some point.
> > 
> > It also switches to one apollo lake device to using gpio leds.
> 
> Hmm... This series misses LEDS maintainer. Okay, I will include them when
> sending v6 after v5.19-rc1.

I have sent v6 [1], please make sure everything is okay.

[1]: https://lore.kernel.org/platform-driver-x86/20220606164138.66535-1-andriy.shevchenko@linux.intel.com/

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 0/4] simatic-ipc additions to p2sb apl lake gpio

[Henning Schild]

Am Tue, 7 Jun 2022 18:18:05 +0300
schrieb Andy Shevchenko <andriy.shevchenko@linux.intel.com>:

> On Mon, May 23, 2022 at 07:24:08PM +0300, Andy Shevchenko wrote:
> > On Fri, May 13, 2022 at 10:36:48AM +0200, Henning Schild wrote:  
> > > changed since v2:
> > >  - put GPIO led module under same config switch
> > >  - some style changes from review
> > >  - init gpios with gpiod_get_index and drop gpiod_set_value
> > >  - drop bogus "if" from watchdog Kconfig
> > > 
> > > changed since v1:
> > >  - rebased
> > >  - split p1 into p1-3
> > > 
> > > This switches the simatic-ipc modules to using the upcoming p2sb 
> > > interface introduced by Andy with 
> > > "platform/x86: introduce p2sb_bar() helper".
> > > The series relies on these p2sb changes already applied and Andy
> > > will likely take them into his series at some point.
> > > 
> > > It also switches to one apollo lake device to using gpio leds.  
> > 
> > Hmm... This series misses LEDS maintainer. Okay, I will include
> > them when sending v6 after v5.19-rc1.  
> 
> I have sent v6 [1], please make sure everything is okay.

I received that and had a quick look over my bits. That all looked like
i sent it so i have no reason to doubt it would be working as expected.

In the coming two weeks i will not be able to get my hands on hardware,
so can not test but am pretty sure it is not needed another time.

regards,
Henning
 
> [1]:
> https://lore.kernel.org/platform-driver-x86/20220606164138.66535-1-andriy.shevchenko@linux.intel.com/