diff mbox series

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

Message ID 20220511153905.13980-3-henning.schild@siemens.com
State Superseded
Headers show
Series simatic-ipc additions to p2sb apl lake gpio | expand

Commit Message

Henning Schild May 11, 2022, 3:39 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/watchdog/Kconfig           |  1 +
 drivers/watchdog/simatic-ipc-wdt.c | 15 ++++++++-------
 2 files changed, 9 insertions(+), 7 deletions(-)

Comments

Henning Schild May 12, 2022, 8:58 a.m. UTC | #1
Am Wed, 11 May 2022 11:03:04 -0700
schrieb Guenter Roeck <linux@roeck-us.net>:

> On 5/11/22 08:39, 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>
> > ---
> >   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 c4e82a8d863f..643a8f5a97b1 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  
> 
> Why "if X86" ? SIEMENS_SIMATIC_IPC already depends on it.

Thanks, will remove that.

> Also, I just noticed that P2SB is neither in mainline nor
> in linux-next, meaning this code won't even compile right now.
> That should be mentioned in the introduction e-mail (the use
> of "introduced" suggests that it is already there; you could
> just use "will be introduced" instead).

It was kind of in the cover letter, but maybe not verbose enough. Sorry
for the confusion. v1 was sent in-reply-to on top of P2SB, maybe i will
do that again for v3 but for sure point out the order in case i send it
without the reply.

Thanks,
Henning

> Thanks,
> Guenter
diff mbox series

Patch

diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index c4e82a8d863f..643a8f5a97b1 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))