linux-kernel - Re: [PATCH] watchdog: it87_wdt: add quirks for some Qotom IT8786 boards

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<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) {