[PATCH v2 0/4] ACPI / watchdog: Add support for WDAT (Watchdog Action Table)

[PATCH v2 0/4] ACPI / watchdog: Add support for WDAT (Watchdog Action Table)

[Mika Westerberg]

Hi,

The WDAT (Watchdog Action Table) is a special ACPI table introduced by
Microsoft [1] that abstracts the watchdog hardware from the OS. Windows
uses this table for its watchdog implementation instead of a native iTCO
driver.

Microsoft re-licensed the WDAT specification to be under Microsoft
Community Promise license [2] so it should be fine to use it in Linux.

This series brings WDAT table support to Linux.

When the driver is enabled and we find out that there is a WDAT table, the
driver will take over the native iTCO watchdog driver. Main advantage in
this is that we do not need to change the native iTCO driver whenever the
hardware changes. For example in Skylake iTCO moved to sit behind SMBus and
the NO_REBOOT bit was hidden behind P2SB (Primary to Sideband). In addition
we can expect this to be tested much better by OEMs who typically validate
that Windows works fine on their hardware/firmware.

Patch [1/4] adds ACPI enumeration support and the driver itself. It also
introduces acpi_has_watchdog() which can be used to check if we should use
ACPI watchdog or native one.

Patches [2-4/4] prevent creation of the native iTCO platform device if we
detect that the ACPI watchdog (WDAT) should be used instead.

The previous version of the series can be found in [3].

Changes from v1:
  * Moved wdat_wdt.c to live under drivers/watchdog
  * Added checks for timer_period, min_count and max_count
  * Use min_hw_heartbeat_ms and max_hw_heartbeat_ms instead of
    min/max_timeout
  * Instead of stopping the watchdog set WDOG_HW_RUNNING
  * Switched to use devm_watchdog_register_device() and dropped
    wdat_wdt_remove()
  * Do not ping watchdog in resume()
  * Added review tag from Guenter Roeck to patches [2-4/4].

[1] http://msdn.microsoft.com/en-us/windows/hardware/gg463320.aspx
[2] https://msdn.microsoft.com/en-us/openspecifications/dn646766.aspx
[3] http://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1230607.html

Mika Westerberg (4):
  ACPI / watchdog: Add support for WDAT hardware watchdog
  mfd: lpc_ich: Do not create iTCO watchdog when WDAT table exists
  i2c: i801: Do not create iTCO watchdog when WDAT table exists
  platform/x86: intel_pmc_ipc: Do not create iTCO watchdog when WDAT
    table exists

 drivers/acpi/Kconfig                 |   3 +
 drivers/acpi/Makefile                |   1 +
 drivers/acpi/acpi_watchdog.c         | 123 ++++++++
 drivers/acpi/internal.h              |  10 +
 drivers/acpi/scan.c                  |   1 +
 drivers/i2c/busses/i2c-i801.c        |   4 +-
 drivers/mfd/lpc_ich.c                |   4 +
 drivers/platform/x86/intel_pmc_ipc.c |  12 +-
 drivers/watchdog/Kconfig             |  13 +
 drivers/watchdog/Makefile            |   1 +
 drivers/watchdog/wdat_wdt.c          | 525 +++++++++++++++++++++++++++++++++++
 include/linux/acpi.h                 |   6 +
 12 files changed, 698 insertions(+), 5 deletions(-)
 create mode 100644 drivers/acpi/acpi_watchdog.c
 create mode 100644 drivers/watchdog/wdat_wdt.c

-- 
2.9.3

[PATCH v2 1/4] ACPI / watchdog: Add support for WDAT hardware watchdog

[Mika Westerberg]

Starting from Intel Skylake the iTCO watchdog timer registers were moved to
reside in the same register space with SMBus host controller.  Not all
needed registers are available though and we need to unhide P2SB (Primary
to Sideband) device briefly to be able to read status of required NO_REBOOT
bit. The i2c-i801.c SMBus driver used to handle this and creation of the
iTCO watchdog platform device.

Windows, on the other hand, does not use the iTCO watchdog hardware
directly even if it is available. Instead it relies on ACPI Watchdog Action
Table (WDAT) table to describe the watchdog hardware to the OS. This table
contains necessary information about the the hardware and also set of
actions which are executed by a driver as needed.

This patch implements a new watchdog driver that takes advantage of the
ACPI WDAT table. We split the functionality into two parts: first part
enumerates the WDAT table and if found, populates resources and creates
platform device for the actual driver. The second part is the driver
itself.

The reason for the split is that this way we can make the driver itself to
be a module and loaded automatically if the WDAT table is found. Otherwise
the module is not loaded.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 drivers/acpi/Kconfig         |   3 +
 drivers/acpi/Makefile        |   1 +
 drivers/acpi/acpi_watchdog.c | 123 ++++++++++
 drivers/acpi/internal.h      |  10 +
 drivers/acpi/scan.c          |   1 +
 drivers/watchdog/Kconfig     |  13 ++
 drivers/watchdog/Makefile    |   1 +
 drivers/watchdog/wdat_wdt.c  | 525 +++++++++++++++++++++++++++++++++++++++++++
 include/linux/acpi.h         |   6 +
 9 files changed, 683 insertions(+)
 create mode 100644 drivers/acpi/acpi_watchdog.c
 create mode 100644 drivers/watchdog/wdat_wdt.c

diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index 445ce28475b3..dcfa7e9e92f5 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -462,6 +462,9 @@ source "drivers/acpi/nfit/Kconfig"
 source "drivers/acpi/apei/Kconfig"
 source "drivers/acpi/dptf/Kconfig"
 
+config ACPI_WATCHDOG
+	bool
+
 config ACPI_EXTLOG
 	tristate "Extended Error Log support"
 	depends on X86_MCE && X86_LOCAL_APIC
diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
index 5ae9d85c5159..3a1fa8f03749 100644
--- a/drivers/acpi/Makefile
+++ b/drivers/acpi/Makefile
@@ -56,6 +56,7 @@ acpi-$(CONFIG_ACPI_NUMA)	+= numa.o
 acpi-$(CONFIG_ACPI_PROCFS_POWER) += cm_sbs.o
 acpi-y				+= acpi_lpat.o
 acpi-$(CONFIG_ACPI_GENERIC_GSI) += gsi.o
+acpi-$(CONFIG_ACPI_WATCHDOG)	+= acpi_watchdog.o
 
 # These are (potentially) separate modules
 
diff --git a/drivers/acpi/acpi_watchdog.c b/drivers/acpi/acpi_watchdog.c
new file mode 100644
index 000000000000..13caebd679f5
--- /dev/null
+++ b/drivers/acpi/acpi_watchdog.c
@@ -0,0 +1,123 @@
+/*
+ * ACPI watchdog table parsing support.
+ *
+ * Copyright (C) 2016, Intel Corporation
+ * Author: Mika Westerberg <mika.westerberg@linux.intel.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#define pr_fmt(fmt) "ACPI: watchdog: " fmt
+
+#include <linux/acpi.h>
+#include <linux/ioport.h>
+#include <linux/platform_device.h>
+
+#include "internal.h"
+
+/**
+ * Returns true if this system should prefer ACPI based watchdog instead of
+ * the native one (which are typically the same hardware).
+ */
+bool acpi_has_watchdog(void)
+{
+	struct acpi_table_header hdr;
+
+	if (acpi_disabled)
+		return false;
+
+	return ACPI_SUCCESS(acpi_get_table_header(ACPI_SIG_WDAT, 0, &hdr));
+}
+EXPORT_SYMBOL_GPL(acpi_has_watchdog);
+
+void __init acpi_watchdog_init(void)
+{
+	const struct acpi_wdat_entry *entries;
+	const struct acpi_table_wdat *wdat;
+	struct list_head resource_list;
+	struct resource_entry *rentry;
+	struct platform_device *pdev;
+	struct resource *resources;
+	size_t nresources = 0;
+	acpi_status status;
+	int i;
+
+	status = acpi_get_table(ACPI_SIG_WDAT, 0,
+				(struct acpi_table_header **)&wdat);
+	if (ACPI_FAILURE(status)) {
+		/* It is fine if there is no WDAT */
+		return;
+	}
+
+	/* Watchdog disabled by BIOS */
+	if (!(wdat->flags & ACPI_WDAT_ENABLED))
+		return;
+
+	/* Skip legacy PCI WDT devices */
+	if (wdat->pci_segment != 0xff || wdat->pci_bus != 0xff ||
+	    wdat->pci_device != 0xff || wdat->pci_function != 0xff)
+		return;
+
+	INIT_LIST_HEAD(&resource_list);
+
+	entries = (struct acpi_wdat_entry *)(wdat + 1);
+	for (i = 0; i < wdat->entries; i++) {
+		const struct acpi_generic_address *gas;
+		struct resource_entry *rentry;
+		struct resource res;
+		bool found;
+
+		gas = &entries[i].register_region;
+
+		res.start = gas->address;
+		if (gas->space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) {
+			res.flags = IORESOURCE_MEM;
+			res.end = res.start + ALIGN(gas->access_width, 4);
+		} else if (gas->space_id == ACPI_ADR_SPACE_SYSTEM_IO) {
+			res.flags = IORESOURCE_IO;
+			res.end = res.start + gas->access_width;
+		} else {
+			pr_warn("Unsupported address space: %u\n",
+				gas->space_id);
+			goto fail_free_resource_list;
+		}
+
+		found = false;
+		resource_list_for_each_entry(rentry, &resource_list) {
+			if (resource_contains(rentry->res, &res)) {
+				found = true;
+				break;
+			}
+		}
+
+		if (!found) {
+			rentry = resource_list_create_entry(NULL, 0);
+			if (!rentry)
+				goto fail_free_resource_list;
+
+			*rentry->res = res;
+			resource_list_add_tail(rentry, &resource_list);
+			nresources++;
+		}
+	}
+
+	resources = kcalloc(nresources, sizeof(*resources), GFP_KERNEL);
+	if (!resources)
+		goto fail_free_resource_list;
+
+	i = 0;
+	resource_list_for_each_entry(rentry, &resource_list)
+		resources[i++] = *rentry->res;
+
+	pdev = platform_device_register_simple("wdat_wdt", PLATFORM_DEVID_NONE,
+					       resources, nresources);
+	if (IS_ERR(pdev))
+		pr_err("Failed to create platform device\n");
+
+	kfree(resources);
+
+fail_free_resource_list:
+	resource_list_free(&resource_list);
+}
diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h
index 940218ff0193..fb9a7ad96756 100644
--- a/drivers/acpi/internal.h
+++ b/drivers/acpi/internal.h
@@ -225,4 +225,14 @@ static inline void suspend_nvs_restore(void) {}
 void acpi_init_properties(struct acpi_device *adev);
 void acpi_free_properties(struct acpi_device *adev);
 
+/*--------------------------------------------------------------------------
+				Watchdog
+  -------------------------------------------------------------------------- */
+
+#ifdef CONFIG_ACPI_WATCHDOG
+void acpi_watchdog_init(void);
+#else
+static inline void acpi_watchdog_init(void) {}
+#endif
+
 #endif /* _ACPI_INTERNAL_H_ */
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index e878fc799af7..3b85d87021da 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -2002,6 +2002,7 @@ int __init acpi_scan_init(void)
 	acpi_pnp_init();
 	acpi_int340x_thermal_init();
 	acpi_amba_init();
+	acpi_watchdog_init();
 
 	acpi_scan_add_handler(&generic_device_handler);
 
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 1bffe006ca9a..50dbaa805658 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -152,6 +152,19 @@ config TANGOX_WATCHDOG
 
 	  This driver can be built as a module. The module name is tangox_wdt.
 
+config WDAT_WDT
+	tristate "ACPI Watchdog Action Table (WDAT)"
+	depends on ACPI
+	select ACPI_WATCHDOG
+	help
+	  This driver adds support for systems with ACPI Watchdog Action
+	  Table (WDAT) table. Servers typically have this but it can be
+	  found on some desktop machines as well. This driver will take
+	  over the native iTCO watchdog driver found on many Intel CPUs.
+
+	  To compile this driver as module, choose M here: the module will
+	  be called wdat_wdt.
+
 config WM831X_WATCHDOG
 	tristate "WM831x watchdog"
 	depends on MFD_WM831X
diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
index c22ad3ea3539..cba00430151b 100644
--- a/drivers/watchdog/Makefile
+++ b/drivers/watchdog/Makefile
@@ -202,6 +202,7 @@ obj-$(CONFIG_DA9062_WATCHDOG) += da9062_wdt.o
 obj-$(CONFIG_DA9063_WATCHDOG) += da9063_wdt.o
 obj-$(CONFIG_GPIO_WATCHDOG)	+= gpio_wdt.o
 obj-$(CONFIG_TANGOX_WATCHDOG) += tangox_wdt.o
+obj-$(CONFIG_WDAT_WDT) += wdat_wdt.o
 obj-$(CONFIG_WM831X_WATCHDOG) += wm831x_wdt.o
 obj-$(CONFIG_WM8350_WATCHDOG) += wm8350_wdt.o
 obj-$(CONFIG_MAX63XX_WATCHDOG) += max63xx_wdt.o
