2006-01-14 02:59:44

by Calin A. Culianu

[permalink] [raw]
Subject: [PATCH] Watchdog: Winsystems EPX-C3 SBC


Hi all,

This is a 2.6 patch that adds support for the watchdog timer built into
the EPX-C3 single board computer manufactured by Winsystems, Inc.

Driver details:

This is for x86 only. This watchdog is pretty basic and simple. It is
only configurable via jumpers on the SBC, and it only has either a 1.5s or
200s interval. The watchdog can either be auto-configured to start as
soon as the machine powers up (bad idea for the 1.5s interval!) or it can
be enabled and disabled by writing to io port 0x1ee. Petting the watchdog
involves writing any value to io port 0x1ef.

The only unfortunate thing about this watchdog (and it is not at all
uncommmon in watchdogs that linux supports) is that it is not a PCI or
ISA-PNP device and as such it isn't at all probeable. Either the watchdog
exists as 2 bytes at 0x1ee, or it doesn't. Thus, using this driver on a
machine that doesn't have that watchdog can potentially hang/crash the
system, etc. So only use this driver if you in fact are on a Winsystems
EPX-C3 SBC.

Anyway this driver fits into the already-existing watchdog framework quite
nicely and I already tested it on my EPX-C3 and it works like a
charm.

Please accept it! :)

Thanks,

-Calin


Attachments:
sbc_epx_c3_watchdog-2.6.patch (7.63 kB)

2006-01-14 19:20:48

by Alan

[permalink] [raw]
Subject: Re: [PATCH] Watchdog: Winsystems EPX-C3 SBC

Some quick comments:

+ if (len) {
+ epx_c3_pet();
+ }

Doesn't need brackets (minor style)

Otherwise it looks excellent but should use request_region and friends
to claim the two ports it uses.

The see the "Sign your work:" bit of Documentation/SubmittingPatches are
it looks ready to go in.

Alan

2006-01-14 20:08:51

by Willy Tarreau

[permalink] [raw]
Subject: Re: [PATCH] Watchdog: Winsystems EPX-C3 SBC

On Sat, Jan 14, 2006 at 07:24:09PM +0000, Alan Cox wrote:
> Some quick comments:
>
> + if (len) {
> + epx_c3_pet();
> + }
>
> Doesn't need brackets (minor style)
>
> Otherwise it looks excellent but should use request_region and friends
> to claim the two ports it uses.

You just remind me that I had to comment out the request_region in another
watchdog driver I wrote, because the hardware was mapped on I/O port 0xF2
which is within the FPU I/O space :

00f0-00ff : fpu

I did not know how to correctly fix this problem, and I could live without
the check, but I found it dirty and never proposed it for inclusion.

It's not the first time I notice that write-only hardware share a reserved
I/O range with other components, and I don't know how to cope with this.
Perhaps it sounds logical not to request_region() as the hardware is meant
to be 100% transparent afterall ?

> The see the "Sign your work:" bit of Documentation/SubmittingPatches are
> it looks ready to go in.
>
> Alan

Willy

2006-01-30 03:49:37

by Marcelo Tosatti

[permalink] [raw]
Subject: Re: [PATCH] Watchdog: Winsystems EPX-C3 SBC

On Sat, Jan 14, 2006 at 07:24:09PM +0000, Alan Cox wrote:
> Some quick comments:
>
> + if (len) {
> + epx_c3_pet();
> + }
>
> Doesn't need brackets (minor style)
>
> Otherwise it looks excellent but should use request_region and friends
> to claim the two ports it uses.

It misses locking to protect against two concurrent writers in case of
a shared /dev/wdt file-descriptor?