All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 2/2] watchdog: pic32-dmt: Add PIC32 deadman timer driver
@ 2016-02-25 18:30 ` Joshua Henderson
  0 siblings, 0 replies; 7+ messages in thread
From: Joshua Henderson @ 2016-02-25 18:30 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-mips, ralf, Purna Chandra Mandal, Joshua Henderson,
	Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	Wim Van Sebroeck, Guenter Roeck, devicetree, linux-watchdog

From: Purna Chandra Mandal <purna.mandal@microchip.com>

Adds support for the deadman timer peripheral found on PIC32 class devices.

The primary function of the deadman timer (DMT) is to reset the processor
in the event of a software malfunction. The DMT is a free-running
instruction fetch timer, which is clocked whenever an instruction fetch
occurs until a count match occurs. Instructions are not fetched when
the processor is in sleep mode.

Signed-off-by: Purna Chandra Mandal <purna.mandal@microchip.com>
Signed-off-by: Joshua Henderson <joshua.henderson@microchip.com>
Cc: Ralf Baechle <ralf@linux-mips.org>
---
Note: Please merge this patch series through the MIPS tree.

Changes since v1:
	- Drop default y in Kconfig
	- Alphabetical include files
	- Use BIT() where appropriate
	- Replace cpu_relax() with nop() and comment why it's needed
	- Return bool on functions where appropriate
	- Add a way to break out of a tight loop
	- Remove static unused function
	- Remove redundant spinlock already covered by watchdog core
	- Remove unecessary error handling when enabling DMT
	- Drop implementation of .get_timeleft
	- Drop calculation of max_timeout
	- Cleanup dev_err() message contents
	- Fix race condition with watchdog device register
	- Unregister watchdog before disabling clock
	- Fix typo in driver name
---
 drivers/watchdog/Kconfig     |   13 +++
 drivers/watchdog/Makefile    |    1 +
 drivers/watchdog/pic32-dmt.c |  260 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 274 insertions(+)
 create mode 100644 drivers/watchdog/pic32-dmt.c

diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 543fa81..53747e9 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -1427,6 +1427,19 @@ config PIC32_WDT
 	  To compile this driver as a loadable module, choose M here.
 	  The module will be called pic32-wdt.
 
+config PIC32_DMT
+	tristate "Microchip PIC32 Deadman Timer"
+	select WATCHDOG_CORE
+	depends on MACH_PIC32
+	help
+	  Watchdog driver for PIC32 instruction fetch counting timer. This specific
+	  timer is typically be used in misson critical and safety critical
+	  applications, where any single failure of the software functionality
+	  and sequencing must be detected.
+
+	  To compile this driver as a loadable module, choose M here.
+	  The module will be called pic32-dmt.
+
 # PARISC Architecture
 
 # POWERPC Architecture
diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
index 244ed80..d051c9c 100644
--- a/drivers/watchdog/Makefile
+++ b/drivers/watchdog/Makefile
@@ -154,6 +154,7 @@ obj-$(CONFIG_RALINK_WDT) += rt2880_wdt.o
 obj-$(CONFIG_IMGPDC_WDT) += imgpdc_wdt.o
 obj-$(CONFIG_MT7621_WDT) += mt7621_wdt.o
 obj-$(CONFIG_PIC32_WDT) += pic32-wdt.o
+obj-$(CONFIG_PIC32_DMT) += pic32-dmt.o
 
 # PARISC Architecture
 
diff --git a/drivers/watchdog/pic32-dmt.c b/drivers/watchdog/pic32-dmt.c
new file mode 100644
index 0000000..91801fa
--- /dev/null
+++ b/drivers/watchdog/pic32-dmt.c
@@ -0,0 +1,260 @@
+/*
+ * PIC32 deadman timer driver
+ *
+ * Purna Chandra Mandal <purna.mandal@microchip.com>
+ * Copyright (c) 2016, Microchip Technology Inc.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/clk.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/pm.h>
+#include <linux/watchdog.h>
+
+#include <asm/mach-pic32/pic32.h>
+
+/* Deadman Timer Regs */
+#define DMTCON_REG	0x00
+#define DMTPRECLR_REG	0x10
+#define DMTCLR_REG	0x20
+#define DMTSTAT_REG	0x30
+#define DMTCNT_REG	0x40
+#define DMTPSCNT_REG	0x60
+#define DMTPSINTV_REG	0x70
+
+/* Deadman Timer Regs fields */
+#define DMT_ON			BIT(15)
+#define DMT_STEP1_KEY		BIT(6)
+#define DMT_STEP2_KEY		BIT(3)
+#define DMTSTAT_WINOPN		BIT(0)
+#define DMTSTAT_EVENT		BIT(5)
+#define DMTSTAT_BAD2		BIT(6)
+#define DMTSTAT_BAD1		BIT(7)
+
+/* Reset Control Register fields for watchdog */
+#define RESETCON_DMT_TIMEOUT    BIT(5)
+
+struct pic32_dmt {
+	void __iomem	*regs;
+	struct clk	*clk;
+};
+
+static inline void dmt_enable(struct pic32_dmt *dmt)
+{
+	writel(DMT_ON, PIC32_SET(dmt->regs + DMTCON_REG));
+}
+
+static inline void dmt_disable(struct pic32_dmt *dmt)
+{
+	writel(DMT_ON, PIC32_CLR(dmt->regs + DMTCON_REG));
+	/*
+	 * Cannot touch registers in the CPU cycle following clearing the
+	 * ON bit.
+	 */
+	nop();
+}
+
+static inline int dmt_bad_status(struct pic32_dmt *dmt)
+{
+	u32 val;
+
+	val = readl(dmt->regs + DMTSTAT_REG);
+	val &= (DMTSTAT_BAD1 | DMTSTAT_BAD2 | DMTSTAT_EVENT);
+	if (val)
+		return -EAGAIN;
+
+	return 0;
+}
+
+static inline int dmt_keepalive(struct pic32_dmt *dmt)
+{
+	u32 v;
+	u32 timeout = 500;
+
+	/* set pre-clear key */
+	writel(DMT_STEP1_KEY << 8, dmt->regs + DMTPRECLR_REG);
+
+	/* wait for DMT window to open */
+	while (--timeout) {
+		v = readl(dmt->regs + DMTSTAT_REG) & DMTSTAT_WINOPN;
+		if (v == DMTSTAT_WINOPN)
+			break;
+	}
+
+	/* apply key2 */
+	writel(DMT_STEP2_KEY, dmt->regs + DMTCLR_REG);
+
+	/* check whether keys are latched correctly */
+	return dmt_bad_status(dmt);
+}
+
+static inline u32 pic32_dmt_get_timeout_secs(struct pic32_dmt *dmt)
+{
+	unsigned long rate;
+
+	rate = clk_get_rate(dmt->clk);
+	if (rate)
+		return readl(dmt->regs + DMTPSCNT_REG) / rate;
+
+	return -EINVAL;
+}
+
+static inline u32 pic32_dmt_bootstatus(struct pic32_dmt *dmt)
+{
+	u32 v;
+	void __iomem *rst_base;
+
+	rst_base = ioremap(PIC32_BASE_RESET, 0x10);
+	if (!rst_base)
+		return 0;
+
+	v = readl(rst_base);
+
+	writel(RESETCON_DMT_TIMEOUT, PIC32_CLR(rst_base));
+
+	iounmap(rst_base);
+	return v & RESETCON_DMT_TIMEOUT;
+}
+
+static int pic32_dmt_start(struct watchdog_device *wdd)
+{
+	struct pic32_dmt *dmt = watchdog_get_drvdata(wdd);
+
+	dmt_enable(dmt);
+	return dmt_keepalive(dmt);
+}
+
+static int pic32_dmt_stop(struct watchdog_device *wdd)
+{
+	struct pic32_dmt *dmt = watchdog_get_drvdata(wdd);
+
+	dmt_disable(dmt);
+
+	return 0;
+}
+
+static int pic32_dmt_ping(struct watchdog_device *wdd)
+{
+	struct pic32_dmt *dmt = watchdog_get_drvdata(wdd);
+
+	return dmt_keepalive(dmt);
+}
+
+static const struct watchdog_ops pic32_dmt_fops = {
+	.owner		= THIS_MODULE,
+	.start		= pic32_dmt_start,
+	.stop		= pic32_dmt_stop,
+	.ping		= pic32_dmt_ping,
+};
+
+static const struct watchdog_info pic32_dmt_ident = {
+	.options	= WDIOF_KEEPALIVEPING |
+			  WDIOF_MAGICCLOSE,
+	.identity	= "PIC32 Deadman Timer",
+};
+
+static struct watchdog_device pic32_dmt_wdd = {
+	.info		= &pic32_dmt_ident,
+	.ops		= &pic32_dmt_fops,
+};
+
+static int pic32_dmt_probe(struct platform_device *pdev)
+{
+	int ret;
+	struct pic32_dmt *dmt;
+	struct resource *mem;
+	struct watchdog_device *wdd = &pic32_dmt_wdd;
+
+	dmt = devm_kzalloc(&pdev->dev, sizeof(*dmt), GFP_KERNEL);
+	if (IS_ERR(dmt))
+		return PTR_ERR(dmt);
+
+	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	dmt->regs = devm_ioremap_resource(&pdev->dev, mem);
+	if (IS_ERR(dmt->regs))
+		return PTR_ERR(dmt->regs);
+
+	dmt->clk = devm_clk_get(&pdev->dev, NULL);
+	if (IS_ERR(dmt->clk)) {
+		dev_err(&pdev->dev, "clk not found\n");
+		return PTR_ERR(dmt->clk);
+	}
+
+	ret = clk_prepare_enable(dmt->clk);
+	if (ret)
+		return ret;
+
+	wdd->timeout = pic32_dmt_get_timeout_secs(dmt);
+	if (!wdd->timeout) {
+		dev_err(&pdev->dev,
+			"failed to read watchdog register timeout\n");
+		ret = -EINVAL;
+		goto out_disable_clk;
+	}
+
+	dev_info(&pdev->dev, "timeout %d\n", wdd->timeout);
+
+	wdd->bootstatus = pic32_dmt_bootstatus(dmt) ? WDIOF_CARDRESET : 0;
+
+	watchdog_set_nowayout(wdd, WATCHDOG_NOWAYOUT);
+	watchdog_set_drvdata(wdd, dmt);
+
+	ret = watchdog_register_device(wdd);
+	if (ret) {
+		dev_err(&pdev->dev, "watchdog register failed, err %d\n", ret);
+		goto out_disable_clk;
+	}
+
+	platform_set_drvdata(pdev, wdd);
+	return 0;
+
+out_disable_clk:
+	clk_disable_unprepare(dmt->clk);
+	return ret;
+}
+
+static int pic32_dmt_remove(struct platform_device *pdev)
+{
+	struct watchdog_device *wdd = platform_get_drvdata(pdev);
+	struct pic32_dmt *dmt = watchdog_get_drvdata(wdd);
+
+	watchdog_unregister_device(wdd);
+	clk_disable_unprepare(dmt->clk);
+
+	return 0;
+}
+
+static const struct of_device_id pic32_dmt_of_ids[] = {
+	{ .compatible = "microchip,pic32mzda-dmt",},
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, pic32_dmt_of_ids);
+
+static struct platform_driver pic32_dmt_driver = {
+	.probe		= pic32_dmt_probe,
+	.remove		= pic32_dmt_remove,
+	.driver		= {
+		.name		= "pic32-dmt",
+		.owner		= THIS_MODULE,
+		.of_match_table = of_match_ptr(pic32_dmt_of_ids),
+	}
+};
+
+module_platform_driver(pic32_dmt_driver);
+
+MODULE_AUTHOR("Purna Chandra Mandal <purna.mandal@microchip.com>");
+MODULE_DESCRIPTION("Microchip PIC32 DMT Driver");
+MODULE_LICENSE("GPL");
-- 
1.7.9.5

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

* [PATCH v2 2/2] watchdog: pic32-dmt: Add PIC32 deadman timer driver
@ 2016-02-25 18:30 ` Joshua Henderson
  0 siblings, 0 replies; 7+ messages in thread
