[PATCH] watchdog: Add MOXA ART watchdog driver

[PATCH] watchdog: Add MOXA ART watchdog driver

[Jonas Jensen]

Add watchdog driver for MOXA ART SoCs.

Signed-off-by: Jonas Jensen <jonas.jensen@gmail.com>
---

Notes:
    Applies to next-20130711

 drivers/watchdog/Kconfig      |  10 +++
 drivers/watchdog/Makefile     |   1 +
 drivers/watchdog/moxart_wdt.c | 190 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 201 insertions(+)
 create mode 100644 drivers/watchdog/moxart_wdt.c

diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 362085d..6883aab 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -382,6 +382,16 @@ config RETU_WATCHDOG
 	  To compile this driver as a module, choose M here: the
 	  module will be called retu_wdt.
 
+config MOXART_WDT
+	tristate "MOXART watchdog"
+	depends on ARCH_MOXART
+	help
+	  Say Y here to include Watchdog timer support for the watchdog
+	  existing on the MOXA ART SoC series platforms.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called moxart_wdt.
+
 # AVR32 Architecture
 
 config AT32AP700X_WDT
diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
index 2f26a0b..7cab53a 100644
--- a/drivers/watchdog/Makefile
+++ b/drivers/watchdog/Makefile
@@ -54,6 +54,7 @@ obj-$(CONFIG_IMX2_WDT) += imx2_wdt.o
 obj-$(CONFIG_UX500_WATCHDOG) += ux500_wdt.o
 obj-$(CONFIG_RETU_WATCHDOG) += retu_wdt.o
 obj-$(CONFIG_BCM2835_WDT) += bcm2835_wdt.o
+obj-$(CONFIG_MOXART_WDT) += moxart_wdt.o
 
 # AVR32 Architecture
 obj-$(CONFIG_AT32AP700X_WDT) += at32ap700x_wdt.o
