linux-watchdog.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/9] watchdog: of_xilinx_wdt: Update on watchdog driver
@ 2020-01-16 13:26 Srinivas Neeli
  2020-01-16 13:26 ` [PATCH 1/9] watchdog: of_xilinx_wdt: Add comment to spinlock Srinivas Neeli
                   ` (8 more replies)
  0 siblings, 9 replies; 16+ messages in thread
From: Srinivas Neeli @ 2020-01-16 13:26 UTC (permalink / raw)
  To: linux, michal.simek, shubhrajyoti.datta, sgoud
  Cc: wim, linux-watchdog, linux-arm-kernel, linux-kernel, git

This patch series does the following:
- Add comment to spinlock
- Use BIT macro
- Use dev_dbg()
- Initialize watchdog via data structure
- Introduce wdttype enum for identification
- Add binding for versal watchdog
- Add versal support
- wire setting up timeout via module parameter/DT
- Skip printing pointer value

Srinivas Goud (6):
  watchdog: of_xilinx_wdt: Add comment to spinlock
  watchdog: of_xilinx_wdt: Used BIT macro
  watchdog: of_xilinx_wdt: Used dev_dbg()
  dt-bindings: watchdog: xilinx: Add binding for Versal watchdog
  watchdog: of_xilinx_wdt: Add Versal support
  watchdog: of_xilinx_wdt: Wire setting up timeout via module
    parameter/DT

Srinivas Neeli (3):
  watchdog: of_xilinx_wdt: Initialize watchdog via data structure
  watchdog: of_xilinx_wdt: Introduce wdttype enum for identification
  watchdog: of_xilinx_wdt: Skip printing pointer value

 .../devicetree/bindings/watchdog/of-xilinx-wdt.txt |  21 +-
 drivers/watchdog/of_xilinx_wdt.c                   | 301 ++++++++++++++++++---
 2 files changed, 275 insertions(+), 47 deletions(-)

-- 
2.7.4


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

* [PATCH 1/9] watchdog: of_xilinx_wdt: Add comment to spinlock
  2020-01-16 13:26 [PATCH 0/9] watchdog: of_xilinx_wdt: Update on watchdog driver Srinivas Neeli
@ 2020-01-16 13:26 ` Srinivas Neeli
  2020-01-16 15:48   ` Guenter Roeck
  2020-01-16 13:26 ` [PATCH 2/9] watchdog: of_xilinx_wdt: Used BIT macro Srinivas Neeli
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 16+ messages in thread
From: Srinivas Neeli @ 2020-01-16 13:26 UTC (permalink / raw)
  To: linux, michal.simek, shubhrajyoti.datta, sgoud
  Cc: wim, linux-watchdog, linux-arm-kernel, linux-kernel, git

From: Srinivas Goud <srinivas.goud@xilinx.com>

Based on checkpatch every spinlock should be documented.
The patch is fixing this issue:
./scripts/checkpatch.pl --strict -f drivers/watchdog/of_xilinx_wdt.c
CHECK: spinlock_t definition without comment
+	spinlock_t spinlock;

Signed-off-by: Srinivas Goud <srinivas.goud@xilinx.com>
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---
 drivers/watchdog/of_xilinx_wdt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/watchdog/of_xilinx_wdt.c b/drivers/watchdog/of_xilinx_wdt.c
index 7fe4f7c3f7ce..00549164b3d7 100644
--- a/drivers/watchdog/of_xilinx_wdt.c
+++ b/drivers/watchdog/of_xilinx_wdt.c
@@ -40,7 +40,7 @@
 struct xwdt_device {
 	void __iomem *base;
 	u32 wdt_interval;
-	spinlock_t spinlock;
+	spinlock_t spinlock; /* spinlock for register handling */
 	struct watchdog_device xilinx_wdt_wdd;
 	struct clk		*clk;
 };
-- 
2.7.4


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

* [PATCH 2/9] watchdog: of_xilinx_wdt: Used BIT macro
  2020-01-16 13:26 [PATCH 0/9] watchdog: of_xilinx_wdt: Update on watchdog driver Srinivas Neeli
  2020-01-16 13:26 ` [PATCH 1/9] watchdog: of_xilinx_wdt: Add comment to spinlock Srinivas Neeli
