diff mbox series

[1/2] simatic-ipc: convert to use common P2SB accessor

Message ID 20220308193522.26696-2-henning.schild@siemens.com
State New
Headers show
Series simatic-ipc additions to p2sb apl lake gpio | expand

Commit Message

Henning Schild March 8, 2022, 7:35 p.m. UTC
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 +++----
 drivers/platform/x86/simatic-ipc.c            | 38 -------------------
 drivers/watchdog/Kconfig                      |  1 +
 drivers/watchdog/simatic-ipc-wdt.c            | 15 ++++----
 .../platform_data/x86/simatic-ipc-base.h      |  2 -
 6 files changed, 17 insertions(+), 54 deletions(-)

Comments

Henning Schild March 8, 2022, 8:09 p.m. UTC | #1
Am Tue, 8 Mar 2022 20:35:21 +0100
schrieb Henning Schild <henning.schild@siemens.com>:

> 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 +++----
>  drivers/platform/x86/simatic-ipc.c            | 38
> ------------------- drivers/watchdog/Kconfig                      |
> 1 + drivers/watchdog/simatic-ipc-wdt.c            | 15 ++++----
>  .../platform_data/x86/simatic-ipc-base.h      |  2 -
>  6 files changed, 17 insertions(+), 54 deletions(-)
> 
> diff --git a/drivers/leds/simple/Kconfig b/drivers/leds/simple/Kconfig
> index 9f6a68336659..9293e6b36c75 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 if X86
>  	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
> ff2c96e73241..215ef5b74236 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
> */ -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 */ +struct resource simatic_ipc_led_mem_res
> = DEFINE_RES_MEM_NAMED(0, 0, KBUILD_MODNAME); 
>  static void *simatic_ipc_led_memory;
>  
> @@ -143,14 +144,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,

After "mfd: lpc_ich: Add support for pinctrl in non-ACPI system" this
will fail because the region will be claimed by pinctrl, did not check
if it would work with !CONFIG_LPC_SCH.

But not a big deal and was expected to happen. I do have a version that
will devm_ioremap (no region) but that seems too hacky to put here.
After all the next patch will remove all that and switch to GPIO.

If we have to propose things in two series i propose to make
CONFIG_LPC_SCH and CONFIG_LEDS_SIEMENS_SIMATIC_IPC conflicting for
intermediate steps.

regards,
Henning

> res); if (IS_ERR(simatic_ipc_led_memory))
> 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/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index c8fa79da23b3..ce44a942fc68 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -1628,6 +1628,7 @@ config SIEMENS_SIMATIC_IPC_WDT
>  	tristate "Siemens Simatic IPC Watchdog"
>  	depends on SIEMENS_SIMATIC_IPC
>  	select WATCHDOG_CORE
> +	select P2SB if X86
>  	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))
> 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 */
diff mbox series

Patch

diff --git a/drivers/leds/simple/Kconfig b/drivers/leds/simple/Kconfig
index 9f6a68336659..9293e6b36c75 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 if X86
 	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 ff2c96e73241..215ef5b74236 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 */
-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 */
+struct resource simatic_ipc_led_mem_res = DEFINE_RES_MEM_NAMED(0, 0, KBUILD_MODNAME);
 
 static void *simatic_ipc_led_memory;
 
@@ -143,14 +144,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))
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/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index c8fa79da23b3..ce44a942fc68 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -1628,6 +1628,7 @@  config SIEMENS_SIMATIC_IPC_WDT
 	tristate "Siemens Simatic IPC Watchdog"
 	depends on SIEMENS_SIMATIC_IPC
 	select WATCHDOG_CORE
+	select P2SB if X86
 	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))
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 */