[PATCH] watchdog: mtx-1_wdt: synchronize timer teardown in remove

[PATCH] watchdog: mtx-1_wdt: synchronize timer teardown in remove

[aviv.daum]

From: avivdaum <aviv.daum@gmail.com>

Replace the ad-hoc completion based teardown path with a proper
timer synchronization call in remove.

Stop the watchdog and delete the timer synchronously using
timer_delete_sync(), then deregister the misc device.

This removes the open FIXME about locking/testing of queue teardown
and makes the driver follow the current timer API pattern used in
watchdog drivers.

Compile-tested: ARCH=mips CROSS_COMPILE=mips-linux-gnu-
  32r2el_defconfig (drivers/watchdog/mtx-1_wdt.o, W=1/W=2)
Compile-tested: ARCH=mips CROSS_COMPILE=mips-linux-gnu-
  mtx1_defconfig (drivers/watchdog/mtx-1_wdt.o, W=1/W=2)
Compile-tested: ARCH=mips CROSS_COMPILE=mips-linux-gnu-
  malta_defconfig + WATCHDOG=y + COMPILE_TEST=y + WDT_MTX1=m
  (drivers/watchdog/mtx-1_wdt.o, W=1/W=2)
Signed-off-by: avivdaum <aviv.daum@gmail.com>
---
 drivers/watchdog/mtx-1_wdt.c | 12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/drivers/watchdog/mtx-1_wdt.c b/drivers/watchdog/mtx-1_wdt.c
index f75426cfa..85fe006de 100644
--- a/drivers/watchdog/mtx-1_wdt.c
+++ b/drivers/watchdog/mtx-1_wdt.c
@@ -33,7 +33,6 @@
 #include <linux/fs.h>
 #include <linux/ioport.h>
 #include <linux/timer.h>
-#include <linux/completion.h>
 #include <linux/jiffies.h>
 #include <linux/watchdog.h>
 #include <linux/platform_device.h>
@@ -46,7 +45,6 @@
 static int ticks = 100 * HZ;
 
 static struct {
-	struct completion stop;
 	spinlock_t lock;
 	int running;
 	struct timer_list timer;
@@ -69,8 +67,6 @@ static void mtx1_wdt_trigger(struct timer_list *unused)
 
 	if (mtx1_wdt_device.queue && ticks)
 		mod_timer(&mtx1_wdt_device.timer, jiffies + MTX1_WDT_INTERVAL);
-	else
-		complete(&mtx1_wdt_device.stop);
 	spin_unlock(&mtx1_wdt_device.lock);
 }
 
@@ -204,7 +200,6 @@ static int mtx1_wdt_probe(struct platform_device *pdev)
 	}
 
 	spin_lock_init(&mtx1_wdt_device.lock);
-	init_completion(&mtx1_wdt_device.stop);
 	mtx1_wdt_device.queue = 0;
 	clear_bit(0, &mtx1_wdt_device.inuse);
 	timer_setup(&mtx1_wdt_device.timer, mtx1_wdt_trigger, 0);
@@ -222,11 +217,8 @@ static int mtx1_wdt_probe(struct platform_device *pdev)
 
 static void mtx1_wdt_remove(struct platform_device *pdev)
 {
-	/* FIXME: do we need to lock this test ? */
-	if (mtx1_wdt_device.queue) {
-		mtx1_wdt_device.queue = 0;
-		wait_for_completion(&mtx1_wdt_device.stop);
-	}
+	mtx1_wdt_stop();
+	timer_delete_sync(&mtx1_wdt_device.timer);
 
 	misc_deregister(&mtx1_wdt_misc);
 }
-- 
2.34.1

Re: [PATCH] watchdog: mtx-1_wdt: synchronize timer teardown in remove

[Guenter Roeck]

On 3/12/26 14:06, aviv.daum@gmail.com wrote:
> From: avivdaum <aviv.daum@gmail.com>
> 
> Replace the ad-hoc completion based teardown path with a proper
> timer synchronization call in remove.
> 
> Stop the watchdog and delete the timer synchronously using
> timer_delete_sync(), then deregister the misc device.
> 
> This removes the open FIXME about locking/testing of queue teardown
> and makes the driver follow the current timer API pattern used in
> watchdog drivers.
> 
> Compile-tested: ARCH=mips CROSS_COMPILE=mips-linux-gnu-
>    32r2el_defconfig (drivers/watchdog/mtx-1_wdt.o, W=1/W=2)
> Compile-tested: ARCH=mips CROSS_COMPILE=mips-linux-gnu-
>    mtx1_defconfig (drivers/watchdog/mtx-1_wdt.o, W=1/W=2)
> Compile-tested: ARCH=mips CROSS_COMPILE=mips-linux-gnu-
>    malta_defconfig + WATCHDOG=y + COMPILE_TEST=y + WDT_MTX1=m
>    (drivers/watchdog/mtx-1_wdt.o, W=1/W=2)
> Signed-off-by: avivdaum <aviv.daum@gmail.com>

NACK, sorry. If you want to do anything with this driver, consider converting
it to use the watchdog subsystem (which would take care care of the timer
functionality).

Guenter