[PATCH v3 0/2] Add driver for Intel Over-Clocking Watchdog

[PATCH v3 0/2] Add driver for Intel Over-Clocking Watchdog

[Diogo Ivo]

This series adds a driver for the Intel Over-Clocking Watchdog found in
the Intel Platform Controller Hub (PCH).

This watchdog is controlled via a simple single-register interface and
would otherwise be standard except for the presence of a LOCK bit that
can only be set once per power cycle, needing extra handling around it.

Due to the way these devices are described in ACPI tables with both the
generic PNP0C02 CID and a more detailed ACPI HID we also need to add
their HIDs to the list of known non-PNP devices. As there are several HIDs
for what seems to be essentially the same hardware but I don't know all
the possible HIDs this series does not include an exhaustive list of all
such HIDs, only those that I could personally test.

Signed-off-by: Diogo Ivo <diogo.ivo@siemens.com>
---
Changes in v3:
 - PATCH 01:
  - Collect R-b from Guenter
 - PATCH 02:
  - Reword the commit message to clarify purpose of patch
- Link to v2: https://lore.kernel.org/r/20250312-ivo-intel_oc_wdt-v2-0-52d09738cd0b@siemens.com

Changes in v2:
- Split v1 into two patches, adding the ACPI IDs in a separate patch
- Initialize hearbeat module parameter to zero
- Clarify wording around lock handling
- Properly print resource with %pR when failing to obtain it
- Enable compile testing and add dependency on HAS_IOPORT
- Drop unneeded ACPI_PTR() and MODULE_ALIAS()
- Link to v1: https://lore.kernel.org/r/20250311-ivo-intel_oc_wdt-v1-1-fd470460d9f5@siemens.com

---
Diogo Ivo (2):
      watchdog: Add driver for Intel OC WDT
      ACPI: PNP: Add Intel OC Watchdog IDs to non-PNP device list

 drivers/acpi/acpi_pnp.c         |   2 +
 drivers/watchdog/Kconfig        |  11 ++
 drivers/watchdog/Makefile       |   1 +
 drivers/watchdog/intel_oc_wdt.c | 233 ++++++++++++++++++++++++++++++++++++++++
 4 files changed, 247 insertions(+)
---
base-commit: b7f94fcf55469ad3ef8a74c35b488dbfa314d1bb
change-id: 20250227-ivo-intel_oc_wdt-7a483a4d6a04

Best regards,
-- 
Diogo Ivo <diogo.ivo@siemens.com>

[PATCH v3 1/2] watchdog: Add driver for Intel OC WDT

[Diogo Ivo]

Add a driver for the Intel Over-Clocking Watchdog found in Intel
Platform Controller (PCH) chipsets. This watchdog is controlled
via a simple single-register interface and would otherwise be
standard except for the presence of a LOCK bit that can only be
set once per power cycle, needing extra handling around it.

Reviewed-by: Guenter Roeck <linux@roeck-us.net>
Signed-off-by: Diogo Ivo <diogo.ivo@siemens.com>
---
v2->v3:
 - Collect R-b from Guenter

v1->v2:
 - Split v1 into two patches, adding the ACPI IDs in a separate patch
 - Initialize hearbeat module parameter to zero
 - Clarify wording around lock handling
 - Properly print resource with %pR when failing to obtain it
 - Enable compile testing and add dependency on HAS_IOPORT
 - Drop unneeded ACPI_PTR() and MODULE_ALIAS()
---
---
 drivers/watchdog/Kconfig        |  11 ++
 drivers/watchdog/Makefile       |   1 +
 drivers/watchdog/intel_oc_wdt.c | 233 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 245 insertions(+)

diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index f81705f8539aa0b12d156a86aae521aa40b4527d..94d058c6c49a19f8fee97d82ceb653b60d1944cd 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -1350,6 +1350,17 @@ config INTEL_MID_WATCHDOG
 
 	  To compile this driver as a module, choose M here.
 
+config INTEL_OC_WATCHDOG
+	tristate "Intel OC Watchdog"
+	depends on (X86 || COMPILE_TEST) && ACPI && HAS_IOPORT
+	select WATCHDOG_CORE
+	help
+	  Hardware driver for Intel Over-Clocking watchdog present in
+	  Platform Controller Hub (PCH) chipsets.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called intel_oc_wdt.
+
 config ITCO_WDT
 	tristate "Intel TCO Timer/Watchdog"
 	depends on X86 && PCI
diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
index 8411626fa162268e8ccd06349f7193b15a9d281a..3a13f3e80a0f460b99b4f1592fcf17cc6428876b 100644
--- a/drivers/watchdog/Makefile
+++ b/drivers/watchdog/Makefile
@@ -149,6 +149,7 @@ obj-$(CONFIG_W83977F_WDT) += w83977f_wdt.o
 obj-$(CONFIG_MACHZ_WDT) += machzwd.o
 obj-$(CONFIG_SBC_EPX_C3_WATCHDOG) += sbc_epx_c3.o
 obj-$(CONFIG_INTEL_MID_WATCHDOG) += intel-mid_wdt.o
+obj-$(CONFIG_INTEL_OC_WATCHDOG) += intel_oc_wdt.o
 obj-$(CONFIG_INTEL_MEI_WDT) += mei_wdt.o
 obj-$(CONFIG_NI903X_WDT) += ni903x_wdt.o
 obj-$(CONFIG_NIC7018_WDT) += nic7018_wdt.o
diff --git a/drivers/watchdog/intel_oc_wdt.c b/drivers/watchdog/intel_oc_wdt.c
new file mode 100644
index 0000000000000000000000000000000000000000..7c0551106981b08f9e1d2eab8f3ddce99a2462aa
--- /dev/null
+++ b/drivers/watchdog/intel_oc_wdt.c
@@ -0,0 +1,233 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Intel OC Watchdog driver
+ *
+ * Copyright (C) 2025, Siemens
+ * Author: Diogo Ivo <diogo.ivo@siemens.com>
+ */
+
+#define DRV_NAME	"intel_oc_wdt"
+
+#include <linux/acpi.h>
+#include <linux/bits.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/moduleparam.h>
+#include <linux/platform_device.h>
+#include <linux/watchdog.h>
+
+#define INTEL_OC_WDT_TOV		GENMASK(9, 0)
+#define INTEL_OC_WDT_MIN_TOV		1
+#define INTEL_OC_WDT_MAX_TOV		1024
+#define INTEL_OC_WDT_DEF_TOV		60
+
+/*
+ * One-time writable lock bit. If set forbids
+ * modification of itself, _TOV and _EN until
+ * next reboot.
+ */
+#define INTEL_OC_WDT_CTL_LCK		BIT(12)
+
+#define INTEL_OC_WDT_EN			BIT(14)
+#define INTEL_OC_WDT_NO_ICCSURV_STS	BIT(24)
+#define INTEL_OC_WDT_ICCSURV_STS	BIT(25)
+#define INTEL_OC_WDT_RLD		BIT(31)
+
+#define INTEL_OC_WDT_STS_BITS (INTEL_OC_WDT_NO_ICCSURV_STS | \
+			       INTEL_OC_WDT_ICCSURV_STS)
+
+#define INTEL_OC_WDT_CTRL_REG(wdt)	((wdt)->ctrl_res->start)
+
+struct intel_oc_wdt {
+	struct watchdog_device wdd;
+	struct resource *ctrl_res;
+	bool locked;
+};
+
+static int heartbeat;
+module_param(heartbeat, uint, 0);
+MODULE_PARM_DESC(heartbeat, "Watchdog heartbeats in seconds. (default="
+		 __MODULE_STRING(WDT_HEARTBEAT) ")");
+
+static bool nowayout = WATCHDOG_NOWAYOUT;
+module_param(nowayout, bool, 0);
+MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started (default="
+		 __MODULE_STRING(WATCHDOG_NOWAYOUT) ")");
+
+static int intel_oc_wdt_start(struct watchdog_device *wdd)
+{
+	struct intel_oc_wdt *oc_wdt = watchdog_get_drvdata(wdd);
+
+	if (oc_wdt->locked)
+		return 0;
+
+	outl(inl(INTEL_OC_WDT_CTRL_REG(oc_wdt)) | INTEL_OC_WDT_EN,
+	     INTEL_OC_WDT_CTRL_REG(oc_wdt));
+
+	return 0;
+}
+
+static int intel_oc_wdt_stop(struct watchdog_device *wdd)
+{
+	struct intel_oc_wdt *oc_wdt = watchdog_get_drvdata(wdd);
+
+	outl(inl(INTEL_OC_WDT_CTRL_REG(oc_wdt)) & ~INTEL_OC_WDT_EN,
+	     INTEL_OC_WDT_CTRL_REG(oc_wdt));
+
+	return 0;
+}
+
+static int intel_oc_wdt_ping(struct watchdog_device *wdd)
+{
+	struct intel_oc_wdt *oc_wdt = watchdog_get_drvdata(wdd);
+
+	outl(inl(INTEL_OC_WDT_CTRL_REG(oc_wdt)) | INTEL_OC_WDT_RLD,
+	     INTEL_OC_WDT_CTRL_REG(oc_wdt));
+
+	return 0;
+}
+
+static int intel_oc_wdt_set_timeout(struct watchdog_device *wdd,
+				    unsigned int t)
+{
+	struct intel_oc_wdt *oc_wdt = watchdog_get_drvdata(wdd);
+
+	outl((inl(INTEL_OC_WDT_CTRL_REG(oc_wdt)) & ~INTEL_OC_WDT_TOV) | (t - 1),
+	     INTEL_OC_WDT_CTRL_REG(oc_wdt));
+
+	wdd->timeout = t;
+
+	return 0;
+}
+
+static const struct watchdog_info intel_oc_wdt_info = {
+	.options = WDIOF_SETTIMEOUT | WDIOF_MAGICCLOSE | WDIOF_KEEPALIVEPING,
+	.identity = DRV_NAME,
+};
+
+static const struct watchdog_ops intel_oc_wdt_ops = {
+	.owner = THIS_MODULE,
+	.start = intel_oc_wdt_start,
+	.stop = intel_oc_wdt_stop,
+	.ping = intel_oc_wdt_ping,
+	.set_timeout = intel_oc_wdt_set_timeout,
+};
+
+static int intel_oc_wdt_setup(struct intel_oc_wdt *oc_wdt)
+{
+	struct watchdog_info *info;
+	unsigned long val;
+
+	val = inl(INTEL_OC_WDT_CTRL_REG(oc_wdt));
+
+	if (val & INTEL_OC_WDT_STS_BITS)
+		oc_wdt->wdd.bootstatus |= WDIOF_CARDRESET;
+
+	oc_wdt->locked = !!(val & INTEL_OC_WDT_CTL_LCK);
+
+	if (val & INTEL_OC_WDT_EN) {
+		/*
+		 * No need to issue a ping here to "commit" the new timeout
+		 * value to hardware as the watchdog core schedules one
+		 * immediately when registering the watchdog.
+		 */
+		set_bit(WDOG_HW_RUNNING, &oc_wdt->wdd.status);
+
+		if (oc_wdt->locked) {
+			info = (struct watchdog_info *)&intel_oc_wdt_info;
+			/*
+			 * Set nowayout unconditionally as we cannot stop
+			 * the watchdog.
+			 */
+			nowayout = true;
+			/*
+			 * If we are locked read the current timeout value
+			 * and inform the core we can't change it.
+			 */
+			oc_wdt->wdd.timeout = (val & INTEL_OC_WDT_TOV) + 1;
+			info->options &= ~WDIOF_SETTIMEOUT;
+
+			dev_info(oc_wdt->wdd.parent,
+				 "Register access locked, heartbeat fixed at: %u s\n",
+				 oc_wdt->wdd.timeout);
+		}
+	} else if (oc_wdt->locked) {
+		/*
+		 * In case the watchdog is disabled and locked there
+		 * is nothing we can do with it so just fail probing.
+		 */
+		return -EACCES;
+	}
+
+	val &= ~INTEL_OC_WDT_TOV;
+	outl(val | (oc_wdt->wdd.timeout - 1), INTEL_OC_WDT_CTRL_REG(oc_wdt));
+
+	return 0;
+}
+
+static int intel_oc_wdt_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct intel_oc_wdt *oc_wdt;
+	struct watchdog_device *wdd;
+	int ret;
+
+	oc_wdt = devm_kzalloc(&pdev->dev, sizeof(*oc_wdt), GFP_KERNEL);
+	if (!oc_wdt)
+		return -ENOMEM;
+
+	oc_wdt->ctrl_res = platform_get_resource(pdev, IORESOURCE_IO, 0);
+	if (!oc_wdt->ctrl_res) {
+		dev_err(&pdev->dev, "missing I/O resource\n");
+		return -ENODEV;
+	}
+
+	if (!devm_request_region(&pdev->dev, oc_wdt->ctrl_res->start,
+				 resource_size(oc_wdt->ctrl_res), pdev->name)) {
+		dev_err(dev, "resource %pR already in use, device disabled\n",
+			oc_wdt->ctrl_res);
+		return -EBUSY;
+	}
+
+	wdd = &oc_wdt->wdd;
+	wdd->min_timeout = INTEL_OC_WDT_MIN_TOV;
+	wdd->max_timeout = INTEL_OC_WDT_MAX_TOV;
+	wdd->timeout = INTEL_OC_WDT_DEF_TOV;
+	wdd->info = &intel_oc_wdt_info;
+	wdd->ops = &intel_oc_wdt_ops;
+	wdd->parent = dev;
+
+	watchdog_init_timeout(wdd, heartbeat, dev);
+
+	ret = intel_oc_wdt_setup(oc_wdt);
+	if (ret)
+		return ret;
+
+	watchdog_set_drvdata(wdd, oc_wdt);
+	watchdog_set_nowayout(wdd, nowayout);
+	watchdog_stop_on_reboot(wdd);
+	watchdog_stop_on_unregister(wdd);
+
+	return devm_watchdog_register_device(dev, wdd);
+}
+
+static const struct acpi_device_id intel_oc_wdt_match[] = {
+	{ "INT3F0D" },
+	{ "INTC1099" },
+	{ },
+};
+MODULE_DEVICE_TABLE(acpi, intel_oc_wdt_match);
+
+static struct platform_driver intel_oc_wdt_platform_driver = {
+	.driver = {
+		.name = DRV_NAME,
+		.acpi_match_table = intel_oc_wdt_match,
+	},
+	.probe = intel_oc_wdt_probe,
+};
+
+module_platform_driver(intel_oc_wdt_platform_driver);
+
+MODULE_AUTHOR("Diogo Ivo <diogo.ivo@siemens.com>");
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Intel OC Watchdog driver");

