All of lore.kernel.org
 help / color / mirror / Atom feed
* [RESEND PATCH] watchdog: ni7018_wdt: Add NIC7018 watchdog driver
@ 2016-10-28  7:51 Hui Chun Ong
  2016-10-28 17:25 ` Guenter Roeck
  0 siblings, 1 reply; 3+ messages in thread
From: Hui Chun Ong @ 2016-10-28  7:51 UTC (permalink / raw)
  To: wim, linux, corbet
  Cc: linux-watchdog, linux-doc, jonathan.hearn, julia.cartwright,
	Hui Chun Ong

Add support for the watchdog timer on PXI Embedded Controller.

Signed-off-by: Hui Chun Ong <hui.chun.ong@ni.com>
---
 Documentation/watchdog/watchdog-parameters.txt |   5 +
 drivers/watchdog/Kconfig                       |  11 +
 drivers/watchdog/Makefile                      |   1 +
 drivers/watchdog/ni7018_wdt.c                  | 506 +++++++++++++++++++++++++
 4 files changed, 523 insertions(+)
 create mode 100644 drivers/watchdog/ni7018_wdt.c

diff --git a/Documentation/watchdog/watchdog-parameters.txt b/Documentation/watchdog/watchdog-parameters.txt
index a8d3642..25dbfa2 100644
--- a/Documentation/watchdog/watchdog-parameters.txt
+++ b/Documentation/watchdog/watchdog-parameters.txt
@@ -204,6 +204,11 @@ mv64x60_wdt:
 nowayout: Watchdog cannot be stopped once started
 	(default=kernel config parameter)
 -------------------------------------------------
+ni7018_wdt:
+timeout: Initial watchdog timeout in seconds (0<timeout<464, default=80)
+nowayout: Watchdog cannot be stopped once started
+	(default=kernel config parameter)
+-------------------------------------------------
 ni903x_wdt:
 timeout: Initial watchdog timeout in seconds (0<timeout<516, default=60)
 nowayout: Watchdog cannot be stopped once started
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index fdd3228..6d185af 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -1314,6 +1314,17 @@ config INTEL_MEI_WDT
 	  To compile this driver as a module, choose M here:
 	  the module will be called mei_wdt.
 
+config NI7018_WDT
+	tristate "NI 7018 Watchdog"
+	depends on X86 && ACPI
+	select WATCHDOG_CORE
+	---help---
+	  This is the device driver for National Instruments NIC7018 watchdog
+	  timer.
+
+	  To compile this driver as a module, choose M here: the module will be
+	  called ni7018_wdt.
+
 config NI903X_WDT
 	tristate "NI 903x/913x Watchdog"
 	depends on X86 && ACPI
diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
index caa9f4a..4605cd2 100644
--- a/drivers/watchdog/Makefile
+++ b/drivers/watchdog/Makefile
@@ -139,6 +139,7 @@ obj-$(CONFIG_INTEL_SCU_WATCHDOG) += intel_scu_watchdog.o
 obj-$(CONFIG_INTEL_MID_WATCHDOG) += intel-mid_wdt.o
 obj-$(CONFIG_INTEL_MEI_WDT) += mei_wdt.o
 obj-$(CONFIG_NI903X_WDT) += ni903x_wdt.o
+obj-$(CONFIG_NI7018_WDT) += ni7018_wdt.o
 
 # M32R Architecture
 
diff --git a/drivers/watchdog/ni7018_wdt.c b/drivers/watchdog/ni7018_wdt.c
new file mode 100644
index 0000000..7690314
--- /dev/null
+++ b/drivers/watchdog/ni7018_wdt.c
@@ -0,0 +1,506 @@
+/*
+ * Copyright (C) 2016 National Instruments Corp.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/acpi.h>
+#include <linux/device.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/sysfs.h>
+#include <linux/watchdog.h>
+
+#define LOCK		0xA5
+#define UNLOCK		0x5A
+
+#define WDT_CTRL_RESET_EN		BIT(7)
+#define WDT_CTRL_TRIG_POLARITY		BIT(4)
+#define WDT_CTRL_TRIG			0x0F
+
+#define WDT_RELOAD_PORT_EN		BIT(7)
+#define WDT_RELOAD_TRIG_POLARITY	BIT(6)
+#define WDT_RELOAD_TRIG			0x0F
+#define WDT_MIN_TRIG_NUM		0
+#define WDT_MAX_TRIG_NUM		8
+
+#define WDT_CTRL		1
+#define WDT_RELOAD_CTRL		2
+#define WDT_PRESET_PRESCALE	4
+#define WDT_REG_LOCK		5
+#define WDT_COUNT		6
+#define WDT_RELOAD_PORT		7
+
+#define WDT_IO_SIZE		8
+
+#define WDT_MIN_TIMEOUT		1
+#define WDT_MAX_TIMEOUT		464
+#define WDT_DEFAULT_TIMEOUT	80
+
+#define WDT_MAX_COUNTER		15
+
+static unsigned int timeout;
+module_param(timeout, uint, 0);
+MODULE_PARM_DESC(timeout,
+		 "Watchdog timeout in seconds. (default="
+		 __MODULE_STRING(WDT_DEFAULT_TIMEOUT) ")");
+
+static int nowayout = WATCHDOG_NOWAYOUT;
+module_param(nowayout, int, S_IRUGO);
+MODULE_PARM_DESC(nowayout,
+		 "Watchdog cannot be stopped once started. (default="
+		 __MODULE_STRING(WATCHDOG_NOWAYOUT) ")");
+
+struct ni7018_wdt {
+	struct device *dev;
+	u16 io_base;
+	u32 period_ms;
+	struct mutex lock;
+	struct watchdog_device wdd;
+};
+
+struct ni7018_config {
+	u32 period_ms;
+	u8 divider;
+};
+
+static const struct ni7018_config ni7018_configs[] = {
+	{   125, 3 },
+	{  2000, 4 },
+	{ 32000, 5 },
+};
+
+static inline u32 ni7018_timeout_ms(u32 period_ms, u8 counter)
+{
+	return period_ms * counter - period_ms / 2;
+}
+
+static const struct ni7018_config *ni7018_get_config(u32 timeout_ms,
+						     u8 *counter)
+{
+	u32 delta, i, best_delta = U32_MAX;
+	const struct ni7018_config *config, *best_config = NULL;
+	u8 count;
+
+	for (i = 0; i < ARRAY_SIZE(ni7018_configs); i++) {
+		config = &ni7018_configs[i];
+
+		count = DIV_ROUND_UP(timeout_ms + config->period_ms / 2,
+				     config->period_ms);
+
+		if (count > WDT_MAX_COUNTER)
+			continue;
+
+		delta = ni7018_timeout_ms(config->period_ms, count) -
+			timeout_ms;
+
+		if (delta < best_delta) {
+			best_delta = delta;
+			best_config = config;
+			*counter = count;
+		}
+	}
+	return best_config;
+}
+
+static int ni7018_set_timeout(struct watchdog_device *wdd,
+			      unsigned int timeout)
+{
+	struct ni7018_wdt *wdt = watchdog_get_drvdata(wdd);
+	const struct ni7018_config *config;
+	u8 counter;
+
+	config = ni7018_get_config(timeout * 1000, &counter);
+	if (!config)
+		return -EINVAL;
+
+	mutex_lock(&wdt->lock);
+
+	outb(counter << 4 | config->divider,
+	     wdt->io_base + WDT_PRESET_PRESCALE);
+
+	wdd->timeout = ni7018_timeout_ms(config->period_ms, counter) / 1000;
+	wdt->period_ms = config->period_ms;
+
+	mutex_unlock(&wdt->lock);
+	return 0;
+}
+
+static int ni7018_start(struct watchdog_device *wdd)
+{
+	struct ni7018_wdt *wdt = watchdog_get_drvdata(wdd);
+	u8 control;
+
+	ni7018_set_timeout(wdd, wdd->timeout);
+
+	mutex_lock(&wdt->lock);
+
+	control = inb(wdt->io_base + WDT_RELOAD_CTRL);
+	outb(control | WDT_RELOAD_PORT_EN, wdt->io_base + WDT_RELOAD_CTRL);
+
+	outb(1, wdt->io_base + WDT_RELOAD_PORT);
+
+	control = inb(wdt->io_base + WDT_CTRL);
+	outb(control | WDT_CTRL_RESET_EN, wdt->io_base + WDT_CTRL);
+
+	mutex_unlock(&wdt->lock);
+	return 0;
+}
+
+static int ni7018_stop(struct watchdog_device *wdd)
+{
+	struct ni7018_wdt *wdt = watchdog_get_drvdata(wdd);
+
+	mutex_lock(&wdt->lock);
+
+	outb(0, wdt->io_base + WDT_CTRL);
+	outb(0, wdt->io_base + WDT_RELOAD_CTRL);
+	outb(0xF0, wdt->io_base + WDT_PRESET_PRESCALE);
+
+	mutex_unlock(&wdt->lock);
+	return 0;
+}
+
+static int ni7018_ping(struct watchdog_device *wdd)
+{
+	struct ni7018_wdt *wdt = watchdog_get_drvdata(wdd);
+
+	mutex_lock(&wdt->lock);
+
+	outb(1, wdt->io_base + WDT_RELOAD_PORT);
+
+	mutex_unlock(&wdt->lock);
+	return 0;
+}
+
+static unsigned int ni7018_get_timeleft(struct watchdog_device *wdd)
+{
+	struct ni7018_wdt *wdt = watchdog_get_drvdata(wdd);
+	u8 count;
+
+	mutex_lock(&wdt->lock);
+
+	count = inb(wdt->io_base + WDT_COUNT) & 0xF;
+
+	mutex_unlock(&wdt->lock);
+
+	if (!count)
+		return 0;
+
+	return ni7018_timeout_ms(wdt->period_ms, count) / 1000;
+}
+
+static acpi_status
+ni7018_resources(struct acpi_resource *res, void *data)
+{
+	struct ni7018_wdt *wdt = data;
+	u16 io_size;
+
+	switch (res->type) {
+	case ACPI_RESOURCE_TYPE_IO:
+		if (wdt->io_base != 0) {
+			dev_err(wdt->dev, "too many IO resources\n");
+			return AE_ERROR;
+		}
+
+		wdt->io_base = res->data.io.minimum;
+		io_size = res->data.io.address_length;
+
+		if (io_size < WDT_IO_SIZE) {
+			dev_err(wdt->dev, "memory region too small\n");
+			return AE_ERROR;
+		}
+
+		if (!devm_request_region(wdt->dev, wdt->io_base, io_size,
+					 KBUILD_MODNAME)) {
+			dev_err(wdt->dev, "failed to get memory region\n");
+			return AE_ERROR;
+		}
+
+		return AE_OK;
+
+	default:
+		/* Ignore unsupported resources */
+		return AE_OK;
+	}
+}
+
+static const struct watchdog_info ni7018_wdd_info = {
+	.options = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE,
+	.identity = "NI 7018 Watchdog",
+};
+
+static const struct watchdog_ops ni7018_wdd_ops = {
+	.owner = THIS_MODULE,
+	.start = ni7018_start,
+	.stop = ni7018_stop,
+	.ping = ni7018_ping,
+	.set_timeout = ni7018_set_timeout,
+	.get_timeleft = ni7018_get_timeleft,
+};
+
+struct ni7018_attr {
+	struct device_attribute dev_attr;
+	u8 offset;
+	u8 bit;
+};
+#define to_ni7018_attr(_attr) \
+	container_of((_attr), struct ni7018_attr, dev_attr)
+
+static ssize_t wdt_attr_show(struct device *dev,
+			     struct device_attribute *da,
+			     char *buf)
+{
+	struct watchdog_device *wdd = dev_get_drvdata(dev);
+	struct ni7018_wdt *wdt = watchdog_get_drvdata(wdd);
+	struct ni7018_attr *attr = to_ni7018_attr(da);
+	u8 control;
+	size_t ret;
+
+	mutex_lock(&wdt->lock);
+
+	control = inb(wdt->io_base + attr->offset);
+
+	mutex_unlock(&wdt->lock);
+
+	ret = sprintf(buf, "%u\n", !!(control & attr->bit));
+	return ret;
+}
+
+static ssize_t wdt_attr_store(struct device *dev,
+			      struct device_attribute *da,
+			      const char *buf,
+			      size_t size)
+{
+	struct watchdog_device *wdd = dev_get_drvdata(dev);
+	struct ni7018_wdt *wdt = watchdog_get_drvdata(wdd);
+	struct ni7018_attr *attr = to_ni7018_attr(da);
+	unsigned long val;
+	u8 control;
+
+	int ret = kstrtoul(buf, 10, &val);
+
+	if (ret)
+		return ret;
+
+	if (val < 0 || val > 1)
+		return -EINVAL;
+
+	mutex_lock(&wdt->lock);
+
+	control = inb(wdt->io_base + attr->offset);
+	if (val)
+		outb(control | attr->bit, wdt->io_base + attr->offset);
+	else
+		outb(control & ~attr->bit, wdt->io_base + attr->offset);
+
+	mutex_unlock(&wdt->lock);
+	return size;
+}
+
+#define WDT_ATTR(_name, _offset, _bit) \
+	struct ni7018_attr dev_attr_##_name = { \
+		.offset = _offset, \
+		.bit = _bit, \
+		.dev_attr = \
+			__ATTR(_name, S_IWUSR | S_IRUGO, \
+			       wdt_attr_show, wdt_attr_store), \
+	}
+
+static WDT_ATTR(enable_reset, WDT_CTRL, WDT_CTRL_RESET_EN);
+static WDT_ATTR(enable_soft_ping, WDT_RELOAD_CTRL, WDT_RELOAD_PORT_EN);
+static WDT_ATTR(trigger_polarity, WDT_CTRL, WDT_CTRL_TRIG_POLARITY);
+static WDT_ATTR(keepalive_trigger_polarity, WDT_RELOAD_CTRL,
+		WDT_RELOAD_TRIG_POLARITY);
+
+static ssize_t wdt_trig_show(struct device *dev,
+			     struct device_attribute *da,
+			     char *buf)
+{
+	struct watchdog_device *wdd = dev_get_drvdata(dev);
+	struct ni7018_wdt *wdt = watchdog_get_drvdata(wdd);
+	struct ni7018_attr *attr = to_ni7018_attr(da);
+	u8 control;
+	size_t ret;
+
+	mutex_lock(&wdt->lock);
+
+	control = inb(wdt->io_base + attr->offset);
+
+	mutex_unlock(&wdt->lock);
+
+	if (control & attr->bit)
+		ret = sprintf(buf, "trig%u\n", (control & attr->bit) - 1);
+	else
+		ret = sprintf(buf, "none\n");
+
+	return ret;
+}
+
+static ssize_t wdt_trig_store(struct device *dev,
+			      struct device_attribute *da,
+			      const char *buf,
+			      size_t size)
+{
+	struct watchdog_device *wdd = dev_get_drvdata(dev);
+	struct ni7018_wdt *wdt = watchdog_get_drvdata(wdd);
+	struct ni7018_attr *attr = to_ni7018_attr(da);
+	unsigned long val;
+	u8 control;
+	int ret;
+
+	if (!strncmp(buf, "trig", 4)) {
+		ret = kstrtoul(buf + 4, 10, &val);
+		if (ret)
+			return ret;
+
+		if (val < WDT_MIN_TRIG_NUM || val > WDT_MAX_TRIG_NUM)
+			return -EINVAL;
+
+		mutex_lock(&wdt->lock);
+
+		control = inb(wdt->io_base + attr->offset);
+		outb((control & ~attr->bit) | (val + 1),
+		     wdt->io_base + attr->offset);
+
+		mutex_unlock(&wdt->lock);
+
+	} else if (!strncmp(buf, "none", 4)) {
+		mutex_lock(&wdt->lock);
+
+		control = inb(wdt->io_base + attr->offset);
+		outb(control & ~attr->bit, wdt->io_base + attr->offset);
+
+		mutex_unlock(&wdt->lock);
+
+	} else {
+		return -EINVAL;
+	}
+
+	return size;
+}
+
+#define WDT_TRIG_ATTR(_name, _offset, _bit) \
+	struct ni7018_attr dev_attr_##_name = { \
+		.offset = _offset, \
+		.bit = _bit, \
+		.dev_attr = \
+			__ATTR(_name, S_IWUSR | S_IRUGO, \
+			       wdt_trig_show, wdt_trig_store), \
+	}
+
+static WDT_TRIG_ATTR(trigger, WDT_CTRL, WDT_CTRL_TRIG);
+static WDT_TRIG_ATTR(keepalive_trigger, WDT_RELOAD_CTRL, WDT_RELOAD_TRIG);
+
+static struct attribute *ni7018_wdt_attrs[] = {
+	&dev_attr_enable_reset.dev_attr.attr,
+	&dev_attr_enable_soft_ping.dev_attr.attr,
+	&dev_attr_trigger_polarity.dev_attr.attr,
+	&dev_attr_keepalive_trigger_polarity.dev_attr.attr,
+	&dev_attr_trigger.dev_attr.attr,
+	&dev_attr_keepalive_trigger.dev_attr.attr,
+	NULL
+};
+ATTRIBUTE_GROUPS(ni7018_wdt);
+
+static int ni7018_add(struct acpi_device *device)
+{
+	struct device *dev = &device->dev;
+	struct watchdog_device *wdd;
+	struct ni7018_wdt *wdt;
+	acpi_status status;
+	int ret;
+
+	wdt = devm_kzalloc(dev, sizeof(*wdt), GFP_KERNEL);
+	if (!wdt)
+		return -ENOMEM;
+
+	device->driver_data = wdt;
+	wdt->dev = dev;
+
+	status = acpi_walk_resources(device->handle, METHOD_NAME__CRS,
+				     ni7018_resources, wdt);
+
+	if (ACPI_FAILURE(status) || wdt->io_base == 0) {
+		dev_err(dev, "failed to get resources\n");
+		return -ENODEV;
+	}
+
+	wdd = &wdt->wdd;
+	wdd->info = &ni7018_wdd_info;
+	wdd->ops = &ni7018_wdd_ops;
+	wdd->min_timeout = WDT_MIN_TIMEOUT;
+	wdd->max_timeout = WDT_MAX_TIMEOUT;
+	wdd->timeout = WDT_DEFAULT_TIMEOUT;
+	wdd->parent = dev;
+	wdd->groups = ni7018_wdt_groups;
+
+	mutex_init(&wdt->lock);
+	watchdog_set_drvdata(wdd, wdt);
+	watchdog_set_nowayout(wdd, nowayout);
+
+	ret = watchdog_init_timeout(wdd, timeout, dev);
+	if (ret)
+		dev_err(dev, "unable to set timeout value, using default\n");
+
+	/* Unlock WDT register */
+	outb(UNLOCK, wdt->io_base + WDT_REG_LOCK);
+
+	ret = watchdog_register_device(wdd);
+	if (ret) {
+		dev_err(dev, "failed to register watchdog\n");
+		goto err_out;
+	}
+
+	dev_dbg(dev, "io_base=0x%04X, timeout=%d, nowayout=%d\n",
+		wdt->io_base, timeout, nowayout);
+	return 0;
+
+err_out:
+	mutex_destroy(&wdt->lock);
+	return ret;
+}
+
+static int ni7018_remove(struct acpi_device *device)
+{
+	struct ni7018_wdt *wdt = acpi_driver_data(device);
+
+	ni7018_stop(&wdt->wdd);
+	watchdog_unregister_device(&wdt->wdd);
+
+	/* Lock WDT register */
+	outb(LOCK, wdt->io_base + WDT_REG_LOCK);
+
+	mutex_destroy(&wdt->lock);
+	return 0;
+}
+
+static const struct acpi_device_id ni7018_device_ids[] = {
+	{"NIC7018", 0},
+	{"", 0},
+};
+MODULE_DEVICE_TABLE(acpi, ni7018_device_ids);
+
+static struct acpi_driver ni7018_acpi_driver = {
+	.name = KBUILD_MODNAME,
+	.ids = ni7018_device_ids,
+	.ops = {
+		.add = ni7018_add,
+		.remove = ni7018_remove,
+	},
+};
+
+module_acpi_driver(ni7018_acpi_driver);
+
+MODULE_DESCRIPTION("NI 7018 Watchdog");
+MODULE_AUTHOR("Hui Chun Ong <hui.chun.ong@ni.com>");
+MODULE_LICENSE("GPL v2");
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [RESEND PATCH] watchdog: ni7018_wdt: Add NIC7018 watchdog driver
  2016-10-28  7:51 [RESEND PATCH] watchdog: ni7018_wdt: Add NIC7018 watchdog driver Hui Chun Ong
@ 2016-10-28 17:25 ` Guenter Roeck
  2016-11-16  2:52   ` Hui Chun Ong
  0 siblings, 1 reply; 3+ messages in thread
From: Guenter Roeck @ 2016-10-28 17:25 UTC (permalink / raw)
  To: Hui Chun Ong
  Cc: wim, corbet, linux-watchdog, linux-doc, jonathan.hearn, julia.cartwright

Hi,

On Fri, Oct 28, 2016 at 03:51:07PM +0800, Hui Chun Ong wrote:
> Add support for the watchdog timer on PXI Embedded Controller.
> 
> Signed-off-by: Hui Chun Ong <hui.chun.ong@ni.com>

The problem with your patch are the non-standard attributes it introduces.

+static WDT_ATTR(enable_reset, WDT_CTRL, WDT_CTRL_RESET_EN);
+static WDT_ATTR(enable_soft_ping, WDT_RELOAD_CTRL, WDT_RELOAD_PORT_EN);
+static WDT_ATTR(trigger_polarity, WDT_CTRL, WDT_CTRL_TRIG_POLARITY);
+static WDT_ATTR(keepalive_trigger_polarity, WDT_RELOAD_CTRL,
+               WDT_RELOAD_TRIG_POLARITY);

At first glance, those appear to be platform configuration parameters,
which should either be hard-coded or configured through devicetree /
platform data. Whatever it is, such non-standard attributes consume a
large amount of time, since reviewers will have to understand what the
attributes are used for. On top of that, it is likely that we'll probably
need several rounds of reviews and arguments to determine if the attributes
are really needed or inappropriate. This all takes a lot of time from
reviewers. Because of that, patches like this tend to end up at the end
of my review queue.

Guenter

> ---
>  Documentation/watchdog/watchdog-parameters.txt |   5 +
>  drivers/watchdog/Kconfig                       |  11 +
>  drivers/watchdog/Makefile                      |   1 +
>  drivers/watchdog/ni7018_wdt.c                  | 506 +++++++++++++++++++++++++
>  4 files changed, 523 insertions(+)
>  create mode 100644 drivers/watchdog/ni7018_wdt.c
> 
> diff --git a/Documentation/watchdog/watchdog-parameters.txt b/Documentation/watchdog/watchdog-parameters.txt
> index a8d3642..25dbfa2 100644
> --- a/Documentation/watchdog/watchdog-parameters.txt
> +++ b/Documentation/watchdog/watchdog-parameters.txt
> @@ -204,6 +204,11 @@ mv64x60_wdt:
>  nowayout: Watchdog cannot be stopped once started
>  	(default=kernel config parameter)
>  -------------------------------------------------
> +ni7018_wdt:
> +timeout: Initial watchdog timeout in seconds (0<timeout<464, default=80)
> +nowayout: Watchdog cannot be stopped once started
> +	(default=kernel config parameter)
> +-------------------------------------------------
>  ni903x_wdt:
>  timeout: Initial watchdog timeout in seconds (0<timeout<516, default=60)
>  nowayout: Watchdog cannot be stopped once started
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index fdd3228..6d185af 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -1314,6 +1314,17 @@ config INTEL_MEI_WDT
>  	  To compile this driver as a module, choose M here:
>  	  the module will be called mei_wdt.
>  
> +config NI7018_WDT
> +	tristate "NI 7018 Watchdog"
> +	depends on X86 && ACPI
> +	select WATCHDOG_CORE
> +	---help---
> +	  This is the device driver for National Instruments NIC7018 watchdog
> +	  timer.
> +
> +	  To compile this driver as a module, choose M here: the module will be
> +	  called ni7018_wdt.
> +
>  config NI903X_WDT
>  	tristate "NI 903x/913x Watchdog"
>  	depends on X86 && ACPI
> diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
> index caa9f4a..4605cd2 100644
> --- a/drivers/watchdog/Makefile
> +++ b/drivers/watchdog/Makefile
> @@ -139,6 +139,7 @@ obj-$(CONFIG_INTEL_SCU_WATCHDOG) += intel_scu_watchdog.o
>  obj-$(CONFIG_INTEL_MID_WATCHDOG) += intel-mid_wdt.o
>  obj-$(CONFIG_INTEL_MEI_WDT) += mei_wdt.o
>  obj-$(CONFIG_NI903X_WDT) += ni903x_wdt.o
> +obj-$(CONFIG_NI7018_WDT) += ni7018_wdt.o
>  
>  # M32R Architecture
>  
> diff --git a/drivers/watchdog/ni7018_wdt.c b/drivers/watchdog/ni7018_wdt.c
> new file mode 100644
> index 0000000..7690314
> --- /dev/null
> +++ b/drivers/watchdog/ni7018_wdt.c
> @@ -0,0 +1,506 @@
> +/*
> + * Copyright (C) 2016 National Instruments Corp.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/acpi.h>
> +#include <linux/device.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/sysfs.h>
> +#include <linux/watchdog.h>
> +
> +#define LOCK		0xA5
> +#define UNLOCK		0x5A
> +
> +#define WDT_CTRL_RESET_EN		BIT(7)
> +#define WDT_CTRL_TRIG_POLARITY		BIT(4)
> +#define WDT_CTRL_TRIG			0x0F
> +
> +#define WDT_RELOAD_PORT_EN		BIT(7)
> +#define WDT_RELOAD_TRIG_POLARITY	BIT(6)
> +#define WDT_RELOAD_TRIG			0x0F
> +#define WDT_MIN_TRIG_NUM		0
> +#define WDT_MAX_TRIG_NUM		8
> +
> +#define WDT_CTRL		1
> +#define WDT_RELOAD_CTRL		2
> +#define WDT_PRESET_PRESCALE	4
> +#define WDT_REG_LOCK		5
> +#define WDT_COUNT		6
> +#define WDT_RELOAD_PORT		7
> +
> +#define WDT_IO_SIZE		8
> +
> +#define WDT_MIN_TIMEOUT		1
> +#define WDT_MAX_TIMEOUT		464
> +#define WDT_DEFAULT_TIMEOUT	80
> +
> +#define WDT_MAX_COUNTER		15
> +
> +static unsigned int timeout;
> +module_param(timeout, uint, 0);
> +MODULE_PARM_DESC(timeout,
> +		 "Watchdog timeout in seconds. (default="
> +		 __MODULE_STRING(WDT_DEFAULT_TIMEOUT) ")");
> +
> +static int nowayout = WATCHDOG_NOWAYOUT;
> +module_param(nowayout, int, S_IRUGO);
> +MODULE_PARM_DESC(nowayout,
> +		 "Watchdog cannot be stopped once started. (default="
> +		 __MODULE_STRING(WATCHDOG_NOWAYOUT) ")");
> +
> +struct ni7018_wdt {
> +	struct device *dev;
> +	u16 io_base;
> +	u32 period_ms;
> +	struct mutex lock;
> +	struct watchdog_device wdd;
> +};
> +
> +struct ni7018_config {
> +	u32 period_ms;
> +	u8 divider;
> +};
> +
> +static const struct ni7018_config ni7018_configs[] = {
> +	{   125, 3 },
> +	{  2000, 4 },
> +	{ 32000, 5 },
> +};
> +
> +static inline u32 ni7018_timeout_ms(u32 period_ms, u8 counter)
> +{
> +	return period_ms * counter - period_ms / 2;
> +}
> +
> +static const struct ni7018_config *ni7018_get_config(u32 timeout_ms,
> +						     u8 *counter)
> +{
> +	u32 delta, i, best_delta = U32_MAX;
> +	const struct ni7018_config *config, *best_config = NULL;
> +	u8 count;
> +
> +	for (i = 0; i < ARRAY_SIZE(ni7018_configs); i++) {
> +		config = &ni7018_configs[i];
> +
> +		count = DIV_ROUND_UP(timeout_ms + config->period_ms / 2,
> +				     config->period_ms);
> +
> +		if (count > WDT_MAX_COUNTER)
> +			continue;
> +
> +		delta = ni7018_timeout_ms(config->period_ms, count) -
> +			timeout_ms;
> +
> +		if (delta < best_delta) {
> +			best_delta = delta;
> +			best_config = config;
> +			*counter = count;
> +		}
> +	}
> +	return best_config;
> +}
> +
> +static int ni7018_set_timeout(struct watchdog_device *wdd,
> +			      unsigned int timeout)
> +{
> +	struct ni7018_wdt *wdt = watchdog_get_drvdata(wdd);
> +	const struct ni7018_config *config;
> +	u8 counter;
> +
> +	config = ni7018_get_config(timeout * 1000, &counter);
> +	if (!config)
> +		return -EINVAL;
> +
> +	mutex_lock(&wdt->lock);
> +
> +	outb(counter << 4 | config->divider,
> +	     wdt->io_base + WDT_PRESET_PRESCALE);
> +
> +	wdd->timeout = ni7018_timeout_ms(config->period_ms, counter) / 1000;
> +	wdt->period_ms = config->period_ms;
> +
> +	mutex_unlock(&wdt->lock);
> +	return 0;
> +}
> +
> +static int ni7018_start(struct watchdog_device *wdd)
> +{
> +	struct ni7018_wdt *wdt = watchdog_get_drvdata(wdd);
> +	u8 control;
> +
> +	ni7018_set_timeout(wdd, wdd->timeout);
> +
> +	mutex_lock(&wdt->lock);
> +
> +	control = inb(wdt->io_base + WDT_RELOAD_CTRL);
> +	outb(control | WDT_RELOAD_PORT_EN, wdt->io_base + WDT_RELOAD_CTRL);
> +
> +	outb(1, wdt->io_base + WDT_RELOAD_PORT);
> +
> +	control = inb(wdt->io_base + WDT_CTRL);
> +	outb(control | WDT_CTRL_RESET_EN, wdt->io_base + WDT_CTRL);
> +
> +	mutex_unlock(&wdt->lock);
> +	return 0;
> +}
> +
> +static int ni7018_stop(struct watchdog_device *wdd)
> +{
> +	struct ni7018_wdt *wdt = watchdog_get_drvdata(wdd);
> +
> +	mutex_lock(&wdt->lock);
> +
> +	outb(0, wdt->io_base + WDT_CTRL);
> +	outb(0, wdt->io_base + WDT_RELOAD_CTRL);
> +	outb(0xF0, wdt->io_base + WDT_PRESET_PRESCALE);
> +
> +	mutex_unlock(&wdt->lock);
> +	return 0;
> +}
> +
> +static int ni7018_ping(struct watchdog_device *wdd)
> +{
> +	struct ni7018_wdt *wdt = watchdog_get_drvdata(wdd);
> +
> +	mutex_lock(&wdt->lock);
> +
> +	outb(1, wdt->io_base + WDT_RELOAD_PORT);
> +
> +	mutex_unlock(&wdt->lock);
> +	return 0;
> +}
> +
> +static unsigned int ni7018_get_timeleft(struct watchdog_device *wdd)
> +{
> +	struct ni7018_wdt *wdt = watchdog_get_drvdata(wdd);
> +	u8 count;
> +
> +	mutex_lock(&wdt->lock);
> +
> +	count = inb(wdt->io_base + WDT_COUNT) & 0xF;
> +
> +	mutex_unlock(&wdt->lock);
> +
> +	if (!count)
> +		return 0;
> +
> +	return ni7018_timeout_ms(wdt->period_ms, count) / 1000;
> +}
> +
> +static acpi_status
> +ni7018_resources(struct acpi_resource *res, void *data)
> +{
> +	struct ni7018_wdt *wdt = data;
> +	u16 io_size;
> +
> +	switch (res->type) {
> +	case ACPI_RESOURCE_TYPE_IO:
> +		if (wdt->io_base != 0) {
> +			dev_err(wdt->dev, "too many IO resources\n");
> +			return AE_ERROR;
> +		}
> +
> +		wdt->io_base = res->data.io.minimum;
> +		io_size = res->data.io.address_length;
> +
> +		if (io_size < WDT_IO_SIZE) {
> +			dev_err(wdt->dev, "memory region too small\n");
> +			return AE_ERROR;
> +		}
> +
> +		if (!devm_request_region(wdt->dev, wdt->io_base, io_size,
> +					 KBUILD_MODNAME)) {
> +			dev_err(wdt->dev, "failed to get memory region\n");
> +			return AE_ERROR;
> +		}
> +
> +		return AE_OK;
> +
> +	default:
> +		/* Ignore unsupported resources */
> +		return AE_OK;
> +	}
> +}
> +
> +static const struct watchdog_info ni7018_wdd_info = {
> +	.options = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE,
> +	.identity = "NI 7018 Watchdog",
> +};
> +
> +static const struct watchdog_ops ni7018_wdd_ops = {
> +	.owner = THIS_MODULE,
> +	.start = ni7018_start,
> +	.stop = ni7018_stop,
> +	.ping = ni7018_ping,
> +	.set_timeout = ni7018_set_timeout,
> +	.get_timeleft = ni7018_get_timeleft,
> +};
> +
> +struct ni7018_attr {
> +	struct device_attribute dev_attr;
> +	u8 offset;
> +	u8 bit;
> +};
> +#define to_ni7018_attr(_attr) \
> +	container_of((_attr), struct ni7018_attr, dev_attr)
> +
> +static ssize_t wdt_attr_show(struct device *dev,
> +			     struct device_attribute *da,
> +			     char *buf)
> +{
> +	struct watchdog_device *wdd = dev_get_drvdata(dev);
> +	struct ni7018_wdt *wdt = watchdog_get_drvdata(wdd);
> +	struct ni7018_attr *attr = to_ni7018_attr(da);
> +	u8 control;
> +	size_t ret;
> +
> +	mutex_lock(&wdt->lock);
> +
> +	control = inb(wdt->io_base + attr->offset);
> +
> +	mutex_unlock(&wdt->lock);
> +
> +	ret = sprintf(buf, "%u\n", !!(control & attr->bit));
> +	return ret;
> +}
> +
> +static ssize_t wdt_attr_store(struct device *dev,
> +			      struct device_attribute *da,
> +			      const char *buf,
> +			      size_t size)
> +{
> +	struct watchdog_device *wdd = dev_get_drvdata(dev);
> +	struct ni7018_wdt *wdt = watchdog_get_drvdata(wdd);
> +	struct ni7018_attr *attr = to_ni7018_attr(da);
> +	unsigned long val;
> +	u8 control;
> +
> +	int ret = kstrtoul(buf, 10, &val);
> +
> +	if (ret)
> +		return ret;
> +
> +	if (val < 0 || val > 1)
> +		return -EINVAL;
> +
> +	mutex_lock(&wdt->lock);
> +
> +	control = inb(wdt->io_base + attr->offset);
> +	if (val)
> +		outb(control | attr->bit, wdt->io_base + attr->offset);
> +	else
> +		outb(control & ~attr->bit, wdt->io_base + attr->offset);
> +
> +	mutex_unlock(&wdt->lock);
> +	return size;
> +}
> +
> +#define WDT_ATTR(_name, _offset, _bit) \
> +	struct ni7018_attr dev_attr_##_name = { \
> +		.offset = _offset, \
> +		.bit = _bit, \
> +		.dev_attr = \
> +			__ATTR(_name, S_IWUSR | S_IRUGO, \
> +			       wdt_attr_show, wdt_attr_store), \
> +	}
> +
> +static WDT_ATTR(enable_reset, WDT_CTRL, WDT_CTRL_RESET_EN);
> +static WDT_ATTR(enable_soft_ping, WDT_RELOAD_CTRL, WDT_RELOAD_PORT_EN);
> +static WDT_ATTR(trigger_polarity, WDT_CTRL, WDT_CTRL_TRIG_POLARITY);
> +static WDT_ATTR(keepalive_trigger_polarity, WDT_RELOAD_CTRL,
> +		WDT_RELOAD_TRIG_POLARITY);
> +
> +static ssize_t wdt_trig_show(struct device *dev,
> +			     struct device_attribute *da,
> +			     char *buf)
> +{
> +	struct watchdog_device *wdd = dev_get_drvdata(dev);
> +	struct ni7018_wdt *wdt = watchdog_get_drvdata(wdd);
> +	struct ni7018_attr *attr = to_ni7018_attr(da);
> +	u8 control;
> +	size_t ret;
> +
> +	mutex_lock(&wdt->lock);
> +
> +	control = inb(wdt->io_base + attr->offset);
> +
> +	mutex_unlock(&wdt->lock);
> +
> +	if (control & attr->bit)
> +		ret = sprintf(buf, "trig%u\n", (control & attr->bit) - 1);
> +	else
> +		ret = sprintf(buf, "none\n");
> +
> +	return ret;
> +}
> +
> +static ssize_t wdt_trig_store(struct device *dev,
> +			      struct device_attribute *da,
> +			      const char *buf,
> +			      size_t size)
> +{
> +	struct watchdog_device *wdd = dev_get_drvdata(dev);
> +	struct ni7018_wdt *wdt = watchdog_get_drvdata(wdd);
> +	struct ni7018_attr *attr = to_ni7018_attr(da);
> +	unsigned long val;
> +	u8 control;
> +	int ret;
> +
> +	if (!strncmp(buf, "trig", 4)) {
> +		ret = kstrtoul(buf + 4, 10, &val);
> +		if (ret)
> +			return ret;
> +
> +		if (val < WDT_MIN_TRIG_NUM || val > WDT_MAX_TRIG_NUM)
> +			return -EINVAL;
> +
> +		mutex_lock(&wdt->lock);
> +
> +		control = inb(wdt->io_base + attr->offset);
> +		outb((control & ~attr->bit) | (val + 1),
> +		     wdt->io_base + attr->offset);
> +
> +		mutex_unlock(&wdt->lock);
> +
> +	} else if (!strncmp(buf, "none", 4)) {
> +		mutex_lock(&wdt->lock);
> +
> +		control = inb(wdt->io_base + attr->offset);
> +		outb(control & ~attr->bit, wdt->io_base + attr->offset);
> +
> +		mutex_unlock(&wdt->lock);
> +
> +	} else {
> +		return -EINVAL;
> +	}
> +
> +	return size;
> +}
> +
> +#define WDT_TRIG_ATTR(_name, _offset, _bit) \
> +	struct ni7018_attr dev_attr_##_name = { \
> +		.offset = _offset, \
> +		.bit = _bit, \
> +		.dev_attr = \
> +			__ATTR(_name, S_IWUSR | S_IRUGO, \
> +			       wdt_trig_show, wdt_trig_store), \
> +	}
> +
> +static WDT_TRIG_ATTR(trigger, WDT_CTRL, WDT_CTRL_TRIG);
> +static WDT_TRIG_ATTR(keepalive_trigger, WDT_RELOAD_CTRL, WDT_RELOAD_TRIG);
> +
> +static struct attribute *ni7018_wdt_attrs[] = {
> +	&dev_attr_enable_reset.dev_attr.attr,
> +	&dev_attr_enable_soft_ping.dev_attr.attr,
> +	&dev_attr_trigger_polarity.dev_attr.attr,
> +	&dev_attr_keepalive_trigger_polarity.dev_attr.attr,
> +	&dev_attr_trigger.dev_attr.attr,
> +	&dev_attr_keepalive_trigger.dev_attr.attr,
> +	NULL
> +};
> +ATTRIBUTE_GROUPS(ni7018_wdt);
> +
> +static int ni7018_add(struct acpi_device *device)
> +{
> +	struct device *dev = &device->dev;
> +	struct watchdog_device *wdd;
> +	struct ni7018_wdt *wdt;
> +	acpi_status status;
> +	int ret;
> +
> +	wdt = devm_kzalloc(dev, sizeof(*wdt), GFP_KERNEL);
> +	if (!wdt)
> +		return -ENOMEM;
> +
> +	device->driver_data = wdt;
> +	wdt->dev = dev;
> +
> +	status = acpi_walk_resources(device->handle, METHOD_NAME__CRS,
> +				     ni7018_resources, wdt);
> +
> +	if (ACPI_FAILURE(status) || wdt->io_base == 0) {
> +		dev_err(dev, "failed to get resources\n");
> +		return -ENODEV;
> +	}
> +
> +	wdd = &wdt->wdd;
> +	wdd->info = &ni7018_wdd_info;
> +	wdd->ops = &ni7018_wdd_ops;
> +	wdd->min_timeout = WDT_MIN_TIMEOUT;
> +	wdd->max_timeout = WDT_MAX_TIMEOUT;
> +	wdd->timeout = WDT_DEFAULT_TIMEOUT;
> +	wdd->parent = dev;
> +	wdd->groups = ni7018_wdt_groups;
> +
> +	mutex_init(&wdt->lock);
> +	watchdog_set_drvdata(wdd, wdt);
> +	watchdog_set_nowayout(wdd, nowayout);
> +
> +	ret = watchdog_init_timeout(wdd, timeout, dev);
> +	if (ret)
> +		dev_err(dev, "unable to set timeout value, using default\n");
> +
> +	/* Unlock WDT register */
> +	outb(UNLOCK, wdt->io_base + WDT_REG_LOCK);
> +
> +	ret = watchdog_register_device(wdd);
> +	if (ret) {
> +		dev_err(dev, "failed to register watchdog\n");
> +		goto err_out;
> +	}
> +
> +	dev_dbg(dev, "io_base=0x%04X, timeout=%d, nowayout=%d\n",
> +		wdt->io_base, timeout, nowayout);
> +	return 0;
> +
> +err_out:
> +	mutex_destroy(&wdt->lock);
> +	return ret;
> +}
> +
> +static int ni7018_remove(struct acpi_device *device)
> +{
> +	struct ni7018_wdt *wdt = acpi_driver_data(device);
> +
> +	ni7018_stop(&wdt->wdd);
> +	watchdog_unregister_device(&wdt->wdd);
> +
> +	/* Lock WDT register */
> +	outb(LOCK, wdt->io_base + WDT_REG_LOCK);
> +
> +	mutex_destroy(&wdt->lock);
> +	return 0;
> +}
> +
> +static const struct acpi_device_id ni7018_device_ids[] = {
> +	{"NIC7018", 0},
> +	{"", 0},
> +};
> +MODULE_DEVICE_TABLE(acpi, ni7018_device_ids);
> +
> +static struct acpi_driver ni7018_acpi_driver = {
> +	.name = KBUILD_MODNAME,
> +	.ids = ni7018_device_ids,
> +	.ops = {
> +		.add = ni7018_add,
> +		.remove = ni7018_remove,
> +	},
> +};
> +
> +module_acpi_driver(ni7018_acpi_driver);
> +
> +MODULE_DESCRIPTION("NI 7018 Watchdog");
> +MODULE_AUTHOR("Hui Chun Ong <hui.chun.ong@ni.com>");
> +MODULE_LICENSE("GPL v2");
> -- 
> 2.7.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [RESEND PATCH] watchdog: ni7018_wdt: Add NIC7018 watchdog driver
  2016-10-28 17:25 ` Guenter Roeck
