2010-12-06 19:26:10

by Giel van Schijndel

[permalink] [raw]
Subject: Re: Fwd: Patch for Fintek F71869 watchdoh

On Mon, Dec 06, 2010 at 11:14:05 +0100, Hans de Goede wrote:
>> -------- Original Message --------
>> Subject: Patch for Fintek F71869 watchdoh
>> Date: Mon, 6 Dec 2010 00:40:43 +0100
>> From: Michel Arboi <[email protected]>
>> Organization: Compilo ergo sum
>> To: Hans de Goede <[email protected]>
>>
>> I'm not sure that you are the right person, but actually I do not know
>> who I should contact. I saw that you were working on the support of
>> the Fintek F71869 sensors.

These are the e-mail addresses to send patches for that driver to:
> ~/git/linux-2.6 $ scripts/get_maintainer.pl -f drivers/watchdog/f71808e_wdt.c
> Wim Van Sebroeck <[email protected]>
> Giel van Schijndel <[email protected]>
> [email protected]
> [email protected]

>> I have a Jetway NC9C-550-LF mobo, I quickly hacked this to support the
>> watchdog. It seems to work.
>
> Here is a patch send to me to add support for the F71869 to the Fintek
> superio watchdog driver.
>
> Michel, I've forwarded you're patch to Giel, who is the author and
> maintainer of the Fintek superio watchdog driver.

> --- ./drivers/watchdog/f71808e_wdt.c 2010-10-20 22:30:22.000000000 +0200
> +++ /tmp/f71808e_wdt.c 2010-12-05 01:19:48.819567802 +0100
> @@ -52,6 +52,8 @@
> #define SIO_F71882_ID 0x0541 /* Chipset ID */
> #define SIO_F71889_ID 0x0723 /* Chipset ID */
>
> +#define SIO_F71869_ID 0x0814
> +
> #define F71882FG_REG_START 0x01
>
> #define F71808FG_REG_WDO_CONF 0xf0

Please insert that constant in the list of other Chipset ID's
alphabetically sorted (to keep that list consistent).

> @@ -98,7 +100,7 @@
> MODULE_PARM_DESC(start_withtimeout, "Start watchdog timer on module load with"
> " given initial timeout. Zero (default) disables this feature.");
>
> -enum chips { f71808fg, f71858fg, f71862fg, f71882fg, f71889fg };
> +enum chips { f71808fg, f71858fg, f71862fg, f71882fg, f71889fg, f71869 };
>
> static const char *f71808e_names[] = {
> "f71808fg",
> @@ -106,6 +108,7 @@
> "f71862fg",
> "f71882fg",
> "f71889fg",
> + "f71869",
> };
>
> /* Super-I/O Function prototypes */
> @@ -308,6 +311,10 @@
> superio_set_bit(watchdog.sioaddr, 0x29, 1);
> break;
>
> + case f71869:
> + /* GPIO14 --> WDTRST# */
> + superio_clear_bit(watchdog.sioaddr, 0x29, 4);
> + break;
> default:
> /*
> * 'default' label to shut up the compiler and catch
> @@ -708,6 +715,9 @@
> case SIO_F71882_ID:
> watchdog.type = f71882fg;
> break;
> + case SIO_F71869_ID:
> + watchdog.type = f71869;
> + break;
> case SIO_F71862_ID:
> case SIO_F71889_ID:
> /* These have a watchdog, though it isn't implemented (yet). */

The rest of that patch looks fine. So if you perform the above
modification you'll get my Ack.

--
Met vriendelijke groet,
With kind regards,
Giel van Schijndel
--
"Measuring programming progress by lines of code is like measuring
aircraft building progress by weight."
-- Bill Gates


Attachments:
(No filename) (2.88 kB)
signature.asc (198.00 B)
Digital signature
Download all attachments

2010-12-06 20:02:23

by Michel Arboi

[permalink] [raw]
Subject: Re: Patch for Fintek F71869 watchdoh

On Mon, 6 Dec 2010 20:19:30 +0100
Giel van Schijndel <[email protected]> wrote:

> Please insert that constant in the list of other Chipset ID's
> alphabetically sorted (to keep that list consistent).

Like this?

> The rest of that patch looks fine. So if you perform the above
> modification you'll get my Ack.

Please note that this was just a quick hack. My basic tests showed that
the board did not reboot by itself, and it rebooted when I stopped the
watchdog process.
I downloaded the chip datasheet from Fintek's site but could not make
much sense out of it. It is rather terse.


Attachments:
(No filename) (584.00 B)
f71808e_wdt.patch (1.34 kB)
Download all attachments

2010-12-06 20:22:05

by Giel van Schijndel

[permalink] [raw]
Subject: Re: Patch for Fintek F71869 watchdoh

On Mon, Dec 06, 2010 at 20:53:45 +0100, Michel Arboi wrote:
> On Mon, 6 Dec 2010 20:19:30 +0100 Giel van Schijndel <[email protected]> wrote:
>> Please insert that constant in the list of other Chipset ID's
>> alphabetically sorted (to keep that list consistent).
>
> Like this?

Aye, looks good.

Acked-By: Giel van Schijndel <[email protected]>

>> The rest of that patch looks fine. So if you perform the above
>> modification you'll get my Ack.
>
> Please note that this was just a quick hack. My basic tests showed
> that the board did not reboot by itself, and it rebooted when I
> stopped the watchdog process.

That's intended behaviour and probably sufficient for testing. Most of
the behaviour isn't chip-specific, only the initialisation and
configuration of the chip is specific to that chip AFAIK.

> I downloaded the chip datasheet from Fintek's site but could not make
> much sense out of it. It is rather terse.

Yes, those datasheets can easily give you a headache when you're trying
to make sense of them.

PS You might want to include patches inline (i.e. as part of the message
text) as that's standard procedure for Linux MLs. That allows easy
reviewing and commenting on patches.

--
Met vriendelijke groet,
With kind regards,
Giel van Schijndel
--
"Always code as if the guy who ends up maintaining your code will be a
violent psychopath who knows where you live."
-- Rick Osborne


Attachments:
(No filename) (1.38 kB)
signature.asc (198.00 B)
Digital signature
Download all attachments

2010-12-06 22:42:17

by Michel Arboi

[permalink] [raw]
Subject: Re: Patch for Fintek F71869 watchdoh

On Mon, 6 Dec 2010 21:21:55 +0100
Giel van Schijndel <[email protected]> wrote:

> That's intended behaviour and probably sufficient for testing. Most
> of the behaviour isn't chip-specific, only the initialisation and
> configuration of the chip is specific to that chip AFAIK.

OK. Hans, if you need some tests for the sensors part, you can send me a
patch and I'll try to help.
[I suspect that the current Linux code works, sensors reports 62 ?C /
59 ?C for the CPU]

> PS You might want to include patches inline (i.e. as part of the
> message text) as that's standard procedure for Linux MLs. That
> allows easy reviewing and commenting on patches.

OK. In fact I did not know how to submit a kernel patch.