-- 
2.48.1

Re: [PATCH v3 1/2] watchdog: Add driver for Intel OC WDT

[Markus Elfring]

> Add a driver for the Intel Over-Clocking Watchdog found in Intel
> Platform Controller (PCH) chipsets. This watchdog is controlled
…

You may occasionally put more than 64 characters into text lines
of such a change description.

Regards,
Markus

Re: [PATCH v3 1/2] watchdog: Add driver for Intel OC WDT

[Diogo Ivo]

Hello,

On 3/17/25 10:55 AM, Diogo Ivo wrote:
> Add a driver for the Intel Over-Clocking Watchdog found in Intel
> Platform Controller (PCH) chipsets. This watchdog is controlled
> via a simple single-register interface and would otherwise be
> standard except for the presence of a LOCK bit that can only be
> set once per power cycle, needing extra handling around it.
> 
> Reviewed-by: Guenter Roeck <linux@roeck-us.net>
> Signed-off-by: Diogo Ivo <diogo.ivo@siemens.com>
> ---
> v2->v3:
>   - Collect R-b from Guenter
> 
> v1->v2:
>   - Split v1 into two patches, adding the ACPI IDs in a separate patch
>   - Initialize hearbeat module parameter to zero
>   - Clarify wording around lock handling
>   - Properly print resource with %pR when failing to obtain it
>   - Enable compile testing and add dependency on HAS_IOPORT
>   - Drop unneeded ACPI_PTR() and MODULE_ALIAS()
> ---
> ---
>   drivers/watchdog/Kconfig        |  11 ++
>   drivers/watchdog/Makefile       |   1 +
>   drivers/watchdog/intel_oc_wdt.c | 233 ++++++++++++++++++++++++++++++++++++++++
>   3 files changed, 245 insertions(+)
> 

Gentle ping on this patch.

Best regards,
Diogo

Re: [PATCH v3 1/2] watchdog: Add driver for Intel OC WDT

[Diogo Ivo]

Hello,

On 4/8/25 9:31 AM, Diogo Ivo wrote:
> Hello,
> 
> On 3/17/25 10:55 AM, Diogo Ivo wrote:
>> Add a driver for the Intel Over-Clocking Watchdog found in Intel
>> Platform Controller (PCH) chipsets. This watchdog is controlled
>> via a simple single-register interface and would otherwise be
>> standard except for the presence of a LOCK bit that can only be
>> set once per power cycle, needing extra handling around it.
>>
>> Reviewed-by: Guenter Roeck <linux@roeck-us.net>
>> Signed-off-by: Diogo Ivo <diogo.ivo@siemens.com>
>> ---
>> v2->v3:
>>   - Collect R-b from Guenter
>>
>> v1->v2:
>>   - Split v1 into two patches, adding the ACPI IDs in a separate patch
>>   - Initialize hearbeat module parameter to zero
>>   - Clarify wording around lock handling
>>   - Properly print resource with %pR when failing to obtain it
>>   - Enable compile testing and add dependency on HAS_IOPORT
>>   - Drop unneeded ACPI_PTR() and MODULE_ALIAS()
>> ---
>> ---
>>   drivers/watchdog/Kconfig        |  11 ++
>>   drivers/watchdog/Makefile       |   1 +
>>   drivers/watchdog/intel_oc_wdt.c | 233 ++++++++++++++++++++++++++++++ 
>> ++++++++++
>>   3 files changed, 245 insertions(+)
>>
> 
> Gentle ping on this patch.

