On Thu, Jul 11, 2024 at 5:48 PM Guenter Roeck <linux@xxxxxxxxxxxx> wrote:
>
> On 7/11/24 15:14, James Hilliard wrote:
> > On Thu, Jul 11, 2024 at 3:42 PM Guenter Roeck <linux@xxxxxxxxxxxx> wrote:
> >>
> >> On 7/11/24 14:09, James Hilliard wrote:
> >>
> >>>> 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 only have one such system left, and the bit is not set on that system.
> >> I avoid buying hardware with ITE Super-IO chips nowadays since their support
> >> for Linux is non-existent.
> >
> > Yeah, I got stuck with a fleet of these boards, trying to make the best of it.
> >
> >>
> >>> I'm thinking we should check for that bit and prevent loading the
> >>> it87_wdt driver if
> >>
> >> No. That would create the risk of no longer loading the driver on systems where
> >> it currently works.
> >
> > Hmm, any idea how likely it would be that the bit could be set on a board
> > which the driver works on?
> >
>
> No idea, but I would not want to disable it just to find out with a flurry
> of angry e-mails.
>
> > Or maybe best to have a quirks table with dmi matching to disable the
> > driver on known broken systems?
> >
> >>
> >>> it's set(maybe along with an override param). That way the wdat_wdt driver I
> >>
> >> I prefer the less invasive version of logging a message. The user can then
> >> block the it87_wdt driver if it doesn't work.
> >
> > Hmm, I build multiple watchdog drivers into the same kernel and somewhat
> > rely on the autodetection working correctly as I support multiple boards
> > with the same kernel build. It's not exactly trivial to conditionally prevent
> > drivers from loading when built into the kernel AFAIU.
> >
>
> Those drivers should never be built into the kernel; they should be built
> as modules, and module load instructions in /etc/modprobe.d (or whatever the
> distribution uses) should be used to determine which drivers to load. I really
> would not want to rely on a bit such as the smi interrupt bit to determine
> if the watchdog is used by ACPI.
>
> This is actually a multi-level problem: Even if there is an ACPI watchdog,
> that does not mean that ACPI uses the Super-IO chip for its watchdog implementation.
> It might as well using the ICH watchdog on Intel systems or the TCO watchdog on
> AMD systems. Similar, even if the SMI interrupt bit is not set, it is essentially
> unknown if the it87_wdt driver actually works, because its reset pins might not
> be connected. Or, of course, both watchdogs might work.
>
> Assuming the wdat_wdt driver auto-loads on your system (I think it should),
> can you write a little script which loads the it87_wdt driver only if the
> wdat_wdt driver is not loaded ?
I'm a bit wary of using scripts to manually choose drivers like this, I
suppose it could work but I'm thinking it may be somewhat brittle.
So I ended up just disabling the it87_wdt driver, but a different batch of
boards also with the it8786 wdt chip we found doesn't appear to have a
functional wdat_wdt and does have working it87_wdt support so I'm now
looking at this again.
I messaged the vendor and they sent me some wdt sample code(that
I still need to test) which is supposed to be for the board that I'm having to
use wdat_wdt on.
It's setting some additional bits(like bit 5) which is interesting.
#include <sys/io.h>
#include <stdio.h>
#include <unistd.h>
#include <sys/io.h>
#include <string.h>
#define BIT0 0x01
#define BIT1 0x02
#define BIT2 0x04
#define BIT3 0x08
#define BIT4 0x10
#define BIT5 0x20
#define BIT6 0x40
#define BIT7 0x80
#define BYTE unsigned char
main(int argc, char* argv[])
{
int TValue;
BYTE data =0;
ioperm(0x2e,1,1);
ioperm(0x2f,1,1);
//Enter SIO Configuration Mode
outb(0x87,0x2e);
outb(0x01,0x2e);
outb(0x55,0x2e);
outb(0x55,0x2e);
//Select Logic Device 7
outb(0x07,0x2e);
outb(0x07,0x2f);
//Setting Default Watch Dog is Disabled
outb(0x71,0x2e);
outb(0x00,0x2f);
//Watchdog Reset Code
outb(0x72,0x2e);
outb(0x80,0x2f);
if(argc == 3)
{
if(strcmp(argv[1], "-M") == 0)
{
outb(0x72,0x2e);
data = inb(0x2f);
data &= ~BIT7;
outb(data,0x2f);
TValue = atoi(argv[2]);
outb(0x73,0x2e);
outb((BYTE)TValue,0x2f);
}
if(strcmp(argv[1], "-S") == 0)
{
outb(0x72,0x2e);
data = inb(0x2f);
data |= BIT7;
outb(data,0x2f);
TValue = atoi(argv[2]);
outb(0x73,0x2e);
outb((BYTE)TValue,0x2f);
}
outb(0x07,0x2e);
outb(0x04,0x2f);
outb(0xFA,0x2e);
data = inb(0x2f);
data |= BIT5;
outb(data,0x2f);
}
}
>
> Actually, just building the wdat_wdt driver into the kernel and it87_wdt as
> module (and loading it via modules.d) should work since the wdat_wdt driver
> would then be instantiated first, and the first watchdog is all that systemd
> cares about.
>
> Thanks,
> Guenter
>
