Bug in sch311x_wdt Linux driver

Bug in sch311x_wdt Linux driver

["David Müller (ELSOFT AG)"]

[-- Attachment #1: Type: text/plain, Size: 765 bytes --]

Hello

I think there is a bug in the Linux driver for the SCH311x watchdog.

The SCH311x chip contains 2 watchdogs. One is the watchdog programmable
by the runtime register at address 0x65-0x68, the other is the watchdog
inside the power on reset generator. This second watchdog has a fixed
timeout value of ~1.6 seconds and is configurable only by the RESGEN
register.

The BIOS normally takes care of the RESGEN watchdog and disables it (at
least) before the OS is booted.

Unfortunately the sch311x_wdt driver clears bit 0 of the RESGEN register
which has the effect that at the latest 1.6 seconds later, a POR is
triggered.


The attached patch fixes this problem by completely removing any
reference to the RESGEN watchdog from the sch311x_wdt driver.


Dave


[-- Attachment #2: sch311x_wdt.c.patch --]
[-- Type: text/plain, Size: 3411 bytes --]

Signed-off-by: Dave Mueller <d.mueller@elsoft.ch>

--- a/drivers/watchdog/sch311x_wdt.c	2011-11-02 17:21:22.955861102 +0100
+++ b/drivers/watchdog/sch311x_wdt.c	2012-04-11 16:14:21.622295578 +0200
@@ -40,7 +40,6 @@
 #define PFX		DRV_NAME ": "

 /* Runtime registers */
-#define RESGEN			0x1d
 #define GP60			0x47
 #define WDT_TIME_OUT		0x65
 #define WDT_VAL			0x66
@@ -68,10 +67,6 @@
 module_param(force_id, ushort, 0);
 MODULE_PARM_DESC(force_id, "Override the detected device ID");

-static unsigned short therm_trip;
-module_param(therm_trip, ushort, 0);
-MODULE_PARM_DESC(therm_trip, "Should a ThermTrip trigger the reset generator");
-
 #define WATCHDOG_TIMEOUT 60		/* 60 sec default timeout */
 static int timeout = WATCHDOG_TIMEOUT;	/* in seconds */
 module_param(timeout, int, 0);
@@ -358,26 +353,17 @@
 static int __devinit sch311x_wdt_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
-	unsigned char val;
 	int err;

 	spin_lock_init(&sch311x_wdt_data.io_lock);

-	if (!request_region(sch311x_wdt_data.runtime_reg + RESGEN, 1,
-								DRV_NAME)) {
-		dev_err(dev, "Failed to request region 0x%04x-0x%04x.\n",
-			sch311x_wdt_data.runtime_reg + RESGEN,
-			sch311x_wdt_data.runtime_reg + RESGEN);
-		err = -EBUSY;
-		goto exit;
-	}

 	if (!request_region(sch311x_wdt_data.runtime_reg + GP60, 1, DRV_NAME)) {
 		dev_err(dev, "Failed to request region 0x%04x-0x%04x.\n",
 			sch311x_wdt_data.runtime_reg + GP60,
 			sch311x_wdt_data.runtime_reg + GP60);
 		err = -EBUSY;
-		goto exit_release_region;
+		goto exit;
 	}

 	if (!request_region(sch311x_wdt_data.runtime_reg + WDT_TIME_OUT, 4,
@@ -386,7 +372,7 @@
 			sch311x_wdt_data.runtime_reg + WDT_TIME_OUT,
 			sch311x_wdt_data.runtime_reg + WDT_CTRL);
 		err = -EBUSY;
-		goto exit_release_region2;
+		goto exit_release_region;
 	}

 	/* Make sure that the watchdog is not running */
@@ -414,24 +400,13 @@
 	/* Get status at boot */
 	sch311x_wdt_get_status(&sch311x_wdt_data.boot_status);

-	/* enable watchdog */
-	/* -- Reset Generator --
-	 * Bit 0   Enable Watchdog Timer Generation: 0* = Enabled, 1 = Disabled
-	 * Bit 1   Thermtrip Source Select: O* = No Source, 1 = Source
-	 * Bit 2   WDT2_CTL: WDT input bit
-	 * Bit 3-7 Reserved
-	 */
-	outb(0, sch311x_wdt_data.runtime_reg + RESGEN);
-	val = therm_trip ? 0x06 : 0x04;
-	outb(val, sch311x_wdt_data.runtime_reg + RESGEN);
-
 	sch311x_wdt_miscdev.parent = dev;

 	err = misc_register(&sch311x_wdt_miscdev);
 	if (err != 0) {
 		dev_err(dev, "cannot register miscdev on minor=%d (err=%d)\n",
 							WATCHDOG_MINOR, err);
-		goto exit_release_region3;
+		goto exit_release_region2;
 	}

 	dev_info(dev,
@@ -440,12 +415,10 @@

 	return 0;

-exit_release_region3:
-	release_region(sch311x_wdt_data.runtime_reg + WDT_TIME_OUT, 4);
 exit_release_region2:
-	release_region(sch311x_wdt_data.runtime_reg + GP60, 1);
+	release_region(sch311x_wdt_data.runtime_reg + WDT_TIME_OUT, 4);
 exit_release_region:
-	release_region(sch311x_wdt_data.runtime_reg + RESGEN, 1);
+	release_region(sch311x_wdt_data.runtime_reg + GP60, 1);
 	sch311x_wdt_data.runtime_reg = 0;
 exit:
 	return err;
@@ -461,7 +434,6 @@
 	misc_deregister(&sch311x_wdt_miscdev);
 	release_region(sch311x_wdt_data.runtime_reg + WDT_TIME_OUT, 4);
 	release_region(sch311x_wdt_data.runtime_reg + GP60, 1);
-	release_region(sch311x_wdt_data.runtime_reg + RESGEN, 1);
 	sch311x_wdt_data.runtime_reg = 0;
 	return 0;
 }