All of lore.kernel.org
 help / color / mirror / Atom feed
From: Wolfram Sang <w.sang@pengutronix.de>
To: linuxppc-dev@ozlabs.org
Subject: [RFC] GPIO-Watchdog in devicetree
Date: Mon, 22 Sep 2008 21:43:57 +0200	[thread overview]
Message-ID: <20080922194357.GA32041@pengutronix.de> (raw)

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

Hello all,

I understood that the device-tree is for describing hardware and should
not contain driver specific information. I have problems drawing this
line right now. I made a driver for watchdogs which are pinged by
toggling a gpio. Currently the device-tree entry looks like this:

	watchdog@gpio {
		compatible = "gpio-watchdog";
		gpios =	<&gpio_simple 19 0>;
	};

Then, there are two module parameters. One to define the initial state of
the gpio (0 or 1), one to define the length of the pulse when serving
the watchdog (default 1 us). Now my question is:

Is it plausible to say that the module parameters would also fit to the
device-tree as properties? Recently, I tend to say so as otherwise the
description of the watchdog is incomplete. Then again, one might argue
to develop a specific watchdog driver instead of a generic one, and use
something like compatible = "<myvendor>, <mywatchdog>" which would
result in lots of duplicated code per watchdog. So, which way to go? I
am really undecided and would be happy to hear opinions.

For completeness, I'll append the current version of my driver.

All the best,

   Wolfram

===

From; Wolfram Sang <w.sang@pengutronix.de>
Subject; of-driver for external gpio-triggered watchdogs

Still needs tests and review before it can go mainline.

Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
---
 drivers/watchdog/Kconfig       |    8 +
 drivers/watchdog/Makefile      |    1 
 drivers/watchdog/of_gpio_wdt.c |  188 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 197 insertions(+)

Index: drivers/watchdog/Kconfig
===================================================================
--- drivers/watchdog/Kconfig.orig
+++ drivers/watchdog/Kconfig
@@ -55,6 +55,14 @@
 	  To compile this driver as a module, choose M here: the
 	  module will be called softdog.
 
+config OF_GPIO_WDT
+	tristate "OF GPIO watchdog"
+	help
+	  This driver handles external watchdogs which are triggered by a gpio.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called of_gpio_wdt.
+
 # ALPHA Architecture
 
 # ARM Architecture
Index: drivers/watchdog/Makefile
===================================================================
--- drivers/watchdog/Makefile.orig
+++ drivers/watchdog/Makefile
@@ -124,4 +124,5 @@
 # XTENSA Architecture
 
 # Architecture Independant
+obj-$(CONFIG_OF_GPIO_WDT) += of_gpio_wdt.o
 obj-$(CONFIG_SOFT_WATCHDOG) += softdog.o
