Re: [RFC] improve it87_wdt (IT8784/IT8786) / keeping WDTCTRL unchanged / deactivate watchdog by setting WDTVALLSB/WDTVALMSB 0

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 11/7/23 05:18, Werner Fischer wrote:
Hi Guenter, hi Wim, hi Hanspeter,

On Mon, 2023-10-16 at 15:16 +0200, Werner Fischer wrote:
Hi Guenter, hi Hanspeter,

I currently testing two devices with IT8784 and IT8786 watchdog
timers.
Although the chips are supported by it87_wdt.c after Hanspeter's
patches back in 2020, the watchdog functionality does not work in my
following test:
- Debian 12 using Kernel 6.1.58 or current 6.6-rc
- loading module it87_wdt
- starting wd_keepalive Deamon
- killing wd_keepalive using signal 9
-> system keeps on running even after the configured watchdog timeout

For debugging purposes, I have used the patch below to report the
content of the watchdog registers 0x71 (WDTCTRL), 0x72 (WDTCFG), 0x73
(WDTVALLSB), and 0x74 (WDTVALMSB) to the system log.

It turned out, that 0x71 (WDTCTRL) has initially the following value
set (before the module changes it to 0x00):
- 8 decimal (IT8784 / IT8786)
- 4 decimal (IT8613)

I figured out, that the following code line makes the watchdog of
IT8784 and IT8786 non-functional for me:
   superio_outb(0x00, WDTCTRL);
I have removed this code in my patch below, then the watchdog works
for IT8784 and IT8786.

I'm not sure, why the WDTCTRL register is set to 0x00 in the code. As
it seems, the register can have different meanings for differnt
IT8xxx chips. Accoring to [1] it seems sufficient to set both
WDTVALLSB and WDTVALMSB to 0x00 to deactivate the watchdog timer:
"When the WDT Time-out Value register is set to a non-zero value, the
WDT loads the value and begin counting down from the value."
This happens e.g. also when wd_keepalive is stopped cleanly.

I am open to support to improve the it87_wdt code.

But before I'm writing and sending a patch, I have the following
question:
* What is the reason, why WDTCTRL is set to 0x00 in the code? and
* Could we think about removing this (at least for IT8784/8786)?
It seems to me that setting WDTCTRL to 0x00 has been in the code from
the beginning.

For my test systems with IT8784 and IT8786 I got the following
information from the system vendor:
"71H bit 3 is the mode choice for the clock input of the IT8784/IT8786
chip. This bit is set to 1 (= PCICLK mode) and can not be set to 0."
Setting it to 0 breaks the watchdog functionality.

Unfortunately, ITE does not provide the specifications PDFs publicly
anymore. But the documentation at [2] provides details regarding the

They really never did, or at least not for a long time. Some board
vendors used to be Linux-friendly and provided datasheets on request,
but that is no longer the case. My recommendation used to be, and still is,
not to use boards with Super-IO chips from ITE to run Linux. This is not
only due to lack of datasheets, but also due to the lack of support from
both chip and board vendors if there are any issues when trying to support
the chips in Linux.

Watchdog Timer Control Register (71h) of an ITE chip, which has the
description "External CLK_IN Select: 1: PCICLK" for bit 3, too.

As it seems system-dependent, removing
   superio_outb(0x00, WDTCTRL);
from the code may lead to problems with other ITE chips, which maybe
could need WDTCTRL set to 0x00.


Bit 4..7 of the register are used to control watchdog timer resets (pings).
Skipping the write entirely is therefore unacceptable even for IT8784/IT8786
because we _don't_ want activity on a (legacy) keyboard, mouse, game,
or infrared port to reset the watchdog timer.

So my idea to be on the safe side for exiting users of it87_wdt, too:
* What do you think about an optional module parameter to let the user
   choose to leave WDTCTRL untouched? (this would make the watchdog work
   e.g. with my test systems with IT8784 and IT8786, too)


Make it conditional for IT8784/IT8786: On those chips, read the value from
the chip and clear all but bit 3. This is the safest we can do. Future
chips can be added as needed.

Guenter




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux