[PATCH V3 0/4] watchdog: xilinx_wwdt: Add Versal watchdog support

[PATCH V3 0/4] watchdog: xilinx_wwdt: Add Versal watchdog support

[Srinivas Neeli]

This patch series does
-Adds dt-bindings for versal watchdog driver.
-Adds support for versal watchdog driver.
-Adds fragment page for xilinx watchdog drivers.

There was a series[1] sent earlier to add versal watchdog support using
pretimeout. In review it was discouraged to use pretimeout for open and
close window . This series is a new implementation of versal watchdog.

Srinivas Neeli (4):
  MAINTAINERS: Add fragment for Xilinx watchdog driver
  dt-bindings: watchdog: xlnx,versal-wwdt: Add versal watchdog bindings
  watchdog: xilinx_wwdt: Add Versal window watchdog support
  MAINTAINERS: Add support for Xilinx versal watchdog

 .../bindings/watchdog/xlnx,versal-wwdt.yaml   |  50 ++++
 MAINTAINERS                                   |  10 +
 drivers/watchdog/Kconfig                      |  18 ++
 drivers/watchdog/Makefile                     |   1 +
 drivers/watchdog/xilinx_wwdt.c                | 215 ++++++++++++++++++
 5 files changed, 294 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/watchdog/xlnx,versal-wwdt.yaml
 create mode 100644 drivers/watchdog/xilinx_wwdt.c

-- 
2.25.1

[PATCH V3 1/4] MAINTAINERS: Add fragment for Xilinx watchdog driver

[Srinivas Neeli]

Added entry for Xilinx xps-timebase watchdog driver.

Signed-off-by: Srinivas Neeli <srinivas.neeli@amd.com>
---
Changes in V3:
-None
---
 MAINTAINERS | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index d8287eb2ab4a..4ed139d56913 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -23071,6 +23071,14 @@ F:	Documentation/devicetree/bindings/media/xilinx/
 F:	drivers/media/platform/xilinx/
 F:	include/uapi/linux/xilinx-v4l2-controls.h
 
+XILINX WATCHDOG DRIVER
+M:	Srinivas Neeli <srinivas.neeli@amd.com>
+R:	Shubhrajyoti Datta <shubhrajyoti.datta@amd.com>
+R:	Michal Simek <michal.simek@amd.com>
+S:	Maintained
+F:	Documentation/devicetree/bindings/watchdog/xlnx,xps-timebase-wdt.yaml
+F:	drivers/watchdog/of_xilinx_wdt.c
+
 XILINX XDMA DRIVER
 M:	Lizhi Hou <lizhi.hou@amd.com>
 M:	Brian Xu <brian.xu@amd.com>
-- 
2.25.1

Re: [PATCH V3 1/4] MAINTAINERS: Add fragment for Xilinx watchdog driver

[Krzysztof Kozlowski]

On 31/03/2023 09:41, Srinivas Neeli wrote:
> Added entry for Xilinx xps-timebase watchdog driver.
> 
> Signed-off-by: Srinivas Neeli <srinivas.neeli@amd.com>
> ---
> Changes in V3:
> -None
> ---
>  MAINTAINERS | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index d8287eb2ab4a..4ed139d56913 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -23071,6 +23071,14 @@ F:	Documentation/devicetree/bindings/media/xilinx/
>  F:	drivers/media/platform/xilinx/
>  F:	include/uapi/linux/xilinx-v4l2-controls.h
>  
> +XILINX WATCHDOG DRIVER
> +M:	Srinivas Neeli <srinivas.neeli@amd.com>
> +R:	Shubhrajyoti Datta <shubhrajyoti.datta@amd.com>
> +R:	Michal Simek <michal.simek@amd.com>
> +S:	Maintained
> +F:	Documentation/devicetree/bindings/watchdog/xlnx,xps-timebase-wdt.yaml
> +F:	drivers/watchdog/of_xilinx_wdt.c

Your patchset is not bisectable. These files do not exist.

Best regards,
Krzysztof

Re: [PATCH V3 1/4] MAINTAINERS: Add fragment for Xilinx watchdog driver

[Krzysztof Kozlowski]

On 03/04/2023 20:30, Krzysztof Kozlowski wrote:
> On 31/03/2023 09:41, Srinivas Neeli wrote:
>> Added entry for Xilinx xps-timebase watchdog driver.
>>
>> Signed-off-by: Srinivas Neeli <srinivas.neeli@amd.com>
>> ---
>> Changes in V3:
>> -None
>> ---
>>  MAINTAINERS | 8 ++++++++
>>  1 file changed, 8 insertions(+)
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index d8287eb2ab4a..4ed139d56913 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -23071,6 +23071,14 @@ F:	Documentation/devicetree/bindings/media/xilinx/
>>  F:	drivers/media/platform/xilinx/
>>  F:	include/uapi/linux/xilinx-v4l2-controls.h
>>  
>> +XILINX WATCHDOG DRIVER
>> +M:	Srinivas Neeli <srinivas.neeli@amd.com>
>> +R:	Shubhrajyoti Datta <shubhrajyoti.datta@amd.com>
>> +R:	Michal Simek <michal.simek@amd.com>
>> +S:	Maintained
>> +F:	Documentation/devicetree/bindings/watchdog/xlnx,xps-timebase-wdt.yaml
>> +F:	drivers/watchdog/of_xilinx_wdt.c
> 
> Your patchset is not bisectable. These files do not exist.

Ah, I was too fast in judging this. It seems you document existing
driver, although nothing in commit msg suggests it.

Best regards,
Krzysztof

Re: [PATCH V3 1/4] MAINTAINERS: Add fragment for Xilinx watchdog driver

[Guenter Roeck]

On Fri, Mar 31, 2023 at 01:11:14PM +0530, Srinivas Neeli wrote:
> Added entry for Xilinx xps-timebase watchdog driver.
> 
> Signed-off-by: Srinivas Neeli <srinivas.neeli@amd.com>

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

> ---
> Changes in V3:
> -None
> ---
>  MAINTAINERS | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index d8287eb2ab4a..4ed139d56913 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -23071,6 +23071,14 @@ F:	Documentation/devicetree/bindings/media/xilinx/
>  F:	drivers/media/platform/xilinx/
>  F:	include/uapi/linux/xilinx-v4l2-controls.h
>  
> +XILINX WATCHDOG DRIVER
> +M:	Srinivas Neeli <srinivas.neeli@amd.com>
> +R:	Shubhrajyoti Datta <shubhrajyoti.datta@amd.com>
> +R:	Michal Simek <michal.simek@amd.com>
> +S:	Maintained
> +F:	Documentation/devicetree/bindings/watchdog/xlnx,xps-timebase-wdt.yaml
> +F:	drivers/watchdog/of_xilinx_wdt.c
> +
>  XILINX XDMA DRIVER
>  M:	Lizhi Hou <lizhi.hou@amd.com>
>  M:	Brian Xu <brian.xu@amd.com>

[PATCH V3 2/4] dt-bindings: watchdog: xlnx,versal-wwdt: Add versal watchdog bindings

[Srinivas Neeli]

Versal watchdog driver uses window watchdog mode. Window watchdog
timer(WWDT) contains closed(first) and open(second) window with
32 bit width. Write to the watchdog timer within predefined window
periods of time. This means a period that is not too soon and
a period that is not too late.

This patch adds devicetree bindings for versal window watchdog device.

Signed-off-by: Srinivas Neeli <srinivas.neeli@amd.com>
---
Changes in V3:
- Removed  xlnx,close_percent property.
Changes in V2:
- Added watchdog ref
- Removed timeout-sec property
- Used 4 spaces for example indentation.
---
 .../bindings/watchdog/xlnx,versal-wwdt.yaml   | 50 +++++++++++++++++++
 1 file changed, 50 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/watchdog/xlnx,versal-wwdt.yaml

diff --git a/Documentation/devicetree/bindings/watchdog/xlnx,versal-wwdt.yaml b/Documentation/devicetree/bindings/watchdog/xlnx,versal-wwdt.yaml
new file mode 100644
index 000000000000..dea1e7c487d3
--- /dev/null
+++ b/Documentation/devicetree/bindings/watchdog/xlnx,versal-wwdt.yaml
@@ -0,0 +1,50 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/watchdog/xlnx,versal-wwdt.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Xilinx Versal window watchdog timer controller
+
+maintainers:
+  - Neeli Srinivas <srinivas.neeli@amd.com>
+
+description:
+  Versal watchdog driver uses window watchdog mode. Window watchdog
+  timer(WWDT) contains closed(first) and open(second) window with
+  32 bit width. Write to the watchdog timer within predefined window
+  periods of time. This means a period that is not too soon and a
+  period that is not too late. The WWDT has to be restarted within
+  the open window time. If software tries to restart WWDT outside of
+  the open window time period, it generates a reset.
+
+allOf:
+  - $ref: /schemas/watchdog/watchdog.yaml#
+
+properties:
+  compatible:
+    enum:
+      - xlnx,versal-wwdt
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - clocks
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    watchdog@fd4d0000 {
+        compatible = "xlnx,versal-wwdt";
+        reg = <0xfd4d0000 0x10000>;
+        clocks = <&clock25>;
+        timeout-sec = <30>;
+    };
+...
-- 
2.25.1

Re: [PATCH V3 2/4] dt-bindings: watchdog: xlnx,versal-wwdt: Add versal watchdog bindings

[Krzysztof Kozlowski]

On 31/03/2023 09:41, Srinivas Neeli wrote:
> Versal watchdog driver uses window watchdog mode. Window watchdog

Subject: drop second/last, redundant "bindings". The "dt-bindings"
prefix is already stating that these are bindings.

> timer(WWDT) contains closed(first) and open(second) window with
> 32 bit width. Write to the watchdog timer within predefined window
> periods of time. This means a period that is not too soon and
> a period that is not too late.
> 
> This patch adds devicetree bindings for versal window watchdog device.

Do not use "This commit/patch", but imperative mood. See:
https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95

> 
> Signed-off-by: Srinivas Neeli <srinivas.neeli@amd.com>
> ---
> Changes in V3:
> - Removed  xlnx,close_percent property.
> Changes in V2:
> - Added watchdog ref
> - Removed timeout-sec property
> - Used 4 spaces for example indentation.
> ---
>  .../bindings/watchdog/xlnx,versal-wwdt.yaml   | 50 +++++++++++++++++++
>  1 file changed, 50 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/watchdog/xlnx,versal-wwdt.yaml
> 
> diff --git a/Documentation/devicetree/bindings/watchdog/xlnx,versal-wwdt.yaml b/Documentation/devicetree/bindings/watchdog/xlnx,versal-wwdt.yaml
> new file mode 100644
> index 000000000000..dea1e7c487d3
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/watchdog/xlnx,versal-wwdt.yaml
> @@ -0,0 +1,50 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/watchdog/xlnx,versal-wwdt.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Xilinx Versal window watchdog timer controller
> +
> +maintainers:
> +  - Neeli Srinivas <srinivas.neeli@amd.com>
> +
> +description:
> +  Versal watchdog driver uses window watchdog mode. Window watchdog