From: Joshua Henderson @ 2016-02-25 18:30 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-mips, ralf, Purna Chandra Mandal, Joshua Henderson,
	Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	Wim Van Sebroeck, Guenter Roeck, devicetree, linux-watchdog

From: Purna Chandra Mandal <purna.mandal@microchip.com>

Adds support for the deadman timer peripheral found on PIC32 class devices.

The primary function of the deadman timer (DMT) is to reset the processor
in the event of a software malfunction. The DMT is a free-running
instruction fetch timer, which is clocked whenever an instruction fetch
occurs until a count match occurs. Instructions are not fetched when
the processor is in sleep mode.

Signed-off-by: Purna Chandra Mandal <purna.mandal@microchip.com>
Signed-off-by: Joshua Henderson <joshua.henderson@microchip.com>
Cc: Ralf Baechle <ralf@linux-mips.org>
---
Note: Please merge this patch series through the MIPS tree.

Changes since v1:
	- Drop default y in Kconfig
	- Alphabetical include files
	- Use BIT() where appropriate
	- Replace cpu_relax() with nop() and comment why it's needed
	- Return bool on functions where appropriate
	- Add a way to break out of a tight loop
	- Remove static unused function
	- Remove redundant spinlock already covered by watchdog core
	- Remove unecessary error handling when enabling DMT
	- Drop implementation of .get_timeleft
	- Drop calculation of max_timeout
	- Cleanup dev_err() message contents
	- Fix race condition with watchdog device register
	- Unregister watchdog before disabling clock
	- Fix typo in driver name
---
 drivers/watchdog/Kconfig     |   13 +++
 drivers/watchdog/Makefile    |    1 +
 drivers/watchdog/pic32-dmt.c |  260 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 274 insertions(+)
 create mode 100644 drivers/watchdog/pic32-dmt.c

diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 543fa81..53747e9 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -1427,6 +1427,19 @@ config PIC32_WDT
 	  To compile this driver as a loadable module, choose M here.
 	  The module will be called pic32-wdt.
 
+config PIC32_DMT
+	tristate "Microchip PIC32 Deadman Timer"
+	select WATCHDOG_CORE
+	depends on MACH_PIC32
+	help
+	  Watchdog driver for PIC32 instruction fetch counting timer. This specific
+	  timer is typically be used in misson critical and safety critical
+	  applications, where any single failure of the software functionality
+	  and sequencing must be detected.
+
+	  To compile this driver as a loadable module, choose M here.
+	  The module will be called pic32-dmt.
+
 # PARISC Architecture
 
 # POWERPC Architecture
diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
index 244ed80..d051c9c 100644
--- a/drivers/watchdog/Makefile
+++ b/drivers/watchdog/Makefile
@@ -154,6 +154,7 @@ obj-$(CONFIG_RALINK_WDT) += rt2880_wdt.o
 obj-$(CONFIG_IMGPDC_WDT) += imgpdc_wdt.o
 obj-$(CONFIG_MT7621_WDT) += mt7621_wdt.o
 obj-$(CONFIG_PIC32_WDT) += pic32-wdt.o
+obj-$(CONFIG_PIC32_DMT) += pic32-dmt.o
 
 # PARISC Architecture
 
diff --git a/drivers/watchdog/pic32-dmt.c b/drivers/watchdog/pic32-dmt.c
new file mode 100644
index 0000000..91801fa
--- /dev/null
+++ b/drivers/watchdog/pic32-dmt.c
@@ -0,0 +1,260 @@
+/*
+ * PIC32 deadman timer driver
+ *
+ * Purna Chandra Mandal <purna.mandal@microchip.com>
+ * Copyright (c) 2016, Microchip Technology Inc.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/clk.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/pm.h>
+#include <linux/watchdog.h>
+
+#include <asm/mach-pic32/pic32.h>
+
+/* Deadman Timer Regs */
+#define DMTCON_REG	0x00
+#define DMTPRECLR_REG	0x10
+#define DMTCLR_REG	0x20
+#define DMTSTAT_REG	0x30
+#define DMTCNT_REG	0x40
+#define DMTPSCNT_REG	0x60
+#define DMTPSINTV_REG	0x70
+
+/* Deadman Timer Regs fields */
+#define DMT_ON			BIT(15)
+#define DMT_STEP1_KEY		BIT(6)
+#define DMT_STEP2_KEY		BIT(3)
+#define DMTSTAT_WINOPN		BIT(0)
+#define DMTSTAT_EVENT		BIT(5)
+#define DMTSTAT_BAD2		BIT(6)
+#define DMTSTAT_BAD1		BIT(7)
+
+/* Reset Control Register fields for watchdog */
+#define RESETCON_DMT_TIMEOUT    BIT(5)
+
+struct pic32_dmt {
+	void __iomem	*regs;
+	struct clk	*clk;
+};
+
+static inline void dmt_enable(struct pic32_dmt *dmt)
+{
+	writel(DMT_ON, PIC32_SET(dmt->regs + DMTCON_REG));
+}
+
+static inline void dmt_disable(struct pic32_dmt *dmt)
+{
+	writel(DMT_ON, PIC32_CLR(dmt->regs + DMTCON_REG));
+	/*
+	 * Cannot touch registers in the CPU cycle following clearing the
+	 * ON bit.
+	 */
+	nop();
+}
+
+static inline int dmt_bad_status(struct pic32_dmt *dmt)
+{
+	u32 val;
+
+	val = readl(dmt->regs + DMTSTAT_REG);
+	val &= (DMTSTAT_BAD1 | DMTSTAT_BAD2 | DMTSTAT_EVENT);
+	if (val)
+		return -EAGAIN;
+
+	return 0;
+}
+
+static inline int dmt_keepalive(struct pic32_dmt *dmt)
+{
+	u32 v;
+	u32 timeout = 500;
+
+	/* set pre-clear key */
+	writel(DMT_STEP1_KEY << 8, dmt->regs + DMTPRECLR_REG);
+
+	/* wait for DMT window to open */
+	while (--timeout) {
+		v = readl(dmt->regs + DMTSTAT_REG) & DMTSTAT_WINOPN;
+		if (v == DMTSTAT_WINOPN)
+			break;
+	}
+
+	/* apply key2 */
+	writel(DMT_STEP2_KEY, dmt->regs + DMTCLR_REG);
+
+	/* check whether keys are latched correctly */
+	return dmt_bad_status(dmt);
+}
+
+static inline u32 pic32_dmt_get_timeout_secs(struct pic32_dmt *dmt)
+{
+	unsigned long rate;
+
+	rate = clk_get_rate(dmt->clk);
+	if (rate)
+		return readl(dmt->regs + DMTPSCNT_REG) / rate;
+
+	return -EINVAL;
+}
+
+static inline u32 pic32_dmt_bootstatus(struct pic32_dmt *dmt)
+{
+	u32 v;
+	void __iomem *rst_base;
+
+	rst_base = ioremap(PIC32_BASE_RESET, 0x10);
+	if (!rst_base)
+		return 0;
+
+	v = readl(rst_base);
+
+	writel(RESETCON_DMT_TIMEOUT, PIC32_CLR(rst_base));
+
+	iounmap(rst_base);
+	return v & RESETCON_DMT_TIMEOUT;
+}
+
+static int pic32_dmt_start(struct watchdog_device *wdd)
+{
+	struct pic32_dmt *dmt = watchdog_get_drvdata(wdd);
+
+	dmt_enable(dmt);
+	return dmt_keepalive(dmt);
+}
+
+static int pic32_dmt_stop(struct watchdog_device *wdd)
+{
+	struct pic32_dmt *dmt = watchdog_get_drvdata(wdd);
+
+	dmt_disable(dmt);
+
+	return 0;
+}
+
+static int pic32_dmt_ping(struct watchdog_device *wdd)
+{
+	struct pic32_dmt *dmt = watchdog_get_drvdata(wdd);
+
+	return dmt_keepalive(dmt);
+}
+
+static const struct watchdog_ops pic32_dmt_fops = {
+	.owner		= THIS_MODULE,
+	.start		= pic32_dmt_start,
+	.stop		= pic32_dmt_stop,
+	.ping		= pic32_dmt_ping,
+};
+
+static const struct watchdog_info pic32_dmt_ident = {
+	.options	= WDIOF_KEEPALIVEPING |
+			  WDIOF_MAGICCLOSE,
+	.identity	= "PIC32 Deadman Timer",
+};
+
+static struct watchdog_device pic32_dmt_wdd = {
+	.info		= &pic32_dmt_ident,
+	.ops		= &pic32_dmt_fops,
+};
+
+static int pic32_dmt_probe(struct platform_device *pdev)
+{
+	int ret;
+	struct pic32_dmt *dmt;
+	struct resource *mem;
+	struct watchdog_device *wdd = &pic32_dmt_wdd;
+
+	dmt = devm_kzalloc(&pdev->dev, sizeof(*dmt), GFP_KERNEL);
+	if (IS_ERR(dmt))
+		return PTR_ERR(dmt);
+
+	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	dmt->regs = devm_ioremap_resource(&pdev->dev, mem);
+	if (IS_ERR(dmt->regs))
+		return PTR_ERR(dmt->regs);
+
+	dmt->clk = devm_clk_get(&pdev->dev, NULL);
+	if (IS_ERR(dmt->clk)) {
+		dev_err(&pdev->dev, "clk not found\n");
+		return PTR_ERR(dmt->clk);
+	}
+
+	ret = clk_prepare_enable(dmt->clk);
+	if (ret)
+		return ret;
+
+	wdd->timeout = pic32_dmt_get_timeout_secs(dmt);
+	if (!wdd->timeout) {
+		dev_err(&pdev->dev,
+			"failed to read watchdog register timeout\n");
+		ret = -EINVAL;
+		goto out_disable_clk;
+	}
+
+	dev_info(&pdev->dev, "timeout %d\n", wdd->timeout);
+
+	wdd->bootstatus = pic32_dmt_bootstatus(dmt) ? WDIOF_CARDRESET : 0;
+
+	watchdog_set_nowayout(wdd, WATCHDOG_NOWAYOUT);
+	watchdog_set_drvdata(wdd, dmt);
+
+	ret = watchdog_register_device(wdd);
+	if (ret) {
+		dev_err(&pdev->dev, "watchdog register failed, err %d\n", ret);
+		goto out_disable_clk;
+	}
+
+	platform_set_drvdata(pdev, wdd);
+	return 0;
+
+out_disable_clk:
+	clk_disable_unprepare(dmt->clk);
+	return ret;
+}
+
+static int pic32_dmt_remove(struct platform_device *pdev)
+{
+	struct watchdog_device *wdd = platform_get_drvdata(pdev);
+	struct pic32_dmt *dmt = watchdog_get_drvdata(wdd);
+
+	watchdog_unregister_device(wdd);
+	clk_disable_unprepare(dmt->clk);
+
+	return 0;
+}
+
+static const struct of_device_id pic32_dmt_of_ids[] = {
+	{ .compatible = "microchip,pic32mzda-dmt",},
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, pic32_dmt_of_ids);
+
+static struct platform_driver pic32_dmt_driver = {
+	.probe		= pic32_dmt_probe,
+	.remove		= pic32_dmt_remove,
+	.driver		= {
+		.name		= "pic32-dmt",
+		.owner		= THIS_MODULE,
+		.of_match_table = of_match_ptr(pic32_dmt_of_ids),
+	}
+};
+
+module_platform_driver(pic32_dmt_driver);
+
+MODULE_AUTHOR("Purna Chandra Mandal <purna.mandal@microchip.com>");
+MODULE_DESCRIPTION("Microchip PIC32 DMT Driver");
+MODULE_LICENSE("GPL");
-- 
1.7.9.5

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

* Re: [PATCH v2 2/2] watchdog: pic32-dmt: Add PIC32 deadman timer driver
@ 2016-02-26 14:11   ` Guenter Roeck
  0 siblings, 0 replies; 7+ messages in thread
From: Guenter Roeck @ 2016-02-26 14:11 UTC (permalink / raw)
  To: Joshua Henderson, linux-kernel
  Cc: linux-mips, ralf, Purna Chandra Mandal, Rob Herring, Pawel Moll,
	Mark Rutland, Ian Campbell, Kumar Gala, Wim Van Sebroeck,
	devicetree, linux-watchdog

On 02/25/2016 10:30 AM, Joshua Henderson wrote:
> From: Purna Chandra Mandal <purna.mandal@microchip.com>
>
> Adds support for the deadman timer peripheral found on PIC32 class devices.
>
> The primary function of the deadman timer (DMT) is to reset the processor
> in the event of a software malfunction. The DMT is a free-running
> instruction fetch timer, which is clocked whenever an instruction fetch
> occurs until a count match occurs. Instructions are not fetched when
> the processor is in sleep mode.
>
> Signed-off-by: Purna Chandra Mandal <purna.mandal@microchip.com>
> Signed-off-by: Joshua Henderson <joshua.henderson@microchip.com>
> Cc: Ralf Baechle <ralf@linux-mips.org>
> ---
> Note: Please merge this patch series through the MIPS tree.
>
> Changes since v1:
> 	- Drop default y in Kconfig
> 	- Alphabetical include files
> 	- Use BIT() where appropriate
> 	- Replace cpu_relax() with nop() and comment why it's needed
> 	- Return bool on functions where appropriate
> 	- Add a way to break out of a tight loop
> 	- Remove static unused function
> 	- Remove redundant spinlock already covered by watchdog core
> 	- Remove unecessary error handling when enabling DMT
> 	- Drop implementation of .get_timeleft
> 	- Drop calculation of max_timeout
> 	- Cleanup dev_err() message contents
> 	- Fix race condition with watchdog device register
> 	- Unregister watchdog before disabling clock
> 	- Fix typo in driver name
> ---
>   drivers/watchdog/Kconfig     |   13 +++
>   drivers/watchdog/Makefile    |    1 +
>   drivers/watchdog/pic32-dmt.c |  260 ++++++++++++++++++++++++++++++++++++++++++
>   3 files changed, 274 insertions(+)
>   create mode 100644 drivers/watchdog/pic32-dmt.c
>
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index 543fa81..53747e9 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -1427,6 +1427,19 @@ config PIC32_WDT
>   	  To compile this driver as a loadable module, choose M here.
>   	  The module will be called pic32-wdt.
>
> +config PIC32_DMT
> +	tristate "Microchip PIC32 Deadman Timer"
> +	select WATCHDOG_CORE
> +	depends on MACH_PIC32
> +	help
> +	  Watchdog driver for PIC32 instruction fetch counting timer. This specific
> +	  timer is typically be used in misson critical and safety critical
> +	  applications, where any single failure of the software functionality
> +	  and sequencing must be detected.
> +
> +	  To compile this driver as a loadable module, choose M here.
> +	  The module will be called pic32-dmt.
> +
>   # PARISC Architecture
>
>   # POWERPC Architecture
> diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
> index 244ed80..d051c9c 100644
> --- a/drivers/watchdog/Makefile
> +++ b/drivers/watchdog/Makefile
> @@ -154,6 +154,7 @@ obj-$(CONFIG_RALINK_WDT) += rt2880_wdt.o
>   obj-$(CONFIG_IMGPDC_WDT) += imgpdc_wdt.o
>   obj-$(CONFIG_MT7621_WDT) += mt7621_wdt.o
>   obj-$(CONFIG_PIC32_WDT) += pic32-wdt.o
> +obj-$(CONFIG_PIC32_DMT) += pic32-dmt.o
>
>   # PARISC Architecture
>
> diff --git a/drivers/watchdog/pic32-dmt.c b/drivers/watchdog/pic32-dmt.c
> new file mode 100644
> index 0000000..91801fa
> --- /dev/null
> +++ b/drivers/watchdog/pic32-dmt.c
> @@ -0,0 +1,260 @@
> +/*
> + * PIC32 deadman timer driver
> + *
> + * Purna Chandra Mandal <purna.mandal@microchip.com>
> + * Copyright (c) 2016, Microchip Technology Inc.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; either version
> + * 2 of the License, or (at your option) any later version.
> + */
> +
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +

No pr_ function left, so this define can be removed.

> +#include <linux/clk.h>
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm.h>
> +#include <linux/watchdog.h>
> +
> +#include <asm/mach-pic32/pic32.h>
> +
> +/* Deadman Timer Regs */
> +#define DMTCON_REG	0x00
> +#define DMTPRECLR_REG	0x10
> +#define DMTCLR_REG	0x20
> +#define DMTSTAT_REG	0x30
> +#define DMTCNT_REG	0x40
> +#define DMTPSCNT_REG	0x60
> +#define DMTPSINTV_REG	0x70
> +
> +/* Deadman Timer Regs fields */
> +#define DMT_ON			BIT(15)
> +#define DMT_STEP1_KEY		BIT(6)
> +#define DMT_STEP2_KEY		BIT(3)
> +#define DMTSTAT_WINOPN		BIT(0)
> +#define DMTSTAT_EVENT		BIT(5)
> +#define DMTSTAT_BAD2		BIT(6)
> +#define DMTSTAT_BAD1		BIT(7)
> +
> +/* Reset Control Register fields for watchdog */
> +#define RESETCON_DMT_TIMEOUT    BIT(5)
> +
> +struct pic32_dmt {
> +	void __iomem	*regs;
> +	struct clk	*clk;
> +};
> +
> +static inline void dmt_enable(struct pic32_dmt *dmt)
> +{
> +	writel(DMT_ON, PIC32_SET(dmt->regs + DMTCON_REG));
> +}
> +
> +static inline void dmt_disable(struct pic32_dmt *dmt)
> +{
> +	writel(DMT_ON, PIC32_CLR(dmt->regs + DMTCON_REG));
> +	/*
> +	 * Cannot touch registers in the CPU cycle following clearing the
> +	 * ON bit.
> +	 */
> +	nop();
> +}
> +
> +static inline int dmt_bad_status(struct pic32_dmt *dmt)
> +{
> +	u32 val;
> +
> +	val = readl(dmt->regs + DMTSTAT_REG);
> +	val &= (DMTSTAT_BAD1 | DMTSTAT_BAD2 | DMTSTAT_EVENT);
> +	if (val)
> +		return -EAGAIN;
> +
> +	return 0;
> +}
> +
> +static inline int dmt_keepalive(struct pic32_dmt *dmt)
> +{
> +	u32 v;
> +	u32 timeout = 500;
> +
> +	/* set pre-clear key */
> +	writel(DMT_STEP1_KEY << 8, dmt->regs + DMTPRECLR_REG);
> +
> +	/* wait for DMT window to open */
> +	while (--timeout) {
> +		v = readl(dmt->regs + DMTSTAT_REG) & DMTSTAT_WINOPN;
> +		if (v == DMTSTAT_WINOPN)
> +			break;
> +	}
> +
> +	/* apply key2 */
> +	writel(DMT_STEP2_KEY, dmt->regs + DMTCLR_REG);
> +
> +	/* check whether keys are latched correctly */
> +	return dmt_bad_status(dmt);
> +}
> +
> +static inline u32 pic32_dmt_get_timeout_secs(struct pic32_dmt *dmt)
> +{
> +	unsigned long rate;
> +
> +	rate = clk_get_rate(dmt->clk);
> +	if (rate)
> +		return readl(dmt->regs + DMTPSCNT_REG) / rate;
> +
> +	return -EINVAL;

Return value is unsigned, and the caller checks for 0 to identify an error,
so you need to return 0 here.

With that fixed, and the pr_fmt dropped, feel free to add

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

to the next revision of this patch.

Thanks,
Guenter

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

* Re: [PATCH v2 2/2] watchdog: pic32-dmt: Add PIC32 deadman timer driver
@ 2016-02-26 14:11   ` Guenter Roeck
  0 siblings, 0 replies; 7+ messages in thread
From: Guenter Roeck @ 2016-02-26 14:11 UTC (permalink / raw)
  To: Joshua Henderson, linux-kernel-u79uwXL29TY76Z2rM5mHXA
  Cc: linux-mips-6z/3iImG2C8G8FEW9MqTrA, ralf-6z/3iImG2C8G8FEW9MqTrA,
	Purna Chandra Mandal, Rob Herring, Pawel Moll, Mark Rutland,
	Ian Campbell, Kumar Gala, Wim Van Sebroeck,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-watchdog-u79uwXL29TY76Z2rM5mHXA

On 02/25/2016 10:30 AM, Joshua Henderson wrote:
> From: Purna Chandra Mandal <purna.mandal-UWL1GkI3JZL3oGB3hsPCZA@public.gmane.org>
>
> Adds support for the deadman timer peripheral found on PIC32 class devices.
>
> The primary function of the deadman timer (DMT) is to reset the processor
> in the event of a software malfunction. The DMT is a free-running
> instruction fetch timer, which is clocked whenever an instruction fetch
> occurs until a count match occurs. Instructions are not fetched when
> the processor is in sleep mode.
>
> Signed-off-by: Purna Chandra Mandal <purna.mandal-UWL1GkI3JZL3oGB3hsPCZA@public.gmane.org>
> Signed-off-by: Joshua Henderson <joshua.henderson-UWL1GkI3JZL3oGB3hsPCZA@public.gmane.org>
> Cc: Ralf Baechle <ralf-6z/3iImG2C8G8FEW9MqTrA@public.gmane.org>
> ---
> Note: Please merge this patch series through the MIPS tree.
>
> Changes since v1:
> 	- Drop default y in Kconfig
> 	- Alphabetical include files
> 	- Use BIT() where appropriate
> 	- Replace cpu_relax() with nop() and comment why it's needed
> 	- Return bool on functions where appropriate
> 	- Add a way to break out of a tight loop
> 	- Remove static unused function
> 	- Remove redundant spinlock already covered by watchdog core
> 	- Remove unecessary error handling when enabling DMT
> 	- Drop implementation of .get_timeleft
> 	- Drop calculation of max_timeout
> 	- Cleanup dev_err() message contents
> 	- Fix race condition with watchdog device register
> 	- Unregister watchdog before disabling clock
> 	- Fix typo in driver name
> ---
>   drivers/watchdog/Kconfig     |   13 +++
>   drivers/watchdog/Makefile    |    1 +
>   drivers/watchdog/pic32-dmt.c |  260 ++++++++++++++++++++++++++++++++++++++++++
>   3 files changed, 274 insertions(+)
>   create mode 100644 drivers/watchdog/pic32-dmt.c
>
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index 543fa81..53747e9 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -1427,6 +1427,19 @@ config PIC32_WDT
>   	  To compile this driver as a loadable module, choose M here.
>   	  The module will be called pic32-wdt.
>
> +config PIC32_DMT
> +	tristate "Microchip PIC32 Deadman Timer"
> +	select WATCHDOG_CORE
> +	depends on MACH_PIC32
> +	help
> +	  Watchdog driver for PIC32 instruction fetch counting timer. This specific
> +	  timer is typically be used in misson critical and safety critical
> +	  applications, where any single failure of the software functionality
> +	  and sequencing must be detected.
> +
> +	  To compile this driver as a loadable module, choose M here.
> +	  The module will be called pic32-dmt.
> +
>   # PARISC Architecture
>
>   # POWERPC Architecture
> diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
> index 244ed80..d051c9c 100644
> --- a/drivers/watchdog/Makefile
> +++ b/drivers/watchdog/Makefile
> @@ -154,6 +154,7 @@ obj-$(CONFIG_RALINK_WDT) += rt2880_wdt.o
>   obj-$(CONFIG_IMGPDC_WDT) += imgpdc_wdt.o
>   obj-$(CONFIG_MT7621_WDT) += mt7621_wdt.o
>   obj-$(CONFIG_PIC32_WDT) += pic32-wdt.o
> +obj-$(CONFIG_PIC32_DMT) += pic32-dmt.o
>
>   # PARISC Architecture
>
> diff --git a/drivers/watchdog/pic32-dmt.c b/drivers/watchdog/pic32-dmt.c
> new file mode 100644
> index 0000000..91801fa
> --- /dev/null
> +++ b/drivers/watchdog/pic32-dmt.c
> @@ -0,0 +1,260 @@
> +/*
> + * PIC32 deadman timer driver
> + *
> + * Purna Chandra Mandal <purna.mandal-UWL1GkI3JZL3oGB3hsPCZA@public.gmane.org>
> + * Copyright (c) 2016, Microchip Technology Inc.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; either version
> + * 2 of the License, or (at your option) any later version.
> + */
> +
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +

No pr_ function left, so this define can be removed.

