[PATCH v5 1/2] watchdog: New watchdog driver for MEN A21 watchdogs

[PATCH v5 1/2] watchdog: New watchdog driver for MEN A21 watchdogs

[Johannes Thumshirn]

This patch adds the driver for the watchdog devices found on MEN Mikro
Elektronik A21 VMEbus CPU Carrier Boards. It has DT-support and uses the
watchdog framework.

Revision 2:
* Removed unneeded open flag in struct a21_wdt_drv
* Corrected 3bit reason code from gpio
* Additional sysfs files are now part of watchdog sysfs
* Changed OFF/ON delay in ping from 400ms to 10ns
* Reworked timeout setting
* Removed a21_wdt_ioctl(...)

Revision 3:
* Changed pr_{err,info} to dev_{err,info}
* Removed out of memory error print
* Transition from "fast" to "slow" mode not allowed by chip

Revision 4:
* Remove reboot_notifier and place disable code into platform_device's shutdown function
* Removed sysfs interface

Revision 5:

* Added setting of .bootstatus on driver init
* Added initial timeout on driver init

Signed-off-by: Johannes Thumshirn <johannes.thumshirn@men.de>
---
 MAINTAINERS                   |    6 ++
 drivers/watchdog/Kconfig      |    8 ++
 drivers/watchdog/Makefile     |    1 +
 drivers/watchdog/mena21_wdt.c |  234 +++++++++++++++++++++++++++++++++++++++++
 4 files changed, 249 insertions(+)
 create mode 100644 drivers/watchdog/mena21_wdt.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 7714c3c..023945a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5307,6 +5307,12 @@ F:	drivers/mtd/
 F:	include/linux/mtd/
 F:	include/uapi/mtd/

+MEN A21 WATCHDOG DRIVER
+M:  	Johannes Thumshirn <johannes.thumshirn@men.de>
+L:	linux-watchdog@vger.kernel.org
+S:	Supported
+F:	drivers/watchdog/mena21_wdt.c
+
 METAG ARCHITECTURE
 M:	James Hogan <james.hogan@imgtec.com>
 S:	Supported
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index e89fc31..192b84d 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -1172,6 +1172,14 @@ config BOOKE_WDT_DEFAULT_TIMEOUT

 	  The value can be overridden by the wdt_period command-line parameter.

+config MEN_A21_WDT
+       tristate "MEN A21 VME CPU Carrier Board Watchdog Timer"
+       select WATCHDOG_CORE
+       help
+        Watchdog driver for MEN A21 VMEbus CPU Carrier Boards.
+
+	If unsure select N here.
+
 # PPC64 Architecture

 config WATCHDOG_RTAS
diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
index a300b94..bffdcb1 100644
--- a/drivers/watchdog/Makefile
+++ b/drivers/watchdog/Makefile
@@ -143,6 +143,7 @@ obj-$(CONFIG_8xxx_WDT) += mpc8xxx_wdt.o
 obj-$(CONFIG_MV64X60_WDT) += mv64x60_wdt.o
 obj-$(CONFIG_PIKA_WDT) += pika_wdt.o
 obj-$(CONFIG_BOOKE_WDT) += booke_wdt.o
+obj-$(CONFIG_MEN_A21_WDT) += mena21_wdt.o

 # PPC64 Architecture
 obj-$(CONFIG_WATCHDOG_RTAS) += wdrtas.o
diff --git a/drivers/watchdog/mena21_wdt.c b/drivers/watchdog/mena21_wdt.c
new file mode 100644
index 0000000..e586c2e
--- /dev/null
+++ b/drivers/watchdog/mena21_wdt.c
@@ -0,0 +1,234 @@
+/*
+ * Watchdog driver for the A21 VME CPU Boards
+ *
+ * Copyright (C) 2013 MEN Mikro Elektronik Nuernberg GmbH
+ *
+ * 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
+ */
+#include <linux/module.h>
+#include <linux/moduleparam.h>
+#include <linux/types.h>
+#include <linux/kernel.h>
+#include <linux/slab.h>
+#include <linux/platform_device.h>
+#include <linux/watchdog.h>
+#include <linux/uaccess.h>
+#include <linux/gpio.h>
+#include <linux/delay.h>
+#include <linux/bitops.h>
+
+#define GPIO_WD_ENAB	169
+#define GPIO_WD_FAST	170
+#define GPIO_WD_TRIG	171
+
+#define GPIO_RST_CAUSE_BASE 166
+
+struct a21_wdt_drv {
+	struct watchdog_device wdt;
+	struct mutex lock;
+};
+
+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 a21_wdt_get_bootstatus(unsigned int *reset)
+{
+	int i;
+
+	for (i = 0; i < 3; i++)
+		*reset |= gpio_get_value(GPIO_RST_CAUSE_BASE + i)
+			? (1 << i) : 0;
+
+	if (*reset >= 8)
+		return -EINVAL;
+
+	return 0;
+}
+
+static int a21_wdt_start(struct watchdog_device *wdt)
+{
+	struct a21_wdt_drv *drv = watchdog_get_drvdata(wdt);
+
+	mutex_lock(&drv->lock);
+
+	gpio_set_value(GPIO_WD_ENAB, 1);
+
+	mutex_unlock(&drv->lock);
+
+	return 0;
+}
+
+static int a21_wdt_stop(struct watchdog_device *wdt)
+{
+	struct a21_wdt_drv *drv = watchdog_get_drvdata(wdt);
+
+	/* We don't stop if WDOG_NO_WAY_OUT is set */
+	if (test_bit(WDOG_NO_WAY_OUT, &wdt->status))
+		return -EINVAL;
+
+	mutex_lock(&drv->lock);
+
+	gpio_set_value(GPIO_WD_ENAB, 0);
+
+	mutex_unlock(&drv->lock);
+
+	return 0;
+}
+
+static int a21_wdt_ping(struct watchdog_device *wdt)
+{
+	struct a21_wdt_drv *drv = watchdog_get_drvdata(wdt);
+
+	mutex_lock(&drv->lock);
+
+	gpio_set_value(GPIO_WD_TRIG, 0);
+	ndelay(10);
+	gpio_set_value(GPIO_WD_TRIG, 1);
+
+	mutex_unlock(&drv->lock);
+
+	return 0;
+}
+
+static int a21_wdt_set_timeout(struct watchdog_device *wdt,
+			       unsigned int timeout)
+{
+	struct a21_wdt_drv *drv = watchdog_get_drvdata(wdt);
+
+	if (timeout != 1 && timeout != 30) {
+		dev_err(wdt->dev, "Only 1 and 30 allowed as timeout\n");
+		return -EINVAL;
+	}
+
+	if (timeout == 30 && wdt->timeout == 1) {
+		dev_err(wdt->dev,
+			"Transition from fast to slow mode not allowed\n");
+		return -EINVAL;
+	}
+
+	mutex_lock(&drv->lock);
+
+	if (timeout == 1)
+		gpio_set_value(GPIO_WD_FAST, 1);
+	else
+		gpio_set_value(GPIO_WD_FAST, 0);
+
+	wdt->timeout = timeout;
+
+	mutex_unlock(&drv->lock);
+
+	return 0;
+}
+
+static const struct watchdog_info a21_wdt_info = {
+	.options = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE,
+	.identity = "MEN A21 Watchdog",
+};
+
+static const struct watchdog_ops a21_wdt_ops = {
+	.owner = THIS_MODULE,
+	.start = a21_wdt_start,
+	.stop = a21_wdt_stop,
+	.ping = a21_wdt_ping,
+	.set_timeout = a21_wdt_set_timeout,
+};
+
+static struct watchdog_device a21_wdt = {
+	.info = &a21_wdt_info,
+	.ops = &a21_wdt_ops,
+	.min_timeout = 1,
+	.max_timeout = 30,
+};
+
+static int a21_wdt_probe(struct platform_device *pdev)
+{
+	struct a21_wdt_drv *drv;
+	unsigned int reset = 0;
+	int ret;
+
+	dev_info(&pdev->dev, "MEN A21 watchdog timer driver enabled\n");
+
+	drv = devm_kzalloc(&pdev->dev, sizeof(struct a21_wdt_drv), GFP_KERNEL);
+	if (!drv)
+		return -ENOMEM;
+
+	mutex_init(&drv->lock);
+	watchdog_set_nowayout(&a21_wdt, nowayout);
+	watchdog_set_drvdata(&a21_wdt, drv);
+
+	ret = watchdog_register_device(&a21_wdt);
+	if (ret) {
+		dev_err(&pdev->dev, "Cannot register watchdog device\n");
+		goto err_register_wd;
+	}
+
+	a21_wdt.timeout = 30;
+	ret = a21_wdt_get_bootstatus(&reset);
+	if (ret)
+		dev_warn(&pdev->dev, "Reset Cause contains invalid data\n");
+	else {
+		if (reset == 2)
+			a21_wdt.bootstatus |= WDIOF_EXTERN1;
+		else if (reset == 4)
+			a21_wdt.bootstatus |= WDIOF_CARDRESET;
+		else if (reset == 5)
+			a21_wdt.bootstatus |= WDIOF_POWERUNDER;
+		else if (reset == 7)
+			a21_wdt.bootstatus |= WDIOF_EXTERN2;
+	}
+
+	dev_set_drvdata(&pdev->dev, drv);
+
+	return 0;
+
+err_register_wd:
+	mutex_destroy(&drv->lock);
+
+	return ret;
+}
+
+static int a21_wdt_remove(struct platform_device *pdev)
+{
+	struct a21_wdt_drv *drv = dev_get_drvdata(&pdev->dev);
+
+	dev_warn(&pdev->dev,
+		"Unregistering A21 watchdog driver, board may reboot\n");
+
+
+	watchdog_unregister_device(&drv->wdt);
+
+	mutex_destroy(&drv->lock);
+
+	return 0;
+}
+
+static void a21_wdt_shutdown(struct platform_device *pdev)
+{
+	gpio_set_value(GPIO_WD_ENAB, 0);
+}
+
+static const struct of_device_id a21_wdt_ids[] = {
+	{ .compatible = "men,a021-wdt" },
+	{ },
+};
+
+static struct platform_driver a21_wdt_driver = {
+	.probe = a21_wdt_probe,
+	.remove = a21_wdt_remove,
+	.shutdown = a21_wdt_shutdown,
+	.driver = {
+		.name = "a21-watchdog",
+		.of_match_table = a21_wdt_ids,
+	},
+};
+
+module_platform_driver(a21_wdt_driver);
+
+MODULE_AUTHOR("MEN Mikro Elektronik");
+MODULE_DESCRIPTION("MEN A21 Watchdog");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:a21-watchdog");
--
1.7.9.5

[PATCH v2 2/2] This patch adds a sysfs interface for the watchdog device found on MEN A21 Boards.

[Johannes Thumshirn]

The newly generated files are:
* rebootcause:
Can be one of:
Power on Reset,
CPU Reset Request,
Push Button,
FPGA Reset Request,
Watchdog,
Local Power Bad,
Invalid or
BDI
and shows the reason of the boards last reboot.

* active:
Shows if the watchdog CPLD is actually running

* allow_disable:
Shows if the watchdog is allowed to be disabled (NOWAYOUT disabled)

* fastmode:
Shows if the CPLD is running in fast mode (1s timeout), once it is in
fastmode it can't be switched back to slow mode (30s timeout) until the
next reboot.

Revision 2:
* Re-worked sysfs patch to apply on re-worked base

Signed-off-by: Johannes Thumshirn <johannes.thumshirn@men.de>
---
 drivers/watchdog/mena21_wdt.c |   88 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 88 insertions(+)

diff --git a/drivers/watchdog/mena21_wdt.c b/drivers/watchdog/mena21_wdt.c
index e586c2e..e11d869 100644
--- a/drivers/watchdog/mena21_wdt.c
+++ b/drivers/watchdog/mena21_wdt.c
@@ -19,6 +19,17 @@
 #include <linux/delay.h>
 #include <linux/bitops.h>

+static char *reset_causes[] = {
+	"Power On Reset",
+	"CPU Reset Request",
+	"Push Button",
+	"FPGA Reset Request",
+	"Watchdog",
+	"Local Power Bad",
+	"Invalid",
+	"BDI",
+};
+
 #define GPIO_WD_ENAB	169
 #define GPIO_WD_FAST	170
 #define GPIO_WD_TRIG	171
@@ -49,6 +60,74 @@ static int a21_wdt_get_bootstatus(unsigned int *reset)
 	return 0;
 }