What is driver? If you mean Linux driver, then drop all references to it
and rephrase to describe hardware.

> +  timer(WWDT) contains closed(first) and open(second) window with
> +  32 bit width. Write to the watchdog timer within predefined window
> +  periods of time. This means a period that is not too soon and a
> +  period that is not too late. The WWDT has to be restarted within
> +  the open window time. If software tries to restart WWDT outside of
> +  the open window time period, it generates a reset.
> +
> +allOf:
> +  - $ref: /schemas/watchdog/watchdog.yaml#

All other examples reference local watchdog.yaml. Don't do it
differently than others (git grep watchdog.yaml)

> +

Best regards,
Krzysztof

RE: [PATCH V3 2/4] dt-bindings: watchdog: xlnx,versal-wwdt: Add versal watchdog bindings

[Neeli, Srinivas]

Hi Krzysztof,

> -----Original Message-----
> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> Sent: Tuesday, April 4, 2023 12:03 AM
> To: Neeli, Srinivas <srinivas.neeli@amd.com>; Datta, Shubhrajyoti
> <shubhrajyoti.datta@amd.com>; Simek, Michal <michal.simek@amd.com>;
> Goud, Srinivas <srinivas.goud@amd.com>; wim@linux-watchdog.org;
> linux@roeck-us.net; robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org
> Cc: linux-watchdog@vger.kernel.org; devicetree@vger.kernel.org; linux-
> kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org; git (AMD-
> Xilinx) <git@amd.com>; git@xilinx.com; neelisrinivas18@gmail.com
> Subject: Re: [PATCH V3 2/4] dt-bindings: watchdog: xlnx,versal-wwdt: Add
> versal watchdog bindings
> 
> On 31/03/2023 09:41, Srinivas Neeli wrote:
> > Versal watchdog driver uses window watchdog mode. Window watchdog
> 
> Subject: drop second/last, redundant "bindings". The "dt-bindings"
> prefix is already stating that these are bindings.
> 
> > timer(WWDT) contains closed(first) and open(second) window with
> > 32 bit width. Write to the watchdog timer within predefined window
> > periods of time. This means a period that is not too soon and a period
> > that is not too late.
> >
> > This patch adds devicetree bindings for versal window watchdog device.
> 
> Do not use "This commit/patch", but imperative mood. See:
> https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/sub
> mitting-patches.rst#L95
> 

Thanks for review,
I will address all changes in next version.

> >
> > Signed-off-by: Srinivas Neeli <srinivas.neeli@amd.com>
> > ---
> > Changes in V3:
> > - Removed  xlnx,close_percent property.
> > Changes in V2:
> > - Added watchdog ref
> > - Removed timeout-sec property
> > - Used 4 spaces for example indentation.
> > ---
> >  .../bindings/watchdog/xlnx,versal-wwdt.yaml   | 50 +++++++++++++++++++
> >  1 file changed, 50 insertions(+)
> >  create mode 100644
> > Documentation/devicetree/bindings/watchdog/xlnx,versal-wwdt.yaml
> >
> > diff --git
> > a/Documentation/devicetree/bindings/watchdog/xlnx,versal-wwdt.yaml
> > b/Documentation/devicetree/bindings/watchdog/xlnx,versal-wwdt.yaml
> > new file mode 100644
> > index 000000000000..dea1e7c487d3
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/watchdog/xlnx,versal-
> wwdt.yaml
> > @@ -0,0 +1,50 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) %YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/watchdog/xlnx,versal-wwdt.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Xilinx Versal window watchdog timer controller
> > +
> > +maintainers:
> > +  - Neeli Srinivas <srinivas.neeli@amd.com>
> > +
> > +description:
> > +  Versal watchdog driver uses window watchdog mode. Window watchdog
> 
> What is driver? If you mean Linux driver, then drop all references to it and
> rephrase to describe hardware.
> 
> > +  timer(WWDT) contains closed(first) and open(second) window with
> > +  32 bit width. Write to the watchdog timer within predefined window
> > + periods of time. This means a period that is not too soon and a
> > + period that is not too late. The WWDT has to be restarted within
> > + the open window time. If software tries to restart WWDT outside of
> > + the open window time period, it generates a reset.
> > +
> > +allOf:
> > +  - $ref: /schemas/watchdog/watchdog.yaml#
> 
> All other examples reference local watchdog.yaml. Don't do it differently
> than others (git grep watchdog.yaml)
> 
> > +
> 
> Best regards,
> Krzysztof

[PATCH V3 3/4] watchdog: xilinx_wwdt: Add Versal window watchdog support

[Srinivas Neeli]

Versal watchdog driver uses window watchdog mode. Window watchdog
timer(WWDT) contains closed(first) and open(second) window with
32 bit width. Write to the watchdog timer within predefined window
periods of time. This means a period that is not too soon and a
period that is not too late. The WWDT has to be restarted within
the open window time. If software tries to restart WWDT outside of
the open window time period, it generates a reset.

