[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <66922a0b-6e30-4501-9262-8bdd224155f9@roeck-us.net>
Date: Thu, 17 Oct 2024 20:59:24 -0700
From: Guenter Roeck <linux@...ck-us.net>
To: James Hilliard <james.hilliard1@...il.com>, linux-watchdog@...r.kernel.org
Cc: Wim Van Sebroeck <wim@...ux-watchdog.org>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] watchdog: it87_wdt: add quirks for some Qotom IT8786
boards
On 10/17/24 20:09, James Hilliard wrote:
> For the watchdog timer to work properly on the QCML04 board we need to
> set PWRGD enable in the Environment Controller Configuration Registers
> Special Configuration Register 1 when it is not already set, this may
> be the case when the watchdog is not enabled from within the BIOS.
>
> For the Qotom QGLK02 board the vendor indicates that the IT8786
> watchdog hardware is not functional due to a conflict with the BIOS
> power-on function, with PWRGD set the watchdog will trigger but the
> board will poweroff rather than restart as expected. Disable the
> it87 driver on this broken hardware.
>
This shouldn't be done in drivers, and it doesn't scale. The driver needs
to be disabled with the mechanism supported by the distribution, for example
in /etc/modprobe.d/blacklist-watchdog.conf, or by whatever other mechanism
the distribution supports for that purpose.
Guenter
> Signed-off-by: James Hilliard <james.hilliard1@...il.com>
> ---
> drivers/watchdog/it87_wdt.c | 55 +++++++++++++++++++++++++++++++++++++
> 1 file changed, 55 insertions(+)
>
> diff --git a/drivers/watchdog/it87_wdt.c b/drivers/watchdog/it87_wdt.c
> index 3e8c15138edd..dec501c21fd3 100644
> --- a/drivers/watchdog/it87_wdt.c
> +++ b/drivers/watchdog/it87_wdt.c
> @@ -20,6 +20,7 @@
>
> #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>
> +#include <linux/dmi.h>
> #include <linux/init.h>
> #include <linux/io.h>
> #include <linux/kernel.h>
> @@ -40,6 +41,7 @@
> #define VAL 0x2f
>
> /* Logical device Numbers LDN */
> +#define EC 0x04
> #define GPIO 0x07
>
> /* Configuration Registers and Functions */
> @@ -73,6 +75,12 @@
> #define IT8784_ID 0x8784
> #define IT8786_ID 0x8786
>
> +/* Environment Controller Configuration Registers LDN=0x04 */
> +#define SCR1 0xfa
> +
> +/* Environment Controller Bits SCR1 */
> +#define WDT_PWRGD 0x20
> +
> /* GPIO Configuration Registers LDN=0x07 */
> #define WDTCTRL 0x71
> #define WDTCFG 0x72
> @@ -240,6 +248,28 @@ static int wdt_set_timeout(struct watchdog_device *wdd, unsigned int t)
> return ret;
> }
>
> +enum {
> + IT87_WDT_BROKEN = BIT(0),
> + IT87_WDT_OUTPUT_THROUGH_PWRGD = BIT(1),
> +};
> +
> +static const struct dmi_system_id it8786_quirks[] = {
> + {
> + /* Qotom Q730P/Q8XXG2-P */
> + .matches = {
> + DMI_EXACT_MATCH(DMI_BOARD_NAME, "QGLK02"),
> + },
> + .driver_data = (void *)IT87_WDT_BROKEN,
> + },
> + {
> + /* Qotom Q30900P */
> + .matches = {
> + DMI_EXACT_MATCH(DMI_BOARD_NAME, "QCML04"),
> + },
> + .driver_data = (void *)IT87_WDT_OUTPUT_THROUGH_PWRGD,
> + }
> +};
> +
> static const struct watchdog_info ident = {
> .options = WDIOF_SETTIMEOUT | WDIOF_MAGICCLOSE | WDIOF_KEEPALIVEPING,
> .firmware_version = 1,
> @@ -261,8 +291,10 @@ static struct watchdog_device wdt_dev = {
>
> static int __init it87_wdt_init(void)
> {
> + const struct dmi_system_id *dmi_id;
> u8 chip_rev;
> u8 ctrl;
> + int quirks = 0;
> int rc;
>
> rc = superio_enter();
> @@ -273,6 +305,20 @@ static int __init it87_wdt_init(void)
> chip_rev = superio_inb(CHIPREV) & 0x0f;
> superio_exit();
>
> + switch (chip_type) {
> + case IT8786_ID:
> + dmi_id = dmi_first_match(it8786_quirks);
> + break;
> + default:
> + dmi_id = NULL;
> + }
> +
> + if (dmi_id)
> + quirks = (long)dmi_id->driver_data;
> +
> + if (quirks & IT87_WDT_BROKEN)
> + return -ENODEV;
> +
> switch (chip_type) {
> case IT8702_ID:
> max_units = 255;
> @@ -333,6 +379,15 @@ static int __init it87_wdt_init(void)
> superio_outb(0x00, WDTCTRL);
> }
>
> + if (quirks & IT87_WDT_OUTPUT_THROUGH_PWRGD) {
> + superio_select(EC);
> + ctrl = superio_inb(SCR1);
> + if (!(ctrl & WDT_PWRGD)) {
> + ctrl |= WDT_PWRGD;
> + superio_outb(ctrl, SCR1);
> + }
> + }
> +
> superio_exit();
>
> if (timeout < 1 || timeout > max_units * 60) {
Powered by blists - more mailing lists