@ 2020-01-16 13:26 ` Srinivas Neeli
  2020-01-16 15:04   ` Guenter Roeck
  2020-01-16 13:26 ` [PATCH 3/9] watchdog: of_xilinx_wdt: Used dev_dbg() Srinivas Neeli
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 16+ messages in thread
From: Srinivas Neeli @ 2020-01-16 13:26 UTC (permalink / raw)
  To: linux, michal.simek, shubhrajyoti.datta, sgoud
  Cc: wim, linux-watchdog, linux-arm-kernel, linux-kernel, git

From: Srinivas Goud <srinivas.goud@xilinx.com>

Used BIT macro instead of mask value.

Signed-off-by: Srinivas Goud <srinivas.goud@xilinx.com>
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---
 drivers/watchdog/of_xilinx_wdt.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/watchdog/of_xilinx_wdt.c b/drivers/watchdog/of_xilinx_wdt.c
index 00549164b3d7..0d7df2370db7 100644
--- a/drivers/watchdog/of_xilinx_wdt.c
+++ b/drivers/watchdog/of_xilinx_wdt.c
@@ -24,12 +24,12 @@
 #define XWT_TBR_OFFSET      0x8 /* Timebase Register Offset */
 
 /* Control/Status Register Masks  */
-#define XWT_CSR0_WRS_MASK   0x00000008 /* Reset status */
-#define XWT_CSR0_WDS_MASK   0x00000004 /* Timer state  */
-#define XWT_CSR0_EWDT1_MASK 0x00000002 /* Enable bit 1 */
+#define XWT_CSR0_WRS_MASK	BIT(3) /* Reset status */
+#define XWT_CSR0_WDS_MASK	BIT(2) /* Timer state  */
+#define XWT_CSR0_EWDT1_MASK	BIT(1) /* Enable bit 1 */
 
 /* Control/Status Register 0/1 bits  */
-#define XWT_CSRX_EWDT2_MASK 0x00000001 /* Enable bit 2 */
+#define XWT_CSRX_EWDT2_MASK	BIT(0) /* Enable bit 2 */
 
 /* SelfTest constants */
 #define XWT_MAX_SELFTEST_LOOP_COUNT 0x00010000
-- 
2.7.4


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

* [PATCH 3/9] watchdog: of_xilinx_wdt: Used dev_dbg()
  2020-01-16 13:26 [PATCH 0/9] watchdog: of_xilinx_wdt: Update on watchdog driver Srinivas Neeli
  2020-01-16 13:26 ` [PATCH 1/9] watchdog: of_xilinx_wdt: Add comment to spinlock Srinivas Neeli
  2020-01-16 13:26 ` [PATCH 2/9] watchdog: of_xilinx_wdt: Used BIT macro Srinivas Neeli
@ 2020-01-16 13:26 ` Srinivas Neeli
  2020-01-16 13:26 ` [PATCH 4/9] watchdog: of_xilinx_wdt: Initialize watchdog via data structure Srinivas Neeli
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 16+ messages in thread
From: Srinivas Neeli @ 2020-01-16 13:26 UTC (permalink / raw)
  To: linux, michal.simek, shubhrajyoti.datta, sgoud
  Cc: wim, linux-watchdog, linux-arm-kernel, linux-kernel, git

From: Srinivas Goud <srinivas.goud@xilinx.com>

This patch removes pr_info in stop function and adds dev_dbg()
in start/stop function to display device specific debug info.

Signed-off-by: Srinivas Goud <srinivas.goud@xilinx.com>
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---
 drivers/watchdog/of_xilinx_wdt.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/watchdog/of_xilinx_wdt.c b/drivers/watchdog/of_xilinx_wdt.c
index 0d7df2370db7..9e67b598907e 100644
--- a/drivers/watchdog/of_xilinx_wdt.c
+++ b/drivers/watchdog/of_xilinx_wdt.c
@@ -50,6 +50,7 @@ static int xilinx_wdt_start(struct watchdog_device *wdd)
 	int ret;
 	u32 control_status_reg;
 	struct xwdt_device *xdev = watchdog_get_drvdata(wdd);
+	struct watchdog_device *xilinx_wdt_wdd = &xdev->xilinx_wdt_wdd;
 
 	ret = clk_enable(xdev->clk);
 	if (ret) {
@@ -70,6 +71,8 @@ static int xilinx_wdt_start(struct watchdog_device *wdd)
 
 	spin_unlock(&xdev->spinlock);
 
+	dev_dbg(xilinx_wdt_wdd->parent, "Watchdog Started!\n");
+
 	return 0;
 }
 
@@ -77,6 +80,7 @@ static int xilinx_wdt_stop(struct watchdog_device *wdd)
 {
 	u32 control_status_reg;
 	struct xwdt_device *xdev = watchdog_get_drvdata(wdd);
+	struct watchdog_device *xilinx_wdt_wdd = &xdev->xilinx_wdt_wdd;
 
 	spin_lock(&xdev->spinlock);
 
@@ -91,7 +95,7 @@ static int xilinx_wdt_stop(struct watchdog_device *wdd)
 
 	clk_disable(xdev->clk);
 
-	pr_info("Stopped!\n");
+	dev_dbg(xilinx_wdt_wdd->parent, "Watchdog Stopped!\n");
 
 	return 0;
 }
-- 
2.7.4


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

* [PATCH 4/9] watchdog: of_xilinx_wdt: Initialize watchdog via data structure
  2020-01-16 13:26 [PATCH 0/9] watchdog: of_xilinx_wdt: Update on watchdog driver Srinivas Neeli
                   ` (2 preceding siblings ...)
  2020-01-16 13:26 ` [PATCH 3/9] watchdog: of_xilinx_wdt: Used dev_dbg() Srinivas Neeli
@ 2020-01-16 13:26 ` Srinivas Neeli
  2020-01-16 13:26 ` [PATCH 5/9] watchdog: of_xilinx_wdt: Introduce wdttype enum for identification Srinivas Neeli
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 16+ messages in thread
From: Srinivas Neeli @ 2020-01-16 13:26 UTC (permalink / raw)
  To: linux, michal.simek, shubhrajyoti.datta, sgoud
  Cc: wim, linux-watchdog, linux-arm-kernel, linux-kernel, git

This patch is preparation for adding new watchdog based on this driver.
of_id->data is storing link xwdt_devtype_data which stores watchdog
info and ops pointers to structures.

Signed-off-by: Srinivas Goud <srinivas.goud@xilinx.com>
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
Signed-off-by: Srinivas Neeli <srinivas.neeli@xilinx.com>
---
 drivers/watchdog/of_xilinx_wdt.c | 50 +++++++++++++++++++++++++++++-----------
 1 file changed, 36 insertions(+), 14 deletions(-)

diff --git a/drivers/watchdog/of_xilinx_wdt.c b/drivers/watchdog/of_xilinx_wdt.c
index 9e67b598907e..b2ce1b90237e 100644
--- a/drivers/watchdog/of_xilinx_wdt.c
+++ b/drivers/watchdog/of_xilinx_wdt.c
@@ -37,6 +37,11 @@
 
 #define WATCHDOG_NAME     "Xilinx Watchdog"
 
+struct xwdt_devtype_data {
+	const struct watchdog_ops *xwdt_ops;
+	const struct watchdog_info *xwdt_info;
+};
+
 struct xwdt_device {
 	void __iomem *base;
 	u32 wdt_interval;
@@ -160,6 +165,20 @@ static void xwdt_clk_disable_unprepare(void *data)
 	clk_disable_unprepare(data);
 }
 
+static const struct xwdt_devtype_data xwdt_wdt_data = {
+	.xwdt_info = &xilinx_wdt_ident,
+	.xwdt_ops = &xilinx_wdt_ops,
+};
+
+static const struct of_device_id xwdt_of_match[] = {
+	{ .compatible = "xlnx,xps-timebase-wdt-1.00.a",
+		.data = &xwdt_wdt_data },
+	{ .compatible = "xlnx,xps-timebase-wdt-1.01.a",
+		.data = &xwdt_wdt_data },
+	{},
+};
+MODULE_DEVICE_TABLE(of, xwdt_of_match);
+
 static int xwdt_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -167,14 +186,23 @@ static int xwdt_probe(struct platform_device *pdev)
 	u32 pfreq = 0, enable_once = 0;
 	struct xwdt_device *xdev;
 	struct watchdog_device *xilinx_wdt_wdd;
+	const struct of_device_id *of_id;
+	const struct xwdt_devtype_data *devtype;
 
 	xdev = devm_kzalloc(dev, sizeof(*xdev), GFP_KERNEL);
 	if (!xdev)
 		return -ENOMEM;
 
 	xilinx_wdt_wdd = &xdev->xilinx_wdt_wdd;
-	xilinx_wdt_wdd->info = &xilinx_wdt_ident;
-	xilinx_wdt_wdd->ops = &xilinx_wdt_ops;
+
+	of_id = of_match_device(xwdt_of_match, &pdev->dev);
+	if (!of_id)
+		return -EINVAL;
+
+	devtype = of_id->data;
+
+	xilinx_wdt_wdd->info = devtype->xwdt_info;
+	xilinx_wdt_wdd->ops = devtype->xwdt_ops;
 	xilinx_wdt_wdd->parent = dev;
 
 	xdev->base = devm_platform_ioremap_resource(pdev, 0);
@@ -264,9 +292,10 @@ static int xwdt_probe(struct platform_device *pdev)
 static int __maybe_unused xwdt_suspend(struct device *dev)
 {
 	struct xwdt_device *xdev = dev_get_drvdata(dev);
+	struct watchdog_device *xilinx_wdt_wdd = &xdev->xilinx_wdt_wdd;
 
-	if (watchdog_active(&xdev->xilinx_wdt_wdd))
-		xilinx_wdt_stop(&xdev->xilinx_wdt_wdd);
+	if (watchdog_active(xilinx_wdt_wdd))
+		xilinx_wdt_wdd->ops->stop(xilinx_wdt_wdd);
 
 	return 0;
 }
@@ -280,24 +309,17 @@ static int __maybe_unused xwdt_suspend(struct device *dev)
 static int __maybe_unused xwdt_resume(struct device *dev)
 {
 	struct xwdt_device *xdev = dev_get_drvdata(dev);
+	struct watchdog_device *xilinx_wdt_wdd = &xdev->xilinx_wdt_wdd;
 	int ret = 0;
 
-	if (watchdog_active(&xdev->xilinx_wdt_wdd))
-		ret = xilinx_wdt_start(&xdev->xilinx_wdt_wdd);
+	if (watchdog_active(xilinx_wdt_wdd))
+		ret = xilinx_wdt_wdd->ops->start(xilinx_wdt_wdd);
 
 	return ret;
 }
 
 static SIMPLE_DEV_PM_OPS(xwdt_pm_ops, xwdt_suspend, xwdt_resume);
 
-/* Match table for of_platform binding */
-static const struct of_device_id xwdt_of_match[] = {
-	{ .compatible = "xlnx,xps-timebase-wdt-1.00.a", },
-	{ .compatible = "xlnx,xps-timebase-wdt-1.01.a", },
-	{},
-};
-MODULE_DEVICE_TABLE(of, xwdt_of_match);
-
 static struct platform_driver xwdt_driver = {
 	.probe       = xwdt_probe,
 	.driver = {
-- 
2.7.4


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

* [PATCH 5/9] watchdog: of_xilinx_wdt: Introduce wdttype enum for identification
  2020-01-16 13:26 [PATCH 0/9] watchdog: of_xilinx_wdt: Update on watchdog driver Srinivas Neeli
                   ` (3 preceding siblings ...)
  2020-01-16 13:26 ` [PATCH 4/9] watchdog: of_xilinx_wdt: Initialize watchdog via data structure Srinivas Neeli
@ 2020-01-16 13:26 ` Srinivas Neeli
  2020-01-16 13:26 ` [PATCH 6/9] dt-bindings: watchdog: xilinx: Add binding for Versal watchdog Srinivas Neeli
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 16+ messages in thread
From: Srinivas Neeli @ 2020-01-16 13:26 UTC (permalink / raw)
  To: linux, michal.simek, shubhrajyoti.datta, sgoud
  Cc: wim, linux-watchdog, linux-arm-kernel, linux-kernel, git

There is a need to identify watchdog type that's why new enum was
was introduced to cover it. Move functionality valid only for this
watchdog type if statement.

Signed-off-by: Srinivas Goud <srinivas.goud@xilinx.com>
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
Signed-off-by: Srinivas Neeli <srinivas.neeli@xilinx.com>
---
 drivers/watchdog/of_xilinx_wdt.c | 64 +++++++++++++++++++++++++++-------------
 1 file changed, 43 insertions(+), 21 deletions(-)

diff --git a/drivers/watchdog/of_xilinx_wdt.c b/drivers/watchdog/of_xilinx_wdt.c
index b2ce1b90237e..3b93b60f1a00 100644
--- a/drivers/watchdog/of_xilinx_wdt.c
+++ b/drivers/watchdog/of_xilinx_wdt.c
@@ -37,7 +37,17 @@
 
 #define WATCHDOG_NAME     "Xilinx Watchdog"
 
+/**
+ * enum xwdt_ip_type - WDT IP type.
+ *
+ * @XWDT_WDT: Soft wdt ip.
+ */
+enum xwdt_ip_type {
+	XWDT_WDT = 0,
+};
+
 struct xwdt_devtype_data {
+	enum xwdt_ip_type wdttype;
 	const struct watchdog_ops *xwdt_ops;
 	const struct watchdog_info *xwdt_info;
 };
@@ -166,6 +176,7 @@ static void xwdt_clk_disable_unprepare(void *data)
 }
 
 static const struct xwdt_devtype_data xwdt_wdt_data = {
+	.wdttype = XWDT_WDT,
 	.xwdt_info = &xilinx_wdt_ident,
 	.xwdt_ops = &xilinx_wdt_ops,
 };
@@ -188,6 +199,7 @@ static int xwdt_probe(struct platform_device *pdev)
 	struct watchdog_device *xilinx_wdt_wdd;
 	const struct of_device_id *of_id;
 	const struct xwdt_devtype_data *devtype;
+	enum xwdt_ip_type wdttype;
 
 	xdev = devm_kzalloc(dev, sizeof(*xdev), GFP_KERNEL);
 	if (!xdev)
@@ -201,6 +213,8 @@ static int xwdt_probe(struct platform_device *pdev)
 
 	devtype = of_id->data;
 
+	wdttype = devtype->wdttype;
+
 	xilinx_wdt_wdd->info = devtype->xwdt_info;
 	xilinx_wdt_wdd->ops = devtype->xwdt_ops;
 	xilinx_wdt_wdd->parent = dev;
@@ -209,18 +223,20 @@ static int xwdt_probe(struct platform_device *pdev)
 	if (IS_ERR(xdev->base))
 		return PTR_ERR(xdev->base);
 
-	rc = of_property_read_u32(dev->of_node, "xlnx,wdt-interval",
-				  &xdev->wdt_interval);
-	if (rc)
-		dev_warn(dev, "Parameter \"xlnx,wdt-interval\" not found\n");
+	if (wdttype == XWDT_WDT) {
+		rc = of_property_read_u32(dev->of_node, "xlnx,wdt-interval",
+					  &xdev->wdt_interval);
+		if (rc)
+			dev_warn(dev, "Parameter \"xlnx,wdt-interval\" not found\n");
 
-	rc = of_property_read_u32(dev->of_node, "xlnx,wdt-enable-once",
-				  &enable_once);
-	if (rc)
-		dev_warn(dev,
-			 "Parameter \"xlnx,wdt-enable-once\" not found\n");
+		rc = of_property_read_u32(dev->of_node, "xlnx,wdt-enable-once",
+					  &enable_once);
+		if (rc)
+			dev_warn(dev,
+				 "Parameter \"xlnx,wdt-enable-once\" not found\n");
 
-	watchdog_set_nowayout(xilinx_wdt_wdd, enable_once);
+		watchdog_set_nowayout(xilinx_wdt_wdd, enable_once);
+	}
 
 	xdev->clk = devm_clk_get(dev, NULL);
 	if (IS_ERR(xdev->clk)) {
@@ -242,13 +258,17 @@ static int xwdt_probe(struct platform_device *pdev)
 		pfreq = clk_get_rate(xdev->clk);
 	}
 
-	/*
-	 * Twice of the 2^wdt_interval / freq  because the first wdt overflow is
-	 * ignored (interrupt), reset is only generated at second wdt overflow
-	 */
-	if (pfreq && xdev->wdt_interval)
-		xilinx_wdt_wdd->timeout = 2 * ((1 << xdev->wdt_interval) /
-					  pfreq);
+	if (wdttype == XWDT_WDT) {
+		/*
+		 * Twice of the 2^wdt_interval / freq  because
+		 * the first wdt overflow is ignored (interrupt),
+		 * reset is only generated at second wdt overflow
+		 */
+		if (pfreq && xdev->wdt_interval)
+			xilinx_wdt_wdd->timeout =
+				2 * ((1 << xdev->wdt_interval) /
+					pfreq);
+	}
 
 	spin_lock_init(&xdev->spinlock);
 	watchdog_set_drvdata(xilinx_wdt_wdd, xdev);
@@ -263,10 +283,12 @@ static int xwdt_probe(struct platform_device *pdev)
 	if (rc)
 		return rc;
 
-	rc = xwdt_selftest(xdev);
-	if (rc == XWT_TIMER_FAILED) {
-		dev_err(dev, "SelfTest routine error\n");
-		return rc;
+	if (wdttype == XWDT_WDT) {
+		rc = xwdt_selftest(xdev);
+		if (rc == XWT_TIMER_FAILED) {
+			dev_err(dev, "SelfTest routine error\n");
+			return rc;
+		}
 	}
 
 	rc = devm_watchdog_register_device(dev, xilinx_wdt_wdd);
-- 
2.7.4


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

* [PATCH 6/9] dt-bindings: watchdog: xilinx: Add binding for Versal watchdog
  2020-01-16 13:26 [PATCH 0/9] watchdog: of_xilinx_wdt: Update on watchdog driver Srinivas Neeli
                   ` (4 preceding siblings ...)
  2020-01-16 13:26 ` [PATCH 5/9] watchdog: of_xilinx_wdt: Introduce wdttype enum for identification Srinivas Neeli
@ 2020-01-16 13:26 ` Srinivas Neeli
  2020-01-16 13:26 ` [PATCH 7/9] watchdog: of_xilinx_wdt: Add Versal support Srinivas Neeli
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 16+ messages in thread
From: Srinivas Neeli @ 2020-01-16 13:26 UTC (permalink / raw)
  To: linux, michal.simek, shubhrajyoti.datta, sgoud
  Cc: wim, linux-watchdog, linux-arm-kernel, linux-kernel, git

From: Srinivas Goud <srinivas.goud@xilinx.com>

Updated watchdog binding for Versal window watchdog.
Added timeout-sec DT property.
timeout-sec is optional property for Versal window watchdog.

Signed-off-by: Srinivas Goud <srinivas.goud@xilinx.com>
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---
 .../devicetree/bindings/watchdog/of-xilinx-wdt.txt  | 21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/watchdog/of-xilinx-wdt.txt b/Documentation/devicetree/bindings/watchdog/of-xilinx-wdt.txt
index c6ae9c9d5e3e..10d68003158d 100644
--- a/Documentation/devicetree/bindings/watchdog/of-xilinx-wdt.txt
+++ b/Documentation/devicetree/bindings/watchdog/of-xilinx-wdt.txt
@@ -1,21 +1,28 @@
-Xilinx AXI/PLB soft-core watchdog Device Tree Bindings
----------------------------------------------------------
+Xilinx AXI/PLB soft-core watchdog and window watchdog Device Tree Bindings
+--------------------------------------------------------------------------
 
 Required properties:
 - compatible		: Should be "xlnx,xps-timebase-wdt-1.00.a" or
-			  "xlnx,xps-timebase-wdt-1.01.a".
+			  "xlnx,xps-timebase-wdt-1.01.a" or
+			  "xlnx,versal-wwdt-1.0".
 - reg			: Physical base address and size
 
 Optional properties:
 - clocks		: Input clock specifier. Refer to common clock
 			  bindings.
 - clock-frequency	: Frequency of clock in Hz
+
+Optional properties for AXI/PLB soft-core watchdog:
 - xlnx,wdt-enable-once	: 0 - Watchdog can be restarted
 			  1 - Watchdog can be enabled just once
 - xlnx,wdt-interval	: Watchdog timeout interval in 2^<val> clock cycles,
 			  <val> is integer from 8 to 31.
 
+Optional properties for window watchdog:
+- timeout-sec          : Watchdog timeout value (in seconds).
+
 Example:
+Xilinx AXI/PLB soft-core watchdog:
 axi-timebase-wdt@40100000 {
 	clock-frequency = <50000000>;
 	compatible = "xlnx,xps-timebase-wdt-1.00.a";
@@ -24,3 +31,11 @@ axi-timebase-wdt@40100000 {
 	xlnx,wdt-enable-once = <0x0>;
 	xlnx,wdt-interval = <0x1b>;
 } ;
+
+Xilinx Versal window watchdog:
+watchdog@fd4d0000 {
+	compatible = "xlnx,versal-wwdt-1.0";
+	reg = <0x0 0xfd4d0000 0x0 0x10000>;
+	clocks = <&clk25>;
+	timeout-sec = <10>;
+} ;
-- 
2.7.4


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

* [PATCH 7/9] watchdog: of_xilinx_wdt: Add Versal support
  2020-01-16 13:26 [PATCH 0/9] watchdog: of_xilinx_wdt: Update on watchdog driver Srinivas Neeli
                   ` (5 preceding siblings ...)
  2020-01-16 13:26 ` [PATCH 6/9] dt-bindings: watchdog: xilinx: Add binding for Versal watchdog Srinivas Neeli
@ 2020-01-16 13:26 ` Srinivas Neeli
  2020-01-16 15:07   ` Guenter Roeck
  2020-01-16 13:26 ` [PATCH 8/9] watchdog: of_xilinx_wdt: Wire setting up timeout via module parameter/DT Srinivas Neeli
  2020-01-16 13:26 ` [PATCH 9/9] watchdog: of_xilinx_wdt: Skip printing pointer value Srinivas Neeli
  8 siblings, 1 reply; 16+ messages in thread
From: Srinivas Neeli @ 2020-01-16 13:26 UTC (permalink / raw)
  To: linux, michal.simek, shubhrajyoti.datta, sgoud
  Cc: wim, linux-watchdog, linux-arm-kernel, linux-kernel, git

From: Srinivas Goud <srinivas.goud@xilinx.com>

Versal watchdog driver uses generic watchdog mode.
Generic watchdog contains closed and open window of equal timeout.
Generic watchdog will generate reset signal if it is not explicitly
refreshed in second window.

Signed-off-by: Srinivas Goud <srinivas.goud@xilinx.com>
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
Signed-off-by: Srinivas Neeli <srinivas.neeli@xilinx.com>
---
 drivers/watchdog/of_xilinx_wdt.c | 150 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 149 insertions(+), 1 deletion(-)

diff --git a/drivers/watchdog/of_xilinx_wdt.c b/drivers/watchdog/of_xilinx_wdt.c
index 3b93b60f1a00..d2c389d9eaa7 100644
--- a/drivers/watchdog/of_xilinx_wdt.c
+++ b/drivers/watchdog/of_xilinx_wdt.c
@@ -2,7 +2,7 @@
 /*
  * Watchdog Device Driver for Xilinx axi/xps_timebase_wdt
  *
- * (C) Copyright 2013 - 2014 Xilinx, Inc.
+ * (C) Copyright 2013 - 2020 Xilinx, Inc.
  * (C) Copyright 2011 (Alejandro Cabrera <aldaya@gmail.com>)
  */
 
@@ -18,10 +18,19 @@
 #include <linux/of_device.h>
 #include <linux/of_address.h>
 
+#define XWT_WWDT_MIN_TIMEOUT		1
+#define XWT_WWDT_MAX_TIMEOUT		80
+
 /* Register offsets for the Wdt device */
 #define XWT_TWCSR0_OFFSET   0x0 /* Control/Status Register0 */
 #define XWT_TWCSR1_OFFSET   0x4 /* Control/Status Register1 */
 #define XWT_TBR_OFFSET      0x8 /* Timebase Register Offset */
+#define XWT_WWREF_OFFSET	0x1000 /* Refresh Register */
+#define XWT_WWCSR_OFFSET	0x2000 /* Control/Status Register */
+#define XWT_WWOFF_OFFSET	0x2008 /* Offset Register */
+#define XWT_WWCMP0_OFFSET	0x2010 /* Compare Value Register0 */
+#define XWT_WWCMP1_OFFSET	0x2014 /* Compare Value Register1 */
+#define XWT_WWWRST_OFFSET	0x2FD0 /* Warm Reset Register */
 
 /* Control/Status Register Masks  */
 #define XWT_CSR0_WRS_MASK	BIT(3) /* Reset status */
@@ -31,6 +40,15 @@
 /* Control/Status Register 0/1 bits  */
 #define XWT_CSRX_EWDT2_MASK	BIT(0) /* Enable bit 2 */
 
+/* Refresh Register Masks */
+#define XWT_WWREF_GWRR_MASK	BIT(0) /* Refresh and start new period */
+
+/* Generic Control/Status Register Masks  */
+#define XWT_WWCSR_GWEN_MASK	BIT(0) /* Enable Bit */
+
+/* Warm Reset Register Masks */
+#define XWT_WWRST_GWWRR_MASK	BIT(0) /* Warm Reset Register */
+
 /* SelfTest constants */
 #define XWT_MAX_SELFTEST_LOOP_COUNT 0x00010000
 #define XWT_TIMER_FAILED            0xFFFFFFFF
@@ -41,9 +59,11 @@
  * enum xwdt_ip_type - WDT IP type.
  *
  * @XWDT_WDT: Soft wdt ip.
+ * @XWDT_WWDT: Window wdt ip.
  */
 enum xwdt_ip_type {
 	XWDT_WDT = 0,
+	XWDT_WWDT,
 };
 
 struct xwdt_devtype_data {
@@ -145,6 +165,126 @@ static const struct watchdog_ops xilinx_wdt_ops = {
 	.ping = xilinx_wdt_keepalive,
 };
 
+static int xilinx_wwdt_start(struct watchdog_device *wdd)
+{
+	int ret;
+	u32 control_status_reg;
+	u64 count;
+	struct xwdt_device *xdev = watchdog_get_drvdata(wdd);
+	struct watchdog_device *xilinx_wdt_wdd = &xdev->xilinx_wdt_wdd;
+
+	unsigned long clock_f = clk_get_rate(xdev->clk);
+
+	/* Calculate timeout count */
+	count = wdd->timeout * clock_f;
+	ret  = clk_enable(xdev->clk);
+	if (ret) {
+		dev_err(wdd->parent, "Failed to enable clock\n");
+		return ret;
+	}
+
+	spin_lock(&xdev->spinlock);
+
+	/*
+	 * Timeout count is half as there are two windows
+	 * first window overflow is ignored (interrupt),
+	 * reset is only generated at second window overflow
+	 */
+	count = count >> 1;
+
+	/* Disable the generic watchdog timer */
+	control_status_reg = ioread32(xdev->base + XWT_WWCSR_OFFSET);
+	control_status_reg &= ~(XWT_WWCSR_GWEN_MASK);
+	iowrite32(control_status_reg, xdev->base + XWT_WWCSR_OFFSET);
+
+	/* Set compare and offset registers for generic watchdog timeout */
+	iowrite32((u32)count, xdev->base + XWT_WWCMP0_OFFSET);
+	iowrite32((u32)0, xdev->base + XWT_WWCMP1_OFFSET);
+	iowrite32((u32)count, xdev->base + XWT_WWOFF_OFFSET);
+
+	/* Enable the generic watchdog timer */
+	control_status_reg = ioread32(xdev->base + XWT_WWCSR_OFFSET);
+	control_status_reg |= (XWT_WWCSR_GWEN_MASK);
+	iowrite32(control_status_reg, xdev->base + XWT_WWCSR_OFFSET);
+
+	spin_unlock(&xdev->spinlock);
+
+	dev_dbg(xilinx_wdt_wdd->parent, "Watchdog Started!\n");
+
+	return 0;
+}
+
+static int xilinx_wwdt_stop(struct watchdog_device *wdd)
+{
+	u32 control_status_reg;
+	struct xwdt_device *xdev = watchdog_get_drvdata(wdd);
+	struct watchdog_device *xilinx_wdt_wdd = &xdev->xilinx_wdt_wdd;
+
+	spin_lock(&xdev->spinlock);
+
+	/* Disable the generic watchdog timer */
+	control_status_reg = ioread32(xdev->base + XWT_WWCSR_OFFSET);
+	control_status_reg &= ~(XWT_WWCSR_GWEN_MASK);
+	iowrite32(control_status_reg, xdev->base + XWT_WWCSR_OFFSET);
+
+	spin_unlock(&xdev->spinlock);
+
+	clk_disable(xdev->clk);
+
+	dev_dbg(xilinx_wdt_wdd->parent, "Watchdog Stopped!\n");
+
+	return 0;
+}
+
+static int xilinx_wwdt_keepalive(struct watchdog_device *wdd)
+{
+	struct xwdt_device *xdev = watchdog_get_drvdata(wdd);
+
+	spin_lock(&xdev->spinlock);
+
+	iowrite32(XWT_WWREF_GWRR_MASK, xdev->base + XWT_WWREF_OFFSET);
+
+	spin_unlock(&xdev->spinlock);
+
+	return 0;
+}
+
+static int xilinx_wwdt_set_timeout(struct watchdog_device *wdd,
+				   unsigned int new_time)
+{
+	struct xwdt_device *xdev = watchdog_get_drvdata(wdd);
+	struct watchdog_device *xilinx_wdt_wdd = &xdev->xilinx_wdt_wdd;
+
+	if (new_time < XWT_WWDT_MIN_TIMEOUT ||
+	    new_time > XWT_WWDT_MAX_TIMEOUT) {
+		dev_warn(xilinx_wdt_wdd->parent,
+			 "timeout value must be %d<=x<=%d, using %d\n",
+				XWT_WWDT_MIN_TIMEOUT,
+				XWT_WWDT_MAX_TIMEOUT, new_time);
+		return -EINVAL;
+	}
+
+	wdd->timeout = new_time;
+
+	return xilinx_wwdt_start(wdd);
+}
+
+static const struct watchdog_info xilinx_wwdt_ident = {
+	.options =  WDIOF_MAGICCLOSE |
+		WDIOF_KEEPALIVEPING |
+		WDIOF_SETTIMEOUT,
+	.firmware_version =	1,
+	.identity = "xlnx_wwdt watchdog",
+};
+
+static const struct watchdog_ops xilinx_wwdt_ops = {
+	.owner = THIS_MODULE,
+	.start = xilinx_wwdt_start,
+	.stop = xilinx_wwdt_stop,
+	.ping = xilinx_wwdt_keepalive,
+	.set_timeout = xilinx_wwdt_set_timeout,
+};
+
 static u32 xwdt_selftest(struct xwdt_device *xdev)
 {
 	int i;
@@ -181,11 +321,19 @@ static const struct xwdt_devtype_data xwdt_wdt_data = {
 	.xwdt_ops = &xilinx_wdt_ops,
 };
 
+static const struct xwdt_devtype_data xwdt_wwdt_data = {
+	.wdttype = XWDT_WWDT,
+	.xwdt_info = &xilinx_wwdt_ident,
+	.xwdt_ops = &xilinx_wwdt_ops,
+};
+
 static const struct of_device_id xwdt_of_match[] = {
 	{ .compatible = "xlnx,xps-timebase-wdt-1.00.a",
 		.data = &xwdt_wdt_data },
 	{ .compatible = "xlnx,xps-timebase-wdt-1.01.a",
 		.data = &xwdt_wdt_data },
+	{ .compatible = "xlnx,versal-wwdt-1.0",
+		.data = &xwdt_wwdt_data },
 	{},
 };
 MODULE_DEVICE_TABLE(of, xwdt_of_match);
-- 
2.7.4


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

* [PATCH 8/9] watchdog: of_xilinx_wdt: Wire setting up timeout via module parameter/DT
  2020-01-16 13:26 [PATCH 0/9] watchdog: of_xilinx_wdt: Update on watchdog driver Srinivas Neeli
                   ` (6 preceding siblings ...)
  2020-01-16 13:26 ` [PATCH 7/9] watchdog: of_xilinx_wdt: Add Versal support Srinivas Neeli
@ 2020-01-16 13:26 ` Srinivas Neeli
  2020-01-16 13:26 ` [PATCH 9/9] watchdog: of_xilinx_wdt: Skip printing pointer value Srinivas Neeli
  8 siblings, 0 replies; 16+ messages in thread
From: Srinivas Neeli @ 2020-01-16 13:26 UTC (permalink / raw)
  To: linux, michal.simek, shubhrajyoti.datta, sgoud
  Cc: wim, linux-watchdog, linux-arm-kernel, linux-kernel, git

From: Srinivas Goud <srinivas.goud@xilinx.com>

Add support for setting up timeout via kernel module parameter or read
timeout-sec via device tree.

Signed-off-by: Srinivas Goud <srinivas.goud@xilinx.com>
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---
 drivers/watchdog/of_xilinx_wdt.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/watchdog/of_xilinx_wdt.c b/drivers/watchdog/of_xilinx_wdt.c
index d2c389d9eaa7..375289e3bed2 100644
--- a/drivers/watchdog/of_xilinx_wdt.c
+++ b/drivers/watchdog/of_xilinx_wdt.c
@@ -18,6 +18,7 @@
 #include <linux/of_device.h>
 #include <linux/of_address.h>
 
+#define XWT_WWDT_DEFAULT_TIMEOUT	10
 #define XWT_WWDT_MIN_TIMEOUT		1
 #define XWT_WWDT_MAX_TIMEOUT		80
 
@@ -55,6 +56,13 @@
 
 #define WATCHDOG_NAME     "Xilinx Watchdog"
 
+static int wdt_timeout;
+
+module_param(wdt_timeout, int, 0644);
+MODULE_PARM_DESC(wdt_timeout,
+		 "Watchdog time in seconds. (default="
+		 __MODULE_STRING(XWT_WWDT_DEFAULT_TIMEOUT) ")");
+
 /**
  * enum xwdt_ip_type - WDT IP type.
  *
@@ -416,6 +424,15 @@ static int xwdt_probe(struct platform_device *pdev)
 			xilinx_wdt_wdd->timeout =
 				2 * ((1 << xdev->wdt_interval) /
 					pfreq);
+	} else {
+		xilinx_wdt_wdd->timeout = XWT_WWDT_DEFAULT_TIMEOUT;
+		xilinx_wdt_wdd->min_timeout = XWT_WWDT_MIN_TIMEOUT;
+		xilinx_wdt_wdd->max_timeout = XWT_WWDT_MAX_TIMEOUT;
+
+		rc = watchdog_init_timeout(xilinx_wdt_wdd,
+					   wdt_timeout, &pdev->dev);
+		if (rc)
+			dev_warn(&pdev->dev, "unable to set timeout value\n");
 	}
 
 	spin_lock_init(&xdev->spinlock);
-- 
2.7.4


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

* [PATCH 9/9] watchdog: of_xilinx_wdt: Skip printing pointer value
  2020-01-16 13:26 [PATCH 0/9] watchdog: of_xilinx_wdt: Update on watchdog driver Srinivas Neeli
                   ` (7 preceding siblings ...)
  2020-01-16 13:26 ` [PATCH 8/9] watchdog: of_xilinx_wdt: Wire setting up timeout via module parameter/DT Srinivas Neeli
@ 2020-01-16 13:26 ` Srinivas Neeli
  8 siblings, 0 replies; 16+ messages in thread
