[-mm PATCH] set enable bit instead of lock bit of Watchdog Timer in Intel 6300 chipset

[-mm PATCH] set enable bit instead of lock bit of Watchdog Timer in Intel 6300 chipset

[Naveen Gupta]

This patch sets the WDT_ENABLE bit of the Lock Register to enable the
watchdog and WDT_LOCK bit only if nowayout is set. The old code always
sets the WDT_LOCK bit of watchdog timer for Intel 6300ESB chipset. So, we
end up locking the watchdog instead of enabling it.

Signed-off-by: Naveen Gupta <ngupta@google.com>

Index: linux-2.6.12/drivers/char/watchdog/i6300esb.c
===================================================================
--- linux-2.6.12.orig/drivers/char/watchdog/i6300esb.c	2005-08-15 11:19:01.000000000 -0700
+++ linux-2.6.12/drivers/char/watchdog/i6300esb.c	2005-08-15 11:21:35.000000000 -0700
@@ -97,7 +97,7 @@
 	u8 val;
 
 	/* Enable or Enable + Lock? */
-	val = 0x02 | nowayout ? 0x01 : 0x00;
+	val = 0x02 | (nowayout ? 0x01 : 0x00);
 
         pci_write_config_byte(esb_pci, ESB_LOCK_REG, val);
 }

Re: [-mm PATCH] set enable bit instead of lock bit of Watchdog Timer in Intel 6300 chipset

[David Härdeman]

On Mon, Aug 15, 2005 at 02:21:05PM -0700, Naveen Gupta wrote:
>
>This patch sets the WDT_ENABLE bit of the Lock Register to enable the
>watchdog and WDT_LOCK bit only if nowayout is set. The old code always
>sets the WDT_LOCK bit of watchdog timer for Intel 6300ESB chipset. So, we
>end up locking the watchdog instead of enabling it.

Naveen,

thanks alot for testing the driver further and finding these bugs. I've 
not been able to do so myself as the only computers available to me with 
this watchdog are production-servers meaning that I've only been able to 
test during scheduled downtimes.

Have you tested and verified that the driver works after these patches 
have been applied?

Re,
David

Re: [-mm PATCH] set enable bit instead of lock bit of Watchdog Timer in Intel 6300 chipset

[Naveen Gupta]

David,

Yes, I have tested these patches. In fact I found these bugs while trying
to make the driver work on our machines.

-Naveen

On Tue, 16 Aug 2005, David Härdeman wrote:

> On Mon, Aug 15, 2005 at 02:21:05PM -0700, Naveen Gupta wrote:
> >
> >This patch sets the WDT_ENABLE bit of the Lock Register to enable the
> >watchdog and WDT_LOCK bit only if nowayout is set. The old code always
> >sets the WDT_LOCK bit of watchdog timer for Intel 6300ESB chipset. So, we
> >end up locking the watchdog instead of enabling it.
> 
> Naveen,
> 
> thanks alot for testing the driver further and finding these bugs. I've 
> not been able to do so myself as the only computers available to me with 
> this watchdog are production-servers meaning that I've only been able to 
> test during scheduled downtimes.
> 
> Have you tested and verified that the driver works after these patches 
> have been applied?
> 
> Re,
> David
> 

Re: [-mm PATCH] set enable bit instead of lock bit of Watchdog Timer in Intel 6300 chipset

[David Härdeman]

On Mon, Aug 15, 2005 at 02:21:05PM -0700, Naveen Gupta wrote:
>
>This patch sets the WDT_ENABLE bit of the Lock Register to enable the
>watchdog and WDT_LOCK bit only if nowayout is set. The old code always
>sets the WDT_LOCK bit of watchdog timer for Intel 6300ESB chipset. So, we
>end up locking the watchdog instead of enabling it.
>
>Signed-off-by: Naveen Gupta <ngupta@google.com>

Acked-by: David Härdeman <david@2gen.com>

Thanks Naveen.

//David

Re: [-mm PATCH] set enable bit instead of lock bit of Watchdog Timer in Intel 6300 chipset

[Wim Van Sebroeck]

Hi All,

> >This patch sets the WDT_ENABLE bit of the Lock Register to enable the
> >watchdog and WDT_LOCK bit only if nowayout is set. The old code always
> >sets the WDT_LOCK bit of watchdog timer for Intel 6300ESB chipset. So, we
> >end up locking the watchdog instead of enabling it.
> >
> >Signed-off-by: Naveen Gupta <ngupta@google.com>
> 
> Acked-by: David Härdeman <david@2gen.com>

I'm setting up the watchdog git trees at the moment (and doing the convertion
from the old bitkeeper repositories) when that's completed I will add these
new patches to the watchdog-tree.
I'll keep you posted.

Greetings,
Wim.