All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH V2 0/3] add UniPhier watchdog support
@ 2017-06-06  9:11 ` Keiji Hayashibara
  0 siblings, 0 replies; 18+ messages in thread
From: Keiji Hayashibara @ 2017-06-06  9:11 UTC (permalink / raw)
  To: wim, linux
  Cc: linux-watchdog, linux-kernel, robh+dt, devicetree,
	masami.hiramatsu, jaswinder.singh, yamada.masahiro,
	hayashi.kunihiko, owada.kiyoshi, Keiji Hayashibara

This series adds support for watchdog timer implemented on UniPhier LD11 and
LD20 SoCs. This driver supports watchdog and system reset for SoCs.

patch V1
http://www.spinics.net/lists/linux-watchdog/msg11782.html

Changes between V2 and V1
=========================
1. Add barrier code in uniphier_watchdog_ping() and __uniphier_watchdog_start().

 When writing WDTCTRL_CLEAR to WDTCTRL register, WDTCTRL_STATUS is updated
 about 62.5usec(2cycle of 32kHz) later. However, if WDTCTRL_CLEAR is written
 before WDTCTRL_STATUS is updated, it can't reboot as SoC specification.
 Therefore, I added barrier code, it waits until WDTCTRL_STATUS is updated.
   
2. Fix issues according to review comments.

 * Use WDOG_STOP_ON_REBOOT instead of shutdown method of struct platform_driver.
 * Separate uniphier-wdt bindings documentation as a patch.
 * Remove delay.h and add bitops.h header file.
 * Use devm_watchdog_register_device().
 * Remove unnecessary code.

Keiji Hayashibara (3):
  devicetree: add UniPhier WDT devicetree bindings documentation
  watchdog: uniphier: add UniPhier watchdog driver
  arm64: dts: uniphier: add watchdog node for LD11 and LD20

 .../devicetree/bindings/watchdog/uniphier-wdt.txt  |  20 ++
 Documentation/watchdog/watchdog-parameters.txt     |   6 +
 arch/arm64/boot/dts/socionext/uniphier-ld11.dtsi   |   4 +
 arch/arm64/boot/dts/socionext/uniphier-ld20.dtsi   |   4 +
 drivers/watchdog/Kconfig                           |  11 +
 drivers/watchdog/Makefile                          |   1 +
 drivers/watchdog/uniphier_wdt.c                    | 275 +++++++++++++++++++++
 7 files changed, 321 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/watchdog/uniphier-wdt.txt
 create mode 100644 drivers/watchdog/uniphier_wdt.c

-- 
2.7.4

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

* [PATCH V2 0/3] add UniPhier watchdog support
@ 2017-06-06  9:11 ` Keiji Hayashibara
  0 siblings, 0 replies; 18+ messages in thread
From: Keiji Hayashibara @ 2017-06-06  9:11 UTC (permalink / raw)
  To: wim-IQzOog9fTRqzQB+pC5nmwQ, linux-0h96xk9xTtrk1uMJSBkQmQ
  Cc: linux-watchdog-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	masami.hiramatsu-QSEj5FYQhm4dnm+yROfE0A,
	jaswinder.singh-QSEj5FYQhm4dnm+yROfE0A,
	yamada.masahiro-uWyLwvC0a2jby3iVrkZq2A,
	hayashi.kunihiko-uWyLwvC0a2jby3iVrkZq2A,
	owada.kiyoshi-uWyLwvC0a2jby3iVrkZq2A, Keiji Hayashibara

This series adds support for watchdog timer implemented on UniPhier LD11 and
LD20 SoCs. This driver supports watchdog and system reset for SoCs.

patch V1
http://www.spinics.net/lists/linux-watchdog/msg11782.html

Changes between V2 and V1
=========================
1. Add barrier code in uniphier_watchdog_ping() and __uniphier_watchdog_start().

 When writing WDTCTRL_CLEAR to WDTCTRL register, WDTCTRL_STATUS is updated
 about 62.5usec(2cycle of 32kHz) later. However, if WDTCTRL_CLEAR is written
 before WDTCTRL_STATUS is updated, it can't reboot as SoC specification.
 Therefore, I added barrier code, it waits until WDTCTRL_STATUS is updated.
   
2. Fix issues according to review comments.

 * Use WDOG_STOP_ON_REBOOT instead of shutdown method of struct platform_driver.
 * Separate uniphier-wdt bindings documentation as a patch.
 * Remove delay.h and add bitops.h header file.
 * Use devm_watchdog_register_device().
 * Remove unnecessary code.

Keiji Hayashibara (3):
  devicetree: add UniPhier WDT devicetree bindings documentation
  watchdog: uniphier: add UniPhier watchdog driver
  arm64: dts: uniphier: add watchdog node for LD11 and LD20

 .../devicetree/bindings/watchdog/uniphier-wdt.txt  |  20 ++
 Documentation/watchdog/watchdog-parameters.txt     |   6 +
 arch/arm64/boot/dts/socionext/uniphier-ld11.dtsi   |   4 +
 arch/arm64/boot/dts/socionext/uniphier-ld20.dtsi   |   4 +
 drivers/watchdog/Kconfig                           |  11 +
 drivers/watchdog/Makefile                          |   1 +
 drivers/watchdog/uniphier_wdt.c                    | 275 +++++++++++++++++++++
 7 files changed, 321 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/watchdog/uniphier-wdt.txt
 create mode 100644 drivers/watchdog/uniphier_wdt.c

-- 
2.7.4

--
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] 18+ messages in thread

* [PATCH V2 1/3] devicetree: add UniPhier WDT devicetree bindings documentation
@ 2017-06-06  9:11   ` Keiji Hayashibara
  0 siblings, 0 replies; 18+ messages in thread
From: Keiji Hayashibara @ 2017-06-06  9:11 UTC (permalink / raw)
  To: wim, linux
  Cc: linux-watchdog, linux-kernel, robh+dt, devicetree,
	masami.hiramatsu, jaswinder.singh, yamada.masahiro,
	hayashi.kunihiko, owada.kiyoshi, Keiji Hayashibara

Add uniphier-wdt bindings documentation.

Signed-off-by: Keiji Hayashibara <hayashibara.keiji@socionext.com>
---
 .../devicetree/bindings/watchdog/uniphier-wdt.txt    | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/watchdog/uniphier-wdt.txt

diff --git a/Documentation/devicetree/bindings/watchdog/uniphier-wdt.txt b/Documentation/devicetree/bindings/watchdog/uniphier-wdt.txt
new file mode 100644
index 0000000..a59d1ba
--- /dev/null
+++ b/Documentation/devicetree/bindings/watchdog/uniphier-wdt.txt
@@ -0,0 +1,20 @@
+UniPhier watchdog timer controller
+
+This UniPhier watchdog timer controller must be under sysctrl compatible node.
+
+Required properties:
+- compatible: should be "socionext,uniphier-wdt"
+
+Example:
+
+	sysctrl@61840000 {
+		compatible = "socionext,uniphier-ld11-sysctrl",
+			     "simple-mfd", "syscon";
+		reg = <0x61840000 0x4000>;
+
+		sys_wdt: watchdog {
+			compatible = "socionext,uniphier-wdt";
+		}
+
+		other nodes ...
+	};
-- 
2.7.4

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

* [PATCH V2 1/3] devicetree: add UniPhier WDT devicetree bindings documentation
@ 2017-06-06  9:11   ` Keiji Hayashibara
  0 siblings, 0 replies; 18+ messages in thread
From: Keiji Hayashibara @ 2017-06-06  9:11 UTC (permalink / raw)
  To: wim-IQzOog9fTRqzQB+pC5nmwQ, linux-0h96xk9xTtrk1uMJSBkQmQ
  Cc: linux-watchdog-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	masami.hiramatsu-QSEj5FYQhm4dnm+yROfE0A,
	jaswinder.singh-QSEj5FYQhm4dnm+yROfE0A,
	yamada.masahiro-uWyLwvC0a2jby3iVrkZq2A,
	hayashi.kunihiko-uWyLwvC0a2jby3iVrkZq2A,
	owada.kiyoshi-uWyLwvC0a2jby3iVrkZq2A, Keiji Hayashibara

Add uniphier-wdt bindings documentation.

Signed-off-by: Keiji Hayashibara <hayashibara.keiji-uWyLwvC0a2jby3iVrkZq2A@public.gmane.org>
---
 .../devicetree/bindings/watchdog/uniphier-wdt.txt    | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/watchdog/uniphier-wdt.txt

diff --git a/Documentation/devicetree/bindings/watchdog/uniphier-wdt.txt b/Documentation/devicetree/bindings/watchdog/uniphier-wdt.txt
new file mode 100644
index 0000000..a59d1ba
--- /dev/null
+++ b/Documentation/devicetree/bindings/watchdog/uniphier-wdt.txt
@@ -0,0 +1,20 @@
+UniPhier watchdog timer controller
+
+This UniPhier watchdog timer controller must be under sysctrl compatible node.
+
+Required properties:
+- compatible: should be "socionext,uniphier-wdt"
+
+Example:
+
+	sysctrl@61840000 {
+		compatible = "socionext,uniphier-ld11-sysctrl",
+			     "simple-mfd", "syscon";
+		reg = <0x61840000 0x4000>;
+
+		sys_wdt: watchdog {
+			compatible = "socionext,uniphier-wdt";
+		}
+
+		other nodes ...
+	};
-- 
2.7.4

--
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 related	[flat|nested] 18+ messages in thread

* [PATCH V2 2/3] watchdog: uniphier: add UniPhier watchdog driver
  2017-06-06  9:11 ` Keiji Hayashibara
  (?)
  (?)
@ 2017-06-06  9:11 ` Keiji Hayashibara
  2017-06-08  2:09     ` Masahiro Yamada
  2017-06-08 12:31     ` Guenter Roeck
  -1 siblings, 2 replies; 18+ messages in thread
From: Keiji Hayashibara @ 2017-06-06  9:11 UTC (permalink / raw)
  To: wim, linux
  Cc: linux-watchdog, linux-kernel, robh+dt, devicetree,
	masami.hiramatsu, jaswinder.singh, yamada.masahiro,
	hayashi.kunihiko, owada.kiyoshi, Keiji Hayashibara

Add a watchdog driver for Socionext UniPhier series SoC.
Note that the timeout value for this device must be a power
of 2 because of the specification.

Signed-off-by: Keiji Hayashibara <hayashibara.keiji@socionext.com>
---
 Documentation/watchdog/watchdog-parameters.txt |   6 +
 drivers/watchdog/Kconfig                       |  11 +
 drivers/watchdog/Makefile                      |   1 +
 drivers/watchdog/uniphier_wdt.c                | 275 +++++++++++++++++++++++++
 4 files changed, 293 insertions(+)
 create mode 100644 drivers/watchdog/uniphier_wdt.c

diff --git a/Documentation/watchdog/watchdog-parameters.txt b/Documentation/watchdog/watchdog-parameters.txt
index 4f7d86d..6f9d7b4 100644
--- a/Documentation/watchdog/watchdog-parameters.txt
+++ b/Documentation/watchdog/watchdog-parameters.txt
@@ -369,6 +369,12 @@ timeout: Watchdog timeout in seconds. (0<timeout<N, default=60)
 nowayout: Watchdog cannot be stopped once started
 	(default=kernel config parameter)
 -------------------------------------------------
+uniphier_wdt:
+timeout: Watchdog timeout in power of two seconds.
+	(1 <= timeout <= 128, default=64)
+nowayout: Watchdog cannot be stopped once started
+	(default=kernel config parameter)
+-------------------------------------------------
 w83627hf_wdt:
 wdt_io: w83627hf/thf WDT io port (default 0x2E)
 timeout: Watchdog timeout in seconds. 1 <= timeout <= 255, default=60.
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 52a70ee..2a9d9a6 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -744,6 +744,17 @@ config ZX2967_WATCHDOG
 	  To compile this driver as a module, choose M here: the
 	  module will be called zx2967_wdt.
 
+config UNIPHIER_WATCHDOG
+	tristate "UniPhier watchdog support"
+	depends on ARCH_UNIPHIER || COMPILE_TEST
+	select WATCHDOG_CORE
+	help
+	  Say Y here to include support watchdog timer embedded
+	  into the UniPhier system.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called uniphier_wdt.
+
 # AVR32 Architecture
 
 config AT32AP700X_WDT
diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
index a2126e2..0928dac 100644
--- a/drivers/watchdog/Makefile
+++ b/drivers/watchdog/Makefile
@@ -84,6 +84,7 @@ obj-$(CONFIG_ATLAS7_WATCHDOG) += atlas7_wdt.o
 obj-$(CONFIG_RENESAS_WDT) += renesas_wdt.o
 obj-$(CONFIG_ASPEED_WATCHDOG) += aspeed_wdt.o
 obj-$(CONFIG_ZX2967_WATCHDOG) += zx2967_wdt.o
+obj-$(CONFIG_UNIPHIER_WATCHDOG) += uniphier_wdt.o
 
 # AVR32 Architecture
 obj-$(CONFIG_AT32AP700X_WDT) += at32ap700x_wdt.o