Another ping on this patch. Is there anything missing from my side
blocking this?

Best regards,
Diogo

[PATCH v3 2/2] ACPI: PNP: Add Intel OC Watchdog IDs to non-PNP device list

[Diogo Ivo]

Intel Over-Clocking Watchdogs are described in ACPI tables by both the
generic PNP0C02 _CID and their ACPI _HID. The presence of the _CID then
causes the PNP scan handler to attach to the watchdog, preventing the
actual watchdog driver from binding. Address this by adding the ACPI
_HIDs to the list of non-PNP devices, so that the PNP scan handler is
bypassed.

Note that these watchdogs can be described by multiple _HIDs for what
seems to be identical hardware. This commit is not a complete list of
all the possible watchdog ACPI _HIDs.

Signed-off-by: Diogo Ivo <diogo.ivo@siemens.com>
---
v2->v3:
 - Reword the commit message to clarify purpose of patch
---
---
 drivers/acpi/acpi_pnp.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/acpi/acpi_pnp.c b/drivers/acpi/acpi_pnp.c
index 01abf26764b00c86f938dea2ed138424f041f880..3f5a1840f573303c71f5d579e32963a5b29d2587 100644
--- a/drivers/acpi/acpi_pnp.c
+++ b/drivers/acpi/acpi_pnp.c
@@ -355,8 +355,10 @@ static bool acpi_pnp_match(const char *idstr, const struct acpi_device_id **matc
  * device represented by it.
  */
 static const struct acpi_device_id acpi_nonpnp_device_ids[] = {
+	{"INT3F0D"},
 	{"INTC1080"},
 	{"INTC1081"},
+	{"INTC1099"},
 	{""},
 };
 

-- 
2.48.1

Re: [PATCH v3 2/2] ACPI: PNP: Add Intel OC Watchdog IDs to non-PNP device list

[Diogo Ivo]

Hello,

On 3/17/25 10:55 AM, Diogo Ivo wrote:
> Intel Over-Clocking Watchdogs are described in ACPI tables by both the
> generic PNP0C02 _CID and their ACPI _HID. The presence of the _CID then
> causes the PNP scan handler to attach to the watchdog, preventing the
> actual watchdog driver from binding. Address this by adding the ACPI
> _HIDs to the list of non-PNP devices, so that the PNP scan handler is
> bypassed.
> 
> Note that these watchdogs can be described by multiple _HIDs for what
> seems to be identical hardware. This commit is not a complete list of
> all the possible watchdog ACPI _HIDs.
> 
> Signed-off-by: Diogo Ivo <diogo.ivo@siemens.com>
> ---
> v2->v3:
>   - Reword the commit message to clarify purpose of patch
> ---
> ---
>   drivers/acpi/acpi_pnp.c | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/acpi/acpi_pnp.c b/drivers/acpi/acpi_pnp.c
> index 01abf26764b00c86f938dea2ed138424f041f880..3f5a1840f573303c71f5d579e32963a5b29d2587 100644
> --- a/drivers/acpi/acpi_pnp.c
> +++ b/drivers/acpi/acpi_pnp.c
> @@ -355,8 +355,10 @@ static bool acpi_pnp_match(const char *idstr, const struct acpi_device_id **matc
>    * device represented by it.
>    */
>   static const struct acpi_device_id acpi_nonpnp_device_ids[] = {
> +	{"INT3F0D"},
>   	{"INTC1080"},
>   	{"INTC1081"},
> +	{"INTC1099"},
>   	{""},
>   };
>   
> 

Gentle ping on this patch.

Best regards,
Diogo

Re: [PATCH v3 2/2] ACPI: PNP: Add Intel OC Watchdog IDs to non-PNP device list

[Rafael J. Wysocki]

On Tue, Mar 25, 2025 at 6:19 PM Diogo Ivo <diogo.ivo@siemens.com> wrote:
>
> Hello,
>
> On 3/17/25 10:55 AM, Diogo Ivo wrote:
> > Intel Over-Clocking Watchdogs are described in ACPI tables by both the
> > generic PNP0C02 _CID and their ACPI _HID. The presence of the _CID then
> > causes the PNP scan handler to attach to the watchdog, preventing the
> > actual watchdog driver from binding. Address this by adding the ACPI
> > _HIDs to the list of non-PNP devices, so that the PNP scan handler is
> > bypassed.
> >
> > Note that these watchdogs can be described by multiple _HIDs for what
> > seems to be identical hardware. This commit is not a complete list of
> > all the possible watchdog ACPI _HIDs.
> >
> > Signed-off-by: Diogo Ivo <diogo.ivo@siemens.com>
> > ---
> > v2->v3:
> >   - Reword the commit message to clarify purpose of patch
> > ---
> > ---
> >   drivers/acpi/acpi_pnp.c | 2 ++
> >   1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/acpi/acpi_pnp.c b/drivers/acpi/acpi_pnp.c
> > index 01abf26764b00c86f938dea2ed138424f041f880..3f5a1840f573303c71f5d579e32963a5b29d2587 100644
> > --- a/drivers/acpi/acpi_pnp.c
> > +++ b/drivers/acpi/acpi_pnp.c
> > @@ -355,8 +355,10 @@ static bool acpi_pnp_match(const char *idstr, const struct acpi_device_id **matc
> >    * device represented by it.
> >    */
> >   static const struct acpi_device_id acpi_nonpnp_device_ids[] = {
> > +     {"INT3F0D"},
> >       {"INTC1080"},
> >       {"INTC1081"},
> > +     {"INTC1099"},
> >       {""},
> >   };
> >
> >
>
> Gentle ping on this patch.

Do you want me to pick it up or do you want to route it through a
different tree?

Re: [PATCH v3 2/2] ACPI: PNP: Add Intel OC Watchdog IDs to non-PNP device list

[Diogo Ivo]

On 3/25/25 6:59 PM, Rafael J. Wysocki wrote:
> On Tue, Mar 25, 2025 at 6:19 PM Diogo Ivo <diogo.ivo@siemens.com> wrote:
>>
>> Hello,
>>
>> On 3/17/25 10:55 AM, Diogo Ivo wrote:
>>> Intel Over-Clocking Watchdogs are described in ACPI tables by both the
>>> generic PNP0C02 _CID and their ACPI _HID. The presence of the _CID then
>>> causes the PNP scan handler to attach to the watchdog, preventing the
>>> actual watchdog driver from binding. Address this by adding the ACPI
>>> _HIDs to the list of non-PNP devices, so that the PNP scan handler is
>>> bypassed.
>>>
>>> Note that these watchdogs can be described by multiple _HIDs for what
>>> seems to be identical hardware. This commit is not a complete list of
>>> all the possible watchdog ACPI _HIDs.
>>>
>>> Signed-off-by: Diogo Ivo <diogo.ivo@siemens.com>
>>> ---
>>> v2->v3:
>>>    - Reword the commit message to clarify purpose of patch
>>> ---
>>> ---
>>>    drivers/acpi/acpi_pnp.c | 2 ++
>>>    1 file changed, 2 insertions(+)
>>>
>>> diff --git a/drivers/acpi/acpi_pnp.c b/drivers/acpi/acpi_pnp.c
>>> index 01abf26764b00c86f938dea2ed138424f041f880..3f5a1840f573303c71f5d579e32963a5b29d2587 100644
>>> --- a/drivers/acpi/acpi_pnp.c
>>> +++ b/drivers/acpi/acpi_pnp.c
>>> @@ -355,8 +355,10 @@ static bool acpi_pnp_match(const char *idstr, const struct acpi_device_id **matc
>>>     * device represented by it.
>>>     */
>>>    static const struct acpi_device_id acpi_nonpnp_device_ids[] = {
>>> +     {"INT3F0D"},
>>>        {"INTC1080"},
>>>        {"INTC1081"},
>>> +     {"INTC1099"},
>>>        {""},
>>>    };
>>>
>>>
>>
>> Gentle ping on this patch.
> 
> Do you want me to pick it up or do you want to route it through a
> different tree?

Unless the watchdog maintainers have any objections it's fine if you
pick it up.

Best regards,
Diogo

Re: [PATCH v3 2/2] ACPI: PNP: Add Intel OC Watchdog IDs to non-PNP device list

[Guenter Roeck]

On 3/26/25 02:01, Diogo Ivo wrote:
> On 3/25/25 6:59 PM, Rafael J. Wysocki wrote:
>> On Tue, Mar 25, 2025 at 6:19 PM Diogo Ivo <diogo.ivo@siemens.com> wrote:
>>>
>>> Hello,
>>>
>>> On 3/17/25 10:55 AM, Diogo Ivo wrote:
>>>> Intel Over-Clocking Watchdogs are described in ACPI tables by both the
>>>> generic PNP0C02 _CID and their ACPI _HID. The presence of the _CID then
>>>> causes the PNP scan handler to attach to the watchdog, preventing the
>>>> actual watchdog driver from binding. Address this by adding the ACPI
>>>> _HIDs to the list of non-PNP devices, so that the PNP scan handler is
>>>> bypassed.
>>>>
>>>> Note that these watchdogs can be described by multiple _HIDs for what
>>>> seems to be identical hardware. This commit is not a complete list of
>>>> all the possible watchdog ACPI _HIDs.
>>>>
>>>> Signed-off-by: Diogo Ivo <diogo.ivo@siemens.com>
>>>> ---
>>>> v2->v3:
>>>>    - Reword the commit message to clarify purpose of patch
>>>> ---
>>>> ---
>>>>    drivers/acpi/acpi_pnp.c | 2 ++
>>>>    1 file changed, 2 insertions(+)
>>>>
>>>> diff --git a/drivers/acpi/acpi_pnp.c b/drivers/acpi/acpi_pnp.c
>>>> index 01abf26764b00c86f938dea2ed138424f041f880..3f5a1840f573303c71f5d579e32963a5b29d2587 100644
>>>> --- a/drivers/acpi/acpi_pnp.c
>>>> +++ b/drivers/acpi/acpi_pnp.c
>>>> @@ -355,8 +355,10 @@ static bool acpi_pnp_match(const char *idstr, const struct acpi_device_id **matc
>>>>     * device represented by it.
>>>>     */
>>>>    static const struct acpi_device_id acpi_nonpnp_device_ids[] = {
>>>> +     {"INT3F0D"},
>>>>        {"INTC1080"},
>>>>        {"INTC1081"},
>>>> +     {"INTC1099"},
>>>>        {""},
>>>>    };
>>>>
>>>>
>>>
>>> Gentle ping on this patch.
>>
>> Do you want me to pick it up or do you want to route it through a
>> different tree?
> 
> Unless the watchdog maintainers have any objections it's fine if you
> pick it up.
> 

Ok with me.

Guenter

Re: [PATCH v3 2/2] ACPI: PNP: Add Intel OC Watchdog IDs to non-PNP device list

[Rafael J. Wysocki]

On Wed, Mar 26, 2025 at 3:02 PM Guenter Roeck <linux@roeck-us.net> wrote:
>
> On 3/26/25 02:01, Diogo Ivo wrote:
> > On 3/25/25 6:59 PM, Rafael J. Wysocki wrote:
> >> On Tue, Mar 25, 2025 at 6:19 PM Diogo Ivo <diogo.ivo@siemens.com> wrote:
> >>>
> >>> Hello,
> >>>
> >>> On 3/17/25 10:55 AM, Diogo Ivo wrote:
> >>>> Intel Over-Clocking Watchdogs are described in ACPI tables by both the
> >>>> generic PNP0C02 _CID and their ACPI _HID. The presence of the _CID then
> >>>> causes the PNP scan handler to attach to the watchdog, preventing the
> >>>> actual watchdog driver from binding. Address this by adding the ACPI
> >>>> _HIDs to the list of non-PNP devices, so that the PNP scan handler is
> >>>> bypassed.
> >>>>
> >>>> Note that these watchdogs can be described by multiple _HIDs for what
> >>>> seems to be identical hardware. This commit is not a complete list of
> >>>> all the possible watchdog ACPI _HIDs.
> >>>>
> >>>> Signed-off-by: Diogo Ivo <diogo.ivo@siemens.com>
> >>>> ---
> >>>> v2->v3:
> >>>>    - Reword the commit message to clarify purpose of patch
> >>>> ---
> >>>> ---
> >>>>    drivers/acpi/acpi_pnp.c | 2 ++
> >>>>    1 file changed, 2 insertions(+)
> >>>>
> >>>> diff --git a/drivers/acpi/acpi_pnp.c b/drivers/acpi/acpi_pnp.c
> >>>> index 01abf26764b00c86f938dea2ed138424f041f880..3f5a1840f573303c71f5d579e32963a5b29d2587 100644
> >>>> --- a/drivers/acpi/acpi_pnp.c
> >>>> +++ b/drivers/acpi/acpi_pnp.c
> >>>> @@ -355,8 +355,10 @@ static bool acpi_pnp_match(const char *idstr, const struct acpi_device_id **matc
> >>>>     * device represented by it.
> >>>>     */
> >>>>    static const struct acpi_device_id acpi_nonpnp_device_ids[] = {
> >>>> +     {"INT3F0D"},
> >>>>        {"INTC1080"},
> >>>>        {"INTC1081"},
> >>>> +     {"INTC1099"},
> >>>>        {""},
> >>>>    };
> >>>>
> >>>>
> >>>
> >>> Gentle ping on this patch.
> >>
> >> Do you want me to pick it up or do you want to route it through a
> >> different tree?
> >
> > Unless the watchdog maintainers have any objections it's fine if you
> > pick it up.
> >
>
> Ok with me.

Applied as 6.15-rc material, thanks!

Re: [PATCH v3 0/2] Add driver for Intel Over-Clocking Watchdog

[Andy Shevchenko]

Mon, Mar 17, 2025 at 10:55:05AM +0000, Diogo Ivo kirjoitti:

+ Mika

> This series adds a driver for the Intel Over-Clocking Watchdog found in
> the Intel Platform Controller Hub (PCH).
> 
> This watchdog is controlled via a simple single-register interface and
> would otherwise be standard except for the presence of a LOCK bit that
> can only be set once per power cycle, needing extra handling around it.
> 
> Due to the way these devices are described in ACPI tables with both the
> generic PNP0C02 CID and a more detailed ACPI HID we also need to add
> their HIDs to the list of known non-PNP devices. As there are several HIDs
> for what seems to be essentially the same hardware but I don't know all
> the possible HIDs this series does not include an exhaustive list of all
> such HIDs, only those that I could personally test.

This doesn't describe why we need the driver. We have established ACPI WDAT
table. Can't you utilise it by providing in the firmware?

-- 
With Best Regards,
Andy Shevchenko

Re: Re: [PATCH v3 0/2] Add driver for Intel Over-Clocking Watchdog

[Diogo Ivo]

Hi Andy,

> This doesn't describe why we need the driver. We have established ACPI WDAT
> table. Can't you utilise it by providing in the firmware?

You are right, I did not make that clear.

When you say we have established WDAT table do you mean that this should be the
mechanism that we use going forward to control watchdogs in ACPI platforms
generally or that you know that the WDAT for this specific watchdog is present
in the firmware?

I wrote the driver because on the two platforms I have access to (a fairly
recent LG Gram with an i7 1260p and a Siemens BX51A) by delving into the
BIOS options I was not able to find a toggle for them to provide a WDAT table
for this watchdog. In fact, on the LG Gram by searching we can get it to pass
us a WDAT table but for the iTCO watchdog (at least that is what I gathered
from my testing).

This means that to the best of my knowledge there are platforms that do not
provide a WDAT table for the Over-Clocking Watchdog which then makes this
driver necessary.

However I am not an expert on this topic so if there are things that I am
missing I would be happy to work on a better approach.

Thanks,
Diogo