@ 2016-11-16  2:52   ` Hui Chun Ong
  0 siblings, 0 replies; 3+ messages in thread
From: Hui Chun Ong @ 2016-11-16  2:52 UTC (permalink / raw)
  To: linux
  Cc: Jonathan Hearn, Julia Cartwright, corbet, wim, linux-watchdog, linux-doc

Hi Guenter,

I'll remove all the attributes for this patch and post a separate patch
with the attributes and explanation for the need and usage of those
attributes.

rgds
Hui Chun

On Fri, 2016-10-28 at 10:25 -0700, Guenter Roeck wrote:
> Hi,
> 
> On Fri, Oct 28, 2016 at 03:51:07PM +0800, Hui Chun Ong wrote:
> > 
> > Add support for the watchdog timer on PXI Embedded Controller.
> > 
> > Signed-off-by: Hui Chun Ong <hui.chun.ong@ni.com>
> The problem with your patch are the non-standard attributes it introduces.
> 
> +static WDT_ATTR(enable_reset, WDT_CTRL, WDT_CTRL_RESET_EN);
> +static WDT_ATTR(enable_soft_ping, WDT_RELOAD_CTRL, WDT_RELOAD_PORT_EN);
> +static WDT_ATTR(trigger_polarity, WDT_CTRL, WDT_CTRL_TRIG_POLARITY);
> +static WDT_ATTR(keepalive_trigger_polarity, WDT_RELOAD_CTRL,
> +               WDT_RELOAD_TRIG_POLARITY);
> 
> At first glance, those appear to be platform configuration parameters,
> which should either be hard-coded or configured through devicetree /
> platform data. Whatever it is, such non-standard attributes consume a
> large amount of time, since reviewers will have to understand what the
> attributes are used for. On top of that, it is likely that we'll probably
> need several rounds of reviews and arguments to determine if the attributes
> are really needed or inappropriate. This all takes a lot of time from
> reviewers. Because of that, patches like this tend to end up at the end
> of my review queue.
> 
> Guenter
> 
> > 
> > ---
> >  Documentation/watchdog/watchdog-parameters.txt |   5 +
> >  drivers/watchdog/Kconfig                       |  11 +
> >  drivers/watchdog/Makefile                      |   1 +
> >  drivers/watchdog/ni7018_wdt.c                  | 506 +++++++++++++++++++++++++
> >  4 files changed, 523 insertions(+)
> >  create mode 100644 drivers/watchdog/ni7018_wdt.c
> > 
> > diff --git a/Documentation/watchdog/watchdog-parameters.txt b/Documentation/watchdog/watchdog-parameters.txt
> > index a8d3642..25dbfa2 100644
> > --- a/Documentation/watchdog/watchdog-parameters.txt
> > +++ b/Documentation/watchdog/watchdog-parameters.txt
> > @@ -204,6 +204,11 @@ mv64x60_wdt:
> >  nowayout: Watchdog cannot be stopped once started
> >  	(default=kernel config parameter)
> >  -------------------------------------------------
> > +ni7018_wdt:
> > +timeout: Initial watchdog timeout in seconds (0
> > +nowayout: Watchdog cannot be stopped once started
> > +	(default=kernel config parameter)
> > +-------------------------------------------------
> >  ni903x_wdt:
> >  timeout: Initial watchdog timeout in seconds (0
> >  nowayout: Watchdog cannot be stopped once started
> > diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> > index fdd3228..6d185af 100644
> > --- a/drivers/watchdog/Kconfig
> > +++ b/drivers/watchdog/Kconfig
> > @@ -1314,6 +1314,17 @@ config INTEL_MEI_WDT
> >  	  To compile this driver as a module, choose M here:
> >  	  the module will be called mei_wdt.
> >  
> > +config NI7018_WDT
> > +	tristate "NI 7018 Watchdog"
> > +	depends on X86 && ACPI
> > +	select WATCHDOG_CORE
> > +	---help---
> > +	  This is the device driver for National Instruments NIC7018 watchdog
> > +	  timer.
> > +
> > +	  To compile this driver as a module, choose M here: the module will be
> > +	  called ni7018_wdt.
> > +
> >  config NI903X_WDT
> >  	tristate "NI 903x/913x Watchdog"
> >  	depends on X86 && ACPI
> > diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
> > index caa9f4a..4605cd2 100644
> > --- a/drivers/watchdog/Makefile
> > +++ b/drivers/watchdog/Makefile
> > @@ -139,6 +139,7 @@ obj-$(CONFIG_INTEL_SCU_WATCHDOG) += intel_scu_watchdog.o
> >  obj-$(CONFIG_INTEL_MID_WATCHDOG) += intel-mid_wdt.o
> >  obj-$(CONFIG_INTEL_MEI_WDT) += mei_wdt.o
> >  obj-$(CONFIG_NI903X_WDT) += ni903x_wdt.o
> > +obj-$(CONFIG_NI7018_WDT) += ni7018_wdt.o
> >  
> >  # M32R Architecture
> >  
> > diff --git a/drivers/watchdog/ni7018_wdt.c b/drivers/watchdog/ni7018_wdt.c
> > new file mode 100644
> > index 0000000..7690314
> > --- /dev/null
> > +++ b/drivers/watchdog/ni7018_wdt.c
> > @@ -0,0 +1,506 @@
> > +/*
> > + * Copyright (C) 2016 National Instruments Corp.
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License as published by
> > + * the Free Software Foundation; either version 2 of the License, or
> > + * (at your option) any later version.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > + * GNU General Public License for more details.
> > + */
> > +
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +#define LOCK		0xA5
> > +#define UNLOCK		0x5A
> > +
> > +#define WDT_CTRL_RESET_EN		BIT(7)
> > +#define WDT_CTRL_TRIG_POLARITY		BIT(4)
> > +#define WDT_CTRL_TRIG			0x0F
> > +
> > +#define WDT_RELOAD_PORT_EN		BIT(7)
> > +#define WDT_RELOAD_TRIG_POLARITY	BIT(6)
> > +#define WDT_RELOAD_TRIG			0x0F
> > +#define WDT_MIN_TRIG_NUM		0
> > +#define WDT_MAX_TRIG_NUM		8
> > +
> > +#define WDT_CTRL		1
> > +#define WDT_RELOAD_CTRL		2
> > +#define WDT_PRESET_PRESCALE	4
> > +#define WDT_REG_LOCK		5
> > +#define WDT_COUNT		6
> > +#define WDT_RELOAD_PORT		7
> > +
> > +#define WDT_IO_SIZE		8
> > +
> > +#define WDT_MIN_TIMEOUT		1
> > +#define WDT_MAX_TIMEOUT		464
> > +#define WDT_DEFAULT_TIMEOUT	80
> > +
> > +#define WDT_MAX_COUNTER		15
> > +
> > +static unsigned int timeout;
> > +module_param(timeout, uint, 0);
> > +MODULE_PARM_DESC(timeout,
> > +		 "Watchdog timeout in seconds. (default="
> > +		 __MODULE_STRING(WDT_DEFAULT_TIMEOUT) ")");
> > +
> > +static int nowayout = WATCHDOG_NOWAYOUT;
> > +module_param(nowayout, int, S_IRUGO);
> > +MODULE_PARM_DESC(nowayout,
> > +		 "Watchdog cannot be stopped once started. (default="
> > +		 __MODULE_STRING(WATCHDOG_NOWAYOUT) ")");
> > +
> > +struct ni7018_wdt {
> > +	struct device *dev;
> > +	u16 io_base;
> > +	u32 period_ms;
> > +	struct mutex lock;
> > +	struct watchdog_device wdd;
> > +};
> > +
> > +struct ni7018_config {
> > +	u32 period_ms;
> > +	u8 divider;
> > +};
> > +
> > +static const struct ni7018_config ni7018_configs[] = {
> > +	{   125, 3 },
> > +	{  2000, 4 },
> > +	{ 32000, 5 },
> > +};
> > +
> > +static inline u32 ni7018_timeout_ms(u32 period_ms, u8 counter)
> > +{
> > +	return period_ms * counter - period_ms / 2;
> > +}
> > +
> > +static const struct ni7018_config *ni7018_get_config(u32 timeout_ms,
> > +						     u8 *counter)
> > +{
> > +	u32 delta, i, best_delta = U32_MAX;
> > +	const struct ni7018_config *config, *best_config = NULL;
> > +	u8 count;
> > +
> > +	for (i = 0; i < ARRAY_SIZE(ni7018_configs); i++) {
> > +		config = &ni7018_configs[i];
> > +
> > +		count = DIV_ROUND_UP(timeout_ms + config->period_ms / 2,
> > +				     config->period_ms);
> > +
> > +		if (count > WDT_MAX_COUNTER)
> > +			continue;
> > +
> > +		delta = ni7018_timeout_ms(config->period_ms, count) -
> > +			timeout_ms;
> > +
> > +		if (delta < best_delta) {
> > +			best_delta = delta;
> > +			best_config = config;
> > +			*counter = count;
> > +		}
> > +	}
> > +	return best_config;
> > +}
> > +
> > +static int ni7018_set_timeout(struct watchdog_device *wdd,
> > +			      unsigned int timeout)
> > +{
> > +	struct ni7018_wdt *wdt = watchdog_get_drvdata(wdd);
> > +	const struct ni7018_config *config;
> > +	u8 counter;
> > +
> > +	config = ni7018_get_config(timeout * 1000, &counter);
> > +	if (!config)
> > +		return -EINVAL;
> > +
> > +	mutex_lock(&wdt->lock);
> > +
> > +	outb(counter << 4 | config->divider,
> > +	     wdt->io_base + WDT_PRESET_PRESCALE);
> > +
> > +	wdd->timeout = ni7018_timeout_ms(config->period_ms, counter) / 1000;
> > +	wdt->period_ms = config->period_ms;
> > +
> > +	mutex_unlock(&wdt->lock);
> > +	return 0;
> > +}
> > +
> > +static int ni7018_start(struct watchdog_device *wdd)
> > +{
> > +	struct ni7018_wdt *wdt = watchdog_get_drvdata(wdd);
> > +	u8 control;
> > +
> > +	ni7018_set_timeout(wdd, wdd->timeout);
> > +
> > +	mutex_lock(&wdt->lock);
> > +
> > +	control = inb(wdt->io_base + WDT_RELOAD_CTRL);
> > +	outb(control | WDT_RELOAD_PORT_EN, wdt->io_base + WDT_RELOAD_CTRL);
> > +
> > +	outb(1, wdt->io_base + WDT_RELOAD_PORT);
> > +
> > +	control = inb(wdt->io_base + WDT_CTRL);
> > +	outb(control | WDT_CTRL_RESET_EN, wdt->io_base + WDT_CTRL);
> > +
> > +	mutex_unlock(&wdt->lock);
> > +	return 0;
> > +}
> > +
> > +static int ni7018_stop(struct watchdog_device *wdd)
> > +{
> > +	struct ni7018_wdt *wdt = watchdog_get_drvdata(wdd);
> > +
> > +	mutex_lock(&wdt->lock);
> > +
> > +	outb(0, wdt->io_base + WDT_CTRL);
> > +	outb(0, wdt->io_base + WDT_RELOAD_CTRL);
> > +	outb(0xF0, wdt->io_base + WDT_PRESET_PRESCALE);
> > +
> > +	mutex_unlock(&wdt->lock);
> > +	return 0;
> > +}
> > +
> > +static int ni7018_ping(struct watchdog_device *wdd)
> > +{
> > +	struct ni7018_wdt *wdt = watchdog_get_drvdata(wdd);
> > +
> > +	mutex_lock(&wdt->lock);
> > +
> > +	outb(1, wdt->io_base + WDT_RELOAD_PORT);
> > +
> > +	mutex_unlock(&wdt->lock);
> > +	return 0;
> > +}
> > +
> > +static unsigned int ni7018_get_timeleft(struct watchdog_device *wdd)
> > +{
> > +	struct ni7018_wdt *wdt = watchdog_get_drvdata(wdd);
> > +	u8 count;
> > +
> > +	mutex_lock(&wdt->lock);
> > +
> > +	count = inb(wdt->io_base + WDT_COUNT) & 0xF;
> > +
> > +	mutex_unlock(&wdt->lock);
> > +
> > +	if (!count)
> > +		return 0;
> > +
> > +	return ni7018_timeout_ms(wdt->period_ms, count) / 1000;
> > +}
> > +
> > +static acpi_status
> > +ni7018_resources(struct acpi_resource *res, void *data)
> > +{
> > +	struct ni7018_wdt *wdt = data;
> > +	u16 io_size;
> > +
> > +	switch (res->type) {
> > +	case ACPI_RESOURCE_TYPE_IO:
> > +		if (wdt->io_base != 0) {
> > +			dev_err(wdt->dev, "too many IO resources\n");
> > +			return AE_ERROR;
> > +		}
> > +
> > +		wdt->io_base = res->data.io.minimum;
> > +		io_size = res->data.io.address_length;
> > +
> > +		if (io_size < WDT_IO_SIZE) {
> > +			dev_err(wdt->dev, "memory region too small\n");
> > +			return AE_ERROR;
> > +		}
> > +
> > +		if (!devm_request_region(wdt->dev, wdt->io_base, io_size,
> > +					 KBUILD_MODNAME)) {
> > +			dev_err(wdt->dev, "failed to get memory region\n");
> > +			return AE_ERROR;
> > +		}
> > +
> > +		return AE_OK;
> > +
> > +	default:
> > +		/* Ignore unsupported resources */
> > +		return AE_OK;
> > +	}
> > +}
> > +
> > +static const struct watchdog_info ni7018_wdd_info = {
> > +	.options = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE,
> > +	.identity = "NI 7018 Watchdog",
> > +};
> > +
> > +static const struct watchdog_ops ni7018_wdd_ops = {
> > +	.owner = THIS_MODULE,
> > +	.start = ni7018_start,
> > +	.stop = ni7018_stop,
> > +	.ping = ni7018_ping,
> > +	.set_timeout = ni7018_set_timeout,
> > +	.get_timeleft = ni7018_get_timeleft,
> > +};
> > +
> > +struct ni7018_attr {
> > +	struct device_attribute dev_attr;
> > +	u8 offset;
> > +	u8 bit;
> > +};
> > +#define to_ni7018_attr(_attr) \
> > +	container_of((_attr), struct ni7018_attr, dev_attr)
> > +
> > +static ssize_t wdt_attr_show(struct device *dev,
> > +			     struct device_attribute *da,
> > +			     char *buf)
> > +{
> > +	struct watchdog_device *wdd = dev_get_drvdata(dev);
> > +	struct ni7018_wdt *wdt = watchdog_get_drvdata(wdd);
> > +	struct ni7018_attr *attr = to_ni7018_attr(da);
> > +	u8 control;
> > +	size_t ret;
> > +
> > +	mutex_lock(&wdt->lock);
> > +
> > +	control = inb(wdt->io_base + attr->offset);
> > +
> > +	mutex_unlock(&wdt->lock);
> > +
> > +	ret = sprintf(buf, "%u\n", !!(control & attr->bit));
> > +	return ret;
> > +}
> > +
> > +static ssize_t wdt_attr_store(struct device *dev,
> > +			      struct device_attribute *da,
> > +			      const char *buf,
> > +			      size_t size)
> > +{
> > +	struct watchdog_device *wdd = dev_get_drvdata(dev);
> > +	struct ni7018_wdt *wdt = watchdog_get_drvdata(wdd);
> > +	struct ni7018_attr *attr = to_ni7018_attr(da);
> > +	unsigned long val;
> > +	u8 control;
> > +
> > +	int ret = kstrtoul(buf, 10, &val);
> > +
> > +	if (ret)
> > +		return ret;
> > +
> > +	if (val < 0 || val > 1)
> > +		return -EINVAL;
> > +
> > +	mutex_lock(&wdt->lock);
> > +
> > +	control = inb(wdt->io_base + attr->offset);
> > +	if (val)
> > +		outb(control | attr->bit, wdt->io_base + attr->offset);
> > +	else
> > +		outb(control & ~attr->bit, wdt->io_base + attr->offset);
> > +
> > +	mutex_unlock(&wdt->lock);
> > +	return size;
> > +}
> > +
> > +#define WDT_ATTR(_name, _offset, _bit) \
> > +	struct ni7018_attr dev_attr_##_name = { \
> > +		.offset = _offset, \
> > +		.bit = _bit, \
> > +		.dev_attr = \
> > +			__ATTR(_name, S_IWUSR | S_IRUGO, \
> > +			       wdt_attr_show, wdt_attr_store), \
> > +	}
> > +
> > +static WDT_ATTR(enable_reset, WDT_CTRL, WDT_CTRL_RESET_EN);
> > +static WDT_ATTR(enable_soft_ping, WDT_RELOAD_CTRL, WDT_RELOAD_PORT_EN);
> > +static WDT_ATTR(trigger_polarity, WDT_CTRL, WDT_CTRL_TRIG_POLARITY);
> > +static WDT_ATTR(keepalive_trigger_polarity, WDT_RELOAD_CTRL,
> > +		WDT_RELOAD_TRIG_POLARITY);
> > +
> > +static ssize_t wdt_trig_show(struct device *dev,
> > +			     struct device_attribute *da,
> > +			     char *buf)
> > +{
> > +	struct watchdog_device *wdd = dev_get_drvdata(dev);
> > +	struct ni7018_wdt *wdt = watchdog_get_drvdata(wdd);
> > +	struct ni7018_attr *attr = to_ni7018_attr(da);
> > +	u8 control;
> > +	size_t ret;
> > +
> > +	mutex_lock(&wdt->lock);
> > +
> > +	control = inb(wdt->io_base + attr->offset);
> > +
> > +	mutex_unlock(&wdt->lock);
> > +
> > +	if (control & attr->bit)
> > +		ret = sprintf(buf, "trig%u\n", (control & attr->bit) - 1);
> > +	else
> > +		ret = sprintf(buf, "none\n");
> > +
> > +	return ret;
> > +}
> > +
> > +static ssize_t wdt_trig_store(struct device *dev,
> > +			      struct device_attribute *da,
> > +			      const char *buf,
> > +			      size_t size)
> > +{
> > +	struct watchdog_device *wdd = dev_get_drvdata(dev);
> > +	struct ni7018_wdt *wdt = watchdog_get_drvdata(wdd);
> > +	struct ni7018_attr *attr = to_ni7018_attr(da);
> > +	unsigned long val;
> > +	u8 control;
> > +	int ret;
> > +
> > +	if (!strncmp(buf, "trig", 4)) {
> > +		ret = kstrtoul(buf + 4, 10, &val);
> > +		if (ret)
> > +			return ret;
> > +
> > +		if (val < WDT_MIN_TRIG_NUM || val > WDT_MAX_TRIG_NUM)
> > +			return -EINVAL;
> > +
> > +		mutex_lock(&wdt->lock);
> > +
> > +		control = inb(wdt->io_base + attr->offset);
> > +		outb((control & ~attr->bit) | (val + 1),
> > +		     wdt->io_base + attr->offset);
> > +
> > +		mutex_unlock(&wdt->lock);
> > +
> > +	} else if (!strncmp(buf, "none", 4)) {
> > +		mutex_lock(&wdt->lock);
> > +
> > +		control = inb(wdt->io_base + attr->offset);
> > +		outb(control & ~attr->bit, wdt->io_base + attr->offset);
> > +
> > +		mutex_unlock(&wdt->lock);
> > +
> > +	} else {
> > +		return -EINVAL;
> > +	}
> > +
> > +	return size;
> > +}
> > +
> > +#define WDT_TRIG_ATTR(_name, _offset, _bit) \
> > +	struct ni7018_attr dev_attr_##_name = { \
> > +		.offset = _offset, \
> > +		.bit = _bit, \
> > +		.dev_attr = \
> > +			__ATTR(_name, S_IWUSR | S_IRUGO, \
> > +			       wdt_trig_show, wdt_trig_store), \
> > +	}
> > +
> > +static WDT_TRIG_ATTR(trigger, WDT_CTRL, WDT_CTRL_TRIG);
> > +static WDT_TRIG_ATTR(keepalive_trigger, WDT_RELOAD_CTRL, WDT_RELOAD_TRIG);
> > +
> > +static struct attribute *ni7018_wdt_attrs[] = {
> > +	&dev_attr_enable_reset.dev_attr.attr,
> > +	&dev_attr_enable_soft_ping.dev_attr.attr,
> > +	&dev_attr_trigger_polarity.dev_attr.attr,
> > +	&dev_attr_keepalive_trigger_polarity.dev_attr.attr,
> > +	&dev_attr_trigger.dev_attr.attr,
> > +	&dev_attr_keepalive_trigger.dev_attr.attr,
> > +	NULL
> > +};
> > +ATTRIBUTE_GROUPS(ni7018_wdt);
> > +
> > +static int ni7018_add(struct acpi_device *device)
> > +{
> > +	struct device *dev = &device->dev;
> > +	struct watchdog_device *wdd;
> > +	struct ni7018_wdt *wdt;
> > +	acpi_status status;
> > +	int ret;
> > +
> > +	wdt = devm_kzalloc(dev, sizeof(*wdt), GFP_KERNEL);
> > +	if (!wdt)
> > +		return -ENOMEM;
> > +
> > +	device->driver_data = wdt;
> > +	wdt->dev = dev;
> > +
> > +	status = acpi_walk_resources(device->handle, METHOD_NAME__CRS,
> > +				     ni7018_resources, wdt);
> > +
> > +	if (ACPI_FAILURE(status) || wdt->io_base == 0) {
> > +		dev_err(dev, "failed to get resources\n");
> > +		return -ENODEV;
> > +	}
> > +
> > +	wdd = &wdt->wdd;
> > +	wdd->info = &ni7018_wdd_info;
> > +	wdd->ops = &ni7018_wdd_ops;
> > +	wdd->min_timeout = WDT_MIN_TIMEOUT;
> > +	wdd->max_timeout = WDT_MAX_TIMEOUT;
> > +	wdd->timeout = WDT_DEFAULT_TIMEOUT;
> > +	wdd->parent = dev;
> > +	wdd->groups = ni7018_wdt_groups;
> > +
> > +	mutex_init(&wdt->lock);
> > +	watchdog_set_drvdata(wdd, wdt);
> > +	watchdog_set_nowayout(wdd, nowayout);
> > +
> > +	ret = watchdog_init_timeout(wdd, timeout, dev);
> > +	if (ret)
> > +		dev_err(dev, "unable to set timeout value, using default\n");
> > +
> > +	/* Unlock WDT register */
> > +	outb(UNLOCK, wdt->io_base + WDT_REG_LOCK);
> > +
> > +	ret = watchdog_register_device(wdd);
> > +	if (ret) {
> > +		dev_err(dev, "failed to register watchdog\n");
> > +		goto err_out;
> > +	}
> > +
> > +	dev_dbg(dev, "io_base=0x%04X, timeout=%d, nowayout=%d\n",
> > +		wdt->io_base, timeout, nowayout);
> > +	return 0;
> > +
> > +err_out:
> > +	mutex_destroy(&wdt->lock);
> > +	return ret;
> > +}
> > +
> > +static int ni7018_remove(struct acpi_device *device)
> > +{
> > +	struct ni7018_wdt *wdt = acpi_driver_data(device);
> > +
> > +	ni7018_stop(&wdt->wdd);
> > +	watchdog_unregister_device(&wdt->wdd);
> > +
> > +	/* Lock WDT register */
> > +	outb(LOCK, wdt->io_base + WDT_REG_LOCK);
> > +
> > +	mutex_destroy(&wdt->lock);
> > +	return 0;
> > +}
> > +
> > +static const struct acpi_device_id ni7018_device_ids[] = {
> > +	{"NIC7018", 0},
> > +	{"", 0},
> > +};
> > +MODULE_DEVICE_TABLE(acpi, ni7018_device_ids);
> > +
> > +static struct acpi_driver ni7018_acpi_driver = {
> > +	.name = KBUILD_MODNAME,
> > +	.ids = ni7018_device_ids,
> > +	.ops = {
> > +		.add = ni7018_add,
> > +		.remove = ni7018_remove,
> > +	},
> > +};
> > +
> > +module_acpi_driver(ni7018_acpi_driver);
> > +
> > +MODULE_DESCRIPTION("NI 7018 Watchdog");
> > +MODULE_AUTHOR("Hui Chun Ong <hui.chun.ong@ni.com>");
> > +MODULE_LICENSE("GPL v2");

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2016-11-16  2:52 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-28  7:51 [RESEND PATCH] watchdog: ni7018_wdt: Add NIC7018 watchdog driver Hui Chun Ong
2016-10-28 17:25 ` Guenter Roeck
2016-11-16  2:52   ` Hui Chun Ong

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.