[PATCHv3 0/2] watchdog: pika_wdt: portability and timer lifecycle fixes

[PATCHv3 0/2] watchdog: pika_wdt: portability and timer lifecycle fixes

[Rosen Penev]

Fix two timer lifecycle bugs in the PIKA watchdog driver:

 - pikawdt_release() could race with a concurrent open() due to
   clearing the open bit before stopping the timer.
 - pikawdt_exit() unmapped FPGA memory without stopping the ping
   timer first, potentially executing the handler on freed memory.

Then replace powerpc-specific I/O accessors with generic ones to
allow COMPILE_TEST builds on other architectures.

v3: fix sashiko reported issues
v2: fix preexisting issues

Rosen Penev (2):
  watchdog: pika_wdt: fix timer lifecycle bugs
  watchdog: pika_wdt: enable COMPILE_TEST builds

 drivers/watchdog/Kconfig    |  3 ++-
 drivers/watchdog/pika_wdt.c | 12 +++++++-----
 2 files changed, 9 insertions(+), 6 deletions(-)

--
2.54.0

[PATCHv3 1/2] watchdog: pika_wdt: fix timer lifecycle bugs

[Rosen Penev]

Two timer lifecycle bugs in the PIKA watchdog driver:

1. pikawdt_release() clears the open bit before stopping the timer,
   allowing a concurrent pikawdt_open() to start a new timer that
   is then immediately deleted.  Move clear_bit after the timer
   stop.  Use timer_shutdown_sync() so the handler cannot re-arm
   after the call returns.  Add timer_setup() in pikawdt_open()
   to re-initialize the timer for the next open.

2. pikawdt_exit() unmaps FPGA IO memory without first stopping the
   ping timer.  If the timer handler fires after iounmap(), it
   executes pikawdt_ping() on freed memory.  Use timer_shutdown_sync()
   which permanently prevents re-arming.

Assisted-by: opencode:big-pickle
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/watchdog/pika_wdt.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/watchdog/pika_wdt.c b/drivers/watchdog/pika_wdt.c
index 87b8988d2520..d567103ec5e4 100644
--- a/drivers/watchdog/pika_wdt.c
+++ b/drivers/watchdog/pika_wdt.c
@@ -117,6 +117,7 @@ static int pikawdt_open(struct inode *inode, struct file *file)
 	if (test_and_set_bit(0, &pikawdt_private.open))
 		return -EBUSY;

+	timer_setup(&pikawdt_private.timer, pikawdt_ping, 0);
 	pikawdt_start();

 	return stream_open(inode, file);
@@ -129,7 +130,7 @@ static int pikawdt_release(struct inode *inode, struct file *file)
 {
 	/* stop internal ping */
 	if (!pikawdt_private.expect_close)
-		timer_delete(&pikawdt_private.timer);
+		timer_shutdown_sync(&pikawdt_private.timer);

 	clear_bit(0, &pikawdt_private.open);
 	pikawdt_private.expect_close = 0;
@@ -291,6 +292,7 @@ static void __exit pikawdt_exit(void)
 {
 	misc_deregister(&pikawdt_miscdev);

+	timer_shutdown_sync(&pikawdt_private.timer);
 	iounmap(pikawdt_private.fpga);
 }

--
2.54.0

Re: [PATCHv3 1/2] watchdog: pika_wdt: fix timer lifecycle bugs

[Guenter Roeck]

On 6/8/26 14:04, Rosen Penev wrote:
> Two timer lifecycle bugs in the PIKA watchdog driver:
> 
> 1. pikawdt_release() clears the open bit before stopping the timer,
>     allowing a concurrent pikawdt_open() to start a new timer that
>     is then immediately deleted.  Move clear_bit after the timer
>     stop.  Use timer_shutdown_sync() so the handler cannot re-arm
>     after the call returns.  Add timer_setup() in pikawdt_open()
>     to re-initialize the timer for the next open.

The clear bit is not moved, and the approach is wrong. More on that below.

> 
> 2. pikawdt_exit() unmaps FPGA IO memory without first stopping the
>     ping timer.  If the timer handler fires after iounmap(), it
>     executes pikawdt_ping() on freed memory.  Use timer_shutdown_sync()
>     which permanently prevents re-arming.
> 
> Assisted-by: opencode:big-pickle
> Signed-off-by: Rosen Penev <rosenp@gmail.com>
> ---
>   drivers/watchdog/pika_wdt.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/watchdog/pika_wdt.c b/drivers/watchdog/pika_wdt.c
> index 87b8988d2520..d567103ec5e4 100644
> --- a/drivers/watchdog/pika_wdt.c
> +++ b/drivers/watchdog/pika_wdt.c
> @@ -117,6 +117,7 @@ static int pikawdt_open(struct inode *inode, struct file *file)
>   	if (test_and_set_bit(0, &pikawdt_private.open))
>   		return -EBUSY;
> 
> +	timer_setup(&pikawdt_private.timer, pikawdt_ping, 0);

Calling timer_setup() in the open function is wrong because the
timer may already be running.

>   	pikawdt_start();
> 
>   	return stream_open(inode, file);
> @@ -129,7 +130,7 @@ static int pikawdt_release(struct inode *inode, struct file *file)
>   {
>   	/* stop internal ping */
>   	if (!pikawdt_private.expect_close)
> -		timer_delete(&pikawdt_private.timer);
> +		timer_shutdown_sync(&pikawdt_private.timer);

This is wrong here. timer_shutdown_sync() is only supposed to be
called if the timer will not be re-enabled. Anything else would be
an API violation.

> 
>   	clear_bit(0, &pikawdt_private.open);
>   	pikawdt_private.expect_close = 0;
> @@ -291,6 +292,7 @@ static void __exit pikawdt_exit(void)
>   {
>   	misc_deregister(&pikawdt_miscdev);
> 
> +	timer_shutdown_sync(&pikawdt_private.timer);
>   	iounmap(pikawdt_private.fpga);
>   }
> 
> --
> 2.54.0
> 
> 

[PATCHv3 2/2] watchdog: pika_wdt: enable COMPILE_TEST builds

[Rosen Penev]

Replace powerpc-specific I/O accessors (in_be32/out_be32) with
generic ones (ioread32be/iowrite32be) so the driver can be
compile-tested on other architectures.  Relax the Kconfig
dependency accordingly and add the required HAS_IOMEM.

Assisted-by: opencode:big-pickle
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/watchdog/Kconfig    | 3 ++-
 drivers/watchdog/pika_wdt.c | 8 ++++----
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 8319c503319a..855736eaca2c 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -2040,7 +2040,8 @@ config 8xxx_WDT

 config PIKA_WDT
 	tristate "PIKA FPGA Watchdog"
-	depends on WARP || (PPC64 && COMPILE_TEST)
+	depends on WARP || COMPILE_TEST
+	depends on HAS_IOMEM
 	default WARP
 	help
 	  This enables the watchdog in the PIKA FPGA. Currently used on
diff --git a/drivers/watchdog/pika_wdt.c b/drivers/watchdog/pika_wdt.c
index d567103ec5e4..e599aff8329b 100644
--- a/drivers/watchdog/pika_wdt.c
+++ b/drivers/watchdog/pika_wdt.c
@@ -77,10 +77,10 @@ static inline void pikawdt_reset(void)
 	 *           seconds. Valid ranges are 1 to 15 seconds. The value can
 	 *           be modified dynamically.
 	 */
-	unsigned reset = in_be32(pikawdt_private.fpga + 0x14);
+	unsigned reset = ioread32be(pikawdt_private.fpga + 0x14);
 	/* enable with max timeout - 15 seconds */
 	reset |= (1 << 7) + (WDT_HW_TIMEOUT << 8);
-	out_be32(pikawdt_private.fpga + 0x14, reset);
+	iowrite32be(reset, pikawdt_private.fpga + 0x14);
 }

 /*
@@ -243,7 +243,7 @@ static int __init pikawdt_init(void)
 		return -ENOMEM;
 	}

-	ident.firmware_version = in_be32(pikawdt_private.fpga + 0x1c) & 0xffff;
+	ident.firmware_version = ioread32be(pikawdt_private.fpga + 0x1c) & 0xffff;

 	/* POST information is in the sd area. */
 	np = of_find_compatible_node(NULL, NULL, "pika,fpga-sd");
@@ -265,7 +265,7 @@ static int __init pikawdt_init(void)
 	 * Bit 31,   WDOG: Set to 1 when the last reset was caused by a watchdog
 	 *           timeout.
 	 */
-	post1 = in_be32(fpga + 0x40);
+	post1 = ioread32be(fpga + 0x40);
 	if (post1 & 0x80000000)
 		pikawdt_private.bootstatus = WDIOF_CARDRESET;

--
2.54.0

Re: [PATCHv3 2/2] watchdog: pika_wdt: enable COMPILE_TEST builds

[Guenter Roeck]

On 6/8/26 14:04, Rosen Penev wrote:
> Replace powerpc-specific I/O accessors (in_be32/out_be32) with
> generic ones (ioread32be/iowrite32be) so the driver can be
> compile-tested on other architectures.  Relax the Kconfig
> dependency accordingly and add the required HAS_IOMEM.
> 
> Assisted-by: opencode:big-pickle
> Signed-off-by: Rosen Penev <rosenp@gmail.com>


I really don't think this is worth the trouble just to be able
to support COMPILE_TEST on all architectures. This is an old watchdog
driver. It should not be touched except to fix bugs.

Didn't I say that before ?

Thanks,
Guenter

Re: [PATCHv3 2/2] watchdog: pika_wdt: enable COMPILE_TEST builds

[Rosen Penev]

On Mon Jun 8, 2026 at 3:55 PM PDT, Guenter Roeck wrote:
> On 6/8/26 14:04, Rosen Penev wrote:
>> Replace powerpc-specific I/O accessors (in_be32/out_be32) with
>> generic ones (ioread32be/iowrite32be) so the driver can be
>> compile-tested on other architectures.  Relax the Kconfig
>> dependency accordingly and add the required HAS_IOMEM.
>>
>> Assisted-by: opencode:big-pickle
>> Signed-off-by: Rosen Penev <rosenp@gmail.com>
>
>
> I really don't think this is worth the trouble just to be able
> to support COMPILE_TEST on all architectures. This is an old watchdog
> driver. It should not be touched except to fix bugs.
>
> Didn't I say that before ?
Sounds like this driver should be left alone then.
>
> Thanks,
> Guenter

Re: [PATCHv3 2/2] watchdog: pika_wdt: enable COMPILE_TEST builds

[Guenter Roeck]

On 6/8/26 16:46, Rosen Penev wrote:
> On Mon Jun 8, 2026 at 3:55 PM PDT, Guenter Roeck wrote:
>> On 6/8/26 14:04, Rosen Penev wrote:
>>> Replace powerpc-specific I/O accessors (in_be32/out_be32) with
>>> generic ones (ioread32be/iowrite32be) so the driver can be
>>> compile-tested on other architectures.  Relax the Kconfig
>>> dependency accordingly and add the required HAS_IOMEM.
>>>
>>> Assisted-by: opencode:big-pickle
>>> Signed-off-by: Rosen Penev <rosenp@gmail.com>
>>
>>
>> I really don't think this is worth the trouble just to be able
>> to support COMPILE_TEST on all architectures. This is an old watchdog
>> driver. It should not be touched except to fix bugs.
>>
>> Didn't I say that before ?
> Sounds like this driver should be left alone then.

Very much so. The driver is for a FPGA based system from around 2007,
and the company behind it went bankrupt in 2022. I would be surprised
if the current Linux kernel even boots on it.

Guenter