Re: [2.4] Watchdog wdt977 (Winbond W83977EF) driver

From: Willy Tarreau
Date: Mon Apr 02 2007 - 01:39:12 EST


Hello,

On Sun, Apr 01, 2007 at 05:14:07PM +0300, Tal Kelrich wrote:
> (resent due to mailer stupidity)
> Hello,
>
> This is my first submitted kernel patch, please be gentle.
>
> Tested and working on AAEON GENE-6310B Subcompact Board
> (also configured for same by default, should work elsewhere)
> patch is against kernel 2.4.34.2
>
> Changes/Features:
>
> Added ioctl support
> Disables watchdog on driver load
> Supports timeout in seconds
> Timeout defaults to 2 minutes
> No longer under NetWinder arch
> Configurable output GP (defaults to GP16)
> Configurable base IO address
> Non standard read only proc interface for status (/proc/watchdog)
>
> Caveats:
>
> No idea if this breaks netwinder, although it really shouldn't
> Only tested with GP16
> Utterly ignores inability to get its IO port, mostly because it's
> already taken. I didn't know a way around that.
> release_region is called regardless of having acquired the region, this
> might be trouble.

While I have no particular problem merging your other driver since it
did not previously exist, I don't like much this one being massively
changed like this, particularly when you cannot test whether it breaks
its original platform (netwinder). I've also noticed that your patch
removes some locking, so I'm even doubting that it will not break
NetWinder.

A less risky solution would be to make a new driver of it (eg: w93977.c
instead of wdt977.c). Also, if you proceed like this, please do not
forget to write an entry in Configure.help.

Regards,
Willy

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/