+static ssize_t rebootcause_show(struct device *dev,
+				struct device_attribute *attr,
+				char *buf)
+{
+	unsigned int reset = 0;
+	int i;
+
+	for (i = 0; i < 3; i++)
+		reset |= gpio_get_value(GPIO_RST_CAUSE_BASE + i)
+			 ? (1 << i) : 0;
+
+	if (reset >= 8)
+		return -EIO;
+
+	return sprintf(buf, "%s\n", reset_causes[reset]);
+}
+static DEVICE_ATTR(rebootcause, S_IRUGO, rebootcause_show, NULL);
+
+static ssize_t active_show(struct device *dev, struct device_attribute *attr,
+			   char *buf)
+{
+	return sprintf(buf, "%d\n", !!gpio_get_value(GPIO_WD_ENAB));
+}
+static DEVICE_ATTR(active, S_IRUGO, active_show, NULL);
+
+static ssize_t allow_disable_show(struct device *dev,
+				  struct device_attribute *attr,
+				  char *buf)
+{
+	return sprintf(buf, "%d\n", !nowayout);
+}
+static DEVICE_ATTR(allow_disable, S_IRUGO, allow_disable_show, NULL);
+
+static ssize_t fastmode_show(struct device *dev, struct device_attribute *attr,
+			     char *buf)
+{
+	return sprintf(buf, "%d\n", !!gpio_get_value(GPIO_WD_FAST));
+}
+static DEVICE_ATTR(fastmode, S_IRUGO, fastmode_show, NULL);
+
+static int a21_wdt_create_files(struct watchdog_device *wdev)
+{
+	int ret;
+
+	ret = device_create_file(wdev->dev, &dev_attr_rebootcause);
+	if (ret)
+		return ret;
+	ret = device_create_file(wdev->dev, &dev_attr_active);
+	if (ret)
+		return ret;
+	ret = device_create_file(wdev->dev, &dev_attr_allow_disable);
+	if (ret)
+		return ret;
+	ret = device_create_file(wdev->dev, &dev_attr_fastmode);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static void a21_wdt_remove_files(struct watchdog_device *wdev)
+{
+	device_remove_file(wdev->dev, &dev_attr_rebootcause);
+	device_remove_file(wdev->dev, &dev_attr_active);
+	device_remove_file(wdev->dev, &dev_attr_allow_disable);
+	device_remove_file(wdev->dev, &dev_attr_fastmode);
+}
+
 static int a21_wdt_start(struct watchdog_device *wdt)
 {
 	struct a21_wdt_drv *drv = watchdog_get_drvdata(wdt);
@@ -181,10 +260,18 @@ static int a21_wdt_probe(struct platform_device *pdev)
 			a21_wdt.bootstatus |= WDIOF_EXTERN2;
 	}

+	ret = a21_wdt_create_files(&a21_wdt);
+	if (ret) {
+		dev_err(&pdev->dev, "Cannot create sysfs entries\n");
+		goto err_create_sysfs;
+	}
+
 	dev_set_drvdata(&pdev->dev, drv);

 	return 0;

+err_create_sysfs:
+	watchdog_unregister_device(&drv->wdt);
 err_register_wd:
 	mutex_destroy(&drv->lock);

@@ -198,6 +285,7 @@ static int a21_wdt_remove(struct platform_device *pdev)
 	dev_warn(&pdev->dev,
 		"Unregistering A21 watchdog driver, board may reboot\n");

+	a21_wdt_remove_files(&drv->wdt);

 	watchdog_unregister_device(&drv->wdt);

--
1.7.9.5

Re: [PATCH v5 1/2] watchdog: New watchdog driver for MEN A21 watchdogs

[Johannes Thumshirn]

On Fri, May 31, 2013 at 10:58:48AM +0200, Johannes Thumshirn wrote:
> This patch adds the driver for the watchdog devices found on MEN Mikro
> Elektronik A21 VMEbus CPU Carrier Boards. It has DT-support and uses the
> watchdog framework.
[...]
> +MODULE_DESCRIPTION("MEN A21 Watchdog");
> +MODULE_LICENSE("GPL");
> +MODULE_ALIAS("platform:a21-watchdog");
> --
> 1.7.9.5
>


Args, sorry messed up the thread :-(.

I'm not so firm with using mutt for patches, but git send-email doesn't seem
to like Exchange here...


Johannes

Re: [PATCH v5 1/2] watchdog: New watchdog driver for MEN A21 watchdogs

[Guenter Roeck]

On Fri, May 31, 2013 at 10:58:48AM +0200, Johannes Thumshirn wrote:
> This patch adds the driver for the watchdog devices found on MEN Mikro
> Elektronik A21 VMEbus CPU Carrier Boards. It has DT-support and uses the
> watchdog framework.
> 
> Revision 2:
> * Removed unneeded open flag in struct a21_wdt_drv
> * Corrected 3bit reason code from gpio
> * Additional sysfs files are now part of watchdog sysfs
> * Changed OFF/ON delay in ping from 400ms to 10ns
> * Reworked timeout setting
> * Removed a21_wdt_ioctl(...)
> 
> Revision 3:
> * Changed pr_{err,info} to dev_{err,info}
> * Removed out of memory error print
> * Transition from "fast" to "slow" mode not allowed by chip
> 
> Revision 4:
> * Remove reboot_notifier and place disable code into platform_device's shutdown function
> * Removed sysfs interface
> 
> Revision 5:
> 
> * Added setting of .bootstatus on driver init
> * Added initial timeout on driver init
> 
> Signed-off-by: Johannes Thumshirn <johannes.thumshirn@men.de>
> ---
>  MAINTAINERS                   |    6 ++
>  drivers/watchdog/Kconfig      |    8 ++
>  drivers/watchdog/Makefile     |    1 +
>  drivers/watchdog/mena21_wdt.c |  234 +++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 249 insertions(+)
>  create mode 100644 drivers/watchdog/mena21_wdt.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 7714c3c..023945a 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -5307,6 +5307,12 @@ F:	drivers/mtd/
>  F:	include/linux/mtd/
>  F:	include/uapi/mtd/
> 
> +MEN A21 WATCHDOG DRIVER
> +M:  	Johannes Thumshirn <johannes.thumshirn@men.de>
> +L:	linux-watchdog@vger.kernel.org
> +S:	Supported
> +F:	drivers/watchdog/mena21_wdt.c
> +
>  METAG ARCHITECTURE
>  M:	James Hogan <james.hogan@imgtec.com>
>  S:	Supported
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index e89fc31..192b84d 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -1172,6 +1172,14 @@ config BOOKE_WDT_DEFAULT_TIMEOUT
> 
>  	  The value can be overridden by the wdt_period command-line parameter.
> 
> +config MEN_A21_WDT
> +       tristate "MEN A21 VME CPU Carrier Board Watchdog Timer"
> +       select WATCHDOG_CORE
> +       help
> +        Watchdog driver for MEN A21 VMEbus CPU Carrier Boards.
> +
> +	If unsure select N here.
> +
>  # PPC64 Architecture
> 
>  config WATCHDOG_RTAS
> diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
> index a300b94..bffdcb1 100644
> --- a/drivers/watchdog/Makefile
> +++ b/drivers/watchdog/Makefile
> @@ -143,6 +143,7 @@ obj-$(CONFIG_8xxx_WDT) += mpc8xxx_wdt.o
>  obj-$(CONFIG_MV64X60_WDT) += mv64x60_wdt.o
>  obj-$(CONFIG_PIKA_WDT) += pika_wdt.o
>  obj-$(CONFIG_BOOKE_WDT) += booke_wdt.o
> +obj-$(CONFIG_MEN_A21_WDT) += mena21_wdt.o
> 
>  # PPC64 Architecture
>  obj-$(CONFIG_WATCHDOG_RTAS) += wdrtas.o
> diff --git a/drivers/watchdog/mena21_wdt.c b/drivers/watchdog/mena21_wdt.c
> new file mode 100644
> index 0000000..e586c2e
> --- /dev/null
> +++ b/drivers/watchdog/mena21_wdt.c
> @@ -0,0 +1,234 @@
> +/*
> + * Watchdog driver for the A21 VME CPU Boards
> + *
> + * Copyright (C) 2013 MEN Mikro Elektronik Nuernberg GmbH
> + *
> + * 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
> + */
> +#include <linux/module.h>
> +#include <linux/moduleparam.h>
> +#include <linux/types.h>
> +#include <linux/kernel.h>
> +#include <linux/slab.h>
> +#include <linux/platform_device.h>
> +#include <linux/watchdog.h>
> +#include <linux/uaccess.h>
> +#include <linux/gpio.h>
> +#include <linux/delay.h>
> +#include <linux/bitops.h>
> +
> +#define GPIO_WD_ENAB	169
> +#define GPIO_WD_FAST	170
> +#define GPIO_WD_TRIG	171
> +
> +#define GPIO_RST_CAUSE_BASE 166
> +
> +struct a21_wdt_drv {
> +	struct watchdog_device wdt;
> +	struct mutex lock;
> +};
> +
> +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 a21_wdt_get_bootstatus(unsigned int *reset)
> +{
> +	int i;
> +
> +	for (i = 0; i < 3; i++)
> +		*reset |= gpio_get_value(GPIO_RST_CAUSE_BASE + i)
> +			? (1 << i) : 0;
> +
> +	if (*reset >= 8)
> +		return -EINVAL;
> +
> +	return 0;
> +}
> +
> +static int a21_wdt_start(struct watchdog_device *wdt)
> +{
> +	struct a21_wdt_drv *drv = watchdog_get_drvdata(wdt);
> +
> +	mutex_lock(&drv->lock);
> +
> +	gpio_set_value(GPIO_WD_ENAB, 1);
> +
> +	mutex_unlock(&drv->lock);
> +
> +	return 0;
> +}
> +
> +static int a21_wdt_stop(struct watchdog_device *wdt)
> +{
> +	struct a21_wdt_drv *drv = watchdog_get_drvdata(wdt);
> +
> +	/* We don't stop if WDOG_NO_WAY_OUT is set */
> +	if (test_bit(WDOG_NO_WAY_OUT, &wdt->status))
> +		return -EINVAL;
> +
> +	mutex_lock(&drv->lock);
> +
> +	gpio_set_value(GPIO_WD_ENAB, 0);
> +
> +	mutex_unlock(&drv->lock);
> +
> +	return 0;
> +}
> +
> +static int a21_wdt_ping(struct watchdog_device *wdt)
> +{
> +	struct a21_wdt_drv *drv = watchdog_get_drvdata(wdt);
> +
> +	mutex_lock(&drv->lock);
> +
> +	gpio_set_value(GPIO_WD_TRIG, 0);
> +	ndelay(10);
> +	gpio_set_value(GPIO_WD_TRIG, 1);
> +
> +	mutex_unlock(&drv->lock);
> +
> +	return 0;
> +}
> +
> +static int a21_wdt_set_timeout(struct watchdog_device *wdt,
> +			       unsigned int timeout)
> +{
> +	struct a21_wdt_drv *drv = watchdog_get_drvdata(wdt);
> +
> +	if (timeout != 1 && timeout != 30) {
> +		dev_err(wdt->dev, "Only 1 and 30 allowed as timeout\n");
> +		return -EINVAL;
> +	}
> +
> +	if (timeout == 30 && wdt->timeout == 1) {
> +		dev_err(wdt->dev,
> +			"Transition from fast to slow mode not allowed\n");
> +		return -EINVAL;
> +	}
> +
> +	mutex_lock(&drv->lock);
> +
> +	if (timeout == 1)
> +		gpio_set_value(GPIO_WD_FAST, 1);
> +	else
> +		gpio_set_value(GPIO_WD_FAST, 0);
> +
> +	wdt->timeout = timeout;
> +
> +	mutex_unlock(&drv->lock);
> +
> +	return 0;
> +}
> +
> +static const struct watchdog_info a21_wdt_info = {
> +	.options = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE,
> +	.identity = "MEN A21 Watchdog",
> +};
> +
> +static const struct watchdog_ops a21_wdt_ops = {
> +	.owner = THIS_MODULE,
> +	.start = a21_wdt_start,
> +	.stop = a21_wdt_stop,
> +	.ping = a21_wdt_ping,
> +	.set_timeout = a21_wdt_set_timeout,
> +};
> +
> +static struct watchdog_device a21_wdt = {
> +	.info = &a21_wdt_info,
> +	.ops = &a21_wdt_ops,
> +	.min_timeout = 1,
> +	.max_timeout = 30,
> +};
> +
> +static int a21_wdt_probe(struct platform_device *pdev)
> +{
> +	struct a21_wdt_drv *drv;
> +	unsigned int reset = 0;
> +	int ret;
> +
> +	dev_info(&pdev->dev, "MEN A21 watchdog timer driver enabled\n");
> +
> +	drv = devm_kzalloc(&pdev->dev, sizeof(struct a21_wdt_drv), GFP_KERNEL);
> +	if (!drv)
> +		return -ENOMEM;
> +
> +	mutex_init(&drv->lock);
> +	watchdog_set_nowayout(&a21_wdt, nowayout);
> +	watchdog_set_drvdata(&a21_wdt, drv);
> +
> +	ret = watchdog_register_device(&a21_wdt);
> +	if (ret) {
> +		dev_err(&pdev->dev, "Cannot register watchdog device\n");
> +		goto err_register_wd;
> +	}
> +
> +	a21_wdt.timeout = 30;

It would be better to use watchdog_init_timeout(), as it automatically
supports setting the timeout through an fdt property. It should be set
before registering the device with the watchdog subsystem.

Thanks,
Guenter

> +	ret = a21_wdt_get_bootstatus(&reset);
> +	if (ret)
> +		dev_warn(&pdev->dev, "Reset Cause contains invalid data\n");
> +	else {
> +		if (reset == 2)
> +			a21_wdt.bootstatus |= WDIOF_EXTERN1;
> +		else if (reset == 4)
> +			a21_wdt.bootstatus |= WDIOF_CARDRESET;
> +		else if (reset == 5)
> +			a21_wdt.bootstatus |= WDIOF_POWERUNDER;
> +		else if (reset == 7)
> +			a21_wdt.bootstatus |= WDIOF_EXTERN2;
> +	}
> +
> +	dev_set_drvdata(&pdev->dev, drv);
> +
> +	return 0;
> +
> +err_register_wd:
> +	mutex_destroy(&drv->lock);
> +
> +	return ret;
> +}
> +
> +static int a21_wdt_remove(struct platform_device *pdev)
> +{
> +	struct a21_wdt_drv *drv = dev_get_drvdata(&pdev->dev);
> +
> +	dev_warn(&pdev->dev,
> +		"Unregistering A21 watchdog driver, board may reboot\n");
> +
> +
> +	watchdog_unregister_device(&drv->wdt);
> +
> +	mutex_destroy(&drv->lock);
> +
> +	return 0;
> +}
> +
> +static void a21_wdt_shutdown(struct platform_device *pdev)
> +{
> +	gpio_set_value(GPIO_WD_ENAB, 0);
> +}
> +
> +static const struct of_device_id a21_wdt_ids[] = {
> +	{ .compatible = "men,a021-wdt" },
> +	{ },
> +};
> +
> +static struct platform_driver a21_wdt_driver = {
> +	.probe = a21_wdt_probe,
> +	.remove = a21_wdt_remove,
> +	.shutdown = a21_wdt_shutdown,
> +	.driver = {
> +		.name = "a21-watchdog",
> +		.of_match_table = a21_wdt_ids,
> +	},
> +};
> +
> +module_platform_driver(a21_wdt_driver);
> +
> +MODULE_AUTHOR("MEN Mikro Elektronik");
> +MODULE_DESCRIPTION("MEN A21 Watchdog");
> +MODULE_LICENSE("GPL");
> +MODULE_ALIAS("platform:a21-watchdog");
> --
> 1.7.9.5
> 
> --
> 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
> 

Re: [PATCH v5 1/2] watchdog: New watchdog driver for MEN A21 watchdogs

[Johannes Thumshirn]

On Fri, May 31, 2013 at 03:36:54AM -0700, Guenter Roeck wrote:
> On Fri, May 31, 2013 at 10:58:48AM +0200, Johannes Thumshirn wrote:
> > This patch adds the driver for the watchdog devices found on MEN Mikro
> > Elektronik A21 VMEbus CPU Carrier Boards. It has DT-support and uses the
> > watchdog framework.
> >
> > Revision 2:
> > * Removed unneeded open flag in struct a21_wdt_drv
> > * Corrected 3bit reason code from gpio
> > * Additional sysfs files are now part of watchdog sysfs
> > * Changed OFF/ON delay in ping from 400ms to 10ns
> > * Reworked timeout setting
> > * Removed a21_wdt_ioctl(...)
> >
> > Revision 3:
> > * Changed pr_{err,info} to dev_{err,info}
> > * Removed out of memory error print
> > * Transition from "fast" to "slow" mode not allowed by chip
> >
> > Revision 4:
> > * Remove reboot_notifier and place disable code into platform_device's shutdown function
> > * Removed sysfs interface
> >
> > Revision 5:
> >
> > * Added setting of .bootstatus on driver init
> > * Added initial timeout on driver init
> >
> > Signed-off-by: Johannes Thumshirn <johannes.thumshirn@men.de>
> > ---
> >  MAINTAINERS                   |    6 ++
> >  drivers/watchdog/Kconfig      |    8 ++
> >  drivers/watchdog/Makefile     |    1 +
> >  drivers/watchdog/mena21_wdt.c |  234 +++++++++++++++++++++++++++++++++++++++++
> >  4 files changed, 249 insertions(+)
> >  create mode 100644 drivers/watchdog/mena21_wdt.c
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 7714c3c..023945a 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -5307,6 +5307,12 @@ F:	drivers/mtd/
> >  F:	include/linux/mtd/
> >  F:	include/uapi/mtd/
> >
> > +MEN A21 WATCHDOG DRIVER
> > +M:  	Johannes Thumshirn <johannes.thumshirn@men.de>
> > +L:	linux-watchdog@vger.kernel.org
> > +S:	Supported
> > +F:	drivers/watchdog/mena21_wdt.c
> > +
> >  METAG ARCHITECTURE
> >  M:	James Hogan <james.hogan@imgtec.com>
> >  S:	Supported
> > diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> > index e89fc31..192b84d 100644
> > --- a/drivers/watchdog/Kconfig
> > +++ b/drivers/watchdog/Kconfig
> > @@ -1172,6 +1172,14 @@ config BOOKE_WDT_DEFAULT_TIMEOUT
> >
> >  	  The value can be overridden by the wdt_period command-line parameter.
> >
> > +config MEN_A21_WDT
> > +       tristate "MEN A21 VME CPU Carrier Board Watchdog Timer"
> > +       select WATCHDOG_CORE
> > +       help
> > +        Watchdog driver for MEN A21 VMEbus CPU Carrier Boards.
> > +
> > +	If unsure select N here.
> > +
> >  # PPC64 Architecture
> >
> >  config WATCHDOG_RTAS
> > diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
> > index a300b94..bffdcb1 100644
> > --- a/drivers/watchdog/Makefile
> > +++ b/drivers/watchdog/Makefile
> > @@ -143,6 +143,7 @@ obj-$(CONFIG_8xxx_WDT) += mpc8xxx_wdt.o
> >  obj-$(CONFIG_MV64X60_WDT) += mv64x60_wdt.o
> >  obj-$(CONFIG_PIKA_WDT) += pika_wdt.o
> >  obj-$(CONFIG_BOOKE_WDT) += booke_wdt.o
> > +obj-$(CONFIG_MEN_A21_WDT) += mena21_wdt.o
> >
> >  # PPC64 Architecture
> >  obj-$(CONFIG_WATCHDOG_RTAS) += wdrtas.o
> > diff --git a/drivers/watchdog/mena21_wdt.c b/drivers/watchdog/mena21_wdt.c
> > new file mode 100644
> > index 0000000..e586c2e
> > --- /dev/null
> > +++ b/drivers/watchdog/mena21_wdt.c
> > @@ -0,0 +1,234 @@
> > +/*
> > + * Watchdog driver for the A21 VME CPU Boards
> > + *
> > + * Copyright (C) 2013 MEN Mikro Elektronik Nuernberg GmbH
> > + *
> > + * 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
> > + */
> > +#include <linux/module.h>
> > +#include <linux/moduleparam.h>
> > +#include <linux/types.h>
> > +#include <linux/kernel.h>
> > +#include <linux/slab.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/watchdog.h>
> > +#include <linux/uaccess.h>
> > +#include <linux/gpio.h>
> > +#include <linux/delay.h>
> > +#include <linux/bitops.h>
> > +
> > +#define GPIO_WD_ENAB	169
> > +#define GPIO_WD_FAST	170
> > +#define GPIO_WD_TRIG	171
> > +
> > +#define GPIO_RST_CAUSE_BASE 166
> > +
> > +struct a21_wdt_drv {
> > +	struct watchdog_device wdt;
> > +	struct mutex lock;
> > +};
> > +
> > +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 a21_wdt_get_bootstatus(unsigned int *reset)
> > +{
> > +	int i;
> > +
> > +	for (i = 0; i < 3; i++)
> > +		*reset |= gpio_get_value(GPIO_RST_CAUSE_BASE + i)
> > +			? (1 << i) : 0;
> > +
> > +	if (*reset >= 8)
> > +		return -EINVAL;
> > +
> > +	return 0;
> > +}
> > +
> > +static int a21_wdt_start(struct watchdog_device *wdt)
> > +{
> > +	struct a21_wdt_drv *drv = watchdog_get_drvdata(wdt);
> > +
> > +	mutex_lock(&drv->lock);
> > +
> > +	gpio_set_value(GPIO_WD_ENAB, 1);
> > +
> > +	mutex_unlock(&drv->lock);
> > +
> > +	return 0;
> > +}
> > +
> > +static int a21_wdt_stop(struct watchdog_device *wdt)
> > +{
> > +	struct a21_wdt_drv *drv = watchdog_get_drvdata(wdt);
> > +
> > +	/* We don't stop if WDOG_NO_WAY_OUT is set */
> > +	if (test_bit(WDOG_NO_WAY_OUT, &wdt->status))
> > +		return -EINVAL;
> > +
> > +	mutex_lock(&drv->lock);
> > +
> > +	gpio_set_value(GPIO_WD_ENAB, 0);
> > +
> > +	mutex_unlock(&drv->lock);
> > +
> > +	return 0;
> > +}
> > +
> > +static int a21_wdt_ping(struct watchdog_device *wdt)
> > +{
> > +	struct a21_wdt_drv *drv = watchdog_get_drvdata(wdt);
> > +
> > +	mutex_lock(&drv->lock);
> > +
> > +	gpio_set_value(GPIO_WD_TRIG, 0);
> > +	ndelay(10);
> > +	gpio_set_value(GPIO_WD_TRIG, 1);
> > +
> > +	mutex_unlock(&drv->lock);
> > +
> > +	return 0;
> > +}
> > +
> > +static int a21_wdt_set_timeout(struct watchdog_device *wdt,
> > +			       unsigned int timeout)
> > +{
> > +	struct a21_wdt_drv *drv = watchdog_get_drvdata(wdt);
> > +
> > +	if (timeout != 1 && timeout != 30) {
> > +		dev_err(wdt->dev, "Only 1 and 30 allowed as timeout\n");
> > +		return -EINVAL;
> > +	}
> > +
> > +	if (timeout == 30 && wdt->timeout == 1) {
> > +		dev_err(wdt->dev,
> > +			"Transition from fast to slow mode not allowed\n");
> > +		return -EINVAL;
> > +	}
> > +
> > +	mutex_lock(&drv->lock);
> > +
> > +	if (timeout == 1)
> > +		gpio_set_value(GPIO_WD_FAST, 1);
> > +	else
> > +		gpio_set_value(GPIO_WD_FAST, 0);
> > +
> > +	wdt->timeout = timeout;
> > +
> > +	mutex_unlock(&drv->lock);
> > +
> > +	return 0;
> > +}
> > +
> > +static const struct watchdog_info a21_wdt_info = {
> > +	.options = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE,
> > +	.identity = "MEN A21 Watchdog",
> > +};
> > +
> > +static const struct watchdog_ops a21_wdt_ops = {
> > +	.owner = THIS_MODULE,
> > +	.start = a21_wdt_start,
> > +	.stop = a21_wdt_stop,
> > +	.ping = a21_wdt_ping,
> > +	.set_timeout = a21_wdt_set_timeout,
> > +};
> > +
> > +static struct watchdog_device a21_wdt = {
> > +	.info = &a21_wdt_info,
> > +	.ops = &a21_wdt_ops,
> > +	.min_timeout = 1,
> > +	.max_timeout = 30,
> > +};
> > +
> > +static int a21_wdt_probe(struct platform_device *pdev)
> > +{
> > +	struct a21_wdt_drv *drv;
> > +	unsigned int reset = 0;
> > +	int ret;
> > +
> > +	dev_info(&pdev->dev, "MEN A21 watchdog timer driver enabled\n");
> > +
> > +	drv = devm_kzalloc(&pdev->dev, sizeof(struct a21_wdt_drv), GFP_KERNEL);
> > +	if (!drv)
> > +		return -ENOMEM;
> > +
> > +	mutex_init(&drv->lock);
> > +	watchdog_set_nowayout(&a21_wdt, nowayout);
> > +	watchdog_set_drvdata(&a21_wdt, drv);
> > +
> > +	ret = watchdog_register_device(&a21_wdt);
> > +	if (ret) {
> > +		dev_err(&pdev->dev, "Cannot register watchdog device\n");
> > +		goto err_register_wd;
> > +	}
> > +
> > +	a21_wdt.timeout = 30;
>
> It would be better to use watchdog_init_timeout(), as it automatically
> supports setting the timeout through an fdt property. It should be set
> before registering the device with the watchdog subsystem.

I thought there would be something like watchdog_init_timeout() but didn't find
it actually. Only found watchdog_set_timeout() which isn't exported...

Up for the next update..

[PATCH v6] watchdog: New watchdog driver for MEN A21 watchdogs

[Johannes Thumshirn]

This patch adds the driver for the watchdog devices found on MEN Mikro
Elektronik A21 VMEbus CPU Carrier Boards. It has DT-support and uses the
watchdog framework.

Revision 2:
* Removed unneeded open flag in struct a21_wdt_drv
* Corrected 3bit reason code from gpio
* Additional sysfs files are now part of watchdog sysfs
* Changed OFF/ON delay in ping from 400ms to 10ns
* Reworked timeout setting
* Removed a21_wdt_ioctl(...)

Revision 3:
* Changed pr_{err,info} to dev_{err,info}
* Removed out of memory error print
* Transition from "fast" to "slow" mode not allowed by chip

Revision 4:
* Remove reboot_notifier and place disable code into platform_device's shutdown function
* Removed sysfs interface

Revision 5:

* Added setting of .bootstatus on driver init
* Added initial timeout on driver init

Revision 6:
* Use watchdog_init_timeout() to initialize timeout

Signed-off-by: Johannes Thumshirn <johannes.thumshirn@men.de>
---
 MAINTAINERS                   |    6 ++
 drivers/watchdog/Kconfig      |    8 ++
 drivers/watchdog/Makefile     |    1 +
 drivers/watchdog/mena21_wdt.c |  234 +++++++++++++++++++++++++++++++++++++++++
 4 files changed, 249 insertions(+)
 create mode 100644 drivers/watchdog/mena21_wdt.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 7714c3c..023945a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5307,6 +5307,12 @@ F:	drivers/mtd/
 F:	include/linux/mtd/
 F:	include/uapi/mtd/

+MEN A21 WATCHDOG DRIVER
+M:  	Johannes Thumshirn <johannes.thumshirn@men.de>
+L:	linux-watchdog@vger.kernel.org
+S:	Supported
+F:	drivers/watchdog/mena21_wdt.c
+
 METAG ARCHITECTURE
 M:	James Hogan <james.hogan@imgtec.com>
 S:	Supported
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index e89fc31..192b84d 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -1172,6 +1172,14 @@ config BOOKE_WDT_DEFAULT_TIMEOUT

 	  The value can be overridden by the wdt_period command-line parameter.

+config MEN_A21_WDT
+       tristate "MEN A21 VME CPU Carrier Board Watchdog Timer"
+       select WATCHDOG_CORE
+       help
+        Watchdog driver for MEN A21 VMEbus CPU Carrier Boards.
+
+	If unsure select N here.
+
 # PPC64 Architecture

 config WATCHDOG_RTAS
diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
index a300b94..bffdcb1 100644
--- a/drivers/watchdog/Makefile
+++ b/drivers/watchdog/Makefile
@@ -143,6 +143,7 @@ obj-$(CONFIG_8xxx_WDT) += mpc8xxx_wdt.o
 obj-$(CONFIG_MV64X60_WDT) += mv64x60_wdt.o
 obj-$(CONFIG_PIKA_WDT) += pika_wdt.o
 obj-$(CONFIG_BOOKE_WDT) += booke_wdt.o
+obj-$(CONFIG_MEN_A21_WDT) += mena21_wdt.o

 # PPC64 Architecture
 obj-$(CONFIG_WATCHDOG_RTAS) += wdrtas.o
diff --git a/drivers/watchdog/mena21_wdt.c b/drivers/watchdog/mena21_wdt.c
new file mode 100644
index 0000000..9874c63
--- /dev/null
+++ b/drivers/watchdog/mena21_wdt.c
@@ -0,0 +1,234 @@
+/*
+ * Watchdog driver for the A21 VME CPU Boards
+ *
+ * Copyright (C) 2013 MEN Mikro Elektronik Nuernberg GmbH
+ *
+ * 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
+ */
+#include <linux/module.h>
+#include <linux/moduleparam.h>
+#include <linux/types.h>
+#include <linux/kernel.h>
+#include <linux/slab.h>
+#include <linux/platform_device.h>
+#include <linux/watchdog.h>
+#include <linux/uaccess.h>
+#include <linux/gpio.h>
+#include <linux/delay.h>
+#include <linux/bitops.h>
+
+#define GPIO_WD_ENAB	169
+#define GPIO_WD_FAST	170
+#define GPIO_WD_TRIG	171
+
+#define GPIO_RST_CAUSE_BASE 166
+
+struct a21_wdt_drv {
+	struct watchdog_device wdt;
+	struct mutex lock;
+};
+
+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 a21_wdt_get_bootstatus(unsigned int *reset)
+{
+	int i;
+
+	for (i = 0; i < 3; i++)
+		*reset |= gpio_get_value(GPIO_RST_CAUSE_BASE + i)
+			? (1 << i) : 0;
+
+	if (*reset >= 8)
+		return -EINVAL;
+
+	return 0;
+}
+
+static int a21_wdt_start(struct watchdog_device *wdt)
+{
+	struct a21_wdt_drv *drv = watchdog_get_drvdata(wdt);
+
+	mutex_lock(&drv->lock);
+
+	gpio_set_value(GPIO_WD_ENAB, 1);
+
+	mutex_unlock(&drv->lock);
+
+	return 0;
+}
+
+static int a21_wdt_stop(struct watchdog_device *wdt)
+{
+	struct a21_wdt_drv *drv = watchdog_get_drvdata(wdt);
+
+	/* We don't stop if WDOG_NO_WAY_OUT is set */
+	if (test_bit(WDOG_NO_WAY_OUT, &wdt->status))
+		return -EINVAL;
+
+	mutex_lock(&drv->lock);
+
+	gpio_set_value(GPIO_WD_ENAB, 0);
+
+	mutex_unlock(&drv->lock);
+
+	return 0;
+}
+
+static int a21_wdt_ping(struct watchdog_device *wdt)
+{
+	struct a21_wdt_drv *drv = watchdog_get_drvdata(wdt);
+
+	mutex_lock(&drv->lock);
+
+	gpio_set_value(GPIO_WD_TRIG, 0);
+	ndelay(10);
+	gpio_set_value(GPIO_WD_TRIG, 1);
+
+	mutex_unlock(&drv->lock);
+
+	return 0;
+}
+
+static int a21_wdt_set_timeout(struct watchdog_device *wdt,
+			       unsigned int timeout)
+{
+	struct a21_wdt_drv *drv = watchdog_get_drvdata(wdt);
+
+	if (timeout != 1 && timeout != 30) {
+		dev_err(wdt->dev, "Only 1 and 30 allowed as timeout\n");
+		return -EINVAL;
+	}
+
+	if (timeout == 30 && wdt->timeout == 1) {
+		dev_err(wdt->dev,
+			"Transition from fast to slow mode not allowed\n");
+		return -EINVAL;
+	}
+
+	mutex_lock(&drv->lock);
+
+	if (timeout == 1)
+		gpio_set_value(GPIO_WD_FAST, 1);
+	else
+		gpio_set_value(GPIO_WD_FAST, 0);
+
+	wdt->timeout = timeout;
+
+	mutex_unlock(&drv->lock);
+
+	return 0;
+}
+
+static const struct watchdog_info a21_wdt_info = {
+	.options = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE,
+	.identity = "MEN A21 Watchdog",
+};
+
+static const struct watchdog_ops a21_wdt_ops = {
+	.owner = THIS_MODULE,
+	.start = a21_wdt_start,
+	.stop = a21_wdt_stop,
+	.ping = a21_wdt_ping,
+	.set_timeout = a21_wdt_set_timeout,
+};
+
+static struct watchdog_device a21_wdt = {
+	.info = &a21_wdt_info,
+	.ops = &a21_wdt_ops,
+	.min_timeout = 1,
+	.max_timeout = 30,
+};
+
+static int a21_wdt_probe(struct platform_device *pdev)
+{
+	struct a21_wdt_drv *drv;
+	unsigned int reset = 0;
+	int ret;
+
+	dev_info(&pdev->dev, "MEN A21 watchdog timer driver enabled\n");
+
+	drv = devm_kzalloc(&pdev->dev, sizeof(struct a21_wdt_drv), GFP_KERNEL);
+	if (!drv)
+		return -ENOMEM;
+
+	mutex_init(&drv->lock);
+	watchdog_init_timeout(&a21_wdt, 30, &pdev->dev);
+	watchdog_set_nowayout(&a21_wdt, nowayout);
+	watchdog_set_drvdata(&a21_wdt, drv);
+
+	ret = watchdog_register_device(&a21_wdt);
+	if (ret) {
+		dev_err(&pdev->dev, "Cannot register watchdog device\n");
+		goto err_register_wd;
+	}
+
+	ret = a21_wdt_get_bootstatus(&reset);
+	if (ret)
+		dev_warn(&pdev->dev, "Reset Cause contains invalid data\n");
+	else {
+		if (reset == 2)
+			a21_wdt.bootstatus |= WDIOF_EXTERN1;
+		else if (reset == 4)
+			a21_wdt.bootstatus |= WDIOF_CARDRESET;
+		else if (reset == 5)
+			a21_wdt.bootstatus |= WDIOF_POWERUNDER;
+		else if (reset == 7)
+			a21_wdt.bootstatus |= WDIOF_EXTERN2;
+	}
+
+	dev_set_drvdata(&pdev->dev, drv);
+
+	return 0;
+
+err_register_wd:
+	mutex_destroy(&drv->lock);
+
+	return ret;
+}
+
+static int a21_wdt_remove(struct platform_device *pdev)
+{
+	struct a21_wdt_drv *drv = dev_get_drvdata(&pdev->dev);
+
+	dev_warn(&pdev->dev,
+		"Unregistering A21 watchdog driver, board may reboot\n");
+
+
+	watchdog_unregister_device(&drv->wdt);
+
+	mutex_destroy(&drv->lock);
+
+	return 0;
+}
+
+static void a21_wdt_shutdown(struct platform_device *pdev)
+{
+	gpio_set_value(GPIO_WD_ENAB, 0);
+}
+
+static const struct of_device_id a21_wdt_ids[] = {
+	{ .compatible = "men,a021-wdt" },
+	{ },
+};
+
+static struct platform_driver a21_wdt_driver = {
+	.probe = a21_wdt_probe,
+	.remove = a21_wdt_remove,
+	.shutdown = a21_wdt_shutdown,
+	.driver = {
+		.name = "a21-watchdog",
+		.of_match_table = a21_wdt_ids,
+	},
+};
+
+module_platform_driver(a21_wdt_driver);
+
+MODULE_AUTHOR("MEN Mikro Elektronik");
+MODULE_DESCRIPTION("MEN A21 Watchdog");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:a21-watchdog");
--
1.7.9.5

Re: [PATCH v6] watchdog: New watchdog driver for MEN A21 watchdogs

[Guenter Roeck]

On Fri, May 31, 2013 at 01:08:52PM +0200, Johannes Thumshirn wrote:
> This patch adds the driver for the watchdog devices found on MEN Mikro
> Elektronik A21 VMEbus CPU Carrier Boards. It has DT-support and uses the
> watchdog framework.
> 
> Revision 2:
> * Removed unneeded open flag in struct a21_wdt_drv
> * Corrected 3bit reason code from gpio
> * Additional sysfs files are now part of watchdog sysfs
> * Changed OFF/ON delay in ping from 400ms to 10ns
> * Reworked timeout setting
> * Removed a21_wdt_ioctl(...)
> 
> Revision 3:
> * Changed pr_{err,info} to dev_{err,info}
> * Removed out of memory error print
> * Transition from "fast" to "slow" mode not allowed by chip
> 
> Revision 4:
> * Remove reboot_notifier and place disable code into platform_device's shutdown function
> * Removed sysfs interface
> 
> Revision 5:
> 
> * Added setting of .bootstatus on driver init
> * Added initial timeout on driver init
> 
> Revision 6:
> * Use watchdog_init_timeout() to initialize timeout
> 
> Signed-off-by: Johannes Thumshirn <johannes.thumshirn@men.de>

Couple of additional comments ... sorry, middle of night here and my thinking is
not at its best.

> ---
>  MAINTAINERS                   |    6 ++
>  drivers/watchdog/Kconfig      |    8 ++
>  drivers/watchdog/Makefile     |    1 +
>  drivers/watchdog/mena21_wdt.c |  234 +++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 249 insertions(+)
>  create mode 100644 drivers/watchdog/mena21_wdt.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 7714c3c..023945a 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -5307,6 +5307,12 @@ F:	drivers/mtd/
>  F:	include/linux/mtd/
>  F:	include/uapi/mtd/
> 
> +MEN A21 WATCHDOG DRIVER
> +M:  	Johannes Thumshirn <johannes.thumshirn@men.de>
> +L:	linux-watchdog@vger.kernel.org
> +S:	Supported
> +F:	drivers/watchdog/mena21_wdt.c
> +
>  METAG ARCHITECTURE
>  M:	James Hogan <james.hogan@imgtec.com>
>  S:	Supported
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index e89fc31..192b84d 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -1172,6 +1172,14 @@ config BOOKE_WDT_DEFAULT_TIMEOUT
> 
>  	  The value can be overridden by the wdt_period command-line parameter.
> 
> +config MEN_A21_WDT
> +       tristate "MEN A21 VME CPU Carrier Board Watchdog Timer"
> +       select WATCHDOG_CORE
> +       help
> +        Watchdog driver for MEN A21 VMEbus CPU Carrier Boards.
> +
> +	If unsure select N here.
> +
>  # PPC64 Architecture
> 
>  config WATCHDOG_RTAS
> diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
> index a300b94..bffdcb1 100644
> --- a/drivers/watchdog/Makefile
> +++ b/drivers/watchdog/Makefile
> @@ -143,6 +143,7 @@ obj-$(CONFIG_8xxx_WDT) += mpc8xxx_wdt.o
>  obj-$(CONFIG_MV64X60_WDT) += mv64x60_wdt.o
>  obj-$(CONFIG_PIKA_WDT) += pika_wdt.o
>  obj-$(CONFIG_BOOKE_WDT) += booke_wdt.o
> +obj-$(CONFIG_MEN_A21_WDT) += mena21_wdt.o
> 
>  # PPC64 Architecture
>  obj-$(CONFIG_WATCHDOG_RTAS) += wdrtas.o
> diff --git a/drivers/watchdog/mena21_wdt.c b/drivers/watchdog/mena21_wdt.c
> new file mode 100644
> index 0000000..9874c63
> --- /dev/null
> +++ b/drivers/watchdog/mena21_wdt.c
> @@ -0,0 +1,234 @@
> +/*
> + * Watchdog driver for the A21 VME CPU Boards
> + *
> + * Copyright (C) 2013 MEN Mikro Elektronik Nuernberg GmbH
> + *
> + * 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
> + */
> +#include <linux/module.h>
> +#include <linux/moduleparam.h>
> +#include <linux/types.h>
> +#include <linux/kernel.h>
> +#include <linux/slab.h>
> +#include <linux/platform_device.h>
> +#include <linux/watchdog.h>
> +#include <linux/uaccess.h>
> +#include <linux/gpio.h>
> +#include <linux/delay.h>
> +#include <linux/bitops.h>
> +
> +#define GPIO_WD_ENAB	169
> +#define GPIO_WD_FAST	170
> +#define GPIO_WD_TRIG	171
> +
> +#define GPIO_RST_CAUSE_BASE 166
> +

I think I asked that before ... as you are supporting devicetree, gpio pins
should really be provided through devicetree properties and not be hardcoded.

> +struct a21_wdt_drv {
> +	struct watchdog_device wdt;
> +	struct mutex lock;
> +};
> +
> +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 a21_wdt_get_bootstatus(unsigned int *reset)
> +{
> +	int i;
> +
> +	for (i = 0; i < 3; i++)
> +		*reset |= gpio_get_value(GPIO_RST_CAUSE_BASE + i)
> +			? (1 << i) : 0;
> +
> +	if (*reset >= 8)
> +		return -EINVAL;
> +
> +	return 0;
> +}
> +
> +static int a21_wdt_start(struct watchdog_device *wdt)
> +{
> +	struct a21_wdt_drv *drv = watchdog_get_drvdata(wdt);
> +
> +	mutex_lock(&drv->lock);
> +
> +	gpio_set_value(GPIO_WD_ENAB, 1);
> +
> +	mutex_unlock(&drv->lock);
> +
> +	return 0;
> +}
> +
> +static int a21_wdt_stop(struct watchdog_device *wdt)
> +{
> +	struct a21_wdt_drv *drv = watchdog_get_drvdata(wdt);
> +
> +	/* We don't stop if WDOG_NO_WAY_OUT is set */
> +	if (test_bit(WDOG_NO_WAY_OUT, &wdt->status))
> +		return -EINVAL;
> +
> +	mutex_lock(&drv->lock);
> +
> +	gpio_set_value(GPIO_WD_ENAB, 0);
> +
> +	mutex_unlock(&drv->lock);
> +
> +	return 0;
> +}
> +
> +static int a21_wdt_ping(struct watchdog_device *wdt)
> +{
> +	struct a21_wdt_drv *drv = watchdog_get_drvdata(wdt);
> +
> +	mutex_lock(&drv->lock);
> +
> +	gpio_set_value(GPIO_WD_TRIG, 0);
> +	ndelay(10);
> +	gpio_set_value(GPIO_WD_TRIG, 1);
> +
> +	mutex_unlock(&drv->lock);
> +
> +	return 0;
> +}
> +
> +static int a21_wdt_set_timeout(struct watchdog_device *wdt,
> +			       unsigned int timeout)
> +{
> +	struct a21_wdt_drv *drv = watchdog_get_drvdata(wdt);
> +
> +	if (timeout != 1 && timeout != 30) {
> +		dev_err(wdt->dev, "Only 1 and 30 allowed as timeout\n");
> +		return -EINVAL;
> +	}
> +
> +	if (timeout == 30 && wdt->timeout == 1) {
> +		dev_err(wdt->dev,
> +			"Transition from fast to slow mode not allowed\n");
> +		return -EINVAL;
> +	}
> +
I dislike that [2 .. 29] timeout values are not supported, and would rather have
you accept that range and set the timeout to 30. Also, it is somewhat undesirable
that the timeout can not be changed back to 30 after being set to 1. 

As Wim recommended, a softdog on top of the hardware watchdog would be the best
solution. I'll leave it up to him to decide if he wants to accept the code
as-is.

> +	mutex_lock(&drv->lock);
> +
> +	if (timeout == 1)
> +		gpio_set_value(GPIO_WD_FAST, 1);
> +	else
> +		gpio_set_value(GPIO_WD_FAST, 0);
> +
> +	wdt->timeout = timeout;
> +
> +	mutex_unlock(&drv->lock);
> +
> +	return 0;
> +}
> +
> +static const struct watchdog_info a21_wdt_info = {
> +	.options = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE,
> +	.identity = "MEN A21 Watchdog",
> +};
> +
> +static const struct watchdog_ops a21_wdt_ops = {
> +	.owner = THIS_MODULE,
> +	.start = a21_wdt_start,
> +	.stop = a21_wdt_stop,
> +	.ping = a21_wdt_ping,
> +	.set_timeout = a21_wdt_set_timeout,
> +};
> +
> +static struct watchdog_device a21_wdt = {
> +	.info = &a21_wdt_info,
> +	.ops = &a21_wdt_ops,
> +	.min_timeout = 1,
> +	.max_timeout = 30,
> +};
> +
> +static int a21_wdt_probe(struct platform_device *pdev)
> +{
> +	struct a21_wdt_drv *drv;
> +	unsigned int reset = 0;
> +	int ret;
> +
> +	dev_info(&pdev->dev, "MEN A21 watchdog timer driver enabled\n");
> +
> +	drv = devm_kzalloc(&pdev->dev, sizeof(struct a21_wdt_drv), GFP_KERNEL);
> +	if (!drv)
> +		return -ENOMEM;
> +
> +	mutex_init(&drv->lock);
> +	watchdog_init_timeout(&a21_wdt, 30, &pdev->dev);
> +	watchdog_set_nowayout(&a21_wdt, nowayout);
> +	watchdog_set_drvdata(&a21_wdt, drv);
> +
> +	ret = watchdog_register_device(&a21_wdt);
> +	if (ret) {
> +		dev_err(&pdev->dev, "Cannot register watchdog device\n");
> +		goto err_register_wd;
> +	}
> +
> +	ret = a21_wdt_get_bootstatus(&reset);
> +	if (ret)
> +		dev_warn(&pdev->dev, "Reset Cause contains invalid data\n");
> +	else {
> +		if (reset == 2)
> +			a21_wdt.bootstatus |= WDIOF_EXTERN1;
> +		else if (reset == 4)
> +			a21_wdt.bootstatus |= WDIOF_CARDRESET;
> +		else if (reset == 5)
> +			a21_wdt.bootstatus |= WDIOF_POWERUNDER;
> +		else if (reset == 7)
> +			a21_wdt.bootstatus |= WDIOF_EXTERN2;

What about other causes ? No useful match ?

I think bootstatus should be set prior to registering the watchdog device
to avoid race conditions where an application reads it prior to being set.

> +	}
> +
> +	dev_set_drvdata(&pdev->dev, drv);
> +
> +	return 0;
> +
> +err_register_wd:
> +	mutex_destroy(&drv->lock);
> +
> +	return ret;
> +}
> +
> +static int a21_wdt_remove(struct platform_device *pdev)
> +{
> +	struct a21_wdt_drv *drv = dev_get_drvdata(&pdev->dev);
> +
> +	dev_warn(&pdev->dev,
> +		"Unregistering A21 watchdog driver, board may reboot\n");
> +
> +
> +	watchdog_unregister_device(&drv->wdt);
> +
> +	mutex_destroy(&drv->lock);
> +
> +	return 0;
> +}
> +
> +static void a21_wdt_shutdown(struct platform_device *pdev)
> +{
> +	gpio_set_value(GPIO_WD_ENAB, 0);
> +}
> +
> +static const struct of_device_id a21_wdt_ids[] = {
> +	{ .compatible = "men,a021-wdt" },
> +	{ },
> +};
> +
> +static struct platform_driver a21_wdt_driver = {
> +	.probe = a21_wdt_probe,
> +	.remove = a21_wdt_remove,
> +	.shutdown = a21_wdt_shutdown,
> +	.driver = {
> +		.name = "a21-watchdog",
> +		.of_match_table = a21_wdt_ids,
> +	},
> +};
> +
> +module_platform_driver(a21_wdt_driver);
> +
> +MODULE_AUTHOR("MEN Mikro Elektronik");
> +MODULE_DESCRIPTION("MEN A21 Watchdog");
> +MODULE_LICENSE("GPL");
> +MODULE_ALIAS("platform:a21-watchdog");
> --
> 1.7.9.5
> 
> --
> 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
> 

Re: [PATCH v6] watchdog: New watchdog driver for MEN A21 watchdogs

[Johannes Thumshirn]

Hi Guenther,
On Fri, May 31, 2013 at 04:40:37AM -0700, Guenter Roeck wrote:
> > +#define GPIO_WD_ENAB	169
> > +#define GPIO_WD_FAST	170
> > +#define GPIO_WD_TRIG	171
> > +
> > +#define GPIO_RST_CAUSE_BASE 166
> > +
>
> I think I asked that before ... as you are supporting devicetree, gpio pins
> should really be provided through devicetree properties and not be hardcoded.
>
Yes you did and I didn't come up with a solution to this problem yet. I understand
and agree to your concerns but I'm lacking example code/documentation for it, maybe
you can point me to an example on that and then I'll update my code accordingly.

> > +struct a21_wdt_drv {
> > +	struct watchdog_device wdt;
> > +	struct mutex lock;
> > +	}
> > +
> > +	if (timeout == 30 && wdt->timeout == 1) {
> > +		dev_err(wdt->dev,
> > +			"Transition from fast to slow mode not allowed\n");
> > +		return -EINVAL;
> > +	}
> > +
> I dislike that [2 .. 29] timeout values are not supported, and would rather have
> you accept that range and set the timeout to 30. Also, it is somewhat undesirable
> that the timeout can not be changed back to 30 after being set to 1.

The not changing back issue was a hardware design criterion and I have no
influence on that one. I've already checked with the IC developer and he said,
this is intended. I don't quite understand why I should accept the timeout and
set it to 30? In my opinion this only introduces some magic the user won't
understand. Instead failing and giving an error message is something a user can
take as a hint for investigating.

> As Wim recommended, a softdog on top of the hardware watchdog would be the best
> solution. I'll leave it up to him to decide if he wants to accept the code
> as-is.

Yes let's see what he's saying.

>
> > +	mutex_lock(&drv->lock);
> > +
> > +	ret = watchdog_register_device(&a21_wdt);
> > +	if (ret) {
> > +		dev_err(&pdev->dev, "Cannot register watchdog device\n");
> > +		goto err_register_wd;
> > +	}
> > +
> > +	ret = a21_wdt_get_bootstatus(&reset);
> > +	if (ret)
> > +		dev_warn(&pdev->dev, "Reset Cause contains invalid data\n");
> > +	else {
> > +		if (reset == 2)
> > +			a21_wdt.bootstatus |= WDIOF_EXTERN1;
> > +		else if (reset == 4)
> > +			a21_wdt.bootstatus |= WDIOF_CARDRESET;
> > +		else if (reset == 5)
> > +			a21_wdt.bootstatus |= WDIOF_POWERUNDER;
> > +		else if (reset == 7)
> > +			a21_wdt.bootstatus |= WDIOF_EXTERN2;
>
> What about other causes ? No useful match ?
>

None I could find. Actually WDIOF_EXTERN[12] already are a pretty creative
mapping in my opinion. EXTERN1 is a "Push Button" event, EXTERN2 is a reset
caused by a JTAG/BDM adapter...

> I think bootstatus should be set prior to registering the watchdog device
> to avoid race conditions where an application reads it prior to being set.
>
Agreed, didn't see that one *doh*, must be fixed before inclusion.
> > +	}

[PATCH v7] watchdog: New watchdog driver for MEN A21 watchdogs

[Johannes Thumshirn]

This patch adds the driver for the watchdog devices found on MEN Mikro
Elektronik A21 VMEbus CPU Carrier Boards. It has DT-support and uses the
watchdog framework.

Revision 2:
* Removed unneeded open flag in struct a21_wdt_drv
* Corrected 3bit reason code from gpio
* Additional sysfs files are now part of watchdog sysfs
* Changed OFF/ON delay in ping from 400ms to 10ns
* Reworked timeout setting
* Removed a21_wdt_ioctl(...)

Revision 3:
* Changed pr_{err,info} to dev_{err,info}
* Removed out of memory error print
* Transition from "fast" to "slow" mode not allowed by chip

Revision 4:
* Remove reboot_notifier and place disable code into platform_device's shutdown function
* Removed sysfs interface

Revision 5:

* Added setting of .bootstatus on driver init
* Added initial timeout on driver init

Revision 6:
* Use watchdog_init_timeout() to initialize timeout

Revision 7:
* Fix possible get_bootstatus race condition

Signed-off-by: Johannes Thumshirn <johannes.thumshirn@men.de>
---
 MAINTAINERS                   |    6 ++
 drivers/watchdog/Kconfig      |    8 ++
 drivers/watchdog/Makefile     |    1 +
 drivers/watchdog/mena21_wdt.c |  234 +++++++++++++++++++++++++++++++++++++++++
 4 files changed, 249 insertions(+)
 create mode 100644 drivers/watchdog/mena21_wdt.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 7714c3c..023945a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5307,6 +5307,12 @@ F:	drivers/mtd/
 F:	include/linux/mtd/
 F:	include/uapi/mtd/

+MEN A21 WATCHDOG DRIVER
+M:  	Johannes Thumshirn <johannes.thumshirn@men.de>
+L:	linux-watchdog@vger.kernel.org
+S:	Supported
+F:	drivers/watchdog/mena21_wdt.c
+
 METAG ARCHITECTURE
 M:	James Hogan <james.hogan@imgtec.com>
 S:	Supported
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index e89fc31..192b84d 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -1172,6 +1172,14 @@ config BOOKE_WDT_DEFAULT_TIMEOUT

 	  The value can be overridden by the wdt_period command-line parameter.

+config MEN_A21_WDT
+       tristate "MEN A21 VME CPU Carrier Board Watchdog Timer"
+       select WATCHDOG_CORE
+       help
+        Watchdog driver for MEN A21 VMEbus CPU Carrier Boards.
+
+	If unsure select N here.
+
 # PPC64 Architecture

 config WATCHDOG_RTAS
diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
index a300b94..bffdcb1 100644
--- a/drivers/watchdog/Makefile
+++ b/drivers/watchdog/Makefile
@@ -143,6 +143,7 @@ obj-$(CONFIG_8xxx_WDT) += mpc8xxx_wdt.o
 obj-$(CONFIG_MV64X60_WDT) += mv64x60_wdt.o
 obj-$(CONFIG_PIKA_WDT) += pika_wdt.o
 obj-$(CONFIG_BOOKE_WDT) += booke_wdt.o
+obj-$(CONFIG_MEN_A21_WDT) += mena21_wdt.o

 # PPC64 Architecture
 obj-$(CONFIG_WATCHDOG_RTAS) += wdrtas.o
diff --git a/drivers/watchdog/mena21_wdt.c b/drivers/watchdog/mena21_wdt.c
new file mode 100644
index 0000000..dec35ec
--- /dev/null
+++ b/drivers/watchdog/mena21_wdt.c
@@ -0,0 +1,234 @@
+/*
+ * Watchdog driver for the A21 VME CPU Boards
+ *
+ * Copyright (C) 2013 MEN Mikro Elektronik Nuernberg GmbH
+ *
+ * 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
+ */
+#include <linux/module.h>
+#include <linux/moduleparam.h>
+#include <linux/types.h>
+#include <linux/kernel.h>
+#include <linux/slab.h>
+#include <linux/platform_device.h>
+#include <linux/watchdog.h>
+#include <linux/uaccess.h>
+#include <linux/gpio.h>
+#include <linux/delay.h>
+#include <linux/bitops.h>
+
+#define GPIO_WD_ENAB	169
+#define GPIO_WD_FAST	170
+#define GPIO_WD_TRIG	171
+
+#define GPIO_RST_CAUSE_BASE 166
+
+struct a21_wdt_drv {
+	struct watchdog_device wdt;
+	struct mutex lock;
+};
+
+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 a21_wdt_get_bootstatus(unsigned int *reset)
+{
+	int i;
+
+	for (i = 0; i < 3; i++)
+		*reset |= gpio_get_value(GPIO_RST_CAUSE_BASE + i)
+			? (1 << i) : 0;
+
+	if (*reset >= 8)
+		return -EINVAL;
+
+	return 0;
+}
+
+static int a21_wdt_start(struct watchdog_device *wdt)
+{
+	struct a21_wdt_drv *drv = watchdog_get_drvdata(wdt);
+
+	mutex_lock(&drv->lock);
+
+	gpio_set_value(GPIO_WD_ENAB, 1);
+
+	mutex_unlock(&drv->lock);
+
+	return 0;
+}
+
+static int a21_wdt_stop(struct watchdog_device *wdt)
+{
+	struct a21_wdt_drv *drv = watchdog_get_drvdata(wdt);
+
+	/* We don't stop if WDOG_NO_WAY_OUT is set */
+	if (test_bit(WDOG_NO_WAY_OUT, &wdt->status))
+		return -EINVAL;
+
+	mutex_lock(&drv->lock);
+
+	gpio_set_value(GPIO_WD_ENAB, 0);
+
+	mutex_unlock(&drv->lock);
+
+	return 0;
+}
+
+static int a21_wdt_ping(struct watchdog_device *wdt)
+{
+	struct a21_wdt_drv *drv = watchdog_get_drvdata(wdt);
+
+	mutex_lock(&drv->lock);
+
+	gpio_set_value(GPIO_WD_TRIG, 0);
+	ndelay(10);
+	gpio_set_value(GPIO_WD_TRIG, 1);
+
+	mutex_unlock(&drv->lock);
+
+	return 0;
+}
+
+static int a21_wdt_set_timeout(struct watchdog_device *wdt,
+			       unsigned int timeout)
+{
+	struct a21_wdt_drv *drv = watchdog_get_drvdata(wdt);
+
+	if (timeout != 1 && timeout != 30) {
+		dev_err(wdt->dev, "Only 1 and 30 allowed as timeout\n");
+		return -EINVAL;
+	}
+
+	if (timeout == 30 && wdt->timeout == 1) {
+		dev_err(wdt->dev,
+			"Transition from fast to slow mode not allowed\n");
+		return -EINVAL;
+	}
+
+	mutex_lock(&drv->lock);
+
+	if (timeout == 1)
+		gpio_set_value(GPIO_WD_FAST, 1);
+	else
+		gpio_set_value(GPIO_WD_FAST, 0);
+
+	wdt->timeout = timeout;
+
+	mutex_unlock(&drv->lock);
+
+	return 0;
+}
+
+static const struct watchdog_info a21_wdt_info = {
+	.options = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE,
+	.identity = "MEN A21 Watchdog",
+};
+
+static const struct watchdog_ops a21_wdt_ops = {
+	.owner = THIS_MODULE,
+	.start = a21_wdt_start,
+	.stop = a21_wdt_stop,
+	.ping = a21_wdt_ping,
+	.set_timeout = a21_wdt_set_timeout,
+};
+
+static struct watchdog_device a21_wdt = {
+	.info = &a21_wdt_info,
+	.ops = &a21_wdt_ops,
+	.min_timeout = 1,
+	.max_timeout = 30,
+};
+
+static int a21_wdt_probe(struct platform_device *pdev)
+{
+	struct a21_wdt_drv *drv;
+	unsigned int reset = 0;
+	int ret;
+
+	dev_info(&pdev->dev, "MEN A21 watchdog timer driver enabled\n");
+
+	drv = devm_kzalloc(&pdev->dev, sizeof(struct a21_wdt_drv), GFP_KERNEL);
+	if (!drv)
+		return -ENOMEM;
+
+	mutex_init(&drv->lock);
+	watchdog_init_timeout(&a21_wdt, 30, &pdev->dev);
+	watchdog_set_nowayout(&a21_wdt, nowayout);
+	watchdog_set_drvdata(&a21_wdt, drv);
+
+	ret = a21_wdt_get_bootstatus(&reset);
+	if (ret)
+		dev_warn(&pdev->dev, "Reset Cause contains invalid data\n");
+	else {
+		if (reset == 2)
+			a21_wdt.bootstatus |= WDIOF_EXTERN1;
+		else if (reset == 4)
+			a21_wdt.bootstatus |= WDIOF_CARDRESET;
+		else if (reset == 5)
+			a21_wdt.bootstatus |= WDIOF_POWERUNDER;
+		else if (reset == 7)
+			a21_wdt.bootstatus |= WDIOF_EXTERN2;
+	}
+
+	ret = watchdog_register_device(&a21_wdt);
+	if (ret) {
+		dev_err(&pdev->dev, "Cannot register watchdog device\n");
+		goto err_register_wd;
+	}
+
+	dev_set_drvdata(&pdev->dev, drv);
+
+	return 0;
+
+err_register_wd:
+	mutex_destroy(&drv->lock);
+
+	return ret;
+}
+
+static int a21_wdt_remove(struct platform_device *pdev)
+{
+	struct a21_wdt_drv *drv = dev_get_drvdata(&pdev->dev);
+
+	dev_warn(&pdev->dev,
+		"Unregistering A21 watchdog driver, board may reboot\n");
+
+
+	watchdog_unregister_device(&drv->wdt);
+
+	mutex_destroy(&drv->lock);
+
+	return 0;
+}
+
+static void a21_wdt_shutdown(struct platform_device *pdev)
+{
+	gpio_set_value(GPIO_WD_ENAB, 0);
+}
+
+static const struct of_device_id a21_wdt_ids[] = {
+	{ .compatible = "men,a021-wdt" },
+	{ },
+};
+
+static struct platform_driver a21_wdt_driver = {
+	.probe = a21_wdt_probe,
+	.remove = a21_wdt_remove,
+	.shutdown = a21_wdt_shutdown,
+	.driver = {
+		.name = "a21-watchdog",
+		.of_match_table = a21_wdt_ids,
+	},
+};
+
+module_platform_driver(a21_wdt_driver);
+
+MODULE_AUTHOR("MEN Mikro Elektronik");
+MODULE_DESCRIPTION("MEN A21 Watchdog");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:a21-watchdog");
--
1.7.9.5

Re: [PATCH v6] watchdog: New watchdog driver for MEN A21 watchdogs

[Guenter Roeck]

On Fri, May 31, 2013 at 02:55:19PM +0200, Johannes Thumshirn wrote:
> Hi Guenther,
> On Fri, May 31, 2013 at 04:40:37AM -0700, Guenter Roeck wrote:
> > > +#define GPIO_WD_ENAB	169
> > > +#define GPIO_WD_FAST	170
> > > +#define GPIO_WD_TRIG	171
> > > +
> > > +#define GPIO_RST_CAUSE_BASE 166
> > > +
> >
> > I think I asked that before ... as you are supporting devicetree, gpio pins
> > should really be provided through devicetree properties and not be hardcoded.
> >
> Yes you did and I didn't come up with a solution to this problem yet. I understand
> and agree to your concerns but I'm lacking example code/documentation for it, maybe
> you can point me to an example on that and then I'll update my code accordingly.
> 

Have a look at Documentation/devicetree/bindings/gpio/gpio-fan.txt and
drivers/hwmon/gpio-fan.c.

> > > +struct a21_wdt_drv {
> > > +	struct watchdog_device wdt;
> > > +	struct mutex lock;
> > > +	}
> > > +
> > > +	if (timeout == 30 && wdt->timeout == 1) {
> > > +		dev_err(wdt->dev,
> > > +			"Transition from fast to slow mode not allowed\n");
> > > +		return -EINVAL;
> > > +	}
> > > +
> > I dislike that [2 .. 29] timeout values are not supported, and would rather have
> > you accept that range and set the timeout to 30. Also, it is somewhat undesirable
> > that the timeout can not be changed back to 30 after being set to 1.
> 
> The not changing back issue was a hardware design criterion and I have no
> influence on that one. I've already checked with the IC developer and he said,
> this is intended. I don't quite understand why I should accept the timeout and
> set it to 30? In my opinion this only introduces some magic the user won't
> understand. Instead failing and giving an error message is something a user can
> take as a hint for investigating.
> 
On the other side you expect the user to know about the 1 / 30 limitation, which
I think is worse. The value actually set is returned by the ioctl per API, so
the application can check the actually configured value. As such, it is not
entirely true that the behavior would be unexpected.

> > As Wim recommended, a softdog on top of the hardware watchdog would be the best
> > solution. I'll leave it up to him to decide if he wants to accept the code
> > as-is.
> 
> Yes let's see what he's saying.
> 
Agreed.

> >
> > > +	mutex_lock(&drv->lock);
> > > +
> > > +	ret = watchdog_register_device(&a21_wdt);
> > > +	if (ret) {
> > > +		dev_err(&pdev->dev, "Cannot register watchdog device\n");
> > > +		goto err_register_wd;
> > > +	}
> > > +
> > > +	ret = a21_wdt_get_bootstatus(&reset);
> > > +	if (ret)
> > > +		dev_warn(&pdev->dev, "Reset Cause contains invalid data\n");
> > > +	else {
> > > +		if (reset == 2)
> > > +			a21_wdt.bootstatus |= WDIOF_EXTERN1;
> > > +		else if (reset == 4)
> > > +			a21_wdt.bootstatus |= WDIOF_CARDRESET;
> > > +		else if (reset == 5)
> > > +			a21_wdt.bootstatus |= WDIOF_POWERUNDER;
> > > +		else if (reset == 7)
> > > +			a21_wdt.bootstatus |= WDIOF_EXTERN2;
> >
> > What about other causes ? No useful match ?
> >
> 
> None I could find. Actually WDIOF_EXTERN[12] already are a pretty creative
> mapping in my opinion. EXTERN1 is a "Push Button" event, EXTERN2 is a reset
> caused by a JTAG/BDM adapter...
> 
ok.

> > I think bootstatus should be set prior to registering the watchdog device
> > to avoid race conditions where an application reads it prior to being set.
> >
> Agreed, didn't see that one *doh*, must be fixed before inclusion.

Happens to me all the time :)

Guenter

Re: [PATCH v5 1/2] watchdog: New watchdog driver for MEN A21 watchdogs

[Arnd Bergmann]

On Friday 31 May 2013 10:58:48 Johannes Thumshirn wrote:
> This patch adds the driver for the watchdog devices found on MEN Mikro
> Elektronik A21 VMEbus CPU Carrier Boards. It has DT-support and uses the
> watchdog framework.
> 

It seems the DT support is rather incomplete


> Signed-off-by: Johannes Thumshirn <johannes.thumshirn@men.de>
> ---
>  MAINTAINERS                   |    6 ++
>  drivers/watchdog/Kconfig      |    8 ++
>  drivers/watchdog/Makefile     |    1 +

Please add a binding document to Documentation/devicetree/bindings/

> +
> +#define GPIO_WD_ENAB	169
> +#define GPIO_WD_FAST	170
> +#define GPIO_WD_TRIG	171
> +
> +#define GPIO_RST_CAUSE_BASE 166

You should never hardwire GPIO numbers like this. There is a DT binding
for GPIO, so use that to get the number of the tree.

	Arnd

Re: [PATCH v7] watchdog: New watchdog driver for MEN A21 watchdogs

[Guenter Roeck]

On Fri, May 31, 2013 at 03:32:14PM +0200, Johannes Thumshirn wrote:
> This patch adds the driver for the watchdog devices found on MEN Mikro
> Elektronik A21 VMEbus CPU Carrier Boards. It has DT-support and uses the
> watchdog framework.
> 
> Revision 2:
> * Removed unneeded open flag in struct a21_wdt_drv
> * Corrected 3bit reason code from gpio
> * Additional sysfs files are now part of watchdog sysfs
> * Changed OFF/ON delay in ping from 400ms to 10ns
> * Reworked timeout setting
> * Removed a21_wdt_ioctl(...)
> 
> Revision 3:
> * Changed pr_{err,info} to dev_{err,info}
> * Removed out of memory error print
> * Transition from "fast" to "slow" mode not allowed by chip
> 
> Revision 4:
> * Remove reboot_notifier and place disable code into platform_device's shutdown function
> * Removed sysfs interface
> 
> Revision 5:
> 
> * Added setting of .bootstatus on driver init
> * Added initial timeout on driver init
> 
> Revision 6:
> * Use watchdog_init_timeout() to initialize timeout
> 
> Revision 7:
> * Fix possible get_bootstatus race condition
> 
> Signed-off-by: Johannes Thumshirn <johannes.thumshirn@men.de>
> ---
>  MAINTAINERS                   |    6 ++
>  drivers/watchdog/Kconfig      |    8 ++
>  drivers/watchdog/Makefile     |    1 +
>  drivers/watchdog/mena21_wdt.c |  234 +++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 249 insertions(+)
>  create mode 100644 drivers/watchdog/mena21_wdt.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 7714c3c..023945a 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -5307,6 +5307,12 @@ F:	drivers/mtd/
>  F:	include/linux/mtd/
>  F:	include/uapi/mtd/
> 
> +MEN A21 WATCHDOG DRIVER
> +M:  	Johannes Thumshirn <johannes.thumshirn@men.de>
> +L:	linux-watchdog@vger.kernel.org
> +S:	Supported
> +F:	drivers/watchdog/mena21_wdt.c
> +
>  METAG ARCHITECTURE
>  M:	James Hogan <james.hogan@imgtec.com>
>  S:	Supported
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index e89fc31..192b84d 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -1172,6 +1172,14 @@ config BOOKE_WDT_DEFAULT_TIMEOUT
> 
>  	  The value can be overridden by the wdt_period command-line parameter.
> 
> +config MEN_A21_WDT
> +       tristate "MEN A21 VME CPU Carrier Board Watchdog Timer"
> +       select WATCHDOG_CORE
> +       help
> +        Watchdog driver for MEN A21 VMEbus CPU Carrier Boards.
> +
> +	If unsure select N here.
> +
>  # PPC64 Architecture
> 
>  config WATCHDOG_RTAS
> diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
> index a300b94..bffdcb1 100644
> --- a/drivers/watchdog/Makefile
> +++ b/drivers/watchdog/Makefile
> @@ -143,6 +143,7 @@ obj-$(CONFIG_8xxx_WDT) += mpc8xxx_wdt.o
>  obj-$(CONFIG_MV64X60_WDT) += mv64x60_wdt.o
>  obj-$(CONFIG_PIKA_WDT) += pika_wdt.o
>  obj-$(CONFIG_BOOKE_WDT) += booke_wdt.o
> +obj-$(CONFIG_MEN_A21_WDT) += mena21_wdt.o
> 
>  # PPC64 Architecture
>  obj-$(CONFIG_WATCHDOG_RTAS) += wdrtas.o
> diff --git a/drivers/watchdog/mena21_wdt.c b/drivers/watchdog/mena21_wdt.c
> new file mode 100644
> index 0000000..dec35ec
> --- /dev/null
> +++ b/drivers/watchdog/mena21_wdt.c
> @@ -0,0 +1,234 @@
> +/*
> + * Watchdog driver for the A21 VME CPU Boards
> + *
> + * Copyright (C) 2013 MEN Mikro Elektronik Nuernberg GmbH
> + *
> + * 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
> + */
> +#include <linux/module.h>
> +#include <linux/moduleparam.h>
> +#include <linux/types.h>
> +#include <linux/kernel.h>
> +#include <linux/slab.h>
> +#include <linux/platform_device.h>
> +#include <linux/watchdog.h>
> +#include <linux/uaccess.h>
> +#include <linux/gpio.h>
> +#include <linux/delay.h>
> +#include <linux/bitops.h>
> +
> +#define GPIO_WD_ENAB	169
> +#define GPIO_WD_FAST	170
> +#define GPIO_WD_TRIG	171
> +
> +#define GPIO_RST_CAUSE_BASE 166
> +
> +struct a21_wdt_drv {
> +	struct watchdog_device wdt;
> +	struct mutex lock;
> +};
> +
> +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 a21_wdt_get_bootstatus(unsigned int *reset)
> +{
> +	int i;
> +
> +	for (i = 0; i < 3; i++)
> +		*reset |= gpio_get_value(GPIO_RST_CAUSE_BASE + i)
> +			? (1 << i) : 0;
> +
> +	if (*reset >= 8)
> +		return -EINVAL;
> +
This can never happen. Even if it does, -EINVAL would not be appropriate.

Also, I don't see why you pass a pointer to the reset code (which is pre-set to 0)
in the first place. Just return the reset cause. If you really need to return an
error, you can pass it as negative return value.

Thanks,
Guenter

> +	return 0;
> +}
> +
> +static int a21_wdt_start(struct watchdog_device *wdt)
> +{
> +	struct a21_wdt_drv *drv = watchdog_get_drvdata(wdt);
> +
> +	mutex_lock(&drv->lock);
> +
> +	gpio_set_value(GPIO_WD_ENAB, 1);
> +
> +	mutex_unlock(&drv->lock);
> +
> +	return 0;
> +}
> +
> +static int a21_wdt_stop(struct watchdog_device *wdt)
> +{
> +	struct a21_wdt_drv *drv = watchdog_get_drvdata(wdt);
> +
> +	/* We don't stop if WDOG_NO_WAY_OUT is set */
> +	if (test_bit(WDOG_NO_WAY_OUT, &wdt->status))
> +		return -EINVAL;
> +
> +	mutex_lock(&drv->lock);
> +
> +	gpio_set_value(GPIO_WD_ENAB, 0);
> +
> +	mutex_unlock(&drv->lock);
> +
> +	return 0;
> +}
> +
> +static int a21_wdt_ping(struct watchdog_device *wdt)
> +{
> +	struct a21_wdt_drv *drv = watchdog_get_drvdata(wdt);
> +
> +	mutex_lock(&drv->lock);
> +
> +	gpio_set_value(GPIO_WD_TRIG, 0);
> +	ndelay(10);
> +	gpio_set_value(GPIO_WD_TRIG, 1);
> +
> +	mutex_unlock(&drv->lock);
> +
> +	return 0;
> +}
> +
> +static int a21_wdt_set_timeout(struct watchdog_device *wdt,
> +			       unsigned int timeout)
> +{
> +	struct a21_wdt_drv *drv = watchdog_get_drvdata(wdt);
> +
> +	if (timeout != 1 && timeout != 30) {
> +		dev_err(wdt->dev, "Only 1 and 30 allowed as timeout\n");
> +		return -EINVAL;
> +	}
> +
> +	if (timeout == 30 && wdt->timeout == 1) {
> +		dev_err(wdt->dev,
> +			"Transition from fast to slow mode not allowed\n");
> +		return -EINVAL;
> +	}
> +
> +	mutex_lock(&drv->lock);
> +
> +	if (timeout == 1)
> +		gpio_set_value(GPIO_WD_FAST, 1);
> +	else
> +		gpio_set_value(GPIO_WD_FAST, 0);
> +
> +	wdt->timeout = timeout;
> +
> +	mutex_unlock(&drv->lock);
> +
> +	return 0;
> +}
> +
> +static const struct watchdog_info a21_wdt_info = {
> +	.options = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE,
> +	.identity = "MEN A21 Watchdog",
> +};
> +
> +static const struct watchdog_ops a21_wdt_ops = {
> +	.owner = THIS_MODULE,
> +	.start = a21_wdt_start,
> +	.stop = a21_wdt_stop,
> +	.ping = a21_wdt_ping,
> +	.set_timeout = a21_wdt_set_timeout,
> +};
> +
> +static struct watchdog_device a21_wdt = {
> +	.info = &a21_wdt_info,
> +	.ops = &a21_wdt_ops,
> +	.min_timeout = 1,
> +	.max_timeout = 30,
> +};
> +
> +static int a21_wdt_probe(struct platform_device *pdev)
> +{
> +	struct a21_wdt_drv *drv;
> +	unsigned int reset = 0;
> +	int ret;
> +
> +	dev_info(&pdev->dev, "MEN A21 watchdog timer driver enabled\n");
> +
> +	drv = devm_kzalloc(&pdev->dev, sizeof(struct a21_wdt_drv), GFP_KERNEL);
> +	if (!drv)
> +		return -ENOMEM;
> +
> +	mutex_init(&drv->lock);
> +	watchdog_init_timeout(&a21_wdt, 30, &pdev->dev);
> +	watchdog_set_nowayout(&a21_wdt, nowayout);
> +	watchdog_set_drvdata(&a21_wdt, drv);
> +
> +	ret = a21_wdt_get_bootstatus(&reset);
> +	if (ret)
> +		dev_warn(&pdev->dev, "Reset Cause contains invalid data\n");
> +	else {
> +		if (reset == 2)
> +			a21_wdt.bootstatus |= WDIOF_EXTERN1;
> +		else if (reset == 4)
> +			a21_wdt.bootstatus |= WDIOF_CARDRESET;
> +		else if (reset == 5)
> +			a21_wdt.bootstatus |= WDIOF_POWERUNDER;
> +		else if (reset == 7)
> +			a21_wdt.bootstatus |= WDIOF_EXTERN2;
> +	}
> +
> +	ret = watchdog_register_device(&a21_wdt);
> +	if (ret) {
> +		dev_err(&pdev->dev, "Cannot register watchdog device\n");
> +		goto err_register_wd;
> +	}
> +
> +	dev_set_drvdata(&pdev->dev, drv);
> +
> +	return 0;
> +
> +err_register_wd:
> +	mutex_destroy(&drv->lock);
> +
> +	return ret;
> +}
> +
> +static int a21_wdt_remove(struct platform_device *pdev)
> +{
> +	struct a21_wdt_drv *drv = dev_get_drvdata(&pdev->dev);
> +
> +	dev_warn(&pdev->dev,
> +		"Unregistering A21 watchdog driver, board may reboot\n");
> +
> +
> +	watchdog_unregister_device(&drv->wdt);
> +
> +	mutex_destroy(&drv->lock);
> +
> +	return 0;
> +}
> +
> +static void a21_wdt_shutdown(struct platform_device *pdev)
> +{
> +	gpio_set_value(GPIO_WD_ENAB, 0);
> +}
> +
> +static const struct of_device_id a21_wdt_ids[] = {
> +	{ .compatible = "men,a021-wdt" },
> +	{ },
> +};
> +
> +static struct platform_driver a21_wdt_driver = {
> +	.probe = a21_wdt_probe,
> +	.remove = a21_wdt_remove,
> +	.shutdown = a21_wdt_shutdown,
> +	.driver = {
> +		.name = "a21-watchdog",
> +		.of_match_table = a21_wdt_ids,
> +	},
> +};
> +
> +module_platform_driver(a21_wdt_driver);
> +
> +MODULE_AUTHOR("MEN Mikro Elektronik");
> +MODULE_DESCRIPTION("MEN A21 Watchdog");
> +MODULE_LICENSE("GPL");
> +MODULE_ALIAS("platform:a21-watchdog");
> --
> 1.7.9.5
> 
> --
> 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
> 

Re: [PATCH v6] watchdog: New watchdog driver for MEN A21 watchdogs

[Johannes Thumshirn]

On Fri, May 31, 2013 at 09:15:23PM -0700, Guenter Roeck wrote:
> On Fri, May 31, 2013 at 02:55:19PM +0200, Johannes Thumshirn wrote:
> > Hi Guenther,
> > On Fri, May 31, 2013 at 04:40:37AM -0700, Guenter Roeck wrote:
> > > > +#define GPIO_WD_ENAB	169
> > > > +#define GPIO_WD_FAST	170
> > > > +#define GPIO_WD_TRIG	171
> > > > +
> > > > +#define GPIO_RST_CAUSE_BASE 166
> > > > +
> > >
> > > I think I asked that before ... as you are supporting devicetree, gpio pins
> > > should really be provided through devicetree properties and not be hardcoded.
> > >
> > Yes you did and I didn't come up with a solution to this problem yet. I understand
> > and agree to your concerns but I'm lacking example code/documentation for it, maybe
> > you can point me to an example on that and then I'll update my code accordingly.
> >
>
> Have a look at Documentation/devicetree/bindings/gpio/gpio-fan.txt and
> drivers/hwmon/gpio-fan.c.

Thanks a lot, this really helped me out. Updated patch is comming today
including the bindings document Arnd Bergmann requested. I only need to rebase
the sysfs patch on top of that changes.

Regards,
Johannes

[PATCH v8] watchdog: New watchdog driver for MEN A21 watchdogs

[Johannes Thumshirn]

This patch adds the driver for the watchdog devices found on MEN Mikro
Elektronik A21 VMEbus CPU Carrier Boards. It has DT-support and uses the
watchdog framework.

Revision 2:
* Removed unneeded open flag in struct a21_wdt_drv
* Corrected 3bit reason code from gpio
* Additional sysfs files are now part of watchdog sysfs
* Changed OFF/ON delay in ping from 400ms to 10ns
* Reworked timeout setting
* Removed a21_wdt_ioctl(...)

Revision 3:
* Changed pr_{err,info} to dev_{err,info}
* Removed out of memory error print
* Transition from "fast" to "slow" mode not allowed by chip

Revision 4:
* Remove reboot_notifier and place disable code into platform_device's shutdown function
* Removed sysfs interface

Revision 5:
* Added setting of .bootstatus on driver init
* Added initial timeout on driver init

Revision 6:
* Use watchdog_init_timeout() to initialize timeout

Revision 7:
* Fix possible get_bootstatus race condition

Revision 8:
* a21_wdt_get_bootstatus() should return reset code
* GPIOs are supplied via DT instead of being hardcoded. Code derived from
  (drivers/hwmon/gpio-fan.c)
* Added Devicetree binding document
* Driver now depends on GPIOLIB

Signed-off-by: Johannes Thumshirn <johannes.thumshirn@men.de>
---
 .../devicetree/bindings/gpio/men-a021-wdt.txt      |   25 ++
 MAINTAINERS                                        |    6 +
 drivers/watchdog/Kconfig                           |   12 +
 drivers/watchdog/Makefile                          |    1 +
 drivers/watchdog/mena21_wdt.c                      |  277 ++++++++++++++++++++
 5 files changed, 321 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/gpio/men-a021-wdt.txt
 create mode 100644 drivers/watchdog/mena21_wdt.c

diff --git a/Documentation/devicetree/bindings/gpio/men-a021-wdt.txt b/Documentation/devicetree/bindings/gpio/men-a021-wdt.txt
new file mode 100644
index 0000000..370dee3
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/men-a021-wdt.txt
@@ -0,0 +1,25 @@
+Bindings for MEN A21 Watchdog device connected to GPIO lines
+
+Required properties:
+- compatible: "men,a021-wdt"
+- gpios: Specifies the pins that control the Watchdog, order:
+  1: Watchdog enable
+  2: Watchdog fast-mode
+  3: Watchdog trigger
+  4: Watchdog reset cause bit 0
+  5: Watchdog reset cause bit 1
+  6: Watchdog reset cause bit 2
+
+Optional properties:
+- None
+
+Example:
+	watchdog {
+		compatible ="men,a021-wdt";
+		gpios = <&gpio3 9  1	/* WD_EN */
+			 &gpio3 10 1	/* WD_FAST */
+			 &gpio3 11 1	/* WD_TRIG */
+			 &gpio3 6  1	/* RST_CAUSE[0] */
+			 &gpio3 7  1	/* RST_CAUSE[1] */
+			 &gpio3 8  1>;	/* RST_CAUSE[2] */
+	};
diff --git a/MAINTAINERS b/MAINTAINERS
index f35a259..bd86661 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5307,6 +5307,12 @@ F:	drivers/mtd/
 F:	include/linux/mtd/
 F:	include/uapi/mtd/

+MEN A21 WATCHDOG DRIVER
+M:  	Johannes Thumshirn <johannes.thumshirn@men.de>
+L:	linux-watchdog@vger.kernel.org
+S:	Supported
+F:	drivers/watchdog/mena21_wdt.c
+
 METAG ARCHITECTURE
 M:	James Hogan <james.hogan@imgtec.com>
 S:	Supported
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index e89fc31..8b143ee 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -1172,6 +1172,18 @@ config BOOKE_WDT_DEFAULT_TIMEOUT

 	  The value can be overridden by the wdt_period command-line parameter.

+config MEN_A21_WDT
+       tristate "MEN A21 VME CPU Carrier Board Watchdog Timer"
+       select WATCHDOG_CORE
+       depends on GPIOLIB
+       help
+        Watchdog driver for MEN A21 VMEbus CPU Carrier Boards.
+
+	The driver can also be built as a module. If so, the module will be
+	called mena21_wdt.
+
+	If unsure select N here.
+
 # PPC64 Architecture

 config WATCHDOG_RTAS
diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
index a300b94..bffdcb1 100644
--- a/drivers/watchdog/Makefile
+++ b/drivers/watchdog/Makefile
@@ -143,6 +143,7 @@ obj-$(CONFIG_8xxx_WDT) += mpc8xxx_wdt.o
 obj-$(CONFIG_MV64X60_WDT) += mv64x60_wdt.o
 obj-$(CONFIG_PIKA_WDT) += pika_wdt.o
 obj-$(CONFIG_BOOKE_WDT) += booke_wdt.o
+obj-$(CONFIG_MEN_A21_WDT) += mena21_wdt.o

 # PPC64 Architecture
 obj-$(CONFIG_WATCHDOG_RTAS) += wdrtas.o
diff --git a/drivers/watchdog/mena21_wdt.c b/drivers/watchdog/mena21_wdt.c
new file mode 100644
index 0000000..29f9c0d
--- /dev/null
+++ b/drivers/watchdog/mena21_wdt.c
@@ -0,0 +1,277 @@
+/*
+ * Watchdog driver for the A21 VME CPU Boards
+ *
+ * Copyright (C) 2013 MEN Mikro Elektronik Nuernberg GmbH
+ *
+ * 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
+ */
+#include <linux/module.h>
+#include <linux/moduleparam.h>
+#include <linux/types.h>
+#include <linux/kernel.h>
+#include <linux/slab.h>
+#include <linux/platform_device.h>
+#include <linux/watchdog.h>
+#include <linux/uaccess.h>
+#include <linux/gpio.h>
+#include <linux/of_gpio.h>
+#include <linux/delay.h>
+#include <linux/bitops.h>
+
+enum a21_wdt_gpios {
+	GPIO_WD_ENAB,
+	GPIO_WD_FAST,
+	GPIO_WD_TRIG,
+	GPIO_WD_RST0,
+	GPIO_WD_RST1,
+	GPIO_WD_RST2,
+};
+
+struct a21_wdt_drv {
+	struct watchdog_device wdt;
+	struct mutex lock;
+	int num_gpios;
+	unsigned *gpios;
+};
+
+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 unsigned int a21_wdt_get_bootstatus(struct a21_wdt_drv *drv)
+{
+	int reset = 0;
+
+	reset |= gpio_get_value(drv->gpios[GPIO_WD_RST0]) ? (1 << 0) : 0;
+	reset |= gpio_get_value(drv->gpios[GPIO_WD_RST1]) ? (1 << 1) : 0;
+	reset |= gpio_get_value(drv->gpios[GPIO_WD_RST2]) ? (1 << 2) : 0;
+
+	return reset;
+}
+
+static int a21_wdt_start(struct watchdog_device *wdt)
+{
+	struct a21_wdt_drv *drv = watchdog_get_drvdata(wdt);
+
+	mutex_lock(&drv->lock);
+
+	gpio_set_value(drv->gpios[GPIO_WD_ENAB], 1);
+
+	mutex_unlock(&drv->lock);
+
+	return 0;
+}
+
+static int a21_wdt_stop(struct watchdog_device *wdt)
+{
+	struct a21_wdt_drv *drv = watchdog_get_drvdata(wdt);
+
+	/* We don't stop if WDOG_NO_WAY_OUT is set */
+	if (test_bit(WDOG_NO_WAY_OUT, &wdt->status))
+		return -EINVAL;
+
+	mutex_lock(&drv->lock);
+
+	gpio_set_value(drv->gpios[GPIO_WD_ENAB], 0);
+
+	mutex_unlock(&drv->lock);
+
+	return 0;
+}
+
+static int a21_wdt_ping(struct watchdog_device *wdt)
+{
+	struct a21_wdt_drv *drv = watchdog_get_drvdata(wdt);
+
+	mutex_lock(&drv->lock);
+
+	gpio_set_value(drv->gpios[GPIO_WD_TRIG], 0);
+	ndelay(10);
+	gpio_set_value(drv->gpios[GPIO_WD_TRIG], 1);
+
+	mutex_unlock(&drv->lock);
+
+	return 0;
+}
+
+static int a21_wdt_set_timeout(struct watchdog_device *wdt,
+			       unsigned int timeout)
+{
+	struct a21_wdt_drv *drv = watchdog_get_drvdata(wdt);
+
+	if (timeout != 1 && timeout != 30) {
+		dev_err(wdt->dev, "Only 1 and 30 allowed as timeout\n");
+		return -EINVAL;
+	}
+
+	if (timeout == 30 && wdt->timeout == 1) {
+		dev_err(wdt->dev,
+			"Transition from fast to slow mode not allowed\n");
+		return -EINVAL;
+	}
+
+	mutex_lock(&drv->lock);
+
+	if (timeout == 1)
+		gpio_set_value(drv->gpios[GPIO_WD_FAST], 1);
+	else
+		gpio_set_value(drv->gpios[GPIO_WD_FAST], 0);
+
+	wdt->timeout = timeout;
+
+	mutex_unlock(&drv->lock);
+
+	return 0;
+}
+
+static const struct watchdog_info a21_wdt_info = {
+	.options = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE,
+	.identity = "MEN A21 Watchdog",
+};
+
+static const struct watchdog_ops a21_wdt_ops = {
+	.owner = THIS_MODULE,
+	.start = a21_wdt_start,
+	.stop = a21_wdt_stop,
+	.ping = a21_wdt_ping,
+	.set_timeout = a21_wdt_set_timeout,
+};
+
+static struct watchdog_device a21_wdt = {
+	.info = &a21_wdt_info,
+	.ops = &a21_wdt_ops,
+	.min_timeout = 1,
+	.max_timeout = 30,
+};
+
+static int a21_wdt_probe(struct platform_device *pdev)
+{
+	struct device_node *node;
+	struct a21_wdt_drv *drv;
+	unsigned int reset = 0;
+	unsigned *gpios;
+	int ret;
+	int i;
+
+	dev_info(&pdev->dev, "MEN A21 watchdog timer driver enabled\n");
+
+	drv = devm_kzalloc(&pdev->dev, sizeof(struct a21_wdt_drv), GFP_KERNEL);
+	if (!drv)
+		return -ENOMEM;
+
+	/* Fill GPIO pin array */
+	node = pdev->dev.of_node;
+
+	drv->num_gpios = of_gpio_count(node);
+	if (drv->num_gpios <= 0) {
+		dev_err(&pdev->dev, "gpios DT property empty / missing");
+		return -ENODEV;
+	}
+
+	gpios = devm_kzalloc(&pdev->dev, drv->num_gpios * sizeof(unsigned),
+			     GFP_KERNEL);
+	if (!gpios)
+		return -ENOMEM;
+
+	for (i = 0; i < drv->num_gpios; i++) {
+		int val;
+
+		val = of_get_gpio(node, i);
+		if (val < 0)
+			return val;
+
+		gpios[i] = val;
+	}
+	drv->gpios = gpios;
+
+	/* Request the used GPIOs */
+	for (i = 0; i < drv->num_gpios; i++) {
+		ret = devm_gpio_request(&pdev->dev, drv->gpios[i],
+					"MEN A21 Watchdog");
+		if (ret)
+			return ret;
+
+		ret = gpio_direction_output(drv->gpios[i],
+					    gpio_get_value(drv->gpios[i]));
+		if (ret)
+			return ret;
+	}
+
+	mutex_init(&drv->lock);
+	watchdog_init_timeout(&a21_wdt, 30, &pdev->dev);
+	watchdog_set_nowayout(&a21_wdt, nowayout);
+	watchdog_set_drvdata(&a21_wdt, drv);
+
+	reset = a21_wdt_get_bootstatus(drv);
+	if (reset == 2)
+		a21_wdt.bootstatus |= WDIOF_EXTERN1;
+	else if (reset == 4)
+		a21_wdt.bootstatus |= WDIOF_CARDRESET;
+	else if (reset == 5)
+		a21_wdt.bootstatus |= WDIOF_POWERUNDER;
+	else if (reset == 7)
+		a21_wdt.bootstatus |= WDIOF_EXTERN2;
+
+
+	ret = watchdog_register_device(&a21_wdt);
+	if (ret) {
+		dev_err(&pdev->dev, "Cannot register watchdog device\n");
+		goto err_register_wd;
+	}
+
+	dev_set_drvdata(&pdev->dev, drv);
+
+	return 0;
+
+err_register_wd:
+	mutex_destroy(&drv->lock);
+
+	return ret;
+}
+
+static int a21_wdt_remove(struct platform_device *pdev)
+{
+	struct a21_wdt_drv *drv = dev_get_drvdata(&pdev->dev);
+
+	dev_warn(&pdev->dev,
+		"Unregistering A21 watchdog driver, board may reboot\n");
+
+
+	watchdog_unregister_device(&drv->wdt);
+
+	mutex_destroy(&drv->lock);
+
+	return 0;
+}
+
+static void a21_wdt_shutdown(struct platform_device *pdev)
+{
+	struct a21_wdt_drv *drv = dev_get_drvdata(&pdev->dev);
+
+	gpio_set_value(drv->gpios[GPIO_WD_ENAB], 0);
+}
+
+static const struct of_device_id a21_wdt_ids[] = {
+	{ .compatible = "men,a021-wdt" },
+	{ },
+};
+
+static struct platform_driver a21_wdt_driver = {
+	.probe = a21_wdt_probe,
+	.remove = a21_wdt_remove,
+	.shutdown = a21_wdt_shutdown,
+	.driver = {
+		.name = "a21-watchdog",
+		.of_match_table = a21_wdt_ids,
+	},
+};
+
+module_platform_driver(a21_wdt_driver);
+
+MODULE_AUTHOR("MEN Mikro Elektronik");
+MODULE_DESCRIPTION("MEN A21 Watchdog");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:a21-watchdog");
--
1.7.9.5

Some problems with sysfs patch (was Re: [PATCH v6] watchdog: New watchdog driver for MEN A21 watchdogs)

[Johannes Thumshirn]

On Mon, Jun 03, 2013 at 11:50:26AM +0200, Johannes Thumshirn wrote:
> On Fri, May 31, 2013 at 09:15:23PM -0700, Guenter Roeck wrote:
> > On Fri, May 31, 2013 at 02:55:19PM +0200, Johannes Thumshirn wrote:
> > > Hi Guenther,
> > > On Fri, May 31, 2013 at 04:40:37AM -0700, Guenter Roeck wrote:
> > > > > +#define GPIO_WD_ENAB	169
> > > > > +#define GPIO_WD_FAST	170
> > > > > +#define GPIO_WD_TRIG	171
> > > > > +
> > > > > +#define GPIO_RST_CAUSE_BASE 166
> > > > > +
> > > >
> > > > I think I asked that before ... as you are supporting devicetree, gpio pins
> > > > should really be provided through devicetree properties and not be hardcoded.
> > > >
> > > Yes you did and I didn't come up with a solution to this problem yet. I understand
> > > and agree to your concerns but I'm lacking example code/documentation for it, maybe
> > > you can point me to an example on that and then I'll update my code accordingly.
> > >
> >
> > Have a look at Documentation/devicetree/bindings/gpio/gpio-fan.txt and
> > drivers/hwmon/gpio-fan.c.
>
> Thanks a lot, this really helped me out. Updated patch is comming today
> including the bindings document Arnd Bergmann requested. I only need to rebase
> the sysfs patch on top of that changes.
>
> Regards,
> Johannes

Regarding the sysfs patch, I have a funny effect in my (rebased) sysfs code.

Given the following code:

+static ssize_t rebootcause_show(struct device *dev,
+                               struct device_attribute *attr,
+                               char *buf)
+{
+       struct a21_wdt_drv *drv = dev_get_drvdata(dev);
+       unsigned int reset = 0;
+
+       if (!drv)
+               return -EIO;
+
+       reset = a21_wdt_get_bootstatus(drv);
+
+       return sprintf(buf, "%s\n", reset_causes[reset]);
+}
+static DEVICE_ATTR(rebootcause, S_IRUGO, rebootcause_show, NULL);

I actually need the check for if (!drv) to prevent an OOPS, as
dev_get_drvdata(dev) returns NULL., though it is set at the end of my probe
function via:

[...]
+       ret = watchdog_register_device(&a21_wdt);
+       if (ret) {
+               dev_err(&pdev->dev, "Cannot register watchdog device\n");
+               goto err_register_wd;
+       }
+
+       dev_set_drvdata(&pdev->dev, drv);
+
+       return 0;
[...]

The watchdog driver itself is working without any troubles.

Some advice here would be worth its weight in gold.

Thanks,
Johannes

Re: Some problems with sysfs patch (was Re: [PATCH v6] watchdog: New watchdog driver for MEN A21 watchdogs)

[Guenter Roeck]

On Thu, Jun 06, 2013 at 12:51:03PM +0200, Johannes Thumshirn wrote:
> On Mon, Jun 03, 2013 at 11:50:26AM +0200, Johannes Thumshirn wrote:
> > On Fri, May 31, 2013 at 09:15:23PM -0700, Guenter Roeck wrote:
> > > On Fri, May 31, 2013 at 02:55:19PM +0200, Johannes Thumshirn wrote:
> > > > Hi Guenther,
> > > > On Fri, May 31, 2013 at 04:40:37AM -0700, Guenter Roeck wrote:
> > > > > > +#define GPIO_WD_ENAB	169
> > > > > > +#define GPIO_WD_FAST	170
> > > > > > +#define GPIO_WD_TRIG	171
> > > > > > +
> > > > > > +#define GPIO_RST_CAUSE_BASE 166
> > > > > > +
> > > > >
> > > > > I think I asked that before ... as you are supporting devicetree, gpio pins
> > > > > should really be provided through devicetree properties and not be hardcoded.
> > > > >
> > > > Yes you did and I didn't come up with a solution to this problem yet. I understand
> > > > and agree to your concerns but I'm lacking example code/documentation for it, maybe
> > > > you can point me to an example on that and then I'll update my code accordingly.
> > > >
> > >
> > > Have a look at Documentation/devicetree/bindings/gpio/gpio-fan.txt and
> > > drivers/hwmon/gpio-fan.c.
> >
> > Thanks a lot, this really helped me out. Updated patch is comming today
> > including the bindings document Arnd Bergmann requested. I only need to rebase
> > the sysfs patch on top of that changes.
> >
> > Regards,
> > Johannes
> 
> Regarding the sysfs patch, I have a funny effect in my (rebased) sysfs code.
> 
> Given the following code:
> 
> +static ssize_t rebootcause_show(struct device *dev,
> +                               struct device_attribute *attr,
> +                               char *buf)
> +{
> +       struct a21_wdt_drv *drv = dev_get_drvdata(dev);
> +       unsigned int reset = 0;
> +
> +       if (!drv)
> +               return -EIO;
> +
> +       reset = a21_wdt_get_bootstatus(drv);
> +
> +       return sprintf(buf, "%s\n", reset_causes[reset]);
> +}
> +static DEVICE_ATTR(rebootcause, S_IRUGO, rebootcause_show, NULL);
> 
> I actually need the check for if (!drv) to prevent an OOPS, as
> dev_get_drvdata(dev) returns NULL., though it is set at the end of my probe
> function via:
> 
> [...]
> +       ret = watchdog_register_device(&a21_wdt);
> +       if (ret) {
> +               dev_err(&pdev->dev, "Cannot register watchdog device\n");
> +               goto err_register_wd;
> +       }
> +
> +       dev_set_drvdata(&pdev->dev, drv);
> +
> +       return 0;
> [...]
> 
> The watchdog driver itself is working without any troubles.
> 
> Some advice here would be worth its weight in gold.
> 
It has to be set before the attribute is created. Are you doing that ? Also, is
'dev' the same device pointer (ie is the 'dev' in your function the same as
&pdev->dev) ?

Guenter

> Thanks,
> Johannes
> 

Re: Some problems with sysfs patch (was Re: [PATCH v6] watchdog: New watchdog driver for MEN A21 watchdogs)

[Johannes Thumshirn]

On Thu, Jun 06, 2013 at 04:31:10AM -0700, Guenter Roeck wrote:
> On Thu, Jun 06, 2013 at 12:51:03PM +0200, Johannes Thumshirn wrote:
> > On Mon, Jun 03, 2013 at 11:50:26AM +0200, Johannes Thumshirn wrote:
> > > On Fri, May 31, 2013 at 09:15:23PM -0700, Guenter Roeck wrote:
> > > > On Fri, May 31, 2013 at 02:55:19PM +0200, Johannes Thumshirn wrote:
> > > > > Hi Guenther,
> > > > > On Fri, May 31, 2013 at 04:40:37AM -0700, Guenter Roeck wrote:
> > > > > > > +#define GPIO_WD_ENAB	169
> > > > > > > +#define GPIO_WD_FAST	170
> > > > > > > +#define GPIO_WD_TRIG	171
> > > > > > > +
> > > > > > > +#define GPIO_RST_CAUSE_BASE 166
> > > > > > > +
> > > > > >
> > > > > > I think I asked that before ... as you are supporting devicetree, gpio pins
> > > > > > should really be provided through devicetree properties and not be hardcoded.
> > > > > >
> > > > > Yes you did and I didn't come up with a solution to this problem yet. I understand
> > > > > and agree to your concerns but I'm lacking example code/documentation for it, maybe
> > > > > you can point me to an example on that and then I'll update my code accordingly.
> > > > >
> > > >
> > > > Have a look at Documentation/devicetree/bindings/gpio/gpio-fan.txt and
> > > > drivers/hwmon/gpio-fan.c.
> > >
> > > Thanks a lot, this really helped me out. Updated patch is comming today
> > > including the bindings document Arnd Bergmann requested. I only need to rebase
> > > the sysfs patch on top of that changes.
> > >
> > > Regards,
> > > Johannes
> >
> > Regarding the sysfs patch, I have a funny effect in my (rebased) sysfs code.
> >
> > Given the following code:
> >
> > +static ssize_t rebootcause_show(struct device *dev,
> > +                               struct device_attribute *attr,
> > +                               char *buf)
> > +{
> > +       struct a21_wdt_drv *drv = dev_get_drvdata(dev);
> > +       unsigned int reset = 0;
> > +
> > +       if (!drv)
> > +               return -EIO;
> > +
> > +       reset = a21_wdt_get_bootstatus(drv);
> > +
> > +       return sprintf(buf, "%s\n", reset_causes[reset]);
> > +}
> > +static DEVICE_ATTR(rebootcause, S_IRUGO, rebootcause_show, NULL);
> >
> > I actually need the check for if (!drv) to prevent an OOPS, as
> > dev_get_drvdata(dev) returns NULL., though it is set at the end of my probe
> > function via:
> >
> > [...]
> > +       ret = watchdog_register_device(&a21_wdt);
> > +       if (ret) {
> > +               dev_err(&pdev->dev, "Cannot register watchdog device\n");
> > +               goto err_register_wd;
> > +       }
> > +
> > +       dev_set_drvdata(&pdev->dev, drv);
> > +
> > +       return 0;
> > [...]
> >
> > The watchdog driver itself is working without any troubles.
> >
> > Some advice here would be worth its weight in gold.
> >
> It has to be set before the attribute is created. Are you doing that ? Also, is
> 'dev' the same device pointer (ie is the 'dev' in your function the same as
> &pdev->dev) ?
>
> Guenter

Hi Guenter,

Thanks for the quick reply. Indeed 'dev' in my function is a struct
watchdog_device's dev (for device_create_file) and not &pdev->dev. Which in turn
are not the same.

A call to dev_set_drvdata(a21_wdt.dev, drv); solved the NULL pointer access. So
last question, is it save to set the drvdata of a dev inside the struct
watchdog_device or do I break something vital that way?

Thanks a lot,
Johannes

Re: Some problems with sysfs patch (was Re: [PATCH v6] watchdog: New watchdog driver for MEN A21 watchdogs)

[Guenter Roeck]

On Thu, Jun 06, 2013 at 03:00:55PM +0200, Johannes Thumshirn wrote:
> On Thu, Jun 06, 2013 at 04:31:10AM -0700, Guenter Roeck wrote:
> > On Thu, Jun 06, 2013 at 12:51:03PM +0200, Johannes Thumshirn wrote:
> > > On Mon, Jun 03, 2013 at 11:50:26AM +0200, Johannes Thumshirn wrote:
> > > > On Fri, May 31, 2013 at 09:15:23PM -0700, Guenter Roeck wrote:
> > > > > On Fri, May 31, 2013 at 02:55:19PM +0200, Johannes Thumshirn wrote:
> > > > > > Hi Guenther,
> > > > > > On Fri, May 31, 2013 at 04:40:37AM -0700, Guenter Roeck wrote:
> > > > > > > > +#define GPIO_WD_ENAB	169
> > > > > > > > +#define GPIO_WD_FAST	170
> > > > > > > > +#define GPIO_WD_TRIG	171
> > > > > > > > +
> > > > > > > > +#define GPIO_RST_CAUSE_BASE 166
> > > > > > > > +
> > > > > > >
> > > > > > > I think I asked that before ... as you are supporting devicetree, gpio pins
> > > > > > > should really be provided through devicetree properties and not be hardcoded.
> > > > > > >
> > > > > > Yes you did and I didn't come up with a solution to this problem yet. I understand
> > > > > > and agree to your concerns but I'm lacking example code/documentation for it, maybe
> > > > > > you can point me to an example on that and then I'll update my code accordingly.
> > > > > >
> > > > >
> > > > > Have a look at Documentation/devicetree/bindings/gpio/gpio-fan.txt and
> > > > > drivers/hwmon/gpio-fan.c.
> > > >
> > > > Thanks a lot, this really helped me out. Updated patch is comming today
> > > > including the bindings document Arnd Bergmann requested. I only need to rebase
> > > > the sysfs patch on top of that changes.
> > > >
> > > > Regards,
> > > > Johannes
> > >
> > > Regarding the sysfs patch, I have a funny effect in my (rebased) sysfs code.
> > >
> > > Given the following code:
> > >
> > > +static ssize_t rebootcause_show(struct device *dev,
> > > +                               struct device_attribute *attr,
> > > +                               char *buf)
> > > +{
> > > +       struct a21_wdt_drv *drv = dev_get_drvdata(dev);
> > > +       unsigned int reset = 0;
> > > +
> > > +       if (!drv)
> > > +               return -EIO;
> > > +
> > > +       reset = a21_wdt_get_bootstatus(drv);
> > > +
> > > +       return sprintf(buf, "%s\n", reset_causes[reset]);
> > > +}
> > > +static DEVICE_ATTR(rebootcause, S_IRUGO, rebootcause_show, NULL);
> > >
> > > I actually need the check for if (!drv) to prevent an OOPS, as
> > > dev_get_drvdata(dev) returns NULL., though it is set at the end of my probe
> > > function via:
> > >
> > > [...]
> > > +       ret = watchdog_register_device(&a21_wdt);
> > > +       if (ret) {
> > > +               dev_err(&pdev->dev, "Cannot register watchdog device\n");
> > > +               goto err_register_wd;
> > > +       }
> > > +
> > > +       dev_set_drvdata(&pdev->dev, drv);
> > > +
> > > +       return 0;
> > > [...]
> > >
> > > The watchdog driver itself is working without any troubles.
> > >
> > > Some advice here would be worth its weight in gold.
> > >
> > It has to be set before the attribute is created. Are you doing that ? Also, is
> > 'dev' the same device pointer (ie is the 'dev' in your function the same as
> > &pdev->dev) ?
> >
> > Guenter
> 
> Hi Guenter,
> 
> Thanks for the quick reply. Indeed 'dev' in my function is a struct
> watchdog_device's dev (for device_create_file) and not &pdev->dev. Which in turn
> are not the same.
> 
> A call to dev_set_drvdata(a21_wdt.dev, drv); solved the NULL pointer access. So
> last question, is it save to set the drvdata of a dev inside the struct
> watchdog_device or do I break something vital that way?
> 
Good question. At issue is if your driver 'owns' struct watchdog_device,
or if the watchdog core owns it.

However, since you know that a21_wdt.dev is the device, you can use
container_of() to get a reference to a21_wdt, and watchdog_get_drvdata
to get access to drv from a21_wdt. So you should not really need it.

Thanks,
Guenter

Re: Some problems with sysfs patch (was Re: [PATCH v6] watchdog: New watchdog driver for MEN A21 watchdogs)

[Johannes Thumshirn]

On Thu, Jun 06, 2013 at 10:08:28AM -0700, Guenter Roeck wrote:
> On Thu, Jun 06, 2013 at 03:00:55PM +0200, Johannes Thumshirn wrote:
> > On Thu, Jun 06, 2013 at 04:31:10AM -0700, Guenter Roeck wrote:
> > > On Thu, Jun 06, 2013 at 12:51:03PM +0200, Johannes Thumshirn wrote:
> > > > On Mon, Jun 03, 2013 at 11:50:26AM +0200, Johannes Thumshirn wrote:
> > > > > On Fri, May 31, 2013 at 09:15:23PM -0700, Guenter Roeck wrote:
> > > > > > On Fri, May 31, 2013 at 02:55:19PM +0200, Johannes Thumshirn wrote:
> > > > > > > Hi Guenther,
> > > > > > > On Fri, May 31, 2013 at 04:40:37AM -0700, Guenter Roeck wrote:
> > > > > > > > > +#define GPIO_WD_ENAB	169
> > > > > > > > > +#define GPIO_WD_FAST	170
> > > > > > > > > +#define GPIO_WD_TRIG	171
> > > > > > > > > +
> > > > > > > > > +#define GPIO_RST_CAUSE_BASE 166
> > > > > > > > > +
> > > > > > > >
> > > > > > > > I think I asked that before ... as you are supporting devicetree, gpio pins
> > > > > > > > should really be provided through devicetree properties and not be hardcoded.
> > > > > > > >
> > > > > > > Yes you did and I didn't come up with a solution to this problem yet. I understand
> > > > > > > and agree to your concerns but I'm lacking example code/documentation for it, maybe
> > > > > > > you can point me to an example on that and then I'll update my code accordingly.
> > > > > > >
> > > > > >
> > > > > > Have a look at Documentation/devicetree/bindings/gpio/gpio-fan.txt and
> > > > > > drivers/hwmon/gpio-fan.c.
> > > > >
> > > > > Thanks a lot, this really helped me out. Updated patch is comming today
> > > > > including the bindings document Arnd Bergmann requested. I only need to rebase
> > > > > the sysfs patch on top of that changes.
> > > > >
> > > > > Regards,
> > > > > Johannes
> > > >
> > > > Regarding the sysfs patch, I have a funny effect in my (rebased) sysfs code.
> > > >
> > > > Given the following code:
> > > >
> > > > +static ssize_t rebootcause_show(struct device *dev,
> > > > +                               struct device_attribute *attr,
> > > > +                               char *buf)
> > > > +{
> > > > +       struct a21_wdt_drv *drv = dev_get_drvdata(dev);
> > > > +       unsigned int reset = 0;
> > > > +
> > > > +       if (!drv)
> > > > +               return -EIO;
> > > > +
> > > > +       reset = a21_wdt_get_bootstatus(drv);
> > > > +
> > > > +       return sprintf(buf, "%s\n", reset_causes[reset]);
> > > > +}
> > > > +static DEVICE_ATTR(rebootcause, S_IRUGO, rebootcause_show, NULL);
> > > >
> > > > I actually need the check for if (!drv) to prevent an OOPS, as
> > > > dev_get_drvdata(dev) returns NULL., though it is set at the end of my probe
> > > > function via:
> > > >
> > > > [...]
> > > > +       ret = watchdog_register_device(&a21_wdt);
> > > > +       if (ret) {
> > > > +               dev_err(&pdev->dev, "Cannot register watchdog device\n");
> > > > +               goto err_register_wd;
> > > > +       }
> > > > +
> > > > +       dev_set_drvdata(&pdev->dev, drv);
> > > > +
> > > > +       return 0;
> > > > [...]
> > > >
> > > > The watchdog driver itself is working without any troubles.
> > > >
> > > > Some advice here would be worth its weight in gold.
> > > >
> > > It has to be set before the attribute is created. Are you doing that ? Also, is
> > > 'dev' the same device pointer (ie is the 'dev' in your function the same as
> > > &pdev->dev) ?
> > >
> > > Guenter
> >
> > Hi Guenter,
> >
> > Thanks for the quick reply. Indeed 'dev' in my function is a struct
> > watchdog_device's dev (for device_create_file) and not &pdev->dev. Which in turn
> > are not the same.
> >
> > A call to dev_set_drvdata(a21_wdt.dev, drv); solved the NULL pointer access. So
> > last question, is it save to set the drvdata of a dev inside the struct
> > watchdog_device or do I break something vital that way?
> >
> Good question. At issue is if your driver 'owns' struct watchdog_device,
> or if the watchdog core owns it.
>
> However, since you know that a21_wdt.dev is the device, you can use
> container_of() to get a reference to a21_wdt, and watchdog_get_drvdata
> to get access to drv from a21_wdt. So you should not really need it.
>
> Thanks,
> Guenter

OK, but doesn't the container_of() macro only work if the parent struct embeds
the child struct and not a pointer to it? But struct watchdog_device only has a
pointer to a struct device (checked my 3.10-rc4 as well as watchdog-next/master,
just in case I missed something).

This would also explain my compiler warnings about "initialization from
incompatible pointer type" when compiling this pice code:

static inline struct a21_wdt_drv *to_a21_wdt_drv(struct device *dev)
{
        struct watchdog_device *wdt = container_of(dev, struct watchdog_device, dev);
        struct a21_wdt_drv *drv = watchdog_get_drvdata(wdt);

        return drv;
}

Regards,
Johannes

Re: Some problems with sysfs patch (was Re: [PATCH v6] watchdog: New watchdog driver for MEN A21 watchdogs)

[Guenter Roeck]

On Fri, Jun 07, 2013 at 09:45:19AM +0200, Johannes Thumshirn wrote:
> On Thu, Jun 06, 2013 at 10:08:28AM -0700, Guenter Roeck wrote:
> > On Thu, Jun 06, 2013 at 03:00:55PM +0200, Johannes Thumshirn wrote:
> > > On Thu, Jun 06, 2013 at 04:31:10AM -0700, Guenter Roeck wrote:
> > > > On Thu, Jun 06, 2013 at 12:51:03PM +0200, Johannes Thumshirn wrote:
> > > > > On Mon, Jun 03, 2013 at 11:50:26AM +0200, Johannes Thumshirn wrote:
> > > > > > On Fri, May 31, 2013 at 09:15:23PM -0700, Guenter Roeck wrote:
> > > > > > > On Fri, May 31, 2013 at 02:55:19PM +0200, Johannes Thumshirn wrote:
> > > > > > > > Hi Guenther,
> > > > > > > > On Fri, May 31, 2013 at 04:40:37AM -0700, Guenter Roeck wrote:
> > > > > > > > > > +#define GPIO_WD_ENAB	169
> > > > > > > > > > +#define GPIO_WD_FAST	170
> > > > > > > > > > +#define GPIO_WD_TRIG	171
> > > > > > > > > > +
> > > > > > > > > > +#define GPIO_RST_CAUSE_BASE 166
> > > > > > > > > > +
> > > > > > > > >
> > > > > > > > > I think I asked that before ... as you are supporting devicetree, gpio pins
> > > > > > > > > should really be provided through devicetree properties and not be hardcoded.
> > > > > > > > >
> > > > > > > > Yes you did and I didn't come up with a solution to this problem yet. I understand
> > > > > > > > and agree to your concerns but I'm lacking example code/documentation for it, maybe
> > > > > > > > you can point me to an example on that and then I'll update my code accordingly.
> > > > > > > >
> > > > > > >
> > > > > > > Have a look at Documentation/devicetree/bindings/gpio/gpio-fan.txt and
> > > > > > > drivers/hwmon/gpio-fan.c.
> > > > > >
> > > > > > Thanks a lot, this really helped me out. Updated patch is comming today
> > > > > > including the bindings document Arnd Bergmann requested. I only need to rebase
> > > > > > the sysfs patch on top of that changes.
> > > > > >
> > > > > > Regards,
> > > > > > Johannes
> > > > >
> > > > > Regarding the sysfs patch, I have a funny effect in my (rebased) sysfs code.
> > > > >
> > > > > Given the following code:
> > > > >
> > > > > +static ssize_t rebootcause_show(struct device *dev,
> > > > > +                               struct device_attribute *attr,
> > > > > +                               char *buf)
> > > > > +{
> > > > > +       struct a21_wdt_drv *drv = dev_get_drvdata(dev);
> > > > > +       unsigned int reset = 0;
> > > > > +
> > > > > +       if (!drv)
> > > > > +               return -EIO;
> > > > > +
> > > > > +       reset = a21_wdt_get_bootstatus(drv);
> > > > > +
> > > > > +       return sprintf(buf, "%s\n", reset_causes[reset]);
> > > > > +}
> > > > > +static DEVICE_ATTR(rebootcause, S_IRUGO, rebootcause_show, NULL);
> > > > >
> > > > > I actually need the check for if (!drv) to prevent an OOPS, as
> > > > > dev_get_drvdata(dev) returns NULL., though it is set at the end of my probe
> > > > > function via:
> > > > >
> > > > > [...]
> > > > > +       ret = watchdog_register_device(&a21_wdt);
> > > > > +       if (ret) {
> > > > > +               dev_err(&pdev->dev, "Cannot register watchdog device\n");
> > > > > +               goto err_register_wd;
> > > > > +       }
> > > > > +
> > > > > +       dev_set_drvdata(&pdev->dev, drv);
> > > > > +
> > > > > +       return 0;
> > > > > [...]
> > > > >
> > > > > The watchdog driver itself is working without any troubles.
> > > > >
> > > > > Some advice here would be worth its weight in gold.
> > > > >
> > > > It has to be set before the attribute is created. Are you doing that ? Also, is
> > > > 'dev' the same device pointer (ie is the 'dev' in your function the same as
> > > > &pdev->dev) ?
> > > >
> > > > Guenter
> > >
> > > Hi Guenter,
> > >
> > > Thanks for the quick reply. Indeed 'dev' in my function is a struct
> > > watchdog_device's dev (for device_create_file) and not &pdev->dev. Which in turn
> > > are not the same.
> > >
> > > A call to dev_set_drvdata(a21_wdt.dev, drv); solved the NULL pointer access. So
> > > last question, is it save to set the drvdata of a dev inside the struct
> > > watchdog_device or do I break something vital that way?
> > >
> > Good question. At issue is if your driver 'owns' struct watchdog_device,
> > or if the watchdog core owns it.
> >
> > However, since you know that a21_wdt.dev is the device, you can use
> > container_of() to get a reference to a21_wdt, and watchdog_get_drvdata
> > to get access to drv from a21_wdt. So you should not really need it.
> >
> > Thanks,
> > Guenter
> 
> OK, but doesn't the container_of() macro only work if the parent struct embeds
> the child struct and not a pointer to it? But struct watchdog_device only has a
> pointer to a struct device (checked my 3.10-rc4 as well as watchdog-next/master,
> just in case I missed something).
> 
Yes, you are right, sorry for the noise.

Guenter

Re: Some problems with sysfs patch (was Re: [PATCH v6] watchdog: New watchdog driver for MEN A21 watchdogs)

[Johannes Thumshirn]

On Fri, Jun 07, 2013 at 02:14:19AM -0700, Guenter Roeck wrote:
> On Fri, Jun 07, 2013 at 09:45:19AM +0200, Johannes Thumshirn wrote:
> > On Thu, Jun 06, 2013 at 10:08:28AM -0700, Guenter Roeck wrote:
> > > On Thu, Jun 06, 2013 at 03:00:55PM +0200, Johannes Thumshirn wrote:
> > > > On Thu, Jun 06, 2013 at 04:31:10AM -0700, Guenter Roeck wrote:
> > > > > On Thu, Jun 06, 2013 at 12:51:03PM +0200, Johannes Thumshirn wrote:
> > > > > > On Mon, Jun 03, 2013 at 11:50:26AM +0200, Johannes Thumshirn wrote:
> > > > > > > On Fri, May 31, 2013 at 09:15:23PM -0700, Guenter Roeck wrote:
> > > > > > > > On Fri, May 31, 2013 at 02:55:19PM +0200, Johannes Thumshirn wrote:
> > > > > > > > > Hi Guenther,
> > > > > > > > > On Fri, May 31, 2013 at 04:40:37AM -0700, Guenter Roeck wrote:
> > > > > > > > > > > +#define GPIO_WD_ENAB	169
> > > > > > > > > > > +#define GPIO_WD_FAST	170
> > > > > > > > > > > +#define GPIO_WD_TRIG	171
> > > > > > > > > > > +
> > > > > > > > > > > +#define GPIO_RST_CAUSE_BASE 166
> > > > > > > > > > > +
> > > > > > > > > >
> > > > > > > > > > I think I asked that before ... as you are supporting devicetree, gpio pins
> > > > > > > > > > should really be provided through devicetree properties and not be hardcoded.
> > > > > > > > > >
> > > > > > > > > Yes you did and I didn't come up with a solution to this problem yet. I understand
> > > > > > > > > and agree to your concerns but I'm lacking example code/documentation for it, maybe
> > > > > > > > > you can point me to an example on that and then I'll update my code accordingly.
> > > > > > > > >
> > > > > > > >
> > > > > > > > Have a look at Documentation/devicetree/bindings/gpio/gpio-fan.txt and
> > > > > > > > drivers/hwmon/gpio-fan.c.
> > > > > > >
> > > > > > > Thanks a lot, this really helped me out. Updated patch is comming today
> > > > > > > including the bindings document Arnd Bergmann requested. I only need to rebase
> > > > > > > the sysfs patch on top of that changes.
> > > > > > >
> > > > > > > Regards,
> > > > > > > Johannes
> > > > > >
> > > > > > Regarding the sysfs patch, I have a funny effect in my (rebased) sysfs code.
> > > > > >
> > > > > > Given the following code:
> > > > > >
> > > > > > +static ssize_t rebootcause_show(struct device *dev,
> > > > > > +                               struct device_attribute *attr,
> > > > > > +                               char *buf)
> > > > > > +{
> > > > > > +       struct a21_wdt_drv *drv = dev_get_drvdata(dev);
> > > > > > +       unsigned int reset = 0;
> > > > > > +
> > > > > > +       if (!drv)
> > > > > > +               return -EIO;
> > > > > > +
> > > > > > +       reset = a21_wdt_get_bootstatus(drv);
> > > > > > +
> > > > > > +       return sprintf(buf, "%s\n", reset_causes[reset]);
> > > > > > +}
> > > > > > +static DEVICE_ATTR(rebootcause, S_IRUGO, rebootcause_show, NULL);
> > > > > >
> > > > > > I actually need the check for if (!drv) to prevent an OOPS, as
> > > > > > dev_get_drvdata(dev) returns NULL., though it is set at the end of my probe
> > > > > > function via:
> > > > > >
> > > > > > [...]
> > > > > > +       ret = watchdog_register_device(&a21_wdt);
> > > > > > +       if (ret) {
> > > > > > +               dev_err(&pdev->dev, "Cannot register watchdog device\n");
> > > > > > +               goto err_register_wd;
> > > > > > +       }
> > > > > > +
> > > > > > +       dev_set_drvdata(&pdev->dev, drv);
> > > > > > +
> > > > > > +       return 0;
> > > > > > [...]
> > > > > >
> > > > > > The watchdog driver itself is working without any troubles.
> > > > > >
> > > > > > Some advice here would be worth its weight in gold.
> > > > > >
> > > > > It has to be set before the attribute is created. Are you doing that ? Also, is
> > > > > 'dev' the same device pointer (ie is the 'dev' in your function the same as
> > > > > &pdev->dev) ?
> > > > >
> > > > > Guenter
> > > >
> > > > Hi Guenter,
> > > >
> > > > Thanks for the quick reply. Indeed 'dev' in my function is a struct
> > > > watchdog_device's dev (for device_create_file) and not &pdev->dev. Which in turn
> > > > are not the same.
> > > >
> > > > A call to dev_set_drvdata(a21_wdt.dev, drv); solved the NULL pointer access. So
> > > > last question, is it save to set the drvdata of a dev inside the struct
> > > > watchdog_device or do I break something vital that way?
> > > >
> > > Good question. At issue is if your driver 'owns' struct watchdog_device,
> > > or if the watchdog core owns it.
> > >
> > > However, since you know that a21_wdt.dev is the device, you can use
> > > container_of() to get a reference to a21_wdt, and watchdog_get_drvdata
> > > to get access to drv from a21_wdt. So you should not really need it.
> > >
> > > Thanks,
> > > Guenter
> >
> > OK, but doesn't the container_of() macro only work if the parent struct embeds
> > the child struct and not a pointer to it? But struct watchdog_device only has a
> > pointer to a struct device (checked my 3.10-rc4 as well as watchdog-next/master,
> > just in case I missed something).
> >
> Yes, you are right, sorry for the noise.
>
> Guenter

No problem. Actually I hoped you tell me I was wrong and point me to the right
practice for this.

Anyway I'll send the patch as it is and hope it's clean for inclusion.

Thanks for your help.

Johannes

[PATCH v8 2/2] watchdog: Sysfs interface for MEN A21 watchdog

[Johannes Thumshirn]

This patch adds a sysfs interface for the watchdog
device found on MEN A21 Boards.

The newly generated files are:
* rebootcause:
Can be one of:
Power on Reset,
CPU Reset Request,
Push Button,
FPGA Reset Request,
Watchdog,
Local Power Bad,
Invalid or
BDI
and shows the reason of the boards last reboot.

* active:
Shows if the watchdog CPLD is actually running

* allow_disable:
Shows if the watchdog is allowed to be disabled (NOWAYOUT disabled)

* fastmode:
Shows if the CPLD is running in fast mode (1s timeout), once it is in
fastmode it can't be switched back to slow mode (30s timeout) until the
next reboot.

Revision 2:
* Re-worked sysfs patch to apply on re-worked base

Revision 3:
* Re-worked sysfs patch to apply on re-worked base
Signed-off-by: Johannes Thumshirn <johannes.thumshirn@men.de>
---
 drivers/watchdog/mena21_wdt.c |   88 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 88 insertions(+)

diff --git a/drivers/watchdog/mena21_wdt.c b/drivers/watchdog/mena21_wdt.c
index 29f9c0d..cae5462 100644
--- a/drivers/watchdog/mena21_wdt.c
+++ b/drivers/watchdog/mena21_wdt.c
@@ -20,6 +20,17 @@
 #include <linux/delay.h>
 #include <linux/bitops.h>

+static char *reset_causes[] = {
+	"Power On Reset",
+	"CPU Reset Request",
+	"Push Button",
+	"FPGA Reset Request",
+	"Watchdog",
+	"Local Power Bad",
+	"Invalid",
+	"BDI",
+};
+
 enum a21_wdt_gpios {
 	GPIO_WD_ENAB,
 	GPIO_WD_FAST,
@@ -52,6 +63,73 @@ static unsigned int a21_wdt_get_bootstatus(struct a21_wdt_drv *drv)
 	return reset;
 }

+static ssize_t rebootcause_show(struct device *dev,
+				struct device_attribute *attr,
+				char *buf)
+{
+	struct a21_wdt_drv *drv = dev_get_drvdata(dev);
+	unsigned int reset = 0;
+
+	reset = a21_wdt_get_bootstatus(drv);
+
+	return sprintf(buf, "%s\n", reset_causes[reset]);
+}
+static DEVICE_ATTR(rebootcause, S_IRUGO, rebootcause_show, NULL);
+
+static ssize_t active_show(struct device *dev, struct device_attribute *attr,
+			   char *buf)
+{
+	struct a21_wdt_drv *drv = dev_get_drvdata(dev);
+
+	return sprintf(buf, "%d\n", !!gpio_get_value(drv->gpios[GPIO_WD_ENAB]));
+}
+static DEVICE_ATTR(active, S_IRUGO, active_show, NULL);
+
+static ssize_t allow_disable_show(struct device *dev,
+				  struct device_attribute *attr,
+				  char *buf)
+{
+	return sprintf(buf, "%d\n", !nowayout);
+}
+static DEVICE_ATTR(allow_disable, S_IRUGO, allow_disable_show, NULL);
+
+static ssize_t fastmode_show(struct device *dev, struct device_attribute *attr,
+			     char *buf)
+{
+	struct a21_wdt_drv *drv = dev_get_drvdata(dev);
+
+	return sprintf(buf, "%d\n", !!gpio_get_value(drv->gpios[GPIO_WD_FAST]));
+}
+static DEVICE_ATTR(fastmode, S_IRUGO, fastmode_show, NULL);
+
+static int a21_wdt_create_files(struct watchdog_device *wdev)
+{
+	int ret;
+
+	ret = device_create_file(wdev->dev, &dev_attr_rebootcause);
+	if (ret)
+		return ret;
+	ret = device_create_file(wdev->dev, &dev_attr_active);
+	if (ret)
+		return ret;
+	ret = device_create_file(wdev->dev, &dev_attr_allow_disable);
+	if (ret)
+		return ret;
+	ret = device_create_file(wdev->dev, &dev_attr_fastmode);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static void a21_wdt_remove_files(struct watchdog_device *wdev)
+{
+	device_remove_file(wdev->dev, &dev_attr_rebootcause);
+	device_remove_file(wdev->dev, &dev_attr_active);
+	device_remove_file(wdev->dev, &dev_attr_allow_disable);
+	device_remove_file(wdev->dev, &dev_attr_fastmode);
+}
+
 static int a21_wdt_start(struct watchdog_device *wdt)
 {
 	struct a21_wdt_drv *drv = watchdog_get_drvdata(wdt);
@@ -223,9 +301,18 @@ static int a21_wdt_probe(struct platform_device *pdev)
 	}

 	dev_set_drvdata(&pdev->dev, drv);
+	dev_set_drvdata(a21_wdt.dev, drv);
+
+	ret = a21_wdt_create_files(&a21_wdt);
+	if (ret) {
+		dev_err(&pdev->dev, "Cannot create sysfs entries\n");
+		goto err_create_sysfs;
+	}

 	return 0;

+err_create_sysfs:
+	watchdog_unregister_device(&drv->wdt);
 err_register_wd:
 	mutex_destroy(&drv->lock);

@@ -239,6 +326,7 @@ static int a21_wdt_remove(struct platform_device *pdev)
 	dev_warn(&pdev->dev,
 		"Unregistering A21 watchdog driver, board may reboot\n");

+	a21_wdt_remove_files(&drv->wdt);

 	watchdog_unregister_device(&drv->wdt);

--
1.7.9.5

Re: [PATCH v8] watchdog: New watchdog driver for MEN A21 watchdogs

[Guenter Roeck]

On Mon, Jun 03, 2013 at 04:34:59PM +0200, Johannes Thumshirn wrote:
> This patch adds the driver for the watchdog devices found on MEN Mikro
> Elektronik A21 VMEbus CPU Carrier Boards. It has DT-support and uses the
> watchdog framework.
> 
> Revision 2:
> * Removed unneeded open flag in struct a21_wdt_drv
> * Corrected 3bit reason code from gpio
> * Additional sysfs files are now part of watchdog sysfs
> * Changed OFF/ON delay in ping from 400ms to 10ns
> * Reworked timeout setting
> * Removed a21_wdt_ioctl(...)
> 
> Revision 3:
> * Changed pr_{err,info} to dev_{err,info}
> * Removed out of memory error print
> * Transition from "fast" to "slow" mode not allowed by chip
> 
> Revision 4:
> * Remove reboot_notifier and place disable code into platform_device's shutdown function
> * Removed sysfs interface
> 
> Revision 5:
> * Added setting of .bootstatus on driver init
> * Added initial timeout on driver init
> 
> Revision 6:
> * Use watchdog_init_timeout() to initialize timeout
> 
> Revision 7:
> * Fix possible get_bootstatus race condition
> 
> Revision 8:
> * a21_wdt_get_bootstatus() should return reset code
> * GPIOs are supplied via DT instead of being hardcoded. Code derived from
>   (drivers/hwmon/gpio-fan.c)
> * Added Devicetree binding document
> * Driver now depends on GPIOLIB
> 
> Signed-off-by: Johannes Thumshirn <johannes.thumshirn@men.de>

Almost good. One comment below.

Thanks,
Guenter

> ---
>  .../devicetree/bindings/gpio/men-a021-wdt.txt      |   25 ++
>  MAINTAINERS                                        |    6 +
>  drivers/watchdog/Kconfig                           |   12 +
>  drivers/watchdog/Makefile                          |    1 +
>  drivers/watchdog/mena21_wdt.c                      |  277 ++++++++++++++++++++
>  5 files changed, 321 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/gpio/men-a021-wdt.txt
>  create mode 100644 drivers/watchdog/mena21_wdt.c
> 
> diff --git a/Documentation/devicetree/bindings/gpio/men-a021-wdt.txt b/Documentation/devicetree/bindings/gpio/men-a021-wdt.txt
> new file mode 100644
> index 0000000..370dee3
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpio/men-a021-wdt.txt
> @@ -0,0 +1,25 @@
> +Bindings for MEN A21 Watchdog device connected to GPIO lines
> +
> +Required properties:
> +- compatible: "men,a021-wdt"
> +- gpios: Specifies the pins that control the Watchdog, order:
> +  1: Watchdog enable
> +  2: Watchdog fast-mode
> +  3: Watchdog trigger
> +  4: Watchdog reset cause bit 0
> +  5: Watchdog reset cause bit 1
> +  6: Watchdog reset cause bit 2
> +
> +Optional properties:
> +- None
> +
> +Example:
> +	watchdog {
> +		compatible ="men,a021-wdt";
> +		gpios = <&gpio3 9  1	/* WD_EN */
> +			 &gpio3 10 1	/* WD_FAST */
> +			 &gpio3 11 1	/* WD_TRIG */
> +			 &gpio3 6  1	/* RST_CAUSE[0] */
> +			 &gpio3 7  1	/* RST_CAUSE[1] */
> +			 &gpio3 8  1>;	/* RST_CAUSE[2] */
> +	};
> diff --git a/MAINTAINERS b/MAINTAINERS
> index f35a259..bd86661 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -5307,6 +5307,12 @@ F:	drivers/mtd/
>  F:	include/linux/mtd/
>  F:	include/uapi/mtd/
> 
> +MEN A21 WATCHDOG DRIVER
> +M:  	Johannes Thumshirn <johannes.thumshirn@men.de>
> +L:	linux-watchdog@vger.kernel.org
> +S:	Supported
> +F:	drivers/watchdog/mena21_wdt.c
> +
>  METAG ARCHITECTURE
>  M:	James Hogan <james.hogan@imgtec.com>
>  S:	Supported
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index e89fc31..8b143ee 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -1172,6 +1172,18 @@ config BOOKE_WDT_DEFAULT_TIMEOUT
> 
>  	  The value can be overridden by the wdt_period command-line parameter.
> 
> +config MEN_A21_WDT
> +       tristate "MEN A21 VME CPU Carrier Board Watchdog Timer"
> +       select WATCHDOG_CORE
> +       depends on GPIOLIB
> +       help
> +        Watchdog driver for MEN A21 VMEbus CPU Carrier Boards.
> +
> +	The driver can also be built as a module. If so, the module will be
> +	called mena21_wdt.
> +
> +	If unsure select N here.
> +
>  # PPC64 Architecture
> 
>  config WATCHDOG_RTAS
> diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
> index a300b94..bffdcb1 100644
> --- a/drivers/watchdog/Makefile
> +++ b/drivers/watchdog/Makefile
> @@ -143,6 +143,7 @@ obj-$(CONFIG_8xxx_WDT) += mpc8xxx_wdt.o
>  obj-$(CONFIG_MV64X60_WDT) += mv64x60_wdt.o
>  obj-$(CONFIG_PIKA_WDT) += pika_wdt.o
>  obj-$(CONFIG_BOOKE_WDT) += booke_wdt.o
> +obj-$(CONFIG_MEN_A21_WDT) += mena21_wdt.o
> 
>  # PPC64 Architecture
>  obj-$(CONFIG_WATCHDOG_RTAS) += wdrtas.o
> diff --git a/drivers/watchdog/mena21_wdt.c b/drivers/watchdog/mena21_wdt.c
> new file mode 100644
> index 0000000..29f9c0d
> --- /dev/null
> +++ b/drivers/watchdog/mena21_wdt.c
> @@ -0,0 +1,277 @@
> +/*
> + * Watchdog driver for the A21 VME CPU Boards
> + *
> + * Copyright (C) 2013 MEN Mikro Elektronik Nuernberg GmbH
> + *
> + * 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
> + */
> +#include <linux/module.h>
> +#include <linux/moduleparam.h>
> +#include <linux/types.h>
> +#include <linux/kernel.h>
> +#include <linux/slab.h>
> +#include <linux/platform_device.h>
> +#include <linux/watchdog.h>
> +#include <linux/uaccess.h>
> +#include <linux/gpio.h>
> +#include <linux/of_gpio.h>
> +#include <linux/delay.h>
> +#include <linux/bitops.h>
> +
> +enum a21_wdt_gpios {
> +	GPIO_WD_ENAB,
> +	GPIO_WD_FAST,
> +	GPIO_WD_TRIG,
> +	GPIO_WD_RST0,
> +	GPIO_WD_RST1,
> +	GPIO_WD_RST2,
> +};
> +
> +struct a21_wdt_drv {
> +	struct watchdog_device wdt;
> +	struct mutex lock;
> +	int num_gpios;
> +	unsigned *gpios;
> +};
> +
> +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 unsigned int a21_wdt_get_bootstatus(struct a21_wdt_drv *drv)
> +{
> +	int reset = 0;
> +
> +	reset |= gpio_get_value(drv->gpios[GPIO_WD_RST0]) ? (1 << 0) : 0;
> +	reset |= gpio_get_value(drv->gpios[GPIO_WD_RST1]) ? (1 << 1) : 0;
> +	reset |= gpio_get_value(drv->gpios[GPIO_WD_RST2]) ? (1 << 2) : 0;
> +
> +	return reset;
> +}
> +
> +static int a21_wdt_start(struct watchdog_device *wdt)
> +{
> +	struct a21_wdt_drv *drv = watchdog_get_drvdata(wdt);
> +
> +	mutex_lock(&drv->lock);
> +
> +	gpio_set_value(drv->gpios[GPIO_WD_ENAB], 1);
> +
> +	mutex_unlock(&drv->lock);
> +
> +	return 0;
> +}
> +
> +static int a21_wdt_stop(struct watchdog_device *wdt)
> +{
> +	struct a21_wdt_drv *drv = watchdog_get_drvdata(wdt);
> +
> +	/* We don't stop if WDOG_NO_WAY_OUT is set */
> +	if (test_bit(WDOG_NO_WAY_OUT, &wdt->status))
> +		return -EINVAL;
> +
> +	mutex_lock(&drv->lock);
> +
> +	gpio_set_value(drv->gpios[GPIO_WD_ENAB], 0);
> +
> +	mutex_unlock(&drv->lock);
> +
> +	return 0;
> +}
> +
> +static int a21_wdt_ping(struct watchdog_device *wdt)
> +{
> +	struct a21_wdt_drv *drv = watchdog_get_drvdata(wdt);
> +
> +	mutex_lock(&drv->lock);
> +
> +	gpio_set_value(drv->gpios[GPIO_WD_TRIG], 0);
> +	ndelay(10);
> +	gpio_set_value(drv->gpios[GPIO_WD_TRIG], 1);
> +
> +	mutex_unlock(&drv->lock);
> +
> +	return 0;
> +}
> +
> +static int a21_wdt_set_timeout(struct watchdog_device *wdt,
> +			       unsigned int timeout)
> +{
> +	struct a21_wdt_drv *drv = watchdog_get_drvdata(wdt);
> +
> +	if (timeout != 1 && timeout != 30) {
> +		dev_err(wdt->dev, "Only 1 and 30 allowed as timeout\n");
> +		return -EINVAL;
> +	}
> +
> +	if (timeout == 30 && wdt->timeout == 1) {
> +		dev_err(wdt->dev,
> +			"Transition from fast to slow mode not allowed\n");
> +		return -EINVAL;
> +	}
> +
> +	mutex_lock(&drv->lock);
> +
> +	if (timeout == 1)
> +		gpio_set_value(drv->gpios[GPIO_WD_FAST], 1);
> +	else
> +		gpio_set_value(drv->gpios[GPIO_WD_FAST], 0);
> +
> +	wdt->timeout = timeout;
> +
> +	mutex_unlock(&drv->lock);
> +
> +	return 0;
> +}
> +
> +static const struct watchdog_info a21_wdt_info = {
> +	.options = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE,
> +	.identity = "MEN A21 Watchdog",
> +};
> +
> +static const struct watchdog_ops a21_wdt_ops = {
> +	.owner = THIS_MODULE,
> +	.start = a21_wdt_start,
> +	.stop = a21_wdt_stop,
> +	.ping = a21_wdt_ping,
> +	.set_timeout = a21_wdt_set_timeout,
> +};
> +
> +static struct watchdog_device a21_wdt = {
> +	.info = &a21_wdt_info,
> +	.ops = &a21_wdt_ops,
> +	.min_timeout = 1,
> +	.max_timeout = 30,
> +};
> +
> +static int a21_wdt_probe(struct platform_device *pdev)
> +{
> +	struct device_node *node;
> +	struct a21_wdt_drv *drv;
> +	unsigned int reset = 0;
> +	unsigned *gpios;
> +	int ret;
> +	int i;
> +
> +	dev_info(&pdev->dev, "MEN A21 watchdog timer driver enabled\n");
> +
> +	drv = devm_kzalloc(&pdev->dev, sizeof(struct a21_wdt_drv), GFP_KERNEL);
> +	if (!drv)
> +		return -ENOMEM;
> +
> +	/* Fill GPIO pin array */
> +	node = pdev->dev.of_node;
> +
> +	drv->num_gpios = of_gpio_count(node);
> +	if (drv->num_gpios <= 0) {
> +		dev_err(&pdev->dev, "gpios DT property empty / missing");
> +		return -ENODEV;
> +	}

Not happy with this. Code accepts one gpio pin, but then assumes throughout the
driver that all pins are defined. I am quite sure I can get it to crash if I
specify only a single gpio pin. num_gpios will be 1, you allocate a single
entry, but then access all 6 of them.

I think it would be easier if you expect a fixed number of gpio pins (6) and abort
if it is not correct. You can then make drv->gpios an array and don't have to
allocate it separately, and you don't need drv->num_gpios (which you don't need
anyway as it is not used outside this function).

> +
> +	gpios = devm_kzalloc(&pdev->dev, drv->num_gpios * sizeof(unsigned),
> +			     GFP_KERNEL);
> +	if (!gpios)
> +		return -ENOMEM;
> +
> +	for (i = 0; i < drv->num_gpios; i++) {
> +		int val;
> +
> +		val = of_get_gpio(node, i);
> +		if (val < 0)
> +			return val;
> +
> +		gpios[i] = val;
> +	}
> +	drv->gpios = gpios;
> +
> +	/* Request the used GPIOs */
> +	for (i = 0; i < drv->num_gpios; i++) {
> +		ret = devm_gpio_request(&pdev->dev, drv->gpios[i],
> +					"MEN A21 Watchdog");
> +		if (ret)
> +			return ret;
> +
> +		ret = gpio_direction_output(drv->gpios[i],
> +					    gpio_get_value(drv->gpios[i]));
> +		if (ret)
> +			return ret;
> +	}
> +
> +	mutex_init(&drv->lock);
> +	watchdog_init_timeout(&a21_wdt, 30, &pdev->dev);
> +	watchdog_set_nowayout(&a21_wdt, nowayout);
> +	watchdog_set_drvdata(&a21_wdt, drv);
> +
> +	reset = a21_wdt_get_bootstatus(drv);
> +	if (reset == 2)
> +		a21_wdt.bootstatus |= WDIOF_EXTERN1;
> +	else if (reset == 4)
> +		a21_wdt.bootstatus |= WDIOF_CARDRESET;
> +	else if (reset == 5)
> +		a21_wdt.bootstatus |= WDIOF_POWERUNDER;
> +	else if (reset == 7)
> +		a21_wdt.bootstatus |= WDIOF_EXTERN2;
> +
> +
> +	ret = watchdog_register_device(&a21_wdt);
> +	if (ret) {
> +		dev_err(&pdev->dev, "Cannot register watchdog device\n");
> +		goto err_register_wd;
> +	}
> +
> +	dev_set_drvdata(&pdev->dev, drv);
> +
> +	return 0;
> +
> +err_register_wd:
> +	mutex_destroy(&drv->lock);
> +
> +	return ret;
> +}
> +
> +static int a21_wdt_remove(struct platform_device *pdev)
> +{
> +	struct a21_wdt_drv *drv = dev_get_drvdata(&pdev->dev);
> +
> +	dev_warn(&pdev->dev,
> +		"Unregistering A21 watchdog driver, board may reboot\n");
> +
> +
> +	watchdog_unregister_device(&drv->wdt);
> +
> +	mutex_destroy(&drv->lock);
> +
> +	return 0;
> +}
> +
> +static void a21_wdt_shutdown(struct platform_device *pdev)
> +{
> +	struct a21_wdt_drv *drv = dev_get_drvdata(&pdev->dev);
> +
> +	gpio_set_value(drv->gpios[GPIO_WD_ENAB], 0);
> +}
> +
> +static const struct of_device_id a21_wdt_ids[] = {
> +	{ .compatible = "men,a021-wdt" },
> +	{ },
> +};
> +
> +static struct platform_driver a21_wdt_driver = {
> +	.probe = a21_wdt_probe,
> +	.remove = a21_wdt_remove,
> +	.shutdown = a21_wdt_shutdown,
> +	.driver = {
> +		.name = "a21-watchdog",
> +		.of_match_table = a21_wdt_ids,
> +	},
> +};
> +
> +module_platform_driver(a21_wdt_driver);
> +
> +MODULE_AUTHOR("MEN Mikro Elektronik");
> +MODULE_DESCRIPTION("MEN A21 Watchdog");
> +MODULE_LICENSE("GPL");
> +MODULE_ALIAS("platform:a21-watchdog");
> --
> 1.7.9.5
> 
> --
> 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
> 

Re: [PATCH v8] watchdog: New watchdog driver for MEN A21 watchdogs

[Johannes Thumshirn]

On Thu, Jun 13, 2013 at 08:55:33PM -0700, Guenter Roeck wrote:
[...]
> > Signed-off-by: Johannes Thumshirn <johannes.thumshirn@men.de>
>
> Almost good. One comment below.
>
> Thanks,
> Guenter
>
Nice to hear. Thanks for the review by the way.

[...]

> > +
> > +	drv->num_gpios = of_gpio_count(node);
> > +	if (drv->num_gpios <= 0) {
> > +		dev_err(&pdev->dev, "gpios DT property empty / missing");
> > +		return -ENODEV;
> > +	}
>
> Not happy with this. Code accepts one gpio pin, but then assumes throughout the
> driver that all pins are defined. I am quite sure I can get it to crash if I
> specify only a single gpio pin. num_gpios will be 1, you allocate a single
> entry, but then access all 6 of them.
>
> I think it would be easier if you expect a fixed number of gpio pins (6) and abort
> if it is not correct. You can then make drv->gpios an array and don't have to
> allocate it separately, and you don't need drv->num_gpios (which you don't need
> anyway as it is not used outside this function).
>

Yup, you're right. Now that you're saying it I see it as well. This is probably a
copy'n'paste error from drivers/hwmon/gpio-fan.c.

@Wim are there any objections from your side?

Byte
Johannes

[PATCH v9] watchdog: New watchdog driver for MEN A21 watchdogs

[Johannes Thumshirn]

This patch adds the driver for the watchdog devices found on MEN Mikro
Elektronik A21 VMEbus CPU Carrier Boards. It has DT-support and uses the
watchdog framework.

Signed-off-by: Johannes Thumshirn <johannes.thumshirn@men.de>
---

 Revision 2:
 * Removed unneeded open flag in struct a21_wdt_drv
 * Corrected 3bit reason code from gpio
 * Additional sysfs files are now part of watchdog sysfs
 * Changed OFF/ON delay in ping from 400ms to 10ns
 * Reworked timeout setting
 * Removed a21_wdt_ioctl(...)

 Revision 3:
 * Changed pr_{err,info} to dev_{err,info}
 * Removed out of memory error print
 * Transition from "fast" to "slow" mode not allowed by chip

 Revision 4:
 * Remove reboot_notifier and place disable code into platform_device's shutdown function
 * Removed sysfs interface

 Revision 5:
 * Added setting of .bootstatus on driver init
 * Added initial timeout on driver init

 Revision 6:
 * Use watchdog_init_timeout() to initialize timeout

 Revision 7:
 * Fix possible get_bootstatus race condition

 Revision 8:
 * a21_wdt_get_bootstatus() should return reset code
 * GPIOs are supplied via DT instead of being hardcoded. Code derived from
   (drivers/hwmon/gpio-fan.c)
 * Added Devicetree binding document
 * Driver now depends on GPIOLIB

 Revision 9:
 * We need 6 GPIOs not only 1 to work
 * GPIO_WD_RST[0..2] are inputs

 .../devicetree/bindings/gpio/men-a021-wdt.txt      |   25 ++
 MAINTAINERS                                        |    6 +
 drivers/watchdog/Kconfig                           |   12 +
 drivers/watchdog/Makefile                          |    1 +
 drivers/watchdog/mena21_wdt.c                      |  276 ++++++++++++++++++++
 5 files changed, 320 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/gpio/men-a021-wdt.txt
 create mode 100644 drivers/watchdog/mena21_wdt.c

diff --git a/Documentation/devicetree/bindings/gpio/men-a021-wdt.txt b/Documentation/devicetree/bindings/gpio/men-a021-wdt.txt
new file mode 100644
index 0000000..370dee3
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/men-a021-wdt.txt
@@ -0,0 +1,25 @@
+Bindings for MEN A21 Watchdog device connected to GPIO lines
+
+Required properties:
+- compatible: "men,a021-wdt"
+- gpios: Specifies the pins that control the Watchdog, order:
+  1: Watchdog enable
+  2: Watchdog fast-mode
+  3: Watchdog trigger
+  4: Watchdog reset cause bit 0
+  5: Watchdog reset cause bit 1
+  6: Watchdog reset cause bit 2
+
+Optional properties:
+- None
+
+Example:
+	watchdog {
+		compatible ="men,a021-wdt";
+		gpios = <&gpio3 9  1	/* WD_EN */
+			 &gpio3 10 1	/* WD_FAST */
+			 &gpio3 11 1	/* WD_TRIG */
+			 &gpio3 6  1	/* RST_CAUSE[0] */
+			 &gpio3 7  1	/* RST_CAUSE[1] */
+			 &gpio3 8  1>;	/* RST_CAUSE[2] */
+	};
diff --git a/MAINTAINERS b/MAINTAINERS
index 5be702c..824261e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5317,6 +5317,12 @@ F:	drivers/mtd/
 F:	include/linux/mtd/
 F:	include/uapi/mtd/

+MEN A21 WATCHDOG DRIVER
+M:  	Johannes Thumshirn <johannes.thumshirn@men.de>
+L:	linux-watchdog@vger.kernel.org
+S:	Supported
+F:	drivers/watchdog/mena21_wdt.c
+
 METAG ARCHITECTURE
 M:	James Hogan <james.hogan@imgtec.com>
 S:	Supported
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index e89fc31..8b143ee 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -1172,6 +1172,18 @@ config BOOKE_WDT_DEFAULT_TIMEOUT

 	  The value can be overridden by the wdt_period command-line parameter.

+config MEN_A21_WDT
+       tristate "MEN A21 VME CPU Carrier Board Watchdog Timer"
+       select WATCHDOG_CORE
+       depends on GPIOLIB
+       help
+        Watchdog driver for MEN A21 VMEbus CPU Carrier Boards.
+
+	The driver can also be built as a module. If so, the module will be
+	called mena21_wdt.
+
+	If unsure select N here.
+
 # PPC64 Architecture

 config WATCHDOG_RTAS
diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
index a300b94..bffdcb1 100644
--- a/drivers/watchdog/Makefile
+++ b/drivers/watchdog/Makefile
@@ -143,6 +143,7 @@ obj-$(CONFIG_8xxx_WDT) += mpc8xxx_wdt.o
 obj-$(CONFIG_MV64X60_WDT) += mv64x60_wdt.o
 obj-$(CONFIG_PIKA_WDT) += pika_wdt.o
 obj-$(CONFIG_BOOKE_WDT) += booke_wdt.o
+obj-$(CONFIG_MEN_A21_WDT) += mena21_wdt.o

 # PPC64 Architecture
 obj-$(CONFIG_WATCHDOG_RTAS) += wdrtas.o
diff --git a/drivers/watchdog/mena21_wdt.c b/drivers/watchdog/mena21_wdt.c
new file mode 100644
index 0000000..0d3409a
--- /dev/null
+++ b/drivers/watchdog/mena21_wdt.c
@@ -0,0 +1,276 @@
+/*
+ * Watchdog driver for the A21 VME CPU Boards
+ *
+ * Copyright (C) 2013 MEN Mikro Elektronik Nuernberg GmbH
+ *
+ * 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
+ */
+#include <linux/module.h>
+#include <linux/moduleparam.h>
+#include <linux/types.h>
+#include <linux/kernel.h>
+#include <linux/slab.h>
+#include <linux/platform_device.h>
+#include <linux/watchdog.h>
+#include <linux/uaccess.h>
+#include <linux/gpio.h>
+#include <linux/of_gpio.h>
+#include <linux/delay.h>
+#include <linux/bitops.h>
+
+#define NUM_GPIOS 6
+
+enum a21_wdt_gpios {
+	GPIO_WD_ENAB,
+	GPIO_WD_FAST,
+	GPIO_WD_TRIG,
+	GPIO_WD_RST0,
+	GPIO_WD_RST1,
+	GPIO_WD_RST2,
+};
+
+struct a21_wdt_drv {
+	struct watchdog_device wdt;
+	struct mutex lock;
+	unsigned gpios[NUM_GPIOS];
+};
+
+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 unsigned int a21_wdt_get_bootstatus(struct a21_wdt_drv *drv)
+{
+	int reset = 0;
+
+	reset |= gpio_get_value(drv->gpios[GPIO_WD_RST0]) ? (1 << 0) : 0;
+	reset |= gpio_get_value(drv->gpios[GPIO_WD_RST1]) ? (1 << 1) : 0;
+	reset |= gpio_get_value(drv->gpios[GPIO_WD_RST2]) ? (1 << 2) : 0;
+
+	return reset;
+}
+
+static int a21_wdt_start(struct watchdog_device *wdt)
+{
+	struct a21_wdt_drv *drv = watchdog_get_drvdata(wdt);
+
+	mutex_lock(&drv->lock);
+
+	gpio_set_value(drv->gpios[GPIO_WD_ENAB], 1);
+
+	mutex_unlock(&drv->lock);
+
+	return 0;
+}
+
+static int a21_wdt_stop(struct watchdog_device *wdt)
+{
+	struct a21_wdt_drv *drv = watchdog_get_drvdata(wdt);
+
+	/* We don't stop if WDOG_NO_WAY_OUT is set */
+	if (test_bit(WDOG_NO_WAY_OUT, &wdt->status))
+		return -EINVAL;
+
+	mutex_lock(&drv->lock);
+
+	gpio_set_value(drv->gpios[GPIO_WD_ENAB], 0);
+
+	mutex_unlock(&drv->lock);
+
+	return 0;
+}
+
+static int a21_wdt_ping(struct watchdog_device *wdt)
+{
+	struct a21_wdt_drv *drv = watchdog_get_drvdata(wdt);
+
+	mutex_lock(&drv->lock);
+
+	gpio_set_value(drv->gpios[GPIO_WD_TRIG], 0);
+	ndelay(10);
+	gpio_set_value(drv->gpios[GPIO_WD_TRIG], 1);
+
+	mutex_unlock(&drv->lock);
+
+	return 0;
+}
+
+static int a21_wdt_set_timeout(struct watchdog_device *wdt,
+			       unsigned int timeout)
+{
+	struct a21_wdt_drv *drv = watchdog_get_drvdata(wdt);
+
+	if (timeout != 1 && timeout != 30) {
+		dev_err(wdt->dev, "Only 1 and 30 allowed as timeout\n");
+		return -EINVAL;
+	}
+
+	if (timeout == 30 && wdt->timeout == 1) {
+		dev_err(wdt->dev,
+			"Transition from fast to slow mode not allowed\n");
+		return -EINVAL;
+	}
+
+	mutex_lock(&drv->lock);
+
+	if (timeout == 1)
+		gpio_set_value(drv->gpios[GPIO_WD_FAST], 1);
+	else
+		gpio_set_value(drv->gpios[GPIO_WD_FAST], 0);
+
+	wdt->timeout = timeout;
+
+	mutex_unlock(&drv->lock);
+
+	return 0;
+}
+
+static const struct watchdog_info a21_wdt_info = {
+	.options = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE,
+	.identity = "MEN A21 Watchdog",
+};
+
+static const struct watchdog_ops a21_wdt_ops = {
+	.owner = THIS_MODULE,
+	.start = a21_wdt_start,
+	.stop = a21_wdt_stop,
+	.ping = a21_wdt_ping,
+	.set_timeout = a21_wdt_set_timeout,
+};
+
+static struct watchdog_device a21_wdt = {
+	.info = &a21_wdt_info,
+	.ops = &a21_wdt_ops,
+	.min_timeout = 1,
+	.max_timeout = 30,
+};
+
+static int a21_wdt_probe(struct platform_device *pdev)
+{
+	struct device_node *node;
+	struct a21_wdt_drv *drv;
+	unsigned int reset = 0;
+	int num_gpios;
+	int ret;
+	int i;
+
+	dev_info(&pdev->dev, "MEN A21 watchdog timer driver enabled\n");
+
+	drv = devm_kzalloc(&pdev->dev, sizeof(struct a21_wdt_drv), GFP_KERNEL);
+	if (!drv)
+		return -ENOMEM;
+
+	/* Fill GPIO pin array */
+	node = pdev->dev.of_node;
+
+	num_gpios = of_gpio_count(node);
+	if (num_gpios != NUM_GPIOS) {
+		dev_err(&pdev->dev, "gpios DT property wrong, got %d want %d",
+			num_gpios, NUM_GPIOS);
+		return -ENODEV;
+	}
+
+	for (i = 0; i < num_gpios; i++) {
+		int val;
+
+		val = of_get_gpio(node, i);
+		if (val < 0)
+			return val;
+
+		drv->gpios[i] = val;
+	}
+
+	/* Request the used GPIOs */
+	for (i = 0; i < num_gpios; i++) {
+		ret = devm_gpio_request(&pdev->dev, drv->gpios[i],
+					"MEN A21 Watchdog");
+		if (ret)
+			return ret;
+
+		if (i < GPIO_WD_RST0)
+			ret = gpio_direction_output(drv->gpios[i],
+						gpio_get_value(drv->gpios[i]));
+		else		/* GPIO_WD_RST[0..2] are inputs */
+			ret = gpio_direction_input(drv->gpios[i]);
+		if (ret)
+			return ret;
+	}
+
+	mutex_init(&drv->lock);
+	watchdog_init_timeout(&a21_wdt, 30, &pdev->dev);
+	watchdog_set_nowayout(&a21_wdt, nowayout);
+	watchdog_set_drvdata(&a21_wdt, drv);
+
+	reset = a21_wdt_get_bootstatus(drv);
+	if (reset == 2)
+		a21_wdt.bootstatus |= WDIOF_EXTERN1;
+	else if (reset == 4)
+		a21_wdt.bootstatus |= WDIOF_CARDRESET;
+	else if (reset == 5)
+		a21_wdt.bootstatus |= WDIOF_POWERUNDER;
+	else if (reset == 7)
+		a21_wdt.bootstatus |= WDIOF_EXTERN2;
+
+
+	ret = watchdog_register_device(&a21_wdt);
+	if (ret) {
+		dev_err(&pdev->dev, "Cannot register watchdog device\n");
+		goto err_register_wd;
+	}
+
+	dev_set_drvdata(&pdev->dev, drv);
+
+	return 0;
+
+err_register_wd:
+	mutex_destroy(&drv->lock);
+
+	return ret;
+}
+
+static int a21_wdt_remove(struct platform_device *pdev)
+{
+	struct a21_wdt_drv *drv = dev_get_drvdata(&pdev->dev);
+
+	dev_warn(&pdev->dev,
+		"Unregistering A21 watchdog driver, board may reboot\n");
+
+
+	watchdog_unregister_device(&drv->wdt);
+
+	mutex_destroy(&drv->lock);
+
+	return 0;
+}
+
+static void a21_wdt_shutdown(struct platform_device *pdev)
+{
+	struct a21_wdt_drv *drv = dev_get_drvdata(&pdev->dev);
+
+	gpio_set_value(drv->gpios[GPIO_WD_ENAB], 0);
+}
+
+static const struct of_device_id a21_wdt_ids[] = {
+	{ .compatible = "men,a021-wdt" },
+	{ },
+};
+
+static struct platform_driver a21_wdt_driver = {
+	.probe = a21_wdt_probe,
+	.remove = a21_wdt_remove,
+	.shutdown = a21_wdt_shutdown,
+	.driver = {
+		.name = "a21-watchdog",
+		.of_match_table = a21_wdt_ids,
+	},
+};
+
+module_platform_driver(a21_wdt_driver);
+
+MODULE_AUTHOR("MEN Mikro Elektronik");
+MODULE_DESCRIPTION("MEN A21 Watchdog");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:a21-watchdog");
--
1.7.9.5

[PATCH v9 2/2] watchdog: Sysfs interface for MEN A21 watchdog

[Johannes Thumshirn]

This patch adds a sysfs interface for the watchdog
device found on MEN A21 Boards.

The newly generated files are:
* rebootcause:
Can be one of:
Power on Reset,
CPU Reset Request,
Push Button,
FPGA Reset Request,
Watchdog,
Local Power Bad,
Invalid or
BDI
and shows the reason of the boards last reboot.

* active:
Shows if the watchdog CPLD is actually running

* allow_disable:
Shows if the watchdog is allowed to be disabled (NOWAYOUT disabled)

* fastmode:
Shows if the CPLD is running in fast mode (1s timeout), once it is in
fastmode it can't be switched back to slow mode (30s timeout) until the
next reboot.

Signed-off-by: Johannes Thumshirn <johannes.thumshirn@men.de>
---

 Revision 2:
 * Re-worked sysfs patch to apply on re-worked base

 Revision 3:
 * Re-worked sysfs patch to apply on re-worked base

 Revision 4:
 * Re-worked sysfs patch to apply on re-worked base

 drivers/watchdog/mena21_wdt.c |   88 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 88 insertions(+)

diff --git a/drivers/watchdog/mena21_wdt.c b/drivers/watchdog/mena21_wdt.c
index 0d3409a..03165be 100644
--- a/drivers/watchdog/mena21_wdt.c
+++ b/drivers/watchdog/mena21_wdt.c
@@ -22,6 +22,17 @@

 #define NUM_GPIOS 6

+static char *reset_causes[] = {
+	"Power On Reset",
+	"CPU Reset Request",
+	"Push Button",
+	"FPGA Reset Request",
+	"Watchdog",
+	"Local Power Bad",
+	"Invalid",
+	"BDI",
+};
+
 enum a21_wdt_gpios {
 	GPIO_WD_ENAB,
 	GPIO_WD_FAST,
@@ -53,6 +64,73 @@ static unsigned int a21_wdt_get_bootstatus(struct a21_wdt_drv *drv)
 	return reset;
 }

+static ssize_t rebootcause_show(struct device *dev,
+				struct device_attribute *attr,
+				char *buf)
+{
+	struct a21_wdt_drv *drv = dev_get_drvdata(dev);
+	unsigned int reset = 0;
+
+	reset = a21_wdt_get_bootstatus(drv);
+
+	return sprintf(buf, "%s\n", reset_causes[reset]);
+}
+static DEVICE_ATTR(rebootcause, S_IRUGO, rebootcause_show, NULL);
+
+static ssize_t active_show(struct device *dev, struct device_attribute *attr,
+			   char *buf)
+{
+	struct a21_wdt_drv *drv = dev_get_drvdata(dev);
+
+	return sprintf(buf, "%d\n", !!gpio_get_value(drv->gpios[GPIO_WD_ENAB]));
+}
+static DEVICE_ATTR(active, S_IRUGO, active_show, NULL);
+
+static ssize_t allow_disable_show(struct device *dev,
+				  struct device_attribute *attr,
+				  char *buf)
+{
+	return sprintf(buf, "%d\n", !nowayout);
+}
+static DEVICE_ATTR(allow_disable, S_IRUGO, allow_disable_show, NULL);
+
+static ssize_t fastmode_show(struct device *dev, struct device_attribute *attr,
+			     char *buf)
+{
+	struct a21_wdt_drv *drv = dev_get_drvdata(dev);
+
+	return sprintf(buf, "%d\n", !!gpio_get_value(drv->gpios[GPIO_WD_FAST]));
+}
+static DEVICE_ATTR(fastmode, S_IRUGO, fastmode_show, NULL);
+
+static int a21_wdt_create_files(struct watchdog_device *wdev)
+{
+	int ret;
+
+	ret = device_create_file(wdev->dev, &dev_attr_rebootcause);
+	if (ret)
+		return ret;
+	ret = device_create_file(wdev->dev, &dev_attr_active);
+	if (ret)
+		return ret;
+	ret = device_create_file(wdev->dev, &dev_attr_allow_disable);
+	if (ret)
+		return ret;
+	ret = device_create_file(wdev->dev, &dev_attr_fastmode);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static void a21_wdt_remove_files(struct watchdog_device *wdev)
+{
+	device_remove_file(wdev->dev, &dev_attr_rebootcause);
+	device_remove_file(wdev->dev, &dev_attr_active);
+	device_remove_file(wdev->dev, &dev_attr_allow_disable);
+	device_remove_file(wdev->dev, &dev_attr_fastmode);
+}
+
 static int a21_wdt_start(struct watchdog_device *wdt)
 {
 	struct a21_wdt_drv *drv = watchdog_get_drvdata(wdt);
@@ -222,9 +300,18 @@ static int a21_wdt_probe(struct platform_device *pdev)
 	}

 	dev_set_drvdata(&pdev->dev, drv);
+	dev_set_drvdata(a21_wdt.dev, drv);
+
+	ret = a21_wdt_create_files(&a21_wdt);
+	if (ret) {
+		dev_err(&pdev->dev, "Cannot create sysfs entries\n");
+		goto err_create_sysfs;
+	}

 	return 0;

+err_create_sysfs:
+	watchdog_unregister_device(&drv->wdt);
 err_register_wd:
 	mutex_destroy(&drv->lock);

@@ -238,6 +325,7 @@ static int a21_wdt_remove(struct platform_device *pdev)
 	dev_warn(&pdev->dev,
 		"Unregistering A21 watchdog driver, board may reboot\n");

+	a21_wdt_remove_files(&drv->wdt);

 	watchdog_unregister_device(&drv->wdt);

--
1.7.9.5

Re: [PATCH v9] watchdog: New watchdog driver for MEN A21 watchdogs

[Guenter Roeck]

On Fri, Jun 14, 2013 at 12:58:37PM +0200, Johannes Thumshirn wrote:
> This patch adds the driver for the watchdog devices found on MEN Mikro
> Elektronik A21 VMEbus CPU Carrier Boards. It has DT-support and uses the
> watchdog framework.
> 
> Signed-off-by: Johannes Thumshirn <johannes.thumshirn@men.de>

Only nitpicks this time.

> ---
> 
>  Revision 2:
>  * Removed unneeded open flag in struct a21_wdt_drv
>  * Corrected 3bit reason code from gpio
>  * Additional sysfs files are now part of watchdog sysfs
>  * Changed OFF/ON delay in ping from 400ms to 10ns
>  * Reworked timeout setting
>  * Removed a21_wdt_ioctl(...)
> 
>  Revision 3:
>  * Changed pr_{err,info} to dev_{err,info}
>  * Removed out of memory error print
>  * Transition from "fast" to "slow" mode not allowed by chip
> 
>  Revision 4:
>  * Remove reboot_notifier and place disable code into platform_device's shutdown function
>  * Removed sysfs interface
> 
>  Revision 5:
>  * Added setting of .bootstatus on driver init
>  * Added initial timeout on driver init
> 
>  Revision 6:
>  * Use watchdog_init_timeout() to initialize timeout
> 
>  Revision 7:
>  * Fix possible get_bootstatus race condition
> 
>  Revision 8:
>  * a21_wdt_get_bootstatus() should return reset code
>  * GPIOs are supplied via DT instead of being hardcoded. Code derived from
>    (drivers/hwmon/gpio-fan.c)
>  * Added Devicetree binding document
>  * Driver now depends on GPIOLIB
> 
>  Revision 9:
>  * We need 6 GPIOs not only 1 to work
>  * GPIO_WD_RST[0..2] are inputs
> 
>  .../devicetree/bindings/gpio/men-a021-wdt.txt      |   25 ++
>  MAINTAINERS                                        |    6 +
>  drivers/watchdog/Kconfig                           |   12 +
>  drivers/watchdog/Makefile                          |    1 +
>  drivers/watchdog/mena21_wdt.c                      |  276 ++++++++++++++++++++
>  5 files changed, 320 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/gpio/men-a021-wdt.txt
>  create mode 100644 drivers/watchdog/mena21_wdt.c
> 
> diff --git a/Documentation/devicetree/bindings/gpio/men-a021-wdt.txt b/Documentation/devicetree/bindings/gpio/men-a021-wdt.txt
> new file mode 100644
> index 0000000..370dee3
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpio/men-a021-wdt.txt
> @@ -0,0 +1,25 @@
> +Bindings for MEN A21 Watchdog device connected to GPIO lines
> +
> +Required properties:
> +- compatible: "men,a021-wdt"
> +- gpios: Specifies the pins that control the Watchdog, order:
> +  1: Watchdog enable
> +  2: Watchdog fast-mode
> +  3: Watchdog trigger
> +  4: Watchdog reset cause bit 0
> +  5: Watchdog reset cause bit 1
> +  6: Watchdog reset cause bit 2
> +
> +Optional properties:
> +- None
> +
> +Example:
> +	watchdog {
> +		compatible ="men,a021-wdt";
> +		gpios = <&gpio3 9  1	/* WD_EN */
> +			 &gpio3 10 1	/* WD_FAST */
> +			 &gpio3 11 1	/* WD_TRIG */
> +			 &gpio3 6  1	/* RST_CAUSE[0] */
> +			 &gpio3 7  1	/* RST_CAUSE[1] */
> +			 &gpio3 8  1>;	/* RST_CAUSE[2] */
> +	};
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 5be702c..824261e 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -5317,6 +5317,12 @@ F:	drivers/mtd/
>  F:	include/linux/mtd/
>  F:	include/uapi/mtd/
> 
> +MEN A21 WATCHDOG DRIVER
> +M:  	Johannes Thumshirn <johannes.thumshirn@men.de>
> +L:	linux-watchdog@vger.kernel.org
> +S:	Supported
> +F:	drivers/watchdog/mena21_wdt.c
> +
>  METAG ARCHITECTURE
>  M:	James Hogan <james.hogan@imgtec.com>
>  S:	Supported
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index e89fc31..8b143ee 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -1172,6 +1172,18 @@ config BOOKE_WDT_DEFAULT_TIMEOUT
> 
>  	  The value can be overridden by the wdt_period command-line parameter.
> 
> +config MEN_A21_WDT
> +       tristate "MEN A21 VME CPU Carrier Board Watchdog Timer"
> +       select WATCHDOG_CORE
> +       depends on GPIOLIB
> +       help
> +        Watchdog driver for MEN A21 VMEbus CPU Carrier Boards.
> +
> +	The driver can also be built as a module. If so, the module will be
> +	called mena21_wdt.
> +
> +	If unsure select N here.
> +
>  # PPC64 Architecture
> 
>  config WATCHDOG_RTAS
> diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
> index a300b94..bffdcb1 100644
> --- a/drivers/watchdog/Makefile
> +++ b/drivers/watchdog/Makefile
> @@ -143,6 +143,7 @@ obj-$(CONFIG_8xxx_WDT) += mpc8xxx_wdt.o
>  obj-$(CONFIG_MV64X60_WDT) += mv64x60_wdt.o
>  obj-$(CONFIG_PIKA_WDT) += pika_wdt.o
>  obj-$(CONFIG_BOOKE_WDT) += booke_wdt.o
> +obj-$(CONFIG_MEN_A21_WDT) += mena21_wdt.o
> 
>  # PPC64 Architecture
>  obj-$(CONFIG_WATCHDOG_RTAS) += wdrtas.o
> diff --git a/drivers/watchdog/mena21_wdt.c b/drivers/watchdog/mena21_wdt.c
> new file mode 100644
> index 0000000..0d3409a
> --- /dev/null
> +++ b/drivers/watchdog/mena21_wdt.c
> @@ -0,0 +1,276 @@
> +/*
> + * Watchdog driver for the A21 VME CPU Boards
> + *
> + * Copyright (C) 2013 MEN Mikro Elektronik Nuernberg GmbH
> + *
> + * 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
> + */
> +#include <linux/module.h>
> +#include <linux/moduleparam.h>
> +#include <linux/types.h>
> +#include <linux/kernel.h>
> +#include <linux/slab.h>
> +#include <linux/platform_device.h>
> +#include <linux/watchdog.h>
> +#include <linux/uaccess.h>
> +#include <linux/gpio.h>
> +#include <linux/of_gpio.h>
> +#include <linux/delay.h>
> +#include <linux/bitops.h>
> +
> +#define NUM_GPIOS 6
> +
> +enum a21_wdt_gpios {
> +	GPIO_WD_ENAB,
> +	GPIO_WD_FAST,
> +	GPIO_WD_TRIG,
> +	GPIO_WD_RST0,
> +	GPIO_WD_RST1,
> +	GPIO_WD_RST2,
> +};
> +
> +struct a21_wdt_drv {
> +	struct watchdog_device wdt;
> +	struct mutex lock;
> +	unsigned gpios[NUM_GPIOS];
> +};
> +
> +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 unsigned int a21_wdt_get_bootstatus(struct a21_wdt_drv *drv)
> +{
> +	int reset = 0;
> +
> +	reset |= gpio_get_value(drv->gpios[GPIO_WD_RST0]) ? (1 << 0) : 0;
> +	reset |= gpio_get_value(drv->gpios[GPIO_WD_RST1]) ? (1 << 1) : 0;
> +	reset |= gpio_get_value(drv->gpios[GPIO_WD_RST2]) ? (1 << 2) : 0;
> +
> +	return reset;
> +}
> +
> +static int a21_wdt_start(struct watchdog_device *wdt)
> +{
> +	struct a21_wdt_drv *drv = watchdog_get_drvdata(wdt);
> +
> +	mutex_lock(&drv->lock);
> +
> +	gpio_set_value(drv->gpios[GPIO_WD_ENAB], 1);
> +
> +	mutex_unlock(&drv->lock);
> +
> +	return 0;
> +}
> +
> +static int a21_wdt_stop(struct watchdog_device *wdt)
> +{
> +	struct a21_wdt_drv *drv = watchdog_get_drvdata(wdt);
> +
> +	/* We don't stop if WDOG_NO_WAY_OUT is set */
> +	if (test_bit(WDOG_NO_WAY_OUT, &wdt->status))
> +		return -EINVAL;
> +
Already handled by watchdog core; no need to check again here.

> +	mutex_lock(&drv->lock);
> +
> +	gpio_set_value(drv->gpios[GPIO_WD_ENAB], 0);
> +
> +	mutex_unlock(&drv->lock);
> +
> +	return 0;
> +}
> +
> +static int a21_wdt_ping(struct watchdog_device *wdt)
> +{
> +	struct a21_wdt_drv *drv = watchdog_get_drvdata(wdt);
> +
> +	mutex_lock(&drv->lock);
> +
> +	gpio_set_value(drv->gpios[GPIO_WD_TRIG], 0);
> +	ndelay(10);

Is the delay specified somewehere or just to be sure ?

> +	gpio_set_value(drv->gpios[GPIO_WD_TRIG], 1);
> +
> +	mutex_unlock(&drv->lock);
> +
> +	return 0;
> +}
> +
> +static int a21_wdt_set_timeout(struct watchdog_device *wdt,
> +			       unsigned int timeout)
> +{
> +	struct a21_wdt_drv *drv = watchdog_get_drvdata(wdt);
> +
> +	if (timeout != 1 && timeout != 30) {
> +		dev_err(wdt->dev, "Only 1 and 30 allowed as timeout\n");
> +		return -EINVAL;
> +	}
> +
> +	if (timeout == 30 && wdt->timeout == 1) {
> +		dev_err(wdt->dev,
> +			"Transition from fast to slow mode not allowed\n");
> +		return -EINVAL;
> +	}
> +
> +	mutex_lock(&drv->lock);
> +
> +	if (timeout == 1)
> +		gpio_set_value(drv->gpios[GPIO_WD_FAST], 1);
> +	else
> +		gpio_set_value(drv->gpios[GPIO_WD_FAST], 0);
> +
> +	wdt->timeout = timeout;
> +
> +	mutex_unlock(&drv->lock);
> +
> +	return 0;
> +}
> +
> +static const struct watchdog_info a21_wdt_info = {
> +	.options = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE,
> +	.identity = "MEN A21 Watchdog",
> +};
> +
> +static const struct watchdog_ops a21_wdt_ops = {
> +	.owner = THIS_MODULE,
> +	.start = a21_wdt_start,
> +	.stop = a21_wdt_stop,
> +	.ping = a21_wdt_ping,
> +	.set_timeout = a21_wdt_set_timeout,
> +};
> +
> +static struct watchdog_device a21_wdt = {
> +	.info = &a21_wdt_info,
> +	.ops = &a21_wdt_ops,
> +	.min_timeout = 1,
> +	.max_timeout = 30,
> +};
> +
> +static int a21_wdt_probe(struct platform_device *pdev)
> +{
> +	struct device_node *node;
> +	struct a21_wdt_drv *drv;
> +	unsigned int reset = 0;
> +	int num_gpios;
> +	int ret;
> +	int i;
> +
> +	dev_info(&pdev->dev, "MEN A21 watchdog timer driver enabled\n");
> +
Move to the very end of this function; otherwise it is misleading if lading the
driver fails.

> +	drv = devm_kzalloc(&pdev->dev, sizeof(struct a21_wdt_drv), GFP_KERNEL);
> +	if (!drv)
> +		return -ENOMEM;
> +
> +	/* Fill GPIO pin array */
> +	node = pdev->dev.of_node;
> +
> +	num_gpios = of_gpio_count(node);
> +	if (num_gpios != NUM_GPIOS) {
> +		dev_err(&pdev->dev, "gpios DT property wrong, got %d want %d",
> +			num_gpios, NUM_GPIOS);
> +		return -ENODEV;
> +	}
> +
> +	for (i = 0; i < num_gpios; i++) {
> +		int val;
> +
> +		val = of_get_gpio(node, i);
> +		if (val < 0)
> +			return val;
> +
> +		drv->gpios[i] = val;
> +	}
> +
> +	/* Request the used GPIOs */
> +	for (i = 0; i < num_gpios; i++) {
> +		ret = devm_gpio_request(&pdev->dev, drv->gpios[i],
> +					"MEN A21 Watchdog");
> +		if (ret)
> +			return ret;
> +
> +		if (i < GPIO_WD_RST0)
> +			ret = gpio_direction_output(drv->gpios[i],
> +						gpio_get_value(drv->gpios[i]));
> +		else		/* GPIO_WD_RST[0..2] are inputs */
> +			ret = gpio_direction_input(drv->gpios[i]);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	mutex_init(&drv->lock);
> +	watchdog_init_timeout(&a21_wdt, 30, &pdev->dev);
> +	watchdog_set_nowayout(&a21_wdt, nowayout);
> +	watchdog_set_drvdata(&a21_wdt, drv);
> +
> +	reset = a21_wdt_get_bootstatus(drv);
> +	if (reset == 2)
> +		a21_wdt.bootstatus |= WDIOF_EXTERN1;
> +	else if (reset == 4)
> +		a21_wdt.bootstatus |= WDIOF_CARDRESET;
> +	else if (reset == 5)
> +		a21_wdt.bootstatus |= WDIOF_POWERUNDER;
> +	else if (reset == 7)
> +		a21_wdt.bootstatus |= WDIOF_EXTERN2;
> +
> +
One empty line is sufficient.

> +	ret = watchdog_register_device(&a21_wdt);
> +	if (ret) {
> +		dev_err(&pdev->dev, "Cannot register watchdog device\n");
> +		goto err_register_wd;
> +	}
> +
> +	dev_set_drvdata(&pdev->dev, drv);
> +
> +	return 0;
> +
> +err_register_wd:
> +	mutex_destroy(&drv->lock);
> +
> +	return ret;
> +}
> +
> +static int a21_wdt_remove(struct platform_device *pdev)
> +{
> +	struct a21_wdt_drv *drv = dev_get_drvdata(&pdev->dev);
> +
> +	dev_warn(&pdev->dev,
> +		"Unregistering A21 watchdog driver, board may reboot\n");
> +
> +
One empty line

> +	watchdog_unregister_device(&drv->wdt);
> +
> +	mutex_destroy(&drv->lock);
> +
> +	return 0;
> +}
> +
> +static void a21_wdt_shutdown(struct platform_device *pdev)
> +{
> +	struct a21_wdt_drv *drv = dev_get_drvdata(&pdev->dev);
> +
> +	gpio_set_value(drv->gpios[GPIO_WD_ENAB], 0);
> +}
> +
> +static const struct of_device_id a21_wdt_ids[] = {
> +	{ .compatible = "men,a021-wdt" },
> +	{ },
> +};
> +
> +static struct platform_driver a21_wdt_driver = {
> +	.probe = a21_wdt_probe,
> +	.remove = a21_wdt_remove,
> +	.shutdown = a21_wdt_shutdown,
> +	.driver = {
> +		.name = "a21-watchdog",
> +		.of_match_table = a21_wdt_ids,
> +	},
> +};
> +
> +module_platform_driver(a21_wdt_driver);
> +
> +MODULE_AUTHOR("MEN Mikro Elektronik");
> +MODULE_DESCRIPTION("MEN A21 Watchdog");
> +MODULE_LICENSE("GPL");
> +MODULE_ALIAS("platform:a21-watchdog");
> --
> 1.7.9.5
> 
> --
> 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
> 

Re: [PATCH v9] watchdog: New watchdog driver for MEN A21 watchdogs

[Johannes Thumshirn]

On Sun, Jun 16, 2013 at 03:07:15PM -0700, Guenter Roeck wrote:
> On Fri, Jun 14, 2013 at 12:58:37PM +0200, Johannes Thumshirn wrote:
> > This patch adds the driver for the watchdog devices found on MEN Mikro
> > Elektronik A21 VMEbus CPU Carrier Boards. It has DT-support and uses the
> > watchdog framework.
> >
> > Signed-off-by: Johannes Thumshirn <johannes.thumshirn@men.de>
>
> Only nitpicks this time.
>

*g*

> > ---
> >
> >  Revision 2:
> >  * Removed unneeded open flag in struct a21_wdt_drv
> >  * Corrected 3bit reason code from gpio
> >  * Additional sysfs files are now part of watchdog sysfs
> >  * Changed OFF/ON delay in ping from 400ms to 10ns
> >  * Reworked timeout setting
> >  * Removed a21_wdt_ioctl(...)
> >
> >  Revision 3:
> >  * Changed pr_{err,info} to dev_{err,info}
> >  * Removed out of memory error print
> >  * Transition from "fast" to "slow" mode not allowed by chip
> >
> >  Revision 4:
> >  * Remove reboot_notifier and place disable code into platform_device's shutdown function
> >  * Removed sysfs interface
> >
> >  Revision 5:
> >  * Added setting of .bootstatus on driver init
> >  * Added initial timeout on driver init
> >
> >  Revision 6:
> >  * Use watchdog_init_timeout() to initialize timeout
> >
> >  Revision 7:
> >  * Fix possible get_bootstatus race condition
> >
> >  Revision 8:
> >  * a21_wdt_get_bootstatus() should return reset code
> >  * GPIOs are supplied via DT instead of being hardcoded. Code derived from
> >    (drivers/hwmon/gpio-fan.c)
> >  * Added Devicetree binding document
> >  * Driver now depends on GPIOLIB
> >
> >  Revision 9:
> >  * We need 6 GPIOs not only 1 to work
> >  * GPIO_WD_RST[0..2] are inputs
> >
> >  .../devicetree/bindings/gpio/men-a021-wdt.txt      |   25 ++
> >  MAINTAINERS                                        |    6 +
> >  drivers/watchdog/Kconfig                           |   12 +
> >  drivers/watchdog/Makefile                          |    1 +
> >  drivers/watchdog/mena21_wdt.c                      |  276 ++++++++++++++++++++
> >  5 files changed, 320 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/gpio/men-a021-wdt.txt
> >  create mode 100644 drivers/watchdog/mena21_wdt.c
> >
> > diff --git a/Documentation/devicetree/bindings/gpio/men-a021-wdt.txt b/Documentation/devicetree/bindings/gpio/men-a021-wdt.txt
> > new file mode 100644
> > index 0000000..370dee3
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/gpio/men-a021-wdt.txt
> > @@ -0,0 +1,25 @@
> > +Bindings for MEN A21 Watchdog device connected to GPIO lines
> > +
> > +Required properties:
> > +- compatible: "men,a021-wdt"
> > +- gpios: Specifies the pins that control the Watchdog, order:
> > +  1: Watchdog enable
> > +  2: Watchdog fast-mode
> > +  3: Watchdog trigger
> > +  4: Watchdog reset cause bit 0
> > +  5: Watchdog reset cause bit 1
> > +  6: Watchdog reset cause bit 2
> > +
> > +Optional properties:
> > +- None
> > +
> > +Example:
> > +	watchdog {
> > +		compatible ="men,a021-wdt";
> > +		gpios = <&gpio3 9  1	/* WD_EN */
> > +			 &gpio3 10 1	/* WD_FAST */
> > +			 &gpio3 11 1	/* WD_TRIG */
> > +			 &gpio3 6  1	/* RST_CAUSE[0] */
> > +			 &gpio3 7  1	/* RST_CAUSE[1] */
> > +			 &gpio3 8  1>;	/* RST_CAUSE[2] */
> > +	};
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 5be702c..824261e 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -5317,6 +5317,12 @@ F:	drivers/mtd/
> >  F:	include/linux/mtd/
> >  F:	include/uapi/mtd/
> >
> > +MEN A21 WATCHDOG DRIVER
> > +M:  	Johannes Thumshirn <johannes.thumshirn@men.de>
> > +L:	linux-watchdog@vger.kernel.org
> > +S:	Supported
> > +F:	drivers/watchdog/mena21_wdt.c
> > +
> >  METAG ARCHITECTURE
> >  M:	James Hogan <james.hogan@imgtec.com>
> >  S:	Supported
> > diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> > index e89fc31..8b143ee 100644
> > --- a/drivers/watchdog/Kconfig
> > +++ b/drivers/watchdog/Kconfig
> > @@ -1172,6 +1172,18 @@ config BOOKE_WDT_DEFAULT_TIMEOUT
> >
> >  	  The value can be overridden by the wdt_period command-line parameter.
> >
> > +config MEN_A21_WDT
> > +       tristate "MEN A21 VME CPU Carrier Board Watchdog Timer"
> > +       select WATCHDOG_CORE
> > +       depends on GPIOLIB
> > +       help
> > +        Watchdog driver for MEN A21 VMEbus CPU Carrier Boards.
> > +
> > +	The driver can also be built as a module. If so, the module will be
> > +	called mena21_wdt.
> > +
> > +	If unsure select N here.
> > +
> >  # PPC64 Architecture
> >
> >  config WATCHDOG_RTAS
> > diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
> > index a300b94..bffdcb1 100644
> > --- a/drivers/watchdog/Makefile
> > +++ b/drivers/watchdog/Makefile
> > @@ -143,6 +143,7 @@ obj-$(CONFIG_8xxx_WDT) += mpc8xxx_wdt.o
> >  obj-$(CONFIG_MV64X60_WDT) += mv64x60_wdt.o
> >  obj-$(CONFIG_PIKA_WDT) += pika_wdt.o
> >  obj-$(CONFIG_BOOKE_WDT) += booke_wdt.o
> > +obj-$(CONFIG_MEN_A21_WDT) += mena21_wdt.o
> >
> >  # PPC64 Architecture
> >  obj-$(CONFIG_WATCHDOG_RTAS) += wdrtas.o
> > diff --git a/drivers/watchdog/mena21_wdt.c b/drivers/watchdog/mena21_wdt.c
> > new file mode 100644
> > index 0000000..0d3409a
> > --- /dev/null
> > +++ b/drivers/watchdog/mena21_wdt.c
> > @@ -0,0 +1,276 @@
> > +/*
> > + * Watchdog driver for the A21 VME CPU Boards
> > + *
> > + * Copyright (C) 2013 MEN Mikro Elektronik Nuernberg GmbH
> > + *
> > + * 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
> > + */
> > +#include <linux/module.h>
> > +#include <linux/moduleparam.h>
> > +#include <linux/types.h>
> > +#include <linux/kernel.h>
> > +#include <linux/slab.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/watchdog.h>
> > +#include <linux/uaccess.h>
> > +#include <linux/gpio.h>
> > +#include <linux/of_gpio.h>
> > +#include <linux/delay.h>
> > +#include <linux/bitops.h>
> > +
> > +#define NUM_GPIOS 6
> > +
> > +enum a21_wdt_gpios {
> > +	GPIO_WD_ENAB,
> > +	GPIO_WD_FAST,
> > +	GPIO_WD_TRIG,
> > +	GPIO_WD_RST0,
> > +	GPIO_WD_RST1,
> > +	GPIO_WD_RST2,
> > +};
> > +
> > +struct a21_wdt_drv {
> > +	struct watchdog_device wdt;
> > +	struct mutex lock;
> > +	unsigned gpios[NUM_GPIOS];
> > +};
> > +
> > +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 unsigned int a21_wdt_get_bootstatus(struct a21_wdt_drv *drv)
> > +{
> > +	int reset = 0;
> > +
> > +	reset |= gpio_get_value(drv->gpios[GPIO_WD_RST0]) ? (1 << 0) : 0;
> > +	reset |= gpio_get_value(drv->gpios[GPIO_WD_RST1]) ? (1 << 1) : 0;
> > +	reset |= gpio_get_value(drv->gpios[GPIO_WD_RST2]) ? (1 << 2) : 0;
> > +
> > +	return reset;
> > +}
> > +
> > +static int a21_wdt_start(struct watchdog_device *wdt)
> > +{
> > +	struct a21_wdt_drv *drv = watchdog_get_drvdata(wdt);
> > +
> > +	mutex_lock(&drv->lock);
> > +
> > +	gpio_set_value(drv->gpios[GPIO_WD_ENAB], 1);
> > +
> > +	mutex_unlock(&drv->lock);
> > +
> > +	return 0;
> > +}
> > +
> > +static int a21_wdt_stop(struct watchdog_device *wdt)
> > +{
> > +	struct a21_wdt_drv *drv = watchdog_get_drvdata(wdt);
> > +
> > +	/* We don't stop if WDOG_NO_WAY_OUT is set */
> > +	if (test_bit(WDOG_NO_WAY_OUT, &wdt->status))
> > +		return -EINVAL;
> > +
> Already handled by watchdog core; no need to check again here.
>

ACK

> > +	mutex_lock(&drv->lock);
> > +
> > +	gpio_set_value(drv->gpios[GPIO_WD_ENAB], 0);
> > +
> > +	mutex_unlock(&drv->lock);
> > +
> > +	return 0;
> > +}
> > +
> > +static int a21_wdt_ping(struct watchdog_device *wdt)
> > +{
> > +	struct a21_wdt_drv *drv = watchdog_get_drvdata(wdt);
> > +
> > +	mutex_lock(&drv->lock);
> > +
> > +	gpio_set_value(drv->gpios[GPIO_WD_TRIG], 0);
> > +	ndelay(10);
>
> Is the delay specified somewehere or just to be sure ?
>

I tried to meter it out. 10ns seemed to be a good value where the reset is still
reliable. Making it smaller can make it a bit unreliable. Also it is roughly
10*CPLD clock, which seems like a good value to me.

> > +	gpio_set_value(drv->gpios[GPIO_WD_TRIG], 1);
> > +
> > +	mutex_unlock(&drv->lock);
> > +
> > +	return 0;
> > +}
> > +
> > +static int a21_wdt_set_timeout(struct watchdog_device *wdt,
> > +			       unsigned int timeout)
> > +{
> > +	struct a21_wdt_drv *drv = watchdog_get_drvdata(wdt);
> > +
> > +	if (timeout != 1 && timeout != 30) {
> > +		dev_err(wdt->dev, "Only 1 and 30 allowed as timeout\n");
> > +		return -EINVAL;
> > +	}
> > +
> > +	if (timeout == 30 && wdt->timeout == 1) {
> > +		dev_err(wdt->dev,
> > +			"Transition from fast to slow mode not allowed\n");
> > +		return -EINVAL;
> > +	}
> > +
> > +	mutex_lock(&drv->lock);
> > +
> > +	if (timeout == 1)
> > +		gpio_set_value(drv->gpios[GPIO_WD_FAST], 1);
> > +	else
> > +		gpio_set_value(drv->gpios[GPIO_WD_FAST], 0);
> > +
> > +	wdt->timeout = timeout;
> > +
> > +	mutex_unlock(&drv->lock);
> > +
> > +	return 0;
> > +}
> > +
> > +static const struct watchdog_info a21_wdt_info = {
> > +	.options = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE,
> > +	.identity = "MEN A21 Watchdog",
> > +};
> > +
> > +static const struct watchdog_ops a21_wdt_ops = {
> > +	.owner = THIS_MODULE,
> > +	.start = a21_wdt_start,
> > +	.stop = a21_wdt_stop,
> > +	.ping = a21_wdt_ping,
> > +	.set_timeout = a21_wdt_set_timeout,
> > +};
> > +
> > +static struct watchdog_device a21_wdt = {
> > +	.info = &a21_wdt_info,
> > +	.ops = &a21_wdt_ops,
> > +	.min_timeout = 1,
> > +	.max_timeout = 30,
> > +};
> > +
> > +static int a21_wdt_probe(struct platform_device *pdev)
> > +{
> > +	struct device_node *node;
> > +	struct a21_wdt_drv *drv;
> > +	unsigned int reset = 0;
> > +	int num_gpios;
> > +	int ret;
> > +	int i;
> > +
> > +	dev_info(&pdev->dev, "MEN A21 watchdog timer driver enabled\n");
> > +
> Move to the very end of this function; otherwise it is misleading if lading the
> driver fails.
>

OK.

> > +	drv = devm_kzalloc(&pdev->dev, sizeof(struct a21_wdt_drv), GFP_KERNEL);
> > +	if (!drv)
> > +		return -ENOMEM;
> > +
> > +	/* Fill GPIO pin array */
> > +	node = pdev->dev.of_node;
> > +
> > +	num_gpios = of_gpio_count(node);
> > +	if (num_gpios != NUM_GPIOS) {
> > +		dev_err(&pdev->dev, "gpios DT property wrong, got %d want %d",
> > +			num_gpios, NUM_GPIOS);
> > +		return -ENODEV;
> > +	}
> > +
> > +	for (i = 0; i < num_gpios; i++) {
> > +		int val;
> > +
> > +		val = of_get_gpio(node, i);
> > +		if (val < 0)
> > +			return val;
> > +
> > +		drv->gpios[i] = val;
> > +	}
> > +
> > +	/* Request the used GPIOs */
> > +	for (i = 0; i < num_gpios; i++) {
> > +		ret = devm_gpio_request(&pdev->dev, drv->gpios[i],
> > +					"MEN A21 Watchdog");
> > +		if (ret)
> > +			return ret;
> > +
> > +		if (i < GPIO_WD_RST0)
> > +			ret = gpio_direction_output(drv->gpios[i],
> > +						gpio_get_value(drv->gpios[i]));
> > +		else		/* GPIO_WD_RST[0..2] are inputs */
> > +			ret = gpio_direction_input(drv->gpios[i]);
> > +		if (ret)
> > +			return ret;
> > +	}
> > +
> > +	mutex_init(&drv->lock);
> > +	watchdog_init_timeout(&a21_wdt, 30, &pdev->dev);
> > +	watchdog_set_nowayout(&a21_wdt, nowayout);
> > +	watchdog_set_drvdata(&a21_wdt, drv);
> > +
> > +	reset = a21_wdt_get_bootstatus(drv);
> > +	if (reset == 2)
> > +		a21_wdt.bootstatus |= WDIOF_EXTERN1;
> > +	else if (reset == 4)
> > +		a21_wdt.bootstatus |= WDIOF_CARDRESET;
> > +	else if (reset == 5)
> > +		a21_wdt.bootstatus |= WDIOF_POWERUNDER;
> > +	else if (reset == 7)
> > +		a21_wdt.bootstatus |= WDIOF_EXTERN2;
> > +
> > +
> One empty line is sufficient.
>
Args. Yep.
> > +	ret = watchdog_register_device(&a21_wdt);
> > +	if (ret) {
> > +		dev_err(&pdev->dev, "Cannot register watchdog device\n");
> > +		goto err_register_wd;
> > +	}
> > +
> > +	dev_set_drvdata(&pdev->dev, drv);
> > +
> > +	return 0;
> > +
> > +err_register_wd:
> > +	mutex_destroy(&drv->lock);
> > +
> > +	return ret;
> > +}
> > +
> > +static int a21_wdt_remove(struct platform_device *pdev)
> > +{
> > +	struct a21_wdt_drv *drv = dev_get_drvdata(&pdev->dev);
> > +
> > +	dev_warn(&pdev->dev,
> > +		"Unregistering A21 watchdog driver, board may reboot\n");
> > +
> > +
> One empty line
>
^^
> > +	watchdog_unregister_device(&drv->wdt);
> > +
> > +	mutex_destroy(&drv->lock);
> > +
> > +	return 0;
> > +}
> > +
> > +static void a21_wdt_shutdown(struct platform_device *pdev)
> > +{
> > +	struct a21_wdt_drv *drv = dev_get_drvdata(&pdev->dev);
> > +
> > +	gpio_set_value(drv->gpios[GPIO_WD_ENAB], 0);
> > +}
> > +
> > +static const struct of_device_id a21_wdt_ids[] = {
> > +	{ .compatible = "men,a021-wdt" },
> > +	{ },
> > +};
> > +
> > +static struct platform_driver a21_wdt_driver = {
> > +	.probe = a21_wdt_probe,
> > +	.remove = a21_wdt_remove,
> > +	.shutdown = a21_wdt_shutdown,
> > +	.driver = {
> > +		.name = "a21-watchdog",
> > +		.of_match_table = a21_wdt_ids,
> > +	},
> > +};
> > +
> > +module_platform_driver(a21_wdt_driver);
> > +
> > +MODULE_AUTHOR("MEN Mikro Elektronik");
> > +MODULE_DESCRIPTION("MEN A21 Watchdog");
> > +MODULE_LICENSE("GPL");
> > +MODULE_ALIAS("platform:a21-watchdog");
> > --
> > 1.7.9.5
> >
> > --
> > 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
> >

[PATCH v10 1/2] watchdog: New watchdog driver for MEN A21 watchdogs

[Johannes Thumshirn]

This patch adds the driver for the watchdog devices found on MEN Mikro
Elektronik A21 VMEbus CPU Carrier Boards. It has DT-support and uses the
watchdog framework.

Signed-off-by: Johannes Thumshirn <johannes.thumshirn@men.de>
---

 Revision 2:
 * Removed unneeded open flag in struct a21_wdt_drv
 * Corrected 3bit reason code from gpio
 * Additional sysfs files are now part of watchdog sysfs
 * Changed OFF/ON delay in ping from 400ms to 10ns
 * Reworked timeout setting
 * Removed a21_wdt_ioctl(...)

 Revision 3:
 * Changed pr_{err,info} to dev_{err,info}
 * Removed out of memory error print
 * Transition from "fast" to "slow" mode not allowed by chip

 Revision 4:
 * Remove reboot_notifier and place disable code into platform_device's shutdown function
 * Removed sysfs interface

 Revision 5:
 * Added setting of .bootstatus on driver init
 * Added initial timeout on driver init

 Revision 6:
 * Use watchdog_init_timeout() to initialize timeout

 Revision 7:
 * Fix possible get_bootstatus race condition

 Revision 8:
 * a21_wdt_get_bootstatus() should return reset code
 * GPIOs are supplied via DT instead of being hardcoded. Code derived from
   (drivers/hwmon/gpio-fan.c)
 * Added Devicetree binding document
 * Driver now depends on GPIOLIB

 Revision 9:
 * We need 6 GPIOs not only 1 to work
 * GPIO_WD_RST[0..2] are inputs

 Revision 10:
 * Don't perform checks of Watchdog core again.
 * Some style fixes

 .../devicetree/bindings/gpio/men-a021-wdt.txt      |   25 ++
 MAINTAINERS                                        |    6 +
 drivers/watchdog/Kconfig                           |   12 +
 drivers/watchdog/Makefile                          |    1 +
 drivers/watchdog/mena21_wdt.c                      |  270 ++++++++++++++++++++
 5 files changed, 314 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/gpio/men-a021-wdt.txt
 create mode 100644 drivers/watchdog/mena21_wdt.c

diff --git a/Documentation/devicetree/bindings/gpio/men-a021-wdt.txt b/Documentation/devicetree/bindings/gpio/men-a021-wdt.txt
new file mode 100644
index 0000000..370dee3
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/men-a021-wdt.txt
@@ -0,0 +1,25 @@
+Bindings for MEN A21 Watchdog device connected to GPIO lines
+
+Required properties:
+- compatible: "men,a021-wdt"
+- gpios: Specifies the pins that control the Watchdog, order:
+  1: Watchdog enable
+  2: Watchdog fast-mode
+  3: Watchdog trigger
+  4: Watchdog reset cause bit 0
+  5: Watchdog reset cause bit 1
+  6: Watchdog reset cause bit 2
+
+Optional properties:
+- None
+
+Example:
+	watchdog {
+		compatible ="men,a021-wdt";
+		gpios = <&gpio3 9  1	/* WD_EN */
+			 &gpio3 10 1	/* WD_FAST */
+			 &gpio3 11 1	/* WD_TRIG */
+			 &gpio3 6  1	/* RST_CAUSE[0] */
+			 &gpio3 7  1	/* RST_CAUSE[1] */
+			 &gpio3 8  1>;	/* RST_CAUSE[2] */
+	};
diff --git a/MAINTAINERS b/MAINTAINERS
index 5be702c..824261e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5317,6 +5317,12 @@ F:	drivers/mtd/
 F:	include/linux/mtd/
 F:	include/uapi/mtd/

+MEN A21 WATCHDOG DRIVER
+M:  	Johannes Thumshirn <johannes.thumshirn@men.de>
+L:	linux-watchdog@vger.kernel.org
+S:	Supported
+F:	drivers/watchdog/mena21_wdt.c
+
 METAG ARCHITECTURE
 M:	James Hogan <james.hogan@imgtec.com>
 S:	Supported
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index e89fc31..8b143ee 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -1172,6 +1172,18 @@ config BOOKE_WDT_DEFAULT_TIMEOUT

 	  The value can be overridden by the wdt_period command-line parameter.

+config MEN_A21_WDT
+       tristate "MEN A21 VME CPU Carrier Board Watchdog Timer"
+       select WATCHDOG_CORE
+       depends on GPIOLIB
+       help
+        Watchdog driver for MEN A21 VMEbus CPU Carrier Boards.
+
+	The driver can also be built as a module. If so, the module will be
+	called mena21_wdt.
+
+	If unsure select N here.
+
 # PPC64 Architecture

 config WATCHDOG_RTAS
diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
index a300b94..bffdcb1 100644
--- a/drivers/watchdog/Makefile
+++ b/drivers/watchdog/Makefile
@@ -143,6 +143,7 @@ obj-$(CONFIG_8xxx_WDT) += mpc8xxx_wdt.o
 obj-$(CONFIG_MV64X60_WDT) += mv64x60_wdt.o
 obj-$(CONFIG_PIKA_WDT) += pika_wdt.o
 obj-$(CONFIG_BOOKE_WDT) += booke_wdt.o
+obj-$(CONFIG_MEN_A21_WDT) += mena21_wdt.o

 # PPC64 Architecture
 obj-$(CONFIG_WATCHDOG_RTAS) += wdrtas.o
diff --git a/drivers/watchdog/mena21_wdt.c b/drivers/watchdog/mena21_wdt.c
new file mode 100644
index 0000000..96dbba9
--- /dev/null
+++ b/drivers/watchdog/mena21_wdt.c
@@ -0,0 +1,270 @@
+/*
+ * Watchdog driver for the A21 VME CPU Boards
+ *
+ * Copyright (C) 2013 MEN Mikro Elektronik Nuernberg GmbH
+ *
+ * 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
+ */
+#include <linux/module.h>
+#include <linux/moduleparam.h>
+#include <linux/types.h>
+#include <linux/kernel.h>
+#include <linux/slab.h>
+#include <linux/platform_device.h>
+#include <linux/watchdog.h>
+#include <linux/uaccess.h>
+#include <linux/gpio.h>
+#include <linux/of_gpio.h>
+#include <linux/delay.h>
+#include <linux/bitops.h>
+
+#define NUM_GPIOS 6
+
+enum a21_wdt_gpios {
+	GPIO_WD_ENAB,
+	GPIO_WD_FAST,
+	GPIO_WD_TRIG,
+	GPIO_WD_RST0,
+	GPIO_WD_RST1,
+	GPIO_WD_RST2,
+};
+
+struct a21_wdt_drv {
+	struct watchdog_device wdt;
+	struct mutex lock;
+	unsigned gpios[NUM_GPIOS];
+};
+
+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 unsigned int a21_wdt_get_bootstatus(struct a21_wdt_drv *drv)
+{
+	int reset = 0;
+
+	reset |= gpio_get_value(drv->gpios[GPIO_WD_RST0]) ? (1 << 0) : 0;
+	reset |= gpio_get_value(drv->gpios[GPIO_WD_RST1]) ? (1 << 1) : 0;
+	reset |= gpio_get_value(drv->gpios[GPIO_WD_RST2]) ? (1 << 2) : 0;
+
+	return reset;
+}
+
+static int a21_wdt_start(struct watchdog_device *wdt)
+{
+	struct a21_wdt_drv *drv = watchdog_get_drvdata(wdt);
+
+	mutex_lock(&drv->lock);
+
+	gpio_set_value(drv->gpios[GPIO_WD_ENAB], 1);
+
+	mutex_unlock(&drv->lock);
+
+	return 0;
+}
+
+static int a21_wdt_stop(struct watchdog_device *wdt)
+{
+	struct a21_wdt_drv *drv = watchdog_get_drvdata(wdt);
+
+	mutex_lock(&drv->lock);
+
+	gpio_set_value(drv->gpios[GPIO_WD_ENAB], 0);
+
+	mutex_unlock(&drv->lock);
+
+	return 0;
+}
+
+static int a21_wdt_ping(struct watchdog_device *wdt)
+{
+	struct a21_wdt_drv *drv = watchdog_get_drvdata(wdt);
+
+	mutex_lock(&drv->lock);
+
+	gpio_set_value(drv->gpios[GPIO_WD_TRIG], 0);
+	ndelay(10);
+	gpio_set_value(drv->gpios[GPIO_WD_TRIG], 1);
+
+	mutex_unlock(&drv->lock);
+
+	return 0;
+}
+
+static int a21_wdt_set_timeout(struct watchdog_device *wdt,
+			       unsigned int timeout)
+{
+	struct a21_wdt_drv *drv = watchdog_get_drvdata(wdt);
+
+	if (timeout != 1 && timeout != 30) {
+		dev_err(wdt->dev, "Only 1 and 30 allowed as timeout\n");
+		return -EINVAL;
+	}
+
+	if (timeout == 30 && wdt->timeout == 1) {
+		dev_err(wdt->dev,
+			"Transition from fast to slow mode not allowed\n");
+		return -EINVAL;
+	}
+
+	mutex_lock(&drv->lock);
+
+	if (timeout == 1)
+		gpio_set_value(drv->gpios[GPIO_WD_FAST], 1);
+	else
+		gpio_set_value(drv->gpios[GPIO_WD_FAST], 0);
+
+	wdt->timeout = timeout;
+
+	mutex_unlock(&drv->lock);
+
+	return 0;
+}
+
+static const struct watchdog_info a21_wdt_info = {
+	.options = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE,
+	.identity = "MEN A21 Watchdog",
+};
+
+static const struct watchdog_ops a21_wdt_ops = {
+	.owner = THIS_MODULE,
+	.start = a21_wdt_start,
+	.stop = a21_wdt_stop,
+	.ping = a21_wdt_ping,
+	.set_timeout = a21_wdt_set_timeout,
+};
+
+static struct watchdog_device a21_wdt = {
+	.info = &a21_wdt_info,
+	.ops = &a21_wdt_ops,
+	.min_timeout = 1,
+	.max_timeout = 30,
+};
+
+static int a21_wdt_probe(struct platform_device *pdev)
+{
+	struct device_node *node;
+	struct a21_wdt_drv *drv;
+	unsigned int reset = 0;
+	int num_gpios;
+	int ret;
+	int i;
+
+	drv = devm_kzalloc(&pdev->dev, sizeof(struct a21_wdt_drv), GFP_KERNEL);
+	if (!drv)
+		return -ENOMEM;
+
+	/* Fill GPIO pin array */
+	node = pdev->dev.of_node;
+
+	num_gpios = of_gpio_count(node);
+	if (num_gpios != NUM_GPIOS) {
+		dev_err(&pdev->dev, "gpios DT property wrong, got %d want %d",
+			num_gpios, NUM_GPIOS);
+		return -ENODEV;
+	}
+
+	for (i = 0; i < num_gpios; i++) {
+		int val;
+
+		val = of_get_gpio(node, i);
+		if (val < 0)
+			return val;
+
+		drv->gpios[i] = val;
+	}
+
+	/* Request the used GPIOs */
+	for (i = 0; i < num_gpios; i++) {
+		ret = devm_gpio_request(&pdev->dev, drv->gpios[i],
+					"MEN A21 Watchdog");
+		if (ret)
+			return ret;
+
+		if (i < GPIO_WD_RST0)
+			ret = gpio_direction_output(drv->gpios[i],
+						gpio_get_value(drv->gpios[i]));
+		else		/* GPIO_WD_RST[0..2] are inputs */
+			ret = gpio_direction_input(drv->gpios[i]);
+		if (ret)
+			return ret;
+	}
+
+	mutex_init(&drv->lock);
+	watchdog_init_timeout(&a21_wdt, 30, &pdev->dev);
+	watchdog_set_nowayout(&a21_wdt, nowayout);
+	watchdog_set_drvdata(&a21_wdt, drv);
+
+	reset = a21_wdt_get_bootstatus(drv);
+	if (reset == 2)
+		a21_wdt.bootstatus |= WDIOF_EXTERN1;
+	else if (reset == 4)
+		a21_wdt.bootstatus |= WDIOF_CARDRESET;
+	else if (reset == 5)
+		a21_wdt.bootstatus |= WDIOF_POWERUNDER;
+	else if (reset == 7)
+		a21_wdt.bootstatus |= WDIOF_EXTERN2;
+
+	ret = watchdog_register_device(&a21_wdt);
+	if (ret) {
+		dev_err(&pdev->dev, "Cannot register watchdog device\n");
+		goto err_register_wd;
+	}
+
+	dev_set_drvdata(&pdev->dev, drv);
+
+	dev_info(&pdev->dev, "MEN A21 watchdog timer driver enabled\n");
+
+	return 0;
+
+err_register_wd:
+	mutex_destroy(&drv->lock);
+
+	return ret;
+}
+
+static int a21_wdt_remove(struct platform_device *pdev)
+{
+	struct a21_wdt_drv *drv = dev_get_drvdata(&pdev->dev);
+
+	dev_warn(&pdev->dev,
+		"Unregistering A21 watchdog driver, board may reboot\n");
+
+	watchdog_unregister_device(&drv->wdt);
+
+	mutex_destroy(&drv->lock);
+
+	return 0;
+}
+
+static void a21_wdt_shutdown(struct platform_device *pdev)
+{
+	struct a21_wdt_drv *drv = dev_get_drvdata(&pdev->dev);
+
+	gpio_set_value(drv->gpios[GPIO_WD_ENAB], 0);
+}
+
+static const struct of_device_id a21_wdt_ids[] = {
+	{ .compatible = "men,a021-wdt" },
+	{ },
+};
+
+static struct platform_driver a21_wdt_driver = {
+	.probe = a21_wdt_probe,
+	.remove = a21_wdt_remove,
+	.shutdown = a21_wdt_shutdown,
+	.driver = {
+		.name = "a21-watchdog",
+		.of_match_table = a21_wdt_ids,
+	},
+};
+
+module_platform_driver(a21_wdt_driver);
+
+MODULE_AUTHOR("MEN Mikro Elektronik");
+MODULE_DESCRIPTION("MEN A21 Watchdog");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:a21-watchdog");
--
1.7.9.5

[PATCH v10 2/2] watchdog: Sysfs interface for MEN A21 watchdog

[Johannes Thumshirn]

This patch adds a sysfs interface for the watchdog
device found on MEN A21 Boards.

The newly generated files are:
* rebootcause:
Can be one of:
Power on Reset,
CPU Reset Request,
Push Button,
FPGA Reset Request,
Watchdog,
Local Power Bad,
Invalid or
BDI
and shows the reason of the boards last reboot.

* active:
Shows if the watchdog CPLD is actually running

* allow_disable:
Shows if the watchdog is allowed to be disabled (NOWAYOUT disabled)

* fastmode:
Shows if the CPLD is running in fast mode (1s timeout), once it is in
fastmode it can't be switched back to slow mode (30s timeout) until the
next reboot.

Revision 2:
* Re-worked sysfs patch to apply on re-worked base

Revision 3:
* Re-worked sysfs patch to apply on re-worked base
Signed-off-by: Johannes Thumshirn <johannes.thumshirn@men.de>
---
 drivers/watchdog/mena21_wdt.c |   89 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 89 insertions(+)

diff --git a/drivers/watchdog/mena21_wdt.c b/drivers/watchdog/mena21_wdt.c
index 96dbba9..91cc90b 100644
--- a/drivers/watchdog/mena21_wdt.c
+++ b/drivers/watchdog/mena21_wdt.c
@@ -22,6 +22,17 @@

 #define NUM_GPIOS 6

+static char *reset_causes[] = {
+	"Power On Reset",
+	"CPU Reset Request",
+	"Push Button",
+	"FPGA Reset Request",
+	"Watchdog",
+	"Local Power Bad",
+	"Invalid",
+	"BDI",
+};
+
 enum a21_wdt_gpios {
 	GPIO_WD_ENAB,
 	GPIO_WD_FAST,
@@ -53,6 +64,73 @@ static unsigned int a21_wdt_get_bootstatus(struct a21_wdt_drv *drv)
 	return reset;
 }

+static ssize_t rebootcause_show(struct device *dev,
+				struct device_attribute *attr,
+				char *buf)
+{
+	struct a21_wdt_drv *drv = dev_get_drvdata(dev);
+	unsigned int reset = 0;
+
+	reset = a21_wdt_get_bootstatus(drv);
+
+	return sprintf(buf, "%s\n", reset_causes[reset]);
+}
+static DEVICE_ATTR(rebootcause, S_IRUGO, rebootcause_show, NULL);
+
+static ssize_t active_show(struct device *dev, struct device_attribute *attr,
+			   char *buf)
+{
+	struct a21_wdt_drv *drv = dev_get_drvdata(dev);
+
+	return sprintf(buf, "%d\n", !!gpio_get_value(drv->gpios[GPIO_WD_ENAB]));
+}
+static DEVICE_ATTR(active, S_IRUGO, active_show, NULL);
+
+static ssize_t allow_disable_show(struct device *dev,
+				  struct device_attribute *attr,
+				  char *buf)
+{
+	return sprintf(buf, "%d\n", !nowayout);
+}
+static DEVICE_ATTR(allow_disable, S_IRUGO, allow_disable_show, NULL);
+
+static ssize_t fastmode_show(struct device *dev, struct device_attribute *attr,
+			     char *buf)
+{
+	struct a21_wdt_drv *drv = dev_get_drvdata(dev);
+
+	return sprintf(buf, "%d\n", !!gpio_get_value(drv->gpios[GPIO_WD_FAST]));
+}
+static DEVICE_ATTR(fastmode, S_IRUGO, fastmode_show, NULL);
+
+static int a21_wdt_create_files(struct watchdog_device *wdev)
+{
+	int ret;
+
+	ret = device_create_file(wdev->dev, &dev_attr_rebootcause);
+	if (ret)
+		return ret;
+	ret = device_create_file(wdev->dev, &dev_attr_active);
+	if (ret)
+		return ret;
+	ret = device_create_file(wdev->dev, &dev_attr_allow_disable);
+	if (ret)
+		return ret;
+	ret = device_create_file(wdev->dev, &dev_attr_fastmode);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static void a21_wdt_remove_files(struct watchdog_device *wdev)
+{
+	device_remove_file(wdev->dev, &dev_attr_rebootcause);
+	device_remove_file(wdev->dev, &dev_attr_active);
+	device_remove_file(wdev->dev, &dev_attr_allow_disable);
+	device_remove_file(wdev->dev, &dev_attr_fastmode);
+}
+
 static int a21_wdt_start(struct watchdog_device *wdt)
 {
 	struct a21_wdt_drv *drv = watchdog_get_drvdata(wdt);
@@ -215,11 +293,20 @@ static int a21_wdt_probe(struct platform_device *pdev)
 	}

 	dev_set_drvdata(&pdev->dev, drv);
+	dev_set_drvdata(a21_wdt.dev, drv);
+
+	ret = a21_wdt_create_files(&a21_wdt);
+	if (ret) {
+		dev_err(&pdev->dev, "Cannot create sysfs entries\n");
+		goto err_create_sysfs;
+	}

 	dev_info(&pdev->dev, "MEN A21 watchdog timer driver enabled\n");

 	return 0;

+err_create_sysfs:
+	watchdog_unregister_device(&drv->wdt);
 err_register_wd:
 	mutex_destroy(&drv->lock);

@@ -233,6 +320,8 @@ static int a21_wdt_remove(struct platform_device *pdev)
 	dev_warn(&pdev->dev,
 		"Unregistering A21 watchdog driver, board may reboot\n");

+	a21_wdt_remove_files(&drv->wdt);
+
 	watchdog_unregister_device(&drv->wdt);

 	mutex_destroy(&drv->lock);
--
1.7.9.5

Re: [PATCH v10 1/2] watchdog: New watchdog driver for MEN A21 watchdogs

[Guenter Roeck]

On Mon, Jun 17, 2013 at 12:22:48PM +0200, Johannes Thumshirn wrote:
> This patch adds the driver for the watchdog devices found on MEN Mikro
> Elektronik A21 VMEbus CPU Carrier Boards. It has DT-support and uses the
> watchdog framework.
> 
> Signed-off-by: Johannes Thumshirn <johannes.thumshirn@men.de>

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

> ---
> 
>  Revision 2:
>  * Removed unneeded open flag in struct a21_wdt_drv
>  * Corrected 3bit reason code from gpio
>  * Additional sysfs files are now part of watchdog sysfs
>  * Changed OFF/ON delay in ping from 400ms to 10ns
>  * Reworked timeout setting
>  * Removed a21_wdt_ioctl(...)
> 
>  Revision 3:
>  * Changed pr_{err,info} to dev_{err,info}
>  * Removed out of memory error print
>  * Transition from "fast" to "slow" mode not allowed by chip
> 
>  Revision 4:
>  * Remove reboot_notifier and place disable code into platform_device's shutdown function
>  * Removed sysfs interface
> 
>  Revision 5:
>  * Added setting of .bootstatus on driver init
>  * Added initial timeout on driver init
> 
>  Revision 6:
>  * Use watchdog_init_timeout() to initialize timeout
> 
>  Revision 7:
>  * Fix possible get_bootstatus race condition
> 
>  Revision 8:
>  * a21_wdt_get_bootstatus() should return reset code
>  * GPIOs are supplied via DT instead of being hardcoded. Code derived from
>    (drivers/hwmon/gpio-fan.c)
>  * Added Devicetree binding document
>  * Driver now depends on GPIOLIB
> 
>  Revision 9:
>  * We need 6 GPIOs not only 1 to work
>  * GPIO_WD_RST[0..2] are inputs
> 
>  Revision 10:
>  * Don't perform checks of Watchdog core again.
>  * Some style fixes
> 
>  .../devicetree/bindings/gpio/men-a021-wdt.txt      |   25 ++
>  MAINTAINERS                                        |    6 +
>  drivers/watchdog/Kconfig                           |   12 +
>  drivers/watchdog/Makefile                          |    1 +
>  drivers/watchdog/mena21_wdt.c                      |  270 ++++++++++++++++++++
>  5 files changed, 314 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/gpio/men-a021-wdt.txt
>  create mode 100644 drivers/watchdog/mena21_wdt.c
> 
> diff --git a/Documentation/devicetree/bindings/gpio/men-a021-wdt.txt b/Documentation/devicetree/bindings/gpio/men-a021-wdt.txt
> new file mode 100644
> index 0000000..370dee3
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpio/men-a021-wdt.txt
> @@ -0,0 +1,25 @@
> +Bindings for MEN A21 Watchdog device connected to GPIO lines
> +
> +Required properties:
> +- compatible: "men,a021-wdt"
> +- gpios: Specifies the pins that control the Watchdog, order:
> +  1: Watchdog enable
> +  2: Watchdog fast-mode
> +  3: Watchdog trigger
> +  4: Watchdog reset cause bit 0
> +  5: Watchdog reset cause bit 1
> +  6: Watchdog reset cause bit 2
> +
> +Optional properties:
> +- None
> +
> +Example:
> +	watchdog {
> +		compatible ="men,a021-wdt";
> +		gpios = <&gpio3 9  1	/* WD_EN */
> +			 &gpio3 10 1	/* WD_FAST */
> +			 &gpio3 11 1	/* WD_TRIG */
> +			 &gpio3 6  1	/* RST_CAUSE[0] */
> +			 &gpio3 7  1	/* RST_CAUSE[1] */
> +			 &gpio3 8  1>;	/* RST_CAUSE[2] */
> +	};
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 5be702c..824261e 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -5317,6 +5317,12 @@ F:	drivers/mtd/
>  F:	include/linux/mtd/
>  F:	include/uapi/mtd/
> 
> +MEN A21 WATCHDOG DRIVER
> +M:  	Johannes Thumshirn <johannes.thumshirn@men.de>
> +L:	linux-watchdog@vger.kernel.org
> +S:	Supported
> +F:	drivers/watchdog/mena21_wdt.c
> +
>  METAG ARCHITECTURE
>  M:	James Hogan <james.hogan@imgtec.com>
>  S:	Supported
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index e89fc31..8b143ee 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -1172,6 +1172,18 @@ config BOOKE_WDT_DEFAULT_TIMEOUT
> 
>  	  The value can be overridden by the wdt_period command-line parameter.
> 
> +config MEN_A21_WDT
> +       tristate "MEN A21 VME CPU Carrier Board Watchdog Timer"
> +       select WATCHDOG_CORE
> +       depends on GPIOLIB
> +       help
> +        Watchdog driver for MEN A21 VMEbus CPU Carrier Boards.
> +
> +	The driver can also be built as a module. If so, the module will be
> +	called mena21_wdt.
> +
> +	If unsure select N here.
> +
>  # PPC64 Architecture
> 
>  config WATCHDOG_RTAS
> diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
> index a300b94..bffdcb1 100644
> --- a/drivers/watchdog/Makefile
> +++ b/drivers/watchdog/Makefile
> @@ -143,6 +143,7 @@ obj-$(CONFIG_8xxx_WDT) += mpc8xxx_wdt.o
>  obj-$(CONFIG_MV64X60_WDT) += mv64x60_wdt.o
>  obj-$(CONFIG_PIKA_WDT) += pika_wdt.o
>  obj-$(CONFIG_BOOKE_WDT) += booke_wdt.o
> +obj-$(CONFIG_MEN_A21_WDT) += mena21_wdt.o
> 
>  # PPC64 Architecture
>  obj-$(CONFIG_WATCHDOG_RTAS) += wdrtas.o
> diff --git a/drivers/watchdog/mena21_wdt.c b/drivers/watchdog/mena21_wdt.c
> new file mode 100644
> index 0000000..96dbba9
> --- /dev/null
> +++ b/drivers/watchdog/mena21_wdt.c
> @@ -0,0 +1,270 @@
> +/*
> + * Watchdog driver for the A21 VME CPU Boards
> + *
> + * Copyright (C) 2013 MEN Mikro Elektronik Nuernberg GmbH
> + *
> + * 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
> + */
> +#include <linux/module.h>
> +#include <linux/moduleparam.h>
> +#include <linux/types.h>
> +#include <linux/kernel.h>
> +#include <linux/slab.h>
> +#include <linux/platform_device.h>
> +#include <linux/watchdog.h>
> +#include <linux/uaccess.h>
> +#include <linux/gpio.h>
> +#include <linux/of_gpio.h>
> +#include <linux/delay.h>
> +#include <linux/bitops.h>
> +
> +#define NUM_GPIOS 6
> +
> +enum a21_wdt_gpios {
> +	GPIO_WD_ENAB,
> +	GPIO_WD_FAST,
> +	GPIO_WD_TRIG,
> +	GPIO_WD_RST0,
> +	GPIO_WD_RST1,
> +	GPIO_WD_RST2,
> +};
> +
> +struct a21_wdt_drv {
> +	struct watchdog_device wdt;
> +	struct mutex lock;
> +	unsigned gpios[NUM_GPIOS];
> +};
> +
> +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 unsigned int a21_wdt_get_bootstatus(struct a21_wdt_drv *drv)
> +{
> +	int reset = 0;
> +
> +	reset |= gpio_get_value(drv->gpios[GPIO_WD_RST0]) ? (1 << 0) : 0;
> +	reset |= gpio_get_value(drv->gpios[GPIO_WD_RST1]) ? (1 << 1) : 0;
> +	reset |= gpio_get_value(drv->gpios[GPIO_WD_RST2]) ? (1 << 2) : 0;
> +
> +	return reset;
> +}
> +
> +static int a21_wdt_start(struct watchdog_device *wdt)
> +{
> +	struct a21_wdt_drv *drv = watchdog_get_drvdata(wdt);
> +
> +	mutex_lock(&drv->lock);
> +
> +	gpio_set_value(drv->gpios[GPIO_WD_ENAB], 1);
> +
> +	mutex_unlock(&drv->lock);
> +
> +	return 0;
> +}
> +
> +static int a21_wdt_stop(struct watchdog_device *wdt)
> +{
> +	struct a21_wdt_drv *drv = watchdog_get_drvdata(wdt);
> +
> +	mutex_lock(&drv->lock);
> +
> +	gpio_set_value(drv->gpios[GPIO_WD_ENAB], 0);
> +
> +	mutex_unlock(&drv->lock);
> +
> +	return 0;
> +}
> +
> +static int a21_wdt_ping(struct watchdog_device *wdt)
> +{
> +	struct a21_wdt_drv *drv = watchdog_get_drvdata(wdt);
> +
> +	mutex_lock(&drv->lock);
> +
> +	gpio_set_value(drv->gpios[GPIO_WD_TRIG], 0);
> +	ndelay(10);
> +	gpio_set_value(drv->gpios[GPIO_WD_TRIG], 1);
> +
> +	mutex_unlock(&drv->lock);
> +
> +	return 0;
> +}
> +
> +static int a21_wdt_set_timeout(struct watchdog_device *wdt,
> +			       unsigned int timeout)
> +{
> +	struct a21_wdt_drv *drv = watchdog_get_drvdata(wdt);
> +
> +	if (timeout != 1 && timeout != 30) {
> +		dev_err(wdt->dev, "Only 1 and 30 allowed as timeout\n");
> +		return -EINVAL;
> +	}
> +
> +	if (timeout == 30 && wdt->timeout == 1) {
> +		dev_err(wdt->dev,
> +			"Transition from fast to slow mode not allowed\n");
> +		return -EINVAL;
> +	}
> +
> +	mutex_lock(&drv->lock);
> +
> +	if (timeout == 1)
> +		gpio_set_value(drv->gpios[GPIO_WD_FAST], 1);
> +	else
> +		gpio_set_value(drv->gpios[GPIO_WD_FAST], 0);
> +
> +	wdt->timeout = timeout;
> +
> +	mutex_unlock(&drv->lock);
> +
> +	return 0;
> +}
> +
> +static const struct watchdog_info a21_wdt_info = {
> +	.options = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE,
> +	.identity = "MEN A21 Watchdog",
> +};
> +
> +static const struct watchdog_ops a21_wdt_ops = {
> +	.owner = THIS_MODULE,
> +	.start = a21_wdt_start,
> +	.stop = a21_wdt_stop,
> +	.ping = a21_wdt_ping,
> +	.set_timeout = a21_wdt_set_timeout,
> +};
> +
> +static struct watchdog_device a21_wdt = {
> +	.info = &a21_wdt_info,
> +	.ops = &a21_wdt_ops,
> +	.min_timeout = 1,
> +	.max_timeout = 30,
> +};
> +
> +static int a21_wdt_probe(struct platform_device *pdev)
> +{
> +	struct device_node *node;
> +	struct a21_wdt_drv *drv;
> +	unsigned int reset = 0;
> +	int num_gpios;
> +	int ret;
> +	int i;
> +
> +	drv = devm_kzalloc(&pdev->dev, sizeof(struct a21_wdt_drv), GFP_KERNEL);
> +	if (!drv)
> +		return -ENOMEM;
> +
> +	/* Fill GPIO pin array */
> +	node = pdev->dev.of_node;
> +
> +	num_gpios = of_gpio_count(node);
> +	if (num_gpios != NUM_GPIOS) {
> +		dev_err(&pdev->dev, "gpios DT property wrong, got %d want %d",
> +			num_gpios, NUM_GPIOS);
> +		return -ENODEV;
> +	}
> +
> +	for (i = 0; i < num_gpios; i++) {
> +		int val;
> +
> +		val = of_get_gpio(node, i);
> +		if (val < 0)
> +			return val;
> +
> +		drv->gpios[i] = val;
> +	}
> +
> +	/* Request the used GPIOs */
> +	for (i = 0; i < num_gpios; i++) {
> +		ret = devm_gpio_request(&pdev->dev, drv->gpios[i],
> +					"MEN A21 Watchdog");
> +		if (ret)
> +			return ret;
> +
> +		if (i < GPIO_WD_RST0)
> +			ret = gpio_direction_output(drv->gpios[i],
> +						gpio_get_value(drv->gpios[i]));
> +		else		/* GPIO_WD_RST[0..2] are inputs */
> +			ret = gpio_direction_input(drv->gpios[i]);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	mutex_init(&drv->lock);
> +	watchdog_init_timeout(&a21_wdt, 30, &pdev->dev);
> +	watchdog_set_nowayout(&a21_wdt, nowayout);
> +	watchdog_set_drvdata(&a21_wdt, drv);
> +
> +	reset = a21_wdt_get_bootstatus(drv);
> +	if (reset == 2)
> +		a21_wdt.bootstatus |= WDIOF_EXTERN1;
> +	else if (reset == 4)
> +		a21_wdt.bootstatus |= WDIOF_CARDRESET;
> +	else if (reset == 5)
> +		a21_wdt.bootstatus |= WDIOF_POWERUNDER;
> +	else if (reset == 7)
> +		a21_wdt.bootstatus |= WDIOF_EXTERN2;
> +
> +	ret = watchdog_register_device(&a21_wdt);
> +	if (ret) {
> +		dev_err(&pdev->dev, "Cannot register watchdog device\n");
> +		goto err_register_wd;
> +	}
> +
> +	dev_set_drvdata(&pdev->dev, drv);
> +
> +	dev_info(&pdev->dev, "MEN A21 watchdog timer driver enabled\n");
> +
> +	return 0;
> +
> +err_register_wd:
> +	mutex_destroy(&drv->lock);
> +
> +	return ret;
> +}
> +
> +static int a21_wdt_remove(struct platform_device *pdev)
> +{
> +	struct a21_wdt_drv *drv = dev_get_drvdata(&pdev->dev);
> +
> +	dev_warn(&pdev->dev,
> +		"Unregistering A21 watchdog driver, board may reboot\n");
> +
> +	watchdog_unregister_device(&drv->wdt);
> +
> +	mutex_destroy(&drv->lock);
> +
> +	return 0;
> +}
> +
> +static void a21_wdt_shutdown(struct platform_device *pdev)
> +{
> +	struct a21_wdt_drv *drv = dev_get_drvdata(&pdev->dev);
> +
> +	gpio_set_value(drv->gpios[GPIO_WD_ENAB], 0);
> +}
> +
> +static const struct of_device_id a21_wdt_ids[] = {
> +	{ .compatible = "men,a021-wdt" },
> +	{ },
> +};
> +
> +static struct platform_driver a21_wdt_driver = {
> +	.probe = a21_wdt_probe,
> +	.remove = a21_wdt_remove,
> +	.shutdown = a21_wdt_shutdown,
> +	.driver = {
> +		.name = "a21-watchdog",
> +		.of_match_table = a21_wdt_ids,
> +	},
> +};
> +
> +module_platform_driver(a21_wdt_driver);
> +
> +MODULE_AUTHOR("MEN Mikro Elektronik");
> +MODULE_DESCRIPTION("MEN A21 Watchdog");
> +MODULE_LICENSE("GPL");
> +MODULE_ALIAS("platform:a21-watchdog");
> --
> 1.7.9.5
> 
> --
> 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
> 

[PATCH RESEND v10 1/2] watchdog: New watchdog driver for MEN A21 watchdogs

[Johannes Thumshirn]

This patch adds the driver for the watchdog devices found on MEN Mikro
Elektronik A21 VMEbus CPU Carrier Boards. It has DT-support and uses the
watchdog framework.

Signed-off-by: Johannes Thumshirn <johannes.thumshirn@men.de>
Reviewed-by: Guenter Roeck <linux@roeck-us.net>
---

 Revision 2:
 * Removed unneeded open flag in struct a21_wdt_drv
 * Corrected 3bit reason code from gpio
 * Additional sysfs files are now part of watchdog sysfs
 * Changed OFF/ON delay in ping from 400ms to 10ns
 * Reworked timeout setting
 * Removed a21_wdt_ioctl(...)

 Revision 3:
 * Changed pr_{err,info} to dev_{err,info}
 * Removed out of memory error print
 * Transition from "fast" to "slow" mode not allowed by chip

 Revision 4:
 * Remove reboot_notifier and place disable code into platform_device's shutdown function
 * Removed sysfs interface

 Revision 5:
 * Added setting of .bootstatus on driver init
 * Added initial timeout on driver init

 Revision 6:
 * Use watchdog_init_timeout() to initialize timeout

 Revision 7:
 * Fix possible get_bootstatus race condition

 Revision 8:
 * a21_wdt_get_bootstatus() should return reset code
 * GPIOs are supplied via DT instead of being hardcoded. Code derived from
   (drivers/hwmon/gpio-fan.c)
 * Added Devicetree binding document
 * Driver now depends on GPIOLIB

 Revision 9:
 * We need 6 GPIOs not only 1 to work
 * GPIO_WD_RST[0..2] are inputs

 Revision 10:
 * Don't perform checks of Watchdog core again.
 * Some style fixes

 .../devicetree/bindings/gpio/men-a021-wdt.txt      |   25 ++
 MAINTAINERS                                        |    6 +
 drivers/watchdog/Kconfig                           |   12 +
 drivers/watchdog/Makefile                          |    1 +
 drivers/watchdog/mena21_wdt.c                      |  270 ++++++++++++++++++++
 5 files changed, 314 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/gpio/men-a021-wdt.txt
 create mode 100644 drivers/watchdog/mena21_wdt.c

diff --git a/Documentation/devicetree/bindings/gpio/men-a021-wdt.txt b/Documentation/devicetree/bindings/gpio/men-a021-wdt.txt
new file mode 100644
index 0000000..370dee3
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/men-a021-wdt.txt
@@ -0,0 +1,25 @@
+Bindings for MEN A21 Watchdog device connected to GPIO lines
+
+Required properties:
+- compatible: "men,a021-wdt"
+- gpios: Specifies the pins that control the Watchdog, order:
+  1: Watchdog enable
+  2: Watchdog fast-mode
+  3: Watchdog trigger
+  4: Watchdog reset cause bit 0
+  5: Watchdog reset cause bit 1
+  6: Watchdog reset cause bit 2
+
+Optional properties:
+- None
+
+Example:
+	watchdog {
+		compatible ="men,a021-wdt";
+		gpios = <&gpio3 9  1	/* WD_EN */
+			 &gpio3 10 1	/* WD_FAST */
+			 &gpio3 11 1	/* WD_TRIG */
+			 &gpio3 6  1	/* RST_CAUSE[0] */
+			 &gpio3 7  1	/* RST_CAUSE[1] */
+			 &gpio3 8  1>;	/* RST_CAUSE[2] */
+	};
diff --git a/MAINTAINERS b/MAINTAINERS
index 5be702c..824261e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5317,6 +5317,12 @@ F:	drivers/mtd/
 F:	include/linux/mtd/
 F:	include/uapi/mtd/

+MEN A21 WATCHDOG DRIVER
+M:  	Johannes Thumshirn <johannes.thumshirn@men.de>
+L:	linux-watchdog@vger.kernel.org
+S:	Supported
+F:	drivers/watchdog/mena21_wdt.c
+
 METAG ARCHITECTURE
 M:	James Hogan <james.hogan@imgtec.com>
 S:	Supported
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index e89fc31..8b143ee 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -1172,6 +1172,18 @@ config BOOKE_WDT_DEFAULT_TIMEOUT

 	  The value can be overridden by the wdt_period command-line parameter.

+config MEN_A21_WDT
+       tristate "MEN A21 VME CPU Carrier Board Watchdog Timer"
+       select WATCHDOG_CORE
+       depends on GPIOLIB
+       help
+        Watchdog driver for MEN A21 VMEbus CPU Carrier Boards.
+
+	The driver can also be built as a module. If so, the module will be
+	called mena21_wdt.
+
+	If unsure select N here.
+
 # PPC64 Architecture

 config WATCHDOG_RTAS
diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
index a300b94..bffdcb1 100644
--- a/drivers/watchdog/Makefile
+++ b/drivers/watchdog/Makefile
@@ -143,6 +143,7 @@ obj-$(CONFIG_8xxx_WDT) += mpc8xxx_wdt.o
 obj-$(CONFIG_MV64X60_WDT) += mv64x60_wdt.o
 obj-$(CONFIG_PIKA_WDT) += pika_wdt.o
 obj-$(CONFIG_BOOKE_WDT) += booke_wdt.o
+obj-$(CONFIG_MEN_A21_WDT) += mena21_wdt.o

 # PPC64 Architecture
 obj-$(CONFIG_WATCHDOG_RTAS) += wdrtas.o
diff --git a/drivers/watchdog/mena21_wdt.c b/drivers/watchdog/mena21_wdt.c
new file mode 100644
index 0000000..96dbba9
--- /dev/null
+++ b/drivers/watchdog/mena21_wdt.c
@@ -0,0 +1,270 @@
+/*
+ * Watchdog driver for the A21 VME CPU Boards
+ *
+ * Copyright (C) 2013 MEN Mikro Elektronik Nuernberg GmbH
+ *
+ * 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
+ */
+#include <linux/module.h>
+#include <linux/moduleparam.h>
+#include <linux/types.h>
+#include <linux/kernel.h>
+#include <linux/slab.h>
+#include <linux/platform_device.h>
+#include <linux/watchdog.h>
+#include <linux/uaccess.h>
+#include <linux/gpio.h>
+#include <linux/of_gpio.h>
+#include <linux/delay.h>
+#include <linux/bitops.h>
+
+#define NUM_GPIOS 6
+
+enum a21_wdt_gpios {
+	GPIO_WD_ENAB,
+	GPIO_WD_FAST,
+	GPIO_WD_TRIG,
+	GPIO_WD_RST0,
+	GPIO_WD_RST1,
+	GPIO_WD_RST2,
+};
+
+struct a21_wdt_drv {
+	struct watchdog_device wdt;
+	struct mutex lock;
+	unsigned gpios[NUM_GPIOS];
+};
+
+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 unsigned int a21_wdt_get_bootstatus(struct a21_wdt_drv *drv)
+{
+	int reset = 0;
+
+	reset |= gpio_get_value(drv->gpios[GPIO_WD_RST0]) ? (1 << 0) : 0;
+	reset |= gpio_get_value(drv->gpios[GPIO_WD_RST1]) ? (1 << 1) : 0;
+	reset |= gpio_get_value(drv->gpios[GPIO_WD_RST2]) ? (1 << 2) : 0;
+
+	return reset;
+}
+
+static int a21_wdt_start(struct watchdog_device *wdt)
+{
+	struct a21_wdt_drv *drv = watchdog_get_drvdata(wdt);
+
+	mutex_lock(&drv->lock);
+
+	gpio_set_value(drv->gpios[GPIO_WD_ENAB], 1);
+
+	mutex_unlock(&drv->lock);
+
+	return 0;
+}
+
+static int a21_wdt_stop(struct watchdog_device *wdt)
+{
+	struct a21_wdt_drv *drv = watchdog_get_drvdata(wdt);
+
+	mutex_lock(&drv->lock);
+
+	gpio_set_value(drv->gpios[GPIO_WD_ENAB], 0);
+
+	mutex_unlock(&drv->lock);
+
+	return 0;
+}
+
+static int a21_wdt_ping(struct watchdog_device *wdt)
+{
+	struct a21_wdt_drv *drv = watchdog_get_drvdata(wdt);
+
+	mutex_lock(&drv->lock);
+
+	gpio_set_value(drv->gpios[GPIO_WD_TRIG], 0);
+	ndelay(10);
+	gpio_set_value(drv->gpios[GPIO_WD_TRIG], 1);
+
+	mutex_unlock(&drv->lock);
+
+	return 0;
+}
+
+static int a21_wdt_set_timeout(struct watchdog_device *wdt,
+			       unsigned int timeout)
+{
+	struct a21_wdt_drv *drv = watchdog_get_drvdata(wdt);
+
+	if (timeout != 1 && timeout != 30) {
+		dev_err(wdt->dev, "Only 1 and 30 allowed as timeout\n");
+		return -EINVAL;
+	}
+
+	if (timeout == 30 && wdt->timeout == 1) {
+		dev_err(wdt->dev,
+			"Transition from fast to slow mode not allowed\n");
+		return -EINVAL;
+	}
+
+	mutex_lock(&drv->lock);
+
+	if (timeout == 1)
+		gpio_set_value(drv->gpios[GPIO_WD_FAST], 1);
+	else
+		gpio_set_value(drv->gpios[GPIO_WD_FAST], 0);
+
+	wdt->timeout = timeout;
+
+	mutex_unlock(&drv->lock);
+
+	return 0;
+}
+
+static const struct watchdog_info a21_wdt_info = {
+	.options = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE,
+	.identity = "MEN A21 Watchdog",
+};
+
+static const struct watchdog_ops a21_wdt_ops = {
+	.owner = THIS_MODULE,
+	.start = a21_wdt_start,
+	.stop = a21_wdt_stop,
+	.ping = a21_wdt_ping,
+	.set_timeout = a21_wdt_set_timeout,
+};
+
+static struct watchdog_device a21_wdt = {
+	.info = &a21_wdt_info,
+	.ops = &a21_wdt_ops,
+	.min_timeout = 1,
+	.max_timeout = 30,
+};
+
+static int a21_wdt_probe(struct platform_device *pdev)
+{
+	struct device_node *node;
+	struct a21_wdt_drv *drv;
+	unsigned int reset = 0;
+	int num_gpios;
+	int ret;
+	int i;
+
+	drv = devm_kzalloc(&pdev->dev, sizeof(struct a21_wdt_drv), GFP_KERNEL);
+	if (!drv)
+		return -ENOMEM;
+
+	/* Fill GPIO pin array */
+	node = pdev->dev.of_node;
+
+	num_gpios = of_gpio_count(node);
+	if (num_gpios != NUM_GPIOS) {
+		dev_err(&pdev->dev, "gpios DT property wrong, got %d want %d",
+			num_gpios, NUM_GPIOS);
+		return -ENODEV;
+	}
+
+	for (i = 0; i < num_gpios; i++) {
+		int val;
+
+		val = of_get_gpio(node, i);
+		if (val < 0)
+			return val;
+
+		drv->gpios[i] = val;
+	}
+
+	/* Request the used GPIOs */
+	for (i = 0; i < num_gpios; i++) {
+		ret = devm_gpio_request(&pdev->dev, drv->gpios[i],
+					"MEN A21 Watchdog");
+		if (ret)
+			return ret;
+
+		if (i < GPIO_WD_RST0)
+			ret = gpio_direction_output(drv->gpios[i],
+						gpio_get_value(drv->gpios[i]));
+		else		/* GPIO_WD_RST[0..2] are inputs */
+			ret = gpio_direction_input(drv->gpios[i]);
+		if (ret)
+			return ret;
+	}
+
+	mutex_init(&drv->lock);
+	watchdog_init_timeout(&a21_wdt, 30, &pdev->dev);
+	watchdog_set_nowayout(&a21_wdt, nowayout);
+	watchdog_set_drvdata(&a21_wdt, drv);
+
+	reset = a21_wdt_get_bootstatus(drv);
+	if (reset == 2)
+		a21_wdt.bootstatus |= WDIOF_EXTERN1;
+	else if (reset == 4)
+		a21_wdt.bootstatus |= WDIOF_CARDRESET;
+	else if (reset == 5)
+		a21_wdt.bootstatus |= WDIOF_POWERUNDER;
+	else if (reset == 7)
+		a21_wdt.bootstatus |= WDIOF_EXTERN2;
+
+	ret = watchdog_register_device(&a21_wdt);
+	if (ret) {
+		dev_err(&pdev->dev, "Cannot register watchdog device\n");
+		goto err_register_wd;
+	}
+
+	dev_set_drvdata(&pdev->dev, drv);
+
+	dev_info(&pdev->dev, "MEN A21 watchdog timer driver enabled\n");
+
+	return 0;
+
+err_register_wd:
+	mutex_destroy(&drv->lock);
+
+	return ret;
+}
+
+static int a21_wdt_remove(struct platform_device *pdev)
+{
+	struct a21_wdt_drv *drv = dev_get_drvdata(&pdev->dev);
+
+	dev_warn(&pdev->dev,
+		"Unregistering A21 watchdog driver, board may reboot\n");
+
+	watchdog_unregister_device(&drv->wdt);
+
+	mutex_destroy(&drv->lock);
+
+	return 0;
+}
+
+static void a21_wdt_shutdown(struct platform_device *pdev)
+{
+	struct a21_wdt_drv *drv = dev_get_drvdata(&pdev->dev);
+
+	gpio_set_value(drv->gpios[GPIO_WD_ENAB], 0);
+}
+
+static const struct of_device_id a21_wdt_ids[] = {
+	{ .compatible = "men,a021-wdt" },
+	{ },
+};
+
+static struct platform_driver a21_wdt_driver = {
+	.probe = a21_wdt_probe,
+	.remove = a21_wdt_remove,
+	.shutdown = a21_wdt_shutdown,
+	.driver = {
+		.name = "a21-watchdog",
+		.of_match_table = a21_wdt_ids,
+	},
+};
+
+module_platform_driver(a21_wdt_driver);
+
+MODULE_AUTHOR("MEN Mikro Elektronik");
+MODULE_DESCRIPTION("MEN A21 Watchdog");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:a21-watchdog");
--
1.7.9.5

[PATCH RESEND v10 2/2] watchdog: Sysfs interface for MEN A21 watchdog

[Johannes Thumshirn]

This patch adds a sysfs interface for the watchdog
device found on MEN A21 Boards.

The newly generated files are:
* rebootcause:
Can be one of:
Power on Reset,
CPU Reset Request,
Push Button,
FPGA Reset Request,
Watchdog,
Local Power Bad,
Invalid or
BDI
and shows the reason of the boards last reboot.

* active:
Shows if the watchdog CPLD is actually running

* allow_disable:
Shows if the watchdog is allowed to be disabled (NOWAYOUT disabled)

* fastmode:
Shows if the CPLD is running in fast mode (1s timeout), once it is in
fastmode it can't be switched back to slow mode (30s timeout) until the
next reboot.

Signed-off-by: Johannes Thumshirn <johannes.thumshirn@men.de>
---

 Revision 2:
 * Re-worked sysfs patch to apply on re-worked base

 Revision 3:
 * Re-worked sysfs patch to apply on re-worked base

 drivers/watchdog/mena21_wdt.c |   89 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 89 insertions(+)

diff --git a/drivers/watchdog/mena21_wdt.c b/drivers/watchdog/mena21_wdt.c
index 96dbba9..91cc90b 100644
--- a/drivers/watchdog/mena21_wdt.c
+++ b/drivers/watchdog/mena21_wdt.c
@@ -22,6 +22,17 @@

 #define NUM_GPIOS 6

+static char *reset_causes[] = {
+	"Power On Reset",
+	"CPU Reset Request",
+	"Push Button",
+	"FPGA Reset Request",
+	"Watchdog",
+	"Local Power Bad",
+	"Invalid",
+	"BDI",
+};
+
 enum a21_wdt_gpios {
 	GPIO_WD_ENAB,
 	GPIO_WD_FAST,
@@ -53,6 +64,73 @@ static unsigned int a21_wdt_get_bootstatus(struct a21_wdt_drv *drv)
 	return reset;
 }

+static ssize_t rebootcause_show(struct device *dev,
+				struct device_attribute *attr,
+				char *buf)
+{
+	struct a21_wdt_drv *drv = dev_get_drvdata(dev);
+	unsigned int reset = 0;
+
+	reset = a21_wdt_get_bootstatus(drv);
+
+	return sprintf(buf, "%s\n", reset_causes[reset]);
+}
+static DEVICE_ATTR(rebootcause, S_IRUGO, rebootcause_show, NULL);
+
+static ssize_t active_show(struct device *dev, struct device_attribute *attr,
+			   char *buf)
+{
+	struct a21_wdt_drv *drv = dev_get_drvdata(dev);
+
+	return sprintf(buf, "%d\n", !!gpio_get_value(drv->gpios[GPIO_WD_ENAB]));
+}
+static DEVICE_ATTR(active, S_IRUGO, active_show, NULL);
+
+static ssize_t allow_disable_show(struct device *dev,
+				  struct device_attribute *attr,
+				  char *buf)
+{
+	return sprintf(buf, "%d\n", !nowayout);
+}
+static DEVICE_ATTR(allow_disable, S_IRUGO, allow_disable_show, NULL);
+
+static ssize_t fastmode_show(struct device *dev, struct device_attribute *attr,
+			     char *buf)
+{
+	struct a21_wdt_drv *drv = dev_get_drvdata(dev);
+
+	return sprintf(buf, "%d\n", !!gpio_get_value(drv->gpios[GPIO_WD_FAST]));
+}
+static DEVICE_ATTR(fastmode, S_IRUGO, fastmode_show, NULL);
+
+static int a21_wdt_create_files(struct watchdog_device *wdev)
+{
+	int ret;
+
+	ret = device_create_file(wdev->dev, &dev_attr_rebootcause);
+	if (ret)
+		return ret;
+	ret = device_create_file(wdev->dev, &dev_attr_active);
+	if (ret)
+		return ret;
+	ret = device_create_file(wdev->dev, &dev_attr_allow_disable);
+	if (ret)
+		return ret;
+	ret = device_create_file(wdev->dev, &dev_attr_fastmode);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static void a21_wdt_remove_files(struct watchdog_device *wdev)
+{
+	device_remove_file(wdev->dev, &dev_attr_rebootcause);
+	device_remove_file(wdev->dev, &dev_attr_active);
+	device_remove_file(wdev->dev, &dev_attr_allow_disable);
+	device_remove_file(wdev->dev, &dev_attr_fastmode);
+}
+
 static int a21_wdt_start(struct watchdog_device *wdt)
 {
 	struct a21_wdt_drv *drv = watchdog_get_drvdata(wdt);
@@ -215,11 +293,20 @@ static int a21_wdt_probe(struct platform_device *pdev)
 	}

 	dev_set_drvdata(&pdev->dev, drv);
+	dev_set_drvdata(a21_wdt.dev, drv);
+
+	ret = a21_wdt_create_files(&a21_wdt);
+	if (ret) {
+		dev_err(&pdev->dev, "Cannot create sysfs entries\n");
+		goto err_create_sysfs;
+	}

 	dev_info(&pdev->dev, "MEN A21 watchdog timer driver enabled\n");

 	return 0;

+err_create_sysfs:
+	watchdog_unregister_device(&drv->wdt);
 err_register_wd:
 	mutex_destroy(&drv->lock);

@@ -233,6 +320,8 @@ static int a21_wdt_remove(struct platform_device *pdev)
 	dev_warn(&pdev->dev,
 		"Unregistering A21 watchdog driver, board may reboot\n");

+	a21_wdt_remove_files(&drv->wdt);
+
 	watchdog_unregister_device(&drv->wdt);

 	mutex_destroy(&drv->lock);
--
1.7.9.5

Re: [PATCH RESEND v10 2/2] watchdog: Sysfs interface for MEN A21 watchdog

[Johannes Thumshirn]

Hi Wim,

I just wanted to know if you already had time to review the driver?  With
Guenter's help I got rid of some of the quirks still in and he issued his
Reviewed-by. I also imporoved devicetree support and wrote the dts binding
document as Arnd Bergmann suggested. I know you're probably quite busy and 3.10
just came out, but it would be cool to know the current status.

Thanks,
Johannes

Re: [PATCH RESEND v10 1/2] watchdog: New watchdog driver for MEN A21 watchdogs

[Wim Van Sebroeck]

Hi Johannes,

> This patch adds the driver for the watchdog devices found on MEN Mikro
> Elektronik A21 VMEbus CPU Carrier Boards. It has DT-support and uses the
> watchdog framework.
> 
> Signed-off-by: Johannes Thumshirn <johannes.thumshirn@men.de>
> Reviewed-by: Guenter Roeck <linux@roeck-us.net>

I added this patch to linux-watchdog-next.
I am still thinking about wether or not we shouldn't add a timer to this
watchdog device driver. But that's something we can change later on.

Kind regards,
Wim.

Re: [PATCH RESEND v10 2/2] watchdog: Sysfs interface for MEN A21 watchdog

[Wim Van Sebroeck]

Hi Johannes,

> This patch adds a sysfs interface for the watchdog
> device found on MEN A21 Boards.
> 
> The newly generated files are:
> * rebootcause:
> Can be one of:
> Power on Reset,
> CPU Reset Request,
> Push Button,
> FPGA Reset Request,
> Watchdog,
> Local Power Bad,
> Invalid or
> BDI
> and shows the reason of the boards last reboot.
> 
> * active:
> Shows if the watchdog CPLD is actually running
> 
> * allow_disable:
> Shows if the watchdog is allowed to be disabled (NOWAYOUT disabled)

allow_disable should be nowayout because that is the general watchdog
parameter we will have for all watchdog drivers.

> * fastmode:
> Shows if the CPLD is running in fast mode (1s timeout), once it is in
> fastmode it can't be switched back to slow mode (30s timeout) until the
> next reboot.
> 
> Signed-off-by: Johannes Thumshirn <johannes.thumshirn@men.de>

I will not add this patch and put it in my waiting queue.
Reason: we should first do the sysfs stuff for the wwatchdog_core (since
active and nowayout are parameters that will be in the sysfs watchdog core).

Kind regards,
Wim.

Re: [PATCH RESEND v10 1/2] watchdog: New watchdog driver for MEN A21 watchdogs

[Johannes Thumshirn]

On Fri, Jul 05, 2013 at 11:01:19PM +0200, Wim Van Sebroeck wrote:
> Hi Johannes,
>
> > This patch adds the driver for the watchdog devices found on MEN Mikro
> > Elektronik A21 VMEbus CPU Carrier Boards. It has DT-support and uses the
> > watchdog framework.
> >
> > Signed-off-by: Johannes Thumshirn <johannes.thumshirn@men.de>
> > Reviewed-by: Guenter Roeck <linux@roeck-us.net>
>
> I added this patch to linux-watchdog-next.
> I am still thinking about wether or not we shouldn't add a timer to this
> watchdog device driver. But that's something we can change later on.

Probably yes, but that really would be some work for watchdog core imho.

Anyway, tanks for adding the patch.

Thanks,
Johannes

>
> Kind regards,
> Wim.
>

Re: [PATCH RESEND v10 2/2] watchdog: Sysfs interface for MEN A21 watchdog

[Johannes Thumshirn]

On Fri, Jul 05, 2013 at 11:03:21PM +0200, Wim Van Sebroeck wrote:
> Hi Johannes,
>
> > This patch adds a sysfs interface for the watchdog
> > device found on MEN A21 Boards.
> >
> > The newly generated files are:
> > * rebootcause:
> > Can be one of:
> > Power on Reset,
> > CPU Reset Request,
> > Push Button,
> > FPGA Reset Request,
> > Watchdog,
> > Local Power Bad,
> > Invalid or
> > BDI
> > and shows the reason of the boards last reboot.
> >
> > * active:
> > Shows if the watchdog CPLD is actually running
> >
> > * allow_disable:
> > Shows if the watchdog is allowed to be disabled (NOWAYOUT disabled)
>
> allow_disable should be nowayout because that is the general watchdog
> parameter we will have for all watchdog drivers.
>
> > * fastmode:
> > Shows if the CPLD is running in fast mode (1s timeout), once it is in
> > fastmode it can't be switched back to slow mode (30s timeout) until the
> > next reboot.
> >
> > Signed-off-by: Johannes Thumshirn <johannes.thumshirn@men.de>
>
> I will not add this patch and put it in my waiting queue.
> Reason: we should first do the sysfs stuff for the wwatchdog_core (since
> active and nowayout are parameters that will be in the sysfs watchdog core).
>
> Kind regards,
> Wim.
>


Ok. If you want to offload it, I think I could spend some time on it the next
weeks, as I'll probably have to write a driver for a Ti TPS3813K33 watchdog
anyway, I can have a look into watchdog core as well.

Byte,
Johannes