> +#include <linux/clk.h>
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm.h>
> +#include <linux/watchdog.h>
> +
> +#include <asm/mach-pic32/pic32.h>
> +
> +/* Deadman Timer Regs */
> +#define DMTCON_REG	0x00
> +#define DMTPRECLR_REG	0x10
> +#define DMTCLR_REG	0x20
> +#define DMTSTAT_REG	0x30
> +#define DMTCNT_REG	0x40
> +#define DMTPSCNT_REG	0x60
> +#define DMTPSINTV_REG	0x70
> +
> +/* Deadman Timer Regs fields */
> +#define DMT_ON			BIT(15)
> +#define DMT_STEP1_KEY		BIT(6)
> +#define DMT_STEP2_KEY		BIT(3)
> +#define DMTSTAT_WINOPN		BIT(0)
> +#define DMTSTAT_EVENT		BIT(5)
> +#define DMTSTAT_BAD2		BIT(6)
> +#define DMTSTAT_BAD1		BIT(7)
> +
> +/* Reset Control Register fields for watchdog */
> +#define RESETCON_DMT_TIMEOUT    BIT(5)
> +
> +struct pic32_dmt {
> +	void __iomem	*regs;
> +	struct clk	*clk;
> +};
> +
> +static inline void dmt_enable(struct pic32_dmt *dmt)
> +{
> +	writel(DMT_ON, PIC32_SET(dmt->regs + DMTCON_REG));
> +}
> +
> +static inline void dmt_disable(struct pic32_dmt *dmt)
> +{
> +	writel(DMT_ON, PIC32_CLR(dmt->regs + DMTCON_REG));
> +	/*
> +	 * Cannot touch registers in the CPU cycle following clearing the
> +	 * ON bit.
> +	 */
> +	nop();
> +}
> +
> +static inline int dmt_bad_status(struct pic32_dmt *dmt)
> +{
> +	u32 val;
> +
> +	val = readl(dmt->regs + DMTSTAT_REG);
> +	val &= (DMTSTAT_BAD1 | DMTSTAT_BAD2 | DMTSTAT_EVENT);
> +	if (val)
> +		return -EAGAIN;
> +
> +	return 0;
> +}
> +
> +static inline int dmt_keepalive(struct pic32_dmt *dmt)
> +{
> +	u32 v;
> +	u32 timeout = 500;
> +
> +	/* set pre-clear key */
> +	writel(DMT_STEP1_KEY << 8, dmt->regs + DMTPRECLR_REG);
> +
> +	/* wait for DMT window to open */
> +	while (--timeout) {
> +		v = readl(dmt->regs + DMTSTAT_REG) & DMTSTAT_WINOPN;
> +		if (v == DMTSTAT_WINOPN)
> +			break;
> +	}
> +
> +	/* apply key2 */
> +	writel(DMT_STEP2_KEY, dmt->regs + DMTCLR_REG);
> +
> +	/* check whether keys are latched correctly */
> +	return dmt_bad_status(dmt);
> +}
> +
> +static inline u32 pic32_dmt_get_timeout_secs(struct pic32_dmt *dmt)
> +{
> +	unsigned long rate;
> +
> +	rate = clk_get_rate(dmt->clk);
> +	if (rate)
> +		return readl(dmt->regs + DMTPSCNT_REG) / rate;
> +
> +	return -EINVAL;

Return value is unsigned, and the caller checks for 0 to identify an error,
so you need to return 0 here.

With that fixed, and the pr_fmt dropped, feel free to add

Reviewed-by: Guenter Roeck <linux-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org>

to the next revision of this patch.

Thanks,
Guenter

--
To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCH v2 2/2] watchdog: pic32-dmt: Add PIC32 deadman timer driver
@ 2016-02-26 17:14     ` Joshua Henderson
  0 siblings, 0 replies; 7+ messages in thread
From: Joshua Henderson @ 2016-02-26 17:14 UTC (permalink / raw)
  To: Guenter Roeck, linux-kernel
  Cc: linux-mips, ralf, Purna Chandra Mandal, Rob Herring, Pawel Moll,
	Mark Rutland, Ian Campbell, Kumar Gala, Wim Van Sebroeck,
	devicetree, linux-watchdog

On 02/26/2016 07:11 AM, Guenter Roeck wrote:
> On 02/25/2016 10:30 AM, Joshua Henderson wrote:
>> From: Purna Chandra Mandal <purna.mandal@microchip.com>
>>
>> Adds support for the deadman timer peripheral found on PIC32 class devices.
>>
>> The primary function of the deadman timer (DMT) is to reset the processor
>> in the event of a software malfunction. The DMT is a free-running
>> instruction fetch timer, which is clocked whenever an instruction fetch
>> occurs until a count match occurs. Instructions are not fetched when
>> the processor is in sleep mode.
>>
>> Signed-off-by: Purna Chandra Mandal <purna.mandal@microchip.com>
>> Signed-off-by: Joshua Henderson <joshua.henderson@microchip.com>
>> Cc: Ralf Baechle <ralf@linux-mips.org>
>> ---
>> Note: Please merge this patch series through the MIPS tree.
>>
>> Changes since v1:
>>     - Drop default y in Kconfig
>>     - Alphabetical include files
>>     - Use BIT() where appropriate
>>     - Replace cpu_relax() with nop() and comment why it's needed
>>     - Return bool on functions where appropriate
>>     - Add a way to break out of a tight loop
>>     - Remove static unused function
>>     - Remove redundant spinlock already covered by watchdog core
>>     - Remove unecessary error handling when enabling DMT
>>     - Drop implementation of .get_timeleft
>>     - Drop calculation of max_timeout
>>     - Cleanup dev_err() message contents
>>     - Fix race condition with watchdog device register
>>     - Unregister watchdog before disabling clock
>>     - Fix typo in driver name
>> ---
>>   drivers/watchdog/Kconfig     |   13 +++
>>   drivers/watchdog/Makefile    |    1 +
>>   drivers/watchdog/pic32-dmt.c |  260 ++++++++++++++++++++++++++++++++++++++++++
>>   3 files changed, 274 insertions(+)
>>   create mode 100644 drivers/watchdog/pic32-dmt.c
>>
>> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
>> index 543fa81..53747e9 100644
>> --- a/drivers/watchdog/Kconfig
>> +++ b/drivers/watchdog/Kconfig
>> @@ -1427,6 +1427,19 @@ config PIC32_WDT
>>         To compile this driver as a loadable module, choose M here.
>>         The module will be called pic32-wdt.
>>
>> +config PIC32_DMT
>> +    tristate "Microchip PIC32 Deadman Timer"
>> +    select WATCHDOG_CORE
>> +    depends on MACH_PIC32
>> +    help
>> +      Watchdog driver for PIC32 instruction fetch counting timer. This specific
>> +      timer is typically be used in misson critical and safety critical
>> +      applications, where any single failure of the software functionality
>> +      and sequencing must be detected.
>> +
>> +      To compile this driver as a loadable module, choose M here.
>> +      The module will be called pic32-dmt.
>> +
>>   # PARISC Architecture
>>
>>   # POWERPC Architecture
>> diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
>> index 244ed80..d051c9c 100644
>> --- a/drivers/watchdog/Makefile
>> +++ b/drivers/watchdog/Makefile
>> @@ -154,6 +154,7 @@ obj-$(CONFIG_RALINK_WDT) += rt2880_wdt.o
>>   obj-$(CONFIG_IMGPDC_WDT) += imgpdc_wdt.o
>>   obj-$(CONFIG_MT7621_WDT) += mt7621_wdt.o
>>   obj-$(CONFIG_PIC32_WDT) += pic32-wdt.o
>> +obj-$(CONFIG_PIC32_DMT) += pic32-dmt.o
>>
>>   # PARISC Architecture
>>
>> diff --git a/drivers/watchdog/pic32-dmt.c b/drivers/watchdog/pic32-dmt.c
>> new file mode 100644
>> index 0000000..91801fa
>> --- /dev/null
>> +++ b/drivers/watchdog/pic32-dmt.c
>> @@ -0,0 +1,260 @@
>> +/*
>> + * PIC32 deadman timer driver
>> + *
>> + * Purna Chandra Mandal <purna.mandal@microchip.com>
>> + * Copyright (c) 2016, Microchip Technology Inc.
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public License
>> + * as published by the Free Software Foundation; either version
>> + * 2 of the License, or (at your option) any later version.
>> + */
>> +
>> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>> +
> 
> No pr_ function left, so this define can be removed.

Ack.

> 
>> +#include <linux/clk.h>
>> +#include <linux/device.h>
>> +#include <linux/err.h>
>> +#include <linux/io.h>
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/of.h>
>> +#include <linux/of_device.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/pm.h>
>> +#include <linux/watchdog.h>
>> +
>> +#include <asm/mach-pic32/pic32.h>
>> +
>> +/* Deadman Timer Regs */
>> +#define DMTCON_REG    0x00
>> +#define DMTPRECLR_REG    0x10
>> +#define DMTCLR_REG    0x20
>> +#define DMTSTAT_REG    0x30
>> +#define DMTCNT_REG    0x40
>> +#define DMTPSCNT_REG    0x60
>> +#define DMTPSINTV_REG    0x70
>> +
>> +/* Deadman Timer Regs fields */
>> +#define DMT_ON            BIT(15)
>> +#define DMT_STEP1_KEY        BIT(6)
>> +#define DMT_STEP2_KEY        BIT(3)
>> +#define DMTSTAT_WINOPN        BIT(0)
>> +#define DMTSTAT_EVENT        BIT(5)
>> +#define DMTSTAT_BAD2        BIT(6)
>> +#define DMTSTAT_BAD1        BIT(7)
>> +
>> +/* Reset Control Register fields for watchdog */
>> +#define RESETCON_DMT_TIMEOUT    BIT(5)
>> +
>> +struct pic32_dmt {
>> +    void __iomem    *regs;
>> +    struct clk    *clk;
>> +};
>> +
>> +static inline void dmt_enable(struct pic32_dmt *dmt)
>> +{
>> +    writel(DMT_ON, PIC32_SET(dmt->regs + DMTCON_REG));
>> +}
>> +
>> +static inline void dmt_disable(struct pic32_dmt *dmt)
>> +{
>> +    writel(DMT_ON, PIC32_CLR(dmt->regs + DMTCON_REG));
>> +    /*
>> +     * Cannot touch registers in the CPU cycle following clearing the
>> +     * ON bit.
>> +     */
>> +    nop();
>> +}
>> +
>> +static inline int dmt_bad_status(struct pic32_dmt *dmt)
>> +{
>> +    u32 val;
>> +
>> +    val = readl(dmt->regs + DMTSTAT_REG);
>> +    val &= (DMTSTAT_BAD1 | DMTSTAT_BAD2 | DMTSTAT_EVENT);
>> +    if (val)
>> +        return -EAGAIN;
>> +
>> +    return 0;
>> +}
>> +
>> +static inline int dmt_keepalive(struct pic32_dmt *dmt)
>> +{
>> +    u32 v;
>> +    u32 timeout = 500;
>> +
>> +    /* set pre-clear key */
>> +    writel(DMT_STEP1_KEY << 8, dmt->regs + DMTPRECLR_REG);
>> +
>> +    /* wait for DMT window to open */
>> +    while (--timeout) {
>> +        v = readl(dmt->regs + DMTSTAT_REG) & DMTSTAT_WINOPN;
>> +        if (v == DMTSTAT_WINOPN)
>> +            break;
>> +    }
>> +
>> +    /* apply key2 */
>> +    writel(DMT_STEP2_KEY, dmt->regs + DMTCLR_REG);
>> +
>> +    /* check whether keys are latched correctly */
>> +    return dmt_bad_status(dmt);
>> +}
>> +
>> +static inline u32 pic32_dmt_get_timeout_secs(struct pic32_dmt *dmt)
>> +{
>> +    unsigned long rate;
>> +
>> +    rate = clk_get_rate(dmt->clk);
>> +    if (rate)
>> +        return readl(dmt->regs + DMTPSCNT_REG) / rate;
>> +
>> +    return -EINVAL;
> 
> Return value is unsigned, and the caller checks for 0 to identify an error,
> so you need to return 0 here.
> 
> With that fixed, and the pr_fmt dropped, feel free to add
> 
> Reviewed-by: Guenter Roeck <linux@roeck-us.net>
> 
> to the next revision of this patch.

Ack.

Thanks,
Josh

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

* Re: [PATCH v2 2/2] watchdog: pic32-dmt: Add PIC32 deadman timer driver
@ 2016-02-26 17:14     ` Joshua Henderson
  0 siblings, 0 replies; 7+ messages in thread
From: Joshua Henderson @ 2016-02-26 17:14 UTC (permalink / raw)
  To: Guenter Roeck, linux-kernel-u79uwXL29TY76Z2rM5mHXA
  Cc: linux-mips-6z/3iImG2C8G8FEW9MqTrA, ralf-6z/3iImG2C8G8FEW9MqTrA,
	Purna Chandra Mandal, Rob Herring, Pawel Moll, Mark Rutland,
	Ian Campbell, Kumar Gala, Wim Van Sebroeck,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-watchdog-u79uwXL29TY76Z2rM5mHXA

