[PATCH v4 2/2] watchdog: fix w83627hf_wdt clear timeout expired
Observed that the Watchdog Timer Status bit can be set when the driver is loaded. Reset it during initialization. The time-out value must be set to 0 explicitly in this case to prevent an immediate reset. Depending on the motherboard and with watchdog disabled in the BIOS, this reset problem can easily be reproduced by the following steps: 1. reboot the system 2. wait for 5+ minutes 3. don't start any watchdog server. 4. just run "modprobe w83627hf_wdt" If the system load the driver after 5 minutes, it rebooted immediately because of timer expired. For example, fsck could take more than 5 minutes to run, then the computer reboot as soon as the driver was loaded. Signed-off-by: Tony Chung --- drivers/watchdog/w83627hf_wdt.c |6 -- 1 files changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/watchdog/w83627hf_wdt.c b/drivers/watchdog/w83627hf_wdt.c index 8fd..7eaa226 100644 --- a/drivers/watchdog/w83627hf_wdt.c +++ b/drivers/watchdog/w83627hf_wdt.c @@ -168,14 +168,16 @@ static void w83627hf_init(void) pr_info("Watchdog already running. Resetting timeout to %d sec\n", wdt_get_timeout_secs(timeout)); outb_p(timeout, WDT_EFDR);/* Write back to CRF6 */ + } else { + outb_p(0, WDT_EFDR);/* disable to prevent reboot */ } w83627hf_setup_crf5(); outb_p(0xF7, WDT_EFER); /* Select CRF7 */ t = inb_p(WDT_EFDR); /* read CRF7 */ - t &= ~0xC0; /* disable keyboard & mouse turning off - watchdog */ + t &= ~0xD0; /* clear timeout occurred and disable keyboard +& mouse turning off watchdog */ outb_p(t, WDT_EFDR);/* Write back to CRF7 */ w83627hf_unselect_wd_register(); -- 1.7.0.4 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v3 1/2] watchdog: improve w83627hf_wdt to timeout in minute
Watchdog should support timeout in minutes. For example, crash dump will usually require 5+ minutes. This patch increase the timeout from 255 seconds to (255*60) seconds and should be good for older LTS releases. This patch need to be rewritten again when this driver migrate to new watchdog infrastructure. Added a new max_timeout variable so we can change it if needed. Signed-off-by: Tony Chung --- drivers/watchdog/w83627hf_wdt.c | 74 ++ 1 files changed, 58 insertions(+), 16 deletions(-) diff --git a/drivers/watchdog/w83627hf_wdt.c b/drivers/watchdog/w83627hf_wdt.c index 92f1326..8fd 100644 --- a/drivers/watchdog/w83627hf_wdt.c +++ b/drivers/watchdog/w83627hf_wdt.c @@ -45,6 +45,7 @@ #define WATCHDOG_NAME "w83627hf/thf/hg/dhg WDT" #define WATCHDOG_TIMEOUT 60/* 60 sec default timeout */ +#define WATCHDOG_MAX_TIMEOUT (60 * 255) static unsigned long wdt_is_open; static char expect_close; @@ -55,11 +56,14 @@ static int wdt_io = 0x2E; module_param(wdt_io, int, 0); MODULE_PARM_DESC(wdt_io, "w83627hf/thf WDT io port (default 0x2E)"); +static bool use_minute; /* timeout unit in minutes if set */ +static const int max_timeout = WATCHDOG_MAX_TIMEOUT; static int timeout = WATCHDOG_TIMEOUT; /* in seconds */ module_param(timeout, int, 0); MODULE_PARM_DESC(timeout, - "Watchdog timeout in seconds. 1 <= timeout <= 255, default=" - __MODULE_STRING(WATCHDOG_TIMEOUT) "."); + "Watchdog timeout in seconds. 1<= timeout <=" + __MODULE_STRING(WATCHDOG_MAX_TIMEOUT) " (default=" + __MODULE_STRING(WATCHDOG_TIMEOUT) ")"); static bool nowayout = WATCHDOG_NOWAYOUT; module_param(nowayout, bool, 0); @@ -107,6 +111,48 @@ static void w83627hf_unselect_wd_register(void) outb_p(0xAA, WDT_EFER); /* Leave extended function mode */ } +/* setup CRF5 register */ +static void w83627hf_setup_crf5(void) +{ + unsigned char t; + + outb_p(0xF5, WDT_EFER); /* Select CRF5 */ + t = inb_p(WDT_EFDR); /* read CRF5 */ + t &= ~0x0C; /* set second mode & disable keyboard + turning off watchdog */ + t |= 0x02;/* enable the WDTO# output low pulse + to the KBRST# pin (PIN60) */ + if (use_minute) + t |= 0x8;/* set timeout in minute */ + + outb_p(t, WDT_EFDR);/* Write back to CRF5 */ +} + +/* set use_minute if t > 255 */ +static inline int wdt_use_minute(int t) +{ + if (t > 255) { + t = DIV_ROUND_UP(t, 60); /* convert secs to minutes */ + use_minute = 1; + } else { + use_minute = 0; + } + + /* setup CRF5 for new timeout unit */ + w83627hf_select_wd_register(); + w83627hf_setup_crf5(); + w83627hf_unselect_wd_register(); + + return t; +} + +/* convert specified value to seconds if use_minute is set */ +static inline int wdt_get_timeout_secs(int t) +{ + return (use_minute) ? (t * 60) : t; +} + + /* tyan motherboards seem to set F5 to 0x4C ? * So explicitly init to appropriate value. */ @@ -120,17 +166,11 @@ static void w83627hf_init(void) t = inb_p(WDT_EFDR); /* read CRF6 */ if (t != 0) { pr_info("Watchdog already running. Resetting timeout to %d sec\n", - timeout); + wdt_get_timeout_secs(timeout)); outb_p(timeout, WDT_EFDR);/* Write back to CRF6 */ } - outb_p(0xF5, WDT_EFER); /* Select CRF5 */ - t = inb_p(WDT_EFDR); /* read CRF5 */ - t &= ~0x0C; /* set second mode & disable keyboard - turning off watchdog */ - t |= 0x02;/* enable the WDTO# output low pulse - to the KBRST# pin (PIN60) */ - outb_p(t, WDT_EFDR);/* Write back to CRF5 */ + w83627hf_setup_crf5(); outb_p(0xF7, WDT_EFER); /* Select CRF7 */ t = inb_p(WDT_EFDR); /* read CRF7 */ @@ -169,9 +209,9 @@ static int wdt_disable(void) static int wdt_set_heartbeat(int t) { - if (t < 1 || t > 255) + if (t < 1 || t > max_timeout) return -EINVAL; - timeout = t; + timeout = wdt_use_minute(t); return 0; } @@ -190,7 +230,7 @@ static int wdt_get_time(void) spin_unlock(_lock); - return timeleft; + return wdt_get_timeout_secs(timeleft); } static ssize_t wdt_write(struct file *file, const char __user *buf, @@ -262,7 +302,8 @@ static long wdt_ioctl(struct file *file, unsigned int cmd, unsigned long arg) wdt_ping(); /* Fall */ case WDIOC_GETTIMEOUT
Re: [PATCH v3 2/2] watchdog: fix w83627hf_wdt clear timeout expired
On Tue, Apr 2, 2013 at 9:21 PM, Guenter Roeck wrote: > > > What is the exact chip type in your system ? I want to have a look into the > datasheet; maybe I can find out how it can trigger without causing a reset. Winbond 83627HF chip I believe BIOS has watchdog disabled otherwise it would have reboot the box. However, the timer just start counting. Comparing to ipmi_watchdog, you can do this: modprobe ipmi_watchdog ... start_now=0 ...action=<> nowayout=1 So it is possible to load the driver without start counting. Notice it is an else, so t is actually 0 already (i.e. expired or never start running): > + } else { > + outb_p(0, WDT_EFDR);/* disable to prevent reboot */ -- - Tony Chung -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v3 1/2] watchdog: improve w83627hf_wdt to timeout in minute
Watchdog should support timeout in minutes. For example, crash dump will usually require 5+ minutes. This patch increase the timeout from 255 seconds to (255*60) seconds and should be good for older LTS releases. This patch need to be rewritten again when this driver migrate to new watchdog infrastructure. Added a new max_timeout variable so we can change it if needed. Signed-off-by: Tony Chung tonychun...@gmail.com --- drivers/watchdog/w83627hf_wdt.c | 74 ++ 1 files changed, 58 insertions(+), 16 deletions(-) diff --git a/drivers/watchdog/w83627hf_wdt.c b/drivers/watchdog/w83627hf_wdt.c index 92f1326..8fd 100644 --- a/drivers/watchdog/w83627hf_wdt.c +++ b/drivers/watchdog/w83627hf_wdt.c @@ -45,6 +45,7 @@ #define WATCHDOG_NAME w83627hf/thf/hg/dhg WDT #define WATCHDOG_TIMEOUT 60/* 60 sec default timeout */ +#define WATCHDOG_MAX_TIMEOUT (60 * 255) static unsigned long wdt_is_open; static char expect_close; @@ -55,11 +56,14 @@ static int wdt_io = 0x2E; module_param(wdt_io, int, 0); MODULE_PARM_DESC(wdt_io, w83627hf/thf WDT io port (default 0x2E)); +static bool use_minute; /* timeout unit in minutes if set */ +static const int max_timeout = WATCHDOG_MAX_TIMEOUT; static int timeout = WATCHDOG_TIMEOUT; /* in seconds */ module_param(timeout, int, 0); MODULE_PARM_DESC(timeout, - Watchdog timeout in seconds. 1 = timeout = 255, default= - __MODULE_STRING(WATCHDOG_TIMEOUT) .); + Watchdog timeout in seconds. 1= timeout = + __MODULE_STRING(WATCHDOG_MAX_TIMEOUT) (default= + __MODULE_STRING(WATCHDOG_TIMEOUT) )); static bool nowayout = WATCHDOG_NOWAYOUT; module_param(nowayout, bool, 0); @@ -107,6 +111,48 @@ static void w83627hf_unselect_wd_register(void) outb_p(0xAA, WDT_EFER); /* Leave extended function mode */ } +/* setup CRF5 register */ +static void w83627hf_setup_crf5(void) +{ + unsigned char t; + + outb_p(0xF5, WDT_EFER); /* Select CRF5 */ + t = inb_p(WDT_EFDR); /* read CRF5 */ + t = ~0x0C; /* set second mode disable keyboard + turning off watchdog */ + t |= 0x02;/* enable the WDTO# output low pulse + to the KBRST# pin (PIN60) */ + if (use_minute) + t |= 0x8;/* set timeout in minute */ + + outb_p(t, WDT_EFDR);/* Write back to CRF5 */ +} + +/* set use_minute if t 255 */ +static inline int wdt_use_minute(int t) +{ + if (t 255) { + t = DIV_ROUND_UP(t, 60); /* convert secs to minutes */ + use_minute = 1; + } else { + use_minute = 0; + } + + /* setup CRF5 for new timeout unit */ + w83627hf_select_wd_register(); + w83627hf_setup_crf5(); + w83627hf_unselect_wd_register(); + + return t; +} + +/* convert specified value to seconds if use_minute is set */ +static inline int wdt_get_timeout_secs(int t) +{ + return (use_minute) ? (t * 60) : t; +} + + /* tyan motherboards seem to set F5 to 0x4C ? * So explicitly init to appropriate value. */ @@ -120,17 +166,11 @@ static void w83627hf_init(void) t = inb_p(WDT_EFDR); /* read CRF6 */ if (t != 0) { pr_info(Watchdog already running. Resetting timeout to %d sec\n, - timeout); + wdt_get_timeout_secs(timeout)); outb_p(timeout, WDT_EFDR);/* Write back to CRF6 */ } - outb_p(0xF5, WDT_EFER); /* Select CRF5 */ - t = inb_p(WDT_EFDR); /* read CRF5 */ - t = ~0x0C; /* set second mode disable keyboard - turning off watchdog */ - t |= 0x02;/* enable the WDTO# output low pulse - to the KBRST# pin (PIN60) */ - outb_p(t, WDT_EFDR);/* Write back to CRF5 */ + w83627hf_setup_crf5(); outb_p(0xF7, WDT_EFER); /* Select CRF7 */ t = inb_p(WDT_EFDR); /* read CRF7 */ @@ -169,9 +209,9 @@ static int wdt_disable(void) static int wdt_set_heartbeat(int t) { - if (t 1 || t 255) + if (t 1 || t max_timeout) return -EINVAL; - timeout = t; + timeout = wdt_use_minute(t); return 0; } @@ -190,7 +230,7 @@ static int wdt_get_time(void) spin_unlock(io_lock); - return timeleft; + return wdt_get_timeout_secs(timeleft); } static ssize_t wdt_write(struct file *file, const char __user *buf, @@ -262,7 +302,8 @@ static long wdt_ioctl(struct file *file, unsigned int cmd, unsigned long arg) wdt_ping(); /* Fall */ case WDIOC_GETTIMEOUT: - return put_user(timeout, p); + timeval = wdt_get_timeout_secs(timeout); + return put_user
[PATCH v4 2/2] watchdog: fix w83627hf_wdt clear timeout expired
Observed that the Watchdog Timer Status bit can be set when the driver is loaded. Reset it during initialization. The time-out value must be set to 0 explicitly in this case to prevent an immediate reset. Depending on the motherboard and with watchdog disabled in the BIOS, this reset problem can easily be reproduced by the following steps: 1. reboot the system 2. wait for 5+ minutes 3. don't start any watchdog server. 4. just run modprobe w83627hf_wdt If the system load the driver after 5 minutes, it rebooted immediately because of timer expired. For example, fsck could take more than 5 minutes to run, then the computer reboot as soon as the driver was loaded. Signed-off-by: Tony Chung tonychun...@gmail.com --- drivers/watchdog/w83627hf_wdt.c |6 -- 1 files changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/watchdog/w83627hf_wdt.c b/drivers/watchdog/w83627hf_wdt.c index 8fd..7eaa226 100644 --- a/drivers/watchdog/w83627hf_wdt.c +++ b/drivers/watchdog/w83627hf_wdt.c @@ -168,14 +168,16 @@ static void w83627hf_init(void) pr_info(Watchdog already running. Resetting timeout to %d sec\n, wdt_get_timeout_secs(timeout)); outb_p(timeout, WDT_EFDR);/* Write back to CRF6 */ + } else { + outb_p(0, WDT_EFDR);/* disable to prevent reboot */ } w83627hf_setup_crf5(); outb_p(0xF7, WDT_EFER); /* Select CRF7 */ t = inb_p(WDT_EFDR); /* read CRF7 */ - t = ~0xC0; /* disable keyboard mouse turning off - watchdog */ + t = ~0xD0; /* clear timeout occurred and disable keyboard + mouse turning off watchdog */ outb_p(t, WDT_EFDR);/* Write back to CRF7 */ w83627hf_unselect_wd_register(); -- 1.7.0.4 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 2/2] watchdog: fix w83627hf_wdt clear timeout expired
On Tue, Apr 2, 2013 at 9:21 PM, Guenter Roeck li...@roeck-us.net wrote: What is the exact chip type in your system ? I want to have a look into the datasheet; maybe I can find out how it can trigger without causing a reset. Winbond 83627HF chip I believe BIOS has watchdog disabled otherwise it would have reboot the box. However, the timer just start counting. Comparing to ipmi_watchdog, you can do this: modprobe ipmi_watchdog ... start_now=0 ...action= nowayout=1 So it is possible to load the driver without start counting. Notice it is an else, so t is actually 0 already (i.e. expired or never start running): + } else { + outb_p(0, WDT_EFDR);/* disable to prevent reboot */ -- - Tony Chung -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 2/2] watchdog: fix w83627hf_wdt clear timeout expired
Thanks Guenter! I agree with you. My first reaction was also about a small watchdog server that will start in early boot process. There are pros and cons. For example, there are many types of watchdog devices such as ipmi_watchdog which can accept more than 255 seconds for timeout. So you really need udev to pick the correct watchdog driver. It could be very complex. Our requirement don't need watchdog protection during the boot process until application is fully up but a driver should not assume anything. Anyway, an unexpected reboot is definitely a bug that need to be fixed. It is really easily reproducible. Depending on your hardware and BIOS settings, just reboot the boot, wait for 5 minutes and then run "insmod w83627hf_wdt.ko". The box just reboot by itself. The watchdog sever is not even started. This line is actually the original fix that is running over a year: outb_p(0, WDT_EFDR);/* disable to prevent reboot */ When I tried to cleanup it up, I thought I don't need it but it turned out it was still needed. When I changed it from 0xC0 to 0xD0, it still reboot. Thanks and best regards, @Tony On Mon, Apr 1, 2013 at 5:07 PM, Guenter Roeck wrote: > On Sun, Mar 31, 2013 at 10:59:32PM -0700, Tony Chung wrote: >> Observed that the w83627hf watchdog timer start counting during reboot. >> If the system load the driver after 5 minutes, it rebooted immediately >> because of timer expired. >> For example, fsck took more than 5 minutes to run, then the computer reboot >> as soon as the driver was loaded. >> > Thinking about it, I wonder if that really solves your problem. > > If the watchdog driver is built into the kernel, and if the watchdog is > already > running, you still may have the problem that it may time out again and trigger > (again) before the watchdog application can be loaded. Ultimately, in such a > situation, the watchdog application, or at least a primitive one, should > really > be started from initramfs and possibly be replaced later on by the "real" > watchdog > application. The watchdog package includes a small binary for that purpose, > though I have no idea if is is loaded in initramfs or only later. > > Ultimately, that also applies to the original problem you tried to solve with > the longer timeout. It doesn't seem correct to have to set a long timeout to > address excessive boot times; in a way that defeats the purpose of a watchdog. > A cleaner solution for that problem would be to make sure that a small > watchdog > application is started early in the boot process. > >> Signed-off-by: Tony Chung >> --- >> drivers/watchdog/w83627hf_wdt.c |6 -- >> 1 files changed, 4 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/watchdog/w83627hf_wdt.c >> b/drivers/watchdog/w83627hf_wdt.c >> index 8fd..7eaa226 100644 >> --- a/drivers/watchdog/w83627hf_wdt.c >> +++ b/drivers/watchdog/w83627hf_wdt.c >> @@ -168,14 +168,16 @@ static void w83627hf_init(void) >> pr_info("Watchdog already running. Resetting timeout to %d >> sec\n", >> wdt_get_timeout_secs(timeout)); >> outb_p(timeout, WDT_EFDR);/* Write back to CRF6 */ >> + } else { >> + outb_p(0, WDT_EFDR);/* disable to prevent reboot */ >> } > Did I miss that earlier, or did yo add it in this version ? > > Reading the counter register just returned 0, so what are the benefits > of writing 0 into it ? > > Thanks, > Guenter > >> >> w83627hf_setup_crf5(); >> >> outb_p(0xF7, WDT_EFER); /* Select CRF7 */ >> t = inb_p(WDT_EFDR); /* read CRF7 */ >> - t &= ~0xC0; /* disable keyboard & mouse turning off >> - watchdog */ >> + t &= ~0xD0; /* clear timeout occurred and disable >> keyboard >> + & mouse turning off watchdog */ >> outb_p(t, WDT_EFDR);/* Write back to CRF7 */ >> >> w83627hf_unselect_wd_register(); >> -- >> 1.7.0.4 >> >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in >> the body of a message to majord...@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html >> -- - Tony Chung -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v3 1/2] watchdog: improve w83627hf_wdt to timeout in minute
Watchdog should support timeout in minutes. For example, crash dump will usually require 5+ minutes. This patch increase the timeout from 255 seconds to (255*60) seconds and should be good for older LTS releases. This patch need to be rewritten again when this driver migrate to new watchdog infrastructure. Added a new max_timeout variable so we can change it if needed. Signed-off-by: Tony Chung --- drivers/watchdog/w83627hf_wdt.c | 74 ++ 1 files changed, 58 insertions(+), 16 deletions(-) diff --git a/drivers/watchdog/w83627hf_wdt.c b/drivers/watchdog/w83627hf_wdt.c index 92f1326..8fd 100644 --- a/drivers/watchdog/w83627hf_wdt.c +++ b/drivers/watchdog/w83627hf_wdt.c @@ -45,6 +45,7 @@ #define WATCHDOG_NAME "w83627hf/thf/hg/dhg WDT" #define WATCHDOG_TIMEOUT 60/* 60 sec default timeout */ +#define WATCHDOG_MAX_TIMEOUT (60 * 255) static unsigned long wdt_is_open; static char expect_close; @@ -55,11 +56,14 @@ static int wdt_io = 0x2E; module_param(wdt_io, int, 0); MODULE_PARM_DESC(wdt_io, "w83627hf/thf WDT io port (default 0x2E)"); +static bool use_minute; /* timeout unit in minutes if set */ +static const int max_timeout = WATCHDOG_MAX_TIMEOUT; static int timeout = WATCHDOG_TIMEOUT; /* in seconds */ module_param(timeout, int, 0); MODULE_PARM_DESC(timeout, - "Watchdog timeout in seconds. 1 <= timeout <= 255, default=" - __MODULE_STRING(WATCHDOG_TIMEOUT) "."); + "Watchdog timeout in seconds. 1<= timeout <=" + __MODULE_STRING(WATCHDOG_MAX_TIMEOUT) " (default=" + __MODULE_STRING(WATCHDOG_TIMEOUT) ")"); static bool nowayout = WATCHDOG_NOWAYOUT; module_param(nowayout, bool, 0); @@ -107,6 +111,48 @@ static void w83627hf_unselect_wd_register(void) outb_p(0xAA, WDT_EFER); /* Leave extended function mode */ } +/* setup CRF5 register */ +static void w83627hf_setup_crf5(void) +{ + unsigned char t; + + outb_p(0xF5, WDT_EFER); /* Select CRF5 */ + t = inb_p(WDT_EFDR); /* read CRF5 */ + t &= ~0x0C; /* set second mode & disable keyboard + turning off watchdog */ + t |= 0x02;/* enable the WDTO# output low pulse + to the KBRST# pin (PIN60) */ + if (use_minute) + t |= 0x8;/* set timeout in minute */ + + outb_p(t, WDT_EFDR);/* Write back to CRF5 */ +} + +/* set use_minute if t > 255 */ +static inline int wdt_use_minute(int t) +{ + if (t > 255) { + t = DIV_ROUND_UP(t, 60); /* convert secs to minutes */ + use_minute = 1; + } else { + use_minute = 0; + } + + /* setup CRF5 for new timeout unit */ + w83627hf_select_wd_register(); + w83627hf_setup_crf5(); + w83627hf_unselect_wd_register(); + + return t; +} + +/* convert specified value to seconds if use_minute is set */ +static inline int wdt_get_timeout_secs(int t) +{ + return (use_minute) ? (t * 60) : t; +} + + /* tyan motherboards seem to set F5 to 0x4C ? * So explicitly init to appropriate value. */ @@ -120,17 +166,11 @@ static void w83627hf_init(void) t = inb_p(WDT_EFDR); /* read CRF6 */ if (t != 0) { pr_info("Watchdog already running. Resetting timeout to %d sec\n", - timeout); + wdt_get_timeout_secs(timeout)); outb_p(timeout, WDT_EFDR);/* Write back to CRF6 */ } - outb_p(0xF5, WDT_EFER); /* Select CRF5 */ - t = inb_p(WDT_EFDR); /* read CRF5 */ - t &= ~0x0C; /* set second mode & disable keyboard - turning off watchdog */ - t |= 0x02;/* enable the WDTO# output low pulse - to the KBRST# pin (PIN60) */ - outb_p(t, WDT_EFDR);/* Write back to CRF5 */ + w83627hf_setup_crf5(); outb_p(0xF7, WDT_EFER); /* Select CRF7 */ t = inb_p(WDT_EFDR); /* read CRF7 */ @@ -169,9 +209,9 @@ static int wdt_disable(void) static int wdt_set_heartbeat(int t) { - if (t < 1 || t > 255) + if (t < 1 || t > max_timeout) return -EINVAL; - timeout = t; + timeout = wdt_use_minute(t); return 0; } @@ -190,7 +230,7 @@ static int wdt_get_time(void) spin_unlock(_lock); - return timeleft; + return wdt_get_timeout_secs(timeleft); } static ssize_t wdt_write(struct file *file, const char __user *buf, @@ -262,7 +302,8 @@ static long wdt_ioctl(struct file *file, unsigned int cmd, unsigned long arg) wdt_ping(); /* Fall */ case WDIOC_GETTIMEOUT
[PATCH v3 2/2] watchdog: fix w83627hf_wdt clear timeout expired
Observed that the w83627hf watchdog timer start counting during reboot. If the system load the driver after 5 minutes, it rebooted immediately because of timer expired. For example, fsck took more than 5 minutes to run, then the computer reboot as soon as the driver was loaded. Signed-off-by: Tony Chung --- drivers/watchdog/w83627hf_wdt.c |6 -- 1 files changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/watchdog/w83627hf_wdt.c b/drivers/watchdog/w83627hf_wdt.c index 8fd..7eaa226 100644 --- a/drivers/watchdog/w83627hf_wdt.c +++ b/drivers/watchdog/w83627hf_wdt.c @@ -168,14 +168,16 @@ static void w83627hf_init(void) pr_info("Watchdog already running. Resetting timeout to %d sec\n", wdt_get_timeout_secs(timeout)); outb_p(timeout, WDT_EFDR);/* Write back to CRF6 */ + } else { + outb_p(0, WDT_EFDR);/* disable to prevent reboot */ } w83627hf_setup_crf5(); outb_p(0xF7, WDT_EFER); /* Select CRF7 */ t = inb_p(WDT_EFDR); /* read CRF7 */ - t &= ~0xC0; /* disable keyboard & mouse turning off - watchdog */ + t &= ~0xD0; /* clear timeout occurred and disable keyboard +& mouse turning off watchdog */ outb_p(t, WDT_EFDR);/* Write back to CRF7 */ w83627hf_unselect_wd_register(); -- 1.7.0.4 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v3 1/2] watchdog: improve w83627hf_wdt to timeout in minute
Watchdog should support timeout in minutes. For example, crash dump will usually require 5+ minutes. This patch increase the timeout from 255 seconds to (255*60) seconds and should be good for older LTS releases. This patch need to be rewritten again when this driver migrate to new watchdog infrastructure. Added a new max_timeout variable so we can change it if needed. Signed-off-by: Tony Chung tonychun...@gmail.com --- drivers/watchdog/w83627hf_wdt.c | 74 ++ 1 files changed, 58 insertions(+), 16 deletions(-) diff --git a/drivers/watchdog/w83627hf_wdt.c b/drivers/watchdog/w83627hf_wdt.c index 92f1326..8fd 100644 --- a/drivers/watchdog/w83627hf_wdt.c +++ b/drivers/watchdog/w83627hf_wdt.c @@ -45,6 +45,7 @@ #define WATCHDOG_NAME w83627hf/thf/hg/dhg WDT #define WATCHDOG_TIMEOUT 60/* 60 sec default timeout */ +#define WATCHDOG_MAX_TIMEOUT (60 * 255) static unsigned long wdt_is_open; static char expect_close; @@ -55,11 +56,14 @@ static int wdt_io = 0x2E; module_param(wdt_io, int, 0); MODULE_PARM_DESC(wdt_io, w83627hf/thf WDT io port (default 0x2E)); +static bool use_minute; /* timeout unit in minutes if set */ +static const int max_timeout = WATCHDOG_MAX_TIMEOUT; static int timeout = WATCHDOG_TIMEOUT; /* in seconds */ module_param(timeout, int, 0); MODULE_PARM_DESC(timeout, - Watchdog timeout in seconds. 1 = timeout = 255, default= - __MODULE_STRING(WATCHDOG_TIMEOUT) .); + Watchdog timeout in seconds. 1= timeout = + __MODULE_STRING(WATCHDOG_MAX_TIMEOUT) (default= + __MODULE_STRING(WATCHDOG_TIMEOUT) )); static bool nowayout = WATCHDOG_NOWAYOUT; module_param(nowayout, bool, 0); @@ -107,6 +111,48 @@ static void w83627hf_unselect_wd_register(void) outb_p(0xAA, WDT_EFER); /* Leave extended function mode */ } +/* setup CRF5 register */ +static void w83627hf_setup_crf5(void) +{ + unsigned char t; + + outb_p(0xF5, WDT_EFER); /* Select CRF5 */ + t = inb_p(WDT_EFDR); /* read CRF5 */ + t = ~0x0C; /* set second mode disable keyboard + turning off watchdog */ + t |= 0x02;/* enable the WDTO# output low pulse + to the KBRST# pin (PIN60) */ + if (use_minute) + t |= 0x8;/* set timeout in minute */ + + outb_p(t, WDT_EFDR);/* Write back to CRF5 */ +} + +/* set use_minute if t 255 */ +static inline int wdt_use_minute(int t) +{ + if (t 255) { + t = DIV_ROUND_UP(t, 60); /* convert secs to minutes */ + use_minute = 1; + } else { + use_minute = 0; + } + + /* setup CRF5 for new timeout unit */ + w83627hf_select_wd_register(); + w83627hf_setup_crf5(); + w83627hf_unselect_wd_register(); + + return t; +} + +/* convert specified value to seconds if use_minute is set */ +static inline int wdt_get_timeout_secs(int t) +{ + return (use_minute) ? (t * 60) : t; +} + + /* tyan motherboards seem to set F5 to 0x4C ? * So explicitly init to appropriate value. */ @@ -120,17 +166,11 @@ static void w83627hf_init(void) t = inb_p(WDT_EFDR); /* read CRF6 */ if (t != 0) { pr_info(Watchdog already running. Resetting timeout to %d sec\n, - timeout); + wdt_get_timeout_secs(timeout)); outb_p(timeout, WDT_EFDR);/* Write back to CRF6 */ } - outb_p(0xF5, WDT_EFER); /* Select CRF5 */ - t = inb_p(WDT_EFDR); /* read CRF5 */ - t = ~0x0C; /* set second mode disable keyboard - turning off watchdog */ - t |= 0x02;/* enable the WDTO# output low pulse - to the KBRST# pin (PIN60) */ - outb_p(t, WDT_EFDR);/* Write back to CRF5 */ + w83627hf_setup_crf5(); outb_p(0xF7, WDT_EFER); /* Select CRF7 */ t = inb_p(WDT_EFDR); /* read CRF7 */ @@ -169,9 +209,9 @@ static int wdt_disable(void) static int wdt_set_heartbeat(int t) { - if (t 1 || t 255) + if (t 1 || t max_timeout) return -EINVAL; - timeout = t; + timeout = wdt_use_minute(t); return 0; } @@ -190,7 +230,7 @@ static int wdt_get_time(void) spin_unlock(io_lock); - return timeleft; + return wdt_get_timeout_secs(timeleft); } static ssize_t wdt_write(struct file *file, const char __user *buf, @@ -262,7 +302,8 @@ static long wdt_ioctl(struct file *file, unsigned int cmd, unsigned long arg) wdt_ping(); /* Fall */ case WDIOC_GETTIMEOUT: - return put_user(timeout, p); + timeval = wdt_get_timeout_secs(timeout); + return put_user
[PATCH v3 2/2] watchdog: fix w83627hf_wdt clear timeout expired
Observed that the w83627hf watchdog timer start counting during reboot. If the system load the driver after 5 minutes, it rebooted immediately because of timer expired. For example, fsck took more than 5 minutes to run, then the computer reboot as soon as the driver was loaded. Signed-off-by: Tony Chung tonychun...@gmail.com --- drivers/watchdog/w83627hf_wdt.c |6 -- 1 files changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/watchdog/w83627hf_wdt.c b/drivers/watchdog/w83627hf_wdt.c index 8fd..7eaa226 100644 --- a/drivers/watchdog/w83627hf_wdt.c +++ b/drivers/watchdog/w83627hf_wdt.c @@ -168,14 +168,16 @@ static void w83627hf_init(void) pr_info(Watchdog already running. Resetting timeout to %d sec\n, wdt_get_timeout_secs(timeout)); outb_p(timeout, WDT_EFDR);/* Write back to CRF6 */ + } else { + outb_p(0, WDT_EFDR);/* disable to prevent reboot */ } w83627hf_setup_crf5(); outb_p(0xF7, WDT_EFER); /* Select CRF7 */ t = inb_p(WDT_EFDR); /* read CRF7 */ - t = ~0xC0; /* disable keyboard mouse turning off - watchdog */ + t = ~0xD0; /* clear timeout occurred and disable keyboard + mouse turning off watchdog */ outb_p(t, WDT_EFDR);/* Write back to CRF7 */ w83627hf_unselect_wd_register(); -- 1.7.0.4 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 2/2] watchdog: fix w83627hf_wdt clear timeout expired
Thanks Guenter! I agree with you. My first reaction was also about a small watchdog server that will start in early boot process. There are pros and cons. For example, there are many types of watchdog devices such as ipmi_watchdog which can accept more than 255 seconds for timeout. So you really need udev to pick the correct watchdog driver. It could be very complex. Our requirement don't need watchdog protection during the boot process until application is fully up but a driver should not assume anything. Anyway, an unexpected reboot is definitely a bug that need to be fixed. It is really easily reproducible. Depending on your hardware and BIOS settings, just reboot the boot, wait for 5 minutes and then run insmod w83627hf_wdt.ko. The box just reboot by itself. The watchdog sever is not even started. This line is actually the original fix that is running over a year: outb_p(0, WDT_EFDR);/* disable to prevent reboot */ When I tried to cleanup it up, I thought I don't need it but it turned out it was still needed. When I changed it from 0xC0 to 0xD0, it still reboot. Thanks and best regards, @Tony On Mon, Apr 1, 2013 at 5:07 PM, Guenter Roeck li...@roeck-us.net wrote: On Sun, Mar 31, 2013 at 10:59:32PM -0700, Tony Chung wrote: Observed that the w83627hf watchdog timer start counting during reboot. If the system load the driver after 5 minutes, it rebooted immediately because of timer expired. For example, fsck took more than 5 minutes to run, then the computer reboot as soon as the driver was loaded. Thinking about it, I wonder if that really solves your problem. If the watchdog driver is built into the kernel, and if the watchdog is already running, you still may have the problem that it may time out again and trigger (again) before the watchdog application can be loaded. Ultimately, in such a situation, the watchdog application, or at least a primitive one, should really be started from initramfs and possibly be replaced later on by the real watchdog application. The watchdog package includes a small binary for that purpose, though I have no idea if is is loaded in initramfs or only later. Ultimately, that also applies to the original problem you tried to solve with the longer timeout. It doesn't seem correct to have to set a long timeout to address excessive boot times; in a way that defeats the purpose of a watchdog. A cleaner solution for that problem would be to make sure that a small watchdog application is started early in the boot process. Signed-off-by: Tony Chung tonychun...@gmail.com --- drivers/watchdog/w83627hf_wdt.c |6 -- 1 files changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/watchdog/w83627hf_wdt.c b/drivers/watchdog/w83627hf_wdt.c index 8fd..7eaa226 100644 --- a/drivers/watchdog/w83627hf_wdt.c +++ b/drivers/watchdog/w83627hf_wdt.c @@ -168,14 +168,16 @@ static void w83627hf_init(void) pr_info(Watchdog already running. Resetting timeout to %d sec\n, wdt_get_timeout_secs(timeout)); outb_p(timeout, WDT_EFDR);/* Write back to CRF6 */ + } else { + outb_p(0, WDT_EFDR);/* disable to prevent reboot */ } Did I miss that earlier, or did yo add it in this version ? Reading the counter register just returned 0, so what are the benefits of writing 0 into it ? Thanks, Guenter w83627hf_setup_crf5(); outb_p(0xF7, WDT_EFER); /* Select CRF7 */ t = inb_p(WDT_EFDR); /* read CRF7 */ - t = ~0xC0; /* disable keyboard mouse turning off - watchdog */ + t = ~0xD0; /* clear timeout occurred and disable keyboard + mouse turning off watchdog */ outb_p(t, WDT_EFDR);/* Write back to CRF7 */ w83627hf_unselect_wd_register(); -- 1.7.0.4 -- To unsubscribe from this list: send the line unsubscribe linux-watchdog in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- - Tony Chung -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCHv2 2/2] watchdog: fix w83627hf_wdt reboot due to timeout expired
How about this? - t &= ~0xC0; /* disable keyboard & mouse turning off - watchdog */ + t &= ~0xE0; /* clear timeout occurred and disable keyboard +& mouse turning off watchdog */ Thanks, @Tony -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCHv2 2/2] watchdog: fix w83627hf_wdt reboot due to timeout expired
How about this? - t = ~0xC0; /* disable keyboard mouse turning off - watchdog */ + t = ~0xE0; /* clear timeout occurred and disable keyboard + mouse turning off watchdog */ Thanks, @Tony -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCHv2 2/2] watchdog: fix w83627hf_wdt reboot due to timeout expired
Observed that the w83627hf watchdog timer start counting during reboot. If the system load the driver after 5 minutes, it rebooted immediately because of timer expired. For example, fsck took more than 5 minutes to run, then reboot will occurred. Signed-off-by: Tony Chung --- drivers/watchdog/w83627hf_wdt.c |5 + 1 files changed, 5 insertions(+), 0 deletions(-) diff --git a/drivers/watchdog/w83627hf_wdt.c b/drivers/watchdog/w83627hf_wdt.c index 8fd..47eb233 100644 --- a/drivers/watchdog/w83627hf_wdt.c +++ b/drivers/watchdog/w83627hf_wdt.c @@ -174,6 +174,11 @@ static void w83627hf_init(void) outb_p(0xF7, WDT_EFER); /* Select CRF7 */ t = inb_p(WDT_EFDR); /* read CRF7 */ + if (t & 0x10) { + pr_info("Watchdog Timer timeout occurred!"); + t &= ~0x10; /* clear the event */ + pr_info("Event cleared\n"); + } t &= ~0xC0; /* disable keyboard & mouse turning off watchdog */ outb_p(t, WDT_EFDR);/* Write back to CRF7 */ -- 1.7.0.4 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCHv2 1/2] watchdog: improve w83627hf_wdt to timeout in minute
Watchdog should support timeout in minutes. For example, crash dump will usually require 5+ minutes. This patch increase the timeout from 255 seconds to (255*60) seconds and should be good for older LTS releases. This patch need to be rewritten again when this driver migrate to new watchdog infrastructure. Added a new max_timeout variable so we can change it if needed. Signed-off-by: Tony Chung --- drivers/watchdog/w83627hf_wdt.c | 74 ++ 1 files changed, 58 insertions(+), 16 deletions(-) diff --git a/drivers/watchdog/w83627hf_wdt.c b/drivers/watchdog/w83627hf_wdt.c index 92f1326..8fd 100644 --- a/drivers/watchdog/w83627hf_wdt.c +++ b/drivers/watchdog/w83627hf_wdt.c @@ -45,6 +45,7 @@ #define WATCHDOG_NAME "w83627hf/thf/hg/dhg WDT" #define WATCHDOG_TIMEOUT 60/* 60 sec default timeout */ +#define WATCHDOG_MAX_TIMEOUT (60 * 255) static unsigned long wdt_is_open; static char expect_close; @@ -55,11 +56,14 @@ static int wdt_io = 0x2E; module_param(wdt_io, int, 0); MODULE_PARM_DESC(wdt_io, "w83627hf/thf WDT io port (default 0x2E)"); +static bool use_minute; /* timeout unit in minutes if set */ +static const int max_timeout = WATCHDOG_MAX_TIMEOUT; static int timeout = WATCHDOG_TIMEOUT; /* in seconds */ module_param(timeout, int, 0); MODULE_PARM_DESC(timeout, - "Watchdog timeout in seconds. 1 <= timeout <= 255, default=" - __MODULE_STRING(WATCHDOG_TIMEOUT) "."); + "Watchdog timeout in seconds. 1<= timeout <=" + __MODULE_STRING(WATCHDOG_MAX_TIMEOUT) " (default=" + __MODULE_STRING(WATCHDOG_TIMEOUT) ")"); static bool nowayout = WATCHDOG_NOWAYOUT; module_param(nowayout, bool, 0); @@ -107,6 +111,48 @@ static void w83627hf_unselect_wd_register(void) outb_p(0xAA, WDT_EFER); /* Leave extended function mode */ } +/* setup CRF5 register */ +static void w83627hf_setup_crf5(void) +{ + unsigned char t; + + outb_p(0xF5, WDT_EFER); /* Select CRF5 */ + t = inb_p(WDT_EFDR); /* read CRF5 */ + t &= ~0x0C; /* set second mode & disable keyboard + turning off watchdog */ + t |= 0x02;/* enable the WDTO# output low pulse + to the KBRST# pin (PIN60) */ + if (use_minute) + t |= 0x8;/* set timeout in minute */ + + outb_p(t, WDT_EFDR);/* Write back to CRF5 */ +} + +/* set use_minute if t > 255 */ +static inline int wdt_use_minute(int t) +{ + if (t > 255) { + t = DIV_ROUND_UP(t, 60); /* convert secs to minutes */ + use_minute = 1; + } else { + use_minute = 0; + } + + /* setup CRF5 for new timeout unit */ + w83627hf_select_wd_register(); + w83627hf_setup_crf5(); + w83627hf_unselect_wd_register(); + + return t; +} + +/* convert specified value to seconds if use_minute is set */ +static inline int wdt_get_timeout_secs(int t) +{ + return (use_minute) ? (t * 60) : t; +} + + /* tyan motherboards seem to set F5 to 0x4C ? * So explicitly init to appropriate value. */ @@ -120,17 +166,11 @@ static void w83627hf_init(void) t = inb_p(WDT_EFDR); /* read CRF6 */ if (t != 0) { pr_info("Watchdog already running. Resetting timeout to %d sec\n", - timeout); + wdt_get_timeout_secs(timeout)); outb_p(timeout, WDT_EFDR);/* Write back to CRF6 */ } - outb_p(0xF5, WDT_EFER); /* Select CRF5 */ - t = inb_p(WDT_EFDR); /* read CRF5 */ - t &= ~0x0C; /* set second mode & disable keyboard - turning off watchdog */ - t |= 0x02;/* enable the WDTO# output low pulse - to the KBRST# pin (PIN60) */ - outb_p(t, WDT_EFDR);/* Write back to CRF5 */ + w83627hf_setup_crf5(); outb_p(0xF7, WDT_EFER); /* Select CRF7 */ t = inb_p(WDT_EFDR); /* read CRF7 */ @@ -169,9 +209,9 @@ static int wdt_disable(void) static int wdt_set_heartbeat(int t) { - if (t < 1 || t > 255) + if (t < 1 || t > max_timeout) return -EINVAL; - timeout = t; + timeout = wdt_use_minute(t); return 0; } @@ -190,7 +230,7 @@ static int wdt_get_time(void) spin_unlock(_lock); - return timeleft; + return wdt_get_timeout_secs(timeleft); } static ssize_t wdt_write(struct file *file, const char __user *buf, @@ -262,7 +302,8 @@ static long wdt_ioctl(struct file *file, unsigned int cmd, unsigned long arg) wdt_ping(); /* Fall */ case WDIOC_GETTIMEOUT
[PATCHv2 1/2] watchdog: improve w83627hf_wdt to timeout in minute
Watchdog should support timeout in minutes. For example, crash dump will usually require 5+ minutes. This patch increase the timeout from 255 seconds to (255*60) seconds and should be good for older LTS releases. This patch need to be rewritten again when this driver migrate to new watchdog infrastructure. Added a new max_timeout variable so we can change it if needed. Signed-off-by: Tony Chung tonychun...@gmail.com --- drivers/watchdog/w83627hf_wdt.c | 74 ++ 1 files changed, 58 insertions(+), 16 deletions(-) diff --git a/drivers/watchdog/w83627hf_wdt.c b/drivers/watchdog/w83627hf_wdt.c index 92f1326..8fd 100644 --- a/drivers/watchdog/w83627hf_wdt.c +++ b/drivers/watchdog/w83627hf_wdt.c @@ -45,6 +45,7 @@ #define WATCHDOG_NAME w83627hf/thf/hg/dhg WDT #define WATCHDOG_TIMEOUT 60/* 60 sec default timeout */ +#define WATCHDOG_MAX_TIMEOUT (60 * 255) static unsigned long wdt_is_open; static char expect_close; @@ -55,11 +56,14 @@ static int wdt_io = 0x2E; module_param(wdt_io, int, 0); MODULE_PARM_DESC(wdt_io, w83627hf/thf WDT io port (default 0x2E)); +static bool use_minute; /* timeout unit in minutes if set */ +static const int max_timeout = WATCHDOG_MAX_TIMEOUT; static int timeout = WATCHDOG_TIMEOUT; /* in seconds */ module_param(timeout, int, 0); MODULE_PARM_DESC(timeout, - Watchdog timeout in seconds. 1 = timeout = 255, default= - __MODULE_STRING(WATCHDOG_TIMEOUT) .); + Watchdog timeout in seconds. 1= timeout = + __MODULE_STRING(WATCHDOG_MAX_TIMEOUT) (default= + __MODULE_STRING(WATCHDOG_TIMEOUT) )); static bool nowayout = WATCHDOG_NOWAYOUT; module_param(nowayout, bool, 0); @@ -107,6 +111,48 @@ static void w83627hf_unselect_wd_register(void) outb_p(0xAA, WDT_EFER); /* Leave extended function mode */ } +/* setup CRF5 register */ +static void w83627hf_setup_crf5(void) +{ + unsigned char t; + + outb_p(0xF5, WDT_EFER); /* Select CRF5 */ + t = inb_p(WDT_EFDR); /* read CRF5 */ + t = ~0x0C; /* set second mode disable keyboard + turning off watchdog */ + t |= 0x02;/* enable the WDTO# output low pulse + to the KBRST# pin (PIN60) */ + if (use_minute) + t |= 0x8;/* set timeout in minute */ + + outb_p(t, WDT_EFDR);/* Write back to CRF5 */ +} + +/* set use_minute if t 255 */ +static inline int wdt_use_minute(int t) +{ + if (t 255) { + t = DIV_ROUND_UP(t, 60); /* convert secs to minutes */ + use_minute = 1; + } else { + use_minute = 0; + } + + /* setup CRF5 for new timeout unit */ + w83627hf_select_wd_register(); + w83627hf_setup_crf5(); + w83627hf_unselect_wd_register(); + + return t; +} + +/* convert specified value to seconds if use_minute is set */ +static inline int wdt_get_timeout_secs(int t) +{ + return (use_minute) ? (t * 60) : t; +} + + /* tyan motherboards seem to set F5 to 0x4C ? * So explicitly init to appropriate value. */ @@ -120,17 +166,11 @@ static void w83627hf_init(void) t = inb_p(WDT_EFDR); /* read CRF6 */ if (t != 0) { pr_info(Watchdog already running. Resetting timeout to %d sec\n, - timeout); + wdt_get_timeout_secs(timeout)); outb_p(timeout, WDT_EFDR);/* Write back to CRF6 */ } - outb_p(0xF5, WDT_EFER); /* Select CRF5 */ - t = inb_p(WDT_EFDR); /* read CRF5 */ - t = ~0x0C; /* set second mode disable keyboard - turning off watchdog */ - t |= 0x02;/* enable the WDTO# output low pulse - to the KBRST# pin (PIN60) */ - outb_p(t, WDT_EFDR);/* Write back to CRF5 */ + w83627hf_setup_crf5(); outb_p(0xF7, WDT_EFER); /* Select CRF7 */ t = inb_p(WDT_EFDR); /* read CRF7 */ @@ -169,9 +209,9 @@ static int wdt_disable(void) static int wdt_set_heartbeat(int t) { - if (t 1 || t 255) + if (t 1 || t max_timeout) return -EINVAL; - timeout = t; + timeout = wdt_use_minute(t); return 0; } @@ -190,7 +230,7 @@ static int wdt_get_time(void) spin_unlock(io_lock); - return timeleft; + return wdt_get_timeout_secs(timeleft); } static ssize_t wdt_write(struct file *file, const char __user *buf, @@ -262,7 +302,8 @@ static long wdt_ioctl(struct file *file, unsigned int cmd, unsigned long arg) wdt_ping(); /* Fall */ case WDIOC_GETTIMEOUT: - return put_user(timeout, p); + timeval = wdt_get_timeout_secs(timeout); + return put_user
[PATCHv2 2/2] watchdog: fix w83627hf_wdt reboot due to timeout expired
Observed that the w83627hf watchdog timer start counting during reboot. If the system load the driver after 5 minutes, it rebooted immediately because of timer expired. For example, fsck took more than 5 minutes to run, then reboot will occurred. Signed-off-by: Tony Chung tonychun...@gmail.com --- drivers/watchdog/w83627hf_wdt.c |5 + 1 files changed, 5 insertions(+), 0 deletions(-) diff --git a/drivers/watchdog/w83627hf_wdt.c b/drivers/watchdog/w83627hf_wdt.c index 8fd..47eb233 100644 --- a/drivers/watchdog/w83627hf_wdt.c +++ b/drivers/watchdog/w83627hf_wdt.c @@ -174,6 +174,11 @@ static void w83627hf_init(void) outb_p(0xF7, WDT_EFER); /* Select CRF7 */ t = inb_p(WDT_EFDR); /* read CRF7 */ + if (t 0x10) { + pr_info(Watchdog Timer timeout occurred!); + t = ~0x10; /* clear the event */ + pr_info(Event cleared\n); + } t = ~0xC0; /* disable keyboard mouse turning off watchdog */ outb_p(t, WDT_EFDR);/* Write back to CRF7 */ -- 1.7.0.4 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/1] watchdog:improve w83627hf_wdt to timeout in minutes
The current maximum of 255 seconds is insufficient. For example, crash dump could take 5+ minutes. Signed-off-by: Tony Chung --- drivers/watchdog/w83627hf_wdt.c | 73 ++ 1 files changed, 57 insertions(+), 16 deletions(-) diff --git a/drivers/watchdog/w83627hf_wdt.c b/drivers/watchdog/w83627hf_wdt.c index 92f1326..d26adfb 100644 --- a/drivers/watchdog/w83627hf_wdt.c +++ b/drivers/watchdog/w83627hf_wdt.c @@ -58,7 +58,7 @@ MODULE_PARM_DESC(wdt_io, "w83627hf/thf WDT io port (default 0x2E)"); static int timeout = WATCHDOG_TIMEOUT; /* in seconds */ module_param(timeout, int, 0); MODULE_PARM_DESC(timeout, - "Watchdog timeout in seconds. 1 <= timeout <= 255, default=" + "Watchdog timeout in seconds. 1 <= timeout <= 15300, default=" __MODULE_STRING(WATCHDOG_TIMEOUT) "."); static bool nowayout = WATCHDOG_NOWAYOUT; @@ -67,6 +67,29 @@ MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started (default=" __MODULE_STRING(WATCHDOG_NOWAYOUT) ")"); +/* timeout unit in minute */ +static bool use_minute; +static char *unit; +static int max_timeout = 15300; /* 255*60 seconds */ + +static inline int wdt_round_secs_to_minute(int t) +{ + return (t + 59) / 60; +} + +static inline int wdt_use_minute(int t) +{ + if (t > 255) { + t = wdt_round_secs_to_minute(t); + use_minute = 1; + unit = "minutes"; + } else { + use_minute = 0; + unit = "secs"; + } + return t; +} + /* * Kernel methods. */ @@ -107,6 +130,23 @@ static void w83627hf_unselect_wd_register(void) outb_p(0xAA, WDT_EFER); /* Leave extended function mode */ } +/* set CRF5 register */ +static void w83627hf_set_crf5(void) +{ + unsigned char t; + + outb_p(0xF5, WDT_EFER); /* Select CRF5 */ + t = inb_p(WDT_EFDR); /* read CRF5 */ + t &= ~0x0C; /* set second mode & disable keyboard + turning off watchdog */ + t |= 0x02;/* enable the WDTO# output low pulse + to the KBRST# pin (PIN60) */ + if (use_minute) + t |= 0x80;/* set timeout in minute */ + + outb_p(t, WDT_EFDR);/* Write back to CRF5 */ +} + /* tyan motherboards seem to set F5 to 0x4C ? * So explicitly init to appropriate value. */ @@ -116,21 +156,16 @@ static void w83627hf_init(void) w83627hf_select_wd_register(); + w83627hf_set_crf5(); + outb_p(0xF6, WDT_EFER); /* Select CRF6 */ t = inb_p(WDT_EFDR); /* read CRF6 */ if (t != 0) { - pr_info("Watchdog already running. Resetting timeout to %d sec\n", - timeout); + pr_info("Watchdog already running. Reset timeout to %d %s\n", + timeout, unit); outb_p(timeout, WDT_EFDR);/* Write back to CRF6 */ } - outb_p(0xF5, WDT_EFER); /* Select CRF5 */ - t = inb_p(WDT_EFDR); /* read CRF5 */ - t &= ~0x0C; /* set second mode & disable keyboard - turning off watchdog */ - t |= 0x02;/* enable the WDTO# output low pulse - to the KBRST# pin (PIN60) */ - outb_p(t, WDT_EFDR);/* Write back to CRF5 */ outb_p(0xF7, WDT_EFER); /* Select CRF7 */ t = inb_p(WDT_EFDR); /* read CRF7 */ @@ -147,6 +182,9 @@ static void wdt_set_time(int timeout) w83627hf_select_wd_register(); + if (use_minute) + w83627hf_set_crf5(); + outb_p(0xF6, WDT_EFER);/* Select CRF6 */ outb_p(timeout, WDT_EFDR); /* Write Timeout counter to CRF6 */ @@ -169,9 +207,9 @@ static int wdt_disable(void) static int wdt_set_heartbeat(int t) { - if (t < 1 || t > 255) + if (t < 1 || t > max_timeout) return -EINVAL; - timeout = t; + timeout = wdt_use_minute(t); return 0; } @@ -262,9 +300,11 @@ static long wdt_ioctl(struct file *file, unsigned int cmd, unsigned long arg) wdt_ping(); /* Fall */ case WDIOC_GETTIMEOUT: - return put_user(timeout, p); + timeval = (use_minute) ? timeout * 60 : timeout; + return put_user(timeval, p); case WDIOC_GETTIMELEFT: timeval = wdt_get_time(); + timeval = (use_minute) ? timeval * 60 : timeval; return put_user(timeval, p); default: return -ENOTTY; @@ -346,7 +386,8 @@ static int __init wdt_init(void) if (wdt_set_heartbeat(timeout)) {
[PATCH 1/1] watchdog:improve w83627hf_wdt to timeout in minutes
The current maximum of 255 seconds is insufficient. For example, crash dump could take 5+ minutes. Signed-off-by: Tony Chung tonychun...@gmail.com --- drivers/watchdog/w83627hf_wdt.c | 73 ++ 1 files changed, 57 insertions(+), 16 deletions(-) diff --git a/drivers/watchdog/w83627hf_wdt.c b/drivers/watchdog/w83627hf_wdt.c index 92f1326..d26adfb 100644 --- a/drivers/watchdog/w83627hf_wdt.c +++ b/drivers/watchdog/w83627hf_wdt.c @@ -58,7 +58,7 @@ MODULE_PARM_DESC(wdt_io, w83627hf/thf WDT io port (default 0x2E)); static int timeout = WATCHDOG_TIMEOUT; /* in seconds */ module_param(timeout, int, 0); MODULE_PARM_DESC(timeout, - Watchdog timeout in seconds. 1 = timeout = 255, default= + Watchdog timeout in seconds. 1 = timeout = 15300, default= __MODULE_STRING(WATCHDOG_TIMEOUT) .); static bool nowayout = WATCHDOG_NOWAYOUT; @@ -67,6 +67,29 @@ MODULE_PARM_DESC(nowayout, Watchdog cannot be stopped once started (default= __MODULE_STRING(WATCHDOG_NOWAYOUT) )); +/* timeout unit in minute */ +static bool use_minute; +static char *unit; +static int max_timeout = 15300; /* 255*60 seconds */ + +static inline int wdt_round_secs_to_minute(int t) +{ + return (t + 59) / 60; +} + +static inline int wdt_use_minute(int t) +{ + if (t 255) { + t = wdt_round_secs_to_minute(t); + use_minute = 1; + unit = minutes; + } else { + use_minute = 0; + unit = secs; + } + return t; +} + /* * Kernel methods. */ @@ -107,6 +130,23 @@ static void w83627hf_unselect_wd_register(void) outb_p(0xAA, WDT_EFER); /* Leave extended function mode */ } +/* set CRF5 register */ +static void w83627hf_set_crf5(void) +{ + unsigned char t; + + outb_p(0xF5, WDT_EFER); /* Select CRF5 */ + t = inb_p(WDT_EFDR); /* read CRF5 */ + t = ~0x0C; /* set second mode disable keyboard + turning off watchdog */ + t |= 0x02;/* enable the WDTO# output low pulse + to the KBRST# pin (PIN60) */ + if (use_minute) + t |= 0x80;/* set timeout in minute */ + + outb_p(t, WDT_EFDR);/* Write back to CRF5 */ +} + /* tyan motherboards seem to set F5 to 0x4C ? * So explicitly init to appropriate value. */ @@ -116,21 +156,16 @@ static void w83627hf_init(void) w83627hf_select_wd_register(); + w83627hf_set_crf5(); + outb_p(0xF6, WDT_EFER); /* Select CRF6 */ t = inb_p(WDT_EFDR); /* read CRF6 */ if (t != 0) { - pr_info(Watchdog already running. Resetting timeout to %d sec\n, - timeout); + pr_info(Watchdog already running. Reset timeout to %d %s\n, + timeout, unit); outb_p(timeout, WDT_EFDR);/* Write back to CRF6 */ } - outb_p(0xF5, WDT_EFER); /* Select CRF5 */ - t = inb_p(WDT_EFDR); /* read CRF5 */ - t = ~0x0C; /* set second mode disable keyboard - turning off watchdog */ - t |= 0x02;/* enable the WDTO# output low pulse - to the KBRST# pin (PIN60) */ - outb_p(t, WDT_EFDR);/* Write back to CRF5 */ outb_p(0xF7, WDT_EFER); /* Select CRF7 */ t = inb_p(WDT_EFDR); /* read CRF7 */ @@ -147,6 +182,9 @@ static void wdt_set_time(int timeout) w83627hf_select_wd_register(); + if (use_minute) + w83627hf_set_crf5(); + outb_p(0xF6, WDT_EFER);/* Select CRF6 */ outb_p(timeout, WDT_EFDR); /* Write Timeout counter to CRF6 */ @@ -169,9 +207,9 @@ static int wdt_disable(void) static int wdt_set_heartbeat(int t) { - if (t 1 || t 255) + if (t 1 || t max_timeout) return -EINVAL; - timeout = t; + timeout = wdt_use_minute(t); return 0; } @@ -262,9 +300,11 @@ static long wdt_ioctl(struct file *file, unsigned int cmd, unsigned long arg) wdt_ping(); /* Fall */ case WDIOC_GETTIMEOUT: - return put_user(timeout, p); + timeval = (use_minute) ? timeout * 60 : timeout; + return put_user(timeval, p); case WDIOC_GETTIMELEFT: timeval = wdt_get_time(); + timeval = (use_minute) ? timeval * 60 : timeval; return put_user(timeval, p); default: return -ENOTTY; @@ -346,7 +386,8 @@ static int __init wdt_init(void) if (wdt_set_heartbeat(timeout)) { wdt_set_heartbeat(WATCHDOG_TIMEOUT); - pr_info(timeout value must be 1 = timeout = 255, using %d\n, + pr_info(timeout value