Re: [PATCH 4/4] watchdog: it87_wdt: Keep WDTCTRL bit 3 unmodified for IT8784/IT8786

[Guenter Roeck <linux@roeck-us.net>]

On 7/6/24 12:06, James Hilliard wrote:
> On Wed, Dec 13, 2023 at 1:45 AM Werner Fischer <devlists@wefi.net> wrote:
>>
>> WDTCTRL bit 3 sets the mode choice for the clock input of IT8784/IT8786.
>> Some motherboards require this bit to be set to 1 (= PCICLK mode),
>> otherwise the watchdog functionality gets broken. The BIOS of those
>> motherboards sets WDTCTRL bit 3 already to 1.
>>
>> Instead of setting all bits of WDTCTRL to 0 by writing 0x00 to it, keep
>> bit 3 of it unchanged for IT8784/IT8786 chips. In this way, bit 3 keeps
>> the status as set by the BIOS of the motherboard.
> 
> I have a board(https://qotom.net/product/94.html) with an IT8786
> revision 4 which is recognized but doesn't seem to ever trigger. Did
> your IT8786 based boards show revision 4 like mine do?
> 
> [    1.607590] it87_wdt: Chip IT8786 revision 4 initialized.
> timeout=60 sec (nowayout=0 testmode=0)
> [    2.367608] systemd[1]: Using hardware watchdog 'IT87 WDT', version
> 1, device /dev/watchdog0
> 
> Docs I have from the vendor just show bit 3 as reserved:
> 
> https://qotom.us/download/SuperIO/IT8786_B_V0.2_industrial_111412.pdf
> 
> 8.10.8 Watch Dog Timer Control Register (Index=71h, Default=00h)
> 
> Bit      Description
> 7        WDT is reset upon a CIR interrupt.
> 6        WDT is reset upon a KBC(Mouse) interrupt.
> 5        WDT is reset upon a KBC(Keyboard) interrupt.
> 4        WDT Status will not be cleared by VCCOK or LRESET#, and only
> be cleared while write one to WDT Status
>           1: Enable
>           0: Disable
> 3-2      Reserved
> 1        Force Time-out
>           This bit is self-cleared.
> 0        WDT Status
>           1: WDT value is equal to 0.
>           0: WDT value is not is equal to 0.
> 
> Any idea why the docs I have would just show bit 3 as reserved?
> 
> Did you have any information from your vendor under what conditions
> bit 3 should be set?
> 

On ITE8784E bit 3 is "External CLK_IN Select".

>>
>> Watchdog tests have been successful with this patch with the following
>> systems:
>>    IT8784: Thomas-Krenn LES plus v2 (YANLING YL-KBRL2 V2)
>>    IT8786: Thomas-Krenn LES plus v3 (YANLING YL-CLU L2)
>>    IT8786: Thomas-Krenn LES network 6L v2 (YANLING YL-CLU6L)
>>
>> Link: https://lore.kernel.org/all/140b264d-341f-465b-8715-dacfe84b3f71@roeck-us.net/
>>
>> Signed-off-by: Werner Fischer <devlists@wefi.net>
>> ---
>>   drivers/watchdog/it87_wdt.c | 14 +++++++++++++-
>>   1 file changed, 13 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/watchdog/it87_wdt.c b/drivers/watchdog/it87_wdt.c
>> index f6a344c002af..9297a5891912 100644
>> --- a/drivers/watchdog/it87_wdt.c
>> +++ b/drivers/watchdog/it87_wdt.c
>> @@ -258,6 +258,7 @@ static struct watchdog_device wdt_dev = {
>>   static int __init it87_wdt_init(void)
>>   {
>>          u8  chip_rev;
>> +       u8 ctrl;
>>          int rc;
>>
>>          rc = superio_enter();
>> @@ -316,7 +317,18 @@ static int __init it87_wdt_init(void)
>>
>>          superio_select(GPIO);
>>          superio_outb(WDT_TOV1, WDTCFG);
>> -       superio_outb(0x00, WDTCTRL);
>> +
>> +       switch (chip_type) {
>> +       case IT8784_ID:
>> +       case IT8786_ID:
>> +               ctrl = superio_inb(WDTCTRL);
> 
> If I print this value out like this:
> pr_warn("WDTCTRL initial: %02x\n", ctrl);
> 
> I get 0x00:
> [    1.607480] it87_wdt: WDTCTRL initial: 00
> 
> Do you think it's required that the kernel set bit 3 for some boards for
> the watchdog to work correctly if not set by the BIOS?
> 
That is done for none of the boards. The code in question does not _clear_ the bit,
but it is never set.

> Or maybe it's required to configure additional registers?
> 
I would suspect that to be the case. You might want to check register 0x72.

Guenter