On 02/26/2016 07:11 AM, Guenter Roeck wrote:
> On 02/25/2016 10:30 AM, Joshua Henderson wrote:
>> From: Purna Chandra Mandal <purna.mandal-UWL1GkI3JZL3oGB3hsPCZA@public.gmane.org>
>>
>> Adds support for the deadman timer peripheral found on PIC32 class devices.
>>
>> The primary function of the deadman timer (DMT) is to reset the processor
>> in the event of a software malfunction. The DMT is a free-running
>> instruction fetch timer, which is clocked whenever an instruction fetch
>> occurs until a count match occurs. Instructions are not fetched when
>> the processor is in sleep mode.
>>
>> Signed-off-by: Purna Chandra Mandal <purna.mandal-UWL1GkI3JZL3oGB3hsPCZA@public.gmane.org>
>> Signed-off-by: Joshua Henderson <joshua.henderson-UWL1GkI3JZL3oGB3hsPCZA@public.gmane.org>
>> Cc: Ralf Baechle <ralf-6z/3iImG2C8G8FEW9MqTrA@public.gmane.org>
>> ---
>> Note: Please merge this patch series through the MIPS tree.
>>
>> Changes since v1:
>>     - Drop default y in Kconfig
>>     - Alphabetical include files
>>     - Use BIT() where appropriate
>>     - Replace cpu_relax() with nop() and comment why it's needed
>>     - Return bool on functions where appropriate
>>     - Add a way to break out of a tight loop
>>     - Remove static unused function
>>     - Remove redundant spinlock already covered by watchdog core
>>     - Remove unecessary error handling when enabling DMT
>>     - Drop implementation of .get_timeleft
>>     - Drop calculation of max_timeout
>>     - Cleanup dev_err() message contents
>>     - Fix race condition with watchdog device register
>>     - Unregister watchdog before disabling clock
>>     - Fix typo in driver name
>> ---
>>   drivers/watchdog/Kconfig     |   13 +++
>>   drivers/watchdog/Makefile    |    1 +
>>   drivers/watchdog/pic32-dmt.c |  260 ++++++++++++++++++++++++++++++++++++++++++
>>   3 files changed, 274 insertions(+)
>>   create mode 100644 drivers/watchdog/pic32-dmt.c
>>
>> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
>> index 543fa81..53747e9 100644
>> --- a/drivers/watchdog/Kconfig
>> +++ b/drivers/watchdog/Kconfig
>> @@ -1427,6 +1427,19 @@ config PIC32_WDT
>>         To compile this driver as a loadable module, choose M here.
>>         The module will be called pic32-wdt.
>>
>> +config PIC32_DMT
>> +    tristate "Microchip PIC32 Deadman Timer"
>> +    select WATCHDOG_CORE
>> +    depends on MACH_PIC32
>> +    help
>> +      Watchdog driver for PIC32 instruction fetch counting timer. This specific
>> +      timer is typically be used in misson critical and safety critical
>> +      applications, where any single failure of the software functionality
>> +      and sequencing must be detected.
>> +
>> +      To compile this driver as a loadable module, choose M here.
>> +      The module will be called pic32-dmt.
>> +
>>   # PARISC Architecture
>>
>>   # POWERPC Architecture
>> diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
>> index 244ed80..d051c9c 100644
>> --- a/drivers/watchdog/Makefile
>> +++ b/drivers/watchdog/Makefile
>> @@ -154,6 +154,7 @@ obj-$(CONFIG_RALINK_WDT) += rt2880_wdt.o
>>   obj-$(CONFIG_IMGPDC_WDT) += imgpdc_wdt.o
>>   obj-$(CONFIG_MT7621_WDT) += mt7621_wdt.o
>>   obj-$(CONFIG_PIC32_WDT) += pic32-wdt.o
>> +obj-$(CONFIG_PIC32_DMT) += pic32-dmt.o
>>
>>   # PARISC Architecture
>>
>> diff --git a/drivers/watchdog/pic32-dmt.c b/drivers/watchdog/pic32-dmt.c
>> new file mode 100644
>> index 0000000..91801fa
>> --- /dev/null
>> +++ b/drivers/watchdog/pic32-dmt.c
>> @@ -0,0 +1,260 @@
>> +/*
>> + * PIC32 deadman timer driver
>> + *
>> + * Purna Chandra Mandal <purna.mandal-UWL1GkI3JZL3oGB3hsPCZA@public.gmane.org>
>> + * Copyright (c) 2016, Microchip Technology Inc.
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public License
>> + * as published by the Free Software Foundation; either version
>> + * 2 of the License, or (at your option) any later version.
>> + */
>> +
>> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>> +
> 
> No pr_ function left, so this define can be removed.

Ack.