diff --git a/drivers/watchdog/uniphier_wdt.c b/drivers/watchdog/uniphier_wdt.c
new file mode 100644
index 0000000..75862a0
--- /dev/null
+++ b/drivers/watchdog/uniphier_wdt.c
@@ -0,0 +1,275 @@
+/*
+ * Watchdog driver for the UniPhier watchdog timer
+ *
+ * (c) Copyright 2014 Panasonic Corporation
+ * (c) Copyright 2016 Socionext Inc.
+ * All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/module.h>
+#include <linux/mfd/syscon.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/watchdog.h>
+#include <linux/bitops.h>
+
+/* WDT timer setting register */
+#define WDTTIMSET			0x3004
+#define   WDTTIMSET_PERIOD_MASK		(0xf << 0)
+#define   WDTTIMSET_PERIOD_1_SEC	(0x3 << 0)
+#define   WDTTIMSET_PERIOD_2_SEC	(0x4 << 0)
+#define   WDTTIMSET_PERIOD_4_SEC	(0x5 << 0)
+#define   WDTTIMSET_PERIOD_8_SEC	(0x6 << 0)
+#define   WDTTIMSET_PERIOD_16_SEC	(0x7 << 0)
+#define   WDTTIMSET_PERIOD_32_SEC	(0x8 << 0)
+#define   WDTTIMSET_PERIOD_64_SEC	(0x9 << 0)
+#define   WDTTIMSET_PERIOD_128_SEC	(0xa << 0)
+
+/* WDT reset selection register */
+#define WDTRSTSEL			0x3008
+#define   WDTRSTSEL_RSTSEL_MASK		(0x3 << 0)
+#define   WDTRSTSEL_RSTSEL_BOTH		(0x0 << 0)
+#define   WDTRSTSEL_RSTSEL_IRQ_ONLY	(0x2 << 0)
+
+/* WDT control register */
+#define WDTCTRL				0x300c
+#define   WDTCTRL_STATUS		BIT(8)
+#define   WDTCTRL_CLEAR			BIT(1)
+#define   WDTCTRL_ENABLE		BIT(0)
+
+#define SEC_TO_WDTTIMSET_PRD(sec) \
+		(ilog2(sec) + WDTTIMSET_PERIOD_1_SEC)
+
+#define WDTST_TIMEOUT			1000 /* usec */
+
+#define WDT_DEFAULT_TIMEOUT		64   /* Default is 64 seconds */
+#define WDT_PERIOD_MIN			1
+#define WDT_PERIOD_MAX			128
+
+static unsigned int timeout = WDT_DEFAULT_TIMEOUT;
+static bool nowayout = WATCHDOG_NOWAYOUT;
+
+struct uniphier_wdt_dev {
+	struct watchdog_device wdt_dev;
+	struct regmap	*regmap;
+};
+
+/*
+ * UniPhier Watchdog operations
+ */
+static int uniphier_watchdog_ping(struct watchdog_device *w)
+{
+	struct uniphier_wdt_dev *wdev = watchdog_get_drvdata(w);
+	unsigned int val;
+	int ret;
+
+	/* Clear counter */
+	ret = regmap_write_bits(wdev->regmap, WDTCTRL,
+				WDTCTRL_CLEAR, WDTCTRL_CLEAR);
+	if (!ret)
+		/*
+		 * As SoC specification, after clear counter,
+		 * it needs to wait until counter status is 1.
+		 */
+		ret = regmap_read_poll_timeout(wdev->regmap, WDTCTRL, val,
+					       (val & WDTCTRL_STATUS),
+					       0, WDTST_TIMEOUT);
+
+	return ret;
+}
+
+static int __uniphier_watchdog_start(struct regmap *regmap, unsigned int sec)
+{
+	unsigned int val;
+	int ret;
+
+	ret = regmap_read_poll_timeout(regmap, WDTCTRL, val,
+				       !(val & WDTCTRL_STATUS),
+				       0, WDTST_TIMEOUT);
+	if (ret)
+		return ret;
+
+	/* Setup period */
+	ret = regmap_write(regmap, WDTTIMSET,
+			   SEC_TO_WDTTIMSET_PRD(sec));
+	if (ret)
+		return ret;
+
+	/* Enable and clear watchdog */
+	ret = regmap_write(regmap, WDTCTRL, WDTCTRL_ENABLE | WDTCTRL_CLEAR);
+	if (!ret)
+		/*
+		 * As SoC specification, after clear counter,
+		 * it needs to wait until counter status is 1.
+		 */
+		ret = regmap_read_poll_timeout(regmap, WDTCTRL, val,
+					       (val & WDTCTRL_STATUS),
+					       0, WDTST_TIMEOUT);
+
+	return ret;
+}
+
+static int __uniphier_watchdog_stop(struct regmap *regmap)
+{
+	/* Disable and stop watchdog */
+	return regmap_write_bits(regmap, WDTCTRL, WDTCTRL_ENABLE, 0);
+}
+
+static int __uniphier_watchdog_restart(struct regmap *regmap, unsigned int sec)
+{
+	int ret;
+
+	ret = __uniphier_watchdog_stop(regmap);
+	if (ret)
+		return ret;
+
+	return __uniphier_watchdog_start(regmap, sec);
+}
+
+static int uniphier_watchdog_start(struct watchdog_device *w)
+{
+	struct uniphier_wdt_dev *wdev = watchdog_get_drvdata(w);
+	unsigned int tmp_timeout;
+
+	tmp_timeout = roundup_pow_of_two(w->timeout);
+
+	return __uniphier_watchdog_start(wdev->regmap, tmp_timeout);
+}
+
+static int uniphier_watchdog_stop(struct watchdog_device *w)
+{
+	struct uniphier_wdt_dev *wdev = watchdog_get_drvdata(w);
+
+	return __uniphier_watchdog_stop(wdev->regmap);
+}
+
+static int uniphier_watchdog_set_timeout(struct watchdog_device *w,
+					 unsigned int t)
+{
+	struct uniphier_wdt_dev *wdev = watchdog_get_drvdata(w);
+	unsigned int tmp_timeout;
+	int ret;
+
+	tmp_timeout = roundup_pow_of_two(t);
+	if (tmp_timeout == w->timeout)
+		return 0;
+
+	if (watchdog_active(w)) {
+		ret = __uniphier_watchdog_restart(wdev->regmap, tmp_timeout);
+		if (ret)
+			return ret;
+	}
+
+	w->timeout = tmp_timeout;
+
+	return 0;
+}
+
+/*
+ * Kernel Interfaces
+ */
+static const struct watchdog_info uniphier_wdt_info = {
+	.identity	= "uniphier-wdt",
+	.options	= WDIOF_SETTIMEOUT |
+			  WDIOF_KEEPALIVEPING |
+			  WDIOF_MAGICCLOSE |
+			  WDIOF_OVERHEAT,
+};
+
+static const struct watchdog_ops uniphier_wdt_ops = {
+	.owner		= THIS_MODULE,
+	.start		= uniphier_watchdog_start,
+	.stop		= uniphier_watchdog_stop,
+	.ping		= uniphier_watchdog_ping,
+	.set_timeout	= uniphier_watchdog_set_timeout,
+};
+
+static int uniphier_wdt_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct uniphier_wdt_dev *wdev;
+	struct regmap *regmap;
+	struct device_node *parent;
+	int ret;
+
+	wdev = devm_kzalloc(dev, sizeof(*wdev), GFP_KERNEL);
+	if (!wdev)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, wdev);
+
+	parent = of_get_parent(dev->of_node); /* parent should be syscon node */
+	regmap = syscon_node_to_regmap(parent);
+	of_node_put(parent);
+	if (IS_ERR(regmap))
+		return PTR_ERR(regmap);
+
+	wdev->regmap = regmap;
+	wdev->wdt_dev.info = &uniphier_wdt_info;
+	wdev->wdt_dev.ops = &uniphier_wdt_ops;
+	wdev->wdt_dev.timeout = WDT_DEFAULT_TIMEOUT;
+	wdev->wdt_dev.max_timeout = WDT_PERIOD_MAX;
+	wdev->wdt_dev.min_timeout = WDT_PERIOD_MIN;
+	wdev->wdt_dev.parent = dev;
+
+	watchdog_init_timeout(&wdev->wdt_dev, timeout, dev);
+	watchdog_set_nowayout(&wdev->wdt_dev, nowayout);
+	watchdog_stop_on_reboot(&wdev->wdt_dev);
+
+	watchdog_set_drvdata(&wdev->wdt_dev, wdev);
+
+	uniphier_watchdog_stop(&wdev->wdt_dev);
+	ret = regmap_write(wdev->regmap, WDTRSTSEL, WDTRSTSEL_RSTSEL_BOTH);
+	if (ret)
+		return ret;
+
+	ret = devm_watchdog_register_device(dev, &wdev->wdt_dev);
+	if (ret)
+		return ret;
+
+	dev_info(dev, "watchdog driver (timeout=%d sec, nowayout=%d)\n",
+		 wdev->wdt_dev.timeout, nowayout);
+
+	return 0;
+}
+
+static const struct of_device_id uniphier_wdt_dt_ids[] = {
+	{ .compatible = "socionext,uniphier-wdt" },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, uniphier_wdt_dt_ids);
+
+static struct platform_driver uniphier_wdt_driver = {
+	.probe		= uniphier_wdt_probe,
+	.driver		= {
+		.name		= "uniphier-wdt",
+		.of_match_table	= uniphier_wdt_dt_ids,
+	},
+};
+
+module_platform_driver(uniphier_wdt_driver);
+
+module_param(timeout, uint, 0000);
+MODULE_PARM_DESC(timeout,
+	"Watchdog timeout seconds in power of 2. (0 < timeout < 128, default="
+				__MODULE_STRING(WDT_DEFAULT_TIMEOUT) ")");
+
+module_param(nowayout, bool, 0000);
+MODULE_PARM_DESC(nowayout,
+	"Watchdog cannot be stopped once started (default="
+				__MODULE_STRING(WATCHDOG_NOWAYOUT) ")");
+
+MODULE_AUTHOR("Socionext Inc.");
+MODULE_DESCRIPTION("UniPhier Watchdog Device Driver");
+MODULE_LICENSE("GPL v2");
-- 
2.7.4

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

* [PATCH V2 3/3] arm64: dts: uniphier: add watchdog node for LD11 and LD20
  2017-06-06  9:11 ` Keiji Hayashibara
                   ` (2 preceding siblings ...)
  (?)
@ 2017-06-06  9:11 ` Keiji Hayashibara
  -1 siblings, 0 replies; 18+ messages in thread
From: Keiji Hayashibara @ 2017-06-06  9:11 UTC (permalink / raw)
  To: wim, linux
  Cc: linux-watchdog, linux-kernel, robh+dt, devicetree,
	masami.hiramatsu, jaswinder.singh, yamada.masahiro,
	hayashi.kunihiko, owada.kiyoshi, Keiji Hayashibara

Add nodes of watchdog timer for UniPhier LD11 and LD20 SoC.
The watchdog timer is included in sysctrl.

Signed-off-by: Keiji Hayashibara <hayashibara.keiji@socionext.com>
---
 arch/arm64/boot/dts/socionext/uniphier-ld11.dtsi | 4 ++++
 arch/arm64/boot/dts/socionext/uniphier-ld20.dtsi | 4 ++++
 2 files changed, 8 insertions(+)

diff --git a/arch/arm64/boot/dts/socionext/uniphier-ld11.dtsi b/arch/arm64/boot/dts/socionext/uniphier-ld11.dtsi
index da881f5..5fee3e3 100644
--- a/arch/arm64/boot/dts/socionext/uniphier-ld11.dtsi
+++ b/arch/arm64/boot/dts/socionext/uniphier-ld11.dtsi
@@ -393,6 +393,10 @@
 				compatible = "socionext,uniphier-ld11-reset";
 				#reset-cells = <1>;
 			};
+
+			watchdog {
+				compatible = "socionext,uniphier-wdt";
+			};
 		};
 	};
 };
diff --git a/arch/arm64/boot/dts/socionext/uniphier-ld20.dtsi b/arch/arm64/boot/dts/socionext/uniphier-ld20.dtsi
index a6b3a70..d4d82c8 100644
--- a/arch/arm64/boot/dts/socionext/uniphier-ld20.dtsi
+++ b/arch/arm64/boot/dts/socionext/uniphier-ld20.dtsi
@@ -417,6 +417,10 @@
 				compatible = "socionext,uniphier-ld20-reset";
 				#reset-cells = <1>;
 			};
+
+			watchdog {
+				compatible = "socionext,uniphier-wdt";
+			};
 		};
 	};
 };
-- 
2.7.4

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

* Re: [PATCH V2 2/3] watchdog: uniphier: add UniPhier watchdog driver
@ 2017-06-08  2:09     ` Masahiro Yamada
  0 siblings, 0 replies; 18+ messages in thread
From: Masahiro Yamada @ 2017-06-08  2:09 UTC (permalink / raw)
  To: Keiji Hayashibara
  Cc: wim, Guenter Roeck, linux-watchdog, Linux Kernel Mailing List,
	Rob Herring, devicetree, Masami Hiramatsu, Jassi Brar,
	Kunihiko Hayashi, Kiyoshi Owada

Hayashibara-san

