On Thu, Jul 11, 2024 at 1:17 PM Guenter Roeck <linux@xxxxxxxxxxxx> wrote: > > On 7/11/24 10:43, James Hilliard wrote: > > On Sat, Jul 6, 2024 at 1:47 PM Guenter Roeck <linux@xxxxxxxxxxxx> wrote: > >> > >> On 7/6/24 12:06, James Hilliard wrote: > >>> On Wed, Dec 13, 2023 at 1:45 AM Werner Fischer <devlists@xxxxxxxx> 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@xxxxxxxxxxxx/ > >>>> > >>>> Signed-off-by: Werner Fischer <devlists@xxxxxxxx> > >>>> --- > >>>> 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. > > > > So it turns out using the wdat_wdt driver works on this board. > > > > If I log register 0xF1 I get a value of 0x44 which the IT8786 docs > > indicate for bit 2: > > This bit is to enable the generation of an SMI# due to WDT’s IRQ (EN_WDT). > > > > Interesting find. I looked into some other ITE datasheets; they all have this bit. > > > If SMI is enabled for the WDT IRQ does that mean one can't use the it87_wdt > > driver and instead must use wdat_wdt? > > > Looks like it. > > > I noticed there's some ACPI resource conflict detection in the hwmon IT8786 > > driver(although the hwmon driver doesn't indicate a resource conflict on this > > board for me. I wonder if there is a resource conflict here with the watchdog > > that should be detected? > > > > The hwmon driver detects the conflict on the hwmon register space, not the > Super-IO configuration address space. I would suspect that pretty much all > systems would show a resource conflict on the Super-IO configuration space. > > The best we could possibly do might be to add a check for the bit in register > 0xf1 and warn the user that they might have to use the ACPI driver if the bit > is set. I am not sure if that would be helpful or just add noise, though. Do your systems which work with the it87_wdt driver have that 0xF1 bit not set? I'm thinking we should check for that bit and prevent loading the it87_wdt driver if it's set(maybe along with an override param). That way the wdat_wdt driver I think should end up as the primary watchdog(systemd only seems to properly handle one watchdog). > > Guenter >