diff --git a/drivers/watchdog/wdat_wdt.c b/drivers/watchdog/wdat_wdt.c
new file mode 100644
index 000000000000..6b6464596674
--- /dev/null
+++ b/drivers/watchdog/wdat_wdt.c
@@ -0,0 +1,525 @@
+/*
+ * ACPI Hardware Watchdog (WDAT) driver.
+ *
+ * Copyright (C) 2016, Intel Corporation
+ * Author: Mika Westerberg <mika.westerberg@linux.intel.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/acpi.h>
+#include <linux/ioport.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/pm.h>
+#include <linux/watchdog.h>
+
+#define MAX_WDAT_ACTIONS ACPI_WDAT_ACTION_RESERVED
+
+/**
+ * struct wdat_instruction - Single ACPI WDAT instruction
+ * @entry: Copy of the ACPI table instruction
+ * @reg: Register the instruction is accessing
+ * @node: Next instruction in action sequence
+ */
+struct wdat_instruction {
+	struct acpi_wdat_entry entry;
+	void __iomem *reg;
+	struct list_head node;
+};
+
+/**
+ * struct wdat_wdt - ACPI WDAT watchdog device
+ * @pdev: Parent platform device
+ * @wdd: Watchdog core device
+ * @period: How long is one watchdog period in ms
+ * @stopped_in_sleep: Is this watchdog stopped by the firmware in S1-S5
+ * @stopped: Was the watchdog stopped by the driver in suspend
+ * @actions: An array of instruction lists indexed by an action number from
+ *           the WDAT table. There can be %NULL entries for not implemented
+ *           actions.
+ */
+struct wdat_wdt {
+	struct platform_device *pdev;
+	struct watchdog_device wdd;
+	unsigned int period;
+	bool stopped_in_sleep;
+	bool stopped;
+	struct list_head *instructions[MAX_WDAT_ACTIONS];
+};
+
+#define to_wdat_wdt(wdd) container_of(wdd, struct wdat_wdt, wdd)
+
+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 wdat_wdt_read(struct wdat_wdt *wdat,
+	 const struct wdat_instruction *instr, u32 *value)
+{
+	const struct acpi_generic_address *gas = &instr->entry.register_region;
+
+	switch (gas->access_width) {
+	case 1:
+		*value = ioread8(instr->reg);
+		break;
+	case 2:
+		*value = ioread16(instr->reg);
+		break;
+	case 3:
+		*value = ioread32(instr->reg);
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	dev_dbg(&wdat->pdev->dev, "Read %#x from 0x%08llx\n", *value,
+		gas->address);
+
+	return 0;
+}
+
+static int wdat_wdt_write(struct wdat_wdt *wdat,
+	const struct wdat_instruction *instr, u32 value)
+{
+	const struct acpi_generic_address *gas = &instr->entry.register_region;
+
+	switch (gas->access_width) {
+	case 1:
+		iowrite8((u8)value, instr->reg);
+		break;
+	case 2:
+		iowrite16((u16)value, instr->reg);
+		break;
+	case 3:
+		iowrite32(value, instr->reg);
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	dev_dbg(&wdat->pdev->dev, "Wrote %#x to 0x%08llx\n", value,
+		gas->address);
+
+	return 0;
+}
+
+static int wdat_wdt_run_action(struct wdat_wdt *wdat, unsigned int action,
+			       u32 param, u32 *retval)
+{
+	struct wdat_instruction *instr;
+
+	if (action >= ARRAY_SIZE(wdat->instructions))
+		return -EINVAL;
+
+	if (!wdat->instructions[action])
+		return -EOPNOTSUPP;
+
+	dev_dbg(&wdat->pdev->dev, "Running action %#x\n", action);
+
+	/* Run each instruction sequentially */
+	list_for_each_entry(instr, wdat->instructions[action], node) {
+		const struct acpi_wdat_entry *entry = &instr->entry;
+		const struct acpi_generic_address *gas;
+		u32 flags, value, mask, x, y;
+		bool preserve;
+		int ret;
+
+		gas = &entry->register_region;
+
+		preserve = entry->instruction & ACPI_WDAT_PRESERVE_REGISTER;
+		flags = entry->instruction & ~ACPI_WDAT_PRESERVE_REGISTER;
+		value = entry->value;
+		mask = entry->mask;
+
+		switch (flags) {
+		case ACPI_WDAT_READ_VALUE:
+			ret = wdat_wdt_read(wdat, instr, &x);
+			if (ret)
+				return ret;
+			x >>= gas->bit_offset;
+			x &= mask;
+			if (retval)
+				*retval = x == value;
+			break;
+
+		case ACPI_WDAT_READ_COUNTDOWN:
+			ret = wdat_wdt_read(wdat, instr, &x);
+			if (ret)
+				return ret;
+			x >>= gas->bit_offset;
+			x &= mask;
+			if (retval)
+				*retval = x;
+			break;
+
+		case ACPI_WDAT_WRITE_VALUE:
+			x = value & mask;
+			x <<= gas->bit_offset;
+			if (preserve) {
+				ret = wdat_wdt_read(wdat, instr, &y);
+				if (ret)
+					return ret;
+				y = y & ~(mask << gas->bit_offset);
+				x |= y;
+			}
+			ret = wdat_wdt_write(wdat, instr, x);
+			if (ret)
+				return ret;
+			break;
+
+		case ACPI_WDAT_WRITE_COUNTDOWN:
+			x = param;
+			x &= mask;
+			x <<= gas->bit_offset;
+			if (preserve) {
+				ret = wdat_wdt_read(wdat, instr, &y);
+				if (ret)
+					return ret;
+				y = y & ~(mask << gas->bit_offset);
+				x |= y;
+			}
+			ret = wdat_wdt_write(wdat, instr, x);
+			if (ret)
+				return ret;
+			break;
+
+		default:
+			dev_err(&wdat->pdev->dev, "Unknown instruction: %u\n",
+				flags);
+			return -EINVAL;
+		}
+	}
+
+	return 0;
+}
+
+static int wdat_wdt_enable_reboot(struct wdat_wdt *wdat)
+{
+	int ret;
+
+	/*
+	 * WDAT specification says that the watchdog is required to reboot
+	 * the system when it fires. However, it also states that it is
+	 * recommeded to make it configurable through hardware register. We
+	 * enable reboot now if it is configrable, just in case.
+	 */
+	ret = wdat_wdt_run_action(wdat, ACPI_WDAT_SET_REBOOT, 0, 0);
+	if (ret && ret != -EOPNOTSUPP) {
+		dev_err(&wdat->pdev->dev,
+			"Failed to enable reboot when watchdog triggers\n");
+		return ret;
+	}
+
+	return 0;
+}
+
+static void wdat_wdt_boot_status(struct wdat_wdt *wdat)
+{
+	u32 boot_status = 0;
+	int ret;
+
+	ret = wdat_wdt_run_action(wdat, ACPI_WDAT_GET_STATUS, 0, &boot_status);
+	if (ret && ret != -EOPNOTSUPP) {
+		dev_err(&wdat->pdev->dev, "Failed to read boot status\n");
+		return;
+	}
+
+	if (boot_status)
+		wdat->wdd.bootstatus = WDIOF_CARDRESET;
+
+	/* Clear the boot status in case BIOS did not do it */
+	ret = wdat_wdt_run_action(wdat, ACPI_WDAT_SET_STATUS, 0, 0);
+	if (ret && ret != -EOPNOTSUPP)
+		dev_err(&wdat->pdev->dev, "Failed to clear boot status\n");
+}
+
+static void wdat_wdt_set_running(struct wdat_wdt *wdat)
+{
+	u32 running = 0;
+	int ret;
+
+	ret = wdat_wdt_run_action(wdat, ACPI_WDAT_GET_RUNNING_STATE, 0,
+				  &running);
+	if (ret && ret != -EOPNOTSUPP)
+		dev_err(&wdat->pdev->dev, "Failed to read running state\n");
+
+	if (running)
+		set_bit(WDOG_HW_RUNNING, &wdat->wdd.status);
+}
+
+static int wdat_wdt_start(struct watchdog_device *wdd)
+{
+	return wdat_wdt_run_action(to_wdat_wdt(wdd),
+				   ACPI_WDAT_SET_RUNNING_STATE, 0, NULL);
+}
+
+static int wdat_wdt_stop(struct watchdog_device *wdd)
+{
+	return wdat_wdt_run_action(to_wdat_wdt(wdd),
+				   ACPI_WDAT_SET_STOPPED_STATE, 0, NULL);
+}
+
+static int wdat_wdt_ping(struct watchdog_device *wdd)
+{
+	return wdat_wdt_run_action(to_wdat_wdt(wdd), ACPI_WDAT_RESET, 0, NULL);
+}
+
+static int wdat_wdt_set_timeout(struct watchdog_device *wdd,
+				unsigned int timeout)
+{
+	struct wdat_wdt *wdat = to_wdat_wdt(wdd);
+	unsigned int periods;
+	int ret;
+
+	periods = timeout * 1000 / wdat->period;
+	ret = wdat_wdt_run_action(wdat, ACPI_WDAT_SET_COUNTDOWN, periods, NULL);
+	if (!ret)
+		wdd->timeout = timeout;
+	return ret;
+}
+
+static unsigned int wdat_wdt_get_timeleft(struct watchdog_device *wdd)
+{
+	struct wdat_wdt *wdat = to_wdat_wdt(wdd);
+	u32 periods = 0;
+
+	wdat_wdt_run_action(wdat, ACPI_WDAT_GET_COUNTDOWN, 0, &periods);
+	return periods * wdat->period / 1000;
+}
+
+static const struct watchdog_info wdat_wdt_info = {
+	.options = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE,
+	.firmware_version = 0,
+	.identity = "wdat_wdt",
+};
+
+static const struct watchdog_ops wdat_wdt_ops = {
+	.owner = THIS_MODULE,
+	.start = wdat_wdt_start,
+	.stop = wdat_wdt_stop,
+	.ping = wdat_wdt_ping,
+	.set_timeout = wdat_wdt_set_timeout,
+	.get_timeleft = wdat_wdt_get_timeleft,
+};
+
+static int wdat_wdt_probe(struct platform_device *pdev)
+{
+	const struct acpi_wdat_entry *entries;
+	const struct acpi_table_wdat *tbl;
+	struct wdat_wdt *wdat;
+	struct resource *res;
+	void __iomem **regs;
+	acpi_status status;
+	int i, ret;
+
+	status = acpi_get_table(ACPI_SIG_WDAT, 0,
+				(struct acpi_table_header **)&tbl);
+	if (ACPI_FAILURE(status))
+		return -ENODEV;
+
+	wdat = devm_kzalloc(&pdev->dev, sizeof(*wdat), GFP_KERNEL);
+	if (!wdat)
+		return -ENOMEM;
+
+	regs = devm_kcalloc(&pdev->dev, pdev->num_resources, sizeof(*regs),
+			    GFP_KERNEL);
+	if (!regs)
+		return -ENOMEM;
+
+	/* WDAT specification wants to have >= 1ms period */
+	if (tbl->timer_period < 1)
+		return -EINVAL;
+	if (tbl->min_count > tbl->max_count)
+		return -EINVAL;
+
+	wdat->period = tbl->timer_period;
+	wdat->wdd.min_hw_heartbeat_ms = wdat->period * tbl->min_count;
+	wdat->wdd.max_hw_heartbeat_ms = wdat->period * tbl->max_count;
+	wdat->stopped_in_sleep = tbl->flags & ACPI_WDAT_STOPPED;
+	wdat->wdd.info = &wdat_wdt_info;
+	wdat->wdd.ops = &wdat_wdt_ops;
+	wdat->pdev = pdev;
+
+	/* Request and map all resources */
+	for (i = 0; i < pdev->num_resources; i++) {
+		void __iomem *reg;
+
+		res = &pdev->resource[i];
+		if (resource_type(res) == IORESOURCE_MEM) {
+			reg = devm_ioremap_resource(&pdev->dev, res);
+		} else if (resource_type(res) == IORESOURCE_IO) {
+			reg = devm_ioport_map(&pdev->dev, res->start, 1);
+		} else {
+			dev_err(&pdev->dev, "Unsupported resource\n");
+			return -EINVAL;
+		}
+
+		if (!reg)
+			return -ENOMEM;
+
+		regs[i] = reg;
+	}
+
+	entries = (struct acpi_wdat_entry *)(tbl + 1);
+	for (i = 0; i < tbl->entries; i++) {
+		const struct acpi_generic_address *gas;
+		struct wdat_instruction *instr;
+		struct list_head *instructions;
+		unsigned int action;
+		struct resource r;
+		int j;
+
+		action = entries[i].action;
+		if (action >= MAX_WDAT_ACTIONS) {
+			dev_dbg(&pdev->dev, "Skipping unknown action: %u\n",
+				action);
+			continue;
+		}
+
+		instr = devm_kzalloc(&pdev->dev, sizeof(*instr), GFP_KERNEL);
+		if (!instr)
+			return -ENOMEM;
+
+		INIT_LIST_HEAD(&instr->node);
+		instr->entry = entries[i];
+
+		gas = &entries[i].register_region;
+
+		memset(&r, 0, sizeof(r));
+		r.start = gas->address;
+		r.end = r.start + gas->access_width;
+		if (gas->space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) {
+			r.flags = IORESOURCE_MEM;
+		} else if (gas->space_id == ACPI_ADR_SPACE_SYSTEM_IO) {
+			r.flags = IORESOURCE_IO;
+		} else {
+			dev_dbg(&pdev->dev, "Unsupported address space: %d\n",
+				gas->space_id);
+			continue;
+		}
+
+		/* Find the matching resource */
+		for (j = 0; j < pdev->num_resources; j++) {
+			res = &pdev->resource[j];
+			if (resource_contains(res, &r)) {
+				instr->reg = regs[j] + r.start - res->start;
+				break;
+			}
+		}
+
+		if (!instr->reg) {
+			dev_err(&pdev->dev, "I/O resource not found\n");
+			return -EINVAL;
+		}
+
+		instructions = wdat->instructions[action];
+		if (!instructions) {
+			instructions = devm_kzalloc(&pdev->dev,
+					sizeof(*instructions), GFP_KERNEL);
+			if (!instructions)
+				return -ENOMEM;
+
+			INIT_LIST_HEAD(instructions);
+			wdat->instructions[action] = instructions;
+		}
+
+		list_add_tail(&instr->node, instructions);
+	}
+
+	wdat_wdt_boot_status(wdat);
+	wdat_wdt_set_running(wdat);
+
+	ret = wdat_wdt_enable_reboot(wdat);
+	if (ret)
+		return ret;
+
+	platform_set_drvdata(pdev, wdat);
+
+	watchdog_set_nowayout(&wdat->wdd, nowayout);
+	return devm_watchdog_register_device(&pdev->dev, &wdat->wdd);
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int wdat_wdt_suspend_noirq(struct device *dev)
+{
+	struct platform_device *pdev = to_platform_device(dev);
+	struct wdat_wdt *wdat = platform_get_drvdata(pdev);
+	int ret;
+
+	if (!watchdog_active(&wdat->wdd))
+		return 0;
+
+	/*
+	 * We need to stop the watchdog if firmare is not doing it or if we
+	 * are going suspend to idle (where firmware is not involved). If
+	 * firmware is stopping the watchdog we kick it here one more time
+	 * to give it some time.
+	 */
+	wdat->stopped = false;
+	if (acpi_target_system_state() == ACPI_STATE_S0 ||
+	    !wdat->stopped_in_sleep) {
+		ret = wdat_wdt_stop(&wdat->wdd);
+		if (!ret)
+			wdat->stopped = true;
+	} else {
+		ret = wdat_wdt_ping(&wdat->wdd);
+	}
+
+	return ret;
+}
+
+static int wdat_wdt_resume_noirq(struct device *dev)
+{
+	struct platform_device *pdev = to_platform_device(dev);
+	struct wdat_wdt *wdat = platform_get_drvdata(pdev);
+	int ret;
+
+	if (!watchdog_active(&wdat->wdd))
+		return 0;
+
+	if (!wdat->stopped) {
+		/*
+		 * Looks like the boot firmware reinitializes the watchdog
+		 * before it hands off to the OS on resume from sleep so we
+		 * stop and reprogram the watchdog here.
+		 */
+		ret = wdat_wdt_stop(&wdat->wdd);
+		if (ret)
+			return ret;
+
+		ret = wdat_wdt_set_timeout(&wdat->wdd, wdat->wdd.timeout);
+		if (ret)
+			return ret;
+
+		ret = wdat_wdt_enable_reboot(wdat);
+		if (ret)
+			return ret;
+	}
+
+	return wdat_wdt_start(&wdat->wdd);
+}
+#endif
+
+static const struct dev_pm_ops wdat_wdt_pm_ops = {
+	SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(wdat_wdt_suspend_noirq,
+				      wdat_wdt_resume_noirq)
+};
+
+static struct platform_driver wdat_wdt_driver = {
+	.probe = wdat_wdt_probe,
+	.driver = {
+		.name = "wdat_wdt",
+		.pm = &wdat_wdt_pm_ops,
+	},
+};
+
+module_platform_driver(wdat_wdt_driver);
+
+MODULE_AUTHOR("Mika Westerberg <mika.westerberg@linux.intel.com>");
+MODULE_DESCRIPTION("ACPI Hardware Watchdog (WDAT) driver");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:wdat_wdt");
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index c5eaf2f80a4c..8ff6ca4a2639 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -1074,4 +1074,10 @@ void acpi_table_upgrade(void);
 static inline void acpi_table_upgrade(void) { }
 #endif
 
+#if defined(CONFIG_ACPI) && defined(CONFIG_ACPI_WATCHDOG)
+extern bool acpi_has_watchdog(void);
+#else
+static inline bool acpi_has_watchdog(void) { return false; }
+#endif
+
 #endif	/*_LINUX_ACPI_H*/
-- 
2.9.3

[PATCH v2 2/4] mfd: lpc_ich: Do not create iTCO watchdog when WDAT table exists

[Mika Westerberg]

ACPI WDAT table is the preferred way to use hardware watchdog over the
native iTCO_wdt. Windows only uses this table for its hardware watchdog
implementation so we should be relatively safe to trust it has been
validated by OEMs

Prevent iTCO watchdog creation if we detect that there is ACPI WDAT table.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Reviewed-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/mfd/lpc_ich.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/mfd/lpc_ich.c b/drivers/mfd/lpc_ich.c
index bd3aa4578346..c8dee47b45d9 100644
--- a/drivers/mfd/lpc_ich.c
+++ b/drivers/mfd/lpc_ich.c
@@ -984,6 +984,10 @@ static int lpc_ich_init_wdt(struct pci_dev *dev)
 	int ret;
 	struct resource *res;
 
+	/* If we have ACPI based watchdog use that instead */
+	if (acpi_has_watchdog())
+		return -ENODEV;
+
 	/* Setup power management base register */
 	pci_read_config_dword(dev, priv->abase, &base_addr_cfg);
 	base_addr = base_addr_cfg & 0x0000ff80;
-- 
2.9.3

[PATCH v2 3/4] i2c: i801: Do not create iTCO watchdog when WDAT table exists

[Mika Westerberg]

ACPI WDAT table is the preferred way to use hardware watchdog over the
native iTCO_wdt. Windows only uses this table for its hardware watchdog
implementation so we should be relatively safe to trust it has been
validated by OEMs

Prevent iTCO watchdog creation if we detect that there is ACPI WDAT table.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Reviewed-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/i2c/busses/i2c-i801.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
index 5ef9b733d153..26298af73232 100644
--- a/drivers/i2c/busses/i2c-i801.c
+++ b/drivers/i2c/busses/i2c-i801.c
@@ -1486,7 +1486,9 @@ static int i801_probe(struct pci_dev *dev, const struct pci_device_id *id)
 		priv->features |= FEATURE_IRQ;
 		priv->features |= FEATURE_SMBUS_PEC;
 		priv->features |= FEATURE_BLOCK_BUFFER;
-		priv->features |= FEATURE_TCO;
+		/* If we have ACPI based watchdog use that instead */
+		if (!acpi_has_watchdog())
+			priv->features |= FEATURE_TCO;
 		priv->features |= FEATURE_HOST_NOTIFY;
 		break;
 
-- 
2.9.3

[PATCH v2 4/4] platform/x86: intel_pmc_ipc: Do not create iTCO watchdog when WDAT table exists

[Mika Westerberg]

ACPI WDAT table is the preferred way to use hardware watchdog over the
native iTCO_wdt. Windows only uses this table for its hardware watchdog
implementation so we should be relatively safe to trust it has been
validated by OEMs.

Prevent iTCO watchdog creation if we detect that there is an ACPI WDAT
table.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Reviewed-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/platform/x86/intel_pmc_ipc.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/platform/x86/intel_pmc_ipc.c b/drivers/platform/x86/intel_pmc_ipc.c
index b86e1bcaa055..a511d518206b 100644
--- a/drivers/platform/x86/intel_pmc_ipc.c
+++ b/drivers/platform/x86/intel_pmc_ipc.c
@@ -651,11 +651,15 @@ static int ipc_create_pmc_devices(void)
 {
 	int ret;
 
-	ret = ipc_create_tco_device();
-	if (ret) {
-		dev_err(ipcdev.dev, "Failed to add tco platform device\n");
-		return ret;
+	/* If we have ACPI based watchdog use that instead */
+	if (!acpi_has_watchdog()) {
+		ret = ipc_create_tco_device();
+		if (ret) {
+			dev_err(ipcdev.dev, "Failed to add tco platform device\n");
+			return ret;
+		}
 	}
+
 	ret = ipc_create_punit_device();
 	if (ret) {
 		dev_err(ipcdev.dev, "Failed to add punit platform device\n");
-- 
2.9.3

Re: [PATCH v2 1/4] ACPI / watchdog: Add support for WDAT hardware watchdog

[Guenter Roeck]

On 09/20/2016 05:30 AM, Mika Westerberg wrote:
> Starting from Intel Skylake the iTCO watchdog timer registers were moved to
> reside in the same register space with SMBus host controller.  Not all
> needed registers are available though and we need to unhide P2SB (Primary
> to Sideband) device briefly to be able to read status of required NO_REBOOT
> bit. The i2c-i801.c SMBus driver used to handle this and creation of the
> iTCO watchdog platform device.
>
> Windows, on the other hand, does not use the iTCO watchdog hardware
> directly even if it is available. Instead it relies on ACPI Watchdog Action
> Table (WDAT) table to describe the watchdog hardware to the OS. This table
> contains necessary information about the the hardware and also set of
> actions which are executed by a driver as needed.
>
> This patch implements a new watchdog driver that takes advantage of the
> ACPI WDAT table. We split the functionality into two parts: first part
> enumerates the WDAT table and if found, populates resources and creates
> platform device for the actual driver. The second part is the driver
> itself.
>
> The reason for the split is that this way we can make the driver itself to
> be a module and loaded automatically if the WDAT table is found. Otherwise
> the module is not loaded.
>
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> ---
>  drivers/acpi/Kconfig         |   3 +
>  drivers/acpi/Makefile        |   1 +
>  drivers/acpi/acpi_watchdog.c | 123 ++++++++++
>  drivers/acpi/internal.h      |  10 +
>  drivers/acpi/scan.c          |   1 +
>  drivers/watchdog/Kconfig     |  13 ++
>  drivers/watchdog/Makefile    |   1 +
>  drivers/watchdog/wdat_wdt.c  | 525 +++++++++++++++++++++++++++++++++++++++++++
>  include/linux/acpi.h         |   6 +
>  9 files changed, 683 insertions(+)
>  create mode 100644 drivers/acpi/acpi_watchdog.c
>  create mode 100644 drivers/watchdog/wdat_wdt.c
>
> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
> index 445ce28475b3..dcfa7e9e92f5 100644
> --- a/drivers/acpi/Kconfig
> +++ b/drivers/acpi/Kconfig
> @@ -462,6 +462,9 @@ source "drivers/acpi/nfit/Kconfig"
>  source "drivers/acpi/apei/Kconfig"
>  source "drivers/acpi/dptf/Kconfig"
>
> +config ACPI_WATCHDOG
> +	bool
> +
>  config ACPI_EXTLOG
>  	tristate "Extended Error Log support"
>  	depends on X86_MCE && X86_LOCAL_APIC
> diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
> index 5ae9d85c5159..3a1fa8f03749 100644
> --- a/drivers/acpi/Makefile
> +++ b/drivers/acpi/Makefile
> @@ -56,6 +56,7 @@ acpi-$(CONFIG_ACPI_NUMA)	+= numa.o
>  acpi-$(CONFIG_ACPI_PROCFS_POWER) += cm_sbs.o
>  acpi-y				+= acpi_lpat.o
>  acpi-$(CONFIG_ACPI_GENERIC_GSI) += gsi.o
> +acpi-$(CONFIG_ACPI_WATCHDOG)	+= acpi_watchdog.o
>
>  # These are (potentially) separate modules
>
> diff --git a/drivers/acpi/acpi_watchdog.c b/drivers/acpi/acpi_watchdog.c
> new file mode 100644
> index 000000000000..13caebd679f5
> --- /dev/null
> +++ b/drivers/acpi/acpi_watchdog.c
> @@ -0,0 +1,123 @@
> +/*
> + * ACPI watchdog table parsing support.
> + *
> + * Copyright (C) 2016, Intel Corporation
> + * Author: Mika Westerberg <mika.westerberg@linux.intel.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#define pr_fmt(fmt) "ACPI: watchdog: " fmt
> +
> +#include <linux/acpi.h>
> +#include <linux/ioport.h>
> +#include <linux/platform_device.h>
> +
> +#include "internal.h"
> +
> +/**
> + * Returns true if this system should prefer ACPI based watchdog instead of
> + * the native one (which are typically the same hardware).
> + */
> +bool acpi_has_watchdog(void)
> +{
> +	struct acpi_table_header hdr;
> +
> +	if (acpi_disabled)
> +		return false;
> +
> +	return ACPI_SUCCESS(acpi_get_table_header(ACPI_SIG_WDAT, 0, &hdr));
> +}
> +EXPORT_SYMBOL_GPL(acpi_has_watchdog);
> +
> +void __init acpi_watchdog_init(void)
> +{
> +	const struct acpi_wdat_entry *entries;
> +	const struct acpi_table_wdat *wdat;
> +	struct list_head resource_list;
> +	struct resource_entry *rentry;
> +	struct platform_device *pdev;
> +	struct resource *resources;
> +	size_t nresources = 0;
> +	acpi_status status;
> +	int i;
> +
> +	status = acpi_get_table(ACPI_SIG_WDAT, 0,
> +				(struct acpi_table_header **)&wdat);
> +	if (ACPI_FAILURE(status)) {
> +		/* It is fine if there is no WDAT */
> +		return;
> +	}
> +
> +	/* Watchdog disabled by BIOS */
> +	if (!(wdat->flags & ACPI_WDAT_ENABLED))
> +		return;
> +
> +	/* Skip legacy PCI WDT devices */
> +	if (wdat->pci_segment != 0xff || wdat->pci_bus != 0xff ||
> +	    wdat->pci_device != 0xff || wdat->pci_function != 0xff)
> +		return;
> +
> +	INIT_LIST_HEAD(&resource_list);
> +
> +	entries = (struct acpi_wdat_entry *)(wdat + 1);
> +	for (i = 0; i < wdat->entries; i++) {
> +		const struct acpi_generic_address *gas;
> +		struct resource_entry *rentry;
> +		struct resource res;
> +		bool found;
> +
> +		gas = &entries[i].register_region;
> +
> +		res.start = gas->address;
> +		if (gas->space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) {
> +			res.flags = IORESOURCE_MEM;
> +			res.end = res.start + ALIGN(gas->access_width, 4);
> +		} else if (gas->space_id == ACPI_ADR_SPACE_SYSTEM_IO) {
> +			res.flags = IORESOURCE_IO;
> +			res.end = res.start + gas->access_width;
> +		} else {
> +			pr_warn("Unsupported address space: %u\n",
> +				gas->space_id);
> +			goto fail_free_resource_list;
> +		}
> +
> +		found = false;
> +		resource_list_for_each_entry(rentry, &resource_list) {
> +			if (resource_contains(rentry->res, &res)) {
> +				found = true;
> +				break;
> +			}
> +		}
> +
> +		if (!found) {
> +			rentry = resource_list_create_entry(NULL, 0);
> +			if (!rentry)
> +				goto fail_free_resource_list;
> +
> +			*rentry->res = res;
> +			resource_list_add_tail(rentry, &resource_list);
> +			nresources++;
> +		}
> +	}
> +
> +	resources = kcalloc(nresources, sizeof(*resources), GFP_KERNEL);
> +	if (!resources)
> +		goto fail_free_resource_list;
> +
> +	i = 0;
> +	resource_list_for_each_entry(rentry, &resource_list)
> +		resources[i++] = *rentry->res;
> +
> +	pdev = platform_device_register_simple("wdat_wdt", PLATFORM_DEVID_NONE,
> +					       resources, nresources);
> +	if (IS_ERR(pdev))
> +		pr_err("Failed to create platform device\n");
> +
> +	kfree(resources);
> +
> +fail_free_resource_list:
> +	resource_list_free(&resource_list);
> +}
> diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h
> index 940218ff0193..fb9a7ad96756 100644
> --- a/drivers/acpi/internal.h
> +++ b/drivers/acpi/internal.h
> @@ -225,4 +225,14 @@ static inline void suspend_nvs_restore(void) {}
>  void acpi_init_properties(struct acpi_device *adev);
>  void acpi_free_properties(struct acpi_device *adev);
>
> +/*--------------------------------------------------------------------------
> +				Watchdog
> +  -------------------------------------------------------------------------- */
> +
> +#ifdef CONFIG_ACPI_WATCHDOG
> +void acpi_watchdog_init(void);
> +#else
> +static inline void acpi_watchdog_init(void) {}
> +#endif
> +
>  #endif /* _ACPI_INTERNAL_H_ */
> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> index e878fc799af7..3b85d87021da 100644
> --- a/drivers/acpi/scan.c
> +++ b/drivers/acpi/scan.c
> @@ -2002,6 +2002,7 @@ int __init acpi_scan_init(void)
>  	acpi_pnp_init();
>  	acpi_int340x_thermal_init();
>  	acpi_amba_init();
> +	acpi_watchdog_init();
>
>  	acpi_scan_add_handler(&generic_device_handler);
>
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index 1bffe006ca9a..50dbaa805658 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -152,6 +152,19 @@ config TANGOX_WATCHDOG
>
>  	  This driver can be built as a module. The module name is tangox_wdt.
>
> +config WDAT_WDT
> +	tristate "ACPI Watchdog Action Table (WDAT)"
> +	depends on ACPI
> +	select ACPI_WATCHDOG
> +	help
> +	  This driver adds support for systems with ACPI Watchdog Action
> +	  Table (WDAT) table. Servers typically have this but it can be
> +	  found on some desktop machines as well. This driver will take
> +	  over the native iTCO watchdog driver found on many Intel CPUs.
> +
> +	  To compile this driver as module, choose M here: the module will
> +	  be called wdat_wdt.
> +
>  config WM831X_WATCHDOG
>  	tristate "WM831x watchdog"
>  	depends on MFD_WM831X
> diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
> index c22ad3ea3539..cba00430151b 100644
> --- a/drivers/watchdog/Makefile
> +++ b/drivers/watchdog/Makefile
> @@ -202,6 +202,7 @@ obj-$(CONFIG_DA9062_WATCHDOG) += da9062_wdt.o
>  obj-$(CONFIG_DA9063_WATCHDOG) += da9063_wdt.o
>  obj-$(CONFIG_GPIO_WATCHDOG)	+= gpio_wdt.o
>  obj-$(CONFIG_TANGOX_WATCHDOG) += tangox_wdt.o
> +obj-$(CONFIG_WDAT_WDT) += wdat_wdt.o
>  obj-$(CONFIG_WM831X_WATCHDOG) += wm831x_wdt.o
>  obj-$(CONFIG_WM8350_WATCHDOG) += wm8350_wdt.o
>  obj-$(CONFIG_MAX63XX_WATCHDOG) += max63xx_wdt.o
> diff --git a/drivers/watchdog/wdat_wdt.c b/drivers/watchdog/wdat_wdt.c
> new file mode 100644
> index 000000000000..6b6464596674
> --- /dev/null
> +++ b/drivers/watchdog/wdat_wdt.c
> @@ -0,0 +1,525 @@
> +/*
> + * ACPI Hardware Watchdog (WDAT) driver.
> + *
> + * Copyright (C) 2016, Intel Corporation
> + * Author: Mika Westerberg <mika.westerberg@linux.intel.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/acpi.h>
> +#include <linux/ioport.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm.h>
> +#include <linux/watchdog.h>
> +
> +#define MAX_WDAT_ACTIONS ACPI_WDAT_ACTION_RESERVED
> +
> +/**
> + * struct wdat_instruction - Single ACPI WDAT instruction
> + * @entry: Copy of the ACPI table instruction
> + * @reg: Register the instruction is accessing
> + * @node: Next instruction in action sequence
> + */
> +struct wdat_instruction {
> +	struct acpi_wdat_entry entry;
> +	void __iomem *reg;
> +	struct list_head node;
> +};
> +
> +/**
> + * struct wdat_wdt - ACPI WDAT watchdog device
> + * @pdev: Parent platform device
> + * @wdd: Watchdog core device
> + * @period: How long is one watchdog period in ms
> + * @stopped_in_sleep: Is this watchdog stopped by the firmware in S1-S5
> + * @stopped: Was the watchdog stopped by the driver in suspend
> + * @actions: An array of instruction lists indexed by an action number from
> + *           the WDAT table. There can be %NULL entries for not implemented
> + *           actions.
> + */
> +struct wdat_wdt {
> +	struct platform_device *pdev;
> +	struct watchdog_device wdd;
> +	unsigned int period;
> +	bool stopped_in_sleep;
> +	bool stopped;
> +	struct list_head *instructions[MAX_WDAT_ACTIONS];
> +};
> +
> +#define to_wdat_wdt(wdd) container_of(wdd, struct wdat_wdt, wdd)
> +
> +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 wdat_wdt_read(struct wdat_wdt *wdat,
> +	 const struct wdat_instruction *instr, u32 *value)
> +{
> +	const struct acpi_generic_address *gas = &instr->entry.register_region;
> +
> +	switch (gas->access_width) {
> +	case 1:
> +		*value = ioread8(instr->reg);
> +		break;
> +	case 2:
> +		*value = ioread16(instr->reg);
> +		break;
> +	case 3:
> +		*value = ioread32(instr->reg);
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	dev_dbg(&wdat->pdev->dev, "Read %#x from 0x%08llx\n", *value,
> +		gas->address);
> +
> +	return 0;
> +}
> +
> +static int wdat_wdt_write(struct wdat_wdt *wdat,
> +	const struct wdat_instruction *instr, u32 value)
> +{
> +	const struct acpi_generic_address *gas = &instr->entry.register_region;
> +
> +	switch (gas->access_width) {
> +	case 1:
> +		iowrite8((u8)value, instr->reg);
> +		break;
> +	case 2:
> +		iowrite16((u16)value, instr->reg);
> +		break;
> +	case 3:
> +		iowrite32(value, instr->reg);
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	dev_dbg(&wdat->pdev->dev, "Wrote %#x to 0x%08llx\n", value,
> +		gas->address);
> +
> +	return 0;
> +}
> +
> +static int wdat_wdt_run_action(struct wdat_wdt *wdat, unsigned int action,
> +			       u32 param, u32 *retval)
> +{
> +	struct wdat_instruction *instr;
> +
> +	if (action >= ARRAY_SIZE(wdat->instructions))
> +		return -EINVAL;
> +
> +	if (!wdat->instructions[action])
> +		return -EOPNOTSUPP;
> +
> +	dev_dbg(&wdat->pdev->dev, "Running action %#x\n", action);
> +
> +	/* Run each instruction sequentially */
> +	list_for_each_entry(instr, wdat->instructions[action], node) {
> +		const struct acpi_wdat_entry *entry = &instr->entry;
> +		const struct acpi_generic_address *gas;
> +		u32 flags, value, mask, x, y;
> +		bool preserve;
> +		int ret;
> +
> +		gas = &entry->register_region;
> +
> +		preserve = entry->instruction & ACPI_WDAT_PRESERVE_REGISTER;
> +		flags = entry->instruction & ~ACPI_WDAT_PRESERVE_REGISTER;
> +		value = entry->value;
> +		mask = entry->mask;
> +
> +		switch (flags) {
> +		case ACPI_WDAT_READ_VALUE:
> +			ret = wdat_wdt_read(wdat, instr, &x);
> +			if (ret)
> +				return ret;
> +			x >>= gas->bit_offset;
> +			x &= mask;
> +			if (retval)
> +				*retval = x == value;
> +			break;
> +
> +		case ACPI_WDAT_READ_COUNTDOWN:
> +			ret = wdat_wdt_read(wdat, instr, &x);
> +			if (ret)
> +				return ret;
> +			x >>= gas->bit_offset;
> +			x &= mask;
> +			if (retval)
> +				*retval = x;
> +			break;
> +
> +		case ACPI_WDAT_WRITE_VALUE:
> +			x = value & mask;
> +			x <<= gas->bit_offset;
> +			if (preserve) {
> +				ret = wdat_wdt_read(wdat, instr, &y);
> +				if (ret)
> +					return ret;
> +				y = y & ~(mask << gas->bit_offset);
> +				x |= y;
> +			}
> +			ret = wdat_wdt_write(wdat, instr, x);
> +			if (ret)
> +				return ret;
> +			break;
> +
> +		case ACPI_WDAT_WRITE_COUNTDOWN:
> +			x = param;
> +			x &= mask;
> +			x <<= gas->bit_offset;
> +			if (preserve) {
> +				ret = wdat_wdt_read(wdat, instr, &y);
> +				if (ret)
> +					return ret;
> +				y = y & ~(mask << gas->bit_offset);
> +				x |= y;
> +			}
> +			ret = wdat_wdt_write(wdat, instr, x);
> +			if (ret)
> +				return ret;
> +			break;
> +
> +		default:
> +			dev_err(&wdat->pdev->dev, "Unknown instruction: %u\n",
> +				flags);
> +			return -EINVAL;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +static int wdat_wdt_enable_reboot(struct wdat_wdt *wdat)
> +{
> +	int ret;
> +
> +	/*
> +	 * WDAT specification says that the watchdog is required to reboot
> +	 * the system when it fires. However, it also states that it is
> +	 * recommeded to make it configurable through hardware register. We
> +	 * enable reboot now if it is configrable, just in case.
> +	 */
> +	ret = wdat_wdt_run_action(wdat, ACPI_WDAT_SET_REBOOT, 0, 0);
> +	if (ret && ret != -EOPNOTSUPP) {
> +		dev_err(&wdat->pdev->dev,
> +			"Failed to enable reboot when watchdog triggers\n");
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static void wdat_wdt_boot_status(struct wdat_wdt *wdat)
> +{
> +	u32 boot_status = 0;
> +	int ret;
> +
> +	ret = wdat_wdt_run_action(wdat, ACPI_WDAT_GET_STATUS, 0, &boot_status);
> +	if (ret && ret != -EOPNOTSUPP) {
> +		dev_err(&wdat->pdev->dev, "Failed to read boot status\n");
> +		return;
> +	}
> +
> +	if (boot_status)
> +		wdat->wdd.bootstatus = WDIOF_CARDRESET;
> +
> +	/* Clear the boot status in case BIOS did not do it */
> +	ret = wdat_wdt_run_action(wdat, ACPI_WDAT_SET_STATUS, 0, 0);
> +	if (ret && ret != -EOPNOTSUPP)
> +		dev_err(&wdat->pdev->dev, "Failed to clear boot status\n");
> +}
> +
> +static void wdat_wdt_set_running(struct wdat_wdt *wdat)
> +{
> +	u32 running = 0;
> +	int ret;
> +
> +	ret = wdat_wdt_run_action(wdat, ACPI_WDAT_GET_RUNNING_STATE, 0,
> +				  &running);
> +	if (ret && ret != -EOPNOTSUPP)
> +		dev_err(&wdat->pdev->dev, "Failed to read running state\n");
> +
> +	if (running)
> +		set_bit(WDOG_HW_RUNNING, &wdat->wdd.status);
> +}
> +
> +static int wdat_wdt_start(struct watchdog_device *wdd)
> +{
> +	return wdat_wdt_run_action(to_wdat_wdt(wdd),
> +				   ACPI_WDAT_SET_RUNNING_STATE, 0, NULL);
> +}
> +
> +static int wdat_wdt_stop(struct watchdog_device *wdd)
> +{
> +	return wdat_wdt_run_action(to_wdat_wdt(wdd),
> +				   ACPI_WDAT_SET_STOPPED_STATE, 0, NULL);
> +}
> +
> +static int wdat_wdt_ping(struct watchdog_device *wdd)
> +{
> +	return wdat_wdt_run_action(to_wdat_wdt(wdd), ACPI_WDAT_RESET, 0, NULL);
> +}
> +
> +static int wdat_wdt_set_timeout(struct watchdog_device *wdd,
> +				unsigned int timeout)
> +{
> +	struct wdat_wdt *wdat = to_wdat_wdt(wdd);
> +	unsigned int periods;
> +	int ret;
> +
> +	periods = timeout * 1000 / wdat->period;
> +	ret = wdat_wdt_run_action(wdat, ACPI_WDAT_SET_COUNTDOWN, periods, NULL);
> +	if (!ret)
> +		wdd->timeout = timeout;
> +	return ret;
> +}
> +
> +static unsigned int wdat_wdt_get_timeleft(struct watchdog_device *wdd)
> +{
> +	struct wdat_wdt *wdat = to_wdat_wdt(wdd);
> +	u32 periods = 0;
> +
> +	wdat_wdt_run_action(wdat, ACPI_WDAT_GET_COUNTDOWN, 0, &periods);
> +	return periods * wdat->period / 1000;
> +}
> +
> +static const struct watchdog_info wdat_wdt_info = {
> +	.options = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE,
> +	.firmware_version = 0,
> +	.identity = "wdat_wdt",
> +};
> +
> +static const struct watchdog_ops wdat_wdt_ops = {
> +	.owner = THIS_MODULE,
> +	.start = wdat_wdt_start,
> +	.stop = wdat_wdt_stop,
> +	.ping = wdat_wdt_ping,
> +	.set_timeout = wdat_wdt_set_timeout,
> +	.get_timeleft = wdat_wdt_get_timeleft,
> +};
> +
> +static int wdat_wdt_probe(struct platform_device *pdev)
> +{
> +	const struct acpi_wdat_entry *entries;
> +	const struct acpi_table_wdat *tbl;
> +	struct wdat_wdt *wdat;
> +	struct resource *res;
> +	void __iomem **regs;
> +	acpi_status status;
> +	int i, ret;
> +
> +	status = acpi_get_table(ACPI_SIG_WDAT, 0,
> +				(struct acpi_table_header **)&tbl);
> +	if (ACPI_FAILURE(status))
> +		return -ENODEV;
> +
> +	wdat = devm_kzalloc(&pdev->dev, sizeof(*wdat), GFP_KERNEL);
> +	if (!wdat)
> +		return -ENOMEM;
> +
> +	regs = devm_kcalloc(&pdev->dev, pdev->num_resources, sizeof(*regs),
> +			    GFP_KERNEL);
> +	if (!regs)
> +		return -ENOMEM;
> +
> +	/* WDAT specification wants to have >= 1ms period */
> +	if (tbl->timer_period < 1)
> +		return -EINVAL;
> +	if (tbl->min_count > tbl->max_count)
> +		return -EINVAL;
> +
> +	wdat->period = tbl->timer_period;
> +	wdat->wdd.min_hw_heartbeat_ms = wdat->period * tbl->min_count;
> +	wdat->wdd.max_hw_heartbeat_ms = wdat->period * tbl->max_count;
> +	wdat->stopped_in_sleep = tbl->flags & ACPI_WDAT_STOPPED;
> +	wdat->wdd.info = &wdat_wdt_info;
> +	wdat->wdd.ops = &wdat_wdt_ops;
> +	wdat->pdev = pdev;
> +
> +	/* Request and map all resources */
> +	for (i = 0; i < pdev->num_resources; i++) {
> +		void __iomem *reg;
> +
> +		res = &pdev->resource[i];
> +		if (resource_type(res) == IORESOURCE_MEM) {
> +			reg = devm_ioremap_resource(&pdev->dev, res);
> +		} else if (resource_type(res) == IORESOURCE_IO) {
> +			reg = devm_ioport_map(&pdev->dev, res->start, 1);
> +		} else {
> +			dev_err(&pdev->dev, "Unsupported resource\n");
> +			return -EINVAL;
> +		}
> +
> +		if (!reg)
> +			return -ENOMEM;
> +
> +		regs[i] = reg;
> +	}
> +
> +	entries = (struct acpi_wdat_entry *)(tbl + 1);
> +	for (i = 0; i < tbl->entries; i++) {
> +		const struct acpi_generic_address *gas;
> +		struct wdat_instruction *instr;
> +		struct list_head *instructions;
> +		unsigned int action;
> +		struct resource r;
> +		int j;
> +
> +		action = entries[i].action;
> +		if (action >= MAX_WDAT_ACTIONS) {
> +			dev_dbg(&pdev->dev, "Skipping unknown action: %u\n",
> +				action);
> +			continue;
> +		}
> +
> +		instr = devm_kzalloc(&pdev->dev, sizeof(*instr), GFP_KERNEL);
> +		if (!instr)
> +			return -ENOMEM;
> +
> +		INIT_LIST_HEAD(&instr->node);
> +		instr->entry = entries[i];
> +
> +		gas = &entries[i].register_region;
> +
> +		memset(&r, 0, sizeof(r));
> +		r.start = gas->address;
> +		r.end = r.start + gas->access_width;
> +		if (gas->space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) {
> +			r.flags = IORESOURCE_MEM;
> +		} else if (gas->space_id == ACPI_ADR_SPACE_SYSTEM_IO) {
> +			r.flags = IORESOURCE_IO;
> +		} else {
> +			dev_dbg(&pdev->dev, "Unsupported address space: %d\n",
> +				gas->space_id);
> +			continue;
> +		}
> +
> +		/* Find the matching resource */
> +		for (j = 0; j < pdev->num_resources; j++) {
> +			res = &pdev->resource[j];
> +			if (resource_contains(res, &r)) {
> +				instr->reg = regs[j] + r.start - res->start;
> +				break;
> +			}
> +		}
> +
> +		if (!instr->reg) {
> +			dev_err(&pdev->dev, "I/O resource not found\n");
> +			return -EINVAL;
> +		}
> +
I must be missing something here. What is happening with instr ?

> +		instructions = wdat->instructions[action];
> +		if (!instructions) {
> +			instructions = devm_kzalloc(&pdev->dev,
> +					sizeof(*instructions), GFP_KERNEL);
> +			if (!instructions)
> +				return -ENOMEM;
> +
> +			INIT_LIST_HEAD(instructions);
> +			wdat->instructions[action] = instructions;

... and I don't really see how wdat->instructions[action] contains anything but
a zero filled array. Confused :-(.

> +		}
> +
> +		list_add_tail(&instr->node, instructions);
> +	}
> +
> +	wdat_wdt_boot_status(wdat);
> +	wdat_wdt_set_running(wdat);
> +
> +	ret = wdat_wdt_enable_reboot(wdat);
> +	if (ret)
> +		return ret;
> +
> +	platform_set_drvdata(pdev, wdat);
> +
> +	watchdog_set_nowayout(&wdat->wdd, nowayout);
> +	return devm_watchdog_register_device(&pdev->dev, &wdat->wdd);
> +}
> +
> +#ifdef CONFIG_PM_SLEEP
> +static int wdat_wdt_suspend_noirq(struct device *dev)
> +{
> +	struct platform_device *pdev = to_platform_device(dev);
> +	struct wdat_wdt *wdat = platform_get_drvdata(pdev);
> +	int ret;
> +
> +	if (!watchdog_active(&wdat->wdd))
> +		return 0;
> +
> +	/*
> +	 * We need to stop the watchdog if firmare is not doing it or if we
> +	 * are going suspend to idle (where firmware is not involved). If
> +	 * firmware is stopping the watchdog we kick it here one more time
> +	 * to give it some time.
> +	 */
> +	wdat->stopped = false;
> +	if (acpi_target_system_state() == ACPI_STATE_S0 ||
> +	    !wdat->stopped_in_sleep) {
> +		ret = wdat_wdt_stop(&wdat->wdd);
> +		if (!ret)
> +			wdat->stopped = true;
> +	} else {
> +		ret = wdat_wdt_ping(&wdat->wdd);
> +	}
> +
> +	return ret;
> +}
> +
> +static int wdat_wdt_resume_noirq(struct device *dev)
> +{
> +	struct platform_device *pdev = to_platform_device(dev);
> +	struct wdat_wdt *wdat = platform_get_drvdata(pdev);
> +	int ret;
> +
> +	if (!watchdog_active(&wdat->wdd))
> +		return 0;
> +
> +	if (!wdat->stopped) {
> +		/*
> +		 * Looks like the boot firmware reinitializes the watchdog
> +		 * before it hands off to the OS on resume from sleep so we
> +		 * stop and reprogram the watchdog here.
> +		 */
> +		ret = wdat_wdt_stop(&wdat->wdd);
> +		if (ret)
> +			return ret;
> +
> +		ret = wdat_wdt_set_timeout(&wdat->wdd, wdat->wdd.timeout);
> +		if (ret)
> +			return ret;
> +
> +		ret = wdat_wdt_enable_reboot(wdat);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	return wdat_wdt_start(&wdat->wdd);
> +}
> +#endif
> +
> +static const struct dev_pm_ops wdat_wdt_pm_ops = {
> +	SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(wdat_wdt_suspend_noirq,
> +				      wdat_wdt_resume_noirq)
> +};
> +
> +static struct platform_driver wdat_wdt_driver = {
> +	.probe = wdat_wdt_probe,
> +	.driver = {
> +		.name = "wdat_wdt",
> +		.pm = &wdat_wdt_pm_ops,
> +	},
> +};
> +
> +module_platform_driver(wdat_wdt_driver);
> +
> +MODULE_AUTHOR("Mika Westerberg <mika.westerberg@linux.intel.com>");
> +MODULE_DESCRIPTION("ACPI Hardware Watchdog (WDAT) driver");
> +MODULE_LICENSE("GPL v2");
> +MODULE_ALIAS("platform:wdat_wdt");
> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
> index c5eaf2f80a4c..8ff6ca4a2639 100644
> --- a/include/linux/acpi.h
> +++ b/include/linux/acpi.h
> @@ -1074,4 +1074,10 @@ void acpi_table_upgrade(void);
>  static inline void acpi_table_upgrade(void) { }
>  #endif
>
> +#if defined(CONFIG_ACPI) && defined(CONFIG_ACPI_WATCHDOG)
> +extern bool acpi_has_watchdog(void);
> +#else
> +static inline bool acpi_has_watchdog(void) { return false; }
> +#endif
> +
>  #endif	/*_LINUX_ACPI_H*/
>

Re: [PATCH v2 1/4] ACPI / watchdog: Add support for WDAT hardware watchdog

[Mika Westerberg]

On Tue, Sep 20, 2016 at 06:37:23AM -0700, Guenter Roeck wrote:
> > +	for (i = 0; i < tbl->entries; i++) {
> > +		const struct acpi_generic_address *gas;
> > +		struct wdat_instruction *instr;
> > +		struct list_head *instructions;
> > +		unsigned int action;
> > +		struct resource r;
> > +		int j;
> > +
> > +		action = entries[i].action;
> > +		if (action >= MAX_WDAT_ACTIONS) {
> > +			dev_dbg(&pdev->dev, "Skipping unknown action: %u\n",
> > +				action);
> > +			continue;
> > +		}
> > +
> > +		instr = devm_kzalloc(&pdev->dev, sizeof(*instr), GFP_KERNEL);
> > +		if (!instr)
> > +			return -ENOMEM;
> > +
> > +		INIT_LIST_HEAD(&instr->node);
> > +		instr->entry = entries[i];
> > +
> > +		gas = &entries[i].register_region;
> > +
> > +		memset(&r, 0, sizeof(r));
> > +		r.start = gas->address;
> > +		r.end = r.start + gas->access_width;
> > +		if (gas->space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) {
> > +			r.flags = IORESOURCE_MEM;
> > +		} else if (gas->space_id == ACPI_ADR_SPACE_SYSTEM_IO) {
> > +			r.flags = IORESOURCE_IO;
> > +		} else {
> > +			dev_dbg(&pdev->dev, "Unsupported address space: %d\n",
> > +				gas->space_id);
> > +			continue;
> > +		}
> > +
> > +		/* Find the matching resource */
> > +		for (j = 0; j < pdev->num_resources; j++) {
> > +			res = &pdev->resource[j];
> > +			if (resource_contains(res, &r)) {
> > +				instr->reg = regs[j] + r.start - res->start;
> > +				break;
> > +			}
> > +		}
> > +
> > +		if (!instr->reg) {
> > +			dev_err(&pdev->dev, "I/O resource not found\n");
> > +			return -EINVAL;
> > +		}
> > +
> I must be missing something here. What is happening with instr ?

Instr contains the actual instruction from WDAT table. There can be
many.

> > +		instructions = wdat->instructions[action];

This will be NULL first time so...

> > +		if (!instructions) {
> > +			instructions = devm_kzalloc(&pdev->dev,
> > +					sizeof(*instructions), GFP_KERNEL);
> > +			if (!instructions)
> > +				return -ENOMEM;
> > +
> > +			INIT_LIST_HEAD(instructions);
> > +			wdat->instructions[action] = instructions;
> 
> ... and I don't really see how wdat->instructions[action] contains anything but
> a zero filled array. Confused :-(.

...we allocate entry for that here. Next time it will not be NULL for
that action.

> 
> > +		}
> > +
> > +		list_add_tail(&instr->node, instructions);

Then we append each instr to the list for this action.

Hope this clarifies :)

Re: [PATCH v2 1/4] ACPI / watchdog: Add support for WDAT hardware watchdog

[Guenter Roeck]

On 09/20/2016 06:50 AM, Mika Westerberg wrote:
> On Tue, Sep 20, 2016 at 06:37:23AM -0700, Guenter Roeck wrote:
>>> +	for (i = 0; i < tbl->entries; i++) {
>>> +		const struct acpi_generic_address *gas;
>>> +		struct wdat_instruction *instr;
>>> +		struct list_head *instructions;
>>> +		unsigned int action;
>>> +		struct resource r;
>>> +		int j;
>>> +
>>> +		action = entries[i].action;
>>> +		if (action >= MAX_WDAT_ACTIONS) {
>>> +			dev_dbg(&pdev->dev, "Skipping unknown action: %u\n",
>>> +				action);
>>> +			continue;
>>> +		}
>>> +
>>> +		instr = devm_kzalloc(&pdev->dev, sizeof(*instr), GFP_KERNEL);
>>> +		if (!instr)
>>> +			return -ENOMEM;
>>> +
>>> +		INIT_LIST_HEAD(&instr->node);
>>> +		instr->entry = entries[i];
>>> +
>>> +		gas = &entries[i].register_region;
>>> +
>>> +		memset(&r, 0, sizeof(r));
>>> +		r.start = gas->address;
>>> +		r.end = r.start + gas->access_width;
>>> +		if (gas->space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) {
>>> +			r.flags = IORESOURCE_MEM;
>>> +		} else if (gas->space_id == ACPI_ADR_SPACE_SYSTEM_IO) {
>>> +			r.flags = IORESOURCE_IO;
>>> +		} else {
>>> +			dev_dbg(&pdev->dev, "Unsupported address space: %d\n",
>>> +				gas->space_id);
>>> +			continue;
>>> +		}
>>> +
>>> +		/* Find the matching resource */
>>> +		for (j = 0; j < pdev->num_resources; j++) {
>>> +			res = &pdev->resource[j];
>>> +			if (resource_contains(res, &r)) {
>>> +				instr->reg = regs[j] + r.start - res->start;
>>> +				break;
>>> +			}
>>> +		}
>>> +
>>> +		if (!instr->reg) {
>>> +			dev_err(&pdev->dev, "I/O resource not found\n");
>>> +			return -EINVAL;
>>> +		}
>>> +
>> I must be missing something here. What is happening with instr ?
>
> Instr contains the actual instruction from WDAT table. There can be
> many.
>
>>> +		instructions = wdat->instructions[action];
>
> This will be NULL first time so...
>
>>> +		if (!instructions) {
>>> +			instructions = devm_kzalloc(&pdev->dev,
>>> +					sizeof(*instructions), GFP_KERNEL);
>>> +			if (!instructions)
>>> +				return -ENOMEM;
>>> +
>>> +			INIT_LIST_HEAD(instructions);
>>> +			wdat->instructions[action] = instructions;
>>
>> ... and I don't really see how wdat->instructions[action] contains anything but
>> a zero filled array. Confused :-(.
>
> ...we allocate entry for that here. Next time it will not be NULL for
> that action.
>
>>
>>> +		}
>>> +
>>> +		list_add_tail(&instr->node, instructions);
>
> Then we append each instr to the list for this action.
>
> Hope this clarifies :)
>
Yes, I somehow didn't see that last line. Too early in the morning,
not enough coffee.

Thanks,
Guenter

Re: [PATCH v2 1/4] ACPI / watchdog: Add support for WDAT hardware watchdog

[Guenter Roeck]

On 09/20/2016 05:30 AM, Mika Westerberg wrote:
> Starting from Intel Skylake the iTCO watchdog timer registers were moved to
> reside in the same register space with SMBus host controller.  Not all
> needed registers are available though and we need to unhide P2SB (Primary
> to Sideband) device briefly to be able to read status of required NO_REBOOT
> bit. The i2c-i801.c SMBus driver used to handle this and creation of the
> iTCO watchdog platform device.
>
> Windows, on the other hand, does not use the iTCO watchdog hardware
> directly even if it is available. Instead it relies on ACPI Watchdog Action
> Table (WDAT) table to describe the watchdog hardware to the OS. This table
> contains necessary information about the the hardware and also set of
> actions which are executed by a driver as needed.
>
> This patch implements a new watchdog driver that takes advantage of the
> ACPI WDAT table. We split the functionality into two parts: first part
> enumerates the WDAT table and if found, populates resources and creates
> platform device for the actual driver. The second part is the driver
> itself.
>
> The reason for the split is that this way we can make the driver itself to
> be a module and loaded automatically if the WDAT table is found. Otherwise
> the module is not loaded.
>
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
>  drivers/acpi/Kconfig         |   3 +
>  drivers/acpi/Makefile        |   1 +
>  drivers/acpi/acpi_watchdog.c | 123 ++++++++++
>  drivers/acpi/internal.h      |  10 +
>  drivers/acpi/scan.c          |   1 +
>  drivers/watchdog/Kconfig     |  13 ++
>  drivers/watchdog/Makefile    |   1 +
>  drivers/watchdog/wdat_wdt.c  | 525 +++++++++++++++++++++++++++++++++++++++++++
>  include/linux/acpi.h         |   6 +
>  9 files changed, 683 insertions(+)
>  create mode 100644 drivers/acpi/acpi_watchdog.c
>  create mode 100644 drivers/watchdog/wdat_wdt.c
>
> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
> index 445ce28475b3..dcfa7e9e92f5 100644
> --- a/drivers/acpi/Kconfig
> +++ b/drivers/acpi/Kconfig
> @@ -462,6 +462,9 @@ source "drivers/acpi/nfit/Kconfig"
>  source "drivers/acpi/apei/Kconfig"
>  source "drivers/acpi/dptf/Kconfig"
>
> +config ACPI_WATCHDOG
> +	bool
> +
>  config ACPI_EXTLOG
>  	tristate "Extended Error Log support"
>  	depends on X86_MCE && X86_LOCAL_APIC
> diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
> index 5ae9d85c5159..3a1fa8f03749 100644
> --- a/drivers/acpi/Makefile
> +++ b/drivers/acpi/Makefile
> @@ -56,6 +56,7 @@ acpi-$(CONFIG_ACPI_NUMA)	+= numa.o
>  acpi-$(CONFIG_ACPI_PROCFS_POWER) += cm_sbs.o
>  acpi-y				+= acpi_lpat.o
>  acpi-$(CONFIG_ACPI_GENERIC_GSI) += gsi.o
> +acpi-$(CONFIG_ACPI_WATCHDOG)	+= acpi_watchdog.o
>
>  # These are (potentially) separate modules
>
> diff --git a/drivers/acpi/acpi_watchdog.c b/drivers/acpi/acpi_watchdog.c
> new file mode 100644
> index 000000000000..13caebd679f5
> --- /dev/null
> +++ b/drivers/acpi/acpi_watchdog.c
> @@ -0,0 +1,123 @@
> +/*
> + * ACPI watchdog table parsing support.
> + *
> + * Copyright (C) 2016, Intel Corporation
> + * Author: Mika Westerberg <mika.westerberg@linux.intel.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#define pr_fmt(fmt) "ACPI: watchdog: " fmt
> +
> +#include <linux/acpi.h>
> +#include <linux/ioport.h>
> +#include <linux/platform_device.h>
> +
> +#include "internal.h"
> +
> +/**
> + * Returns true if this system should prefer ACPI based watchdog instead of
> + * the native one (which are typically the same hardware).
> + */
> +bool acpi_has_watchdog(void)
> +{
> +	struct acpi_table_header hdr;
> +
> +	if (acpi_disabled)
> +		return false;
> +
> +	return ACPI_SUCCESS(acpi_get_table_header(ACPI_SIG_WDAT, 0, &hdr));
> +}
> +EXPORT_SYMBOL_GPL(acpi_has_watchdog);
> +
> +void __init acpi_watchdog_init(void)
> +{
> +	const struct acpi_wdat_entry *entries;
> +	const struct acpi_table_wdat *wdat;
> +	struct list_head resource_list;
> +	struct resource_entry *rentry;
> +	struct platform_device *pdev;
> +	struct resource *resources;
> +	size_t nresources = 0;
> +	acpi_status status;
> +	int i;
> +
> +	status = acpi_get_table(ACPI_SIG_WDAT, 0,
> +				(struct acpi_table_header **)&wdat);
> +	if (ACPI_FAILURE(status)) {
> +		/* It is fine if there is no WDAT */
> +		return;
> +	}
> +
> +	/* Watchdog disabled by BIOS */
> +	if (!(wdat->flags & ACPI_WDAT_ENABLED))
> +		return;
> +
> +	/* Skip legacy PCI WDT devices */
> +	if (wdat->pci_segment != 0xff || wdat->pci_bus != 0xff ||
> +	    wdat->pci_device != 0xff || wdat->pci_function != 0xff)
> +		return;
> +
> +	INIT_LIST_HEAD(&resource_list);
> +
> +	entries = (struct acpi_wdat_entry *)(wdat + 1);
> +	for (i = 0; i < wdat->entries; i++) {
> +		const struct acpi_generic_address *gas;
> +		struct resource_entry *rentry;
> +		struct resource res;
> +		bool found;
> +
> +		gas = &entries[i].register_region;
> +
> +		res.start = gas->address;
> +		if (gas->space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) {
> +			res.flags = IORESOURCE_MEM;
> +			res.end = res.start + ALIGN(gas->access_width, 4);
> +		} else if (gas->space_id == ACPI_ADR_SPACE_SYSTEM_IO) {
> +			res.flags = IORESOURCE_IO;
> +			res.end = res.start + gas->access_width;
> +		} else {
> +			pr_warn("Unsupported address space: %u\n",
> +				gas->space_id);
> +			goto fail_free_resource_list;
> +		}
> +
> +		found = false;
> +		resource_list_for_each_entry(rentry, &resource_list) {
> +			if (resource_contains(rentry->res, &res)) {
> +				found = true;
> +				break;
> +			}
> +		}
> +
> +		if (!found) {
> +			rentry = resource_list_create_entry(NULL, 0);
> +			if (!rentry)
> +				goto fail_free_resource_list;
> +
> +			*rentry->res = res;
> +			resource_list_add_tail(rentry, &resource_list);
> +			nresources++;
> +		}
> +	}
> +
> +	resources = kcalloc(nresources, sizeof(*resources), GFP_KERNEL);
> +	if (!resources)
> +		goto fail_free_resource_list;
> +
> +	i = 0;
> +	resource_list_for_each_entry(rentry, &resource_list)
> +		resources[i++] = *rentry->res;
> +
> +	pdev = platform_device_register_simple("wdat_wdt", PLATFORM_DEVID_NONE,
> +					       resources, nresources);
> +	if (IS_ERR(pdev))
> +		pr_err("Failed to create platform device\n");
> +
> +	kfree(resources);
> +
> +fail_free_resource_list:
> +	resource_list_free(&resource_list);
> +}
> diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h
> index 940218ff0193..fb9a7ad96756 100644
> --- a/drivers/acpi/internal.h
> +++ b/drivers/acpi/internal.h
> @@ -225,4 +225,14 @@ static inline void suspend_nvs_restore(void) {}
>  void acpi_init_properties(struct acpi_device *adev);
>  void acpi_free_properties(struct acpi_device *adev);
>
> +/*--------------------------------------------------------------------------
> +				Watchdog
> +  -------------------------------------------------------------------------- */
> +
> +#ifdef CONFIG_ACPI_WATCHDOG
> +void acpi_watchdog_init(void);
> +#else
> +static inline void acpi_watchdog_init(void) {}
> +#endif
> +
>  #endif /* _ACPI_INTERNAL_H_ */
> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> index e878fc799af7..3b85d87021da 100644
> --- a/drivers/acpi/scan.c
> +++ b/drivers/acpi/scan.c
> @@ -2002,6 +2002,7 @@ int __init acpi_scan_init(void)
>  	acpi_pnp_init();
>  	acpi_int340x_thermal_init();
>  	acpi_amba_init();
> +	acpi_watchdog_init();
>
>  	acpi_scan_add_handler(&generic_device_handler);
>
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index 1bffe006ca9a..50dbaa805658 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -152,6 +152,19 @@ config TANGOX_WATCHDOG
>
>  	  This driver can be built as a module. The module name is tangox_wdt.
>
> +config WDAT_WDT
> +	tristate "ACPI Watchdog Action Table (WDAT)"
> +	depends on ACPI
> +	select ACPI_WATCHDOG
> +	help
> +	  This driver adds support for systems with ACPI Watchdog Action
> +	  Table (WDAT) table. Servers typically have this but it can be
> +	  found on some desktop machines as well. This driver will take
> +	  over the native iTCO watchdog driver found on many Intel CPUs.
> +
> +	  To compile this driver as module, choose M here: the module will
> +	  be called wdat_wdt.
> +
>  config WM831X_WATCHDOG
>  	tristate "WM831x watchdog"
>  	depends on MFD_WM831X
> diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
> index c22ad3ea3539..cba00430151b 100644
> --- a/drivers/watchdog/Makefile
> +++ b/drivers/watchdog/Makefile
> @@ -202,6 +202,7 @@ obj-$(CONFIG_DA9062_WATCHDOG) += da9062_wdt.o
>  obj-$(CONFIG_DA9063_WATCHDOG) += da9063_wdt.o
>  obj-$(CONFIG_GPIO_WATCHDOG)	+= gpio_wdt.o
>  obj-$(CONFIG_TANGOX_WATCHDOG) += tangox_wdt.o
> +obj-$(CONFIG_WDAT_WDT) += wdat_wdt.o
>  obj-$(CONFIG_WM831X_WATCHDOG) += wm831x_wdt.o
>  obj-$(CONFIG_WM8350_WATCHDOG) += wm8350_wdt.o
>  obj-$(CONFIG_MAX63XX_WATCHDOG) += max63xx_wdt.o
> diff --git a/drivers/watchdog/wdat_wdt.c b/drivers/watchdog/wdat_wdt.c
> new file mode 100644
> index 000000000000..6b6464596674
> --- /dev/null
> +++ b/drivers/watchdog/wdat_wdt.c
> @@ -0,0 +1,525 @@
> +/*
> + * ACPI Hardware Watchdog (WDAT) driver.
> + *
> + * Copyright (C) 2016, Intel Corporation
> + * Author: Mika Westerberg <mika.westerberg@linux.intel.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/acpi.h>
> +#include <linux/ioport.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm.h>
> +#include <linux/watchdog.h>
> +
> +#define MAX_WDAT_ACTIONS ACPI_WDAT_ACTION_RESERVED
> +
> +/**
> + * struct wdat_instruction - Single ACPI WDAT instruction
> + * @entry: Copy of the ACPI table instruction
> + * @reg: Register the instruction is accessing
> + * @node: Next instruction in action sequence
> + */
> +struct wdat_instruction {
> +	struct acpi_wdat_entry entry;
> +	void __iomem *reg;
> +	struct list_head node;
> +};
> +
> +/**
> + * struct wdat_wdt - ACPI WDAT watchdog device
> + * @pdev: Parent platform device
> + * @wdd: Watchdog core device
> + * @period: How long is one watchdog period in ms
> + * @stopped_in_sleep: Is this watchdog stopped by the firmware in S1-S5
> + * @stopped: Was the watchdog stopped by the driver in suspend
> + * @actions: An array of instruction lists indexed by an action number from
> + *           the WDAT table. There can be %NULL entries for not implemented
> + *           actions.
> + */
> +struct wdat_wdt {
> +	struct platform_device *pdev;
> +	struct watchdog_device wdd;
> +	unsigned int period;
> +	bool stopped_in_sleep;
> +	bool stopped;
> +	struct list_head *instructions[MAX_WDAT_ACTIONS];
> +};
> +
> +#define to_wdat_wdt(wdd) container_of(wdd, struct wdat_wdt, wdd)
> +
> +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 wdat_wdt_read(struct wdat_wdt *wdat,
> +	 const struct wdat_instruction *instr, u32 *value)
> +{
> +	const struct acpi_generic_address *gas = &instr->entry.register_region;
> +
> +	switch (gas->access_width) {
> +	case 1:
> +		*value = ioread8(instr->reg);
> +		break;
> +	case 2:
> +		*value = ioread16(instr->reg);
> +		break;
> +	case 3:
> +		*value = ioread32(instr->reg);
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	dev_dbg(&wdat->pdev->dev, "Read %#x from 0x%08llx\n", *value,
> +		gas->address);
> +
> +	return 0;
> +}
> +
> +static int wdat_wdt_write(struct wdat_wdt *wdat,
> +	const struct wdat_instruction *instr, u32 value)
> +{
> +	const struct acpi_generic_address *gas = &instr->entry.register_region;
> +
> +	switch (gas->access_width) {
> +	case 1:
> +		iowrite8((u8)value, instr->reg);
> +		break;
> +	case 2:
> +		iowrite16((u16)value, instr->reg);
> +		break;
> +	case 3:
> +		iowrite32(value, instr->reg);
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	dev_dbg(&wdat->pdev->dev, "Wrote %#x to 0x%08llx\n", value,
> +		gas->address);
> +
> +	return 0;
> +}
> +
> +static int wdat_wdt_run_action(struct wdat_wdt *wdat, unsigned int action,
> +			       u32 param, u32 *retval)
> +{
> +	struct wdat_instruction *instr;
> +
> +	if (action >= ARRAY_SIZE(wdat->instructions))
> +		return -EINVAL;
> +
> +	if (!wdat->instructions[action])
> +		return -EOPNOTSUPP;
> +
> +	dev_dbg(&wdat->pdev->dev, "Running action %#x\n", action);
> +
> +	/* Run each instruction sequentially */
> +	list_for_each_entry(instr, wdat->instructions[action], node) {
> +		const struct acpi_wdat_entry *entry = &instr->entry;
> +		const struct acpi_generic_address *gas;
> +		u32 flags, value, mask, x, y;
> +		bool preserve;
> +		int ret;
> +
> +		gas = &entry->register_region;
> +
> +		preserve = entry->instruction & ACPI_WDAT_PRESERVE_REGISTER;
> +		flags = entry->instruction & ~ACPI_WDAT_PRESERVE_REGISTER;
> +		value = entry->value;
> +		mask = entry->mask;
> +
> +		switch (flags) {
> +		case ACPI_WDAT_READ_VALUE:
> +			ret = wdat_wdt_read(wdat, instr, &x);
> +			if (ret)
> +				return ret;
> +			x >>= gas->bit_offset;
> +			x &= mask;
> +			if (retval)
> +				*retval = x == value;
> +			break;
> +
> +		case ACPI_WDAT_READ_COUNTDOWN:
> +			ret = wdat_wdt_read(wdat, instr, &x);
> +			if (ret)
> +				return ret;
> +			x >>= gas->bit_offset;
> +			x &= mask;
> +			if (retval)
> +				*retval = x;
> +			break;
> +
> +		case ACPI_WDAT_WRITE_VALUE:
> +			x = value & mask;
> +			x <<= gas->bit_offset;
> +			if (preserve) {
> +				ret = wdat_wdt_read(wdat, instr, &y);
> +				if (ret)
> +					return ret;
> +				y = y & ~(mask << gas->bit_offset);
> +				x |= y;
> +			}
> +			ret = wdat_wdt_write(wdat, instr, x);
> +			if (ret)
> +				return ret;
> +			break;
> +
> +		case ACPI_WDAT_WRITE_COUNTDOWN:
> +			x = param;
> +			x &= mask;
> +			x <<= gas->bit_offset;
> +			if (preserve) {
> +				ret = wdat_wdt_read(wdat, instr, &y);
> +				if (ret)
> +					return ret;
> +				y = y & ~(mask << gas->bit_offset);
> +				x |= y;
> +			}
> +			ret = wdat_wdt_write(wdat, instr, x);
> +			if (ret)
> +				return ret;
> +			break;
> +
> +		default:
> +			dev_err(&wdat->pdev->dev, "Unknown instruction: %u\n",
> +				flags);
> +			return -EINVAL;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +static int wdat_wdt_enable_reboot(struct wdat_wdt *wdat)
> +{
> +	int ret;
> +
> +	/*
> +	 * WDAT specification says that the watchdog is required to reboot
> +	 * the system when it fires. However, it also states that it is
> +	 * recommeded to make it configurable through hardware register. We
> +	 * enable reboot now if it is configrable, just in case.
> +	 */
> +	ret = wdat_wdt_run_action(wdat, ACPI_WDAT_SET_REBOOT, 0, 0);
> +	if (ret && ret != -EOPNOTSUPP) {
> +		dev_err(&wdat->pdev->dev,
> +			"Failed to enable reboot when watchdog triggers\n");
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static void wdat_wdt_boot_status(struct wdat_wdt *wdat)
> +{
> +	u32 boot_status = 0;
> +	int ret;
> +
> +	ret = wdat_wdt_run_action(wdat, ACPI_WDAT_GET_STATUS, 0, &boot_status);
> +	if (ret && ret != -EOPNOTSUPP) {
> +		dev_err(&wdat->pdev->dev, "Failed to read boot status\n");
> +		return;
> +	}
> +
> +	if (boot_status)
> +		wdat->wdd.bootstatus = WDIOF_CARDRESET;
> +
> +	/* Clear the boot status in case BIOS did not do it */
> +	ret = wdat_wdt_run_action(wdat, ACPI_WDAT_SET_STATUS, 0, 0);
> +	if (ret && ret != -EOPNOTSUPP)
> +		dev_err(&wdat->pdev->dev, "Failed to clear boot status\n");
> +}
> +
> +static void wdat_wdt_set_running(struct wdat_wdt *wdat)
> +{
> +	u32 running = 0;
> +	int ret;
> +
> +	ret = wdat_wdt_run_action(wdat, ACPI_WDAT_GET_RUNNING_STATE, 0,
> +				  &running);
> +	if (ret && ret != -EOPNOTSUPP)
> +		dev_err(&wdat->pdev->dev, "Failed to read running state\n");
> +
> +	if (running)
> +		set_bit(WDOG_HW_RUNNING, &wdat->wdd.status);
> +}
> +
> +static int wdat_wdt_start(struct watchdog_device *wdd)
> +{
> +	return wdat_wdt_run_action(to_wdat_wdt(wdd),
> +				   ACPI_WDAT_SET_RUNNING_STATE, 0, NULL);
> +}
> +
> +static int wdat_wdt_stop(struct watchdog_device *wdd)
> +{
> +	return wdat_wdt_run_action(to_wdat_wdt(wdd),
> +				   ACPI_WDAT_SET_STOPPED_STATE, 0, NULL);
> +}
> +
> +static int wdat_wdt_ping(struct watchdog_device *wdd)
> +{
> +	return wdat_wdt_run_action(to_wdat_wdt(wdd), ACPI_WDAT_RESET, 0, NULL);
> +}
> +
> +static int wdat_wdt_set_timeout(struct watchdog_device *wdd,
> +				unsigned int timeout)
> +{
> +	struct wdat_wdt *wdat = to_wdat_wdt(wdd);
> +	unsigned int periods;
> +	int ret;
> +
> +	periods = timeout * 1000 / wdat->period;
> +	ret = wdat_wdt_run_action(wdat, ACPI_WDAT_SET_COUNTDOWN, periods, NULL);
> +	if (!ret)
> +		wdd->timeout = timeout;
> +	return ret;
> +}
> +
> +static unsigned int wdat_wdt_get_timeleft(struct watchdog_device *wdd)
> +{
> +	struct wdat_wdt *wdat = to_wdat_wdt(wdd);
> +	u32 periods = 0;
> +
> +	wdat_wdt_run_action(wdat, ACPI_WDAT_GET_COUNTDOWN, 0, &periods);
> +	return periods * wdat->period / 1000;
> +}
> +
> +static const struct watchdog_info wdat_wdt_info = {
> +	.options = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE,
> +	.firmware_version = 0,
> +	.identity = "wdat_wdt",
> +};
> +
> +static const struct watchdog_ops wdat_wdt_ops = {
> +	.owner = THIS_MODULE,
> +	.start = wdat_wdt_start,
> +	.stop = wdat_wdt_stop,
> +	.ping = wdat_wdt_ping,
> +	.set_timeout = wdat_wdt_set_timeout,
> +	.get_timeleft = wdat_wdt_get_timeleft,
> +};
> +
> +static int wdat_wdt_probe(struct platform_device *pdev)
> +{
> +	const struct acpi_wdat_entry *entries;
> +	const struct acpi_table_wdat *tbl;
> +	struct wdat_wdt *wdat;
> +	struct resource *res;
> +	void __iomem **regs;
> +	acpi_status status;
> +	int i, ret;
> +
> +	status = acpi_get_table(ACPI_SIG_WDAT, 0,
> +				(struct acpi_table_header **)&tbl);
> +	if (ACPI_FAILURE(status))
> +		return -ENODEV;
> +
> +	wdat = devm_kzalloc(&pdev->dev, sizeof(*wdat), GFP_KERNEL);
> +	if (!wdat)
> +		return -ENOMEM;
> +
> +	regs = devm_kcalloc(&pdev->dev, pdev->num_resources, sizeof(*regs),
> +			    GFP_KERNEL);
> +	if (!regs)
> +		return -ENOMEM;
> +
> +	/* WDAT specification wants to have >= 1ms period */
> +	if (tbl->timer_period < 1)
> +		return -EINVAL;
> +	if (tbl->min_count > tbl->max_count)
> +		return -EINVAL;
> +
> +	wdat->period = tbl->timer_period;
> +	wdat->wdd.min_hw_heartbeat_ms = wdat->period * tbl->min_count;
> +	wdat->wdd.max_hw_heartbeat_ms = wdat->period * tbl->max_count;
> +	wdat->stopped_in_sleep = tbl->flags & ACPI_WDAT_STOPPED;
> +	wdat->wdd.info = &wdat_wdt_info;
> +	wdat->wdd.ops = &wdat_wdt_ops;
> +	wdat->pdev = pdev;
> +
> +	/* Request and map all resources */
> +	for (i = 0; i < pdev->num_resources; i++) {
> +		void __iomem *reg;
> +
> +		res = &pdev->resource[i];
> +		if (resource_type(res) == IORESOURCE_MEM) {
> +			reg = devm_ioremap_resource(&pdev->dev, res);
> +		} else if (resource_type(res) == IORESOURCE_IO) {
> +			reg = devm_ioport_map(&pdev->dev, res->start, 1);
> +		} else {
> +			dev_err(&pdev->dev, "Unsupported resource\n");
> +			return -EINVAL;
> +		}
> +
> +		if (!reg)
> +			return -ENOMEM;
> +
> +		regs[i] = reg;
> +	}
> +
> +	entries = (struct acpi_wdat_entry *)(tbl + 1);
> +	for (i = 0; i < tbl->entries; i++) {
> +		const struct acpi_generic_address *gas;
> +		struct wdat_instruction *instr;
> +		struct list_head *instructions;
> +		unsigned int action;
> +		struct resource r;
> +		int j;
> +
> +		action = entries[i].action;
> +		if (action >= MAX_WDAT_ACTIONS) {
> +			dev_dbg(&pdev->dev, "Skipping unknown action: %u\n",
> +				action);
> +			continue;
> +		}
> +
> +		instr = devm_kzalloc(&pdev->dev, sizeof(*instr), GFP_KERNEL);
> +		if (!instr)
> +			return -ENOMEM;
> +
> +		INIT_LIST_HEAD(&instr->node);
> +		instr->entry = entries[i];
> +
> +		gas = &entries[i].register_region;
> +
> +		memset(&r, 0, sizeof(r));
> +		r.start = gas->address;
> +		r.end = r.start + gas->access_width;
> +		if (gas->space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) {
> +			r.flags = IORESOURCE_MEM;
> +		} else if (gas->space_id == ACPI_ADR_SPACE_SYSTEM_IO) {
> +			r.flags = IORESOURCE_IO;
> +		} else {
> +			dev_dbg(&pdev->dev, "Unsupported address space: %d\n",
> +				gas->space_id);
> +			continue;
> +		}
> +
> +		/* Find the matching resource */
> +		for (j = 0; j < pdev->num_resources; j++) {
> +			res = &pdev->resource[j];
> +			if (resource_contains(res, &r)) {
> +				instr->reg = regs[j] + r.start - res->start;
> +				break;
> +			}
> +		}
> +
> +		if (!instr->reg) {
> +			dev_err(&pdev->dev, "I/O resource not found\n");
> +			return -EINVAL;
> +		}
> +
> +		instructions = wdat->instructions[action];
> +		if (!instructions) {
> +			instructions = devm_kzalloc(&pdev->dev,
> +					sizeof(*instructions), GFP_KERNEL);
> +			if (!instructions)
> +				return -ENOMEM;
> +
> +			INIT_LIST_HEAD(instructions);
> +			wdat->instructions[action] = instructions;
> +		}
> +
> +		list_add_tail(&instr->node, instructions);
> +	}
> +
> +	wdat_wdt_boot_status(wdat);
> +	wdat_wdt_set_running(wdat);
> +
> +	ret = wdat_wdt_enable_reboot(wdat);
> +	if (ret)
> +		return ret;
> +
> +	platform_set_drvdata(pdev, wdat);
> +
> +	watchdog_set_nowayout(&wdat->wdd, nowayout);
> +	return devm_watchdog_register_device(&pdev->dev, &wdat->wdd);
> +}
> +
> +#ifdef CONFIG_PM_SLEEP
> +static int wdat_wdt_suspend_noirq(struct device *dev)
> +{
> +	struct platform_device *pdev = to_platform_device(dev);
> +	struct wdat_wdt *wdat = platform_get_drvdata(pdev);
> +	int ret;
> +
> +	if (!watchdog_active(&wdat->wdd))
> +		return 0;
> +
> +	/*
> +	 * We need to stop the watchdog if firmare is not doing it or if we
> +	 * are going suspend to idle (where firmware is not involved). If
> +	 * firmware is stopping the watchdog we kick it here one more time
> +	 * to give it some time.
> +	 */
> +	wdat->stopped = false;
> +	if (acpi_target_system_state() == ACPI_STATE_S0 ||
> +	    !wdat->stopped_in_sleep) {
> +		ret = wdat_wdt_stop(&wdat->wdd);
> +		if (!ret)
> +			wdat->stopped = true;
> +	} else {
> +		ret = wdat_wdt_ping(&wdat->wdd);
> +	}
> +
> +	return ret;
> +}
> +
> +static int wdat_wdt_resume_noirq(struct device *dev)
> +{
> +	struct platform_device *pdev = to_platform_device(dev);
> +	struct wdat_wdt *wdat = platform_get_drvdata(pdev);
> +	int ret;
> +
> +	if (!watchdog_active(&wdat->wdd))
> +		return 0;
> +
> +	if (!wdat->stopped) {
> +		/*
> +		 * Looks like the boot firmware reinitializes the watchdog
> +		 * before it hands off to the OS on resume from sleep so we
> +		 * stop and reprogram the watchdog here.
> +		 */
> +		ret = wdat_wdt_stop(&wdat->wdd);
> +		if (ret)
> +			return ret;
> +
> +		ret = wdat_wdt_set_timeout(&wdat->wdd, wdat->wdd.timeout);
> +		if (ret)
> +			return ret;
> +
> +		ret = wdat_wdt_enable_reboot(wdat);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	return wdat_wdt_start(&wdat->wdd);
> +}
> +#endif
> +
> +static const struct dev_pm_ops wdat_wdt_pm_ops = {
> +	SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(wdat_wdt_suspend_noirq,
> +				      wdat_wdt_resume_noirq)
> +};
> +
> +static struct platform_driver wdat_wdt_driver = {
> +	.probe = wdat_wdt_probe,
> +	.driver = {
> +		.name = "wdat_wdt",
> +		.pm = &wdat_wdt_pm_ops,
> +	},
> +};
> +
> +module_platform_driver(wdat_wdt_driver);
> +
> +MODULE_AUTHOR("Mika Westerberg <mika.westerberg@linux.intel.com>");
> +MODULE_DESCRIPTION("ACPI Hardware Watchdog (WDAT) driver");
> +MODULE_LICENSE("GPL v2");
> +MODULE_ALIAS("platform:wdat_wdt");
> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
> index c5eaf2f80a4c..8ff6ca4a2639 100644
> --- a/include/linux/acpi.h
> +++ b/include/linux/acpi.h
> @@ -1074,4 +1074,10 @@ void acpi_table_upgrade(void);
>  static inline void acpi_table_upgrade(void) { }
>  #endif
>
> +#if defined(CONFIG_ACPI) && defined(CONFIG_ACPI_WATCHDOG)
> +extern bool acpi_has_watchdog(void);
> +#else
> +static inline bool acpi_has_watchdog(void) { return false; }
> +#endif
> +
>  #endif	/*_LINUX_ACPI_H*/
>

Re: [PATCH v2 3/4] i2c: i801: Do not create iTCO watchdog when WDAT table exists

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 560 bytes --]

On Tue, Sep 20, 2016 at 03:30:53PM +0300, Mika Westerberg wrote:
> ACPI WDAT table is the preferred way to use hardware watchdog over the
> native iTCO_wdt. Windows only uses this table for its hardware watchdog
> implementation so we should be relatively safe to trust it has been
> validated by OEMs
> 
> Prevent iTCO watchdog creation if we detect that there is ACPI WDAT table.
> 
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> Reviewed-by: Guenter Roeck <linux@roeck-us.net>

Acked-by: Wolfram Sang <wsa@the-dreams.de>


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v2 3/4] i2c: i801: Do not create iTCO watchdog when WDAT table exists

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 611 bytes --]

On Tue, Sep 20, 2016 at 03:30:53PM +0300, Mika Westerberg wrote:
> ACPI WDAT table is the preferred way to use hardware watchdog over the
> native iTCO_wdt. Windows only uses this table for its hardware watchdog
> implementation so we should be relatively safe to trust it has been
> validated by OEMs
> 
> Prevent iTCO watchdog creation if we detect that there is ACPI WDAT table.
> 
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> Reviewed-by: Guenter Roeck <linux@roeck-us.net>

BTW I assume this patch does not go via the i2c tree. Please tell me if
you think otherwise.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v2 0/4] ACPI / watchdog: Add support for WDAT (Watchdog Action Table)

[Rafael J. Wysocki]

On Tuesday, September 20, 2016 03:30:50 PM Mika Westerberg wrote:
> Hi,
> 
> The WDAT (Watchdog Action Table) is a special ACPI table introduced by
> Microsoft [1] that abstracts the watchdog hardware from the OS. Windows
> uses this table for its watchdog implementation instead of a native iTCO
> driver.
> 
> Microsoft re-licensed the WDAT specification to be under Microsoft
> Community Promise license [2] so it should be fine to use it in Linux.
> 
> This series brings WDAT table support to Linux.
> 
> When the driver is enabled and we find out that there is a WDAT table, the
> driver will take over the native iTCO watchdog driver. Main advantage in
> this is that we do not need to change the native iTCO driver whenever the
> hardware changes. For example in Skylake iTCO moved to sit behind SMBus and
> the NO_REBOOT bit was hidden behind P2SB (Primary to Sideband). In addition
> we can expect this to be tested much better by OEMs who typically validate
> that Windows works fine on their hardware/firmware.
> 
> Patch [1/4] adds ACPI enumeration support and the driver itself. It also
> introduces acpi_has_watchdog() which can be used to check if we should use
> ACPI watchdog or native one.
> 
> Patches [2-4/4] prevent creation of the native iTCO platform device if we
> detect that the ACPI watchdog (WDAT) should be used instead.
> 
> The previous version of the series can be found in [3].
> 
> Changes from v1:
>   * Moved wdat_wdt.c to live under drivers/watchdog
>   * Added checks for timer_period, min_count and max_count
>   * Use min_hw_heartbeat_ms and max_hw_heartbeat_ms instead of
>     min/max_timeout
>   * Instead of stopping the watchdog set WDOG_HW_RUNNING
>   * Switched to use devm_watchdog_register_device() and dropped
>     wdat_wdt_remove()
>   * Do not ping watchdog in resume()
>   * Added review tag from Guenter Roeck to patches [2-4/4].
> 
> [1] http://msdn.microsoft.com/en-us/windows/hardware/gg463320.aspx
> [2] https://msdn.microsoft.com/en-us/openspecifications/dn646766.aspx
> [3] http://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1230607.html
> 
> Mika Westerberg (4):
>   ACPI / watchdog: Add support for WDAT hardware watchdog
>   mfd: lpc_ich: Do not create iTCO watchdog when WDAT table exists
>   i2c: i801: Do not create iTCO watchdog when WDAT table exists
>   platform/x86: intel_pmc_ipc: Do not create iTCO watchdog when WDAT
>     table exists
> 
>  drivers/acpi/Kconfig                 |   3 +
>  drivers/acpi/Makefile                |   1 +
>  drivers/acpi/acpi_watchdog.c         | 123 ++++++++
>  drivers/acpi/internal.h              |  10 +
>  drivers/acpi/scan.c                  |   1 +
>  drivers/i2c/busses/i2c-i801.c        |   4 +-
>  drivers/mfd/lpc_ich.c                |   4 +
>  drivers/platform/x86/intel_pmc_ipc.c |  12 +-
>  drivers/watchdog/Kconfig             |  13 +
>  drivers/watchdog/Makefile            |   1 +
>  drivers/watchdog/wdat_wdt.c          | 525 +++++++++++++++++++++++++++++++++++
>  include/linux/acpi.h                 |   6 +
>  12 files changed, 698 insertions(+), 5 deletions(-)
>  create mode 100644 drivers/acpi/acpi_watchdog.c
>  create mode 100644 drivers/watchdog/wdat_wdt.c

I'm queing up this series for 4.9.

Please let me know if there are any objections.

Thanks,
Rafael

Re: [PATCH v2 2/4] mfd: lpc_ich: Do not create iTCO watchdog when WDAT table exists

[Lee Jones]

On Tue, 20 Sep 2016, Mika Westerberg wrote:

> ACPI WDAT table is the preferred way to use hardware watchdog over the
> native iTCO_wdt. Windows only uses this table for its hardware watchdog
> implementation so we should be relatively safe to trust it has been
> validated by OEMs
> 
> Prevent iTCO watchdog creation if we detect that there is ACPI WDAT table.
> 
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> Reviewed-by: Guenter Roeck <linux@roeck-us.net>
> ---
>  drivers/mfd/lpc_ich.c | 4 ++++
>  1 file changed, 4 insertions(+)

Applied, thanks.

> diff --git a/drivers/mfd/lpc_ich.c b/drivers/mfd/lpc_ich.c
> index bd3aa4578346..c8dee47b45d9 100644
> --- a/drivers/mfd/lpc_ich.c
> +++ b/drivers/mfd/lpc_ich.c
> @@ -984,6 +984,10 @@ static int lpc_ich_init_wdt(struct pci_dev *dev)
>  	int ret;
>  	struct resource *res;
>  
> +	/* If we have ACPI based watchdog use that instead */
> +	if (acpi_has_watchdog())
> +		return -ENODEV;
> +
>  	/* Setup power management base register */
>  	pci_read_config_dword(dev, priv->abase, &base_addr_cfg);
>  	base_addr = base_addr_cfg & 0x0000ff80;

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v2 2/4] mfd: lpc_ich: Do not create iTCO watchdog when WDAT table exists

[Rafael J. Wysocki]

On Tuesday, September 27, 2016 08:41:14 PM Lee Jones wrote:
> On Tue, 20 Sep 2016, Mika Westerberg wrote:
> 
> > ACPI WDAT table is the preferred way to use hardware watchdog over the
> > native iTCO_wdt. Windows only uses this table for its hardware watchdog
> > implementation so we should be relatively safe to trust it has been
> > validated by OEMs
> > 
> > Prevent iTCO watchdog creation if we detect that there is ACPI WDAT table.
> > 
> > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> > Reviewed-by: Guenter Roeck <linux@roeck-us.net>
> > ---
> >  drivers/mfd/lpc_ich.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> 
> Applied, thanks.

Well, I applied this too.

And it depends on the [1/4], doesn't it?

> 
> > diff --git a/drivers/mfd/lpc_ich.c b/drivers/mfd/lpc_ich.c
> > index bd3aa4578346..c8dee47b45d9 100644
> > --- a/drivers/mfd/lpc_ich.c
> > +++ b/drivers/mfd/lpc_ich.c
> > @@ -984,6 +984,10 @@ static int lpc_ich_init_wdt(struct pci_dev *dev)
> >  	int ret;
> >  	struct resource *res;
> >  
> > +	/* If we have ACPI based watchdog use that instead */
> > +	if (acpi_has_watchdog())
> > +		return -ENODEV;
> > +
> >  	/* Setup power management base register */
> >  	pci_read_config_dword(dev, priv->abase, &base_addr_cfg);
> >  	base_addr = base_addr_cfg & 0x0000ff80;
> 
> 

Thanks,
Rafael

Re: [PATCH v2 2/4] mfd: lpc_ich: Do not create iTCO watchdog when WDAT table exists

[Lee Jones]

On Wed, 28 Sep 2016, Rafael J. Wysocki wrote:

> On Tuesday, September 27, 2016 08:41:14 PM Lee Jones wrote:
> > On Tue, 20 Sep 2016, Mika Westerberg wrote:
> > 
> > > ACPI WDAT table is the preferred way to use hardware watchdog over the
> > > native iTCO_wdt. Windows only uses this table for its hardware watchdog
> > > implementation so we should be relatively safe to trust it has been
> > > validated by OEMs
> > > 
> > > Prevent iTCO watchdog creation if we detect that there is ACPI WDAT table.
> > > 
> > > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> > > Reviewed-by: Guenter Roeck <linux@roeck-us.net>
> > > ---
> > >  drivers/mfd/lpc_ich.c | 4 ++++
> > >  1 file changed, 4 insertions(+)
> > 
> > Applied, thanks.
> 
> Well, I applied this too.

How can you apply this without an MFD Ack?

> And it depends on the [1/4], doesn't it?

Yes, I just found that out myself. :)

Well I only have 4 lines of changes in drivers/mfd/lpc_ich.c, so I
guess it'll be okay to apply this without the fear of conflicts.

Do that end, please apply my:

Acked-by: Lee Jones <lee.jones@linaro.org>
  
> > > diff --git a/drivers/mfd/lpc_ich.c b/drivers/mfd/lpc_ich.c
> > > index bd3aa4578346..c8dee47b45d9 100644
> > > --- a/drivers/mfd/lpc_ich.c
> > > +++ b/drivers/mfd/lpc_ich.c
> > > @@ -984,6 +984,10 @@ static int lpc_ich_init_wdt(struct pci_dev *dev)
> > >  	int ret;
> > >  	struct resource *res;
> > >  
> > > +	/* If we have ACPI based watchdog use that instead */
> > > +	if (acpi_has_watchdog())
> > > +		return -ENODEV;
> > > +
> > >  	/* Setup power management base register */
> > >  	pci_read_config_dword(dev, priv->abase, &base_addr_cfg);
> > >  	base_addr = base_addr_cfg & 0x0000ff80;
> > 
> > 
> 
> Thanks,
> Rafael
> 

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v2 2/4] mfd: lpc_ich: Do not create iTCO watchdog when WDAT table exists

[Rafael J. Wysocki]

On Wednesday, September 28, 2016 02:09:41 AM Lee Jones wrote:
> On Wed, 28 Sep 2016, Rafael J. Wysocki wrote:
> 
> > On Tuesday, September 27, 2016 08:41:14 PM Lee Jones wrote:
> > > On Tue, 20 Sep 2016, Mika Westerberg wrote:
> > > 
> > > > ACPI WDAT table is the preferred way to use hardware watchdog over the
> > > > native iTCO_wdt. Windows only uses this table for its hardware watchdog
> > > > implementation so we should be relatively safe to trust it has been
> > > > validated by OEMs
> > > > 
> > > > Prevent iTCO watchdog creation if we detect that there is ACPI WDAT table.
> > > > 
> > > > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> > > > Reviewed-by: Guenter Roeck <linux@roeck-us.net>
> > > > ---
> > > >  drivers/mfd/lpc_ich.c | 4 ++++
> > > >  1 file changed, 4 insertions(+)
> > > 
> > > Applied, thanks.
> > 
> > Well, I applied this too.
> 
> How can you apply this without an MFD Ack?

First, Guenter has reviewed it.

Second, there was no response to this:

http://marc.info/?l=linux-acpi&m=147467687316117&w=4

> > And it depends on the [1/4], doesn't it?
> 
> Yes, I just found that out myself. :)
> 
> Well I only have 4 lines of changes in drivers/mfd/lpc_ich.c, so I
> guess it'll be okay to apply this without the fear of conflicts.
> 
> Do that end, please apply my:
> 
> Acked-by: Lee Jones <lee.jones@linaro.org>

Thanks!

Best,
Rafael

Re: [PATCH v2 2/4] mfd: lpc_ich: Do not create iTCO watchdog when WDAT table exists

[Lee Jones]

On Wed, 28 Sep 2016, Rafael J. Wysocki wrote:

> On Wednesday, September 28, 2016 02:09:41 AM Lee Jones wrote:
> > On Wed, 28 Sep 2016, Rafael J. Wysocki wrote:
> > 
> > > On Tuesday, September 27, 2016 08:41:14 PM Lee Jones wrote:
> > > > On Tue, 20 Sep 2016, Mika Westerberg wrote:
> > > > 
> > > > > ACPI WDAT table is the preferred way to use hardware watchdog over the
> > > > > native iTCO_wdt. Windows only uses this table for its hardware watchdog
> > > > > implementation so we should be relatively safe to trust it has been
> > > > > validated by OEMs
> > > > > 
> > > > > Prevent iTCO watchdog creation if we detect that there is ACPI WDAT table.
> > > > > 
> > > > > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> > > > > Reviewed-by: Guenter Roeck <linux@roeck-us.net>
> > > > > ---
> > > > >  drivers/mfd/lpc_ich.c | 4 ++++
> > > > >  1 file changed, 4 insertions(+)
> > > > 
> > > > Applied, thanks.
> > > 
> > > Well, I applied this too.
> > 
> > How can you apply this without an MFD Ack?
> 
> First, Guenter has reviewed it.

Guenter isn't the MFD Maintiner.

> Second, there was no response to this:
> 
> http://marc.info/?l=linux-acpi&m=147467687316117&w=4

This in my queue.  I was on vacation last week.

> > > And it depends on the [1/4], doesn't it?
> > 
> > Yes, I just found that out myself. :)
> > 
> > Well I only have 4 lines of changes in drivers/mfd/lpc_ich.c, so I
> > guess it'll be okay to apply this without the fear of conflicts.
> > 
> > Do that end, please apply my:
> > 
> > Acked-by: Lee Jones <lee.jones@linaro.org>
> 
> Thanks!

NP

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog