Skip to contentSkip to conversation

Watchdog: fix clearing of the watchdog interrupt

PATCH New Russell King 4 participants 4 messages Jun 18, 2013 – Jun 27, 2013 Watchdog device drivers

lore
Share & tools
lore
#0 lore raw

PATCH 2/2 Watchdog: fix clearing of the watchdog interrupt

The bits in BRIDGE_CAUSE are documented as RW0C - read, write 0 to clear. If we read the register, mask off the watchdog bit, and write it back, we're actually clearing every interrupt which wasn't pending at the time we read the register - and that is racy.

Fix this to only write ~WATCHDOG_BIT to the register, which means we write as zero only the watchdog bit.

---
 drivers/watchdog/orion_wdt.c |    4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)
drivers/watchdog/orion_wdt.c +1 -3
index 4074244..4ea5fcc 100644
@@ -70,9 +70,7 @@ static int orion_wdt_start(struct watchdog_device *wdt_dev)
 	writel(wdt_tclk * wdt_dev->timeout, wdt_reg + WDT_VAL);
 
 	/* Clear watchdog timer interrupt */
-	reg = readl(BRIDGE_CAUSE);
-	reg &= ~WDT_INT_REQ;
-	writel(reg, BRIDGE_CAUSE);
+	writel(~WDT_INT_REQ, BRIDGE_CAUSE);
 
 	/* Enable watchdog timer */
 	reg = readl(wdt_reg + TIMER_CTRL);
Signature · 1 line
1.7.4.4
#1 lore raw

PATCH 2/2 Watchdog: fix clearing of the watchdog interrupt

Quoted 5 lines · Russell King

The bits in BRIDGE_CAUSE are documented as RW0C - read, write 0 to clear. If we read the register, mask off the watchdog bit, and write it back, we're actually clearing every interrupt which wasn't pending at the time we read the register - and that is racy.

Quoted 7 lines

Fix this to only write ~WATCHDOG_BIT to the register, which means we write as zero only the watchdog bit.

---
 drivers/watchdog/orion_wdt.c |    4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)

thx,

Jason.

#2 lore raw

PATCH 2/2 Watchdog: fix clearing of the watchdog interrupt

In reply to Russell King
Quoted 30 lines · Russell King

The bits in BRIDGE_CAUSE are documented as RW0C - read, write 0 to clear. If we read the register, mask off the watchdog bit, and write it back, we're actually clearing every interrupt which wasn't pending at the time we read the register - and that is racy.

Fix this to only write ~WATCHDOG_BIT to the register, which means we write as zero only the watchdog bit.

---
 drivers/watchdog/orion_wdt.c |    4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)
drivers/watchdog/orion_wdt.c +1 -3
index 4074244..4ea5fcc 100644
@@ -70,9 +70,7 @@ static int orion_wdt_start(struct watchdog_device *wdt_dev)
 	writel(wdt_tclk * wdt_dev->timeout, wdt_reg + WDT_VAL);
 
 	/* Clear watchdog timer interrupt */
-	reg = readl(BRIDGE_CAUSE);
-	reg &= ~WDT_INT_REQ;
-	writel(reg, BRIDGE_CAUSE);
+	writel(~WDT_INT_REQ, BRIDGE_CAUSE);
 
 	/* Enable watchdog timer */
 	reg = readl(wdt_reg + TIMER_CTRL);
Signature · 1 line
1.7.4.4

Hi Russell

For this, and patch 1/2:

On a kirkwood.

Andrew

#3 lore raw

Hi All,

Both patches have been added to linux-watchdog-next.

Kind regards,
Wim.