diff --git a/drivers/watchdog/moxart_wdt.c b/drivers/watchdog/moxart_wdt.c
new file mode 100644
index 0000000..c6c00bf
--- /dev/null
+++ b/drivers/watchdog/moxart_wdt.c
@@ -0,0 +1,190 @@
+/*
+ * MOXA ART SoCs watchdog driver.
+ *
+ * Copyright (C) 2013 Jonas Jensen
+ *
+ * Jonas Jensen <jonas.jensen@gmail.com>
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2.  This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+
+#include <linux/clk.h>
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/moduleparam.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/platform_device.h>
+#include <linux/types.h>
+#include <linux/watchdog.h>
+
+#include <asm/system_misc.h>
+
+#define REG_COUNT			0x4
+#define REG_MODE			0x8
+#define REG_ENABLE			0xC
+
+struct moxart_wdt_dev {
+	struct watchdog_device wdt_dev;
+	void __iomem *wdt_base;
+};
+
+static struct moxart_wdt_dev *moxart_wdt;
+static bool nowayout = WATCHDOG_NOWAYOUT;
+static int heartbeat;
+static unsigned int clock_frequency;
+static unsigned int max_timeout;
+
+static void moxart_wdt_restart(enum reboot_mode reboot_mode, const char *cmd)
+{
+	writel(1, moxart_wdt->wdt_base + REG_COUNT);
+	writel(0x5ab9, moxart_wdt->wdt_base + REG_MODE);
+	writel(0x03, moxart_wdt->wdt_base + REG_ENABLE);
+}
+
+static int moxart_wdt_stop(struct watchdog_device *wdt_dev)
+{
+	struct moxart_wdt_dev *moxart_wdt = watchdog_get_drvdata(wdt_dev);
+	void __iomem *wdt_base = moxart_wdt->wdt_base;
+
+	writel(0, wdt_base + REG_ENABLE);
+
+	return 0;
+}
+
+static int moxart_wdt_start(struct watchdog_device *wdt_dev)
+{
+	struct moxart_wdt_dev *moxart_wdt = watchdog_get_drvdata(wdt_dev);
+	void __iomem *wdt_base = moxart_wdt->wdt_base;
+
+	writel(clock_frequency * moxart_wdt->wdt_dev.timeout,
+	       wdt_base + REG_COUNT);
+	writel(0x5ab9, wdt_base + REG_MODE);
+	writel(0x03, wdt_base + REG_ENABLE);
+
+	return 0;
+}
+
+static int moxart_wdt_ping(struct watchdog_device *wdt_dev)
+{
+	moxart_wdt_start(wdt_dev);
+	return 0;
+}
+
+static int moxart_wdt_set_timeout(struct watchdog_device *wdt_dev,
+				  unsigned int timeout)
+{
+	dev_dbg(wdt_dev->dev, "%s: timeout=%u\n", __func__, timeout);
+
+	if (timeout > max_timeout)
+		return -EINVAL;
+
+	moxart_wdt->wdt_dev.timeout = timeout;
+
+	return 0;
+}
+
+static const struct watchdog_info moxart_wdt_info = {
+	.identity       = "moxart-wdt",
+	.options        = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING |
+			  WDIOF_MAGICCLOSE,
+};
+
+static const struct watchdog_ops moxart_wdt_ops = {
+	.owner          = THIS_MODULE,
+	.start          = moxart_wdt_start,
+	.stop           = moxart_wdt_stop,
+	.ping           = moxart_wdt_ping,
+	.set_timeout    = moxart_wdt_set_timeout,
+};
+
+static int moxart_wdt_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *node = dev->of_node;
+	struct resource *res;
+	struct clk *clk;
+	int err;
+
+	moxart_wdt = devm_kzalloc(&pdev->dev, sizeof(*moxart_wdt), GFP_KERNEL);
+	if (!moxart_wdt)
+		return -EINVAL;
+
+	platform_set_drvdata(pdev, moxart_wdt);
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	moxart_wdt->wdt_base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(moxart_wdt->wdt_base))
+		return PTR_ERR(moxart_wdt->wdt_base);
+
+	arm_pm_restart = moxart_wdt_restart;
+
+	clk = of_clk_get(node, 0);
+	if (IS_ERR(clk)) {
+		pr_err("%s: of_clk_get failed\n", __func__);
+		return PTR_ERR(clk);
+	}
+
+	clock_frequency = clk_get_rate(clk);
+
+	max_timeout = UINT_MAX / clock_frequency;
+
+	moxart_wdt->wdt_dev.info = &moxart_wdt_info;
+	moxart_wdt->wdt_dev.ops = &moxart_wdt_ops;
+	moxart_wdt->wdt_dev.timeout = max_timeout;
+	moxart_wdt->wdt_dev.min_timeout = 1;
+	moxart_wdt->wdt_dev.max_timeout = max_timeout;
+	moxart_wdt->wdt_dev.parent = &pdev->dev;
+
+	watchdog_init_timeout(&moxart_wdt->wdt_dev, heartbeat, &pdev->dev);
+	watchdog_set_nowayout(&moxart_wdt->wdt_dev, nowayout);
+
+	watchdog_set_drvdata(&moxart_wdt->wdt_dev, moxart_wdt);
+
+	err = watchdog_register_device(&moxart_wdt->wdt_dev);
+	if (unlikely(err))
+		return err;
+
+	dev_dbg(dev, "Watchdog enabled (heartbeat=%d sec, nowayout=%d)\n",
+		 moxart_wdt->wdt_dev.timeout, nowayout);
+
+	return 0;
+}
+
+static int moxart_wdt_remove(struct platform_device *pdev)
+{
+	struct moxart_wdt_dev *moxart_wdt = platform_get_drvdata(pdev);
+
+	watchdog_unregister_device(&moxart_wdt->wdt_dev);
+	watchdog_set_drvdata(&moxart_wdt->wdt_dev, NULL);
+
+	return 0;
+}
+
+static const struct of_device_id moxart_watchdog_match[] = {
+	{ .compatible = "moxa,moxart-watchdog" },
+	{ },
+};
+
+static struct platform_driver moxart_wdt_driver = {
+	.probe      = moxart_wdt_probe,
+	.remove     = moxart_wdt_remove,
+	.driver     = {
+		.name		= "moxart-watchdog",
+		.owner		= THIS_MODULE,
+		.of_match_table	= moxart_watchdog_match,
+	},
+};
+module_platform_driver(moxart_wdt_driver);
+
+module_param(heartbeat, int, 0);
+MODULE_PARM_DESC(heartbeat, "Watchdog heartbeat in seconds");
+
+MODULE_DESCRIPTION("MOXART watchdog driver");
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Jonas Jensen <jonas.jensen@gmail.com>");
-- 
1.8.2.1

Re: [PATCH] watchdog: Add MOXA ART watchdog driver

[Guenter Roeck]

On Thu, Jul 11, 2013 at 02:29:46PM +0200, Jonas Jensen wrote:
> Add watchdog driver for MOXA ART SoCs.
> 
> Signed-off-by: Jonas Jensen <jonas.jensen@gmail.com>
> ---
> 
> Notes:
>     Applies to next-20130711
> 
>  drivers/watchdog/Kconfig      |  10 +++
>  drivers/watchdog/Makefile     |   1 +
>  drivers/watchdog/moxart_wdt.c | 190 ++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 201 insertions(+)
>  create mode 100644 drivers/watchdog/moxart_wdt.c
> 
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index 362085d..6883aab 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -382,6 +382,16 @@ config RETU_WATCHDOG
>  	  To compile this driver as a module, choose M here: the
>  	  module will be called retu_wdt.
>  
> +config MOXART_WDT
> +	tristate "MOXART watchdog"
> +	depends on ARCH_MOXART
> +	help
> +	  Say Y here to include Watchdog timer support for the watchdog
> +	  existing on the MOXA ART SoC series platforms.
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called moxart_wdt.
> +
>  # AVR32 Architecture
>  
>  config AT32AP700X_WDT
> diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
> index 2f26a0b..7cab53a 100644
> --- a/drivers/watchdog/Makefile
> +++ b/drivers/watchdog/Makefile
> @@ -54,6 +54,7 @@ obj-$(CONFIG_IMX2_WDT) += imx2_wdt.o
>  obj-$(CONFIG_UX500_WATCHDOG) += ux500_wdt.o
>  obj-$(CONFIG_RETU_WATCHDOG) += retu_wdt.o
>  obj-$(CONFIG_BCM2835_WDT) += bcm2835_wdt.o
> +obj-$(CONFIG_MOXART_WDT) += moxart_wdt.o
>  
>  # AVR32 Architecture
>  obj-$(CONFIG_AT32AP700X_WDT) += at32ap700x_wdt.o
> diff --git a/drivers/watchdog/moxart_wdt.c b/drivers/watchdog/moxart_wdt.c
> new file mode 100644
> index 0000000..c6c00bf
> --- /dev/null
> +++ b/drivers/watchdog/moxart_wdt.c
> @@ -0,0 +1,190 @@
> +/*
> + * MOXA ART SoCs watchdog driver.
> + *
> + * Copyright (C) 2013 Jonas Jensen
> + *
> + * Jonas Jensen <jonas.jensen@gmail.com>
> + *
> + * This file is licensed under the terms of the GNU General Public
> + * License version 2.  This program is licensed "as is" without any
> + * warranty of any kind, whether express or implied.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/err.h>
> +#include <linux/init.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/moduleparam.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/platform_device.h>
> +#include <linux/types.h>
> +#include <linux/watchdog.h>
> +
> +#include <asm/system_misc.h>
> +
> +#define REG_COUNT			0x4
> +#define REG_MODE			0x8
> +#define REG_ENABLE			0xC
> +
> +struct moxart_wdt_dev {
> +	struct watchdog_device wdt_dev;
> +	void __iomem *wdt_base;
> +};
> +
> +static struct moxart_wdt_dev *moxart_wdt;
> +static bool nowayout = WATCHDOG_NOWAYOUT;
> +static int heartbeat;
> +static unsigned int clock_frequency;
> +static unsigned int max_timeout;
> +
> +static void moxart_wdt_restart(enum reboot_mode reboot_mode, const char *cmd)
> +{
> +	writel(1, moxart_wdt->wdt_base + REG_COUNT);
> +	writel(0x5ab9, moxart_wdt->wdt_base + REG_MODE);
> +	writel(0x03, moxart_wdt->wdt_base + REG_ENABLE);
> +}
> +
> +static int moxart_wdt_stop(struct watchdog_device *wdt_dev)
> +{
> +	struct moxart_wdt_dev *moxart_wdt = watchdog_get_drvdata(wdt_dev);
> +	void __iomem *wdt_base = moxart_wdt->wdt_base;
> +
> +	writel(0, wdt_base + REG_ENABLE);
> +
> +	return 0;
> +}
> +
> +static int moxart_wdt_start(struct watchdog_device *wdt_dev)
> +{
> +	struct moxart_wdt_dev *moxart_wdt = watchdog_get_drvdata(wdt_dev);
> +	void __iomem *wdt_base = moxart_wdt->wdt_base;
> +
> +	writel(clock_frequency * moxart_wdt->wdt_dev.timeout,
> +	       wdt_base + REG_COUNT);
> +	writel(0x5ab9, wdt_base + REG_MODE);
> +	writel(0x03, wdt_base + REG_ENABLE);
> +
> +	return 0;
> +}
> +
> +static int moxart_wdt_ping(struct watchdog_device *wdt_dev)
> +{
> +	moxart_wdt_start(wdt_dev);
> +	return 0;
> +}

This function is unnecessary. If it is not there, the infrastructure code will
call the start function automatically.

> +
> +static int moxart_wdt_set_timeout(struct watchdog_device *wdt_dev,
> +				  unsigned int timeout)
> +{
> +	dev_dbg(wdt_dev->dev, "%s: timeout=%u\n", __func__, timeout);
> +
> +	if (timeout > max_timeout)
> +		return -EINVAL;

Unnecessary check; handled by infrastructure.

> +
> +	moxart_wdt->wdt_dev.timeout = timeout;
> +
> +	return 0;
> +}
> +
> +static const struct watchdog_info moxart_wdt_info = {
> +	.identity       = "moxart-wdt",
> +	.options        = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING |
> +			  WDIOF_MAGICCLOSE,
> +};
> +
> +static const struct watchdog_ops moxart_wdt_ops = {
> +	.owner          = THIS_MODULE,
> +	.start          = moxart_wdt_start,
> +	.stop           = moxart_wdt_stop,
> +	.ping           = moxart_wdt_ping,
> +	.set_timeout    = moxart_wdt_set_timeout,
> +};
> +
> +static int moxart_wdt_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct device_node *node = dev->of_node;
> +	struct resource *res;
> +	struct clk *clk;
> +	int err;
> +
> +	moxart_wdt = devm_kzalloc(&pdev->dev, sizeof(*moxart_wdt), GFP_KERNEL);
> +	if (!moxart_wdt)
> +		return -EINVAL;
> +
> +	platform_set_drvdata(pdev, moxart_wdt);
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	moxart_wdt->wdt_base = devm_ioremap_resource(dev, res);
> +	if (IS_ERR(moxart_wdt->wdt_base))
> +		return PTR_ERR(moxart_wdt->wdt_base);
> +
> +	arm_pm_restart = moxart_wdt_restart;

It is quite unusual that system restart code is implemented in a watchdog
driver (which may not even be compiled into an image, or not be loaded).

Also, I'd like to see what happens if the driver is built as module and
unloaded before the restart. Unless I am missing something, this should result
in a nice crash.

> +
> +	clk = of_clk_get(node, 0);
> +	if (IS_ERR(clk)) {
> +		pr_err("%s: of_clk_get failed\n", __func__);
> +		return PTR_ERR(clk);

.... even more so if it fails to load for any reason.

> +	}
> +
> +	clock_frequency = clk_get_rate(clk);
> +
> +	max_timeout = UINT_MAX / clock_frequency;
> +
> +	moxart_wdt->wdt_dev.info = &moxart_wdt_info;
> +	moxart_wdt->wdt_dev.ops = &moxart_wdt_ops;
> +	moxart_wdt->wdt_dev.timeout = max_timeout;
> +	moxart_wdt->wdt_dev.min_timeout = 1;
> +	moxart_wdt->wdt_dev.max_timeout = max_timeout;
> +	moxart_wdt->wdt_dev.parent = &pdev->dev;
> +
> +	watchdog_init_timeout(&moxart_wdt->wdt_dev, heartbeat, &pdev->dev);
> +	watchdog_set_nowayout(&moxart_wdt->wdt_dev, nowayout);
> +
> +	watchdog_set_drvdata(&moxart_wdt->wdt_dev, moxart_wdt);
> +
> +	err = watchdog_register_device(&moxart_wdt->wdt_dev);
> +	if (unlikely(err))
> +		return err;
> +
> +	dev_dbg(dev, "Watchdog enabled (heartbeat=%d sec, nowayout=%d)\n",
> +		 moxart_wdt->wdt_dev.timeout, nowayout);
> +
> +	return 0;
> +}
> +
> +static int moxart_wdt_remove(struct platform_device *pdev)
> +{
> +	struct moxart_wdt_dev *moxart_wdt = platform_get_drvdata(pdev);
> +
> +	watchdog_unregister_device(&moxart_wdt->wdt_dev);
> +	watchdog_set_drvdata(&moxart_wdt->wdt_dev, NULL);

This is unnecessary.

> +
> +	return 0;
> +}
> +
> +static const struct of_device_id moxart_watchdog_match[] = {
> +	{ .compatible = "moxa,moxart-watchdog" },
> +	{ },
> +};
> +
> +static struct platform_driver moxart_wdt_driver = {
> +	.probe      = moxart_wdt_probe,
> +	.remove     = moxart_wdt_remove,
> +	.driver     = {
> +		.name		= "moxart-watchdog",
> +		.owner		= THIS_MODULE,
> +		.of_match_table	= moxart_watchdog_match,
> +	},
> +};
> +module_platform_driver(moxart_wdt_driver);
> +
> +module_param(heartbeat, int, 0);
> +MODULE_PARM_DESC(heartbeat, "Watchdog heartbeat in seconds");
> +
> +MODULE_DESCRIPTION("MOXART watchdog driver");
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Jonas Jensen <jonas.jensen@gmail.com>");
> -- 
> 1.8.2.1
> 
> --
> 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 v2] watchdog: Add MOXA ART watchdog driver

[Jonas Jensen]

Add watchdog driver for MOXA ART SoCs.

Signed-off-by: Jonas Jensen <jonas.jensen@gmail.com>
---

Notes:
    Thanks for the review Guenter.
    
    Changes since v1:
    
    1. remove moxart_wdt_ping
    2. remove unnecessary max timeout check in moxart_wdt_set_timeout
    3. remove arm_pm_restart and moxart_wdt_restart function
       (moved to moxart platform startup code)
    4. remove unnecessary watchdog_set_drvdata from moxart_wdt_remove
    5. stop the watchdog on driver remove
    
    Applies to next-20130716

 drivers/watchdog/Kconfig      |  10 +++
 drivers/watchdog/Makefile     |   1 +
 drivers/watchdog/moxart_wdt.c | 171 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 182 insertions(+)
 create mode 100644 drivers/watchdog/moxart_wdt.c

diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 362085d..6883aab 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -382,6 +382,16 @@ config RETU_WATCHDOG
 	  To compile this driver as a module, choose M here: the
 	  module will be called retu_wdt.
 
+config MOXART_WDT
+	tristate "MOXART watchdog"
+	depends on ARCH_MOXART
+	help
+	  Say Y here to include Watchdog timer support for the watchdog
+	  existing on the MOXA ART SoC series platforms.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called moxart_wdt.
+
 # AVR32 Architecture
 
 config AT32AP700X_WDT
diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
index 2f26a0b..7cab53a 100644
--- a/drivers/watchdog/Makefile
+++ b/drivers/watchdog/Makefile
@@ -54,6 +54,7 @@ obj-$(CONFIG_IMX2_WDT) += imx2_wdt.o
 obj-$(CONFIG_UX500_WATCHDOG) += ux500_wdt.o
 obj-$(CONFIG_RETU_WATCHDOG) += retu_wdt.o
 obj-$(CONFIG_BCM2835_WDT) += bcm2835_wdt.o
+obj-$(CONFIG_MOXART_WDT) += moxart_wdt.o
 
 # AVR32 Architecture
 obj-$(CONFIG_AT32AP700X_WDT) += at32ap700x_wdt.o
diff --git a/drivers/watchdog/moxart_wdt.c b/drivers/watchdog/moxart_wdt.c
new file mode 100644
index 0000000..966855d
--- /dev/null
+++ b/drivers/watchdog/moxart_wdt.c
@@ -0,0 +1,171 @@
+/*
+ * MOXA ART SoCs watchdog driver.
+ *
+ * Copyright (C) 2013 Jonas Jensen
+ *
+ * Jonas Jensen <jonas.jensen@gmail.com>
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2.  This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+
+#include <linux/clk.h>
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/moduleparam.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/platform_device.h>
+#include <linux/types.h>
+#include <linux/watchdog.h>
+
+#include <asm/system_misc.h>
+
+#define REG_COUNT			0x4
+#define REG_MODE			0x8
+#define REG_ENABLE			0xC
+
+struct moxart_wdt_dev {
+	struct watchdog_device wdt_dev;
+	void __iomem *wdt_base;
+};
+
+static struct moxart_wdt_dev *moxart_wdt;
+static bool nowayout = WATCHDOG_NOWAYOUT;
+static int heartbeat;
+static unsigned int clock_frequency;
+static unsigned int max_timeout;
+
+static int moxart_wdt_stop(struct watchdog_device *wdt_dev)
+{
+	struct moxart_wdt_dev *moxart_wdt = watchdog_get_drvdata(wdt_dev);
+	void __iomem *wdt_base = moxart_wdt->wdt_base;
+
+	writel(0, wdt_base + REG_ENABLE);
+
+	return 0;
+}
+
+static int moxart_wdt_start(struct watchdog_device *wdt_dev)
+{
+	struct moxart_wdt_dev *moxart_wdt = watchdog_get_drvdata(wdt_dev);
+	void __iomem *wdt_base = moxart_wdt->wdt_base;
+
+	writel(clock_frequency * moxart_wdt->wdt_dev.timeout,
+	       wdt_base + REG_COUNT);
+	writel(0x5ab9, wdt_base + REG_MODE);
+	writel(0x03, wdt_base + REG_ENABLE);
+
+	return 0;
+}
+
+static int moxart_wdt_set_timeout(struct watchdog_device *wdt_dev,
+				  unsigned int timeout)
+{
+	dev_dbg(wdt_dev->dev, "%s: timeout=%u\n", __func__, timeout);
+
+	moxart_wdt->wdt_dev.timeout = timeout;
+
+	return 0;
+}
+
+static const struct watchdog_info moxart_wdt_info = {
+	.identity       = "moxart-wdt",
+	.options        = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING |
+			  WDIOF_MAGICCLOSE,
+};
+
+static const struct watchdog_ops moxart_wdt_ops = {
+	.owner          = THIS_MODULE,
+	.start          = moxart_wdt_start,
+	.stop           = moxart_wdt_stop,
+	.set_timeout    = moxart_wdt_set_timeout,
+};
+
+static int moxart_wdt_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *node = dev->of_node;
+	struct resource *res;
+	struct clk *clk;
+	int err;
+
+	moxart_wdt = devm_kzalloc(&pdev->dev, sizeof(*moxart_wdt), GFP_KERNEL);
+	if (!moxart_wdt)
+		return -EINVAL;
+
+	platform_set_drvdata(pdev, moxart_wdt);
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	moxart_wdt->wdt_base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(moxart_wdt->wdt_base))
+		return PTR_ERR(moxart_wdt->wdt_base);
+
+	clk = of_clk_get(node, 0);
+	if (IS_ERR(clk)) {
+		pr_err("%s: of_clk_get failed\n", __func__);
+		return PTR_ERR(clk);
+	}
+
+	clock_frequency = clk_get_rate(clk);
+
+	max_timeout = UINT_MAX / clock_frequency;
+
+	moxart_wdt->wdt_dev.info = &moxart_wdt_info;
+	moxart_wdt->wdt_dev.ops = &moxart_wdt_ops;
+	moxart_wdt->wdt_dev.timeout = max_timeout;
+	moxart_wdt->wdt_dev.min_timeout = 1;
+	moxart_wdt->wdt_dev.max_timeout = max_timeout;
+	moxart_wdt->wdt_dev.parent = &pdev->dev;
+
+	watchdog_init_timeout(&moxart_wdt->wdt_dev, heartbeat, &pdev->dev);
+	watchdog_set_nowayout(&moxart_wdt->wdt_dev, nowayout);
+
+	watchdog_set_drvdata(&moxart_wdt->wdt_dev, moxart_wdt);
+
+	err = watchdog_register_device(&moxart_wdt->wdt_dev);
+	if (unlikely(err))
+		return err;
+
+	dev_dbg(dev, "Watchdog enabled (heartbeat=%d sec, nowayout=%d)\n",
+		moxart_wdt->wdt_dev.timeout, nowayout);
+
+	return 0;
+}
+
+static int moxart_wdt_remove(struct platform_device *pdev)
+{
+	struct moxart_wdt_dev *moxart_wdt = platform_get_drvdata(pdev);
+
+	moxart_wdt_stop(&moxart_wdt->wdt_dev);
+	watchdog_unregister_device(&moxart_wdt->wdt_dev);
+
+	return 0;
+}
+
+static const struct of_device_id moxart_watchdog_match[] = {
+	{ .compatible = "moxa,moxart-watchdog" },
+	{ },
+};
+
+static struct platform_driver moxart_wdt_driver = {
+	.probe      = moxart_wdt_probe,
+	.remove     = moxart_wdt_remove,
+	.driver     = {
+		.name		= "moxart-watchdog",
+		.owner		= THIS_MODULE,
+		.of_match_table	= moxart_watchdog_match,
+	},
+};
+module_platform_driver(moxart_wdt_driver);
+
+module_param(heartbeat, int, 0);
+MODULE_PARM_DESC(heartbeat, "Watchdog heartbeat in seconds");
+
+MODULE_DESCRIPTION("MOXART watchdog driver");
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Jonas Jensen <jonas.jensen@gmail.com>");
-- 
1.8.2.1

[PATCH v3] watchdog: Add MOXA ART watchdog driver

[Jonas Jensen]

Add watchdog driver for MOXA ART SoCs.

Signed-off-by: Jonas Jensen <jonas.jensen@gmail.com>
---

Notes:
    Changes since v2:
    
    1. add devicetree bindings document
    
    Applies to next-20130716

 .../bindings/watchdog/moxa,moxart-watchdog.txt     |  15 ++
 drivers/watchdog/Kconfig                           |  10 ++
 drivers/watchdog/Makefile                          |   1 +
 drivers/watchdog/moxart_wdt.c                      | 171 +++++++++++++++++++++
 4 files changed, 197 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/watchdog/moxa,moxart-watchdog.txt
 create mode 100644 drivers/watchdog/moxart_wdt.c

diff --git a/Documentation/devicetree/bindings/watchdog/moxa,moxart-watchdog.txt b/Documentation/devicetree/bindings/watchdog/moxa,moxart-watchdog.txt
new file mode 100644
index 0000000..fb99e50
--- /dev/null
+++ b/Documentation/devicetree/bindings/watchdog/moxa,moxart-watchdog.txt
@@ -0,0 +1,15 @@
+MOXA ART Watchdog timer
+
+Required properties:
+
+- compatible : Should be "moxa,moxart-watchdog"
+- reg : Should contain registers location and length
+- clocks : Should contain phandle for APB clock "clkapb"
+
+Example:
+
+	watchdog: watchdog@98500000 {
+		compatible = "moxa,moxart-watchdog";
+		reg = <0x98500000 0x10>;
+		clocks = <&clkapb>;
+	};
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 362085d..6883aab 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -382,6 +382,16 @@ config RETU_WATCHDOG
 	  To compile this driver as a module, choose M here: the
 	  module will be called retu_wdt.
 
+config MOXART_WDT
+	tristate "MOXART watchdog"
+	depends on ARCH_MOXART
+	help
+	  Say Y here to include Watchdog timer support for the watchdog
+	  existing on the MOXA ART SoC series platforms.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called moxart_wdt.
+
 # AVR32 Architecture
 
 config AT32AP700X_WDT
diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
index 2f26a0b..7cab53a 100644
--- a/drivers/watchdog/Makefile
+++ b/drivers/watchdog/Makefile
@@ -54,6 +54,7 @@ obj-$(CONFIG_IMX2_WDT) += imx2_wdt.o
 obj-$(CONFIG_UX500_WATCHDOG) += ux500_wdt.o
 obj-$(CONFIG_RETU_WATCHDOG) += retu_wdt.o
 obj-$(CONFIG_BCM2835_WDT) += bcm2835_wdt.o
+obj-$(CONFIG_MOXART_WDT) += moxart_wdt.o
 
 # AVR32 Architecture
 obj-$(CONFIG_AT32AP700X_WDT) += at32ap700x_wdt.o
diff --git a/drivers/watchdog/moxart_wdt.c b/drivers/watchdog/moxart_wdt.c
new file mode 100644
index 0000000..966855d
--- /dev/null
+++ b/drivers/watchdog/moxart_wdt.c
@@ -0,0 +1,171 @@
+/*
+ * MOXA ART SoCs watchdog driver.
+ *
+ * Copyright (C) 2013 Jonas Jensen
+ *
+ * Jonas Jensen <jonas.jensen@gmail.com>
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2.  This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+
+#include <linux/clk.h>
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/moduleparam.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/platform_device.h>
+#include <linux/types.h>
+#include <linux/watchdog.h>
+
+#include <asm/system_misc.h>
+
+#define REG_COUNT			0x4
+#define REG_MODE			0x8
+#define REG_ENABLE			0xC
+
+struct moxart_wdt_dev {
+	struct watchdog_device wdt_dev;
+	void __iomem *wdt_base;
+};
+
+static struct moxart_wdt_dev *moxart_wdt;
+static bool nowayout = WATCHDOG_NOWAYOUT;
+static int heartbeat;
+static unsigned int clock_frequency;
+static unsigned int max_timeout;
+
+static int moxart_wdt_stop(struct watchdog_device *wdt_dev)
+{
+	struct moxart_wdt_dev *moxart_wdt = watchdog_get_drvdata(wdt_dev);
+	void __iomem *wdt_base = moxart_wdt->wdt_base;
+
+	writel(0, wdt_base + REG_ENABLE);
+
+	return 0;
+}
+
+static int moxart_wdt_start(struct watchdog_device *wdt_dev)
+{
+	struct moxart_wdt_dev *moxart_wdt = watchdog_get_drvdata(wdt_dev);
+	void __iomem *wdt_base = moxart_wdt->wdt_base;
+
+	writel(clock_frequency * moxart_wdt->wdt_dev.timeout,
+	       wdt_base + REG_COUNT);
+	writel(0x5ab9, wdt_base + REG_MODE);
+	writel(0x03, wdt_base + REG_ENABLE);
+
+	return 0;
+}
+
+static int moxart_wdt_set_timeout(struct watchdog_device *wdt_dev,
+				  unsigned int timeout)
+{
+	dev_dbg(wdt_dev->dev, "%s: timeout=%u\n", __func__, timeout);
+
+	moxart_wdt->wdt_dev.timeout = timeout;
+
+	return 0;
+}
+
+static const struct watchdog_info moxart_wdt_info = {
+	.identity       = "moxart-wdt",
+	.options        = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING |
+			  WDIOF_MAGICCLOSE,
+};
+
+static const struct watchdog_ops moxart_wdt_ops = {
+	.owner          = THIS_MODULE,
+	.start          = moxart_wdt_start,
+	.stop           = moxart_wdt_stop,
+	.set_timeout    = moxart_wdt_set_timeout,
+};
+
+static int moxart_wdt_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *node = dev->of_node;
+	struct resource *res;
+	struct clk *clk;
+	int err;
+
+	moxart_wdt = devm_kzalloc(&pdev->dev, sizeof(*moxart_wdt), GFP_KERNEL);
+	if (!moxart_wdt)
+		return -EINVAL;
+
+	platform_set_drvdata(pdev, moxart_wdt);
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	moxart_wdt->wdt_base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(moxart_wdt->wdt_base))
+		return PTR_ERR(moxart_wdt->wdt_base);
+
+	clk = of_clk_get(node, 0);
+	if (IS_ERR(clk)) {
+		pr_err("%s: of_clk_get failed\n", __func__);
+		return PTR_ERR(clk);
+	}
+
+	clock_frequency = clk_get_rate(clk);
+
+	max_timeout = UINT_MAX / clock_frequency;
+
+	moxart_wdt->wdt_dev.info = &moxart_wdt_info;
+	moxart_wdt->wdt_dev.ops = &moxart_wdt_ops;
+	moxart_wdt->wdt_dev.timeout = max_timeout;
+	moxart_wdt->wdt_dev.min_timeout = 1;
+	moxart_wdt->wdt_dev.max_timeout = max_timeout;
+	moxart_wdt->wdt_dev.parent = &pdev->dev;
+
+	watchdog_init_timeout(&moxart_wdt->wdt_dev, heartbeat, &pdev->dev);
+	watchdog_set_nowayout(&moxart_wdt->wdt_dev, nowayout);
+
+	watchdog_set_drvdata(&moxart_wdt->wdt_dev, moxart_wdt);
+
+	err = watchdog_register_device(&moxart_wdt->wdt_dev);
+	if (unlikely(err))
+		return err;
+
+	dev_dbg(dev, "Watchdog enabled (heartbeat=%d sec, nowayout=%d)\n",
+		moxart_wdt->wdt_dev.timeout, nowayout);
+
+	return 0;
+}
+
+static int moxart_wdt_remove(struct platform_device *pdev)
+{
+	struct moxart_wdt_dev *moxart_wdt = platform_get_drvdata(pdev);
+
+	moxart_wdt_stop(&moxart_wdt->wdt_dev);
+	watchdog_unregister_device(&moxart_wdt->wdt_dev);
+
+	return 0;
+}
+
+static const struct of_device_id moxart_watchdog_match[] = {
+	{ .compatible = "moxa,moxart-watchdog" },
+	{ },
+};
+
+static struct platform_driver moxart_wdt_driver = {
+	.probe      = moxart_wdt_probe,
+	.remove     = moxart_wdt_remove,
+	.driver     = {
+		.name		= "moxart-watchdog",
+		.owner		= THIS_MODULE,
+		.of_match_table	= moxart_watchdog_match,
+	},
+};
+module_platform_driver(moxart_wdt_driver);
+
+module_param(heartbeat, int, 0);
+MODULE_PARM_DESC(heartbeat, "Watchdog heartbeat in seconds");
+
+MODULE_DESCRIPTION("MOXART watchdog driver");
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Jonas Jensen <jonas.jensen@gmail.com>");
-- 
1.8.2.1

Re: [PATCH v3] watchdog: Add MOXA ART watchdog driver

[Guenter Roeck]

On Wed, Jul 17, 2013 at 11:24:51AM +0200, Jonas Jensen wrote:
> Add watchdog driver for MOXA ART SoCs.
> 
> Signed-off-by: Jonas Jensen <jonas.jensen@gmail.com>
> ---
> 
> Notes:
>     Changes since v2:
>     
>     1. add devicetree bindings document
>     
>     Applies to next-20130716
> 
>  .../bindings/watchdog/moxa,moxart-watchdog.txt     |  15 ++
>  drivers/watchdog/Kconfig                           |  10 ++
>  drivers/watchdog/Makefile                          |   1 +
>  drivers/watchdog/moxart_wdt.c                      | 171 +++++++++++++++++++++
>  4 files changed, 197 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/watchdog/moxa,moxart-watchdog.txt
>  create mode 100644 drivers/watchdog/moxart_wdt.c
> 
> diff --git a/Documentation/devicetree/bindings/watchdog/moxa,moxart-watchdog.txt b/Documentation/devicetree/bindings/watchdog/moxa,moxart-watchdog.txt
> new file mode 100644
> index 0000000..fb99e50
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/watchdog/moxa,moxart-watchdog.txt
> @@ -0,0 +1,15 @@
> +MOXA ART Watchdog timer
> +
> +Required properties:
> +
> +- compatible : Should be "moxa,moxart-watchdog"
> +- reg : Should contain registers location and length
> +- clocks : Should contain phandle for APB clock "clkapb"
> +
> +Example:
> +
> +	watchdog: watchdog@98500000 {
> +		compatible = "moxa,moxart-watchdog";
> +		reg = <0x98500000 0x10>;
> +		clocks = <&clkapb>;
> +	};
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index 362085d..6883aab 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -382,6 +382,16 @@ config RETU_WATCHDOG
>  	  To compile this driver as a module, choose M here: the
>  	  module will be called retu_wdt.
>  
> +config MOXART_WDT
> +	tristate "MOXART watchdog"
> +	depends on ARCH_MOXART
> +	help
> +	  Say Y here to include Watchdog timer support for the watchdog
> +	  existing on the MOXA ART SoC series platforms.
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called moxart_wdt.
> +
>  # AVR32 Architecture
>  
>  config AT32AP700X_WDT
> diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
> index 2f26a0b..7cab53a 100644
> --- a/drivers/watchdog/Makefile
> +++ b/drivers/watchdog/Makefile
> @@ -54,6 +54,7 @@ obj-$(CONFIG_IMX2_WDT) += imx2_wdt.o
>  obj-$(CONFIG_UX500_WATCHDOG) += ux500_wdt.o
>  obj-$(CONFIG_RETU_WATCHDOG) += retu_wdt.o
>  obj-$(CONFIG_BCM2835_WDT) += bcm2835_wdt.o
> +obj-$(CONFIG_MOXART_WDT) += moxart_wdt.o
>  
>  # AVR32 Architecture
>  obj-$(CONFIG_AT32AP700X_WDT) += at32ap700x_wdt.o
> diff --git a/drivers/watchdog/moxart_wdt.c b/drivers/watchdog/moxart_wdt.c
> new file mode 100644
> index 0000000..966855d
> --- /dev/null
> +++ b/drivers/watchdog/moxart_wdt.c
> @@ -0,0 +1,171 @@
> +/*
> + * MOXA ART SoCs watchdog driver.
> + *
> + * Copyright (C) 2013 Jonas Jensen
> + *
> + * Jonas Jensen <jonas.jensen@gmail.com>
> + *
> + * This file is licensed under the terms of the GNU General Public
> + * License version 2.  This program is licensed "as is" without any
> + * warranty of any kind, whether express or implied.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/err.h>
> +#include <linux/init.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/moduleparam.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>

Do you need the above two includes ? 

> +#include <linux/platform_device.h>
> +#include <linux/types.h>
> +#include <linux/watchdog.h>
> +
> +#include <asm/system_misc.h>
> +
> +#define REG_COUNT			0x4
> +#define REG_MODE			0x8
> +#define REG_ENABLE			0xC
> +
> +struct moxart_wdt_dev {
> +	struct watchdog_device wdt_dev;
> +	void __iomem *wdt_base;
> +};
> +
> +static struct moxart_wdt_dev *moxart_wdt;

I don't think this variable is really needed as static variable;
you should be able to use platform_get_drvdata() to get it whereever needed.
In fact, you sometimes use the static variable and sometimes a local variable
with the same name. I find this confusing.

> +static bool nowayout = WATCHDOG_NOWAYOUT;

Only used in probe function and should be declared there.

> +static int heartbeat;
> +static unsigned int clock_frequency;

Curious - why did you declare this variable as static variable, but wdt_base is
part of moxart_wdt_dev.

> +static unsigned int max_timeout;

This variable is only used in the probe function and should thus be declared
there.

> +static int moxart_wdt_stop(struct watchdog_device *wdt_dev)
> +{
> +	struct moxart_wdt_dev *moxart_wdt = watchdog_get_drvdata(wdt_dev);
> +	void __iomem *wdt_base = moxart_wdt->wdt_base;
> +
> +	writel(0, wdt_base + REG_ENABLE);
> +
> +	return 0;
> +}
> +
> +static int moxart_wdt_start(struct watchdog_device *wdt_dev)
> +{
> +	struct moxart_wdt_dev *moxart_wdt = watchdog_get_drvdata(wdt_dev);
> +	void __iomem *wdt_base = moxart_wdt->wdt_base;
> +
> +	writel(clock_frequency * moxart_wdt->wdt_dev.timeout,
> +	       wdt_base + REG_COUNT);
> +	writel(0x5ab9, wdt_base + REG_MODE);
> +	writel(0x03, wdt_base + REG_ENABLE);
> +
> +	return 0;
> +}
> +
> +static int moxart_wdt_set_timeout(struct watchdog_device *wdt_dev,
> +				  unsigned int timeout)
> +{
> +	dev_dbg(wdt_dev->dev, "%s: timeout=%u\n", __func__, timeout);
> +
> +	moxart_wdt->wdt_dev.timeout = timeout;
> +
> +	return 0;
> +}
> +
> +static const struct watchdog_info moxart_wdt_info = {
> +	.identity       = "moxart-wdt",
> +	.options        = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING |
> +			  WDIOF_MAGICCLOSE,
> +};
> +
> +static const struct watchdog_ops moxart_wdt_ops = {
> +	.owner          = THIS_MODULE,
> +	.start          = moxart_wdt_start,
> +	.stop           = moxart_wdt_stop,
> +	.set_timeout    = moxart_wdt_set_timeout,
> +};
> +
> +static int moxart_wdt_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct device_node *node = dev->of_node;
> +	struct resource *res;
> +	struct clk *clk;
> +	int err;
> +
> +	moxart_wdt = devm_kzalloc(&pdev->dev, sizeof(*moxart_wdt), GFP_KERNEL);
> +	if (!moxart_wdt)
> +		return -EINVAL;
> +
> +	platform_set_drvdata(pdev, moxart_wdt);
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	moxart_wdt->wdt_base = devm_ioremap_resource(dev, res);
> +	if (IS_ERR(moxart_wdt->wdt_base))
> +		return PTR_ERR(moxart_wdt->wdt_base);
> +
> +	clk = of_clk_get(node, 0);
> +	if (IS_ERR(clk)) {
> +		pr_err("%s: of_clk_get failed\n", __func__);
> +		return PTR_ERR(clk);
> +	}
> +
> +	clock_frequency = clk_get_rate(clk);

Other callers of this function check the return value. Maybe the error can not
happen here, but I am a bit concerned that the function may return 0, in which
case you would have a problem with division by zero below. Is it guaranteed
that this can never happen ?

> +
> +	max_timeout = UINT_MAX / clock_frequency;
> +
> +	moxart_wdt->wdt_dev.info = &moxart_wdt_info;
> +	moxart_wdt->wdt_dev.ops = &moxart_wdt_ops;
> +	moxart_wdt->wdt_dev.timeout = max_timeout;
> +	moxart_wdt->wdt_dev.min_timeout = 1;
> +	moxart_wdt->wdt_dev.max_timeout = max_timeout;
> +	moxart_wdt->wdt_dev.parent = &pdev->dev;
> +
> +	watchdog_init_timeout(&moxart_wdt->wdt_dev, heartbeat, &pdev->dev);
> +	watchdog_set_nowayout(&moxart_wdt->wdt_dev, nowayout);
> +
> +	watchdog_set_drvdata(&moxart_wdt->wdt_dev, moxart_wdt);
> +
> +	err = watchdog_register_device(&moxart_wdt->wdt_dev);
> +	if (unlikely(err))
> +		return err;
> +
> +	dev_dbg(dev, "Watchdog enabled (heartbeat=%d sec, nowayout=%d)\n",
> +		moxart_wdt->wdt_dev.timeout, nowayout);
> +
> +	return 0;
> +}
> +
> +static int moxart_wdt_remove(struct platform_device *pdev)
> +{
> +	struct moxart_wdt_dev *moxart_wdt = platform_get_drvdata(pdev);
> +
> +	moxart_wdt_stop(&moxart_wdt->wdt_dev);
> +	watchdog_unregister_device(&moxart_wdt->wdt_dev);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id moxart_watchdog_match[] = {
> +	{ .compatible = "moxa,moxart-watchdog" },
> +	{ },
> +};
> +
> +static struct platform_driver moxart_wdt_driver = {
> +	.probe      = moxart_wdt_probe,
> +	.remove     = moxart_wdt_remove,
> +	.driver     = {
> +		.name		= "moxart-watchdog",
> +		.owner		= THIS_MODULE,
> +		.of_match_table	= moxart_watchdog_match,
> +	},
> +};
> +module_platform_driver(moxart_wdt_driver);
> +
> +module_param(heartbeat, int, 0);
> +MODULE_PARM_DESC(heartbeat, "Watchdog heartbeat in seconds");
> +
> +MODULE_DESCRIPTION("MOXART watchdog driver");
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Jonas Jensen <jonas.jensen@gmail.com>");
> -- 
> 1.8.2.1
> 
> 

[PATCH v4] watchdog: Add MOXA ART watchdog driver

[Jonas Jensen]

Add watchdog driver for MOXA ART SoCs.

Signed-off-by: Jonas Jensen <jonas.jensen@gmail.com>
---

Notes:
    v4 should fix remaining issues pointed out by Guenter.
    
    Changes since v3:
    
    1. remove unnecessary header includes:
       linux/of.h
       linux/of_address.h
       linux/err.h
       linux/init.h
       linux/kernel.h,
       linux/moduleparam.h
       linux/types.h
       asm/system_misc.h
    2. move clock_frequency to moxart_wdt_dev
    3. remove local pointer "void __iomem *wdt_base" use moxart_wdt->base instead
    4. get clock_frequency from watchdog_get_drvdata
    5. move max_timeout variable to moxart_wdt_probe
    6. check for division by zero moxart_wdt->clock_frequency
    7. if devm_kzalloc fails, return -ENOMEM instead of -EINVAL
    
    Applies to next-20130716

 .../bindings/watchdog/moxa,moxart-watchdog.txt     |  15 ++
 drivers/watchdog/Kconfig                           |  10 ++
 drivers/watchdog/Makefile                          |   1 +
 drivers/watchdog/moxart_wdt.c                      | 166 +++++++++++++++++++++
 4 files changed, 192 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/watchdog/moxa,moxart-watchdog.txt
 create mode 100644 drivers/watchdog/moxart_wdt.c

diff --git a/Documentation/devicetree/bindings/watchdog/moxa,moxart-watchdog.txt b/Documentation/devicetree/bindings/watchdog/moxa,moxart-watchdog.txt
new file mode 100644
index 0000000..5507f2b
--- /dev/null
+++ b/Documentation/devicetree/bindings/watchdog/moxa,moxart-watchdog.txt
@@ -0,0 +1,15 @@
+MOXA ART Watchdog timer
+
+Required properties:
+
+- compatible : Should be "moxa,moxart-watchdog"
+- reg : Should contain registers location and length
+- clocks : Should contain phandle for the MOXA ART core clock "coreclk"
+
+Example:
+
+	watchdog: watchdog@98500000 {
+		compatible = "moxa,moxart-watchdog";
+		reg = <0x98500000 0x10>;
+		clocks = <&coreclk>;
+	};
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 362085d..6883aab 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -382,6 +382,16 @@ config RETU_WATCHDOG
 	  To compile this driver as a module, choose M here: the
 	  module will be called retu_wdt.
 
+config MOXART_WDT
+	tristate "MOXART watchdog"
+	depends on ARCH_MOXART
+	help
+	  Say Y here to include Watchdog timer support for the watchdog
+	  existing on the MOXA ART SoC series platforms.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called moxart_wdt.
+
 # AVR32 Architecture
 
 config AT32AP700X_WDT
diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
index 2f26a0b..7cab53a 100644
--- a/drivers/watchdog/Makefile
+++ b/drivers/watchdog/Makefile
@@ -54,6 +54,7 @@ obj-$(CONFIG_IMX2_WDT) += imx2_wdt.o
 obj-$(CONFIG_UX500_WATCHDOG) += ux500_wdt.o
 obj-$(CONFIG_RETU_WATCHDOG) += retu_wdt.o
 obj-$(CONFIG_BCM2835_WDT) += bcm2835_wdt.o
+obj-$(CONFIG_MOXART_WDT) += moxart_wdt.o
 
 # AVR32 Architecture
 obj-$(CONFIG_AT32AP700X_WDT) += at32ap700x_wdt.o
diff --git a/drivers/watchdog/moxart_wdt.c b/drivers/watchdog/moxart_wdt.c
new file mode 100644
index 0000000..9a59c3d
--- /dev/null
+++ b/drivers/watchdog/moxart_wdt.c
@@ -0,0 +1,166 @@
+/*
+ * MOXA ART SoCs watchdog driver.
+ *
+ * Copyright (C) 2013 Jonas Jensen
+ *
+ * Jonas Jensen <jonas.jensen@gmail.com>
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2.  This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+
+#include <linux/clk.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/watchdog.h>
+
+#define REG_COUNT			0x4
+#define REG_MODE			0x8
+#define REG_ENABLE			0xC
+
+struct moxart_wdt_dev {
+	struct watchdog_device dev;
+	void __iomem *base;
+	unsigned int clock_frequency;
+};
+
+static int heartbeat;
+
+static int moxart_wdt_stop(struct watchdog_device *wdt_dev)
+{
+	struct moxart_wdt_dev *moxart_wdt = watchdog_get_drvdata(wdt_dev);
+
+	writel(0, moxart_wdt->base + REG_ENABLE);
+
+	return 0;
+}
+
+static int moxart_wdt_start(struct watchdog_device *wdt_dev)
+{
+	struct moxart_wdt_dev *moxart_wdt = watchdog_get_drvdata(wdt_dev);
+
+	writel(moxart_wdt->clock_frequency * moxart_wdt->dev.timeout,
+	       moxart_wdt->base + REG_COUNT);
+	writel(0x5ab9, moxart_wdt->base + REG_MODE);
+	writel(0x03, moxart_wdt->base + REG_ENABLE);
+
+	return 0;
+}
+
+static int moxart_wdt_set_timeout(struct watchdog_device *wdt_dev,
+				  unsigned int timeout)
+{
+	struct moxart_wdt_dev *moxart_wdt = watchdog_get_drvdata(wdt_dev);
+
+	dev_dbg(wdt_dev->dev, "%s: timeout=%u\n", __func__, timeout);
+
+	moxart_wdt->dev.timeout = timeout;
+
+	return 0;
+}
+
+static const struct watchdog_info moxart_wdt_info = {
+	.identity       = "moxart-wdt",
+	.options        = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING |
+			  WDIOF_MAGICCLOSE,
+};
+
+static const struct watchdog_ops moxart_wdt_ops = {
+	.owner          = THIS_MODULE,
+	.start          = moxart_wdt_start,
+	.stop           = moxart_wdt_stop,
+	.set_timeout    = moxart_wdt_set_timeout,
+};
+
+static int moxart_wdt_probe(struct platform_device *pdev)
+{
+	struct moxart_wdt_dev *moxart_wdt;
+	struct device *dev = &pdev->dev;
+	struct device_node *node = dev->of_node;
+	struct resource *res;
+	struct clk *clk;
+	int err;
+	unsigned int max_timeout;
+	bool nowayout = WATCHDOG_NOWAYOUT;
+
+	moxart_wdt = devm_kzalloc(&pdev->dev, sizeof(*moxart_wdt), GFP_KERNEL);
+	if (!moxart_wdt)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, moxart_wdt);
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	moxart_wdt->base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(moxart_wdt->base))
+		return PTR_ERR(moxart_wdt->base);
+
+	clk = of_clk_get(node, 0);
+	if (IS_ERR(clk)) {
+		pr_err("%s: of_clk_get failed\n", __func__);
+		return PTR_ERR(clk);
+	}
+
+	moxart_wdt->clock_frequency = clk_get_rate(clk);
+	if (moxart_wdt->clock_frequency == 0) {
+		pr_err("%s: incorrect clock frequency\n", __func__);
+		return -EINVAL;
+	}
+
+	max_timeout = UINT_MAX / moxart_wdt->clock_frequency;
+
+	moxart_wdt->dev.info = &moxart_wdt_info;
+	moxart_wdt->dev.ops = &moxart_wdt_ops;
+	moxart_wdt->dev.timeout = max_timeout;
+	moxart_wdt->dev.min_timeout = 1;
+	moxart_wdt->dev.max_timeout = max_timeout;
+	moxart_wdt->dev.parent = &pdev->dev;
+
+	watchdog_init_timeout(&moxart_wdt->dev, heartbeat, &pdev->dev);
+	watchdog_set_nowayout(&moxart_wdt->dev, nowayout);
+
+	watchdog_set_drvdata(&moxart_wdt->dev, moxart_wdt);
+
+	err = watchdog_register_device(&moxart_wdt->dev);
+	if (unlikely(err))
+		return err;
+
+	dev_dbg(dev, "Watchdog enabled (heartbeat=%d sec, nowayout=%d)\n",
+		moxart_wdt->dev.timeout, nowayout);
+
+	return 0;
+}
+
+static int moxart_wdt_remove(struct platform_device *pdev)
+{
+	struct moxart_wdt_dev *moxart_wdt = platform_get_drvdata(pdev);
+
+	moxart_wdt_stop(&moxart_wdt->dev);
+	watchdog_unregister_device(&moxart_wdt->dev);
+
+	return 0;
+}
+
+static const struct of_device_id moxart_watchdog_match[] = {
+	{ .compatible = "moxa,moxart-watchdog" },
+	{ },
+};
+
+static struct platform_driver moxart_wdt_driver = {
+	.probe      = moxart_wdt_probe,
+	.remove     = moxart_wdt_remove,
+	.driver     = {
+		.name		= "moxart-watchdog",
+		.owner		= THIS_MODULE,
+		.of_match_table	= moxart_watchdog_match,
+	},
+};
+module_platform_driver(moxart_wdt_driver);
+
+module_param(heartbeat, int, 0);
+MODULE_PARM_DESC(heartbeat, "Watchdog heartbeat in seconds");
+
+MODULE_DESCRIPTION("MOXART watchdog driver");
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Jonas Jensen <jonas.jensen@gmail.com>");
-- 
1.8.2.1

Re: [PATCH v4] watchdog: Add MOXA ART watchdog driver

[Arnd Bergmann]

On Friday 19 July 2013, Jonas Jensen wrote:
> diff --git a/Documentation/devicetree/bindings/watchdog/moxa,moxart-watchdog.txt b/Documentation/devicetree/bindings/watchdog/moxa,moxart-watchdog.txt
> new file mode 100644
> index 0000000..5507f2b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/watchdog/moxa,moxart-watchdog.txt
> @@ -0,0 +1,15 @@
> +MOXA ART Watchdog timer
> +
> +Required properties:
> +
> +- compatible : Should be "moxa,moxart-watchdog"
> +- reg : Should contain registers location and length
> +- clocks : Should contain phandle for the MOXA ART core clock "coreclk"
> +
> +Example:
> +
> +       watchdog: watchdog@98500000 {
> +               compatible = "moxa,moxart-watchdog";
> +               reg = <0x98500000 0x10>;
> +               clocks = <&coreclk>;
> +       };

I think the property descriptions need to be clarified here:

* I think "should" makes no sense for the "compatible" property since it is
  required here, you probably mean "must", see http://www.ietf.org/rfc/rfc2119.txt. 

* for the clock, make sure that the description is from the point of view of
  the device you are describing, not from the perspective of the clock controller.
  The device should not make any assumptions about who provides a clock, only
  what it is used for, and it sounds like "coreclk" is an SoC-wide identifier,
  not the name of the clock input of the watchdog device. This is not as important
  as you don't define a "clock-names" property here, butsomething to keep in
  mind anyway.

	Arnd

Re: [PATCH v4] watchdog: Add MOXA ART watchdog driver

[Guenter Roeck]

On Fri, Jul 19, 2013 at 12:23:22PM +0200, Jonas Jensen wrote:
> Add watchdog driver for MOXA ART SoCs.
> 
> Signed-off-by: Jonas Jensen <jonas.jensen@gmail.com>
> ---
> 
> Notes:
>     v4 should fix remaining issues pointed out by Guenter.
>     
>     Changes since v3:
>     
>     1. remove unnecessary header includes:
>        linux/of.h
>        linux/of_address.h
>        linux/err.h
>        linux/init.h
>        linux/kernel.h,
>        linux/moduleparam.h
>        linux/types.h
>        asm/system_misc.h

Seems you removed too many, but I'll need to have a closer look.
General guideline: If a definition from an include file is used, it must be
included. You can not depend on an include file being included from another
include file.

Thanks,
Guenter

>     2. move clock_frequency to moxart_wdt_dev
>     3. remove local pointer "void __iomem *wdt_base" use moxart_wdt->base instead
>     4. get clock_frequency from watchdog_get_drvdata
>     5. move max_timeout variable to moxart_wdt_probe
>     6. check for division by zero moxart_wdt->clock_frequency
>     7. if devm_kzalloc fails, return -ENOMEM instead of -EINVAL
>     
>     Applies to next-20130716
> 
>  .../bindings/watchdog/moxa,moxart-watchdog.txt     |  15 ++
>  drivers/watchdog/Kconfig                           |  10 ++
>  drivers/watchdog/Makefile                          |   1 +
>  drivers/watchdog/moxart_wdt.c                      | 166 +++++++++++++++++++++
>  4 files changed, 192 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/watchdog/moxa,moxart-watchdog.txt
>  create mode 100644 drivers/watchdog/moxart_wdt.c
> 
> diff --git a/Documentation/devicetree/bindings/watchdog/moxa,moxart-watchdog.txt b/Documentation/devicetree/bindings/watchdog/moxa,moxart-watchdog.txt
> new file mode 100644
> index 0000000..5507f2b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/watchdog/moxa,moxart-watchdog.txt
> @@ -0,0 +1,15 @@
> +MOXA ART Watchdog timer
> +
> +Required properties:
> +
> +- compatible : Should be "moxa,moxart-watchdog"
> +- reg : Should contain registers location and length
> +- clocks : Should contain phandle for the MOXA ART core clock "coreclk"
> +
> +Example:
> +
> +	watchdog: watchdog@98500000 {
> +		compatible = "moxa,moxart-watchdog";
> +		reg = <0x98500000 0x10>;
> +		clocks = <&coreclk>;
> +	};
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index 362085d..6883aab 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -382,6 +382,16 @@ config RETU_WATCHDOG
>  	  To compile this driver as a module, choose M here: the
>  	  module will be called retu_wdt.
>  
> +config MOXART_WDT
> +	tristate "MOXART watchdog"
> +	depends on ARCH_MOXART
> +	help
> +	  Say Y here to include Watchdog timer support for the watchdog
> +	  existing on the MOXA ART SoC series platforms.
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called moxart_wdt.
> +
>  # AVR32 Architecture
>  
>  config AT32AP700X_WDT
> diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
> index 2f26a0b..7cab53a 100644
> --- a/drivers/watchdog/Makefile
> +++ b/drivers/watchdog/Makefile
> @@ -54,6 +54,7 @@ obj-$(CONFIG_IMX2_WDT) += imx2_wdt.o
>  obj-$(CONFIG_UX500_WATCHDOG) += ux500_wdt.o
>  obj-$(CONFIG_RETU_WATCHDOG) += retu_wdt.o
>  obj-$(CONFIG_BCM2835_WDT) += bcm2835_wdt.o
> +obj-$(CONFIG_MOXART_WDT) += moxart_wdt.o
>  
>  # AVR32 Architecture
>  obj-$(CONFIG_AT32AP700X_WDT) += at32ap700x_wdt.o
> diff --git a/drivers/watchdog/moxart_wdt.c b/drivers/watchdog/moxart_wdt.c
> new file mode 100644
> index 0000000..9a59c3d
> --- /dev/null
> +++ b/drivers/watchdog/moxart_wdt.c
> @@ -0,0 +1,166 @@
> +/*
> + * MOXA ART SoCs watchdog driver.
> + *
> + * Copyright (C) 2013 Jonas Jensen
> + *
> + * Jonas Jensen <jonas.jensen@gmail.com>
> + *
> + * This file is licensed under the terms of the GNU General Public
> + * License version 2.  This program is licensed "as is" without any
> + * warranty of any kind, whether express or implied.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/watchdog.h>
> +
> +#define REG_COUNT			0x4
> +#define REG_MODE			0x8
> +#define REG_ENABLE			0xC
> +
> +struct moxart_wdt_dev {
> +	struct watchdog_device dev;
> +	void __iomem *base;
> +	unsigned int clock_frequency;
> +};
> +
> +static int heartbeat;
> +
> +static int moxart_wdt_stop(struct watchdog_device *wdt_dev)
> +{
> +	struct moxart_wdt_dev *moxart_wdt = watchdog_get_drvdata(wdt_dev);
> +
> +	writel(0, moxart_wdt->base + REG_ENABLE);
> +
> +	return 0;
> +}
> +
> +static int moxart_wdt_start(struct watchdog_device *wdt_dev)
> +{
> +	struct moxart_wdt_dev *moxart_wdt = watchdog_get_drvdata(wdt_dev);
> +
> +	writel(moxart_wdt->clock_frequency * moxart_wdt->dev.timeout,
> +	       moxart_wdt->base + REG_COUNT);
> +	writel(0x5ab9, moxart_wdt->base + REG_MODE);
> +	writel(0x03, moxart_wdt->base + REG_ENABLE);
> +
> +	return 0;
> +}
> +
> +static int moxart_wdt_set_timeout(struct watchdog_device *wdt_dev,
> +				  unsigned int timeout)
> +{
> +	struct moxart_wdt_dev *moxart_wdt = watchdog_get_drvdata(wdt_dev);
> +
> +	dev_dbg(wdt_dev->dev, "%s: timeout=%u\n", __func__, timeout);
> +
> +	moxart_wdt->dev.timeout = timeout;
> +
> +	return 0;
> +}
> +
> +static const struct watchdog_info moxart_wdt_info = {
> +	.identity       = "moxart-wdt",
> +	.options        = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING |
> +			  WDIOF_MAGICCLOSE,
> +};
> +
> +static const struct watchdog_ops moxart_wdt_ops = {
> +	.owner          = THIS_MODULE,
> +	.start          = moxart_wdt_start,
> +	.stop           = moxart_wdt_stop,
> +	.set_timeout    = moxart_wdt_set_timeout,
> +};
> +
> +static int moxart_wdt_probe(struct platform_device *pdev)
> +{
> +	struct moxart_wdt_dev *moxart_wdt;
> +	struct device *dev = &pdev->dev;
> +	struct device_node *node = dev->of_node;
> +	struct resource *res;
> +	struct clk *clk;
> +	int err;
> +	unsigned int max_timeout;
> +	bool nowayout = WATCHDOG_NOWAYOUT;
> +
> +	moxart_wdt = devm_kzalloc(&pdev->dev, sizeof(*moxart_wdt), GFP_KERNEL);
> +	if (!moxart_wdt)
> +		return -ENOMEM;
> +
> +	platform_set_drvdata(pdev, moxart_wdt);
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	moxart_wdt->base = devm_ioremap_resource(dev, res);
> +	if (IS_ERR(moxart_wdt->base))
> +		return PTR_ERR(moxart_wdt->base);
> +
> +	clk = of_clk_get(node, 0);
> +	if (IS_ERR(clk)) {
> +		pr_err("%s: of_clk_get failed\n", __func__);
> +		return PTR_ERR(clk);
> +	}
> +
> +	moxart_wdt->clock_frequency = clk_get_rate(clk);
> +	if (moxart_wdt->clock_frequency == 0) {
> +		pr_err("%s: incorrect clock frequency\n", __func__);
> +		return -EINVAL;
> +	}
> +
> +	max_timeout = UINT_MAX / moxart_wdt->clock_frequency;
> +
> +	moxart_wdt->dev.info = &moxart_wdt_info;
> +	moxart_wdt->dev.ops = &moxart_wdt_ops;
> +	moxart_wdt->dev.timeout = max_timeout;
> +	moxart_wdt->dev.min_timeout = 1;
> +	moxart_wdt->dev.max_timeout = max_timeout;
> +	moxart_wdt->dev.parent = &pdev->dev;
> +
> +	watchdog_init_timeout(&moxart_wdt->dev, heartbeat, &pdev->dev);
> +	watchdog_set_nowayout(&moxart_wdt->dev, nowayout);
> +
> +	watchdog_set_drvdata(&moxart_wdt->dev, moxart_wdt);
> +
> +	err = watchdog_register_device(&moxart_wdt->dev);
> +	if (unlikely(err))
> +		return err;
> +
> +	dev_dbg(dev, "Watchdog enabled (heartbeat=%d sec, nowayout=%d)\n",
> +		moxart_wdt->dev.timeout, nowayout);
> +
> +	return 0;
> +}
> +
> +static int moxart_wdt_remove(struct platform_device *pdev)
> +{
> +	struct moxart_wdt_dev *moxart_wdt = platform_get_drvdata(pdev);
> +
> +	moxart_wdt_stop(&moxart_wdt->dev);
> +	watchdog_unregister_device(&moxart_wdt->dev);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id moxart_watchdog_match[] = {
> +	{ .compatible = "moxa,moxart-watchdog" },
> +	{ },
> +};
> +
> +static struct platform_driver moxart_wdt_driver = {
> +	.probe      = moxart_wdt_probe,
> +	.remove     = moxart_wdt_remove,
> +	.driver     = {
> +		.name		= "moxart-watchdog",
> +		.owner		= THIS_MODULE,
> +		.of_match_table	= moxart_watchdog_match,
> +	},
> +};
> +module_platform_driver(moxart_wdt_driver);
> +
> +module_param(heartbeat, int, 0);
> +MODULE_PARM_DESC(heartbeat, "Watchdog heartbeat in seconds");
> +
> +MODULE_DESCRIPTION("MOXART watchdog driver");
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Jonas Jensen <jonas.jensen@gmail.com>");
> -- 
> 1.8.2.1
> 
> 

[PATCH v5] watchdog: Add MOXA ART watchdog driver

[Jonas Jensen]

Add watchdog driver for MOXA ART SoCs.

Signed-off-by: Jonas Jensen <jonas.jensen@gmail.com>
---

Notes:
    Changes since v4:
    
    1. #include <linux/err.h> because IS_ERR comes from there
    2. #include <linux/kernel.h> because UINT_MAX comes from there
    3. #include <linux/moduleparam.h> because module_param() comes from there
    
    device tree bindings document:
    4. describe compatible variable "Must be" instead of "Should be".
    5. change description so it's from the point of view of the device
    
    Applies to next-20130729

 .../bindings/watchdog/moxa,moxart-watchdog.txt     |  15 ++
 drivers/watchdog/Kconfig                           |  10 ++
 drivers/watchdog/Makefile                          |   1 +
 drivers/watchdog/moxart_wdt.c                      | 169 +++++++++++++++++++++
 4 files changed, 195 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/watchdog/moxa,moxart-watchdog.txt
 create mode 100644 drivers/watchdog/moxart_wdt.c

diff --git a/Documentation/devicetree/bindings/watchdog/moxa,moxart-watchdog.txt b/Documentation/devicetree/bindings/watchdog/moxa,moxart-watchdog.txt
new file mode 100644
index 0000000..1169857
--- /dev/null
+++ b/Documentation/devicetree/bindings/watchdog/moxa,moxart-watchdog.txt
@@ -0,0 +1,15 @@
+MOXA ART Watchdog timer
+
+Required properties:
+
+- compatible : Must be "moxa,moxart-watchdog"
+- reg : Should contain registers location and length
+- clocks : Should contain phandle for the clock that drives the counter
+
+Example:
+
+	watchdog: watchdog@98500000 {
+		compatible = "moxa,moxart-watchdog";
+		reg = <0x98500000 0x10>;
+		clocks = <&coreclk>;
+	};
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 362085d..6883aab 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -382,6 +382,16 @@ config RETU_WATCHDOG
 	  To compile this driver as a module, choose M here: the
 	  module will be called retu_wdt.
 
+config MOXART_WDT
+	tristate "MOXART watchdog"
+	depends on ARCH_MOXART
+	help
+	  Say Y here to include Watchdog timer support for the watchdog
+	  existing on the MOXA ART SoC series platforms.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called moxart_wdt.
+
 # AVR32 Architecture
 
 config AT32AP700X_WDT
diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
index 2f26a0b..7cab53a 100644
--- a/drivers/watchdog/Makefile
+++ b/drivers/watchdog/Makefile
@@ -54,6 +54,7 @@ obj-$(CONFIG_IMX2_WDT) += imx2_wdt.o
 obj-$(CONFIG_UX500_WATCHDOG) += ux500_wdt.o
 obj-$(CONFIG_RETU_WATCHDOG) += retu_wdt.o
 obj-$(CONFIG_BCM2835_WDT) += bcm2835_wdt.o
+obj-$(CONFIG_MOXART_WDT) += moxart_wdt.o
 
 # AVR32 Architecture
 obj-$(CONFIG_AT32AP700X_WDT) += at32ap700x_wdt.o
diff --git a/drivers/watchdog/moxart_wdt.c b/drivers/watchdog/moxart_wdt.c
new file mode 100644
index 0000000..d79dc9c
--- /dev/null
+++ b/drivers/watchdog/moxart_wdt.c
@@ -0,0 +1,169 @@
+/*
+ * MOXA ART SoCs watchdog driver.
+ *
+ * Copyright (C) 2013 Jonas Jensen
+ *
+ * Jonas Jensen <jonas.jensen@gmail.com>
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2.  This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+
+#include <linux/clk.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/err.h>
+#include <linux/kernel.h>
+#include <linux/platform_device.h>
+#include <linux/watchdog.h>
+#include <linux/moduleparam.h>
+
+#define REG_COUNT			0x4
+#define REG_MODE			0x8
+#define REG_ENABLE			0xC
+
+struct moxart_wdt_dev {
+	struct watchdog_device dev;
+	void __iomem *base;
+	unsigned int clock_frequency;
+};
+
+static int heartbeat;
+
+static int moxart_wdt_stop(struct watchdog_device *wdt_dev)
+{
+	struct moxart_wdt_dev *moxart_wdt = watchdog_get_drvdata(wdt_dev);
+
+	writel(0, moxart_wdt->base + REG_ENABLE);
+
+	return 0;
+}
+
+static int moxart_wdt_start(struct watchdog_device *wdt_dev)
+{
+	struct moxart_wdt_dev *moxart_wdt = watchdog_get_drvdata(wdt_dev);
+
+	writel(moxart_wdt->clock_frequency * moxart_wdt->dev.timeout,
+	       moxart_wdt->base + REG_COUNT);
+	writel(0x5ab9, moxart_wdt->base + REG_MODE);
+	writel(0x03, moxart_wdt->base + REG_ENABLE);
+
+	return 0;
+}
+
+static int moxart_wdt_set_timeout(struct watchdog_device *wdt_dev,
+				  unsigned int timeout)
+{
+	struct moxart_wdt_dev *moxart_wdt = watchdog_get_drvdata(wdt_dev);
+
+	dev_dbg(wdt_dev->dev, "%s: timeout=%u\n", __func__, timeout);
+
+	moxart_wdt->dev.timeout = timeout;
+
+	return 0;
+}
+
+static const struct watchdog_info moxart_wdt_info = {
+	.identity       = "moxart-wdt",
+	.options        = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING |
+			  WDIOF_MAGICCLOSE,
+};
+
+static const struct watchdog_ops moxart_wdt_ops = {
+	.owner          = THIS_MODULE,
+	.start          = moxart_wdt_start,
+	.stop           = moxart_wdt_stop,
+	.set_timeout    = moxart_wdt_set_timeout,
+};
+
+static int moxart_wdt_probe(struct platform_device *pdev)
+{
+	struct moxart_wdt_dev *moxart_wdt;
+	struct device *dev = &pdev->dev;
+	struct device_node *node = dev->of_node;
+	struct resource *res;
+	struct clk *clk;
+	int err;
+	unsigned int max_timeout;
+	bool nowayout = WATCHDOG_NOWAYOUT;
+
+	moxart_wdt = devm_kzalloc(&pdev->dev, sizeof(*moxart_wdt), GFP_KERNEL);
+	if (!moxart_wdt)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, moxart_wdt);
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	moxart_wdt->base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(moxart_wdt->base))
+		return PTR_ERR(moxart_wdt->base);
+
+	clk = of_clk_get(node, 0);
+	if (IS_ERR(clk)) {
+		pr_err("%s: of_clk_get failed\n", __func__);
+		return PTR_ERR(clk);
+	}
+
+	moxart_wdt->clock_frequency = clk_get_rate(clk);
+	if (moxart_wdt->clock_frequency == 0) {
+		pr_err("%s: incorrect clock frequency\n", __func__);
+		return -EINVAL;
+	}
+
+	max_timeout = UINT_MAX / moxart_wdt->clock_frequency;
+
+	moxart_wdt->dev.info = &moxart_wdt_info;
+	moxart_wdt->dev.ops = &moxart_wdt_ops;
+	moxart_wdt->dev.timeout = max_timeout;
+	moxart_wdt->dev.min_timeout = 1;
+	moxart_wdt->dev.max_timeout = max_timeout;
+	moxart_wdt->dev.parent = &pdev->dev;
+
+	watchdog_init_timeout(&moxart_wdt->dev, heartbeat, &pdev->dev);
+	watchdog_set_nowayout(&moxart_wdt->dev, nowayout);
+
+	watchdog_set_drvdata(&moxart_wdt->dev, moxart_wdt);
+
+	err = watchdog_register_device(&moxart_wdt->dev);
+	if (unlikely(err))
+		return err;
+
+	dev_dbg(dev, "Watchdog enabled (heartbeat=%d sec, nowayout=%d)\n",
+		moxart_wdt->dev.timeout, nowayout);
+
+	return 0;
+}
+
+static int moxart_wdt_remove(struct platform_device *pdev)
+{
+	struct moxart_wdt_dev *moxart_wdt = platform_get_drvdata(pdev);
+
+	moxart_wdt_stop(&moxart_wdt->dev);
+	watchdog_unregister_device(&moxart_wdt->dev);
+
+	return 0;
+}
+
+static const struct of_device_id moxart_watchdog_match[] = {
+	{ .compatible = "moxa,moxart-watchdog" },
+	{ },
+};
+
+static struct platform_driver moxart_wdt_driver = {
+	.probe      = moxart_wdt_probe,
+	.remove     = moxart_wdt_remove,
+	.driver     = {
+		.name		= "moxart-watchdog",
+		.owner		= THIS_MODULE,
+		.of_match_table	= moxart_watchdog_match,
+	},
+};
+module_platform_driver(moxart_wdt_driver);
+
+module_param(heartbeat, int, 0);
+MODULE_PARM_DESC(heartbeat, "Watchdog heartbeat in seconds");
+
+MODULE_DESCRIPTION("MOXART watchdog driver");
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Jonas Jensen <jonas.jensen@gmail.com>");
-- 
1.8.2.1

Re: [PATCH v5] watchdog: Add MOXA ART watchdog driver

[Guenter Roeck]

On 07/29/2013 03:20 AM, Jonas Jensen wrote:
> Add watchdog driver for MOXA ART SoCs.
>
> Signed-off-by: Jonas Jensen <jonas.jensen@gmail.com>
> ---
>
> Notes:
>      Changes since v4:
>
>      1. #include <linux/err.h> because IS_ERR comes from there
>      2. #include <linux/kernel.h> because UINT_MAX comes from there
>      3. #include <linux/moduleparam.h> because module_param() comes from there
>
>      device tree bindings document:
>      4. describe compatible variable "Must be" instead of "Should be".
>      5. change description so it's from the point of view of the device
>
>      Applies to next-20130729
>
>   .../bindings/watchdog/moxa,moxart-watchdog.txt     |  15 ++
>   drivers/watchdog/Kconfig                           |  10 ++
>   drivers/watchdog/Makefile                          |   1 +
>   drivers/watchdog/moxart_wdt.c                      | 169 +++++++++++++++++++++
>   4 files changed, 195 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/watchdog/moxa,moxart-watchdog.txt
>   create mode 100644 drivers/watchdog/moxart_wdt.c
>
> diff --git a/Documentation/devicetree/bindings/watchdog/moxa,moxart-watchdog.txt b/Documentation/devicetree/bindings/watchdog/moxa,moxart-watchdog.txt
> new file mode 100644
> index 0000000..1169857
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/watchdog/moxa,moxart-watchdog.txt
> @@ -0,0 +1,15 @@
> +MOXA ART Watchdog timer
> +
> +Required properties:
> +
> +- compatible : Must be "moxa,moxart-watchdog"
> +- reg : Should contain registers location and length
> +- clocks : Should contain phandle for the clock that drives the counter
> +
> +Example:
> +
> +	watchdog: watchdog@98500000 {
> +		compatible = "moxa,moxart-watchdog";
> +		reg = <0x98500000 0x10>;
> +		clocks = <&coreclk>;
> +	};
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index 362085d..6883aab 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -382,6 +382,16 @@ config RETU_WATCHDOG
>   	  To compile this driver as a module, choose M here: the
>   	  module will be called retu_wdt.
>
> +config MOXART_WDT
> +	tristate "MOXART watchdog"
> +	depends on ARCH_MOXART
> +	help
> +	  Say Y here to include Watchdog timer support for the watchdog
> +	  existing on the MOXA ART SoC series platforms.
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called moxart_wdt.
> +
>   # AVR32 Architecture
>
>   config AT32AP700X_WDT
> diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
> index 2f26a0b..7cab53a 100644
> --- a/drivers/watchdog/Makefile
> +++ b/drivers/watchdog/Makefile
> @@ -54,6 +54,7 @@ obj-$(CONFIG_IMX2_WDT) += imx2_wdt.o
>   obj-$(CONFIG_UX500_WATCHDOG) += ux500_wdt.o
>   obj-$(CONFIG_RETU_WATCHDOG) += retu_wdt.o
>   obj-$(CONFIG_BCM2835_WDT) += bcm2835_wdt.o
> +obj-$(CONFIG_MOXART_WDT) += moxart_wdt.o
>
>   # AVR32 Architecture
>   obj-$(CONFIG_AT32AP700X_WDT) += at32ap700x_wdt.o
> diff --git a/drivers/watchdog/moxart_wdt.c b/drivers/watchdog/moxart_wdt.c
> new file mode 100644
> index 0000000..d79dc9c
> --- /dev/null
> +++ b/drivers/watchdog/moxart_wdt.c
> @@ -0,0 +1,169 @@
> +/*
> + * MOXA ART SoCs watchdog driver.
> + *
> + * Copyright (C) 2013 Jonas Jensen
> + *
> + * Jonas Jensen <jonas.jensen@gmail.com>
> + *
> + * This file is licensed under the terms of the GNU General Public
> + * License version 2.  This program is licensed "as is" without any
> + * warranty of any kind, whether express or implied.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/err.h>
> +#include <linux/kernel.h>
> +#include <linux/platform_device.h>
> +#include <linux/watchdog.h>
> +#include <linux/moduleparam.h>
> +
> +#define REG_COUNT			0x4
> +#define REG_MODE			0x8
> +#define REG_ENABLE			0xC
> +
> +struct moxart_wdt_dev {
> +	struct watchdog_device dev;
> +	void __iomem *base;
> +	unsigned int clock_frequency;
> +};
> +
> +static int heartbeat;
> +
> +static int moxart_wdt_stop(struct watchdog_device *wdt_dev)
> +{
> +	struct moxart_wdt_dev *moxart_wdt = watchdog_get_drvdata(wdt_dev);
> +
> +	writel(0, moxart_wdt->base + REG_ENABLE);
> +
> +	return 0;
> +}
> +
> +static int moxart_wdt_start(struct watchdog_device *wdt_dev)
> +{
> +	struct moxart_wdt_dev *moxart_wdt = watchdog_get_drvdata(wdt_dev);
> +
> +	writel(moxart_wdt->clock_frequency * moxart_wdt->dev.timeout,
> +	       moxart_wdt->base + REG_COUNT);
> +	writel(0x5ab9, moxart_wdt->base + REG_MODE);
> +	writel(0x03, moxart_wdt->base + REG_ENABLE);
> +
> +	return 0;
> +}
> +
> +static int moxart_wdt_set_timeout(struct watchdog_device *wdt_dev,
> +				  unsigned int timeout)
> +{
> +	struct moxart_wdt_dev *moxart_wdt = watchdog_get_drvdata(wdt_dev);
> +
> +	dev_dbg(wdt_dev->dev, "%s: timeout=%u\n", __func__, timeout);
> +
> +	moxart_wdt->dev.timeout = timeout;
> +
Stumbled over this one. Unless I am missing something, &moxart_wdt->dev
and dev are the same, so
	dev->timeout = timeout;
would accomplish the same.

Guenter

> +	return 0;
> +}
> +
> +static const struct watchdog_info moxart_wdt_info = {
> +	.identity       = "moxart-wdt",
> +	.options        = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING |
> +			  WDIOF_MAGICCLOSE,
> +};
> +
> +static const struct watchdog_ops moxart_wdt_ops = {
> +	.owner          = THIS_MODULE,
> +	.start          = moxart_wdt_start,
> +	.stop           = moxart_wdt_stop,
> +	.set_timeout    = moxart_wdt_set_timeout,
> +};
> +
> +static int moxart_wdt_probe(struct platform_device *pdev)
> +{
> +	struct moxart_wdt_dev *moxart_wdt;
> +	struct device *dev = &pdev->dev;
> +	struct device_node *node = dev->of_node;
> +	struct resource *res;
> +	struct clk *clk;
> +	int err;
> +	unsigned int max_timeout;
> +	bool nowayout = WATCHDOG_NOWAYOUT;
> +
> +	moxart_wdt = devm_kzalloc(&pdev->dev, sizeof(*moxart_wdt), GFP_KERNEL);
> +	if (!moxart_wdt)
> +		return -ENOMEM;
> +
> +	platform_set_drvdata(pdev, moxart_wdt);
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	moxart_wdt->base = devm_ioremap_resource(dev, res);
> +	if (IS_ERR(moxart_wdt->base))
> +		return PTR_ERR(moxart_wdt->base);
> +
> +	clk = of_clk_get(node, 0);
> +	if (IS_ERR(clk)) {
> +		pr_err("%s: of_clk_get failed\n", __func__);
> +		return PTR_ERR(clk);
> +	}
> +
> +	moxart_wdt->clock_frequency = clk_get_rate(clk);
> +	if (moxart_wdt->clock_frequency == 0) {
> +		pr_err("%s: incorrect clock frequency\n", __func__);
> +		return -EINVAL;
> +	}
> +
> +	max_timeout = UINT_MAX / moxart_wdt->clock_frequency;
> +
> +	moxart_wdt->dev.info = &moxart_wdt_info;
> +	moxart_wdt->dev.ops = &moxart_wdt_ops;
> +	moxart_wdt->dev.timeout = max_timeout;
> +	moxart_wdt->dev.min_timeout = 1;
> +	moxart_wdt->dev.max_timeout = max_timeout;
> +	moxart_wdt->dev.parent = &pdev->dev;
> +
> +	watchdog_init_timeout(&moxart_wdt->dev, heartbeat, &pdev->dev);
> +	watchdog_set_nowayout(&moxart_wdt->dev, nowayout);
> +
> +	watchdog_set_drvdata(&moxart_wdt->dev, moxart_wdt);
> +
> +	err = watchdog_register_device(&moxart_wdt->dev);
> +	if (unlikely(err))
> +		return err;
> +
> +	dev_dbg(dev, "Watchdog enabled (heartbeat=%d sec, nowayout=%d)\n",
> +		moxart_wdt->dev.timeout, nowayout);
> +
> +	return 0;
> +}
> +
> +static int moxart_wdt_remove(struct platform_device *pdev)
> +{
> +	struct moxart_wdt_dev *moxart_wdt = platform_get_drvdata(pdev);
> +
> +	moxart_wdt_stop(&moxart_wdt->dev);
> +	watchdog_unregister_device(&moxart_wdt->dev);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id moxart_watchdog_match[] = {
> +	{ .compatible = "moxa,moxart-watchdog" },
> +	{ },
> +};
> +
> +static struct platform_driver moxart_wdt_driver = {
> +	.probe      = moxart_wdt_probe,
> +	.remove     = moxart_wdt_remove,
> +	.driver     = {
> +		.name		= "moxart-watchdog",
> +		.owner		= THIS_MODULE,
> +		.of_match_table	= moxart_watchdog_match,
> +	},
> +};
> +module_platform_driver(moxart_wdt_driver);
> +
> +module_param(heartbeat, int, 0);
> +MODULE_PARM_DESC(heartbeat, "Watchdog heartbeat in seconds");
> +
> +MODULE_DESCRIPTION("MOXART watchdog driver");
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Jonas Jensen <jonas.jensen@gmail.com>");
>

[PATCH v6] watchdog: Add MOXA ART watchdog driver

[Jonas Jensen]

Add watchdog driver for MOXA ART SoCs.

Signed-off-by: Jonas Jensen <jonas.jensen@gmail.com>
---

Notes:
    Changes since v5:
    
    1. replace "moxart_wdt->dev.timeout" with "wdt_dev->timeout"
    
    Applies to next-20130729

 .../bindings/watchdog/moxa,moxart-watchdog.txt     |  15 ++
 drivers/watchdog/Kconfig                           |  10 ++
 drivers/watchdog/Makefile                          |   1 +
 drivers/watchdog/moxart_wdt.c                      | 165 +++++++++++++++++++++
 4 files changed, 191 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/watchdog/moxa,moxart-watchdog.txt
 create mode 100644 drivers/watchdog/moxart_wdt.c

diff --git a/Documentation/devicetree/bindings/watchdog/moxa,moxart-watchdog.txt b/Documentation/devicetree/bindings/watchdog/moxa,moxart-watchdog.txt
new file mode 100644
index 0000000..1169857
--- /dev/null
+++ b/Documentation/devicetree/bindings/watchdog/moxa,moxart-watchdog.txt
@@ -0,0 +1,15 @@
+MOXA ART Watchdog timer
+
+Required properties:
+
+- compatible : Must be "moxa,moxart-watchdog"
+- reg : Should contain registers location and length
+- clocks : Should contain phandle for the clock that drives the counter
+
+Example:
+
+	watchdog: watchdog@98500000 {
+		compatible = "moxa,moxart-watchdog";
+		reg = <0x98500000 0x10>;
+		clocks = <&coreclk>;
+	};
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 362085d..6883aab 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -382,6 +382,16 @@ config RETU_WATCHDOG
 	  To compile this driver as a module, choose M here: the
 	  module will be called retu_wdt.
 
+config MOXART_WDT
+	tristate "MOXART watchdog"
+	depends on ARCH_MOXART
+	help
+	  Say Y here to include Watchdog timer support for the watchdog
+	  existing on the MOXA ART SoC series platforms.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called moxart_wdt.
+
 # AVR32 Architecture
 
 config AT32AP700X_WDT
diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
index 2f26a0b..7cab53a 100644
--- a/drivers/watchdog/Makefile
+++ b/drivers/watchdog/Makefile
@@ -54,6 +54,7 @@ obj-$(CONFIG_IMX2_WDT) += imx2_wdt.o
 obj-$(CONFIG_UX500_WATCHDOG) += ux500_wdt.o
 obj-$(CONFIG_RETU_WATCHDOG) += retu_wdt.o
 obj-$(CONFIG_BCM2835_WDT) += bcm2835_wdt.o
+obj-$(CONFIG_MOXART_WDT) += moxart_wdt.o
 
 # AVR32 Architecture
 obj-$(CONFIG_AT32AP700X_WDT) += at32ap700x_wdt.o
diff --git a/drivers/watchdog/moxart_wdt.c b/drivers/watchdog/moxart_wdt.c
new file mode 100644
index 0000000..70b5806
--- /dev/null
+++ b/drivers/watchdog/moxart_wdt.c
@@ -0,0 +1,165 @@
+/*
+ * MOXA ART SoCs watchdog driver.
+ *
+ * Copyright (C) 2013 Jonas Jensen
+ *
+ * Jonas Jensen <jonas.jensen@gmail.com>
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2.  This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+
+#include <linux/clk.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/err.h>
+#include <linux/kernel.h>
+#include <linux/platform_device.h>
+#include <linux/watchdog.h>
+#include <linux/moduleparam.h>
+
+#define REG_COUNT			0x4
+#define REG_MODE			0x8
+#define REG_ENABLE			0xC
+
+struct moxart_wdt_dev {
+	struct watchdog_device dev;
+	void __iomem *base;
+	unsigned int clock_frequency;
+};
+
+static int heartbeat;
+
+static int moxart_wdt_stop(struct watchdog_device *wdt_dev)
+{
+	struct moxart_wdt_dev *moxart_wdt = watchdog_get_drvdata(wdt_dev);
+
+	writel(0, moxart_wdt->base + REG_ENABLE);
+
+	return 0;
+}
+
+static int moxart_wdt_start(struct watchdog_device *wdt_dev)
+{
+	struct moxart_wdt_dev *moxart_wdt = watchdog_get_drvdata(wdt_dev);
+
+	writel(moxart_wdt->clock_frequency * wdt_dev->timeout,
+	       moxart_wdt->base + REG_COUNT);
+	writel(0x5ab9, moxart_wdt->base + REG_MODE);
+	writel(0x03, moxart_wdt->base + REG_ENABLE);
+
+	return 0;
+}
+
+static int moxart_wdt_set_timeout(struct watchdog_device *wdt_dev,
+				  unsigned int timeout)
+{
+	wdt_dev->timeout = timeout;
+
+	return 0;
+}
+
+static const struct watchdog_info moxart_wdt_info = {
+	.identity       = "moxart-wdt",
+	.options        = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING |
+			  WDIOF_MAGICCLOSE,
+};
+
+static const struct watchdog_ops moxart_wdt_ops = {
+	.owner          = THIS_MODULE,
+	.start          = moxart_wdt_start,
+	.stop           = moxart_wdt_stop,
+	.set_timeout    = moxart_wdt_set_timeout,
+};
+
+static int moxart_wdt_probe(struct platform_device *pdev)
+{
+	struct moxart_wdt_dev *moxart_wdt;
+	struct device *dev = &pdev->dev;
+	struct device_node *node = dev->of_node;
+	struct resource *res;
+	struct clk *clk;
+	int err;
+	unsigned int max_timeout;
+	bool nowayout = WATCHDOG_NOWAYOUT;
+
+	moxart_wdt = devm_kzalloc(&pdev->dev, sizeof(*moxart_wdt), GFP_KERNEL);
+	if (!moxart_wdt)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, moxart_wdt);
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	moxart_wdt->base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(moxart_wdt->base))
+		return PTR_ERR(moxart_wdt->base);
+
+	clk = of_clk_get(node, 0);
+	if (IS_ERR(clk)) {
+		pr_err("%s: of_clk_get failed\n", __func__);
+		return PTR_ERR(clk);
+	}
+
+	moxart_wdt->clock_frequency = clk_get_rate(clk);
+	if (moxart_wdt->clock_frequency == 0) {
+		pr_err("%s: incorrect clock frequency\n", __func__);
+		return -EINVAL;
+	}
+
+	max_timeout = UINT_MAX / moxart_wdt->clock_frequency;
+
+	moxart_wdt->dev.info = &moxart_wdt_info;
+	moxart_wdt->dev.ops = &moxart_wdt_ops;
+	moxart_wdt->dev.timeout = max_timeout;
+	moxart_wdt->dev.min_timeout = 1;
+	moxart_wdt->dev.max_timeout = max_timeout;
+	moxart_wdt->dev.parent = &pdev->dev;
+
+	watchdog_init_timeout(&moxart_wdt->dev, heartbeat, &pdev->dev);
+	watchdog_set_nowayout(&moxart_wdt->dev, nowayout);
+
+	watchdog_set_drvdata(&moxart_wdt->dev, moxart_wdt);
+
+	err = watchdog_register_device(&moxart_wdt->dev);
+	if (unlikely(err))
+		return err;
+
+	dev_dbg(dev, "Watchdog enabled (heartbeat=%d sec, nowayout=%d)\n",
+		moxart_wdt->dev.timeout, nowayout);
+
+	return 0;
+}
+
+static int moxart_wdt_remove(struct platform_device *pdev)
+{
+	struct moxart_wdt_dev *moxart_wdt = platform_get_drvdata(pdev);
+
+	moxart_wdt_stop(&moxart_wdt->dev);
+	watchdog_unregister_device(&moxart_wdt->dev);
+
+	return 0;
+}
+
+static const struct of_device_id moxart_watchdog_match[] = {
+	{ .compatible = "moxa,moxart-watchdog" },
+	{ },
+};
+
+static struct platform_driver moxart_wdt_driver = {
+	.probe      = moxart_wdt_probe,
+	.remove     = moxart_wdt_remove,
+	.driver     = {
+		.name		= "moxart-watchdog",
+		.owner		= THIS_MODULE,
+		.of_match_table	= moxart_watchdog_match,
+	},
+};
+module_platform_driver(moxart_wdt_driver);
+
+module_param(heartbeat, int, 0);
+MODULE_PARM_DESC(heartbeat, "Watchdog heartbeat in seconds");
+
+MODULE_DESCRIPTION("MOXART watchdog driver");
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Jonas Jensen <jonas.jensen@gmail.com>");
-- 
1.8.2.1

Re: [PATCH v6] watchdog: Add MOXA ART watchdog driver

[Guenter Roeck]

On 07/29/2013 04:41 AM, Jonas Jensen wrote:
> Add watchdog driver for MOXA ART SoCs.
>
> Signed-off-by: Jonas Jensen <jonas.jensen@gmail.com>
> ---
>
> Notes:
>      Changes since v5:
>
>      1. replace "moxart_wdt->dev.timeout" with "wdt_dev->timeout"
>
>      Applies to next-20130729
>
>   .../bindings/watchdog/moxa,moxart-watchdog.txt     |  15 ++
>   drivers/watchdog/Kconfig                           |  10 ++
>   drivers/watchdog/Makefile                          |   1 +
>   drivers/watchdog/moxart_wdt.c                      | 165 +++++++++++++++++++++
>   4 files changed, 191 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/watchdog/moxa,moxart-watchdog.txt
>   create mode 100644 drivers/watchdog/moxart_wdt.c
>
> diff --git a/Documentation/devicetree/bindings/watchdog/moxa,moxart-watchdog.txt b/Documentation/devicetree/bindings/watchdog/moxa,moxart-watchdog.txt
> new file mode 100644
> index 0000000..1169857
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/watchdog/moxa,moxart-watchdog.txt
> @@ -0,0 +1,15 @@
> +MOXA ART Watchdog timer
> +
> +Required properties:
> +
> +- compatible : Must be "moxa,moxart-watchdog"
> +- reg : Should contain registers location and length
> +- clocks : Should contain phandle for the clock that drives the counter
> +
> +Example:
> +
> +	watchdog: watchdog@98500000 {
> +		compatible = "moxa,moxart-watchdog";
> +		reg = <0x98500000 0x10>;
> +		clocks = <&coreclk>;
> +	};
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index 362085d..6883aab 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -382,6 +382,16 @@ config RETU_WATCHDOG
>   	  To compile this driver as a module, choose M here: the
>   	  module will be called retu_wdt.
>
> +config MOXART_WDT
> +	tristate "MOXART watchdog"
> +	depends on ARCH_MOXART
> +	help
> +	  Say Y here to include Watchdog timer support for the watchdog
> +	  existing on the MOXA ART SoC series platforms.
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called moxart_wdt.
> +
>   # AVR32 Architecture
>
>   config AT32AP700X_WDT
> diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
> index 2f26a0b..7cab53a 100644
> --- a/drivers/watchdog/Makefile
> +++ b/drivers/watchdog/Makefile
> @@ -54,6 +54,7 @@ obj-$(CONFIG_IMX2_WDT) += imx2_wdt.o
>   obj-$(CONFIG_UX500_WATCHDOG) += ux500_wdt.o
>   obj-$(CONFIG_RETU_WATCHDOG) += retu_wdt.o
>   obj-$(CONFIG_BCM2835_WDT) += bcm2835_wdt.o
> +obj-$(CONFIG_MOXART_WDT) += moxart_wdt.o
>
>   # AVR32 Architecture
>   obj-$(CONFIG_AT32AP700X_WDT) += at32ap700x_wdt.o
> diff --git a/drivers/watchdog/moxart_wdt.c b/drivers/watchdog/moxart_wdt.c
> new file mode 100644
> index 0000000..70b5806
> --- /dev/null
> +++ b/drivers/watchdog/moxart_wdt.c
> @@ -0,0 +1,165 @@
> +/*
> + * MOXA ART SoCs watchdog driver.
> + *
> + * Copyright (C) 2013 Jonas Jensen
> + *
> + * Jonas Jensen <jonas.jensen@gmail.com>
> + *
> + * This file is licensed under the terms of the GNU General Public
> + * License version 2.  This program is licensed "as is" without any
> + * warranty of any kind, whether express or implied.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/err.h>
> +#include <linux/kernel.h>
> +#include <linux/platform_device.h>
> +#include <linux/watchdog.h>
> +#include <linux/moduleparam.h>
> +
> +#define REG_COUNT			0x4
> +#define REG_MODE			0x8
> +#define REG_ENABLE			0xC
> +
> +struct moxart_wdt_dev {
> +	struct watchdog_device dev;
> +	void __iomem *base;
> +	unsigned int clock_frequency;
> +};
> +
> +static int heartbeat;
> +
> +static int moxart_wdt_stop(struct watchdog_device *wdt_dev)
> +{
> +	struct moxart_wdt_dev *moxart_wdt = watchdog_get_drvdata(wdt_dev);
> +
> +	writel(0, moxart_wdt->base + REG_ENABLE);
> +
> +	return 0;
> +}
> +
> +static int moxart_wdt_start(struct watchdog_device *wdt_dev)
> +{
> +	struct moxart_wdt_dev *moxart_wdt = watchdog_get_drvdata(wdt_dev);
> +
> +	writel(moxart_wdt->clock_frequency * wdt_dev->timeout,
> +	       moxart_wdt->base + REG_COUNT);
> +	writel(0x5ab9, moxart_wdt->base + REG_MODE);
> +	writel(0x03, moxart_wdt->base + REG_ENABLE);
> +
> +	return 0;
> +}
> +
> +static int moxart_wdt_set_timeout(struct watchdog_device *wdt_dev,
> +				  unsigned int timeout)
> +{
> +	wdt_dev->timeout = timeout;
> +
> +	return 0;
> +}
> +
> +static const struct watchdog_info moxart_wdt_info = {
> +	.identity       = "moxart-wdt",
> +	.options        = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING |
> +			  WDIOF_MAGICCLOSE,
> +};
> +
> +static const struct watchdog_ops moxart_wdt_ops = {
> +	.owner          = THIS_MODULE,
> +	.start          = moxart_wdt_start,
> +	.stop           = moxart_wdt_stop,
> +	.set_timeout    = moxart_wdt_set_timeout,
> +};
> +
> +static int moxart_wdt_probe(struct platform_device *pdev)
> +{
> +	struct moxart_wdt_dev *moxart_wdt;
> +	struct device *dev = &pdev->dev;

I have my nitpicking day ;). Even though you declare dev = &pdev->dev,
you use &pdev->dev several times below.

Assuming you change this, feel free to add

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


> +	struct device_node *node = dev->of_node;
> +	struct resource *res;
> +	struct clk *clk;
> +	int err;
> +	unsigned int max_timeout;
> +	bool nowayout = WATCHDOG_NOWAYOUT;
> +
> +	moxart_wdt = devm_kzalloc(&pdev->dev, sizeof(*moxart_wdt), GFP_KERNEL);
> +	if (!moxart_wdt)
> +		return -ENOMEM;
> +
> +	platform_set_drvdata(pdev, moxart_wdt);
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	moxart_wdt->base = devm_ioremap_resource(dev, res);
> +	if (IS_ERR(moxart_wdt->base))
> +		return PTR_ERR(moxart_wdt->base);
> +
> +	clk = of_clk_get(node, 0);
> +	if (IS_ERR(clk)) {
> +		pr_err("%s: of_clk_get failed\n", __func__);
> +		return PTR_ERR(clk);
> +	}
> +
> +	moxart_wdt->clock_frequency = clk_get_rate(clk);
> +	if (moxart_wdt->clock_frequency == 0) {
> +		pr_err("%s: incorrect clock frequency\n", __func__);
> +		return -EINVAL;
> +	}
> +
> +	max_timeout = UINT_MAX / moxart_wdt->clock_frequency;
> +
> +	moxart_wdt->dev.info = &moxart_wdt_info;
> +	moxart_wdt->dev.ops = &moxart_wdt_ops;
> +	moxart_wdt->dev.timeout = max_timeout;
> +	moxart_wdt->dev.min_timeout = 1;
> +	moxart_wdt->dev.max_timeout = max_timeout;
> +	moxart_wdt->dev.parent = &pdev->dev;
> +
> +	watchdog_init_timeout(&moxart_wdt->dev, heartbeat, &pdev->dev);
> +	watchdog_set_nowayout(&moxart_wdt->dev, nowayout);
> +
> +	watchdog_set_drvdata(&moxart_wdt->dev, moxart_wdt);
> +
> +	err = watchdog_register_device(&moxart_wdt->dev);
> +	if (unlikely(err))
> +		return err;
> +
> +	dev_dbg(dev, "Watchdog enabled (heartbeat=%d sec, nowayout=%d)\n",
> +		moxart_wdt->dev.timeout, nowayout);
> +
> +	return 0;
> +}
> +
> +static int moxart_wdt_remove(struct platform_device *pdev)
> +{
> +	struct moxart_wdt_dev *moxart_wdt = platform_get_drvdata(pdev);
> +
> +	moxart_wdt_stop(&moxart_wdt->dev);
> +	watchdog_unregister_device(&moxart_wdt->dev);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id moxart_watchdog_match[] = {
> +	{ .compatible = "moxa,moxart-watchdog" },
> +	{ },
> +};
> +
> +static struct platform_driver moxart_wdt_driver = {
> +	.probe      = moxart_wdt_probe,
> +	.remove     = moxart_wdt_remove,
> +	.driver     = {
> +		.name		= "moxart-watchdog",
> +		.owner		= THIS_MODULE,
> +		.of_match_table	= moxart_watchdog_match,
> +	},
> +};
> +module_platform_driver(moxart_wdt_driver);
> +
> +module_param(heartbeat, int, 0);
> +MODULE_PARM_DESC(heartbeat, "Watchdog heartbeat in seconds");
> +
> +MODULE_DESCRIPTION("MOXART watchdog driver");
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Jonas Jensen <jonas.jensen@gmail.com>");
>

[PATCH v7] watchdog: Add MOXA ART watchdog driver

[Jonas Jensen]

Add watchdog driver for MOXA ART SoCs.

Signed-off-by: Jonas Jensen <jonas.jensen@gmail.com>
---

Notes:
    Changes since v6:
    
    1. use local struct device *dev pointer, replace "&pdev->dev" with "dev"
    
    Applies to next-20130729

 .../bindings/watchdog/moxa,moxart-watchdog.txt     |  15 ++
 drivers/watchdog/Kconfig                           |  10 ++
 drivers/watchdog/Makefile                          |   1 +
 drivers/watchdog/moxart_wdt.c                      | 165 +++++++++++++++++++++
 4 files changed, 191 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/watchdog/moxa,moxart-watchdog.txt
 create mode 100644 drivers/watchdog/moxart_wdt.c

diff --git a/Documentation/devicetree/bindings/watchdog/moxa,moxart-watchdog.txt b/Documentation/devicetree/bindings/watchdog/moxa,moxart-watchdog.txt
new file mode 100644
index 0000000..1169857
--- /dev/null
+++ b/Documentation/devicetree/bindings/watchdog/moxa,moxart-watchdog.txt
@@ -0,0 +1,15 @@
+MOXA ART Watchdog timer
+
+Required properties:
+
+- compatible : Must be "moxa,moxart-watchdog"
+- reg : Should contain registers location and length
+- clocks : Should contain phandle for the clock that drives the counter
+
+Example:
+
+	watchdog: watchdog@98500000 {
+		compatible = "moxa,moxart-watchdog";
+		reg = <0x98500000 0x10>;
+		clocks = <&coreclk>;
+	};
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 362085d..6883aab 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -382,6 +382,16 @@ config RETU_WATCHDOG
 	  To compile this driver as a module, choose M here: the
 	  module will be called retu_wdt.
 
+config MOXART_WDT
+	tristate "MOXART watchdog"
+	depends on ARCH_MOXART
+	help
+	  Say Y here to include Watchdog timer support for the watchdog
+	  existing on the MOXA ART SoC series platforms.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called moxart_wdt.
+
 # AVR32 Architecture
 
 config AT32AP700X_WDT
diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
index 2f26a0b..7cab53a 100644
--- a/drivers/watchdog/Makefile
+++ b/drivers/watchdog/Makefile
@@ -54,6 +54,7 @@ obj-$(CONFIG_IMX2_WDT) += imx2_wdt.o
 obj-$(CONFIG_UX500_WATCHDOG) += ux500_wdt.o
 obj-$(CONFIG_RETU_WATCHDOG) += retu_wdt.o
 obj-$(CONFIG_BCM2835_WDT) += bcm2835_wdt.o
+obj-$(CONFIG_MOXART_WDT) += moxart_wdt.o
 
 # AVR32 Architecture
 obj-$(CONFIG_AT32AP700X_WDT) += at32ap700x_wdt.o
diff --git a/drivers/watchdog/moxart_wdt.c b/drivers/watchdog/moxart_wdt.c
new file mode 100644
index 0000000..48dc334
--- /dev/null
+++ b/drivers/watchdog/moxart_wdt.c
@@ -0,0 +1,165 @@
+/*
+ * MOXA ART SoCs watchdog driver.
+ *
+ * Copyright (C) 2013 Jonas Jensen
+ *
+ * Jonas Jensen <jonas.jensen@gmail.com>
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2.  This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+
+#include <linux/clk.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/err.h>
+#include <linux/kernel.h>
+#include <linux/platform_device.h>
+#include <linux/watchdog.h>
+#include <linux/moduleparam.h>
+
+#define REG_COUNT			0x4
+#define REG_MODE			0x8
+#define REG_ENABLE			0xC
+
+struct moxart_wdt_dev {
+	struct watchdog_device dev;
+	void __iomem *base;
+	unsigned int clock_frequency;
+};
+
+static int heartbeat;
+
+static int moxart_wdt_stop(struct watchdog_device *wdt_dev)
+{
+	struct moxart_wdt_dev *moxart_wdt = watchdog_get_drvdata(wdt_dev);
+
+	writel(0, moxart_wdt->base + REG_ENABLE);
+
+	return 0;
+}
+
+static int moxart_wdt_start(struct watchdog_device *wdt_dev)
+{
+	struct moxart_wdt_dev *moxart_wdt = watchdog_get_drvdata(wdt_dev);
+
+	writel(moxart_wdt->clock_frequency * wdt_dev->timeout,
+	       moxart_wdt->base + REG_COUNT);
+	writel(0x5ab9, moxart_wdt->base + REG_MODE);
+	writel(0x03, moxart_wdt->base + REG_ENABLE);
+
+	return 0;
+}
+
+static int moxart_wdt_set_timeout(struct watchdog_device *wdt_dev,
+				  unsigned int timeout)
+{
+	wdt_dev->timeout = timeout;
+
+	return 0;
+}
+
+static const struct watchdog_info moxart_wdt_info = {
+	.identity       = "moxart-wdt",
+	.options        = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING |
+			  WDIOF_MAGICCLOSE,
+};
+
+static const struct watchdog_ops moxart_wdt_ops = {
+	.owner          = THIS_MODULE,
+	.start          = moxart_wdt_start,
+	.stop           = moxart_wdt_stop,
+	.set_timeout    = moxart_wdt_set_timeout,
+};
+
+static int moxart_wdt_probe(struct platform_device *pdev)
+{
+	struct moxart_wdt_dev *moxart_wdt;
+	struct device *dev = &pdev->dev;
+	struct device_node *node = dev->of_node;
+	struct resource *res;
+	struct clk *clk;
+	int err;
+	unsigned int max_timeout;
+	bool nowayout = WATCHDOG_NOWAYOUT;
+
+	moxart_wdt = devm_kzalloc(dev, sizeof(*moxart_wdt), GFP_KERNEL);
+	if (!moxart_wdt)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, moxart_wdt);
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	moxart_wdt->base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(moxart_wdt->base))
+		return PTR_ERR(moxart_wdt->base);
+
+	clk = of_clk_get(node, 0);
+	if (IS_ERR(clk)) {
+		pr_err("%s: of_clk_get failed\n", __func__);
+		return PTR_ERR(clk);
+	}
+
+	moxart_wdt->clock_frequency = clk_get_rate(clk);
+	if (moxart_wdt->clock_frequency == 0) {
+		pr_err("%s: incorrect clock frequency\n", __func__);
+		return -EINVAL;
+	}
+
+	max_timeout = UINT_MAX / moxart_wdt->clock_frequency;
+
+	moxart_wdt->dev.info = &moxart_wdt_info;
+	moxart_wdt->dev.ops = &moxart_wdt_ops;
+	moxart_wdt->dev.timeout = max_timeout;
+	moxart_wdt->dev.min_timeout = 1;
+	moxart_wdt->dev.max_timeout = max_timeout;
+	moxart_wdt->dev.parent = dev;
+
+	watchdog_init_timeout(&moxart_wdt->dev, heartbeat, dev);
+	watchdog_set_nowayout(&moxart_wdt->dev, nowayout);
+
+	watchdog_set_drvdata(&moxart_wdt->dev, moxart_wdt);
+
+	err = watchdog_register_device(&moxart_wdt->dev);
+	if (unlikely(err))
+		return err;
+
+	dev_dbg(dev, "Watchdog enabled (heartbeat=%d sec, nowayout=%d)\n",
+		moxart_wdt->dev.timeout, nowayout);
+
+	return 0;
+}
+
+static int moxart_wdt_remove(struct platform_device *pdev)
+{
+	struct moxart_wdt_dev *moxart_wdt = platform_get_drvdata(pdev);
+
+	moxart_wdt_stop(&moxart_wdt->dev);
+	watchdog_unregister_device(&moxart_wdt->dev);
+
+	return 0;
+}
+
+static const struct of_device_id moxart_watchdog_match[] = {
+	{ .compatible = "moxa,moxart-watchdog" },
+	{ },
+};
+
+static struct platform_driver moxart_wdt_driver = {
+	.probe      = moxart_wdt_probe,
+	.remove     = moxart_wdt_remove,
+	.driver     = {
+		.name		= "moxart-watchdog",
+		.owner		= THIS_MODULE,
+		.of_match_table	= moxart_watchdog_match,
+	},
+};
+module_platform_driver(moxart_wdt_driver);
+
+module_param(heartbeat, int, 0);
+MODULE_PARM_DESC(heartbeat, "Watchdog heartbeat in seconds");
+
+MODULE_DESCRIPTION("MOXART watchdog driver");
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Jonas Jensen <jonas.jensen@gmail.com>");
-- 
1.8.2.1

Re: [PATCH v7] watchdog: Add MOXA ART watchdog driver

[Guenter Roeck]

On 07/29/2013 05:33 AM, Jonas Jensen wrote:
> Add watchdog driver for MOXA ART SoCs.
>
> Signed-off-by: Jonas Jensen <jonas.jensen@gmail.com>

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

Re: [PATCH v7] watchdog: Add MOXA ART watchdog driver

[Mark Rutland]

On Mon, Jul 29, 2013 at 01:33:48PM +0100, Jonas Jensen wrote:
> Add watchdog driver for MOXA ART SoCs.
> 

It would be nice to have a fuller description here.

> Signed-off-by: Jonas Jensen <jonas.jensen@gmail.com>
> ---
> 
> Notes:
>     Changes since v6:
>     
>     1. use local struct device *dev pointer, replace "&pdev->dev" with "dev"
>     
>     Applies to next-20130729
> 
>  .../bindings/watchdog/moxa,moxart-watchdog.txt     |  15 ++
>  drivers/watchdog/Kconfig                           |  10 ++
>  drivers/watchdog/Makefile                          |   1 +
>  drivers/watchdog/moxart_wdt.c                      | 165 +++++++++++++++++++++
>  4 files changed, 191 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/watchdog/moxa,moxart-watchdog.txt
>  create mode 100644 drivers/watchdog/moxart_wdt.c
> 
> diff --git a/Documentation/devicetree/bindings/watchdog/moxa,moxart-watchdog.txt b/Documentation/devicetree/bindings/watchdog/moxa,moxart-watchdog.txt
> new file mode 100644
> index 0000000..1169857
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/watchdog/moxa,moxart-watchdog.txt
> @@ -0,0 +1,15 @@
> +MOXA ART Watchdog timer
> +
> +Required properties:
> +
> +- compatible : Must be "moxa,moxart-watchdog"
> +- reg : Should contain registers location and length
> +- clocks : Should contain phandle for the clock that drives the counter
> +
> +Example:
> +
> +	watchdog: watchdog@98500000 {
> +		compatible = "moxa,moxart-watchdog";
> +		reg = <0x98500000 0x10>;
> +		clocks = <&coreclk>;
> +	};

This seems sensible to me. I assume the watchdog is a standalone unit,
and not part of some larger piece of IP?

> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index 362085d..6883aab 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -382,6 +382,16 @@ config RETU_WATCHDOG
>  	  To compile this driver as a module, choose M here: the
>  	  module will be called retu_wdt.
>  
> +config MOXART_WDT
> +	tristate "MOXART watchdog"
> +	depends on ARCH_MOXART
> +	help
> +	  Say Y here to include Watchdog timer support for the watchdog
> +	  existing on the MOXA ART SoC series platforms.
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called moxart_wdt.
> +
>  # AVR32 Architecture
>  
>  config AT32AP700X_WDT
> diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
> index 2f26a0b..7cab53a 100644
> --- a/drivers/watchdog/Makefile
> +++ b/drivers/watchdog/Makefile
> @@ -54,6 +54,7 @@ obj-$(CONFIG_IMX2_WDT) += imx2_wdt.o
>  obj-$(CONFIG_UX500_WATCHDOG) += ux500_wdt.o
>  obj-$(CONFIG_RETU_WATCHDOG) += retu_wdt.o
>  obj-$(CONFIG_BCM2835_WDT) += bcm2835_wdt.o
> +obj-$(CONFIG_MOXART_WDT) += moxart_wdt.o
>  
>  # AVR32 Architecture
>  obj-$(CONFIG_AT32AP700X_WDT) += at32ap700x_wdt.o
> diff --git a/drivers/watchdog/moxart_wdt.c b/drivers/watchdog/moxart_wdt.c
> new file mode 100644
> index 0000000..48dc334
> --- /dev/null
> +++ b/drivers/watchdog/moxart_wdt.c
> @@ -0,0 +1,165 @@
> +/*
> + * MOXA ART SoCs watchdog driver.
> + *
> + * Copyright (C) 2013 Jonas Jensen
> + *
> + * Jonas Jensen <jonas.jensen@gmail.com>
> + *
> + * This file is licensed under the terms of the GNU General Public
> + * License version 2.  This program is licensed "as is" without any
> + * warranty of any kind, whether express or implied.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/err.h>
> +#include <linux/kernel.h>
> +#include <linux/platform_device.h>
> +#include <linux/watchdog.h>
> +#include <linux/moduleparam.h>
> +
> +#define REG_COUNT			0x4
> +#define REG_MODE			0x8
> +#define REG_ENABLE			0xC
> +
> +struct moxart_wdt_dev {
> +	struct watchdog_device dev;
> +	void __iomem *base;
> +	unsigned int clock_frequency;
> +};
> +
> +static int heartbeat;
> +
> +static int moxart_wdt_stop(struct watchdog_device *wdt_dev)
> +{
> +	struct moxart_wdt_dev *moxart_wdt = watchdog_get_drvdata(wdt_dev);
> +
> +	writel(0, moxart_wdt->base + REG_ENABLE);
> +
> +	return 0;
> +}
> +
> +static int moxart_wdt_start(struct watchdog_device *wdt_dev)
> +{
> +	struct moxart_wdt_dev *moxart_wdt = watchdog_get_drvdata(wdt_dev);
> +
> +	writel(moxart_wdt->clock_frequency * wdt_dev->timeout,
> +	       moxart_wdt->base + REG_COUNT);
> +	writel(0x5ab9, moxart_wdt->base + REG_MODE);
> +	writel(0x03, moxart_wdt->base + REG_ENABLE);
> +
> +	return 0;
> +}
> +
> +static int moxart_wdt_set_timeout(struct watchdog_device *wdt_dev,
> +				  unsigned int timeout)
> +{
> +	wdt_dev->timeout = timeout;
> +
> +	return 0;
> +}
> +
> +static const struct watchdog_info moxart_wdt_info = {
> +	.identity       = "moxart-wdt",
> +	.options        = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING |
> +			  WDIOF_MAGICCLOSE,
> +};
> +
> +static const struct watchdog_ops moxart_wdt_ops = {
> +	.owner          = THIS_MODULE,
> +	.start          = moxart_wdt_start,
> +	.stop           = moxart_wdt_stop,
> +	.set_timeout    = moxart_wdt_set_timeout,
> +};
> +
> +static int moxart_wdt_probe(struct platform_device *pdev)
> +{
> +	struct moxart_wdt_dev *moxart_wdt;
> +	struct device *dev = &pdev->dev;
> +	struct device_node *node = dev->of_node;
> +	struct resource *res;
> +	struct clk *clk;
> +	int err;
> +	unsigned int max_timeout;
> +	bool nowayout = WATCHDOG_NOWAYOUT;
> +
> +	moxart_wdt = devm_kzalloc(dev, sizeof(*moxart_wdt), GFP_KERNEL);
> +	if (!moxart_wdt)
> +		return -ENOMEM;
> +
> +	platform_set_drvdata(pdev, moxart_wdt);
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	moxart_wdt->base = devm_ioremap_resource(dev, res);
> +	if (IS_ERR(moxart_wdt->base))
> +		return PTR_ERR(moxart_wdt->base);
> +
> +	clk = of_clk_get(node, 0);
> +	if (IS_ERR(clk)) {
> +		pr_err("%s: of_clk_get failed\n", __func__);
> +		return PTR_ERR(clk);
> +	}
> +
> +	moxart_wdt->clock_frequency = clk_get_rate(clk);
> +	if (moxart_wdt->clock_frequency == 0) {
> +		pr_err("%s: incorrect clock frequency\n", __func__);
> +		return -EINVAL;
> +	}
> +
> +	max_timeout = UINT_MAX / moxart_wdt->clock_frequency;
> +
> +	moxart_wdt->dev.info = &moxart_wdt_info;
> +	moxart_wdt->dev.ops = &moxart_wdt_ops;
> +	moxart_wdt->dev.timeout = max_timeout;
> +	moxart_wdt->dev.min_timeout = 1;
> +	moxart_wdt->dev.max_timeout = max_timeout;
> +	moxart_wdt->dev.parent = dev;
> +
> +	watchdog_init_timeout(&moxart_wdt->dev, heartbeat, dev);
> +	watchdog_set_nowayout(&moxart_wdt->dev, nowayout);
> +
> +	watchdog_set_drvdata(&moxart_wdt->dev, moxart_wdt);
> +
> +	err = watchdog_register_device(&moxart_wdt->dev);
> +	if (unlikely(err))
> +		return err;

This is a probe path. Is the use of unlikely() really appropriate here?
I suspect it doesn't make any appreciable difference, and should go.

> +
> +	dev_dbg(dev, "Watchdog enabled (heartbeat=%d sec, nowayout=%d)\n",
> +		moxart_wdt->dev.timeout, nowayout);
> +
> +	return 0;
> +}

Thanks,
Mark.

[PATCH v8] watchdog: Add MOXA ART watchdog driver

[Jonas Jensen]

This patch adds a watchdog driver for the main hardware watchdog timer
found on MOXA ART SoCs.

The MOXA ART SoC provides one writable timer register, restarting
the hardware once it reaches zero. The register is auto decremented
every APB clock cycle.

Signed-off-by: Jonas Jensen <jonas.jensen@gmail.com>
---

Notes:
    Changes since v7:
    
    1. remove unlikely from watchdog_register_device return value check
    2. elaborate commit message
    
    Applies to next-20130802

 .../bindings/watchdog/moxa,moxart-watchdog.txt     |  15 ++
 drivers/watchdog/Kconfig                           |  10 ++
 drivers/watchdog/Makefile                          |   1 +
 drivers/watchdog/moxart_wdt.c                      | 165 +++++++++++++++++++++
 4 files changed, 191 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/watchdog/moxa,moxart-watchdog.txt
 create mode 100644 drivers/watchdog/moxart_wdt.c

diff --git a/Documentation/devicetree/bindings/watchdog/moxa,moxart-watchdog.txt b/Documentation/devicetree/bindings/watchdog/moxa,moxart-watchdog.txt
new file mode 100644
index 0000000..1169857
--- /dev/null
+++ b/Documentation/devicetree/bindings/watchdog/moxa,moxart-watchdog.txt
@@ -0,0 +1,15 @@
+MOXA ART Watchdog timer
+
+Required properties:
+
+- compatible : Must be "moxa,moxart-watchdog"
+- reg : Should contain registers location and length
+- clocks : Should contain phandle for the clock that drives the counter
+
+Example:
+
+	watchdog: watchdog@98500000 {
+		compatible = "moxa,moxart-watchdog";
+		reg = <0x98500000 0x10>;
+		clocks = <&coreclk>;
+	};
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 362085d..6883aab 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -382,6 +382,16 @@ config RETU_WATCHDOG
 	  To compile this driver as a module, choose M here: the
 	  module will be called retu_wdt.
 
+config MOXART_WDT
+	tristate "MOXART watchdog"
+	depends on ARCH_MOXART
+	help
+	  Say Y here to include Watchdog timer support for the watchdog
+	  existing on the MOXA ART SoC series platforms.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called moxart_wdt.
+
 # AVR32 Architecture
 
 config AT32AP700X_WDT
diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
index 2f26a0b..7cab53a 100644
--- a/drivers/watchdog/Makefile
+++ b/drivers/watchdog/Makefile
@@ -54,6 +54,7 @@ obj-$(CONFIG_IMX2_WDT) += imx2_wdt.o
 obj-$(CONFIG_UX500_WATCHDOG) += ux500_wdt.o
 obj-$(CONFIG_RETU_WATCHDOG) += retu_wdt.o
 obj-$(CONFIG_BCM2835_WDT) += bcm2835_wdt.o
+obj-$(CONFIG_MOXART_WDT) += moxart_wdt.o
 
 # AVR32 Architecture
 obj-$(CONFIG_AT32AP700X_WDT) += at32ap700x_wdt.o
diff --git a/drivers/watchdog/moxart_wdt.c b/drivers/watchdog/moxart_wdt.c
new file mode 100644
index 0000000..4166e4d
--- /dev/null
+++ b/drivers/watchdog/moxart_wdt.c
@@ -0,0 +1,165 @@
+/*
+ * MOXA ART SoCs watchdog driver.
+ *
+ * Copyright (C) 2013 Jonas Jensen
+ *
+ * Jonas Jensen <jonas.jensen@gmail.com>
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2.  This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+
+#include <linux/clk.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/err.h>
+#include <linux/kernel.h>
+#include <linux/platform_device.h>
+#include <linux/watchdog.h>
+#include <linux/moduleparam.h>
+
+#define REG_COUNT			0x4
+#define REG_MODE			0x8
+#define REG_ENABLE			0xC
+
+struct moxart_wdt_dev {
+	struct watchdog_device dev;
+	void __iomem *base;
+	unsigned int clock_frequency;
+};
+
+static int heartbeat;
+
+static int moxart_wdt_stop(struct watchdog_device *wdt_dev)
+{
+	struct moxart_wdt_dev *moxart_wdt = watchdog_get_drvdata(wdt_dev);
+
+	writel(0, moxart_wdt->base + REG_ENABLE);
+
+	return 0;
+}
+
+static int moxart_wdt_start(struct watchdog_device *wdt_dev)
+{
+	struct moxart_wdt_dev *moxart_wdt = watchdog_get_drvdata(wdt_dev);
+
+	writel(moxart_wdt->clock_frequency * wdt_dev->timeout,
+	       moxart_wdt->base + REG_COUNT);
+	writel(0x5ab9, moxart_wdt->base + REG_MODE);
+	writel(0x03, moxart_wdt->base + REG_ENABLE);
+
+	return 0;
+}
+
+static int moxart_wdt_set_timeout(struct watchdog_device *wdt_dev,
+				  unsigned int timeout)
+{
+	wdt_dev->timeout = timeout;
+
+	return 0;
+}
+
+static const struct watchdog_info moxart_wdt_info = {
+	.identity       = "moxart-wdt",
+	.options        = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING |
+			  WDIOF_MAGICCLOSE,
+};
+
+static const struct watchdog_ops moxart_wdt_ops = {
+	.owner          = THIS_MODULE,
+	.start          = moxart_wdt_start,
+	.stop           = moxart_wdt_stop,
+	.set_timeout    = moxart_wdt_set_timeout,
+};
+
+static int moxart_wdt_probe(struct platform_device *pdev)
+{
+	struct moxart_wdt_dev *moxart_wdt;
+	struct device *dev = &pdev->dev;
+	struct device_node *node = dev->of_node;
+	struct resource *res;
+	struct clk *clk;
+	int err;
+	unsigned int max_timeout;
+	bool nowayout = WATCHDOG_NOWAYOUT;
+
+	moxart_wdt = devm_kzalloc(dev, sizeof(*moxart_wdt), GFP_KERNEL);
+	if (!moxart_wdt)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, moxart_wdt);
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	moxart_wdt->base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(moxart_wdt->base))
+		return PTR_ERR(moxart_wdt->base);
+
+	clk = of_clk_get(node, 0);
+	if (IS_ERR(clk)) {
+		pr_err("%s: of_clk_get failed\n", __func__);
+		return PTR_ERR(clk);
+	}
+
+	moxart_wdt->clock_frequency = clk_get_rate(clk);
+	if (moxart_wdt->clock_frequency == 0) {
+		pr_err("%s: incorrect clock frequency\n", __func__);
+		return -EINVAL;
+	}
+
+	max_timeout = UINT_MAX / moxart_wdt->clock_frequency;
+
+	moxart_wdt->dev.info = &moxart_wdt_info;
+	moxart_wdt->dev.ops = &moxart_wdt_ops;
+	moxart_wdt->dev.timeout = max_timeout;
+	moxart_wdt->dev.min_timeout = 1;
+	moxart_wdt->dev.max_timeout = max_timeout;
+	moxart_wdt->dev.parent = dev;
+
+	watchdog_init_timeout(&moxart_wdt->dev, heartbeat, dev);
+	watchdog_set_nowayout(&moxart_wdt->dev, nowayout);
+
+	watchdog_set_drvdata(&moxart_wdt->dev, moxart_wdt);
+
+	err = watchdog_register_device(&moxart_wdt->dev);
+	if (err)
+		return err;
+
+	dev_dbg(dev, "Watchdog enabled (heartbeat=%d sec, nowayout=%d)\n",
+		moxart_wdt->dev.timeout, nowayout);
+
+	return 0;
+}
+
+static int moxart_wdt_remove(struct platform_device *pdev)
+{
+	struct moxart_wdt_dev *moxart_wdt = platform_get_drvdata(pdev);
+
+	moxart_wdt_stop(&moxart_wdt->dev);
+	watchdog_unregister_device(&moxart_wdt->dev);
+
+	return 0;
+}
+
+static const struct of_device_id moxart_watchdog_match[] = {
+	{ .compatible = "moxa,moxart-watchdog" },
+	{ },
+};
+
+static struct platform_driver moxart_wdt_driver = {
+	.probe      = moxart_wdt_probe,
+	.remove     = moxart_wdt_remove,
+	.driver     = {
+		.name		= "moxart-watchdog",
+		.owner		= THIS_MODULE,
+		.of_match_table	= moxart_watchdog_match,
+	},
+};
+module_platform_driver(moxart_wdt_driver);
+
+module_param(heartbeat, int, 0);
+MODULE_PARM_DESC(heartbeat, "Watchdog heartbeat in seconds");
+
+MODULE_DESCRIPTION("MOXART watchdog driver");
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Jonas Jensen <jonas.jensen@gmail.com>");
-- 
1.8.2.1

Re: [PATCH v7] watchdog: Add MOXA ART watchdog driver

[Jonas Jensen]

Hi Mark,

Thanks for the replies.

On 2 August 2013 13:41, Mark Rutland <mark.rutland@arm.com> wrote:
> It would be nice to have a fuller description here.

I have tried to elaborate the commit message, please point out if it
should be formatted differently.

>> +Example:
>> +
>> +     watchdog: watchdog@98500000 {
>> +             compatible = "moxa,moxart-watchdog";
>> +             reg = <0x98500000 0x10>;
>> +             clocks = <&coreclk>;
>> +     };
>
> This seems sensible to me. I assume the watchdog is a standalone unit,
> and not part of some larger piece of IP?

I think it's standalone, unfortunately documentation on the SoC (which
this is likely part of) is not publicly available.

The register is not shared with anything else.

> This is a probe path. Is the use of unlikely() really appropriate here?
> I suspect it doesn't make any appreciable difference, and should go.

No, it just happened to get copied from a reference driver.

Best regards,
Jonas

Re: [PATCH v7] watchdog: Add MOXA ART watchdog driver

[Guenter Roeck]

On 08/02/2013 04:41 AM, Mark Rutland wrote:
> On Mon, Jul 29, 2013 at 01:33:48PM +0100, Jonas Jensen wrote:
>> Add watchdog driver for MOXA ART SoCs.
>>
[ ... ]
>> +
>> +	err = watchdog_register_device(&moxart_wdt->dev);
>> +	if (unlikely(err))
>> +		return err;
>
> This is a probe path. Is the use of unlikely() really appropriate here?
> I suspect it doesn't make any appreciable difference, and should go.
>
Just wondering, for my education - why ? Is there s rule that unlikely()
shall not be used in the probe path ? If so, I would like to know it and
its reasoning to be able to apply it to my own reviews.

Thanks,
Guenter

Re: [PATCH v8] watchdog: Add MOXA ART watchdog driver

[Guenter Roeck]

On 08/02/2013 07:40 AM, Jonas Jensen wrote:
> This patch adds a watchdog driver for the main hardware watchdog timer
> found on MOXA ART SoCs.
>
> The MOXA ART SoC provides one writable timer register, restarting
> the hardware once it reaches zero. The register is auto decremented
> every APB clock cycle.
>
> Signed-off-by: Jonas Jensen <jonas.jensen@gmail.com>

Still wondering where the requirement comes from not to use unlikely()
in probe functions. Must be one of those tribal knowledge things people
have been talking about.

Nevertheless, still

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

Guenter

Re: [PATCH v7] watchdog: Add MOXA ART watchdog driver

[Mark Rutland]

On Fri, Aug 02, 2013 at 05:39:49PM +0100, Guenter Roeck wrote:
> On 08/02/2013 04:41 AM, Mark Rutland wrote:
> > On Mon, Jul 29, 2013 at 01:33:48PM +0100, Jonas Jensen wrote:
> >> Add watchdog driver for MOXA ART SoCs.
> >>
> [ ... ]
> >> +
> >> +	err = watchdog_register_device(&moxart_wdt->dev);
> >> +	if (unlikely(err))
> >> +		return err;
> >
> > This is a probe path. Is the use of unlikely() really appropriate here?
> > I suspect it doesn't make any appreciable difference, and should go.
> >
> Just wondering, for my education - why ? Is there s rule that unlikely()
> shall not be used in the probe path ? If so, I would like to know it and
> its reasoning to be able to apply it to my own reviews.

I thought there had been some discussion on LKML about likely and
unlikely being abused, but in some quick searching I couldn't find the
thread I remembered. I found a comment from Steve Rostedt [1], but
that's not necessarily canonical.

As I understand it, likely() and unlikely() should be used to tell the
compiler to optimise for very specific cases where we know the branch
likelihood far better than the compiler, and the performance gain is
significant (i.e. hot paths). Using them elsewhere may lead to
performance and/or size degradation, as we're asking the compiler to do
something other than what it believes is optimal.

Probing is far from a hot path, and it seems in this case at least that
unlikely() is only present as part of some duplicated black magic.

Thanks,
Mark.

[1] https://lkml.org/lkml/2011/1/3/135

Re: [PATCH v8] watchdog: Add MOXA ART watchdog driver

[Wim Van Sebroeck]

Hi Jonas,

> This patch adds a watchdog driver for the main hardware watchdog timer
> found on MOXA ART SoCs.
> 
> The MOXA ART SoC provides one writable timer register, restarting
> the hardware once it reaches zero. The register is auto decremented
> every APB clock cycle.
> 
> Signed-off-by: Jonas Jensen <jonas.jensen@gmail.com>

Added to linux-watchdog-next.

Kind regards,
Wim.