Just a nit.


> +
> +#include <linux/module.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/watchdog.h>
> +#include <linux/bitops.h>

I think you should have added <linux/bitops.h> on the top
to keep the includes alphabetically sorted.


-- 
Best Regards
Masahiro Yamada

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

* Re: [PATCH V2 2/3] watchdog: uniphier: add UniPhier watchdog driver
@ 2017-06-08  2:09     ` Masahiro Yamada
  0 siblings, 0 replies; 18+ messages in thread
From: Masahiro Yamada @ 2017-06-08  2:09 UTC (permalink / raw)
  To: Keiji Hayashibara
  Cc: wim-IQzOog9fTRqzQB+pC5nmwQ, Guenter Roeck,
	linux-watchdog-u79uwXL29TY76Z2rM5mHXA, Linux Kernel Mailing List,
	Rob Herring, devicetree-u79uwXL29TY76Z2rM5mHXA, Masami Hiramatsu,
	Jassi Brar, Kunihiko Hayashi, Kiyoshi Owada

Hayashibara-san

Just a nit.


> +
> +#include <linux/module.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/watchdog.h>
> +#include <linux/bitops.h>

I think you should have added <linux/bitops.h> on the top
to keep the includes alphabetically sorted.


-- 
Best Regards
Masahiro Yamada
--
To unsubscribe from this list: send the line "unsubscribe devicetree" 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] 18+ messages in thread

* Re: [PATCH V2 1/3] devicetree: add UniPhier WDT devicetree bindings documentation
@ 2017-06-08  2:11     ` Masahiro Yamada
  0 siblings, 0 replies; 18+ messages in thread
From: Masahiro Yamada @ 2017-06-08  2:11 UTC (permalink / raw)
  To: Keiji Hayashibara
  Cc: wim, Guenter Roeck, linux-watchdog, Linux Kernel Mailing List,
	Rob Herring, devicetree, Masami Hiramatsu, Jassi Brar,
	Kunihiko Hayashi, Kiyoshi Owada

Hayashibara-san


Just nitpicking.


2017-06-06 18:11 GMT+09:00 Keiji Hayashibara <hayashibara.keiji@socionext.com>:
> Add uniphier-wdt bindings documentation.
>
> Signed-off-by: Keiji Hayashibara <hayashibara.keiji@socionext.com>


Maybe "dt-bindings: watchdog: ..." for the subject.

This seems what Rob prefers.

For example,
https://patchwork.kernel.org/patch/9606365/



> ---
>  .../devicetree/bindings/watchdog/uniphier-wdt.txt    | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/watchdog/uniphier-wdt.txt
>
> diff --git a/Documentation/devicetree/bindings/watchdog/uniphier-wdt.txt b/Documentation/devicetree/bindings/watchdog/uniphier-wdt.txt
> new file mode 100644
> index 0000000..a59d1ba
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/watchdog/uniphier-wdt.txt
> @@ -0,0 +1,20 @@
> +UniPhier watchdog timer controller
> +
> +This UniPhier watchdog timer controller must be under sysctrl compatible node.

Do you mean "syscon compatible" ?


> +Required properties:
> +- compatible: should be "socionext,uniphier-wdt"
> +
> +Example:
> +
> +       sysctrl@61840000 {
> +               compatible = "socionext,uniphier-ld11-sysctrl",
> +                            "simple-mfd", "syscon";
> +               reg = <0x61840000 0x4000>;
> +
> +               sys_wdt: watchdog {
> +                       compatible = "socionext,uniphier-wdt";
> +               }

Could you remove the "sys_dwt: " label?
I'd like the example matched with actual DT files.


Thanks.


> +               other nodes ...
> +       };
> --
> 2.7.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Best Regards
Masahiro Yamada

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

* Re: [PATCH V2 1/3] devicetree: add UniPhier WDT devicetree bindings documentation
@ 2017-06-08  2:11     ` Masahiro Yamada
  0 siblings, 0 replies; 18+ messages in thread
From: Masahiro Yamada @ 2017-06-08  2:11 UTC (permalink / raw)
  To: Keiji Hayashibara
  Cc: wim-IQzOog9fTRqzQB+pC5nmwQ, Guenter Roeck,
	linux-watchdog-u79uwXL29TY76Z2rM5mHXA, Linux Kernel Mailing List,
	Rob Herring, devicetree-u79uwXL29TY76Z2rM5mHXA, Masami Hiramatsu,
	Jassi Brar, Kunihiko Hayashi, Kiyoshi Owada

Hayashibara-san


Just nitpicking.


2017-06-06 18:11 GMT+09:00 Keiji Hayashibara <hayashibara.keiji-uWyLwvC0a2jby3iVrkZq2A@public.gmane.org>:
> Add uniphier-wdt bindings documentation.
>
> Signed-off-by: Keiji Hayashibara <hayashibara.keiji-uWyLwvC0a2jby3iVrkZq2A@public.gmane.org>


Maybe "dt-bindings: watchdog: ..." for the subject.

This seems what Rob prefers.

For example,
https://patchwork.kernel.org/patch/9606365/



> ---
>  .../devicetree/bindings/watchdog/uniphier-wdt.txt    | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/watchdog/uniphier-wdt.txt
>
> diff --git a/Documentation/devicetree/bindings/watchdog/uniphier-wdt.txt b/Documentation/devicetree/bindings/watchdog/uniphier-wdt.txt
> new file mode 100644
> index 0000000..a59d1ba
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/watchdog/uniphier-wdt.txt
> @@ -0,0 +1,20 @@
> +UniPhier watchdog timer controller
> +
> +This UniPhier watchdog timer controller must be under sysctrl compatible node.

Do you mean "syscon compatible" ?


> +Required properties:
> +- compatible: should be "socionext,uniphier-wdt"
> +
> +Example:
> +
> +       sysctrl@61840000 {
> +               compatible = "socionext,uniphier-ld11-sysctrl",
> +                            "simple-mfd", "syscon";
> +               reg = <0x61840000 0x4000>;
> +
> +               sys_wdt: watchdog {
> +                       compatible = "socionext,uniphier-wdt";
> +               }

Could you remove the "sys_dwt: " label?
I'd like the example matched with actual DT files.


Thanks.


> +               other nodes ...
> +       };
> --
> 2.7.4
>
> --
> 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



-- 
Best Regards
Masahiro Yamada
--
To unsubscribe from this list: send the line "unsubscribe devicetree" 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] 18+ messages in thread

* RE: [PATCH V2 2/3] watchdog: uniphier: add UniPhier watchdog driver
@ 2017-06-08  5:51       ` Keiji Hayashibara
  0 siblings, 0 replies; 18+ messages in thread
From: Keiji Hayashibara @ 2017-06-08  5:51 UTC (permalink / raw)
  To: Yamada, Masahiro/山田 真弘
  Cc: wim, Guenter Roeck, linux-watchdog, Linux Kernel Mailing List,
	Rob Herring, devicetree, Masami Hiramatsu, Jassi Brar, Hayashi,
	Kunihiko/林 邦彦,
	Owada, Kiyoshi/大和田 清志

Hello Yamada-san,

> > +
> > +#include <linux/module.h>
> > +#include <linux/mfd/syscon.h>
> > +#include <linux/of.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/regmap.h>
> > +#include <linux/watchdog.h>
> > +#include <linux/bitops.h>
> 
> I think you should have added <linux/bitops.h> on the top
> to keep the includes alphabetically sorted.

Sorry, I will fix in v3 patch.

----------
Best Regards,
Keiji Hayashibara

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

* RE: [PATCH V2 2/3] watchdog: uniphier: add UniPhier watchdog driver
@ 2017-06-08  5:51       ` Keiji Hayashibara
  0 siblings, 0 replies; 18+ messages in thread
From: Keiji Hayashibara @ 2017-06-08  5:51 UTC (permalink / raw)
  To: Yamada, Masahiro/山田 真弘
  Cc: wim-IQzOog9fTRqzQB+pC5nmwQ, Guenter Roeck,
	linux-watchdog-u79uwXL29TY76Z2rM5mHXA, Linux Kernel Mailing List,
	Rob Herring, devicetree-u79uwXL29TY76Z2rM5mHXA, Masami Hiramatsu,
	Jassi Brar, Hayashi, Kunihiko/林 邦彦,
	Owada, Kiyoshi/大和田 清志

Hello Yamada-san,

> > +
> > +#include <linux/module.h>
> > +#include <linux/mfd/syscon.h>
> > +#include <linux/of.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/regmap.h>
> > +#include <linux/watchdog.h>
> > +#include <linux/bitops.h>
> 
> I think you should have added <linux/bitops.h> on the top
> to keep the includes alphabetically sorted.

Sorry, I will fix in v3 patch.

----------
Best Regards,
Keiji Hayashibara


--
To unsubscribe from this list: send the line "unsubscribe devicetree" 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] 18+ messages in thread

* RE: [PATCH V2 1/3] devicetree: add UniPhier WDT devicetree bindings documentation
@ 2017-06-08  6:22       ` Keiji Hayashibara
  0 siblings, 0 replies; 18+ messages in thread
From: Keiji Hayashibara @ 2017-06-08  6:22 UTC (permalink / raw)
  To: Yamada, Masahiro/山田 真弘
  Cc: wim, Guenter Roeck, linux-watchdog, Linux Kernel Mailing List,
	Rob Herring, devicetree, Masami Hiramatsu, Jassi Brar, Hayashi,
	Kunihiko/林 邦彦,
	Owada, Kiyoshi/大和田 清志

Hello Yamada-san

Thank you for your comment.

> > Add uniphier-wdt bindings documentation.
> >
> > Signed-off-by: Keiji Hayashibara <hayashibara.keiji@socionext.com>
> 
> 
> Maybe "dt-bindings: watchdog: ..." for the subject.
> 
> This seems what Rob prefers.
> 
> For example,
> https://patchwork.kernel.org/patch/9606365/

OK. I will change the subject in v3 patch.

> 
> > ---
> >  .../devicetree/bindings/watchdog/uniphier-wdt.txt    | 20 ++++++++++++++++++++
> >  1 file changed, 20 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/watchdog/uniphier-wdt.txt
> >
> > diff --git a/Documentation/devicetree/bindings/watchdog/uniphier-wdt.txt
> b/Documentation/devicetree/bindings/watchdog/uniphier-wdt.txt
> > new file mode 100644
> > index 0000000..a59d1ba
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/watchdog/uniphier-wdt.txt
> > @@ -0,0 +1,20 @@
> > +UniPhier watchdog timer controller
> > +
> > +This UniPhier watchdog timer controller must be under sysctrl compatible node.
> 
> Do you mean "syscon compatible" ?

The "compatible" word is inappropriate.
I will remove it in v3 patch.

> 
> > +Required properties:
> > +- compatible: should be "socionext,uniphier-wdt"
> > +
> > +Example:
> > +
> > +       sysctrl@61840000 {
> > +               compatible = "socionext,uniphier-ld11-sysctrl",
> > +                            "simple-mfd", "syscon";
> > +               reg = <0x61840000 0x4000>;
> > +
> > +               sys_wdt: watchdog {
> > +                       compatible = "socionext,uniphier-wdt";
> > +               }
> 
> Could you remove the "sys_dwt: " label?
> I'd like the example matched with actual DT files.

I will remove it.

> 
> Thanks.
> 
> 
> > +               other nodes ...
> > +       };
> > --
> > 2.7.4
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html

----------
Best Regards,
Keiji Hayashibara

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

* RE: [PATCH V2 1/3] devicetree: add UniPhier WDT devicetree bindings documentation
@ 2017-06-08  6:22       ` Keiji Hayashibara
  0 siblings, 0 replies; 18+ messages in thread
From: Keiji Hayashibara @ 2017-06-08  6:22 UTC (permalink / raw)
  To: Yamada, Masahiro/山田 真弘
  Cc: wim-IQzOog9fTRqzQB+pC5nmwQ, Guenter Roeck,
	linux-watchdog-u79uwXL29TY76Z2rM5mHXA, Linux Kernel Mailing List,
	Rob Herring, devicetree-u79uwXL29TY76Z2rM5mHXA, Masami Hiramatsu,
	Jassi Brar, Hayashi, Kunihiko/林 邦彦,
	Owada, Kiyoshi/大和田 清志

Hello Yamada-san

Thank you for your comment.

> > Add uniphier-wdt bindings documentation.
> >
> > Signed-off-by: Keiji Hayashibara <hayashibara.keiji-uWyLwvC0a2jby3iVrkZq2A@public.gmane.org>
> 
> 
> Maybe "dt-bindings: watchdog: ..." for the subject.
> 
> This seems what Rob prefers.
> 
> For example,
> https://patchwork.kernel.org/patch/9606365/

OK. I will change the subject in v3 patch.

> 
> > ---
> >  .../devicetree/bindings/watchdog/uniphier-wdt.txt    | 20 ++++++++++++++++++++
> >  1 file changed, 20 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/watchdog/uniphier-wdt.txt
> >
> > diff --git a/Documentation/devicetree/bindings/watchdog/uniphier-wdt.txt
> b/Documentation/devicetree/bindings/watchdog/uniphier-wdt.txt
> > new file mode 100644
> > index 0000000..a59d1ba
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/watchdog/uniphier-wdt.txt
> > @@ -0,0 +1,20 @@
> > +UniPhier watchdog timer controller
> > +
> > +This UniPhier watchdog timer controller must be under sysctrl compatible node.
> 
> Do you mean "syscon compatible" ?

The "compatible" word is inappropriate.
I will remove it in v3 patch.

> 
> > +Required properties:
> > +- compatible: should be "socionext,uniphier-wdt"
> > +
> > +Example:
> > +
> > +       sysctrl@61840000 {
> > +               compatible = "socionext,uniphier-ld11-sysctrl",
> > +                            "simple-mfd", "syscon";
> > +               reg = <0x61840000 0x4000>;
> > +
> > +               sys_wdt: watchdog {
> > +                       compatible = "socionext,uniphier-wdt";
> > +               }
> 
> Could you remove the "sys_dwt: " label?
> I'd like the example matched with actual DT files.

I will remove it.

> 
> Thanks.
> 
> 
> > +               other nodes ...
> > +       };
> > --
> > 2.7.4
> >
> > --
> > 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

----------
Best Regards,
Keiji Hayashibara


--
To unsubscribe from this list: send the line "unsubscribe devicetree" 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] 18+ messages in thread

* Re: [PATCH V2 2/3] watchdog: uniphier: add UniPhier watchdog driver
@ 2017-06-08 12:31     ` Guenter Roeck
  0 siblings, 0 replies; 18+ messages in thread
