| Message ID | 20250311-ivo-intel_oc_wdt-v1-1-fd470460d9f5@siemens.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | watchdog: Add driver for Intel OC WDT | expand |
On 3/11/25 06:18, 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. > > Signed-off-by: Diogo Ivo <diogo.ivo@siemens.com> > --- > drivers/acpi/acpi_pnp.c | 2 + > drivers/watchdog/Kconfig | 11 ++ > drivers/watchdog/Makefile | 1 + > drivers/watchdog/intel_oc_wdt.c | 235 ++++++++++++++++++++++++++++++++++++++++ > 4 files changed, 249 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"}, > {""}, > }; This needs to be a separate patch. > > diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig > index f81705f8539aa0b12d156a86aae521aa40b4527d..16e6df441bb344c0f91b40bd76b6322ad3016e72 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 && ACPI > + 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..0a2df3440024090f7e342fe7da895a7930ac09b2 > --- /dev/null > +++ b/drivers/watchdog/intel_oc_wdt.c > @@ -0,0 +1,235 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Intel OC Watchdog driver > + * > + * Copyright (C) 2025, Siemens SA > + * 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 > + > +/* > + * 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; > +}; > + > +#define WDT_HEARTBEAT 60 > +static int heartbeat = WDT_HEARTBEAT; Normally this is set to 0 and the default timeout is initialized together with min_timeout and max_timeout. This lets the watchdog core override it with devicetree data (if that is available). > +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 and read the current timeout. > + */ But the timeout is read below ? Do you mean "change the current timeout" ? > + nowayout = true; > + > + 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, "I/O address 0x%04llx already in use, device disabled\n", > + (u64)(oc_wdt->ctrl_res->start)); Use %pa or %pR/%pr, and watch out for multi-line alignment. Guenter
Hi Guenter, thanks for the quick review! On 3/11/25 2:10 PM, Guenter Roeck wrote: > On 3/11/25 06:18, 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. >> >> Signed-off-by: Diogo Ivo <diogo.ivo@siemens.com> >> --- >> drivers/acpi/acpi_pnp.c | 2 + >> drivers/watchdog/Kconfig | 11 ++ >> drivers/watchdog/Makefile | 1 + >> drivers/watchdog/intel_oc_wdt.c | 235 ++++++++++++++++++++++++++++++ >> ++++++++++ >> 4 files changed, 249 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"}, >> {""}, >> }; > > This needs to be a separate patch. I will split it for v2. >> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig >> index >> f81705f8539aa0b12d156a86aae521aa40b4527d..16e6df441bb344c0f91b40bd76b6322ad3016e72 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 && ACPI >> + 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..0a2df3440024090f7e342fe7da895a7930ac09b2 >> --- /dev/null >> +++ b/drivers/watchdog/intel_oc_wdt.c >> @@ -0,0 +1,235 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> +/* >> + * Intel OC Watchdog driver >> + * >> + * Copyright (C) 2025, Siemens SA >> + * 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 >> + >> +/* >> + * 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; >> +}; >> + >> +#define WDT_HEARTBEAT 60 >> +static int heartbeat = WDT_HEARTBEAT; > > Normally this is set to 0 and the default timeout is initialized together > with min_timeout and max_timeout. This lets the watchdog core override it > with devicetree data (if that is available). Ok, thank you for the insight. I will address this for v2. It is unlikely that this driver will have devicetree data but it's better to follow best practice. >> +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 and read the current timeout. >> + */ > > But the timeout is read below ? Do you mean "change the current timeout" ? In this case where the BIOS both enabled the watchdog and set the LOCK bit we cannot change the timeout anymore, meaning that we have to read the value currently in the register and pass it to the watchdog core, which is what is done below. >> + nowayout = true; >> + >> + 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, "I/O address 0x%04llx already in use, device >> disabled\n", >> + (u64)(oc_wdt->ctrl_res->start)); > > Use %pa or %pR/%pr, and watch out for multi-line alignment. I will address this for v2. > > Guenter > Best regards, Diogo
On 3/11/25 07:59, Diogo Ivo wrote: > Hi Guenter, thanks for the quick review! > > On 3/11/25 2:10 PM, Guenter Roeck wrote: >> On 3/11/25 06:18, 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. >>> >>> Signed-off-by: Diogo Ivo <diogo.ivo@siemens.com> >>> --- >>> drivers/acpi/acpi_pnp.c | 2 + >>> drivers/watchdog/Kconfig | 11 ++ >>> drivers/watchdog/Makefile | 1 + >>> drivers/watchdog/intel_oc_wdt.c | 235 ++++++++++++++++++++++++++++++ ++++++++++ >>> 4 files changed, 249 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"}, >>> {""}, >>> }; >> >> This needs to be a separate patch. > > I will split it for v2. > >>> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig >>> index f81705f8539aa0b12d156a86aae521aa40b4527d..16e6df441bb344c0f91b40bd76b6322ad3016e72 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 && ACPI >>> + 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..0a2df3440024090f7e342fe7da895a7930ac09b2 >>> --- /dev/null >>> +++ b/drivers/watchdog/intel_oc_wdt.c >>> @@ -0,0 +1,235 @@ >>> +// SPDX-License-Identifier: GPL-2.0 >>> +/* >>> + * Intel OC Watchdog driver >>> + * >>> + * Copyright (C) 2025, Siemens SA >>> + * 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 >>> + >>> +/* >>> + * 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; >>> +}; >>> + >>> +#define WDT_HEARTBEAT 60 >>> +static int heartbeat = WDT_HEARTBEAT; >> >> Normally this is set to 0 and the default timeout is initialized together >> with min_timeout and max_timeout. This lets the watchdog core override it >> with devicetree data (if that is available). > > Ok, thank you for the insight. I will address this for v2. > It is unlikely that this driver will have devicetree data but it's > better to follow best practice. > I just submitted a patch to have the watchdog core also look into firmware data, which would include data provided by ACPI. >>> +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 and read the current timeout. >>> + */ >> >> But the timeout is read below ? Do you mean "change the current timeout" ? > > In this case where the BIOS both enabled the watchdog and set the LOCK > bit we cannot change the timeout anymore, meaning that we have to read > the value currently in the register and pass it to the watchdog core, > which is what is done below. > Yes, but the comment says " we cannot stop the watchdog and _read_ the current timeout" (emphasis added), suggesting that the current timeout can not be _read_ if locked is true. >>> + nowayout = true; >>> + >>> + oc_wdt->wdd.timeout = (val & INTEL_OC_WDT_TOV) + 1; However, this code does read the timeout even if locked is set. That suggests that it is not possible to _change_ the timeout if locked is set, but that it is possible to read the current timeout. Guenter
On 3/11/25 4:06 PM, Guenter Roeck wrote: > On 3/11/25 07:59, Diogo Ivo wrote: >> On 3/11/25 2:10 PM, Guenter Roeck wrote: >>> On 3/11/25 06:18, 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. >>>> >>>> Signed-off-by: Diogo Ivo <diogo.ivo@siemens.com> >>>> --- >>>> drivers/acpi/acpi_pnp.c | 2 + >>>> drivers/watchdog/Kconfig | 11 ++ >>>> drivers/watchdog/Makefile | 1 + >>>> drivers/watchdog/intel_oc_wdt.c | 235 ++++++++++++++++++++++++++++ >>>> ++ ++++++++++ >>>> 4 files changed, 249 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"}, >>>> {""}, >>>> }; >>> >>> This needs to be a separate patch. >> >> I will split it for v2. >> >>>> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig >>>> index >>>> f81705f8539aa0b12d156a86aae521aa40b4527d..16e6df441bb344c0f91b40bd76b6322ad3016e72 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 && ACPI >>>> + 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..0a2df3440024090f7e342fe7da895a7930ac09b2 >>>> --- /dev/null >>>> +++ b/drivers/watchdog/intel_oc_wdt.c >>>> @@ -0,0 +1,235 @@ >>>> +// SPDX-License-Identifier: GPL-2.0 >>>> +/* >>>> + * Intel OC Watchdog driver >>>> + * >>>> + * Copyright (C) 2025, Siemens SA >>>> + * 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 >>>> + >>>> +/* >>>> + * 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; >>>> +}; >>>> + >>>> +#define WDT_HEARTBEAT 60 >>>> +static int heartbeat = WDT_HEARTBEAT; >>> >>> Normally this is set to 0 and the default timeout is initialized >>> together >>> with min_timeout and max_timeout. This lets the watchdog core >>> override it >>> with devicetree data (if that is available). >> >> Ok, thank you for the insight. I will address this for v2. >> It is unlikely that this driver will have devicetree data but it's >> better to follow best practice. >> > > I just submitted a patch to have the watchdog core also look into > firmware data, > which would include data provided by ACPI. Great to know, and all the more reason to apply your suggested change. >>>> +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 and read the current timeout. >>>> + */ >>> >>> But the timeout is read below ? Do you mean "change the current >>> timeout" ? >> >> In this case where the BIOS both enabled the watchdog and set the LOCK >> bit we cannot change the timeout anymore, meaning that we have to read >> the value currently in the register and pass it to the watchdog core, >> which is what is done below. >> > Yes, but the comment says " we cannot stop the watchdog and _read_ the > current timeout" (emphasis added), suggesting that the current timeout > can not be _read_ if locked is true. > >>>> + nowayout = true; >>>> + >>>> + oc_wdt->wdd.timeout = (val & INTEL_OC_WDT_TOV) + 1; > > However, this code does read the timeout even if locked is set. That > suggests that it is not possible to _change_ the timeout if locked is set, > but that it is possible to read the current timeout. Sorry for the confusing wording. Yes, according to the documentation [1] this is exactly the case, we cannot change the timeout but it is possible to read it. I will word this better in v2 to avoid confusion. Best regards, Diogo [1]: https://edc.intel.com/content/www/us/en/design/publications/14th-generation-core-processors-ioe-p-registers/over-clocking-wdt-control-oc-wdt-ctl-offset-54/
On 11/03/2025 14:18, Diogo Ivo wrote: > diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig > index f81705f8539aa0b12d156a86aae521aa40b4527d..16e6df441bb344c0f91b40bd76b6322ad3016e72 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 && ACPI Why can't this be compile tested? ... > +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 = ACPI_PTR(intel_oc_wdt_match), Drop ACPI_PTR, causes warnigns and is not really beneficial. > + }, > + .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"); > +MODULE_ALIAS("platform:" DRV_NAME); Drop alias, you have match table. Best regards, Krzysztof
Hi Krzysztof, thanks for the review. On 3/12/25 8:50 AM, Krzysztof Kozlowski wrote: > On 11/03/2025 14:18, Diogo Ivo wrote: >> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig >> index f81705f8539aa0b12d156a86aae521aa40b4527d..16e6df441bb344c0f91b40bd76b6322ad3016e72 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 && ACPI > > Why can't this be compile tested? I'll add it in v2 as well as HAS_IOPORT. >> +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 = ACPI_PTR(intel_oc_wdt_match), > > Drop ACPI_PTR, causes warnigns and is not really beneficial. I'll drop it in v2. >> + }, >> + .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"); >> +MODULE_ALIAS("platform:" DRV_NAME); > > Drop alias, you have match table. I'll drop it in v2. > Best regards, > Krzysztof Best regards, Diogo
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"}, {""}, }; diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig index f81705f8539aa0b12d156a86aae521aa40b4527d..16e6df441bb344c0f91b40bd76b6322ad3016e72 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 && ACPI + 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..0a2df3440024090f7e342fe7da895a7930ac09b2 --- /dev/null +++ b/drivers/watchdog/intel_oc_wdt.c @@ -0,0 +1,235 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Intel OC Watchdog driver + * + * Copyright (C) 2025, Siemens SA + * 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 + +/* + * 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; +}; + +#define WDT_HEARTBEAT 60 +static int heartbeat = WDT_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 and read the current timeout. + */ + nowayout = true; + + 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, "I/O address 0x%04llx already in use, device disabled\n", + (u64)(oc_wdt->ctrl_res->start)); + return -EBUSY; + } + + wdd = &oc_wdt->wdd; + wdd->min_timeout = INTEL_OC_WDT_MIN_TOV; + wdd->max_timeout = INTEL_OC_WDT_MAX_TOV; + wdd->info = &intel_oc_wdt_info; + wdd->ops = &intel_oc_wdt_ops; + wdd->parent = dev; + + ret = watchdog_init_timeout(wdd, heartbeat, dev); + if (ret) { + dev_info(dev, "invalid heartbeat (%d): %u s, defaulting to %d s\n", + ret, heartbeat, WDT_HEARTBEAT); + wdd->timeout = WDT_HEARTBEAT; + } + + 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 = ACPI_PTR(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"); +MODULE_ALIAS("platform:" DRV_NAME);
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. Signed-off-by: Diogo Ivo <diogo.ivo@siemens.com> --- drivers/acpi/acpi_pnp.c | 2 + drivers/watchdog/Kconfig | 11 ++ drivers/watchdog/Makefile | 1 + drivers/watchdog/intel_oc_wdt.c | 235 ++++++++++++++++++++++++++++++++++++++++ 4 files changed, 249 insertions(+) --- base-commit: 4d872d51bc9d7b899c1f61534e3dbde72613f627 change-id: 20250227-ivo-intel_oc_wdt-7a483a4d6a04 Best regards,