Date: Mon, 11 Dec 2006 16:08:06 -0500 From: John Baldwin <jhb@freebsd.org> To: freebsd-current@freebsd.org Cc: Nick Hibma <nick@van-laarhoven.org> Subject: Re: Slight interface change on the watchdog fido Message-ID: <200612111608.06677.jhb@freebsd.org> In-Reply-To: <20061210110419.H42195@localhost> References: <20061210110419.H42195@localhost>
next in thread | previous in thread | raw e-mail | index | archive | help
On Sunday 10 December 2006 05:04, Nick Hibma wrote:
> I'm planning on committing the following change to make the implementations
of
> the various hardware watchdogs more consistent. A timeout of 0 passed to the
> ioctl is now a valid input and will disable the watchdog. Previously it
> produced an error for the Elan chip watchdog, which is _not_ what you want
to
> see when disarming a watchdog :-) I'd appreciate review of the individual
> changes in the files by the following people. The change as a whole was
> discussed with phk.
>
> cognet@freebsd.org i80321_wdog.c (*)
> des ichwd.c (**)
> ambrisko ipmi.c
> marius mk48txx.c
> phk kern_clock.c, elan-mmcr.c, watchdog.c (**)
>
> (*) The i80321_wdog.c cannot be disarmed. Is this correct?
> (**) These have been tested to arm, disarm, and fire. Others have only been
> compile tested.
>
> This change has been tested on 6.2-STABLE and 7-CURRENT.
>
> Index: sys/dev/ipmi/ipmi.c
> ===================================================================
> RCS file: /home/ncvs/src/sys/dev/ipmi/ipmi.c,v
> retrieving revision 1.3.2.3
> diff -u -r1.3.2.3 ipmi.c
> --- sys/dev/ipmi/ipmi.c 19 Oct 2006 14:50:48 -0000 1.3.2.3
> +++ sys/dev/ipmi/ipmi.c 9 Dec 2006 12:40:47 -0000
> @@ -649,25 +649,14 @@
> struct ipmi_softc *sc = arg;
> unsigned int timeout;
>
> - /* disable / enable */
> - if (!(cmd & WD_ACTIVE)) {
> - ipmi_set_watchdog(sc, 0);
> - *error = 0;
> - return;
> - }
> -
> cmd &= WD_INTERVAL;
> - /* convert from power-of-to-ns to WDT ticks */
> - if (cmd >= 64) {
> - *error = EINVAL;
> - return;
> + if (cmd > 0 && cmd <= 63) {
> + timeout = ((uint64_t)1 << cmd) / 1800000000;
> + ipmi_set_watchdog(sc, timeout);
> + *error = 0;
> + } else {
> + ipmi_set_watchdog(sc, 0);
> }
> - timeout = ((uint64_t)1 << cmd) / 1800000000;
> -
> - /* reload */
> - ipmi_set_watchdog(sc, timeout);
> -
> - *error = 0;
> }
>
> #ifdef CLONING
It would be nice to not lose the comments. Might also be nice to reduce the
diff (so it doesn't have to reindent everything) by just adding a simple test
after masking off WD_INTERVAL like so:
if (cmd == 0 || cmd >= 64) {
ipmi_set_watchdog(sc, 0);
return;
}
--
John Baldwin
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200612111608.06677.jhb>