From: Srinivas Neeli @ 2020-01-16 13:26 UTC (permalink / raw)
  To: linux, michal.simek, shubhrajyoti.datta, sgoud
  Cc: wim, linux-watchdog, linux-arm-kernel, linux-kernel, git

"%p" is not printing the pointer value.
In driver, printing pointer value is not useful so avoiding print.

Signed-off-by: Srinivas Neeli <srinivas.neeli@xilinx.com>
---
 drivers/watchdog/of_xilinx_wdt.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/watchdog/of_xilinx_wdt.c b/drivers/watchdog/of_xilinx_wdt.c
index 375289e3bed2..37133527977d 100644
--- a/drivers/watchdog/of_xilinx_wdt.c
+++ b/drivers/watchdog/of_xilinx_wdt.c
@@ -462,8 +462,8 @@ static int xwdt_probe(struct platform_device *pdev)
 
 	clk_disable(xdev->clk);
 
-	dev_info(dev, "Xilinx Watchdog Timer at %p with timeout %ds\n",
-		 xdev->base, xilinx_wdt_wdd->timeout);
+	dev_info(dev, "Xilinx Watchdog Timer with timeout %ds\n",
+		 xilinx_wdt_wdd->timeout);
 
 	platform_set_drvdata(pdev, xdev);
 