Signed-off-by: Srinivas Neeli <srinivas.neeli@amd.com>
---
Changes in V3:
-Removed "xlnx,close_percent" support from dtb.
-Added "xlnx,close_percent" property as module paratmeter.
-Updated code with devm_clk_get_enabled().
Changes in V2:
- Takes "xlnx,close-percent" property from device tree parameter.
- Removed clk_disable() function.
- Removed module parameter permisions and using readomly.
- Added check for close_percent( 0 < close_perecent < 100).
- Updated other minor comments.
---
 drivers/watchdog/Kconfig       |  18 +++
 drivers/watchdog/Makefile      |   1 +
 drivers/watchdog/xilinx_wwdt.c | 215 +++++++++++++++++++++++++++++++++
 3 files changed, 234 insertions(+)
 create mode 100644 drivers/watchdog/xilinx_wwdt.c

diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index f0872970daf9..ec4b522ae29e 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -304,6 +304,24 @@ config XILINX_WATCHDOG
 	  To compile this driver as a module, choose M here: the
 	  module will be called of_xilinx_wdt.
 
+config XILINX_WINDOW_WATCHDOG
+	tristate "Xilinx window watchdog timer"
+	depends on HAS_IOMEM
+	depends on ARM64
+	select WATCHDOG_CORE
+	help
+	  Window watchdog driver for the versal_wwdt IP core.
+	  Window watchdog timer(WWDT) contains closed(first) and
+	  open(second) window with 32 bit width. Write to the watchdog
+	  timer within predefined window periods of time. This means
+	  a period that is not too soon and a period that is not too
+	  late. The WWDT has to be restarted within the open window time.
+	  If software tries to restart WWDT outside of the open window
+	  time period, it generates a reset.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called xilinx_wwdt.
+
 config ZIIRAVE_WATCHDOG
 	tristate "Zodiac RAVE Watchdog Timer"
 	depends on I2C
diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
index 9cbf6580f16c..6cb5f1dfb492 100644
--- a/drivers/watchdog/Makefile
+++ b/drivers/watchdog/Makefile
@@ -157,6 +157,7 @@ obj-$(CONFIG_M54xx_WATCHDOG) += m54xx_wdt.o
 
 # MicroBlaze Architecture
 obj-$(CONFIG_XILINX_WATCHDOG) += of_xilinx_wdt.o
+obj-$(CONFIG_XILINX_WINDOW_WATCHDOG) += xilinx_wwdt.o
 
 # MIPS Architecture
 obj-$(CONFIG_ATH79_WDT) += ath79_wdt.o