Index: drivers/watchdog/of_gpio_wdt.c
===================================================================
--- /dev/null
+++ drivers/watchdog/of_gpio_wdt.c
@@ -0,0 +1,188 @@
+/*
+ * of_gpio_wdt.c - driver for gpio-driven watchdogs
+ *
+ * Copyright (C) 2008 Pengutronix e.K.
+ *
+ * Author: Wolfram Sang <w.sang@pengutronix.de>
+ *
+ * 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; version 2 of the License.
+ *
+ */
+
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/miscdevice.h>
+#include <linux/of_platform.h>
+#include <linux/of_gpio.h>
+#include <linux/fs.h>
+#include <linux/module.h>
+#include <linux/watchdog.h>
+#include <linux/gpio.h>
+#include <linux/io.h>
+#include <linux/uaccess.h>
+
+#define DRIVER_NAME "of-gpio-wdt"
+
+
+static int wdt_gpio;
+
+static int wdt_init_state;
+module_param(wdt_init_state, bool, 0);
+MODULE_PARM_DESC(wdt_init_state,
+	"Initial state of the gpio pin (0/1), default = 0");
+
+static int wdt_toggle_delay = 1;
+module_param(wdt_toggle_delay, int, 0);
+MODULE_PARM_DESC(wdt_toggle_delay,
+	"Delay in us to keep the gpio triggered, default = 1");
+
+static void of_gpio_wdt_ping(void)
+{
+	gpio_set_value(wdt_gpio, wdt_init_state ^ 1);
+	udelay(wdt_toggle_delay);
+	gpio_set_value(wdt_gpio, wdt_init_state);
+}
+
+static ssize_t of_gpio_wdt_write(struct file *file, const char __user *buf,
+				 size_t count, loff_t *ppos)
+{
+	if (count)
+		of_gpio_wdt_ping();
+	return count;
+}
+
+static int of_gpio_wdt_open(struct inode *inode, struct file *file)
+{
+	of_gpio_wdt_ping();
+	return nonseekable_open(inode, file);
+}
+
+static int of_gpio_wdt_release(struct inode *inode, struct file *file)
+{
+	printk(KERN_CRIT "Unexpected close on watchdog device. "
+			 "File is closed, but watchdog is still running!\n");
+	return 0;
+}
+
+static int of_gpio_wdt_ioctl(struct inode *inode, struct file *file,
+				unsigned int cmd, unsigned long arg)
+{
+	void __user *argp = (void __user *)arg;
+	static struct watchdog_info ident = {
+		.options = WDIOF_KEEPALIVEPING,
+		.firmware_version = 0,
+		.identity = DRIVER_NAME,
+	};
+
+	switch (cmd) {
+
+	case WDIOC_GETSUPPORT:
+		if (copy_to_user(argp, &ident, sizeof(ident)))
+			return -EFAULT;
+		break;
+
+	case WDIOC_KEEPALIVE:
+		of_gpio_wdt_ping();
+		break;
+
+	case WDIOC_GETTEMP:
+	case WDIOC_GETTIMEOUT:
+	case WDIOC_SETOPTIONS:
+	case WDIOC_SETTIMEOUT:
+		return -EOPNOTSUPP;
+
+	default:
+		return -ENOTTY;
+	}
+
+	return 0;
+}
+
+static const struct file_operations of_gpio_wdt_fops = {
+	.owner		= THIS_MODULE,
+	.llseek		= no_llseek,
+	.write		= of_gpio_wdt_write,
+	.ioctl		= of_gpio_wdt_ioctl,
+	.open		= of_gpio_wdt_open,
+	.release	= of_gpio_wdt_release,
+};
+
+static struct miscdevice of_gpio_wdt_miscdev = {
+	.minor	= WATCHDOG_MINOR,
+	.name	= "watchdog",
+	.fops	= &of_gpio_wdt_fops,
+};
+
+static int __devinit of_gpio_wdt_probe(struct of_device *op,
+			const struct of_device_id *match)
+{
+	int ret;
+
+	wdt_gpio = of_get_gpio(op->node, 0);
+	if (wdt_gpio < 0) {
+		dev_err(&op->dev, "could not determine gpio! (err=%d)\n",
+			wdt_gpio);
+		return wdt_gpio;
+	}
+
+	ret = gpio_request(wdt_gpio, DRIVER_NAME);
+	if (ret) {
+		dev_err(&op->dev, "could not get gpio! (err=%d)\n", ret);
+		return ret;
+	}
+
+	gpio_direction_output(wdt_gpio, wdt_init_state);
+
+	ret = misc_register(&of_gpio_wdt_miscdev);
+	if (ret) {
+		dev_err(&op->dev, "cannot register miscdev on minor=%d "
+				"(err=%d)\n", WATCHDOG_MINOR, ret);
+		gpio_free(wdt_gpio);
+		return -ENODEV;
+	}
+
+	dev_info(&op->dev, "gpio-watchdog driver started using gpio %d.\n",
+		wdt_gpio);
+	return 0;
+}
+
+static int __devexit of_gpio_wdt_remove(struct of_device *op)
+{
+	misc_deregister(&of_gpio_wdt_miscdev);
+	gpio_free(wdt_gpio);
+	return 0;
+}
+
+static struct of_device_id of_gpio_wdt_match[] = {
+	{ .compatible = "gpio-watchdog", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, of_gpio_wdt_match);
+
+static struct of_platform_driver of_gpio_wdt_driver = {
+	.owner = THIS_MODULE,
+	.name = DRIVER_NAME,
+	.match_table = of_gpio_wdt_match,
+	.probe = of_gpio_wdt_probe,
+	.remove = __devexit_p(of_gpio_wdt_remove),
+};
+
+static int __init of_gpio_wdt_init(void)
+{
+	return of_register_platform_driver(&of_gpio_wdt_driver);
+}
+
+static void __exit of_gpio_wdt_exit(void)
+{
+	of_unregister_platform_driver(&of_gpio_wdt_driver);
+}
+
+module_init(of_gpio_wdt_init);
+module_exit(of_gpio_wdt_exit);
+
+MODULE_AUTHOR("Wolfram Sang <w.sang@pengutronix.de>");
+MODULE_DESCRIPTION("Driver for gpio-triggered watchdogs");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS_MISCDEV(WATCHDOG_MINOR);

-- 
  Dipl.-Ing. Wolfram Sang | http://www.pengutronix.de
 Pengutronix - Linux Solutions for Science and Industry

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

             reply	other threads:[~2008-09-22 19:44 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-09-22 19:43 Wolfram Sang [this message]
2008-09-23 15:02 ` [RFC] GPIO-Watchdog in devicetree Grant Likely
2008-09-25 17:59   ` Scott Wood
2008-09-25 18:10     ` Grant Likely
2008-09-25 18:21       ` Scott Wood
2008-09-26  1:15         ` David Gibson
2008-09-26  1:15 ` David Gibson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20080922194357.GA32041@pengutronix.de \
    --to=w.sang@pengutronix.de \
    --cc=linuxppc-dev@ozlabs.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.