-- 
2.7.4


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

* Re: [PATCH 2/9] watchdog: of_xilinx_wdt: Used BIT macro
  2020-01-16 13:26 ` [PATCH 2/9] watchdog: of_xilinx_wdt: Used BIT macro Srinivas Neeli
@ 2020-01-16 15:04   ` Guenter Roeck
  0 siblings, 0 replies; 16+ messages in thread
From: Guenter Roeck @ 2020-01-16 15:04 UTC (permalink / raw)
  To: Srinivas Neeli, michal.simek, shubhrajyoti.datta, sgoud
  Cc: wim, linux-watchdog, linux-arm-kernel, linux-kernel, git

On 1/16/20 5:26 AM, Srinivas Neeli wrote:
> From: Srinivas Goud <srinivas.goud@xilinx.com>
> 
> Used BIT macro instead of mask value.
> 
> Signed-off-by: Srinivas Goud <srinivas.goud@xilinx.com>
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> ---
>   drivers/watchdog/of_xilinx_wdt.c | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/watchdog/of_xilinx_wdt.c b/drivers/watchdog/of_xilinx_wdt.c
> index 00549164b3d7..0d7df2370db7 100644
> --- a/drivers/watchdog/of_xilinx_wdt.c
> +++ b/drivers/watchdog/of_xilinx_wdt.c
> @@ -24,12 +24,12 @@
>   #define XWT_TBR_OFFSET      0x8 /* Timebase Register Offset */
>   
>   /* Control/Status Register Masks  */
> -#define XWT_CSR0_WRS_MASK   0x00000008 /* Reset status */
> -#define XWT_CSR0_WDS_MASK   0x00000004 /* Timer state  */
> -#define XWT_CSR0_EWDT1_MASK 0x00000002 /* Enable bit 1 */
> +#define XWT_CSR0_WRS_MASK	BIT(3) /* Reset status */
> +#define XWT_CSR0_WDS_MASK	BIT(2) /* Timer state  */
> +#define XWT_CSR0_EWDT1_MASK	BIT(1) /* Enable bit 1 */
>   
>   /* Control/Status Register 0/1 bits  */
> -#define XWT_CSRX_EWDT2_MASK 0x00000001 /* Enable bit 2 */
> +#define XWT_CSRX_EWDT2_MASK	BIT(0) /* Enable bit 2 */
>   
>   /* SelfTest constants */
>   #define XWT_MAX_SELFTEST_LOOP_COUNT 0x00010000
> 
Using bitops also requires including linux/bits.h explicitly.

