Skip to contentSkip to conversation

ib700wdt watchdog driver for 2.6.6

PATCH In review Patrice Bouchand 2 participants 5 messages May 10, 2004 – May 11, 2004 Char and misc drivers

lore
Share & tools
lore

Patrice Bouchand

author
#0 lore raw

PATCH 1/1 ib700wdt watchdog driver for 2.6.6

Hello all,

The following is a kernel 2.6.6 patch for the ib700wdt watchdog
driver. I tried to mail Charles Howes, but his address seems to be down.

The modified file is ib700wdt.c

The thing which is modified :

ibwdt_ping() : a bug removed ,port value must be written not the timeout in
second

Comments are welcome,

Best regards

Patrice BOUCHAND

------------------------------------------------------------------------------------------------------------------

ib700wdt.c 2004-05-10 08:44:50.000000000 +0200 renamed from ib700wdt.c.orig 2004-05-10 08:57:54.000000000 +0200 +1 -1
@@ -135,7 +135,7 @@
 ibwdt_ping(void)
 {
        /* Write a watchdog value */
-       outb_p(wd_times[wd_margin], WDT_START);
+       outb_p(wd_margin, WDT_START);
 }
 
 static ssize_t
#1 lore raw
Quoted 10 lines · Patrice Bouchand

--- ./ib700wdt.c.orig 2004-05-10 08:57:54.000000000 +0200
+++ ./ib700wdt.c 2004-05-10 08:44:50.000000000 +0200
@@ -135,7 +135,7 @@
ibwdt_ping(void)

  {
         /* Write a watchdog value */

- outb_p(wd_times[wd_margin], WDT_START);
+ outb_p(wd_margin, WDT_START);
}

The patch certainly looks sensible, but what about ibwdt_close() and ibwdt_notify_sys()? They're doing

		outb_p(wd_times[wd_margin], WDT_STOP);

which also seems peculiar.

author
#2 lore raw
Quoted 6 lines

The patch certainly looks sensible, but what about ibwdt_close() and ibwdt_notify_sys()? They're doing

		outb_p(wd_times[wd_margin], WDT_STOP);

which also seems peculiar.

The value written in the WDT_STOP register is not important. As soon as something is written , the watchdog timer stops.
But you are right, things will be cleaner if we use the following patch.

Thanks for the comments.

Best regards

Patrice Bouchand

-----------------------------------------------------------------------------------------------------------------------------------

ib700wdt.c 2004-05-11 10:50:54.000000000 +0200 renamed from ib700wdt.c.orig 2004-05-10 08:57:54.000000000 +0200 +3 -3
@@ -135,7 +135,7 @@
 ibwdt_ping(void)
 {
        /* Write a watchdog value */
-       outb_p(wd_times[wd_margin], WDT_START);
+       outb_p(wd_margin, WDT_START);
 }
 
 static ssize_t

@@ -234,7 +234,7 @@
 {
        spin_lock(&ibwdt_lock);
        if (expect_close == 42)
-               outb_p(wd_times[wd_margin], WDT_STOP);
+               outb_p(wd_margin, WDT_STOP);
        else
                printk(KERN_CRIT PFX "WDT device closed unexpectedly.  WDT will not stop!\n");
 

@@ -254,7 +254,7 @@
 {
        if (code == SYS_DOWN || code == SYS_HALT) {
                /* Turn the WDT off */
-               outb_p(wd_times[wd_margin], WDT_STOP);
+               outb_p(wd_margin, WDT_STOP);
        }
        return NOTIFY_DONE;
 }
#3 lore raw
Quoted 9 lines · Patrice Bouchand
(quoted diff) +0 -0
 	The value written in the WDT_STOP register is not important. As soon as something is written , the watchdog timer stops.
  But you are right, things will be cleaner if we use the following patch.
 
 	Thanks for the comments.
 
 	Best regards
 
 			Patrice Bouchand

Review

Quoted 10 lines
(quoted diff) +0 -0
 
 -----------------------------------------------------------------------------------------------------------------------------------
 --- ./ib700wdt.c.orig   2004-05-10 08:57:54.000000000 +0200
 +++ ib700wdt.c  2004-05-11 10:50:54.000000000 +0200
 @@ -135,7 +135,7 @@
  ibwdt_ping(void)
  {
         /* Write a watchdog value */
 -       outb_p(wd_times[wd_margin], WDT_START);
 +       outb_p(wd_margin, WDT_START);

It would be clearer still if we wrote a simple 0 in there. Does that sounds OK?

btw, please send patches in `patch -p1' format, as below. And your mailer seems to be converting tabs to spaces.

Thanks.

From: Patrice Bouchand <[email protected]>

The value written in the WDT_STOP register is not important. As soon as something is written, the watchdog timer stops. But things will be cleaner if we use the following patch.

---

 25-akpm/drivers/char/watchdog/ib700wdt.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
drivers/char/watchdog/ib700wdt.c renamed from drivers/char/watchdog/ib700wdt.c~ib700wdt-fix-2 +2 -2
diff -puN drivers/char/watchdog/ib700wdt.c~ib700wdt-fix-2 drivers/char/watchdog/ib700wdt.c
@@ -234,7 +234,7 @@ ibwdt_close(struct inode *inode, struct 
 {
 	spin_lock(&ibwdt_lock);
 	if (expect_close == 42)
-		outb_p(wd_times[wd_margin], WDT_STOP);
+		outb_p(0, WDT_STOP);
 	else
 		printk(KERN_CRIT PFX "WDT device closed unexpectedly.  WDT will not stop!\n");
 

@@ -254,7 +254,7 @@ ibwdt_notify_sys(struct notifier_block *
 {
 	if (code == SYS_DOWN || code == SYS_HALT) {
 		/* Turn the WDT off */
-		outb_p(wd_times[wd_margin], WDT_STOP);
+		outb_p(0, WDT_STOP);
 	}
 	return NOTIFY_DONE;
 }

_

author
#4 lore raw
Quoted 3 lines

It would be clearer still if we wrote a simple 0 in there. Does that sounds OK?

Yes, it is OK.

Quoted 2 lines

btw, please send patches in `patch -p1' format, as below. And your mailer seems to be converting tabs to spaces.

Ok, I will take care about that the next times.

Best regards

Patrice Bouchand