From: Guenter Roeck @ 2017-06-08 12:31 UTC (permalink / raw)
  To: Keiji Hayashibara, wim
  Cc: linux-watchdog, linux-kernel, robh+dt, devicetree,
	masami.hiramatsu, jaswinder.singh, yamada.masahiro,
	hayashi.kunihiko, owada.kiyoshi

On 06/06/2017 02:11 AM, Keiji Hayashibara wrote:
> Add a watchdog driver for Socionext UniPhier series SoC.
> Note that the timeout value for this device must be a power
> of 2 because of the specification.
> 
> Signed-off-by: Keiji Hayashibara <hayashibara.keiji@socionext.com>
> ---
>   Documentation/watchdog/watchdog-parameters.txt |   6 +
>   drivers/watchdog/Kconfig                       |  11 +
>   drivers/watchdog/Makefile                      |   1 +
>   drivers/watchdog/uniphier_wdt.c                | 275 +++++++++++++++++++++++++
>   4 files changed, 293 insertions(+)
>   create mode 100644 drivers/watchdog/uniphier_wdt.c
> 
> diff --git a/Documentation/watchdog/watchdog-parameters.txt b/Documentation/watchdog/watchdog-parameters.txt
> index 4f7d86d..6f9d7b4 100644
> --- a/Documentation/watchdog/watchdog-parameters.txt
> +++ b/Documentation/watchdog/watchdog-parameters.txt
> @@ -369,6 +369,12 @@ timeout: Watchdog timeout in seconds. (0<timeout<N, default=60)
>   nowayout: Watchdog cannot be stopped once started
>   	(default=kernel config parameter)
>   -------------------------------------------------
> +uniphier_wdt:
> +timeout: Watchdog timeout in power of two seconds.
> +	(1 <= timeout <= 128, default=64)
> +nowayout: Watchdog cannot be stopped once started
> +	(default=kernel config parameter)
> +-------------------------------------------------
>   w83627hf_wdt:
>   wdt_io: w83627hf/thf WDT io port (default 0x2E)
>   timeout: Watchdog timeout in seconds. 1 <= timeout <= 255, default=60.
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index 52a70ee..2a9d9a6 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -744,6 +744,17 @@ config ZX2967_WATCHDOG
>   	  To compile this driver as a module, choose M here: the
>   	  module will be called zx2967_wdt.
>   
> +config UNIPHIER_WATCHDOG
> +	tristate "UniPhier watchdog support"
> +	depends on ARCH_UNIPHIER || COMPILE_TEST
> +	select WATCHDOG_CORE
> +	help
> +	  Say Y here to include support watchdog timer embedded
> +	  into the UniPhier system.
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called uniphier_wdt.
> +
>   # AVR32 Architecture
>   
>   config AT32AP700X_WDT
> diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
> index a2126e2..0928dac 100644
> --- a/drivers/watchdog/Makefile
> +++ b/drivers/watchdog/Makefile
> @@ -84,6 +84,7 @@ obj-$(CONFIG_ATLAS7_WATCHDOG) += atlas7_wdt.o
>   obj-$(CONFIG_RENESAS_WDT) += renesas_wdt.o
>   obj-$(CONFIG_ASPEED_WATCHDOG) += aspeed_wdt.o
>   obj-$(CONFIG_ZX2967_WATCHDOG) += zx2967_wdt.o
> +obj-$(CONFIG_UNIPHIER_WATCHDOG) += uniphier_wdt.o
>   
>   # AVR32 Architecture
>   obj-$(CONFIG_AT32AP700X_WDT) += at32ap700x_wdt.o
> diff --git a/drivers/watchdog/uniphier_wdt.c b/drivers/watchdog/uniphier_wdt.c
> new file mode 100644
> index 0000000..75862a0
> --- /dev/null
> +++ b/drivers/watchdog/uniphier_wdt.c
> @@ -0,0 +1,275 @@
> +/*
> + * Watchdog driver for the UniPhier watchdog timer
> + *
> + * (c) Copyright 2014 Panasonic Corporation
> + * (c) Copyright 2016 Socionext Inc.
> + * All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/watchdog.h>
> +#include <linux/bitops.h>
> +
> +/* WDT timer setting register */
> +#define WDTTIMSET			0x3004
> +#define   WDTTIMSET_PERIOD_MASK		(0xf << 0)
> +#define   WDTTIMSET_PERIOD_1_SEC	(0x3 << 0)
> +#define   WDTTIMSET_PERIOD_2_SEC	(0x4 << 0)
> +#define   WDTTIMSET_PERIOD_4_SEC	(0x5 << 0)
> +#define   WDTTIMSET_PERIOD_8_SEC	(0x6 << 0)
> +#define   WDTTIMSET_PERIOD_16_SEC	(0x7 << 0)
> +#define   WDTTIMSET_PERIOD_32_SEC	(0x8 << 0)
> +#define   WDTTIMSET_PERIOD_64_SEC	(0x9 << 0)
> +#define   WDTTIMSET_PERIOD_128_SEC	(0xa << 0)
> +
> +/* WDT reset selection register */
> +#define WDTRSTSEL			0x3008
> +#define   WDTRSTSEL_RSTSEL_MASK		(0x3 << 0)
> +#define   WDTRSTSEL_RSTSEL_BOTH		(0x0 << 0)
> +#define   WDTRSTSEL_RSTSEL_IRQ_ONLY	(0x2 << 0)
> +
> +/* WDT control register */
> +#define WDTCTRL				0x300c
> +#define   WDTCTRL_STATUS		BIT(8)
> +#define   WDTCTRL_CLEAR			BIT(1)
> +#define   WDTCTRL_ENABLE		BIT(0)
> +
> +#define SEC_TO_WDTTIMSET_PRD(sec) \
> +		(ilog2(sec) + WDTTIMSET_PERIOD_1_SEC)
> +
> +#define WDTST_TIMEOUT			1000 /* usec */
> +
> +#define WDT_DEFAULT_TIMEOUT		64   /* Default is 64 seconds */
> +#define WDT_PERIOD_MIN			1
> +#define WDT_PERIOD_MAX			128
> +
> +static unsigned int timeout = WDT_DEFAULT_TIMEOUT;

This should be initialized to 0 (see below).