Guenter



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

* Re: [PATCH 7/9] watchdog: of_xilinx_wdt: Add Versal support
  2020-01-16 13:26 ` [PATCH 7/9] watchdog: of_xilinx_wdt: Add Versal support Srinivas Neeli
@ 2020-01-16 15:07   ` Guenter Roeck
  0 siblings, 0 replies; 16+ messages in thread
From: Guenter Roeck @ 2020-01-16 15:07 UTC (permalink / raw)
  To: Srinivas Neeli, michal.simek, shubhrajyoti.datta, sgoud
  Cc: wim, linux-watchdog, linux-arm-kernel, linux-kernel, git

On 1/16/20 5:26 AM, Srinivas Neeli wrote:
> From: Srinivas Goud <srinivas.goud@xilinx.com>
> 
> Versal watchdog driver uses generic watchdog mode.
> Generic watchdog contains closed and open window of equal timeout.
> Generic watchdog will generate reset signal if it is not explicitly
> refreshed in second window.
> 

WHy don't you just implement a separate driver for this watchdog ?
It seems to be completely different. I don't immediately see the
benefit of having a single driver. If there is one, please explain
in detail.

Thanks,
Guenter

> Signed-off-by: Srinivas Goud <srinivas.goud@xilinx.com>
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> Signed-off-by: Srinivas Neeli <srinivas.neeli@xilinx.com>
> ---
>   drivers/watchdog/of_xilinx_wdt.c | 150 ++++++++++++++++++++++++++++++++++++++-
>   1 file changed, 149 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/watchdog/of_xilinx_wdt.c b/drivers/watchdog/of_xilinx_wdt.c
> index 3b93b60f1a00..d2c389d9eaa7 100644
> --- a/drivers/watchdog/of_xilinx_wdt.c
> +++ b/drivers/watchdog/of_xilinx_wdt.c
> @@ -2,7 +2,7 @@
>   /*
>    * Watchdog Device Driver for Xilinx axi/xps_timebase_wdt
>    *
> - * (C) Copyright 2013 - 2014 Xilinx, Inc.
> + * (C) Copyright 2013 - 2020 Xilinx, Inc.
>    * (C) Copyright 2011 (Alejandro Cabrera <aldaya@gmail.com>)
>    */
>   
> @@ -18,10 +18,19 @@
>   #include <linux/of_device.h>
>   #include <linux/of_address.h>
>   
> +#define XWT_WWDT_MIN_TIMEOUT		1
> +#define XWT_WWDT_MAX_TIMEOUT		80
> +
>   /* Register offsets for the Wdt device */
>   #define XWT_TWCSR0_OFFSET   0x0 /* Control/Status Register0 */
>   #define XWT_TWCSR1_OFFSET   0x4 /* Control/Status Register1 */
>   #define XWT_TBR_OFFSET      0x8 /* Timebase Register Offset */
> +#define XWT_WWREF_OFFSET	0x1000 /* Refresh Register */
> +#define XWT_WWCSR_OFFSET	0x2000 /* Control/Status Register */
> +#define XWT_WWOFF_OFFSET	0x2008 /* Offset Register */
> +#define XWT_WWCMP0_OFFSET	0x2010 /* Compare Value Register0 */
> +#define XWT_WWCMP1_OFFSET	0x2014 /* Compare Value Register1 */
> +#define XWT_WWWRST_OFFSET	0x2FD0 /* Warm Reset Register */
>   
>   /* Control/Status Register Masks  */
>   #define XWT_CSR0_WRS_MASK	BIT(3) /* Reset status */
> @@ -31,6 +40,15 @@
>   /* Control/Status Register 0/1 bits  */
>   #define XWT_CSRX_EWDT2_MASK	BIT(0) /* Enable bit 2 */
>   
> +/* Refresh Register Masks */
> +#define XWT_WWREF_GWRR_MASK	BIT(0) /* Refresh and start new period */
> +
> +/* Generic Control/Status Register Masks  */
> +#define XWT_WWCSR_GWEN_MASK	BIT(0) /* Enable Bit */
> +
> +/* Warm Reset Register Masks */
> +#define XWT_WWRST_GWWRR_MASK	BIT(0) /* Warm Reset Register */
> +
>   /* SelfTest constants */
>   #define XWT_MAX_SELFTEST_LOOP_COUNT 0x00010000
>   #define XWT_TIMER_FAILED            0xFFFFFFFF
> @@ -41,9 +59,11 @@
>    * enum xwdt_ip_type - WDT IP type.
>    *
>    * @XWDT_WDT: Soft wdt ip.
> + * @XWDT_WWDT: Window wdt ip.
>    */
>   enum xwdt_ip_type {
>   	XWDT_WDT = 0,
> +	XWDT_WWDT,
>   };
>   
>   struct xwdt_devtype_data {
> @@ -145,6 +165,126 @@ static const struct watchdog_ops xilinx_wdt_ops = {
>   	.ping = xilinx_wdt_keepalive,
>   };
>   
> +static int xilinx_wwdt_start(struct watchdog_device *wdd)
> +{
> +	int ret;
> +	u32 control_status_reg;
> +	u64 count;
> +	struct xwdt_device *xdev = watchdog_get_drvdata(wdd);
> +	struct watchdog_device *xilinx_wdt_wdd = &xdev->xilinx_wdt_wdd;
> +
> +	unsigned long clock_f = clk_get_rate(xdev->clk);
> +
> +	/* Calculate timeout count */
> +	count = wdd->timeout * clock_f;
> +	ret  = clk_enable(xdev->clk);
> +	if (ret) {
> +		dev_err(wdd->parent, "Failed to enable clock\n");
> +		return ret;
> +	}
> +
> +	spin_lock(&xdev->spinlock);
> +
> +	/*
> +	 * Timeout count is half as there are two windows
> +	 * first window overflow is ignored (interrupt),
> +	 * reset is only generated at second window overflow
> +	 */
> +	count = count >> 1;
> +
> +	/* Disable the generic watchdog timer */
> +	control_status_reg = ioread32(xdev->base + XWT_WWCSR_OFFSET);
> +	control_status_reg &= ~(XWT_WWCSR_GWEN_MASK);
> +	iowrite32(control_status_reg, xdev->base + XWT_WWCSR_OFFSET);
> +
> +	/* Set compare and offset registers for generic watchdog timeout */
> +	iowrite32((u32)count, xdev->base + XWT_WWCMP0_OFFSET);
> +	iowrite32((u32)0, xdev->base + XWT_WWCMP1_OFFSET);
> +	iowrite32((u32)count, xdev->base + XWT_WWOFF_OFFSET);
> +
> +	/* Enable the generic watchdog timer */
> +	control_status_reg = ioread32(xdev->base + XWT_WWCSR_OFFSET);
> +	control_status_reg |= (XWT_WWCSR_GWEN_MASK);
> +	iowrite32(control_status_reg, xdev->base + XWT_WWCSR_OFFSET);
> +
> +	spin_unlock(&xdev->spinlock);
> +
> +	dev_dbg(xilinx_wdt_wdd->parent, "Watchdog Started!\n");
> +
> +	return 0;
> +}
> +
> +static int xilinx_wwdt_stop(struct watchdog_device *wdd)
> +{
> +	u32 control_status_reg;
> +	struct xwdt_device *xdev = watchdog_get_drvdata(wdd);
> +	struct watchdog_device *xilinx_wdt_wdd = &xdev->xilinx_wdt_wdd;
> +
> +	spin_lock(&xdev->spinlock);
> +
> +	/* Disable the generic watchdog timer */
> +	control_status_reg = ioread32(xdev->base + XWT_WWCSR_OFFSET);
> +	control_status_reg &= ~(XWT_WWCSR_GWEN_MASK);
> +	iowrite32(control_status_reg, xdev->base + XWT_WWCSR_OFFSET);
> +
> +	spin_unlock(&xdev->spinlock);
> +
> +	clk_disable(xdev->clk);
> +
> +	dev_dbg(xilinx_wdt_wdd->parent, "Watchdog Stopped!\n");
> +
> +	return 0;
> +}
> +
> +static int xilinx_wwdt_keepalive(struct watchdog_device *wdd)
> +{
> +	struct xwdt_device *xdev = watchdog_get_drvdata(wdd);
> +
> +	spin_lock(&xdev->spinlock);
> +
> +	iowrite32(XWT_WWREF_GWRR_MASK, xdev->base + XWT_WWREF_OFFSET);
> +
> +	spin_unlock(&xdev->spinlock);
> +
> +	return 0;
> +}
> +
> +static int xilinx_wwdt_set_timeout(struct watchdog_device *wdd,
> +				   unsigned int new_time)
> +{
> +	struct xwdt_device *xdev = watchdog_get_drvdata(wdd);
> +	struct watchdog_device *xilinx_wdt_wdd = &xdev->xilinx_wdt_wdd;
> +
> +	if (new_time < XWT_WWDT_MIN_TIMEOUT ||
> +	    new_time > XWT_WWDT_MAX_TIMEOUT) {
> +		dev_warn(xilinx_wdt_wdd->parent,
> +			 "timeout value must be %d<=x<=%d, using %d\n",
> +				XWT_WWDT_MIN_TIMEOUT,
> +				XWT_WWDT_MAX_TIMEOUT, new_time);
> +		return -EINVAL;
> +	}
> +
> +	wdd->timeout = new_time;
> +
> +	return xilinx_wwdt_start(wdd);
> +}
> +
> +static const struct watchdog_info xilinx_wwdt_ident = {
> +	.options =  WDIOF_MAGICCLOSE |
> +		WDIOF_KEEPALIVEPING |
> +		WDIOF_SETTIMEOUT,
> +	.firmware_version =	1,
> +	.identity = "xlnx_wwdt watchdog",
> +};
> +
> +static const struct watchdog_ops xilinx_wwdt_ops = {
> +	.owner = THIS_MODULE,
> +	.start = xilinx_wwdt_start,
> +	.stop = xilinx_wwdt_stop,
> +	.ping = xilinx_wwdt_keepalive,
> +	.set_timeout = xilinx_wwdt_set_timeout,
> +};
> +
>   static u32 xwdt_selftest(struct xwdt_device *xdev)
>   {
>   	int i;
> @@ -181,11 +321,19 @@ static const struct xwdt_devtype_data xwdt_wdt_data = {
>   	.xwdt_ops = &xilinx_wdt_ops,
>   };
>   
> +static const struct xwdt_devtype_data xwdt_wwdt_data = {
> +	.wdttype = XWDT_WWDT,
> +	.xwdt_info = &xilinx_wwdt_ident,
> +	.xwdt_ops = &xilinx_wwdt_ops,
> +};
> +
>   static const struct of_device_id xwdt_of_match[] = {
>   	{ .compatible = "xlnx,xps-timebase-wdt-1.00.a",
>   		.data = &xwdt_wdt_data },
>   	{ .compatible = "xlnx,xps-timebase-wdt-1.01.a",
>   		.data = &xwdt_wdt_data },
> +	{ .compatible = "xlnx,versal-wwdt-1.0",
> +		.data = &xwdt_wwdt_data },
>   	{},
>   };
>   MODULE_DEVICE_TABLE(of, xwdt_of_match);
> 


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

* Re: [PATCH 1/9] watchdog: of_xilinx_wdt: Add comment to spinlock
  2020-01-16 13:26 ` [PATCH 1/9] watchdog: of_xilinx_wdt: Add comment to spinlock Srinivas Neeli
@ 2020-01-16 15:48   ` Guenter Roeck
  0 siblings, 0 replies; 16+ messages in thread
From: Guenter Roeck @ 2020-01-16 15:48 UTC (permalink / raw)
  To: Srinivas Neeli
  Cc: michal.simek, shubhrajyoti.datta, sgoud, wim, linux-watchdog,
	linux-arm-kernel, linux-kernel, git

On Thu, Jan 16, 2020 at 06:56:49PM +0530, Srinivas Neeli wrote:
> From: Srinivas Goud <srinivas.goud@xilinx.com>
> 
> Based on checkpatch every spinlock should be documented.
> The patch is fixing this issue:
> ./scripts/checkpatch.pl --strict -f drivers/watchdog/of_xilinx_wdt.c
> CHECK: spinlock_t definition without comment
> +	spinlock_t spinlock;

One of the most useless feedback messages from checkpatch.

> 
> Signed-off-by: Srinivas Goud <srinivas.goud@xilinx.com>
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> ---
>  drivers/watchdog/of_xilinx_wdt.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/watchdog/of_xilinx_wdt.c b/drivers/watchdog/of_xilinx_wdt.c
> index 7fe4f7c3f7ce..00549164b3d7 100644
> --- a/drivers/watchdog/of_xilinx_wdt.c
> +++ b/drivers/watchdog/of_xilinx_wdt.c
> @@ -40,7 +40,7 @@
>  struct xwdt_device {
>  	void __iomem *base;
>  	u32 wdt_interval;
> -	spinlock_t spinlock;
> +	spinlock_t spinlock; /* spinlock for register handling */

I don't see the added value here. Besides, what does the lock actually do ?
Watchdog drivers are single-open, so it seems quite difficult for any
of the protected functions to be called multiple times. The spinlock doesn't
disable interrupts, so register accesses by other drivers are still possible.
What am I missing ?

Guenter

>  	struct watchdog_device xilinx_wdt_wdd;
>  	struct clk		*clk;
>  };
> -- 
> 2.7.4
> 

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

* RE: [PATCH 2/9] watchdog: of_xilinx_wdt: Used BIT macro
  2021-03-16  2:23   ` Guenter Roeck
@ 2021-03-24  6:06     ` Srinivas Neeli
  0 siblings, 0 replies; 16+ messages in thread
From: Srinivas Neeli @ 2021-03-24  6:06 UTC (permalink / raw)
  To: Guenter Roeck, Michal Simek, Shubhrajyoti Datta, Srinivas Goud
  Cc: wim, linux-watchdog, linux-arm-kernel, linux-kernel, git, Srinivas Goud

Hi,

> -----Original Message-----
> From: Guenter Roeck <groeck7@gmail.com> On Behalf Of Guenter Roeck
> Sent: Tuesday, March 16, 2021 7:53 AM
> To: Srinivas Neeli <sneeli@xilinx.com>; Michal Simek <michals@xilinx.com>;
> Shubhrajyoti Datta <shubhraj@xilinx.com>; Srinivas Goud
> <sgoud@xilinx.com>
> Cc: wim@linux-watchdog.org; linux-watchdog@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org; linux-kernel@vger.kernel.org; git
> <git@xilinx.com>; Srinivas Goud <sgoud@xilinx.com>
> Subject: Re: [PATCH 2/9] watchdog: of_xilinx_wdt: Used BIT macro
> 
> On 3/15/21 3:46 AM, Srinivas Neeli wrote:
> > From: Srinivas Goud <srinivas.goud@xilinx.com>
> >
> > Used BIT macro instead of mask value.
> >
> > Signed-off-by: Srinivas Goud <srinivas.goud@xilinx.com>
> > Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> > Signed-off-by: Srinivas Neeli <srinivas.neeli@xilinx.com>
> > ---
> >  drivers/watchdog/of_xilinx_wdt.c | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/watchdog/of_xilinx_wdt.c
> > b/drivers/watchdog/of_xilinx_wdt.c
> > index 00549164b3d7..0d7df2370db7 100644
> > --- a/drivers/watchdog/of_xilinx_wdt.c
> > +++ b/drivers/watchdog/of_xilinx_wdt.c
> > @@ -24,12 +24,12 @@
> >  #define XWT_TBR_OFFSET      0x8 /* Timebase Register Offset */
> >
> >  /* Control/Status Register Masks  */
> > -#define XWT_CSR0_WRS_MASK   0x00000008 /* Reset status */
> > -#define XWT_CSR0_WDS_MASK   0x00000004 /* Timer state  */
> > -#define XWT_CSR0_EWDT1_MASK 0x00000002 /* Enable bit 1 */
> > +#define XWT_CSR0_WRS_MASK	BIT(3) /* Reset status */
> > +#define XWT_CSR0_WDS_MASK	BIT(2) /* Timer state  */
> > +#define XWT_CSR0_EWDT1_MASK	BIT(1) /* Enable bit 1 */
> >
> >  /* Control/Status Register 0/1 bits  */ -#define XWT_CSRX_EWDT2_MASK
> > 0x00000001 /* Enable bit 2 */
> > +#define XWT_CSRX_EWDT2_MASK	BIT(0) /* Enable bit 2 */
> >
> 
> Requires #include <linux/bits.h>

Will update in V2 series.

> 
> >  /* SelfTest constants */
> >  #define XWT_MAX_SELFTEST_LOOP_COUNT 0x00010000
> >


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

* Re: [PATCH 2/9] watchdog: of_xilinx_wdt: Used BIT macro
  2021-03-15 10:46 ` [PATCH 2/9] watchdog: of_xilinx_wdt: Used BIT macro Srinivas Neeli
@ 2021-03-16  2:23   ` Guenter Roeck
  2021-03-24  6:06     ` Srinivas Neeli
  0 siblings, 1 reply; 16+ messages in thread
From: Guenter Roeck @ 2021-03-16  2:23 UTC (permalink / raw)
  To: Srinivas Neeli, michal.simek, shubhrajyoti.datta, sgoud
  Cc: wim, linux-watchdog, linux-arm-kernel, linux-kernel, git, Srinivas Goud

On 3/15/21 3:46 AM, Srinivas Neeli wrote:
> From: Srinivas Goud <srinivas.goud@xilinx.com>
> 
> Used BIT macro instead of mask value.
> 
> Signed-off-by: Srinivas Goud <srinivas.goud@xilinx.com>
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> Signed-off-by: Srinivas Neeli <srinivas.neeli@xilinx.com>
> ---
>  drivers/watchdog/of_xilinx_wdt.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/watchdog/of_xilinx_wdt.c b/drivers/watchdog/of_xilinx_wdt.c
> index 00549164b3d7..0d7df2370db7 100644
> --- a/drivers/watchdog/of_xilinx_wdt.c
> +++ b/drivers/watchdog/of_xilinx_wdt.c
> @@ -24,12 +24,12 @@
>  #define XWT_TBR_OFFSET      0x8 /* Timebase Register Offset */
>  
>  /* Control/Status Register Masks  */
> -#define XWT_CSR0_WRS_MASK   0x00000008 /* Reset status */
> -#define XWT_CSR0_WDS_MASK   0x00000004 /* Timer state  */
> -#define XWT_CSR0_EWDT1_MASK 0x00000002 /* Enable bit 1 */
> +#define XWT_CSR0_WRS_MASK	BIT(3) /* Reset status */
> +#define XWT_CSR0_WDS_MASK	BIT(2) /* Timer state  */
> +#define XWT_CSR0_EWDT1_MASK	BIT(1) /* Enable bit 1 */
>  
>  /* Control/Status Register 0/1 bits  */
> -#define XWT_CSRX_EWDT2_MASK 0x00000001 /* Enable bit 2 */
> +#define XWT_CSRX_EWDT2_MASK	BIT(0) /* Enable bit 2 */
>  

Requires #include <linux/bits.h>

>  /* SelfTest constants */
>  #define XWT_MAX_SELFTEST_LOOP_COUNT 0x00010000
> 


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

* [PATCH 2/9] watchdog: of_xilinx_wdt: Used BIT macro
  2021-03-15 10:46 [PATCH 0/9] watchdog: of_xilinx_wdt: Update on xilinx watchdog driver Srinivas Neeli
@ 2021-03-15 10:46 ` Srinivas Neeli
  2021-03-16  2:23   ` Guenter Roeck
  0 siblings, 1 reply; 16+ messages in thread
From: Srinivas Neeli @ 2021-03-15 10:46 UTC (permalink / raw)
  To: linux, michal.simek, shubhrajyoti.datta, sgoud
  Cc: wim, linux-watchdog, linux-arm-kernel, linux-kernel, git,
	Srinivas Goud, Srinivas Neeli

From: Srinivas Goud <srinivas.goud@xilinx.com>

Used BIT macro instead of mask value.

Signed-off-by: Srinivas Goud <srinivas.goud@xilinx.com>
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
Signed-off-by: Srinivas Neeli <srinivas.neeli@xilinx.com>
---
 drivers/watchdog/of_xilinx_wdt.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/watchdog/of_xilinx_wdt.c b/drivers/watchdog/of_xilinx_wdt.c
index 00549164b3d7..0d7df2370db7 100644
--- a/drivers/watchdog/of_xilinx_wdt.c
+++ b/drivers/watchdog/of_xilinx_wdt.c
@@ -24,12 +24,12 @@
 #define XWT_TBR_OFFSET      0x8 /* Timebase Register Offset */
 
 /* Control/Status Register Masks  */
-#define XWT_CSR0_WRS_MASK   0x00000008 /* Reset status */
-#define XWT_CSR0_WDS_MASK   0x00000004 /* Timer state  */
-#define XWT_CSR0_EWDT1_MASK 0x00000002 /* Enable bit 1 */
+#define XWT_CSR0_WRS_MASK	BIT(3) /* Reset status */
+#define XWT_CSR0_WDS_MASK	BIT(2) /* Timer state  */
+#define XWT_CSR0_EWDT1_MASK	BIT(1) /* Enable bit 1 */
 
 /* Control/Status Register 0/1 bits  */
-#define XWT_CSRX_EWDT2_MASK 0x00000001 /* Enable bit 2 */
+#define XWT_CSRX_EWDT2_MASK	BIT(0) /* Enable bit 2 */
 
 /* SelfTest constants */
 #define XWT_MAX_SELFTEST_LOOP_COUNT 0x00010000
-- 
2.7.4


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

end of thread, other threads:[~2021-03-24  6:07 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-16 13:26 [PATCH 0/9] watchdog: of_xilinx_wdt: Update on watchdog driver Srinivas Neeli
2020-01-16 13:26 ` [PATCH 1/9] watchdog: of_xilinx_wdt: Add comment to spinlock Srinivas Neeli
2020-01-16 15:48   ` Guenter Roeck
2020-01-16 13:26 ` [PATCH 2/9] watchdog: of_xilinx_wdt: Used BIT macro Srinivas Neeli
2020-01-16 15:04   ` Guenter Roeck
2020-01-16 13:26 ` [PATCH 3/9] watchdog: of_xilinx_wdt: Used dev_dbg() Srinivas Neeli
2020-01-16 13:26 ` [PATCH 4/9] watchdog: of_xilinx_wdt: Initialize watchdog via data structure Srinivas Neeli
2020-01-16 13:26 ` [PATCH 5/9] watchdog: of_xilinx_wdt: Introduce wdttype enum for identification Srinivas Neeli
2020-01-16 13:26 ` [PATCH 6/9] dt-bindings: watchdog: xilinx: Add binding for Versal watchdog Srinivas Neeli
2020-01-16 13:26 ` [PATCH 7/9] watchdog: of_xilinx_wdt: Add Versal support Srinivas Neeli
2020-01-16 15:07   ` Guenter Roeck
2020-01-16 13:26 ` [PATCH 8/9] watchdog: of_xilinx_wdt: Wire setting up timeout via module parameter/DT Srinivas Neeli
2020-01-16 13:26 ` [PATCH 9/9] watchdog: of_xilinx_wdt: Skip printing pointer value Srinivas Neeli
2021-03-15 10:46 [PATCH 0/9] watchdog: of_xilinx_wdt: Update on xilinx watchdog driver Srinivas Neeli
2021-03-15 10:46 ` [PATCH 2/9] watchdog: of_xilinx_wdt: Used BIT macro Srinivas Neeli
2021-03-16  2:23   ` Guenter Roeck
2021-03-24  6:06     ` Srinivas Neeli

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).