> 
>> +#include <linux/clk.h>
>> +#include <linux/device.h>
>> +#include <linux/err.h>
>> +#include <linux/io.h>
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/of.h>
>> +#include <linux/of_device.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/pm.h>
>> +#include <linux/watchdog.h>
>> +
>> +#include <asm/mach-pic32/pic32.h>
>> +
>> +/* Deadman Timer Regs */
>> +#define DMTCON_REG    0x00
>> +#define DMTPRECLR_REG    0x10
>> +#define DMTCLR_REG    0x20
>> +#define DMTSTAT_REG    0x30
>> +#define DMTCNT_REG    0x40
>> +#define DMTPSCNT_REG    0x60
>> +#define DMTPSINTV_REG    0x70
>> +
>> +/* Deadman Timer Regs fields */
>> +#define DMT_ON            BIT(15)
>> +#define DMT_STEP1_KEY        BIT(6)
>> +#define DMT_STEP2_KEY        BIT(3)
>> +#define DMTSTAT_WINOPN        BIT(0)
>> +#define DMTSTAT_EVENT        BIT(5)
>> +#define DMTSTAT_BAD2        BIT(6)
>> +#define DMTSTAT_BAD1        BIT(7)
>> +
>> +/* Reset Control Register fields for watchdog */
>> +#define RESETCON_DMT_TIMEOUT    BIT(5)
>> +
>> +struct pic32_dmt {
>> +    void __iomem    *regs;
>> +    struct clk    *clk;
>> +};
>> +
>> +static inline void dmt_enable(struct pic32_dmt *dmt)
>> +{
>> +    writel(DMT_ON, PIC32_SET(dmt->regs + DMTCON_REG));
>> +}
>> +
>> +static inline void dmt_disable(struct pic32_dmt *dmt)
>> +{
>> +    writel(DMT_ON, PIC32_CLR(dmt->regs + DMTCON_REG));
>> +    /*
>> +     * Cannot touch registers in the CPU cycle following clearing the
>> +     * ON bit.
>> +     */
>> +    nop();
>> +}
>> +
>> +static inline int dmt_bad_status(struct pic32_dmt *dmt)
>> +{
>> +    u32 val;
>> +
>> +    val = readl(dmt->regs + DMTSTAT_REG);
>> +    val &= (DMTSTAT_BAD1 | DMTSTAT_BAD2 | DMTSTAT_EVENT);
>> +    if (val)
>> +        return -EAGAIN;
>> +
>> +    return 0;
>> +}
>> +
>> +static inline int dmt_keepalive(struct pic32_dmt *dmt)
>> +{
>> +    u32 v;
>> +    u32 timeout = 500;
>> +
>> +    /* set pre-clear key */
>> +    writel(DMT_STEP1_KEY << 8, dmt->regs + DMTPRECLR_REG);
>> +
>> +    /* wait for DMT window to open */
>> +    while (--timeout) {
>> +        v = readl(dmt->regs + DMTSTAT_REG) & DMTSTAT_WINOPN;
>> +        if (v == DMTSTAT_WINOPN)
>> +            break;
>> +    }
>> +
>> +    /* apply key2 */
>> +    writel(DMT_STEP2_KEY, dmt->regs + DMTCLR_REG);
>> +
>> +    /* check whether keys are latched correctly */
>> +    return dmt_bad_status(dmt);
>> +}
>> +
>> +static inline u32 pic32_dmt_get_timeout_secs(struct pic32_dmt *dmt)
>> +{
>> +    unsigned long rate;
>> +
>> +    rate = clk_get_rate(dmt->clk);
>> +    if (rate)
>> +        return readl(dmt->regs + DMTPSCNT_REG) / rate;
>> +
>> +    return -EINVAL;
> 
> Return value is unsigned, and the caller checks for 0 to identify an error,
> so you need to return 0 here.
> 
> With that fixed, and the pr_fmt dropped, feel free to add
> 
> Reviewed-by: Guenter Roeck <linux-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org>
> 
> to the next revision of this patch.

Ack.

Thanks,
Josh
--
To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCH v2 2/2] watchdog: pic32-dmt: Add PIC32 deadman timer driver
@ 2016-02-26 17:14     ` Joshua Henderson
  0 siblings, 0 replies; 7+ messages in thread
From: Joshua Henderson @ 2016-02-26 17:14 UTC (permalink / raw)
  To: Guenter Roeck, linux-kernel
  Cc: linux-mips, ralf, Purna Chandra Mandal, Rob Herring, Pawel Moll,
	Mark Rutland, Ian Campbell, Kumar Gala, Wim Van Sebroeck,
	devicetree, linux-watchdog

On 02/26/2016 07:11 AM, Guenter Roeck wrote:
> On 02/25/2016 10:30 AM, Joshua Henderson wrote:
>> From: Purna Chandra Mandal <purna.mandal@microchip.com>
>>
>> Adds support for the deadman timer peripheral found on PIC32 class devices.
>>
>> The primary function of the deadman timer (DMT) is to reset the processor
>> in the event of a software malfunction. The DMT is a free-running
>> instruction fetch timer, which is clocked whenever an instruction fetch
>> occurs until a count match occurs. Instructions are not fetched when
>> the processor is in sleep mode.
>>
>> Signed-off-by: Purna Chandra Mandal <purna.mandal@microchip.com>
>> Signed-off-by: Joshua Henderson <joshua.henderson@microchip.com>
>> Cc: Ralf Baechle <ralf@linux-mips.org>
>> ---
>> Note: Please merge this patch series through the MIPS tree.
>>
>> Changes since v1:
>>     - Drop default y in Kconfig
>>     - Alphabetical include files
>>     - Use BIT() where appropriate
>>     - Replace cpu_relax() with nop() and comment why it's needed
>>     - Return bool on functions where appropriate
>>     - Add a way to break out of a tight loop
>>     - Remove static unused function
>>     - Remove redundant spinlock already covered by watchdog core
>>     - Remove unecessary error handling when enabling DMT
>>     - Drop implementation of .get_timeleft
>>     - Drop calculation of max_timeout
>>     - Cleanup dev_err() message contents
>>     - Fix race condition with watchdog device register
>>     - Unregister watchdog before disabling clock
>>     - Fix typo in driver name
>> ---
>>   drivers/watchdog/Kconfig     |   13 +++
>>   drivers/watchdog/Makefile    |    1 +
>>   drivers/watchdog/pic32-dmt.c |  260 ++++++++++++++++++++++++++++++++++++++++++
>>   3 files changed, 274 insertions(+)
>>   create mode 100644 drivers/watchdog/pic32-dmt.c
>>
>> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
>> index 543fa81..53747e9 100644
>> --- a/drivers/watchdog/Kconfig
>> +++ b/drivers/watchdog/Kconfig
>> @@ -1427,6 +1427,19 @@ config PIC32_WDT
>>         To compile this driver as a loadable module, choose M here.
>>         The module will be called pic32-wdt.
>>
>> +config PIC32_DMT
>> +    tristate "Microchip PIC32 Deadman Timer"
>> +    select WATCHDOG_CORE
>> +    depends on MACH_PIC32
>> +    help
>> +      Watchdog driver for PIC32 instruction fetch counting timer. This specific
>> +      timer is typically be used in misson critical and safety critical
>> +      applications, where any single failure of the software functionality
>> +      and sequencing must be detected.
>> +
>> +      To compile this driver as a loadable module, choose M here.
>> +      The module will be called pic32-dmt.
>> +
>>   # PARISC Architecture
>>
>>   # POWERPC Architecture
>> diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
>> index 244ed80..d051c9c 100644
>> --- a/drivers/watchdog/Makefile
>> +++ b/drivers/watchdog/Makefile
>> @@ -154,6 +154,7 @@ obj-$(CONFIG_RALINK_WDT) += rt2880_wdt.o
>>   obj-$(CONFIG_IMGPDC_WDT) += imgpdc_wdt.o
>>   obj-$(CONFIG_MT7621_WDT) += mt7621_wdt.o
>>   obj-$(CONFIG_PIC32_WDT) += pic32-wdt.o
>> +obj-$(CONFIG_PIC32_DMT) += pic32-dmt.o
>>
>>   # PARISC Architecture
>>
>> diff --git a/drivers/watchdog/pic32-dmt.c b/drivers/watchdog/pic32-dmt.c
>> new file mode 100644
>> index 0000000..91801fa
>> --- /dev/null
>> +++ b/drivers/watchdog/pic32-dmt.c
>> @@ -0,0 +1,260 @@
>> +/*
>> + * PIC32 deadman timer driver
>> + *
>> + * Purna Chandra Mandal <purna.mandal@microchip.com>
>> + * Copyright (c) 2016, Microchip Technology Inc.
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public License
>> + * as published by the Free Software Foundation; either version
>> + * 2 of the License, or (at your option) any later version.
>> + */
>> +
>> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>> +
> 
> No pr_ function left, so this define can be removed.

Ack.

> 
>> +#include <linux/clk.h>
>> +#include <linux/device.h>
>> +#include <linux/err.h>
>> +#include <linux/io.h>
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/of.h>
>> +#include <linux/of_device.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/pm.h>
>> +#include <linux/watchdog.h>
>> +
>> +#include <asm/mach-pic32/pic32.h>
>> +
>> +/* Deadman Timer Regs */
>> +#define DMTCON_REG    0x00
>> +#define DMTPRECLR_REG    0x10
>> +#define DMTCLR_REG    0x20
>> +#define DMTSTAT_REG    0x30
>> +#define DMTCNT_REG    0x40
>> +#define DMTPSCNT_REG    0x60
>> +#define DMTPSINTV_REG    0x70
>> +
>> +/* Deadman Timer Regs fields */
>> +#define DMT_ON            BIT(15)
>> +#define DMT_STEP1_KEY        BIT(6)
>> +#define DMT_STEP2_KEY        BIT(3)
>> +#define DMTSTAT_WINOPN        BIT(0)
>> +#define DMTSTAT_EVENT        BIT(5)
>> +#define DMTSTAT_BAD2        BIT(6)
>> +#define DMTSTAT_BAD1        BIT(7)
>> +
>> +/* Reset Control Register fields for watchdog */
>> +#define RESETCON_DMT_TIMEOUT    BIT(5)
>> +
>> +struct pic32_dmt {
>> +    void __iomem    *regs;
>> +    struct clk    *clk;
>> +};
>> +
>> +static inline void dmt_enable(struct pic32_dmt *dmt)
>> +{
>> +    writel(DMT_ON, PIC32_SET(dmt->regs + DMTCON_REG));
>> +}
>> +
>> +static inline void dmt_disable(struct pic32_dmt *dmt)
>> +{
>> +    writel(DMT_ON, PIC32_CLR(dmt->regs + DMTCON_REG));
>> +    /*
>> +     * Cannot touch registers in the CPU cycle following clearing the
>> +     * ON bit.
>> +     */
>> +    nop();
>> +}
>> +
>> +static inline int dmt_bad_status(struct pic32_dmt *dmt)
>> +{
>> +    u32 val;
>> +
>> +    val = readl(dmt->regs + DMTSTAT_REG);
>> +    val &= (DMTSTAT_BAD1 | DMTSTAT_BAD2 | DMTSTAT_EVENT);
>> +    if (val)
>> +        return -EAGAIN;
>> +
>> +    return 0;
>> +}
>> +
>> +static inline int dmt_keepalive(struct pic32_dmt *dmt)
>> +{
>> +    u32 v;
>> +    u32 timeout = 500;
>> +
>> +    /* set pre-clear key */
>> +    writel(DMT_STEP1_KEY << 8, dmt->regs + DMTPRECLR_REG);
>> +
>> +    /* wait for DMT window to open */
>> +    while (--timeout) {
>> +        v = readl(dmt->regs + DMTSTAT_REG) & DMTSTAT_WINOPN;
>> +        if (v == DMTSTAT_WINOPN)
>> +            break;
>> +    }
>> +
>> +    /* apply key2 */
>> +    writel(DMT_STEP2_KEY, dmt->regs + DMTCLR_REG);
>> +
>> +    /* check whether keys are latched correctly */
>> +    return dmt_bad_status(dmt);
>> +}
>> +
>> +static inline u32 pic32_dmt_get_timeout_secs(struct pic32_dmt *dmt)
>> +{
>> +    unsigned long rate;
>> +
>> +    rate = clk_get_rate(dmt->clk);
>> +    if (rate)
>> +        return readl(dmt->regs + DMTPSCNT_REG) / rate;
>> +
>> +    return -EINVAL;
> 
> Return value is unsigned, and the caller checks for 0 to identify an error,
> so you need to return 0 here.
> 
> With that fixed, and the pr_fmt dropped, feel free to add
> 
> Reviewed-by: Guenter Roeck <linux@roeck-us.net>
> 
> to the next revision of this patch.

Ack.

Thanks,
Josh

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

end of thread, other threads:[~2016-02-26 17:14 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-25 18:30 [PATCH v2 2/2] watchdog: pic32-dmt: Add PIC32 deadman timer driver Joshua Henderson
2016-02-25 18:30 ` Joshua Henderson
2016-02-26 14:11 ` Guenter Roeck
2016-02-26 14:11   ` Guenter Roeck
2016-02-26 17:14   ` Joshua Henderson
2016-02-26 17:14     ` Joshua Henderson
2016-02-26 17:14     ` Joshua Henderson

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