diff --git a/drivers/watchdog/xilinx_wwdt.c b/drivers/watchdog/xilinx_wwdt.c
new file mode 100644
index 000000000000..7e9205bbf160
--- /dev/null
+++ b/drivers/watchdog/xilinx_wwdt.c
@@ -0,0 +1,215 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Window watchdog device driver for Xilinx Versal WWDT
+ *
+ * Copyright (C) 2022 - 2023, Advanced Micro Devices, Inc.
+ */
+
+#include <linux/clk.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/ioport.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/of_address.h>
+#include <linux/watchdog.h>
+
+#define XWWDT_DEFAULT_TIMEOUT	40
+#define XWWDT_MIN_TIMEOUT	1
+
+/* Register offsets for the WWDT device */
+#define XWWDT_MWR_OFFSET	0x00
+#define XWWDT_ESR_OFFSET	0x04
+#define XWWDT_FCR_OFFSET	0x08
+#define XWWDT_FWR_OFFSET	0x0c
+#define XWWDT_SWR_OFFSET	0x10
+
+/* Master Write Control Register Masks */
+#define XWWDT_MWR_MASK		BIT(0)
+
+/* Enable and Status Register Masks */
+#define XWWDT_ESR_WINT_MASK	BIT(16)
+#define XWWDT_ESR_WSW_MASK	BIT(8)
+#define XWWDT_ESR_WEN_MASK	BIT(0)
+
+#define XWWDT_CLOSE_WINDOW_PERCENT	50
+
+static int xwwdt_timeout;
+static int xclosed_window_percent;
+
+module_param(xwwdt_timeout, int, 0);
+MODULE_PARM_DESC(xwwdt_timeout,
+		 "Watchdog time in seconds. (default="
+		 __MODULE_STRING(XWWDT_DEFAULT_TIMEOUT) ")");
+module_param(xclosed_window_percent, int, 0);
+MODULE_PARM_DESC(xclosed_window_percent,
+		 "Watchdog closed window percentage. (default="
+		 __MODULE_STRING(XWWDT_CLOSE_WINDOW_PERCENT) ")");
+/**
+ * struct xwwdt_device - Watchdog device structure
+ * @base: base io address of WDT device
+ * @spinlock: spinlock for IO register access
+ * @xilinx_wwdt_wdd: watchdog device structure
+ * @freq: source clock frequency of WWDT
+ * @close_percent: Closed window percent
+ */
+struct xwwdt_device {
+	void __iomem *base;
+	spinlock_t spinlock; /* spinlock for register handling */
+	struct watchdog_device xilinx_wwdt_wdd;
+	unsigned long freq;
+	u32 close_percent;
+};
+
+static int xilinx_wwdt_start(struct watchdog_device *wdd)
+{
+	struct xwwdt_device *xdev = watchdog_get_drvdata(wdd);
+	struct watchdog_device *xilinx_wwdt_wdd = &xdev->xilinx_wwdt_wdd;
+	u64 time_out, closed_timeout, open_timeout;
+	u32 control_status_reg;
+
+	/* Calculate timeout count */
+	time_out = xdev->freq * wdd->timeout;
+
+	if (xdev->close_percent && xdev->close_percent < 100) {
+		closed_timeout = (time_out * xdev->close_percent) / 100;
+		open_timeout = time_out - closed_timeout;
+		wdd->min_hw_heartbeat_ms = xdev->close_percent * 10 * wdd->timeout;
+	} else {
+		/* Calculate XWWDT_CLOSE_WINDOW_PERCENT of timeout */
+		time_out *= XWWDT_CLOSE_WINDOW_PERCENT;
+		time_out /= 100;
+		wdd->min_hw_heartbeat_ms = XWWDT_CLOSE_WINDOW_PERCENT * 10 * wdd->timeout;
+	}
+
+	spin_lock(&xdev->spinlock);
+
+	iowrite32(XWWDT_MWR_MASK, xdev->base + XWWDT_MWR_OFFSET);
+	iowrite32(~(u32)XWWDT_ESR_WEN_MASK, xdev->base + XWWDT_ESR_OFFSET);
+
+	if (xdev->close_percent && xdev->close_percent < 100) {
+		iowrite32((u32)closed_timeout, xdev->base + XWWDT_FWR_OFFSET);
+		iowrite32((u32)open_timeout, xdev->base + XWWDT_SWR_OFFSET);
+	} else {
+		/* Configure closed and open windows with XWWDT_CLOSE_WINDOW_PERCENT of timeout */
+		iowrite32((u32)time_out, xdev->base + XWWDT_FWR_OFFSET);
+		iowrite32((u32)time_out, xdev->base + XWWDT_SWR_OFFSET);
+	}
+
+	/* Enable the window watchdog timer */
+	control_status_reg = ioread32(xdev->base + XWWDT_ESR_OFFSET);
+	control_status_reg |= XWWDT_ESR_WEN_MASK;
+	iowrite32(control_status_reg, xdev->base + XWWDT_ESR_OFFSET);
+
+	spin_unlock(&xdev->spinlock);
+
+	dev_dbg(xilinx_wwdt_wdd->parent, "Watchdog Started!\n");
+
+	return 0;
+}
+
+static int xilinx_wwdt_keepalive(struct watchdog_device *wdd)
+{
+	struct xwwdt_device *xdev = watchdog_get_drvdata(wdd);
+	u32 control_status_reg;
+
+	spin_lock(&xdev->spinlock);
+
+	/* Enable write access control bit for the window watchdog */
+	iowrite32(XWWDT_MWR_MASK, xdev->base + XWWDT_MWR_OFFSET);
+
+	/* Trigger restart kick to watchdog */
+	control_status_reg = ioread32(xdev->base + XWWDT_ESR_OFFSET);
+	control_status_reg |= XWWDT_ESR_WSW_MASK;
+	iowrite32(control_status_reg, xdev->base + XWWDT_ESR_OFFSET);
+
+	spin_unlock(&xdev->spinlock);
+
+	return 0;
+}
+
+static const struct watchdog_info xilinx_wwdt_ident = {
+	.options = WDIOF_KEEPALIVEPING |
+		WDIOF_SETTIMEOUT,
+	.firmware_version = 1,
+	.identity = "xlnx_window watchdog",
+};
+
+static const struct watchdog_ops xilinx_wwdt_ops = {
+	.owner = THIS_MODULE,
+	.start = xilinx_wwdt_start,
+	.ping = xilinx_wwdt_keepalive,
+};
+
+static int xwwdt_probe(struct platform_device *pdev)
+{
+	struct watchdog_device *xilinx_wwdt_wdd;
+	struct device *dev = &pdev->dev;
+	struct xwwdt_device *xdev;
+	struct clk *clk;
+	int ret;
+
+	xdev = devm_kzalloc(dev, sizeof(*xdev), GFP_KERNEL);
+	if (!xdev)
+		return -ENOMEM;
+
+	xilinx_wwdt_wdd = &xdev->xilinx_wwdt_wdd;
+	xilinx_wwdt_wdd->info = &xilinx_wwdt_ident;
+	xilinx_wwdt_wdd->ops = &xilinx_wwdt_ops;
+	xilinx_wwdt_wdd->parent = dev;
+
+	xdev->base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(xdev->base))
+		return PTR_ERR(xdev->base);
+
+	clk = devm_clk_get_enabled(dev, NULL);
+	if (IS_ERR(clk))
+		return PTR_ERR(clk);
+
+	xdev->freq = clk_get_rate(clk);
+	if (!xdev->freq)
+		return -EINVAL;
+
+	xilinx_wwdt_wdd->timeout = XWWDT_DEFAULT_TIMEOUT;
+	xilinx_wwdt_wdd->min_timeout = XWWDT_MIN_TIMEOUT;
+	xilinx_wwdt_wdd->max_hw_heartbeat_ms = 1000 * xilinx_wwdt_wdd->timeout;
+	xdev->close_percent = xclosed_window_percent;
+
+	ret = watchdog_init_timeout(xilinx_wwdt_wdd,
+				    xwwdt_timeout, &pdev->dev);
+	if (ret)
+		dev_info(&pdev->dev, "Configured default timeout value\n");
+
+	spin_lock_init(&xdev->spinlock);
+	watchdog_set_drvdata(xilinx_wwdt_wdd, xdev);
+	watchdog_set_nowayout(xilinx_wwdt_wdd, WATCHDOG_NOWAYOUT);
+
+	ret = devm_watchdog_register_device(dev, xilinx_wwdt_wdd);
+	if (ret)
+		return ret;
+
+	dev_info(dev, "Xilinx window watchdog Timer with timeout %ds\n",
+		 xilinx_wwdt_wdd->timeout);
+
+	return 0;
+}
+
+static const struct of_device_id xwwdt_of_match[] = {
+	{ .compatible = "xlnx,versal-wwdt", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, xwwdt_of_match);
+
+static struct platform_driver xwwdt_driver = {
+	.probe = xwwdt_probe,
+	.driver = {
+		.name = "Xilinx window watchdog",
+		.of_match_table = xwwdt_of_match,
+	},
+};
+
+module_platform_driver(xwwdt_driver);
+
+MODULE_AUTHOR("Neeli Srinivas <srinivas.neeli@amd.com>");
+MODULE_DESCRIPTION("Xilinx window watchdog driver");
+MODULE_LICENSE("GPL");
-- 
2.25.1

Re: [PATCH V3 3/4] watchdog: xilinx_wwdt: Add Versal window watchdog support

[Guenter Roeck]

On Fri, Mar 31, 2023 at 01:11:16PM +0530, Srinivas Neeli wrote:
> Versal watchdog driver uses window watchdog mode. Window watchdog
> timer(WWDT) contains closed(first) and open(second) window with
> 32 bit width. Write to the watchdog timer within predefined window
> periods of time. This means a period that is not too soon and a
> period that is not too late. The WWDT has to be restarted within
> the open window time. If software tries to restart WWDT outside of
> the open window time period, it generates a reset.
> 
> Signed-off-by: Srinivas Neeli <srinivas.neeli@amd.com>
> ---
> Changes in V3:
> -Removed "xlnx,close_percent" support from dtb.
> -Added "xlnx,close_percent" property as module paratmeter.
> -Updated code with devm_clk_get_enabled().
> Changes in V2:
> - Takes "xlnx,close-percent" property from device tree parameter.
> - Removed clk_disable() function.
> - Removed module parameter permisions and using readomly.
> - Added check for close_percent( 0 < close_perecent < 100).
> - Updated other minor comments.
> ---
>  drivers/watchdog/Kconfig       |  18 +++
>  drivers/watchdog/Makefile      |   1 +
>  drivers/watchdog/xilinx_wwdt.c | 215 +++++++++++++++++++++++++++++++++
>  3 files changed, 234 insertions(+)
>  create mode 100644 drivers/watchdog/xilinx_wwdt.c
> 
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index f0872970daf9..ec4b522ae29e 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -304,6 +304,24 @@ config XILINX_WATCHDOG
>  	  To compile this driver as a module, choose M here: the
>  	  module will be called of_xilinx_wdt.
>  
> +config XILINX_WINDOW_WATCHDOG
> +	tristate "Xilinx window watchdog timer"
> +	depends on HAS_IOMEM
> +	depends on ARM64
> +	select WATCHDOG_CORE
> +	help
> +	  Window watchdog driver for the versal_wwdt IP core.
> +	  Window watchdog timer(WWDT) contains closed(first) and
> +	  open(second) window with 32 bit width. Write to the watchdog
> +	  timer within predefined window periods of time. This means
> +	  a period that is not too soon and a period that is not too
> +	  late. The WWDT has to be restarted within the open window time.
> +	  If software tries to restart WWDT outside of the open window
> +	  time period, it generates a reset.
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called xilinx_wwdt.
> +
>  config ZIIRAVE_WATCHDOG
>  	tristate "Zodiac RAVE Watchdog Timer"
>  	depends on I2C
> diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
> index 9cbf6580f16c..6cb5f1dfb492 100644
> --- a/drivers/watchdog/Makefile
> +++ b/drivers/watchdog/Makefile
> @@ -157,6 +157,7 @@ obj-$(CONFIG_M54xx_WATCHDOG) += m54xx_wdt.o
>  
>  # MicroBlaze Architecture
>  obj-$(CONFIG_XILINX_WATCHDOG) += of_xilinx_wdt.o
> +obj-$(CONFIG_XILINX_WINDOW_WATCHDOG) += xilinx_wwdt.o
>  
>  # MIPS Architecture
>  obj-$(CONFIG_ATH79_WDT) += ath79_wdt.o
> diff --git a/drivers/watchdog/xilinx_wwdt.c b/drivers/watchdog/xilinx_wwdt.c
> new file mode 100644
> index 000000000000..7e9205bbf160
> --- /dev/null
> +++ b/drivers/watchdog/xilinx_wwdt.c
> @@ -0,0 +1,215 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Window watchdog device driver for Xilinx Versal WWDT
> + *
> + * Copyright (C) 2022 - 2023, Advanced Micro Devices, Inc.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/ioport.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>
> +#include <linux/of_address.h>
> +#include <linux/watchdog.h>
> +
> +#define XWWDT_DEFAULT_TIMEOUT	40
> +#define XWWDT_MIN_TIMEOUT	1
> +
> +/* Register offsets for the WWDT device */
> +#define XWWDT_MWR_OFFSET	0x00
> +#define XWWDT_ESR_OFFSET	0x04
> +#define XWWDT_FCR_OFFSET	0x08
> +#define XWWDT_FWR_OFFSET	0x0c
> +#define XWWDT_SWR_OFFSET	0x10
> +
> +/* Master Write Control Register Masks */
> +#define XWWDT_MWR_MASK		BIT(0)
> +
> +/* Enable and Status Register Masks */
> +#define XWWDT_ESR_WINT_MASK	BIT(16)
> +#define XWWDT_ESR_WSW_MASK	BIT(8)
> +#define XWWDT_ESR_WEN_MASK	BIT(0)
> +
> +#define XWWDT_CLOSE_WINDOW_PERCENT	50
> +
> +static int xwwdt_timeout;
> +static int xclosed_window_percent;
> +
> +module_param(xwwdt_timeout, int, 0);
> +MODULE_PARM_DESC(xwwdt_timeout,
> +		 "Watchdog time in seconds. (default="
> +		 __MODULE_STRING(XWWDT_DEFAULT_TIMEOUT) ")");
> +module_param(xclosed_window_percent, int, 0);
> +MODULE_PARM_DESC(xclosed_window_percent,
> +		 "Watchdog closed window percentage. (default="
> +		 __MODULE_STRING(XWWDT_CLOSE_WINDOW_PERCENT) ")");

The prefixes for those module parameters are really unnecessary.

> +/**
> + * struct xwwdt_device - Watchdog device structure
> + * @base: base io address of WDT device
> + * @spinlock: spinlock for IO register access
> + * @xilinx_wwdt_wdd: watchdog device structure
> + * @freq: source clock frequency of WWDT
> + * @close_percent: Closed window percent
> + */
> +struct xwwdt_device {
> +	void __iomem *base;
> +	spinlock_t spinlock; /* spinlock for register handling */
> +	struct watchdog_device xilinx_wwdt_wdd;
> +	unsigned long freq;
> +	u32 close_percent;
> +};
> +
> +static int xilinx_wwdt_start(struct watchdog_device *wdd)
> +{
> +	struct xwwdt_device *xdev = watchdog_get_drvdata(wdd);
> +	struct watchdog_device *xilinx_wwdt_wdd = &xdev->xilinx_wwdt_wdd;
> +	u64 time_out, closed_timeout, open_timeout;
> +	u32 control_status_reg;
> +
> +	/* Calculate timeout count */
> +	time_out = xdev->freq * wdd->timeout;
> +
> +	if (xdev->close_percent && xdev->close_percent < 100) {
> +		closed_timeout = (time_out * xdev->close_percent) / 100;
> +		open_timeout = time_out - closed_timeout;
> +		wdd->min_hw_heartbeat_ms = xdev->close_percent * 10 * wdd->timeout;
> +	} else {
> +		/* Calculate XWWDT_CLOSE_WINDOW_PERCENT of timeout */
> +		time_out *= XWWDT_CLOSE_WINDOW_PERCENT;
> +		time_out /= 100;
> +		wdd->min_hw_heartbeat_ms = XWWDT_CLOSE_WINDOW_PERCENT * 10 * wdd->timeout;
> +	}

Why not set close_percent to the default value in the probe function
if it is not provided as module parameter ?

> +
> +	spin_lock(&xdev->spinlock);
> +
> +	iowrite32(XWWDT_MWR_MASK, xdev->base + XWWDT_MWR_OFFSET);
> +	iowrite32(~(u32)XWWDT_ESR_WEN_MASK, xdev->base + XWWDT_ESR_OFFSET);
> +
> +	if (xdev->close_percent && xdev->close_percent < 100) {

This should really be validated once in the probe function.
The provided value should always be valid here.

> +		iowrite32((u32)closed_timeout, xdev->base + XWWDT_FWR_OFFSET);
> +		iowrite32((u32)open_timeout, xdev->base + XWWDT_SWR_OFFSET);
> +	} else {
> +		/* Configure closed and open windows with XWWDT_CLOSE_WINDOW_PERCENT of timeout */
> +		iowrite32((u32)time_out, xdev->base + XWWDT_FWR_OFFSET);
> +		iowrite32((u32)time_out, xdev->base + XWWDT_SWR_OFFSET);
> +	}
> +
> +	/* Enable the window watchdog timer */
> +	control_status_reg = ioread32(xdev->base + XWWDT_ESR_OFFSET);
> +	control_status_reg |= XWWDT_ESR_WEN_MASK;
> +	iowrite32(control_status_reg, xdev->base + XWWDT_ESR_OFFSET);
> +
> +	spin_unlock(&xdev->spinlock);
> +
> +	dev_dbg(xilinx_wwdt_wdd->parent, "Watchdog Started!\n");
> +
> +	return 0;
> +}
> +
> +static int xilinx_wwdt_keepalive(struct watchdog_device *wdd)
> +{
> +	struct xwwdt_device *xdev = watchdog_get_drvdata(wdd);
> +	u32 control_status_reg;
> +
> +	spin_lock(&xdev->spinlock);
> +
> +	/* Enable write access control bit for the window watchdog */
> +	iowrite32(XWWDT_MWR_MASK, xdev->base + XWWDT_MWR_OFFSET);
> +
> +	/* Trigger restart kick to watchdog */
> +	control_status_reg = ioread32(xdev->base + XWWDT_ESR_OFFSET);
> +	control_status_reg |= XWWDT_ESR_WSW_MASK;
> +	iowrite32(control_status_reg, xdev->base + XWWDT_ESR_OFFSET);
> +
> +	spin_unlock(&xdev->spinlock);
> +
> +	return 0;
> +}
> +
> +static const struct watchdog_info xilinx_wwdt_ident = {
> +	.options = WDIOF_KEEPALIVEPING |
> +		WDIOF_SETTIMEOUT,
> +	.firmware_version = 1,
> +	.identity = "xlnx_window watchdog",
> +};
> +
> +static const struct watchdog_ops xilinx_wwdt_ops = {
> +	.owner = THIS_MODULE,
> +	.start = xilinx_wwdt_start,
> +	.ping = xilinx_wwdt_keepalive,
> +};
> +
> +static int xwwdt_probe(struct platform_device *pdev)
> +{
> +	struct watchdog_device *xilinx_wwdt_wdd;
> +	struct device *dev = &pdev->dev;
> +	struct xwwdt_device *xdev;
> +	struct clk *clk;
> +	int ret;
> +
> +	xdev = devm_kzalloc(dev, sizeof(*xdev), GFP_KERNEL);
> +	if (!xdev)
> +		return -ENOMEM;
> +
> +	xilinx_wwdt_wdd = &xdev->xilinx_wwdt_wdd;
> +	xilinx_wwdt_wdd->info = &xilinx_wwdt_ident;
> +	xilinx_wwdt_wdd->ops = &xilinx_wwdt_ops;
> +	xilinx_wwdt_wdd->parent = dev;
> +
> +	xdev->base = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(xdev->base))
> +		return PTR_ERR(xdev->base);
> +
> +	clk = devm_clk_get_enabled(dev, NULL);
> +	if (IS_ERR(clk))
> +		return PTR_ERR(clk);
> +
> +	xdev->freq = clk_get_rate(clk);
> +	if (!xdev->freq)
> +		return -EINVAL;
> +
> +	xilinx_wwdt_wdd->timeout = XWWDT_DEFAULT_TIMEOUT;
> +	xilinx_wwdt_wdd->min_timeout = XWWDT_MIN_TIMEOUT;
> +	xilinx_wwdt_wdd->max_hw_heartbeat_ms = 1000 * xilinx_wwdt_wdd->timeout;

Is it indeed correct that the maximum timeout supported
by the hardware is XWWDT_DEFAULT_TIMEOUT, ie 40 seconds ?

> +	xdev->close_percent = xclosed_window_percent;
> +
> +	ret = watchdog_init_timeout(xilinx_wwdt_wdd,
> +				    xwwdt_timeout, &pdev->dev);
> +	if (ret)
> +		dev_info(&pdev->dev, "Configured default timeout value\n");

Isn't that a bit redundant with the message below ?

> +
> +	spin_lock_init(&xdev->spinlock);
> +	watchdog_set_drvdata(xilinx_wwdt_wdd, xdev);
> +	watchdog_set_nowayout(xilinx_wwdt_wdd, WATCHDOG_NOWAYOUT);
> +
> +	ret = devm_watchdog_register_device(dev, xilinx_wwdt_wdd);
> +	if (ret)
> +		return ret;
> +
> +	dev_info(dev, "Xilinx window watchdog Timer with timeout %ds\n",
> +		 xilinx_wwdt_wdd->timeout);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id xwwdt_of_match[] = {
> +	{ .compatible = "xlnx,versal-wwdt", },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, xwwdt_of_match);
> +
> +static struct platform_driver xwwdt_driver = {
> +	.probe = xwwdt_probe,
> +	.driver = {
> +		.name = "Xilinx window watchdog",
> +		.of_match_table = xwwdt_of_match,
> +	},
> +};
> +
> +module_platform_driver(xwwdt_driver);
> +
> +MODULE_AUTHOR("Neeli Srinivas <srinivas.neeli@amd.com>");
> +MODULE_DESCRIPTION("Xilinx window watchdog driver");
> +MODULE_LICENSE("GPL");

[PATCH V3 4/4] MAINTAINERS: Add support for Xilinx versal watchdog

[Srinivas Neeli]

Added entry for Xilinx versal watchdog driver.

Signed-off-by: Srinivas Neeli <srinivas.neeli@amd.com>
---
Changes in V3:
-None
---
 MAINTAINERS | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 4ed139d56913..7b862ef7c876 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -23076,8 +23076,10 @@ M:	Srinivas Neeli <srinivas.neeli@amd.com>
 R:	Shubhrajyoti Datta <shubhrajyoti.datta@amd.com>
 R:	Michal Simek <michal.simek@amd.com>
 S:	Maintained
+F:	Documentation/devicetree/bindings/watchdog/xlnx,versal-wwdt.yaml
 F:	Documentation/devicetree/bindings/watchdog/xlnx,xps-timebase-wdt.yaml
 F:	drivers/watchdog/of_xilinx_wdt.c
+F:	drivers/watchdog/xilinx_wwdt.c
 
 XILINX XDMA DRIVER
 M:	Lizhi Hou <lizhi.hou@amd.com>
-- 
2.25.1