> +static bool nowayout = WATCHDOG_NOWAYOUT;
> +
> +struct uniphier_wdt_dev {
> +	struct watchdog_device wdt_dev;
> +	struct regmap	*regmap;
> +};
> +
> +/*
> + * UniPhier Watchdog operations
> + */
> +static int uniphier_watchdog_ping(struct watchdog_device *w)
> +{
> +	struct uniphier_wdt_dev *wdev = watchdog_get_drvdata(w);
> +	unsigned int val;
> +	int ret;
> +
> +	/* Clear counter */
> +	ret = regmap_write_bits(wdev->regmap, WDTCTRL,
> +				WDTCTRL_CLEAR, WDTCTRL_CLEAR);
> +	if (!ret)
> +		/*
> +		 * As SoC specification, after clear counter,
> +		 * it needs to wait until counter status is 1.
> +		 */
> +		ret = regmap_read_poll_timeout(wdev->regmap, WDTCTRL, val,
> +					       (val & WDTCTRL_STATUS),
> +					       0, WDTST_TIMEOUT);
> +
> +	return ret;
> +}
> +
> +static int __uniphier_watchdog_start(struct regmap *regmap, unsigned int sec)
> +{
> +	unsigned int val;
> +	int ret;
> +
> +	ret = regmap_read_poll_timeout(regmap, WDTCTRL, val,
> +				       !(val & WDTCTRL_STATUS),
> +				       0, WDTST_TIMEOUT);
> +	if (ret)
> +		return ret;
> +
> +	/* Setup period */
> +	ret = regmap_write(regmap, WDTTIMSET,
> +			   SEC_TO_WDTTIMSET_PRD(sec));
> +	if (ret)
> +		return ret;
> +
> +	/* Enable and clear watchdog */
> +	ret = regmap_write(regmap, WDTCTRL, WDTCTRL_ENABLE | WDTCTRL_CLEAR);
> +	if (!ret)
> +		/*
> +		 * As SoC specification, after clear counter,
> +		 * it needs to wait until counter status is 1.
> +		 */
> +		ret = regmap_read_poll_timeout(regmap, WDTCTRL, val,
> +					       (val & WDTCTRL_STATUS),
> +					       0, WDTST_TIMEOUT);
> +
> +	return ret;
> +}
> +
> +static int __uniphier_watchdog_stop(struct regmap *regmap)
> +{
> +	/* Disable and stop watchdog */
> +	return regmap_write_bits(regmap, WDTCTRL, WDTCTRL_ENABLE, 0);
> +}
> +
> +static int __uniphier_watchdog_restart(struct regmap *regmap, unsigned int sec)
> +{
> +	int ret;
> +
> +	ret = __uniphier_watchdog_stop(regmap);
> +	if (ret)
> +		return ret;
> +
> +	return __uniphier_watchdog_start(regmap, sec);
> +}
> +
> +static int uniphier_watchdog_start(struct watchdog_device *w)
> +{
> +	struct uniphier_wdt_dev *wdev = watchdog_get_drvdata(w);
> +	unsigned int tmp_timeout;
> +
> +	tmp_timeout = roundup_pow_of_two(w->timeout);
> +
> +	return __uniphier_watchdog_start(wdev->regmap, tmp_timeout);
> +}
> +
> +static int uniphier_watchdog_stop(struct watchdog_device *w)
> +{
> +	struct uniphier_wdt_dev *wdev = watchdog_get_drvdata(w);
> +
> +	return __uniphier_watchdog_stop(wdev->regmap);
> +}
> +
> +static int uniphier_watchdog_set_timeout(struct watchdog_device *w,
> +					 unsigned int t)
> +{
> +	struct uniphier_wdt_dev *wdev = watchdog_get_drvdata(w);
> +	unsigned int tmp_timeout;
> +	int ret;
> +
> +	tmp_timeout = roundup_pow_of_two(t);
> +	if (tmp_timeout == w->timeout)
> +		return 0;
> +
> +	if (watchdog_active(w)) {
> +		ret = __uniphier_watchdog_restart(wdev->regmap, tmp_timeout);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	w->timeout = tmp_timeout;
> +
> +	return 0;
> +}
> +
> +/*
> + * Kernel Interfaces
> + */
> +static const struct watchdog_info uniphier_wdt_info = {
> +	.identity	= "uniphier-wdt",
> +	.options	= WDIOF_SETTIMEOUT |
> +			  WDIOF_KEEPALIVEPING |
> +			  WDIOF_MAGICCLOSE |
> +			  WDIOF_OVERHEAT,
> +};
> +
> +static const struct watchdog_ops uniphier_wdt_ops = {
> +	.owner		= THIS_MODULE,
> +	.start		= uniphier_watchdog_start,
> +	.stop		= uniphier_watchdog_stop,
> +	.ping		= uniphier_watchdog_ping,
> +	.set_timeout	= uniphier_watchdog_set_timeout,
> +};
> +
> +static int uniphier_wdt_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct uniphier_wdt_dev *wdev;
> +	struct regmap *regmap;
> +	struct device_node *parent;
> +	int ret;
> +
> +	wdev = devm_kzalloc(dev, sizeof(*wdev), GFP_KERNEL);
> +	if (!wdev)
> +		return -ENOMEM;
> +
> +	platform_set_drvdata(pdev, wdev);
> +
> +	parent = of_get_parent(dev->of_node); /* parent should be syscon node */
> +	regmap = syscon_node_to_regmap(parent);
> +	of_node_put(parent);
> +	if (IS_ERR(regmap))
> +		return PTR_ERR(regmap);
> +
> +	wdev->regmap = regmap;
> +	wdev->wdt_dev.info = &uniphier_wdt_info;
> +	wdev->wdt_dev.ops = &uniphier_wdt_ops;
> +	wdev->wdt_dev.timeout = WDT_DEFAULT_TIMEOUT;
> +	wdev->wdt_dev.max_timeout = WDT_PERIOD_MAX;
> +	wdev->wdt_dev.min_timeout = WDT_PERIOD_MIN;
> +	wdev->wdt_dev.parent = dev;
> +
> +	watchdog_init_timeout(&wdev->wdt_dev, timeout, dev);

This only really makes sense if the default for 'timeout' is 0.
Otherwise it will never take a timeout from devicetree unless timeout is
explicitly set to 0 as module parameter, which does not make much sense.

> +	watchdog_set_nowayout(&wdev->wdt_dev, nowayout);
> +	watchdog_stop_on_reboot(&wdev->wdt_dev);
> +
> +	watchdog_set_drvdata(&wdev->wdt_dev, wdev);
> +
> +	uniphier_watchdog_stop(&wdev->wdt_dev);
> +	ret = regmap_write(wdev->regmap, WDTRSTSEL, WDTRSTSEL_RSTSEL_BOTH);
> +	if (ret)
> +		return ret;
> +
> +	ret = devm_watchdog_register_device(dev, &wdev->wdt_dev);
> +	if (ret)
> +		return ret;
> +
> +	dev_info(dev, "watchdog driver (timeout=%d sec, nowayout=%d)\n",
> +		 wdev->wdt_dev.timeout, nowayout);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id uniphier_wdt_dt_ids[] = {
> +	{ .compatible = "socionext,uniphier-wdt" },
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, uniphier_wdt_dt_ids);
> +
> +static struct platform_driver uniphier_wdt_driver = {
> +	.probe		= uniphier_wdt_probe,
> +	.driver		= {
> +		.name		= "uniphier-wdt",
> +		.of_match_table	= uniphier_wdt_dt_ids,
> +	},
> +};
> +
> +module_platform_driver(uniphier_wdt_driver);
> +
> +module_param(timeout, uint, 0000);
> +MODULE_PARM_DESC(timeout,
> +	"Watchdog timeout seconds in power of 2. (0 < timeout < 128, default="
> +				__MODULE_STRING(WDT_DEFAULT_TIMEOUT) ")");
> +
> +module_param(nowayout, bool, 0000);
> +MODULE_PARM_DESC(nowayout,
> +	"Watchdog cannot be stopped once started (default="
> +				__MODULE_STRING(WATCHDOG_NOWAYOUT) ")");
> +
> +MODULE_AUTHOR("Socionext Inc.");
> +MODULE_DESCRIPTION("UniPhier Watchdog Device Driver");
> +MODULE_LICENSE("GPL v2");
> 

The license here does not match the license in the header (which states GPL2+).

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

* Re: [PATCH V2 2/3] watchdog: uniphier: add UniPhier watchdog driver
@ 2017-06-08 12:31     ` Guenter Roeck
  0 siblings, 0 replies; 18+ messages in thread
From: Guenter Roeck @ 2017-06-08 12:31 UTC (permalink / raw)
  To: Keiji Hayashibara, wim-IQzOog9fTRqzQB+pC5nmwQ
  Cc: linux-watchdog-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	masami.hiramatsu-QSEj5FYQhm4dnm+yROfE0A,
	jaswinder.singh-QSEj5FYQhm4dnm+yROfE0A,
	yamada.masahiro-uWyLwvC0a2jby3iVrkZq2A,
	hayashi.kunihiko-uWyLwvC0a2jby3iVrkZq2A,
	owada.kiyoshi-uWyLwvC0a2jby3iVrkZq2A

On 06/06/2017 02:11 AM, Keiji Hayashibara wrote:
> Add a watchdog driver for Socionext UniPhier series SoC.
> Note that the timeout value for this device must be a power
> of 2 because of the specification.
> 
> Signed-off-by: Keiji Hayashibara <hayashibara.keiji-uWyLwvC0a2jby3iVrkZq2A@public.gmane.org>
> ---
>   Documentation/watchdog/watchdog-parameters.txt |   6 +
>   drivers/watchdog/Kconfig                       |  11 +
>   drivers/watchdog/Makefile                      |   1 +
>   drivers/watchdog/uniphier_wdt.c                | 275 +++++++++++++++++++++++++
>   4 files changed, 293 insertions(+)
>   create mode 100644 drivers/watchdog/uniphier_wdt.c
> 
> diff --git a/Documentation/watchdog/watchdog-parameters.txt b/Documentation/watchdog/watchdog-parameters.txt
> index 4f7d86d..6f9d7b4 100644
> --- a/Documentation/watchdog/watchdog-parameters.txt
> +++ b/Documentation/watchdog/watchdog-parameters.txt
> @@ -369,6 +369,12 @@ timeout: Watchdog timeout in seconds. (0<timeout<N, default=60)
>   nowayout: Watchdog cannot be stopped once started
>   	(default=kernel config parameter)
>   -------------------------------------------------
> +uniphier_wdt:
> +timeout: Watchdog timeout in power of two seconds.
> +	(1 <= timeout <= 128, default=64)
> +nowayout: Watchdog cannot be stopped once started
> +	(default=kernel config parameter)
> +-------------------------------------------------
>   w83627hf_wdt:
>   wdt_io: w83627hf/thf WDT io port (default 0x2E)
>   timeout: Watchdog timeout in seconds. 1 <= timeout <= 255, default=60.
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index 52a70ee..2a9d9a6 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -744,6 +744,17 @@ config ZX2967_WATCHDOG
>   	  To compile this driver as a module, choose M here: the
>   	  module will be called zx2967_wdt.
>   
> +config UNIPHIER_WATCHDOG
> +	tristate "UniPhier watchdog support"
> +	depends on ARCH_UNIPHIER || COMPILE_TEST
> +	select WATCHDOG_CORE
> +	help
> +	  Say Y here to include support watchdog timer embedded
> +	  into the UniPhier system.
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called uniphier_wdt.
> +
>   # AVR32 Architecture
>   
>   config AT32AP700X_WDT
> diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
> index a2126e2..0928dac 100644
> --- a/drivers/watchdog/Makefile
> +++ b/drivers/watchdog/Makefile
> @@ -84,6 +84,7 @@ obj-$(CONFIG_ATLAS7_WATCHDOG) += atlas7_wdt.o
>   obj-$(CONFIG_RENESAS_WDT) += renesas_wdt.o
>   obj-$(CONFIG_ASPEED_WATCHDOG) += aspeed_wdt.o
>   obj-$(CONFIG_ZX2967_WATCHDOG) += zx2967_wdt.o
> +obj-$(CONFIG_UNIPHIER_WATCHDOG) += uniphier_wdt.o
>   
>   # AVR32 Architecture
>   obj-$(CONFIG_AT32AP700X_WDT) += at32ap700x_wdt.o
> diff --git a/drivers/watchdog/uniphier_wdt.c b/drivers/watchdog/uniphier_wdt.c
> new file mode 100644
> index 0000000..75862a0
> --- /dev/null
> +++ b/drivers/watchdog/uniphier_wdt.c
> @@ -0,0 +1,275 @@
> +/*
> + * Watchdog driver for the UniPhier watchdog timer
> + *
> + * (c) Copyright 2014 Panasonic Corporation
> + * (c) Copyright 2016 Socionext Inc.
> + * All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/watchdog.h>
> +#include <linux/bitops.h>
> +
> +/* WDT timer setting register */
> +#define WDTTIMSET			0x3004
> +#define   WDTTIMSET_PERIOD_MASK		(0xf << 0)
> +#define   WDTTIMSET_PERIOD_1_SEC	(0x3 << 0)
> +#define   WDTTIMSET_PERIOD_2_SEC	(0x4 << 0)
> +#define   WDTTIMSET_PERIOD_4_SEC	(0x5 << 0)
> +#define   WDTTIMSET_PERIOD_8_SEC	(0x6 << 0)
> +#define   WDTTIMSET_PERIOD_16_SEC	(0x7 << 0)
> +#define   WDTTIMSET_PERIOD_32_SEC	(0x8 << 0)
> +#define   WDTTIMSET_PERIOD_64_SEC	(0x9 << 0)
> +#define   WDTTIMSET_PERIOD_128_SEC	(0xa << 0)
> +
> +/* WDT reset selection register */
> +#define WDTRSTSEL			0x3008
> +#define   WDTRSTSEL_RSTSEL_MASK		(0x3 << 0)
> +#define   WDTRSTSEL_RSTSEL_BOTH		(0x0 << 0)
> +#define   WDTRSTSEL_RSTSEL_IRQ_ONLY	(0x2 << 0)
> +
> +/* WDT control register */
> +#define WDTCTRL				0x300c
> +#define   WDTCTRL_STATUS		BIT(8)
> +#define   WDTCTRL_CLEAR			BIT(1)
> +#define   WDTCTRL_ENABLE		BIT(0)
> +
> +#define SEC_TO_WDTTIMSET_PRD(sec) \
> +		(ilog2(sec) + WDTTIMSET_PERIOD_1_SEC)
> +
> +#define WDTST_TIMEOUT			1000 /* usec */
> +
> +#define WDT_DEFAULT_TIMEOUT		64   /* Default is 64 seconds */
> +#define WDT_PERIOD_MIN			1
> +#define WDT_PERIOD_MAX			128
> +
> +static unsigned int timeout = WDT_DEFAULT_TIMEOUT;

This should be initialized to 0 (see below).

> +static bool nowayout = WATCHDOG_NOWAYOUT;
> +
> +struct uniphier_wdt_dev {
> +	struct watchdog_device wdt_dev;
> +	struct regmap	*regmap;
> +};
> +
> +/*
> + * UniPhier Watchdog operations
> + */
> +static int uniphier_watchdog_ping(struct watchdog_device *w)
> +{
> +	struct uniphier_wdt_dev *wdev = watchdog_get_drvdata(w);
> +	unsigned int val;
> +	int ret;
> +
> +	/* Clear counter */
> +	ret = regmap_write_bits(wdev->regmap, WDTCTRL,
> +				WDTCTRL_CLEAR, WDTCTRL_CLEAR);
> +	if (!ret)
> +		/*
> +		 * As SoC specification, after clear counter,
> +		 * it needs to wait until counter status is 1.
> +		 */
> +		ret = regmap_read_poll_timeout(wdev->regmap, WDTCTRL, val,
> +					       (val & WDTCTRL_STATUS),
> +					       0, WDTST_TIMEOUT);
> +
> +	return ret;
> +}
> +
> +static int __uniphier_watchdog_start(struct regmap *regmap, unsigned int sec)
> +{
> +	unsigned int val;
> +	int ret;
> +
> +	ret = regmap_read_poll_timeout(regmap, WDTCTRL, val,
> +				       !(val & WDTCTRL_STATUS),
> +				       0, WDTST_TIMEOUT);
> +	if (ret)
> +		return ret;
> +
> +	/* Setup period */
> +	ret = regmap_write(regmap, WDTTIMSET,
> +			   SEC_TO_WDTTIMSET_PRD(sec));
> +	if (ret)
> +		return ret;
> +
> +	/* Enable and clear watchdog */
> +	ret = regmap_write(regmap, WDTCTRL, WDTCTRL_ENABLE | WDTCTRL_CLEAR);
> +	if (!ret)
> +		/*
> +		 * As SoC specification, after clear counter,
> +		 * it needs to wait until counter status is 1.
> +		 */
> +		ret = regmap_read_poll_timeout(regmap, WDTCTRL, val,
> +					       (val & WDTCTRL_STATUS),
> +					       0, WDTST_TIMEOUT);
> +
> +	return ret;
> +}
> +
> +static int __uniphier_watchdog_stop(struct regmap *regmap)
> +{
> +	/* Disable and stop watchdog */
> +	return regmap_write_bits(regmap, WDTCTRL, WDTCTRL_ENABLE, 0);
> +}
> +
> +static int __uniphier_watchdog_restart(struct regmap *regmap, unsigned int sec)
> +{
> +	int ret;
> +
> +	ret = __uniphier_watchdog_stop(regmap);
> +	if (ret)
> +		return ret;
> +
> +	return __uniphier_watchdog_start(regmap, sec);
> +}
> +
> +static int uniphier_watchdog_start(struct watchdog_device *w)
> +{
> +	struct uniphier_wdt_dev *wdev = watchdog_get_drvdata(w);
> +	unsigned int tmp_timeout;
> +
> +	tmp_timeout = roundup_pow_of_two(w->timeout);
> +
> +	return __uniphier_watchdog_start(wdev->regmap, tmp_timeout);
> +}
> +
> +static int uniphier_watchdog_stop(struct watchdog_device *w)
> +{
> +	struct uniphier_wdt_dev *wdev = watchdog_get_drvdata(w);
> +
> +	return __uniphier_watchdog_stop(wdev->regmap);
> +}
> +
> +static int uniphier_watchdog_set_timeout(struct watchdog_device *w,
> +					 unsigned int t)
> +{
> +	struct uniphier_wdt_dev *wdev = watchdog_get_drvdata(w);
> +	unsigned int tmp_timeout;
> +	int ret;
> +
> +	tmp_timeout = roundup_pow_of_two(t);
> +	if (tmp_timeout == w->timeout)
> +		return 0;
> +
> +	if (watchdog_active(w)) {
> +		ret = __uniphier_watchdog_restart(wdev->regmap, tmp_timeout);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	w->timeout = tmp_timeout;
> +
> +	return 0;
> +}
> +
> +/*
> + * Kernel Interfaces
> + */
> +static const struct watchdog_info uniphier_wdt_info = {
> +	.identity	= "uniphier-wdt",
> +	.options	= WDIOF_SETTIMEOUT |
> +			  WDIOF_KEEPALIVEPING |
> +			  WDIOF_MAGICCLOSE |
> +			  WDIOF_OVERHEAT,
> +};
> +
> +static const struct watchdog_ops uniphier_wdt_ops = {
> +	.owner		= THIS_MODULE,
> +	.start		= uniphier_watchdog_start,
> +	.stop		= uniphier_watchdog_stop,
> +	.ping		= uniphier_watchdog_ping,
> +	.set_timeout	= uniphier_watchdog_set_timeout,
> +};
> +
> +static int uniphier_wdt_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct uniphier_wdt_dev *wdev;
> +	struct regmap *regmap;
> +	struct device_node *parent;
> +	int ret;
> +
> +	wdev = devm_kzalloc(dev, sizeof(*wdev), GFP_KERNEL);
> +	if (!wdev)
> +		return -ENOMEM;
> +
> +	platform_set_drvdata(pdev, wdev);
> +
> +	parent = of_get_parent(dev->of_node); /* parent should be syscon node */
> +	regmap = syscon_node_to_regmap(parent);
> +	of_node_put(parent);
> +	if (IS_ERR(regmap))
> +		return PTR_ERR(regmap);
> +
> +	wdev->regmap = regmap;
> +	wdev->wdt_dev.info = &uniphier_wdt_info;
> +	wdev->wdt_dev.ops = &uniphier_wdt_ops;
> +	wdev->wdt_dev.timeout = WDT_DEFAULT_TIMEOUT;
> +	wdev->wdt_dev.max_timeout = WDT_PERIOD_MAX;
> +	wdev->wdt_dev.min_timeout = WDT_PERIOD_MIN;
> +	wdev->wdt_dev.parent = dev;
> +
> +	watchdog_init_timeout(&wdev->wdt_dev, timeout, dev);

This only really makes sense if the default for 'timeout' is 0.
Otherwise it will never take a timeout from devicetree unless timeout is
explicitly set to 0 as module parameter, which does not make much sense.

> +	watchdog_set_nowayout(&wdev->wdt_dev, nowayout);
> +	watchdog_stop_on_reboot(&wdev->wdt_dev);
> +
> +	watchdog_set_drvdata(&wdev->wdt_dev, wdev);
> +
> +	uniphier_watchdog_stop(&wdev->wdt_dev);
> +	ret = regmap_write(wdev->regmap, WDTRSTSEL, WDTRSTSEL_RSTSEL_BOTH);
> +	if (ret)
> +		return ret;
> +
> +	ret = devm_watchdog_register_device(dev, &wdev->wdt_dev);
> +	if (ret)
> +		return ret;
> +
> +	dev_info(dev, "watchdog driver (timeout=%d sec, nowayout=%d)\n",
> +		 wdev->wdt_dev.timeout, nowayout);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id uniphier_wdt_dt_ids[] = {
> +	{ .compatible = "socionext,uniphier-wdt" },
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, uniphier_wdt_dt_ids);
> +
> +static struct platform_driver uniphier_wdt_driver = {
> +	.probe		= uniphier_wdt_probe,
> +	.driver		= {
> +		.name		= "uniphier-wdt",
> +		.of_match_table	= uniphier_wdt_dt_ids,
> +	},
> +};
> +
> +module_platform_driver(uniphier_wdt_driver);
> +
> +module_param(timeout, uint, 0000);
> +MODULE_PARM_DESC(timeout,
> +	"Watchdog timeout seconds in power of 2. (0 < timeout < 128, default="
> +				__MODULE_STRING(WDT_DEFAULT_TIMEOUT) ")");
> +
> +module_param(nowayout, bool, 0000);
> +MODULE_PARM_DESC(nowayout,
> +	"Watchdog cannot be stopped once started (default="
> +				__MODULE_STRING(WATCHDOG_NOWAYOUT) ")");
> +
> +MODULE_AUTHOR("Socionext Inc.");
> +MODULE_DESCRIPTION("UniPhier Watchdog Device Driver");
> +MODULE_LICENSE("GPL v2");
> 

The license here does not match the license in the header (which states GPL2+).
--
To unsubscribe from this list: send the line "unsubscribe devicetree" 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] 18+ messages in thread

* RE: [PATCH V2 2/3] watchdog: uniphier: add UniPhier watchdog driver
@ 2017-06-09 10:22       ` Keiji Hayashibara
  0 siblings, 0 replies; 18+ messages in thread
From: Keiji Hayashibara @ 2017-06-09 10:22 UTC (permalink / raw)
  To: 'Guenter Roeck', wim
  Cc: linux-watchdog, linux-kernel, robh+dt, devicetree,
	masami.hiramatsu, jaswinder.singh, Yamada,
	Masahiro/山田 真弘,
	Hayashi, Kunihiko/林 邦彦,
	Owada, Kiyoshi/大和田 清志

Hi Guenter,

Thank you for your comment.

> On 06/06/2017 02:11 AM, Keiji Hayashibara wrote:
> > Add a watchdog driver for Socionext UniPhier series SoC.
> > Note that the timeout value for this device must be a power
> > of 2 because of the specification.
> >
> > Signed-off-by: Keiji Hayashibara <hayashibara.keiji@socionext.com>
> > ---
> >   Documentation/watchdog/watchdog-parameters.txt |   6 +
> >   drivers/watchdog/Kconfig                       |  11 +
> >   drivers/watchdog/Makefile                      |   1 +
> >   drivers/watchdog/uniphier_wdt.c                | 275 +++++++++++++++++++++++++
> >   4 files changed, 293 insertions(+)
> >   create mode 100644 drivers/watchdog/uniphier_wdt.c
> >
> > diff --git a/Documentation/watchdog/watchdog-parameters.txt b/Documentation/watchdog/watchdog-parameters.txt
> > index 4f7d86d..6f9d7b4 100644
> > --- a/Documentation/watchdog/watchdog-parameters.txt
> > +++ b/Documentation/watchdog/watchdog-parameters.txt
> > @@ -369,6 +369,12 @@ timeout: Watchdog timeout in seconds. (0<timeout<N, default=60)
> >   nowayout: Watchdog cannot be stopped once started
> >   	(default=kernel config parameter)
> >   -------------------------------------------------
> > +uniphier_wdt:
> > +timeout: Watchdog timeout in power of two seconds.
> > +	(1 <= timeout <= 128, default=64)
> > +nowayout: Watchdog cannot be stopped once started
> > +	(default=kernel config parameter)
> > +-------------------------------------------------
> >   w83627hf_wdt:
> >   wdt_io: w83627hf/thf WDT io port (default 0x2E)
> >   timeout: Watchdog timeout in seconds. 1 <= timeout <= 255, default=60.
> > diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> > index 52a70ee..2a9d9a6 100644
> > --- a/drivers/watchdog/Kconfig
> > +++ b/drivers/watchdog/Kconfig
> > @@ -744,6 +744,17 @@ config ZX2967_WATCHDOG
> >   	  To compile this driver as a module, choose M here: the
> >   	  module will be called zx2967_wdt.
> >
> > +config UNIPHIER_WATCHDOG
> > +	tristate "UniPhier watchdog support"
> > +	depends on ARCH_UNIPHIER || COMPILE_TEST
> > +	select WATCHDOG_CORE
> > +	help
> > +	  Say Y here to include support watchdog timer embedded
> > +	  into the UniPhier system.
> > +
> > +	  To compile this driver as a module, choose M here: the
> > +	  module will be called uniphier_wdt.
> > +
> >   # AVR32 Architecture
> >
> >   config AT32AP700X_WDT
> > diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
> > index a2126e2..0928dac 100644
> > --- a/drivers/watchdog/Makefile
> > +++ b/drivers/watchdog/Makefile
> > @@ -84,6 +84,7 @@ obj-$(CONFIG_ATLAS7_WATCHDOG) += atlas7_wdt.o
> >   obj-$(CONFIG_RENESAS_WDT) += renesas_wdt.o
> >   obj-$(CONFIG_ASPEED_WATCHDOG) += aspeed_wdt.o
> >   obj-$(CONFIG_ZX2967_WATCHDOG) += zx2967_wdt.o
> > +obj-$(CONFIG_UNIPHIER_WATCHDOG) += uniphier_wdt.o
> >
> >   # AVR32 Architecture
> >   obj-$(CONFIG_AT32AP700X_WDT) += at32ap700x_wdt.o
> > diff --git a/drivers/watchdog/uniphier_wdt.c b/drivers/watchdog/uniphier_wdt.c
> > new file mode 100644
> > index 0000000..75862a0
> > --- /dev/null
> > +++ b/drivers/watchdog/uniphier_wdt.c
> > @@ -0,0 +1,275 @@
> > +/*
> > + * Watchdog driver for the UniPhier watchdog timer
> > + *
> > + * (c) Copyright 2014 Panasonic Corporation
> > + * (c) Copyright 2016 Socionext Inc.
> > + * All rights reserved.
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License as published by
> > + * the Free Software Foundation; either version 2 of the License, or
> > + * (at your option) any later version.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > + * GNU General Public License for more details.
> > + */
> > +
> > +#include <linux/module.h>
> > +#include <linux/mfd/syscon.h>
> > +#include <linux/of.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/regmap.h>
> > +#include <linux/watchdog.h>
> > +#include <linux/bitops.h>
> > +
> > +/* WDT timer setting register */
> > +#define WDTTIMSET			0x3004
> > +#define   WDTTIMSET_PERIOD_MASK		(0xf << 0)
> > +#define   WDTTIMSET_PERIOD_1_SEC	(0x3 << 0)
> > +#define   WDTTIMSET_PERIOD_2_SEC	(0x4 << 0)
> > +#define   WDTTIMSET_PERIOD_4_SEC	(0x5 << 0)
> > +#define   WDTTIMSET_PERIOD_8_SEC	(0x6 << 0)
> > +#define   WDTTIMSET_PERIOD_16_SEC	(0x7 << 0)
> > +#define   WDTTIMSET_PERIOD_32_SEC	(0x8 << 0)
> > +#define   WDTTIMSET_PERIOD_64_SEC	(0x9 << 0)
> > +#define   WDTTIMSET_PERIOD_128_SEC	(0xa << 0)
> > +
> > +/* WDT reset selection register */
> > +#define WDTRSTSEL			0x3008
> > +#define   WDTRSTSEL_RSTSEL_MASK		(0x3 << 0)
> > +#define   WDTRSTSEL_RSTSEL_BOTH		(0x0 << 0)
> > +#define   WDTRSTSEL_RSTSEL_IRQ_ONLY	(0x2 << 0)
> > +
> > +/* WDT control register */
> > +#define WDTCTRL				0x300c
> > +#define   WDTCTRL_STATUS		BIT(8)
> > +#define   WDTCTRL_CLEAR			BIT(1)
> > +#define   WDTCTRL_ENABLE		BIT(0)
> > +
> > +#define SEC_TO_WDTTIMSET_PRD(sec) \
> > +		(ilog2(sec) + WDTTIMSET_PERIOD_1_SEC)
> > +
> > +#define WDTST_TIMEOUT			1000 /* usec */
> > +
> > +#define WDT_DEFAULT_TIMEOUT		64   /* Default is 64 seconds */
> > +#define WDT_PERIOD_MIN			1
> > +#define WDT_PERIOD_MAX			128
> > +
> > +static unsigned int timeout = WDT_DEFAULT_TIMEOUT;
> 
> This should be initialized to 0 (see below).

OK. I fix it.

> > +static bool nowayout = WATCHDOG_NOWAYOUT;
> > +
> > +struct uniphier_wdt_dev {
> > +	struct watchdog_device wdt_dev;
> > +	struct regmap	*regmap;
> > +};
> > +
> > +/*
> > + * UniPhier Watchdog operations
> > + */
> > +static int uniphier_watchdog_ping(struct watchdog_device *w)
> > +{
> > +	struct uniphier_wdt_dev *wdev = watchdog_get_drvdata(w);
> > +	unsigned int val;
> > +	int ret;
> > +
> > +	/* Clear counter */
> > +	ret = regmap_write_bits(wdev->regmap, WDTCTRL,
> > +				WDTCTRL_CLEAR, WDTCTRL_CLEAR);
> > +	if (!ret)
> > +		/*
> > +		 * As SoC specification, after clear counter,
> > +		 * it needs to wait until counter status is 1.
> > +		 */
> > +		ret = regmap_read_poll_timeout(wdev->regmap, WDTCTRL, val,
> > +					       (val & WDTCTRL_STATUS),
> > +					       0, WDTST_TIMEOUT);
> > +
> > +	return ret;
> > +}
> > +
> > +static int __uniphier_watchdog_start(struct regmap *regmap, unsigned int sec)
> > +{
> > +	unsigned int val;
> > +	int ret;
> > +
> > +	ret = regmap_read_poll_timeout(regmap, WDTCTRL, val,
> > +				       !(val & WDTCTRL_STATUS),
> > +				       0, WDTST_TIMEOUT);
> > +	if (ret)
> > +		return ret;
> > +
> > +	/* Setup period */
> > +	ret = regmap_write(regmap, WDTTIMSET,
> > +			   SEC_TO_WDTTIMSET_PRD(sec));
> > +	if (ret)
> > +		return ret;
> > +
> > +	/* Enable and clear watchdog */
> > +	ret = regmap_write(regmap, WDTCTRL, WDTCTRL_ENABLE | WDTCTRL_CLEAR);
> > +	if (!ret)
> > +		/*
> > +		 * As SoC specification, after clear counter,
> > +		 * it needs to wait until counter status is 1.
> > +		 */
> > +		ret = regmap_read_poll_timeout(regmap, WDTCTRL, val,
> > +					       (val & WDTCTRL_STATUS),
> > +					       0, WDTST_TIMEOUT);
> > +
> > +	return ret;
> > +}
> > +
> > +static int __uniphier_watchdog_stop(struct regmap *regmap)
> > +{
> > +	/* Disable and stop watchdog */
> > +	return regmap_write_bits(regmap, WDTCTRL, WDTCTRL_ENABLE, 0);
> > +}
> > +
> > +static int __uniphier_watchdog_restart(struct regmap *regmap, unsigned int sec)
> > +{
> > +	int ret;
> > +
> > +	ret = __uniphier_watchdog_stop(regmap);
> > +	if (ret)
> > +		return ret;
> > +
> > +	return __uniphier_watchdog_start(regmap, sec);
> > +}
> > +
> > +static int uniphier_watchdog_start(struct watchdog_device *w)
> > +{
> > +	struct uniphier_wdt_dev *wdev = watchdog_get_drvdata(w);
> > +	unsigned int tmp_timeout;
> > +
> > +	tmp_timeout = roundup_pow_of_two(w->timeout);
> > +
> > +	return __uniphier_watchdog_start(wdev->regmap, tmp_timeout);
> > +}
> > +
> > +static int uniphier_watchdog_stop(struct watchdog_device *w)
> > +{
> > +	struct uniphier_wdt_dev *wdev = watchdog_get_drvdata(w);
> > +
> > +	return __uniphier_watchdog_stop(wdev->regmap);
> > +}
> > +
> > +static int uniphier_watchdog_set_timeout(struct watchdog_device *w,
> > +					 unsigned int t)
> > +{
> > +	struct uniphier_wdt_dev *wdev = watchdog_get_drvdata(w);
> > +	unsigned int tmp_timeout;
> > +	int ret;
> > +
> > +	tmp_timeout = roundup_pow_of_two(t);
> > +	if (tmp_timeout == w->timeout)
> > +		return 0;
> > +
> > +	if (watchdog_active(w)) {
> > +		ret = __uniphier_watchdog_restart(wdev->regmap, tmp_timeout);
> > +		if (ret)
> > +			return ret;
> > +	}
> > +
> > +	w->timeout = tmp_timeout;
> > +
> > +	return 0;
> > +}
> > +
> > +/*
> > + * Kernel Interfaces
> > + */
> > +static const struct watchdog_info uniphier_wdt_info = {
> > +	.identity	= "uniphier-wdt",
> > +	.options	= WDIOF_SETTIMEOUT |
> > +			  WDIOF_KEEPALIVEPING |
> > +			  WDIOF_MAGICCLOSE |
> > +			  WDIOF_OVERHEAT,
> > +};
> > +
> > +static const struct watchdog_ops uniphier_wdt_ops = {
> > +	.owner		= THIS_MODULE,
> > +	.start		= uniphier_watchdog_start,
> > +	.stop		= uniphier_watchdog_stop,
> > +	.ping		= uniphier_watchdog_ping,
> > +	.set_timeout	= uniphier_watchdog_set_timeout,
> > +};
> > +
> > +static int uniphier_wdt_probe(struct platform_device *pdev)
> > +{
> > +	struct device *dev = &pdev->dev;
> > +	struct uniphier_wdt_dev *wdev;
> > +	struct regmap *regmap;
> > +	struct device_node *parent;
> > +	int ret;
> > +
> > +	wdev = devm_kzalloc(dev, sizeof(*wdev), GFP_KERNEL);
> > +	if (!wdev)
> > +		return -ENOMEM;
> > +
> > +	platform_set_drvdata(pdev, wdev);
> > +
> > +	parent = of_get_parent(dev->of_node); /* parent should be syscon node */
> > +	regmap = syscon_node_to_regmap(parent);
> > +	of_node_put(parent);
> > +	if (IS_ERR(regmap))
> > +		return PTR_ERR(regmap);
> > +
> > +	wdev->regmap = regmap;
> > +	wdev->wdt_dev.info = &uniphier_wdt_info;
> > +	wdev->wdt_dev.ops = &uniphier_wdt_ops;
> > +	wdev->wdt_dev.timeout = WDT_DEFAULT_TIMEOUT;
> > +	wdev->wdt_dev.max_timeout = WDT_PERIOD_MAX;
> > +	wdev->wdt_dev.min_timeout = WDT_PERIOD_MIN;
> > +	wdev->wdt_dev.parent = dev;
> > +
> > +	watchdog_init_timeout(&wdev->wdt_dev, timeout, dev);
> 
> This only really makes sense if the default for 'timeout' is 0.
> Otherwise it will never take a timeout from devicetree unless timeout is
> explicitly set to 0 as module parameter, which does not make much sense.

It is certainly so.
I will fix it in next v3 patch.


> > +	watchdog_set_nowayout(&wdev->wdt_dev, nowayout);
> > +	watchdog_stop_on_reboot(&wdev->wdt_dev);
> > +
> > +	watchdog_set_drvdata(&wdev->wdt_dev, wdev);
> > +
> > +	uniphier_watchdog_stop(&wdev->wdt_dev);
> > +	ret = regmap_write(wdev->regmap, WDTRSTSEL, WDTRSTSEL_RSTSEL_BOTH);
> > +	if (ret)
> > +		return ret;
> > +
> > +	ret = devm_watchdog_register_device(dev, &wdev->wdt_dev);
> > +	if (ret)
> > +		return ret;
> > +
> > +	dev_info(dev, "watchdog driver (timeout=%d sec, nowayout=%d)\n",
> > +		 wdev->wdt_dev.timeout, nowayout);
> > +
> > +	return 0;
> > +}
> > +
> > +static const struct of_device_id uniphier_wdt_dt_ids[] = {
> > +	{ .compatible = "socionext,uniphier-wdt" },
> > +	{ /* sentinel */ }
> > +};
> > +MODULE_DEVICE_TABLE(of, uniphier_wdt_dt_ids);
> > +
> > +static struct platform_driver uniphier_wdt_driver = {
> > +	.probe		= uniphier_wdt_probe,
> > +	.driver		= {
> > +		.name		= "uniphier-wdt",
> > +		.of_match_table	= uniphier_wdt_dt_ids,
> > +	},
> > +};
> > +
> > +module_platform_driver(uniphier_wdt_driver);
> > +
> > +module_param(timeout, uint, 0000);
> > +MODULE_PARM_DESC(timeout,
> > +	"Watchdog timeout seconds in power of 2. (0 < timeout < 128, default="
> > +				__MODULE_STRING(WDT_DEFAULT_TIMEOUT) ")");
> > +
> > +module_param(nowayout, bool, 0000);
> > +MODULE_PARM_DESC(nowayout,
> > +	"Watchdog cannot be stopped once started (default="
> > +				__MODULE_STRING(WATCHDOG_NOWAYOUT) ")");
> > +
> > +MODULE_AUTHOR("Socionext Inc.");
> > +MODULE_DESCRIPTION("UniPhier Watchdog Device Driver");
> > +MODULE_LICENSE("GPL v2");
> >
> 
> The license here does not match the license in the header (which states GPL2+).

I will modify in next v3 patch.

----------
Best Regards,
Keiji Hayashibara

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

* RE: [PATCH V2 2/3] watchdog: uniphier: add UniPhier watchdog driver
@ 2017-06-09 10:22       ` Keiji Hayashibara
  0 siblings, 0 replies; 18+ messages in thread
From: Keiji Hayashibara @ 2017-06-09 10:22 UTC (permalink / raw)
  To: 'Guenter Roeck', wim-IQzOog9fTRqzQB+pC5nmwQ
  Cc: linux-watchdog-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	masami.hiramatsu-QSEj5FYQhm4dnm+yROfE0A,
	jaswinder.singh-QSEj5FYQhm4dnm+yROfE0A, Yamada,
	Masahiro/山田 真弘,
	Hayashi, Kunihiko/林 邦彦,
	Owada, Kiyoshi/大和田 清志

Hi Guenter,

Thank you for your comment.

> On 06/06/2017 02:11 AM, Keiji Hayashibara wrote:
> > Add a watchdog driver for Socionext UniPhier series SoC.
> > Note that the timeout value for this device must be a power
> > of 2 because of the specification.
> >
> > Signed-off-by: Keiji Hayashibara <hayashibara.keiji-uWyLwvC0a2jby3iVrkZq2A@public.gmane.org>
> > ---
> >   Documentation/watchdog/watchdog-parameters.txt |   6 +
> >   drivers/watchdog/Kconfig                       |  11 +
> >   drivers/watchdog/Makefile                      |   1 +
> >   drivers/watchdog/uniphier_wdt.c                | 275 +++++++++++++++++++++++++
> >   4 files changed, 293 insertions(+)
> >   create mode 100644 drivers/watchdog/uniphier_wdt.c
> >
> > diff --git a/Documentation/watchdog/watchdog-parameters.txt b/Documentation/watchdog/watchdog-parameters.txt
> > index 4f7d86d..6f9d7b4 100644
> > --- a/Documentation/watchdog/watchdog-parameters.txt
> > +++ b/Documentation/watchdog/watchdog-parameters.txt
> > @@ -369,6 +369,12 @@ timeout: Watchdog timeout in seconds. (0<timeout<N, default=60)
> >   nowayout: Watchdog cannot be stopped once started
> >   	(default=kernel config parameter)
> >   -------------------------------------------------
> > +uniphier_wdt:
> > +timeout: Watchdog timeout in power of two seconds.
> > +	(1 <= timeout <= 128, default=64)
> > +nowayout: Watchdog cannot be stopped once started
> > +	(default=kernel config parameter)
> > +-------------------------------------------------
> >   w83627hf_wdt:
> >   wdt_io: w83627hf/thf WDT io port (default 0x2E)
> >   timeout: Watchdog timeout in seconds. 1 <= timeout <= 255, default=60.
> > diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> > index 52a70ee..2a9d9a6 100644
> > --- a/drivers/watchdog/Kconfig
> > +++ b/drivers/watchdog/Kconfig
> > @@ -744,6 +744,17 @@ config ZX2967_WATCHDOG
> >   	  To compile this driver as a module, choose M here: the
> >   	  module will be called zx2967_wdt.
> >
> > +config UNIPHIER_WATCHDOG
> > +	tristate "UniPhier watchdog support"
> > +	depends on ARCH_UNIPHIER || COMPILE_TEST
> > +	select WATCHDOG_CORE
> > +	help
> > +	  Say Y here to include support watchdog timer embedded
> > +	  into the UniPhier system.
> > +
> > +	  To compile this driver as a module, choose M here: the
> > +	  module will be called uniphier_wdt.
> > +
> >   # AVR32 Architecture
> >
> >   config AT32AP700X_WDT
> > diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
> > index a2126e2..0928dac 100644
> > --- a/drivers/watchdog/Makefile
> > +++ b/drivers/watchdog/Makefile
> > @@ -84,6 +84,7 @@ obj-$(CONFIG_ATLAS7_WATCHDOG) += atlas7_wdt.o
> >   obj-$(CONFIG_RENESAS_WDT) += renesas_wdt.o
> >   obj-$(CONFIG_ASPEED_WATCHDOG) += aspeed_wdt.o
> >   obj-$(CONFIG_ZX2967_WATCHDOG) += zx2967_wdt.o
> > +obj-$(CONFIG_UNIPHIER_WATCHDOG) += uniphier_wdt.o
> >
> >   # AVR32 Architecture
> >   obj-$(CONFIG_AT32AP700X_WDT) += at32ap700x_wdt.o
> > diff --git a/drivers/watchdog/uniphier_wdt.c b/drivers/watchdog/uniphier_wdt.c
> > new file mode 100644
> > index 0000000..75862a0
> > --- /dev/null
> > +++ b/drivers/watchdog/uniphier_wdt.c
> > @@ -0,0 +1,275 @@
> > +/*
> > + * Watchdog driver for the UniPhier watchdog timer
> > + *
> > + * (c) Copyright 2014 Panasonic Corporation
> > + * (c) Copyright 2016 Socionext Inc.
> > + * All rights reserved.
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License as published by
> > + * the Free Software Foundation; either version 2 of the License, or
> > + * (at your option) any later version.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > + * GNU General Public License for more details.
> > + */
> > +
> > +#include <linux/module.h>
> > +#include <linux/mfd/syscon.h>
> > +#include <linux/of.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/regmap.h>
> > +#include <linux/watchdog.h>
> > +#include <linux/bitops.h>
> > +
> > +/* WDT timer setting register */
> > +#define WDTTIMSET			0x3004
> > +#define   WDTTIMSET_PERIOD_MASK		(0xf << 0)
> > +#define   WDTTIMSET_PERIOD_1_SEC	(0x3 << 0)
> > +#define   WDTTIMSET_PERIOD_2_SEC	(0x4 << 0)
> > +#define   WDTTIMSET_PERIOD_4_SEC	(0x5 << 0)
> > +#define   WDTTIMSET_PERIOD_8_SEC	(0x6 << 0)
> > +#define   WDTTIMSET_PERIOD_16_SEC	(0x7 << 0)
> > +#define   WDTTIMSET_PERIOD_32_SEC	(0x8 << 0)
> > +#define   WDTTIMSET_PERIOD_64_SEC	(0x9 << 0)
> > +#define   WDTTIMSET_PERIOD_128_SEC	(0xa << 0)
> > +
> > +/* WDT reset selection register */
> > +#define WDTRSTSEL			0x3008
> > +#define   WDTRSTSEL_RSTSEL_MASK		(0x3 << 0)
> > +#define   WDTRSTSEL_RSTSEL_BOTH		(0x0 << 0)
> > +#define   WDTRSTSEL_RSTSEL_IRQ_ONLY	(0x2 << 0)
> > +
> > +/* WDT control register */
> > +#define WDTCTRL				0x300c
> > +#define   WDTCTRL_STATUS		BIT(8)
> > +#define   WDTCTRL_CLEAR			BIT(1)
> > +#define   WDTCTRL_ENABLE		BIT(0)
> > +
> > +#define SEC_TO_WDTTIMSET_PRD(sec) \
> > +		(ilog2(sec) + WDTTIMSET_PERIOD_1_SEC)
> > +
> > +#define WDTST_TIMEOUT			1000 /* usec */
> > +
> > +#define WDT_DEFAULT_TIMEOUT		64   /* Default is 64 seconds */
> > +#define WDT_PERIOD_MIN			1
> > +#define WDT_PERIOD_MAX			128
> > +
> > +static unsigned int timeout = WDT_DEFAULT_TIMEOUT;
> 
> This should be initialized to 0 (see below).

OK. I fix it.

> > +static bool nowayout = WATCHDOG_NOWAYOUT;
> > +
> > +struct uniphier_wdt_dev {
> > +	struct watchdog_device wdt_dev;
> > +	struct regmap	*regmap;
> > +};
> > +
> > +/*
> > + * UniPhier Watchdog operations
> > + */
> > +static int uniphier_watchdog_ping(struct watchdog_device *w)
> > +{
> > +	struct uniphier_wdt_dev *wdev = watchdog_get_drvdata(w);
> > +	unsigned int val;
> > +	int ret;
> > +
> > +	/* Clear counter */
> > +	ret = regmap_write_bits(wdev->regmap, WDTCTRL,
> > +				WDTCTRL_CLEAR, WDTCTRL_CLEAR);
> > +	if (!ret)
> > +		/*
> > +		 * As SoC specification, after clear counter,
> > +		 * it needs to wait until counter status is 1.
> > +		 */
> > +		ret = regmap_read_poll_timeout(wdev->regmap, WDTCTRL, val,
> > +					       (val & WDTCTRL_STATUS),
> > +					       0, WDTST_TIMEOUT);
> > +
> > +	return ret;
> > +}
> > +
> > +static int __uniphier_watchdog_start(struct regmap *regmap, unsigned int sec)
> > +{
> > +	unsigned int val;
> > +	int ret;
> > +
> > +	ret = regmap_read_poll_timeout(regmap, WDTCTRL, val,
> > +				       !(val & WDTCTRL_STATUS),
> > +				       0, WDTST_TIMEOUT);
> > +	if (ret)
> > +		return ret;
> > +
> > +	/* Setup period */
> > +	ret = regmap_write(regmap, WDTTIMSET,
> > +			   SEC_TO_WDTTIMSET_PRD(sec));
> > +	if (ret)
> > +		return ret;
> > +
> > +	/* Enable and clear watchdog */
> > +	ret = regmap_write(regmap, WDTCTRL, WDTCTRL_ENABLE | WDTCTRL_CLEAR);
> > +	if (!ret)
> > +		/*
> > +		 * As SoC specification, after clear counter,
> > +		 * it needs to wait until counter status is 1.
> > +		 */
> > +		ret = regmap_read_poll_timeout(regmap, WDTCTRL, val,
> > +					       (val & WDTCTRL_STATUS),
> > +					       0, WDTST_TIMEOUT);
> > +
> > +	return ret;
> > +}
> > +
> > +static int __uniphier_watchdog_stop(struct regmap *regmap)
> > +{
> > +	/* Disable and stop watchdog */
> > +	return regmap_write_bits(regmap, WDTCTRL, WDTCTRL_ENABLE, 0);
> > +}
> > +
> > +static int __uniphier_watchdog_restart(struct regmap *regmap, unsigned int sec)
> > +{
> > +	int ret;
> > +
> > +	ret = __uniphier_watchdog_stop(regmap);
> > +	if (ret)
> > +		return ret;
> > +
> > +	return __uniphier_watchdog_start(regmap, sec);
> > +}
> > +
> > +static int uniphier_watchdog_start(struct watchdog_device *w)
> > +{
> > +	struct uniphier_wdt_dev *wdev = watchdog_get_drvdata(w);
> > +	unsigned int tmp_timeout;
> > +
> > +	tmp_timeout = roundup_pow_of_two(w->timeout);
> > +
> > +	return __uniphier_watchdog_start(wdev->regmap, tmp_timeout);
> > +}
> > +
> > +static int uniphier_watchdog_stop(struct watchdog_device *w)
> > +{
> > +	struct uniphier_wdt_dev *wdev = watchdog_get_drvdata(w);
> > +
> > +	return __uniphier_watchdog_stop(wdev->regmap);
> > +}
> > +
> > +static int uniphier_watchdog_set_timeout(struct watchdog_device *w,
> > +					 unsigned int t)
> > +{
> > +	struct uniphier_wdt_dev *wdev = watchdog_get_drvdata(w);
> > +	unsigned int tmp_timeout;
> > +	int ret;
> > +
> > +	tmp_timeout = roundup_pow_of_two(t);
> > +	if (tmp_timeout == w->timeout)
> > +		return 0;
> > +
> > +	if (watchdog_active(w)) {
> > +		ret = __uniphier_watchdog_restart(wdev->regmap, tmp_timeout);
> > +		if (ret)
> > +			return ret;
> > +	}
> > +
> > +	w->timeout = tmp_timeout;
> > +
> > +	return 0;
> > +}
> > +
> > +/*
> > + * Kernel Interfaces
> > + */
> > +static const struct watchdog_info uniphier_wdt_info = {
> > +	.identity	= "uniphier-wdt",
> > +	.options	= WDIOF_SETTIMEOUT |
> > +			  WDIOF_KEEPALIVEPING |
> > +			  WDIOF_MAGICCLOSE |
> > +			  WDIOF_OVERHEAT,
> > +};
> > +
> > +static const struct watchdog_ops uniphier_wdt_ops = {
> > +	.owner		= THIS_MODULE,
> > +	.start		= uniphier_watchdog_start,
> > +	.stop		= uniphier_watchdog_stop,
> > +	.ping		= uniphier_watchdog_ping,
> > +	.set_timeout	= uniphier_watchdog_set_timeout,
> > +};
> > +
> > +static int uniphier_wdt_probe(struct platform_device *pdev)
> > +{
> > +	struct device *dev = &pdev->dev;
> > +	struct uniphier_wdt_dev *wdev;
> > +	struct regmap *regmap;
> > +	struct device_node *parent;
> > +	int ret;
> > +
> > +	wdev = devm_kzalloc(dev, sizeof(*wdev), GFP_KERNEL);
> > +	if (!wdev)
> > +		return -ENOMEM;
> > +
> > +	platform_set_drvdata(pdev, wdev);
> > +
> > +	parent = of_get_parent(dev->of_node); /* parent should be syscon node */
> > +	regmap = syscon_node_to_regmap(parent);
> > +	of_node_put(parent);
> > +	if (IS_ERR(regmap))
> > +		return PTR_ERR(regmap);
> > +
> > +	wdev->regmap = regmap;
> > +	wdev->wdt_dev.info = &uniphier_wdt_info;
> > +	wdev->wdt_dev.ops = &uniphier_wdt_ops;
> > +	wdev->wdt_dev.timeout = WDT_DEFAULT_TIMEOUT;
> > +	wdev->wdt_dev.max_timeout = WDT_PERIOD_MAX;
> > +	wdev->wdt_dev.min_timeout = WDT_PERIOD_MIN;
> > +	wdev->wdt_dev.parent = dev;
> > +
> > +	watchdog_init_timeout(&wdev->wdt_dev, timeout, dev);
> 
> This only really makes sense if the default for 'timeout' is 0.
> Otherwise it will never take a timeout from devicetree unless timeout is
> explicitly set to 0 as module parameter, which does not make much sense.

It is certainly so.
I will fix it in next v3 patch.


> > +	watchdog_set_nowayout(&wdev->wdt_dev, nowayout);
> > +	watchdog_stop_on_reboot(&wdev->wdt_dev);
> > +
> > +	watchdog_set_drvdata(&wdev->wdt_dev, wdev);
> > +
> > +	uniphier_watchdog_stop(&wdev->wdt_dev);
> > +	ret = regmap_write(wdev->regmap, WDTRSTSEL, WDTRSTSEL_RSTSEL_BOTH);
> > +	if (ret)
> > +		return ret;
> > +
> > +	ret = devm_watchdog_register_device(dev, &wdev->wdt_dev);
> > +	if (ret)
> > +		return ret;
> > +
> > +	dev_info(dev, "watchdog driver (timeout=%d sec, nowayout=%d)\n",
> > +		 wdev->wdt_dev.timeout, nowayout);
> > +
> > +	return 0;
> > +}
> > +
> > +static const struct of_device_id uniphier_wdt_dt_ids[] = {
> > +	{ .compatible = "socionext,uniphier-wdt" },
> > +	{ /* sentinel */ }
> > +};
> > +MODULE_DEVICE_TABLE(of, uniphier_wdt_dt_ids);
> > +
> > +static struct platform_driver uniphier_wdt_driver = {
> > +	.probe		= uniphier_wdt_probe,
> > +	.driver		= {
> > +		.name		= "uniphier-wdt",
> > +		.of_match_table	= uniphier_wdt_dt_ids,
> > +	},
> > +};
> > +
> > +module_platform_driver(uniphier_wdt_driver);
> > +
> > +module_param(timeout, uint, 0000);
> > +MODULE_PARM_DESC(timeout,
> > +	"Watchdog timeout seconds in power of 2. (0 < timeout < 128, default="
> > +				__MODULE_STRING(WDT_DEFAULT_TIMEOUT) ")");
> > +
> > +module_param(nowayout, bool, 0000);
> > +MODULE_PARM_DESC(nowayout,
> > +	"Watchdog cannot be stopped once started (default="
> > +				__MODULE_STRING(WATCHDOG_NOWAYOUT) ")");
> > +
> > +MODULE_AUTHOR("Socionext Inc.");
> > +MODULE_DESCRIPTION("UniPhier Watchdog Device Driver");
> > +MODULE_LICENSE("GPL v2");
> >
> 
> The license here does not match the license in the header (which states GPL2+).

I will modify in next v3 patch.

----------
Best Regards,
Keiji Hayashibara


--
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] 18+ messages in thread

end of thread, other threads:[~2017-06-09 10:23 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-06-06  9:11 [PATCH V2 0/3] add UniPhier watchdog support Keiji Hayashibara
2017-06-06  9:11 ` Keiji Hayashibara
2017-06-06  9:11 ` [PATCH V2 1/3] devicetree: add UniPhier WDT devicetree bindings documentation Keiji Hayashibara
2017-06-06  9:11   ` Keiji Hayashibara
2017-06-08  2:11   ` Masahiro Yamada
2017-06-08  2:11     ` Masahiro Yamada
2017-06-08  6:22     ` Keiji Hayashibara
2017-06-08  6:22       ` Keiji Hayashibara
2017-06-06  9:11 ` [PATCH V2 2/3] watchdog: uniphier: add UniPhier watchdog driver Keiji Hayashibara
2017-06-08  2:09   ` Masahiro Yamada
2017-06-08  2:09     ` Masahiro Yamada
2017-06-08  5:51     ` Keiji Hayashibara
2017-06-08  5:51       ` Keiji Hayashibara
2017-06-08 12:31   ` Guenter Roeck
2017-06-08 12:31     ` Guenter Roeck
2017-06-09 10:22     ` Keiji Hayashibara
2017-06-09 10:22       ` Keiji Hayashibara
2017-06-06  9:11 ` [PATCH V2 3/3] arm64: dts: uniphier: add watchdog node for LD11 and LD20 Keiji Hayashibara

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.