[PATCH v3 0/7] watchdog: dw_wdt: Take Baikal-T1 DW WDT peculiarities into account

[PATCH v3 0/7] watchdog: dw_wdt: Take Baikal-T1 DW WDT peculiarities into account

[Serge Semin]

Merge window is upon us. Please review/merge in/whatever the rest of the
patches.

There were a few features enabled at the time of the Baikal-T1 SoC DW WDT
IP synthesis, which weren't taken into account in the DW WDT driver available
in the kernel. First of all the SoC engineers synthesized the watchdog core
with WDT_USE_FIX_TOP set to false (don't really know why, but they did).
Due to this the timer reset values weren't fixed as the driver expected
but were initialized with a pre-defined values selected by the engineers.
Secondly the driver expected that the watchdog APB bus and the timer had
synchronous reference clocks, while Baikal-T1 SoC DW WDT was created with
asynchronous ones. So the driver should enable two clock devices: APB bus
clocks and a separate timer reference clock. Finally DW Watchdog Timer is
capable of generating a pre-timeout interrupt if corresponding config is
enabled. The problem was that the pre-timeout IRQ happens when the set
timeout elapses, while the actual WDT expiration and subsequent reboot take
place in the next timeout. This makes the pre-timeout functionality
implementation a bit tricky, since in this case we would have to find a
WDT timeout twice smaller the requested timeout. All of the changes described
above are provided by the patches in this patchset.

In addition traditionally we replaced the legacy plain text-based dt-binding
file with yaml-based one and added the controller registers dump DebugFS node
to ease the driver debug procedure.

This patchset is rebased and tested on the mainline Linux kernel 5.6-rc4:
base-commit: 0e698dfa2822 ("Linux 5.7-rc4")
tag: v5.7-rc4

Changelog v2:
- Rearrange SoBs.
- Discard BE copyright header from the binding file.
- Replace "additionalProperties: false" with "unevaluatedProperties: false"
  property in the binding.
- Move the APB3 clocks support declared in the dt binding file into a
  dedicated patch.
- Move $ref to the root level of the "snps,watchdog-tops" property
  so does the constraints.
- Make Pre-timeout IRQs support being optional.
- Add "ms" suffix to the methods returning msec and convert the methods
  with no "ms" suffix to return a timeout in sec.
- Make sure minimum timeout is at least 1 sec.
- Refactor the timeouts calculation procedure to to retain the timeouts in
  the ascending order.
- Make sure there is no integer overflow in milliseconds calculation. It
  is saved in a dedicated uint field of the timeout structure.
- Discard timeout/pretimeout/ping/enable DebugFS nodes. Registers state
  dump node is only left.

Link: https://lore.kernel.org/linux-watchdog/20200510105807.880-1-Sergey.Semin@baikalelectronics.ru/
Changelog v3:
- Add Rob's Reviewed-by tag to the DT-related patches.
- Remove items from the "snps,watchdog-tops" property and move the
  minItems and maxItems constraints to the root level of it.

Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Maxim Kaurkin <Maxim.Kaurkin@baikalelectronics.ru>
Cc: Pavel Parkhomenko <Pavel.Parkhomenko@baikalelectronics.ru>
Cc: Ramil Zaripov <Ramil.Zaripov@baikalelectronics.ru>
Cc: Ekaterina Skachko <Ekaterina.Skachko@baikalelectronics.ru>
Cc: Vadim Vlasov <V.Vlasov@baikalelectronics.ru>
Cc: Alexey Kolotnikov <Alexey.Kolotnikov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: linux-mips@vger.kernel.org
Cc: linux-watchdog@vger.kernel.org
Cc: devicetree@vger.kernel.org
Cc: linux-kernel@vger.kernel.org

Serge Semin (7):
  dt-bindings: watchdog: Convert DW WDT binding to DT schema
  dt-bindings: watchdog: dw-wdt: Support devices with asynch clocks
  dt-bindings: watchdog: dw-wdt: Add watchdog TOPs array property
  watchdog: dw_wdt: Support devices with non-fixed TOP values
  watchdog: dw_wdt: Support devices with asynch clocks
  watchdog: dw_wdt: Add pre-timeouts support
  watchdog: dw_wdt: Add DebugFS files

 .../devicetree/bindings/watchdog/dw_wdt.txt   |  24 -
 .../bindings/watchdog/snps,dw-wdt.yaml        |  90 ++++
 drivers/watchdog/dw_wdt.c                     | 437 ++++++++++++++++--
 3 files changed, 494 insertions(+), 57 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/watchdog/dw_wdt.txt
 create mode 100644 Documentation/devicetree/bindings/watchdog/snps,dw-wdt.yaml

-- 
2.26.2

[PATCH v3 1/7] dt-bindings: watchdog: Convert DW WDT binding to DT schema

[Serge Semin]

Modern device tree bindings are supposed to be created as YAML-files
in accordance with dt-schema. This commit replaces the DW Watchdog
legacy bare text bindings with YAML file. As before the binding states
that the corresponding dts node is supposed to have a registers
range, a watchdog timer references clock source, optional reset line and
pre-timeout interrupt.

Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Reviewed-by: Rob Herring <robh@kernel.org>
Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: linux-mips@vger.kernel.org

---

Changelog v2:
- Rearrange SoBs.
- Discard BE copyright header.
- Replace "additionalProperties: false" with "unevaluatedProperties: false"
  property.
- Discard interrupts property from the required properties list.
- Remove a label definition from the binding example.
- Move the asynchronous APB3 clock support into a dedicated patch.
---
 .../devicetree/bindings/watchdog/dw_wdt.txt   | 24 ---------
 .../bindings/watchdog/snps,dw-wdt.yaml        | 50 +++++++++++++++++++
 2 files changed, 50 insertions(+), 24 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/watchdog/dw_wdt.txt
 create mode 100644 Documentation/devicetree/bindings/watchdog/snps,dw-wdt.yaml

diff --git a/Documentation/devicetree/bindings/watchdog/dw_wdt.txt b/Documentation/devicetree/bindings/watchdog/dw_wdt.txt
deleted file mode 100644
index eb0914420c7c..000000000000
--- a/Documentation/devicetree/bindings/watchdog/dw_wdt.txt
+++ /dev/null
@@ -1,24 +0,0 @@
-Synopsys Designware Watchdog Timer
-
-Required Properties:
-
-- compatible	: Should contain "snps,dw-wdt"
-- reg		: Base address and size of the watchdog timer registers.
-- clocks	: phandle + clock-specifier for the clock that drives the
-		watchdog timer.
-
-Optional Properties:
-
-- interrupts	: The interrupt used for the watchdog timeout warning.
-- resets	: phandle pointing to the system reset controller with
-		line index for the watchdog.
-
-Example:
-
-	watchdog0: wd@ffd02000 {
-		compatible = "snps,dw-wdt";
-		reg = <0xffd02000 0x1000>;
-		interrupts = <0 171 4>;
-		clocks = <&per_base_clk>;
-		resets = <&rst WDT0_RESET>;
-	};
diff --git a/Documentation/devicetree/bindings/watchdog/snps,dw-wdt.yaml b/Documentation/devicetree/bindings/watchdog/snps,dw-wdt.yaml
new file mode 100644
index 000000000000..4f6944756ab4
--- /dev/null
+++ b/Documentation/devicetree/bindings/watchdog/snps,dw-wdt.yaml
@@ -0,0 +1,50 @@
+# SPDX-License-Identifier: GPL-2.0-only
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/watchdog/snps,dw-wdt.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Synopsys Designware Watchdog Timer
+
+allOf:
+  - $ref: "watchdog.yaml#"
+
+maintainers:
+  - Jamie Iles <jamie@jamieiles.com>
+
+properties:
+  compatible:
+    const: snps,dw-wdt
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    description: DW Watchdog pre-timeout interrupt
+    maxItems: 1
+
+  clocks:
+    items:
+      - description: Watchdog timer reference clock
+
+  resets:
+    description: Phandle to the DW Watchdog reset lane
+    maxItems: 1
+
+unevaluatedProperties: false
+
+required:
+  - compatible
+  - reg
+  - clocks
+
+examples:
+  - |
+    watchdog@ffd02000 {
+      compatible = "snps,dw-wdt";
+      reg = <0xffd02000 0x1000>;
+      interrupts = <0 171 4>;
+      clocks = <&per_base_clk>;
+      resets = <&wdt_rst>;
+    };
+...
-- 
2.26.2

[PATCH v3 2/7] dt-bindings: watchdog: dw-wdt: Support devices with asynch clocks

[Serge Semin]

DW Watchdog IP core can be synthesised with asynchronous timer/APB
clocks support (WDT_ASYNC_CLK_MODE_ENABLE == 1). In this case
separate clock signals are supposed to be used to feed watchdog timer
and APB interface of the device. Let's update the DW Watchdog DT node
schema so it would support the optional APB3 bus clock specified along
with the mandatory watchdog timer reference clock.

Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Reviewed-by: Rob Herring <robh@kernel.org>
Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: linux-mips@vger.kernel.org

---

Changelog v2:
- It's a new patch unpinned from the previous one.
---
 .../devicetree/bindings/watchdog/snps,dw-wdt.yaml         | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/Documentation/devicetree/bindings/watchdog/snps,dw-wdt.yaml b/Documentation/devicetree/bindings/watchdog/snps,dw-wdt.yaml
index 4f6944756ab4..5bf6dc6377f3 100644
--- a/Documentation/devicetree/bindings/watchdog/snps,dw-wdt.yaml
+++ b/Documentation/devicetree/bindings/watchdog/snps,dw-wdt.yaml
@@ -24,8 +24,16 @@ properties:
     maxItems: 1
 
   clocks:
+    minItems: 1
     items:
       - description: Watchdog timer reference clock
+      - description: APB3 interface clock
+
+  clock-names:
+    minItems: 1
+    items:
+      - const: tclk
+      - const: pclk
 
   resets:
     description: Phandle to the DW Watchdog reset lane
-- 
2.26.2

[PATCH v3 3/7] dt-bindings: watchdog: dw-wdt: Add watchdog TOPs array property

[Serge Semin]

In case if DW Watchdog IP core is built with WDT_USE_FIX_TOP == false,
a custom timeout periods are used to preset the timer counter. In
this case that periods should be specified in a new "snps,watchdog-tops"
property of the DW watchdog dts node.

Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Reviewed-by: Rob Herring <robh@kernel.org>
Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: linux-mips@vger.kernel.org

---

Changelog v2:
- Rearrange SoBs.
- Move $ref to the root level of the "snps,watchdog-tops" property
  so does the constraints.
- Add default TOP values array.
- Discard the label definition from the new bindings example.

Changelog v3:
- Remove items property and move the minItems and maxItems constraints to
  the root level of the snps,watchdog-tops property.
---
 .../bindings/watchdog/snps,dw-wdt.yaml        | 32 +++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/Documentation/devicetree/bindings/watchdog/snps,dw-wdt.yaml b/Documentation/devicetree/bindings/watchdog/snps,dw-wdt.yaml
index 5bf6dc6377f3..d9fc7bb851b1 100644
--- a/Documentation/devicetree/bindings/watchdog/snps,dw-wdt.yaml
+++ b/Documentation/devicetree/bindings/watchdog/snps,dw-wdt.yaml
@@ -39,6 +39,23 @@ properties:
     description: Phandle to the DW Watchdog reset lane
     maxItems: 1
 
+  snps,watchdog-tops:
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+    description: |
+      DW APB Watchdog custom timer intervals - Timeout Period ranges (TOPs).
+      Each TOP is a number loaded into the watchdog counter at the moment of
+      the timer restart. The counter decrementing happens each tick of the
+      reference clock. Therefore the TOPs array is equivalent to an array of
+      the timer expiration intervals supported by the DW APB Watchdog. Note
+      DW APB Watchdog IP-core might be synthesized with fixed TOP values,
+      in which case this property is unnecessary with default TOPs utilized.
+    default: [0x0001000 0x0002000 0x0004000 0x0008000
+      0x0010000 0x0020000 0x0040000 0x0080000
+      0x0100000 0x0200000 0x0400000 0x0800000
+      0x1000000 0x2000000 0x4000000 0x8000000]
+    minItems: 16
+    maxItems: 16
+
 unevaluatedProperties: false
 
 required:
@@ -55,4 +72,19 @@ examples:
       clocks = <&per_base_clk>;
       resets = <&wdt_rst>;
     };
+
+  - |
+    watchdog@ffd02000 {
+      compatible = "snps,dw-wdt";
+      reg = <0xffd02000 0x1000>;
+      interrupts = <0 171 4>;
+      clocks = <&per_base_clk>;
+      clock-names = "tclk";
+      snps,watchdog-tops = <0x000000FF 0x000001FF 0x000003FF
+                            0x000007FF 0x0000FFFF 0x0001FFFF
+                            0x0003FFFF 0x0007FFFF 0x000FFFFF
+                            0x001FFFFF 0x003FFFFF 0x007FFFFF
+                            0x00FFFFFF 0x01FFFFFF 0x03FFFFFF
+                            0x07FFFFFF>;
+    };
 ...
-- 
2.26.2

[PATCH v3 4/7] watchdog: dw_wdt: Support devices with non-fixed TOP values

[Serge Semin]

In case if the DW Watchdog IP core is synthesised with
WDT_USE_FIX_TOP == false, the TOP interval indexes make the device
to load a custom periods to the counter. These periods are hardwired
at the IP synthesis stage and can be within [2^8, 2^(WDT_CNT_WIDTH - 1)].
Alas their values can't be detected at runtime and must be somehow
supplied to the driver so one could properly determine the watchdog
timeout intervals. For this purpose we suggest to have a vendor-
specific dts property "snps,watchdog-tops" utilized, which would
provide an array of sixteen counter values. At device probe stage they
will be used to initialize the watchdog device timeouts determined
from the array values and current clocks source rate.

In order to have custom TOP values supported the driver must be
altered in the following way. First of all the fixed-top values
ready-to-use array must be determined for compatibility with currently
supported devices, which were synthesised with WDT_USE_FIX_TOP == true.
It will be used if either fixed TOP feature is detected being enabled or
no custom TOPs are fetched from the device dt node. Secondly at the probe
stage we must initialize an array of the watchdog timeouts corresponding
to the detected TOPs list and the reference clock rate. For generality the
procedure of initialization is designed in a way to support the TOPs array
with no limitations on the items order or value. Finally the watchdog
period search methods should be altered to support the new timeouts data
structure.

Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: linux-mips@vger.kernel.org
Cc: devicetree@vger.kernel.org

---

Changelog v2:
- Rearrange SoBs.
- Add "ms" suffix to the methods returning msec and convert the methods
  with no "ms" suffix to return a timeout in sec.
- Make sure minimum timeout is at least 1 sec.
- Refactor the timeouts calculation procedure to retain the timeouts in
  the ascending order.
- Make sure there is no integer overflow in milliseconds calculation. It
  is saved in a dedicated uint field of the timeout structure.
---
 drivers/watchdog/dw_wdt.c | 191 +++++++++++++++++++++++++++++++++-----
 1 file changed, 167 insertions(+), 24 deletions(-)

diff --git a/drivers/watchdog/dw_wdt.c b/drivers/watchdog/dw_wdt.c
index fba21de2bbad..693c0d1fd796 100644
--- a/drivers/watchdog/dw_wdt.c
+++ b/drivers/watchdog/dw_wdt.c
@@ -13,6 +13,8 @@
  */
 
 #include <linux/bitops.h>
+#include <linux/limits.h>
+#include <linux/kernel.h>
 #include <linux/clk.h>
 #include <linux/delay.h>
 #include <linux/err.h>
@@ -34,21 +36,40 @@
 #define WDOG_CURRENT_COUNT_REG_OFFSET	    0x08
 #define WDOG_COUNTER_RESTART_REG_OFFSET     0x0c
 #define WDOG_COUNTER_RESTART_KICK_VALUE	    0x76
+#define WDOG_COMP_PARAMS_1_REG_OFFSET       0xf4
+#define WDOG_COMP_PARAMS_1_USE_FIX_TOP      BIT(6)
 
-/* The maximum TOP (timeout period) value that can be set in the watchdog. */
-#define DW_WDT_MAX_TOP		15
+/* There are sixteen TOPs (timeout periods) that can be set in the watchdog. */
+#define DW_WDT_NUM_TOPS		16
+#define DW_WDT_FIX_TOP(_idx)	(1U << (16 + _idx))
 
 #define DW_WDT_DEFAULT_SECONDS	30
 
+static const u32 dw_wdt_fix_tops[DW_WDT_NUM_TOPS] = {
+	DW_WDT_FIX_TOP(0), DW_WDT_FIX_TOP(1), DW_WDT_FIX_TOP(2),
+	DW_WDT_FIX_TOP(3), DW_WDT_FIX_TOP(4), DW_WDT_FIX_TOP(5),
+	DW_WDT_FIX_TOP(6), DW_WDT_FIX_TOP(7), DW_WDT_FIX_TOP(8),
+	DW_WDT_FIX_TOP(9), DW_WDT_FIX_TOP(10), DW_WDT_FIX_TOP(11),
+	DW_WDT_FIX_TOP(12), DW_WDT_FIX_TOP(13), DW_WDT_FIX_TOP(14),
+	DW_WDT_FIX_TOP(15)
+};
+
 static bool nowayout = WATCHDOG_NOWAYOUT;
 module_param(nowayout, bool, 0);
 MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started "
 		 "(default=" __MODULE_STRING(WATCHDOG_NOWAYOUT) ")");
 
+struct dw_wdt_timeout {
+	u32 top_val;
+	unsigned int sec;
+	unsigned int msec;
+};
+
 struct dw_wdt {
 	void __iomem		*regs;
 	struct clk		*clk;
 	unsigned long		rate;
+	struct dw_wdt_timeout	timeouts[DW_WDT_NUM_TOPS];
 	struct watchdog_device	wdd;
 	struct reset_control	*rst;
 	/* Save/restore */
@@ -64,20 +85,66 @@ static inline int dw_wdt_is_enabled(struct dw_wdt *dw_wdt)
 		WDOG_CONTROL_REG_WDT_EN_MASK;
 }
 
-static inline int dw_wdt_top_in_seconds(struct dw_wdt *dw_wdt, unsigned top)
+static unsigned int dw_wdt_find_best_top(struct dw_wdt *dw_wdt,
+					 unsigned int timeout, u32 *top_val)
 {
+	int idx;
+
 	/*
-	 * There are 16 possible timeout values in 0..15 where the number of
-	 * cycles is 2 ^ (16 + i) and the watchdog counts down.
+	 * Find a TOP with timeout greater or equal to the requested number.
+	 * Note we'll select a TOP with maximum timeout if the requested
+	 * timeout couldn't be reached.
 	 */
-	return (1U << (16 + top)) / dw_wdt->rate;
+	for (idx = 0; idx < DW_WDT_NUM_TOPS; ++idx) {
+		if (dw_wdt->timeouts[idx].sec >= timeout)
+			break;
+	}
+
+	if (idx == DW_WDT_NUM_TOPS)
+		--idx;
+
+	*top_val = dw_wdt->timeouts[idx].top_val;
+
+	return dw_wdt->timeouts[idx].sec;
 }
 
-static int dw_wdt_get_top(struct dw_wdt *dw_wdt)
+static unsigned int dw_wdt_get_min_timeout(struct dw_wdt *dw_wdt)
 {
-	int top = readl(dw_wdt->regs + WDOG_TIMEOUT_RANGE_REG_OFFSET) & 0xF;
+	int idx;
+
+	/*
+	 * We'll find a timeout greater or equal to one second anyway because
+	 * the driver probe would have failed if there was none.
+	 */
+	for (idx = 0; idx < DW_WDT_NUM_TOPS; ++idx) {
+		if (dw_wdt->timeouts[idx].sec)
+			break;
+	}
 
-	return dw_wdt_top_in_seconds(dw_wdt, top);
+	return dw_wdt->timeouts[idx].sec;
+}
+
+static unsigned int dw_wdt_get_max_timeout_ms(struct dw_wdt *dw_wdt)
+{
+	struct dw_wdt_timeout *timeout = &dw_wdt->timeouts[DW_WDT_NUM_TOPS - 1];
+	u64 msec;
+
+	msec = (u64)timeout->sec * MSEC_PER_SEC + timeout->msec;
+
+	return msec < UINT_MAX ? msec : UINT_MAX;
+}
+
+static unsigned int dw_wdt_get_timeout(struct dw_wdt *dw_wdt)
+{
+	int top_val = readl(dw_wdt->regs + WDOG_TIMEOUT_RANGE_REG_OFFSET) & 0xF;
+	int idx;
+
+	for (idx = 0; idx < DW_WDT_NUM_TOPS; ++idx) {
+		if (dw_wdt->timeouts[idx].top_val == top_val)
+			break;
+	}
+
+	return dw_wdt->timeouts[idx].sec;
 }
 
 static int dw_wdt_ping(struct watchdog_device *wdd)
@@ -93,17 +160,10 @@ static int dw_wdt_ping(struct watchdog_device *wdd)
 static int dw_wdt_set_timeout(struct watchdog_device *wdd, unsigned int top_s)
 {
 	struct dw_wdt *dw_wdt = to_dw_wdt(wdd);
-	int i, top_val = DW_WDT_MAX_TOP;
+	unsigned int timeout;
+	u32 top_val;
 
-	/*
-	 * Iterate over the timeout values until we find the closest match. We
-	 * always look for >=.
-	 */
-	for (i = 0; i <= DW_WDT_MAX_TOP; ++i)
-		if (dw_wdt_top_in_seconds(dw_wdt, i) >= top_s) {
-			top_val = i;
-			break;
-		}
+	timeout = dw_wdt_find_best_top(dw_wdt, top_s, &top_val);
 
 	/*
 	 * Set the new value in the watchdog.  Some versions of dw_wdt
@@ -120,7 +180,7 @@ static int dw_wdt_set_timeout(struct watchdog_device *wdd, unsigned int top_s)
 	 * wdd->max_hw_heartbeat_ms
 	 */
 	if (top_s * 1000 <= wdd->max_hw_heartbeat_ms)
-		wdd->timeout = dw_wdt_top_in_seconds(dw_wdt, top_val);
+		wdd->timeout = timeout;
 	else
 		wdd->timeout = top_s;
 
@@ -238,6 +298,86 @@ static int dw_wdt_resume(struct device *dev)
 
 static SIMPLE_DEV_PM_OPS(dw_wdt_pm_ops, dw_wdt_suspend, dw_wdt_resume);
 
+/*
+ * In case if DW WDT IP core is synthesized with fixed TOP feature disabled the
+ * TOPs array can be arbitrary ordered with nearly any sixteen uint numbers
+ * depending on the system engineer imagination. The next method handles the
+ * passed TOPs array to pre-calculate the effective timeouts and to sort the
+ * TOP items out in the ascending order with respect to the timeouts.
+ */
+
+static void dw_wdt_handle_tops(struct dw_wdt *dw_wdt, const u32 *tops)
+{
+	struct dw_wdt_timeout tout, *dst;
+	int val, tidx;
+	u64 msec;
+
+	/*
+	 * We walk over the passed TOPs array and calculate corresponding
+	 * timeouts in seconds and milliseconds. The milliseconds granularity
+	 * is needed to distinguish the TOPs with very close timeouts and to
+	 * set the watchdog max heartbeat setting further.
+	 */
+	for (val = 0; val < DW_WDT_NUM_TOPS; ++val) {
+		tout.top_val = val;
+		tout.sec = tops[val] / dw_wdt->rate;
+		msec = (u64)tops[val] * MSEC_PER_SEC;
+		do_div(msec, dw_wdt->rate);
+		tout.msec = msec - ((u64)tout.sec * MSEC_PER_SEC);
+
+		/*
+		 * Find a suitable place for the current TOP in the timeouts
+		 * array so that the list is remained in the ascending order.
+		 */
+		for (tidx = 0; tidx < val; ++tidx) {
+			dst = &dw_wdt->timeouts[tidx];
+			if (tout.sec > dst->sec || (tout.sec == dst->sec &&
+			    tout.msec >= dst->msec))
+				continue;
+			else
+				swap(*dst, tout);
+		}
+
+		dw_wdt->timeouts[val] = tout;
+	}
+}
+
+static int dw_wdt_init_timeouts(struct dw_wdt *dw_wdt, struct device *dev)
+{
+	u32 data, of_tops[DW_WDT_NUM_TOPS];
+	const u32 *tops;
+	int ret;
+
+	/*
+	 * Retrieve custom or fixed counter values depending on the
+	 * WDT_USE_FIX_TOP flag found in the component specific parameters
+	 * #1 register.
+	 */
+	data = readl(dw_wdt->regs + WDOG_COMP_PARAMS_1_REG_OFFSET);
+	if (data & WDOG_COMP_PARAMS_1_USE_FIX_TOP) {
+		tops = dw_wdt_fix_tops;
+	} else {
+		ret = of_property_read_variable_u32_array(dev_of_node(dev),
+			"snps,watchdog-tops", of_tops, DW_WDT_NUM_TOPS,
+			DW_WDT_NUM_TOPS);
+		if (ret < 0) {
+			dev_warn(dev, "No valid TOPs array specified\n");
+			tops = dw_wdt_fix_tops;
+		} else {
+			tops = of_tops;
+		}
+	}
+
+	/* Convert the specified TOPs into an array of watchdog timeouts. */
+	dw_wdt_handle_tops(dw_wdt, tops);
+	if (!dw_wdt->timeouts[DW_WDT_NUM_TOPS - 1].sec) {
+		dev_err(dev, "No any valid TOP detected\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 static int dw_wdt_drv_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -275,12 +415,15 @@ static int dw_wdt_drv_probe(struct platform_device *pdev)
 
 	reset_control_deassert(dw_wdt->rst);
 
+	ret = dw_wdt_init_timeouts(dw_wdt, dev);
+	if (ret)
+		goto out_disable_clk;
+
 	wdd = &dw_wdt->wdd;
 	wdd->info = &dw_wdt_ident;
 	wdd->ops = &dw_wdt_ops;
-	wdd->min_timeout = 1;
-	wdd->max_hw_heartbeat_ms =
-		dw_wdt_top_in_seconds(dw_wdt, DW_WDT_MAX_TOP) * 1000;
+	wdd->min_timeout = dw_wdt_get_min_timeout(dw_wdt);
+	wdd->max_hw_heartbeat_ms = dw_wdt_get_max_timeout_ms(dw_wdt);
 	wdd->parent = dev;
 
 	watchdog_set_drvdata(wdd, dw_wdt);
@@ -293,7 +436,7 @@ static int dw_wdt_drv_probe(struct platform_device *pdev)
 	 * devicetree.
 	 */
 	if (dw_wdt_is_enabled(dw_wdt)) {
-		wdd->timeout = dw_wdt_get_top(dw_wdt);
+		wdd->timeout = dw_wdt_get_timeout(dw_wdt);
 		set_bit(WDOG_HW_RUNNING, &wdd->status);
 	} else {
 		wdd->timeout = DW_WDT_DEFAULT_SECONDS;
-- 
2.26.2

[PATCH v3 5/7] watchdog: dw_wdt: Support devices with asynch clocks

[Serge Semin]

DW Watchdog IP core can be synthesised with asynchronous timer/APB
clocks support (WDT_ASYNC_CLK_MODE_ENABLE == 1). In this case
separate clock signals are supposed to be used to feed watchdog timer
and APB interface of the device. Currently the driver supports
the synchronous mode only. Since there is no way to determine which
mode was actually activated for device from its registers, we have to
rely on the platform device configuration data. If optional "pclk"
clock source is supplied, we consider the device working in asynchronous
mode, otherwise the driver falls back to the synchronous configuration.

Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: linux-mips@vger.kernel.org
Cc: devicetree@vger.kernel.org

---

Changelog v2:
- Rearrange SoBs.
---
 drivers/watchdog/dw_wdt.c | 48 +++++++++++++++++++++++++++++++++++----
 1 file changed, 43 insertions(+), 5 deletions(-)

diff --git a/drivers/watchdog/dw_wdt.c b/drivers/watchdog/dw_wdt.c
index 693c0d1fd796..efbc36872670 100644
--- a/drivers/watchdog/dw_wdt.c
+++ b/drivers/watchdog/dw_wdt.c
@@ -68,6 +68,7 @@ struct dw_wdt_timeout {
 struct dw_wdt {
 	void __iomem		*regs;
 	struct clk		*clk;
+	struct clk		*pclk;
 	unsigned long		rate;
 	struct dw_wdt_timeout	timeouts[DW_WDT_NUM_TOPS];
 	struct watchdog_device	wdd;
@@ -274,6 +275,7 @@ static int dw_wdt_suspend(struct device *dev)
 	dw_wdt->control = readl(dw_wdt->regs + WDOG_CONTROL_REG_OFFSET);
 	dw_wdt->timeout = readl(dw_wdt->regs + WDOG_TIMEOUT_RANGE_REG_OFFSET);
 
+	clk_disable_unprepare(dw_wdt->pclk);
 	clk_disable_unprepare(dw_wdt->clk);
 
 	return 0;
@@ -287,6 +289,12 @@ static int dw_wdt_resume(struct device *dev)
 	if (err)
 		return err;
 
+	err = clk_prepare_enable(dw_wdt->pclk);
+	if (err) {
+		clk_disable_unprepare(dw_wdt->clk);
+		return err;
+	}
+
 	writel(dw_wdt->timeout, dw_wdt->regs + WDOG_TIMEOUT_RANGE_REG_OFFSET);
 	writel(dw_wdt->control, dw_wdt->regs + WDOG_CONTROL_REG_OFFSET);
 
@@ -393,9 +401,18 @@ static int dw_wdt_drv_probe(struct platform_device *pdev)
 	if (IS_ERR(dw_wdt->regs))
 		return PTR_ERR(dw_wdt->regs);
 
-	dw_wdt->clk = devm_clk_get(dev, NULL);
-	if (IS_ERR(dw_wdt->clk))
-		return PTR_ERR(dw_wdt->clk);
+	/*
+	 * Try to request the watchdog dedicated timer clock source. It must
+	 * be supplied if asynchronous mode is enabled. Otherwise fallback
+	 * to the common timer/bus clocks configuration, in which the very
+	 * first found clock supply both timer and APB signals.
+	 */
+	dw_wdt->clk = devm_clk_get(dev, "tclk");
+	if (IS_ERR(dw_wdt->clk)) {
+		dw_wdt->clk = devm_clk_get(dev, NULL);
+		if (IS_ERR(dw_wdt->clk))
+			return PTR_ERR(dw_wdt->clk);
+	}
 
 	ret = clk_prepare_enable(dw_wdt->clk);
 	if (ret)
@@ -407,10 +424,27 @@ static int dw_wdt_drv_probe(struct platform_device *pdev)
 		goto out_disable_clk;
 	}
 
+	/*
+	 * Request APB clock if device is configured with async clocks mode.
+	 * In this case both tclk and pclk clocks are supposed to be specified.
+	 * Alas we can't know for sure whether async mode was really activated,
+	 * so the pclk phandle reference is left optional. If it couldn't be
+	 * found we consider the device configured in synchronous clocks mode.
+	 */
+	dw_wdt->pclk = devm_clk_get_optional(dev, "pclk");
+	if (IS_ERR(dw_wdt->pclk)) {
+		ret = PTR_ERR(dw_wdt->pclk);
+		goto out_disable_clk;
+	}
+
+	ret = clk_prepare_enable(dw_wdt->pclk);
+	if (ret)
+		goto out_disable_clk;
+
 	dw_wdt->rst = devm_reset_control_get_optional_shared(&pdev->dev, NULL);
 	if (IS_ERR(dw_wdt->rst)) {
 		ret = PTR_ERR(dw_wdt->rst);
-		goto out_disable_clk;
+		goto out_disable_pclk;
 	}
 
 	reset_control_deassert(dw_wdt->rst);
@@ -449,10 +483,13 @@ static int dw_wdt_drv_probe(struct platform_device *pdev)
 
 	ret = watchdog_register_device(wdd);
 	if (ret)
-		goto out_disable_clk;
+		goto out_disable_pclk;
 
 	return 0;
 
+out_disable_pclk:
+	clk_disable_unprepare(dw_wdt->pclk);
+
 out_disable_clk:
 	clk_disable_unprepare(dw_wdt->clk);
 	return ret;
@@ -464,6 +501,7 @@ static int dw_wdt_drv_remove(struct platform_device *pdev)
 
 	watchdog_unregister_device(&dw_wdt->wdd);
 	reset_control_assert(dw_wdt->rst);
+	clk_disable_unprepare(dw_wdt->pclk);
 	clk_disable_unprepare(dw_wdt->clk);
 
 	return 0;
-- 
2.26.2

[PATCH v3 6/7] watchdog: dw_wdt: Add pre-timeouts support

[Serge Semin]

DW Watchdog can rise an interrupt in case if IRQ request mode is enabled
and timer reaches the zero value. In this case the IRQ lane is left
pending until either the next watchdog kick event (watchdog restart) or
until the WDT_EOI register is read or the device/system reset. This
interface can be used to implement the pre-timeout functionality
optionally provided by the Linux kernel watchdog devices.

IRQ mode provides a two stages timeout interface. It means the IRQ is
raised when the counter reaches zero, while the system reset occurs only
after subsequent timeout if the timer restart is not performed. Due to
this peculiarity the pre-timeout value is actually set to the achieved
hardware timeout, while the real watchdog timeout is considered to be
twice as much of it. This applies a significant limitation on the
pre-timeout values, so current implementation supports either zero value,
which disables the pre-timeout events, or non-zero values, which imply
the pre-timeout to be at least half of the current watchdog timeout.

Note that we ask the interrupt controller to detect the rising-edge
pre-timeout interrupts to prevent the high-level-IRQs flood, since
if the pre-timeout happens, the IRQ lane will be left pending until
it's cleared by the timer restart.

Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: linux-mips@vger.kernel.org
Cc: devicetree@vger.kernel.org

---

Changelog v2:
- Rearrange SoBs.
- Make the Pre-timeout IRQ being optionally supported.
---
 drivers/watchdog/dw_wdt.c | 138 +++++++++++++++++++++++++++++++++++---
 1 file changed, 130 insertions(+), 8 deletions(-)

diff --git a/drivers/watchdog/dw_wdt.c b/drivers/watchdog/dw_wdt.c
index efbc36872670..3cd7c485cd70 100644
--- a/drivers/watchdog/dw_wdt.c
+++ b/drivers/watchdog/dw_wdt.c
@@ -22,6 +22,7 @@
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/moduleparam.h>
+#include <linux/interrupt.h>
 #include <linux/of.h>
 #include <linux/pm.h>
 #include <linux/platform_device.h>
@@ -36,6 +37,8 @@
 #define WDOG_CURRENT_COUNT_REG_OFFSET	    0x08
 #define WDOG_COUNTER_RESTART_REG_OFFSET     0x0c
 #define WDOG_COUNTER_RESTART_KICK_VALUE	    0x76
+#define WDOG_INTERRUPT_STATUS_REG_OFFSET    0x10
+#define WDOG_INTERRUPT_CLEAR_REG_OFFSET     0x14
 #define WDOG_COMP_PARAMS_1_REG_OFFSET       0xf4
 #define WDOG_COMP_PARAMS_1_USE_FIX_TOP      BIT(6)
 
@@ -59,6 +62,11 @@ module_param(nowayout, bool, 0);
 MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started "
 		 "(default=" __MODULE_STRING(WATCHDOG_NOWAYOUT) ")");
 
+enum dw_wdt_rmod {
+	DW_WDT_RMOD_RESET = 1,
+	DW_WDT_RMOD_IRQ = 2
+};
+
 struct dw_wdt_timeout {
 	u32 top_val;
 	unsigned int sec;
@@ -70,6 +78,7 @@ struct dw_wdt {
 	struct clk		*clk;
 	struct clk		*pclk;
 	unsigned long		rate;
+	enum dw_wdt_rmod	rmod;
 	struct dw_wdt_timeout	timeouts[DW_WDT_NUM_TOPS];
 	struct watchdog_device	wdd;
 	struct reset_control	*rst;
@@ -86,6 +95,20 @@ static inline int dw_wdt_is_enabled(struct dw_wdt *dw_wdt)
 		WDOG_CONTROL_REG_WDT_EN_MASK;
 }
 
+static void dw_wdt_update_mode(struct dw_wdt *dw_wdt, enum dw_wdt_rmod rmod)
+{
+	u32 val;
+
+	val = readl(dw_wdt->regs + WDOG_CONTROL_REG_OFFSET);
+	if (rmod == DW_WDT_RMOD_IRQ)
+		val |= WDOG_CONTROL_REG_RESP_MODE_MASK;
+	else
+		val &= ~WDOG_CONTROL_REG_RESP_MODE_MASK;
+	writel(val, dw_wdt->regs + WDOG_CONTROL_REG_OFFSET);
+
+	dw_wdt->rmod = rmod;
+}
+
 static unsigned int dw_wdt_find_best_top(struct dw_wdt *dw_wdt,
 					 unsigned int timeout, u32 *top_val)
 {
@@ -145,7 +168,11 @@ static unsigned int dw_wdt_get_timeout(struct dw_wdt *dw_wdt)
 			break;
 	}
 
-	return dw_wdt->timeouts[idx].sec;
+	/*
+	 * In IRQ mode due to the two stages counter, the actual timeout is
+	 * twice greater than the TOP setting.
+	 */
+	return dw_wdt->timeouts[idx].sec * dw_wdt->rmod;
 }
 
 static int dw_wdt_ping(struct watchdog_device *wdd)
@@ -164,7 +191,20 @@ static int dw_wdt_set_timeout(struct watchdog_device *wdd, unsigned int top_s)
 	unsigned int timeout;
 	u32 top_val;
 
-	timeout = dw_wdt_find_best_top(dw_wdt, top_s, &top_val);
+	/*
+	 * Note IRQ mode being enabled means having a non-zero pre-timeout
+	 * setup. In this case we try to find a TOP as close to the half of the
+	 * requested timeout as possible since DW Watchdog IRQ mode is designed
+	 * in two stages way - first timeout rises the pre-timeout interrupt,
+	 * second timeout performs the system reset. So basically the effective
+	 * watchdog-caused reset happens after two watchdog TOPs elapsed.
+	 */
+	timeout = dw_wdt_find_best_top(dw_wdt, DIV_ROUND_UP(top_s, dw_wdt->rmod),
+				       &top_val);
+	if (dw_wdt->rmod == DW_WDT_RMOD_IRQ)
+		wdd->pretimeout = timeout;
+	else
+		wdd->pretimeout = 0;
 
 	/*
 	 * Set the new value in the watchdog.  Some versions of dw_wdt
@@ -175,25 +215,47 @@ static int dw_wdt_set_timeout(struct watchdog_device *wdd, unsigned int top_s)
 	writel(top_val | top_val << WDOG_TIMEOUT_RANGE_TOPINIT_SHIFT,
 	       dw_wdt->regs + WDOG_TIMEOUT_RANGE_REG_OFFSET);
 
+	/* Kick new TOP value into the watchdog counter if activated. */
+	if (watchdog_active(wdd))
+		dw_wdt_ping(wdd);
+
 	/*
 	 * In case users set bigger timeout value than HW can support,
 	 * kernel(watchdog_dev.c) helps to feed watchdog before
 	 * wdd->max_hw_heartbeat_ms
 	 */
 	if (top_s * 1000 <= wdd->max_hw_heartbeat_ms)
-		wdd->timeout = timeout;
+		wdd->timeout = timeout * dw_wdt->rmod;
 	else
 		wdd->timeout = top_s;
 
 	return 0;
 }
 
+static int dw_wdt_set_pretimeout(struct watchdog_device *wdd, unsigned int req)
+{
+	struct dw_wdt *dw_wdt = to_dw_wdt(wdd);
+
+	/*
+	 * We ignore actual value of the timeout passed from user-space
+	 * using it as a flag whether the pretimeout functionality is intended
+	 * to be activated.
+	 */
+	dw_wdt_update_mode(dw_wdt, req ? DW_WDT_RMOD_IRQ : DW_WDT_RMOD_RESET);
+	dw_wdt_set_timeout(wdd, wdd->timeout);
+
+	return 0;
+}
+
 static void dw_wdt_arm_system_reset(struct dw_wdt *dw_wdt)
 {
 	u32 val = readl(dw_wdt->regs + WDOG_CONTROL_REG_OFFSET);
 
-	/* Disable interrupt mode; always perform system reset. */
-	val &= ~WDOG_CONTROL_REG_RESP_MODE_MASK;
+	/* Disable/enable interrupt mode depending on the RMOD flag. */
+	if (dw_wdt->rmod == DW_WDT_RMOD_IRQ)
+		val |= WDOG_CONTROL_REG_RESP_MODE_MASK;
+	else
+		val &= ~WDOG_CONTROL_REG_RESP_MODE_MASK;
 	/* Enable watchdog. */
 	val |= WDOG_CONTROL_REG_WDT_EN_MASK;
 	writel(val, dw_wdt->regs + WDOG_CONTROL_REG_OFFSET);
@@ -231,6 +293,7 @@ static int dw_wdt_restart(struct watchdog_device *wdd,
 	struct dw_wdt *dw_wdt = to_dw_wdt(wdd);
 
 	writel(0, dw_wdt->regs + WDOG_TIMEOUT_RANGE_REG_OFFSET);
+	dw_wdt_update_mode(dw_wdt, DW_WDT_RMOD_RESET);
 	if (dw_wdt_is_enabled(dw_wdt))
 		writel(WDOG_COUNTER_RESTART_KICK_VALUE,
 		       dw_wdt->regs + WDOG_COUNTER_RESTART_REG_OFFSET);
@@ -246,9 +309,19 @@ static int dw_wdt_restart(struct watchdog_device *wdd,
 static unsigned int dw_wdt_get_timeleft(struct watchdog_device *wdd)
 {
 	struct dw_wdt *dw_wdt = to_dw_wdt(wdd);
+	unsigned int sec;
+	u32 val;
+
+	val = readl(dw_wdt->regs + WDOG_CURRENT_COUNT_REG_OFFSET);
+	sec = val / dw_wdt->rate;
 
-	return readl(dw_wdt->regs + WDOG_CURRENT_COUNT_REG_OFFSET) /
-		dw_wdt->rate;
+	if (dw_wdt->rmod == DW_WDT_RMOD_IRQ) {
+		val = readl(dw_wdt->regs + WDOG_INTERRUPT_STATUS_REG_OFFSET);
+		if (!val)
+			sec += wdd->pretimeout;
+	}
+
+	return sec;
 }
 
 static const struct watchdog_info dw_wdt_ident = {
@@ -257,16 +330,41 @@ static const struct watchdog_info dw_wdt_ident = {
 	.identity	= "Synopsys DesignWare Watchdog",
 };
 
+static const struct watchdog_info dw_wdt_pt_ident = {
+	.options	= WDIOF_KEEPALIVEPING | WDIOF_SETTIMEOUT |
+			  WDIOF_PRETIMEOUT | WDIOF_MAGICCLOSE,
+	.identity	= "Synopsys DesignWare Watchdog",
+};
+
 static const struct watchdog_ops dw_wdt_ops = {
 	.owner		= THIS_MODULE,
 	.start		= dw_wdt_start,
 	.stop		= dw_wdt_stop,
 	.ping		= dw_wdt_ping,
 	.set_timeout	= dw_wdt_set_timeout,
+	.set_pretimeout	= dw_wdt_set_pretimeout,
 	.get_timeleft	= dw_wdt_get_timeleft,
 	.restart	= dw_wdt_restart,
 };
 
+static irqreturn_t dw_wdt_irq(int irq, void *devid)
+{
+	struct dw_wdt *dw_wdt = devid;
+	u32 val;
+
+	/*
+	 * We don't clear the IRQ status. It's supposed to be done by the
+	 * following ping operations.
+	 */
+	val = readl(dw_wdt->regs + WDOG_INTERRUPT_STATUS_REG_OFFSET);
+	if (!val)
+		return IRQ_NONE;
+
+	watchdog_notify_pretimeout(&dw_wdt->wdd);
+
+	return IRQ_HANDLED;
+}
+
 #ifdef CONFIG_PM_SLEEP
 static int dw_wdt_suspend(struct device *dev)
 {
@@ -447,6 +545,31 @@ static int dw_wdt_drv_probe(struct platform_device *pdev)
 		goto out_disable_pclk;
 	}
 
+	/* Enable normal reset without pre-timeout by default. */
+	dw_wdt_update_mode(dw_wdt, DW_WDT_RMOD_RESET);
+
+	/*
+	 * Pre-timeout IRQ is optional, since some hardware may lack support
+	 * of it. Note we must request rising-edge IRQ, since the lane is left
+	 * pending either until the next watchdog kick event or up to the
+	 * system reset.
+	 */
+	ret = platform_get_irq_optional(pdev, 0);
+	if (ret >= 0) {
+		ret = devm_request_irq(dev, ret, dw_wdt_irq,
+				       IRQF_SHARED | IRQF_TRIGGER_RISING,
+				       pdev->name, dw_wdt);
+		if (ret)
+			goto out_disable_pclk;
+
+		dw_wdt->wdd.info = &dw_wdt_pt_ident;
+	} else {
+		if (ret == -EPROBE_DEFER)
+			goto out_disable_pclk;
+
+		dw_wdt->wdd.info = &dw_wdt_ident;
+	}
+
 	reset_control_deassert(dw_wdt->rst);
 
 	ret = dw_wdt_init_timeouts(dw_wdt, dev);
@@ -454,7 +577,6 @@ static int dw_wdt_drv_probe(struct platform_device *pdev)
 		goto out_disable_clk;
 
 	wdd = &dw_wdt->wdd;
-	wdd->info = &dw_wdt_ident;
 	wdd->ops = &dw_wdt_ops;
 	wdd->min_timeout = dw_wdt_get_min_timeout(dw_wdt);
 	wdd->max_hw_heartbeat_ms = dw_wdt_get_max_timeout_ms(dw_wdt);
-- 
2.26.2

[PATCH v3 7/7] watchdog: dw_wdt: Add DebugFS files

[Serge Semin]

For the sake of the easier device-driver debug procedure, we added a
DebugFS file with the controller registers state. It's available only if
kernel is configured with DebugFS support.

Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: linux-mips@vger.kernel.org
Cc: devicetree@vger.kernel.org

---

Changelog v2:
- Rearrange SoBs.
- Discard timeout/pretimeout/ping/enable DebugFS nodes. Registers state
  dump node is only left.
---
 drivers/watchdog/dw_wdt.c | 68 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 68 insertions(+)

diff --git a/drivers/watchdog/dw_wdt.c b/drivers/watchdog/dw_wdt.c
index 3cd7c485cd70..012681baaa6d 100644
--- a/drivers/watchdog/dw_wdt.c
+++ b/drivers/watchdog/dw_wdt.c
@@ -28,6 +28,7 @@
 #include <linux/platform_device.h>
 #include <linux/reset.h>
 #include <linux/watchdog.h>
+#include <linux/debugfs.h>
 
 #define WDOG_CONTROL_REG_OFFSET		    0x00
 #define WDOG_CONTROL_REG_WDT_EN_MASK	    0x01
@@ -39,8 +40,14 @@
 #define WDOG_COUNTER_RESTART_KICK_VALUE	    0x76
 #define WDOG_INTERRUPT_STATUS_REG_OFFSET    0x10
 #define WDOG_INTERRUPT_CLEAR_REG_OFFSET     0x14
+#define WDOG_COMP_PARAMS_5_REG_OFFSET       0xe4
+#define WDOG_COMP_PARAMS_4_REG_OFFSET       0xe8
+#define WDOG_COMP_PARAMS_3_REG_OFFSET       0xec
+#define WDOG_COMP_PARAMS_2_REG_OFFSET       0xf0
 #define WDOG_COMP_PARAMS_1_REG_OFFSET       0xf4
 #define WDOG_COMP_PARAMS_1_USE_FIX_TOP      BIT(6)
+#define WDOG_COMP_VERSION_REG_OFFSET        0xf8
+#define WDOG_COMP_TYPE_REG_OFFSET           0xfc
 
 /* There are sixteen TOPs (timeout periods) that can be set in the watchdog. */
 #define DW_WDT_NUM_TOPS		16
@@ -85,6 +92,10 @@ struct dw_wdt {
 	/* Save/restore */
 	u32			control;
 	u32			timeout;
+
+#ifdef CONFIG_DEBUG_FS
+	struct dentry		*dbgfs_dir;
+#endif
 };
 
 #define to_dw_wdt(wdd)	container_of(wdd, struct dw_wdt, wdd)
@@ -484,6 +495,59 @@ static int dw_wdt_init_timeouts(struct dw_wdt *dw_wdt, struct device *dev)
 	return 0;
 }
 
+#ifdef CONFIG_DEBUG_FS
+
+#define DW_WDT_DBGFS_REG(_name, _off) \
+{				      \
+	.name = _name,		      \
+	.offset = _off		      \
+}
+
+static const struct debugfs_reg32 dw_wdt_dbgfs_regs[] = {
+	DW_WDT_DBGFS_REG("cr", WDOG_CONTROL_REG_OFFSET),
+	DW_WDT_DBGFS_REG("torr", WDOG_TIMEOUT_RANGE_REG_OFFSET),
+	DW_WDT_DBGFS_REG("ccvr", WDOG_CURRENT_COUNT_REG_OFFSET),
+	DW_WDT_DBGFS_REG("crr", WDOG_COUNTER_RESTART_REG_OFFSET),
+	DW_WDT_DBGFS_REG("stat", WDOG_INTERRUPT_STATUS_REG_OFFSET),
+	DW_WDT_DBGFS_REG("param5", WDOG_COMP_PARAMS_5_REG_OFFSET),
+	DW_WDT_DBGFS_REG("param4", WDOG_COMP_PARAMS_4_REG_OFFSET),
+	DW_WDT_DBGFS_REG("param3", WDOG_COMP_PARAMS_3_REG_OFFSET),
+	DW_WDT_DBGFS_REG("param2", WDOG_COMP_PARAMS_2_REG_OFFSET),
+	DW_WDT_DBGFS_REG("param1", WDOG_COMP_PARAMS_1_REG_OFFSET),
+	DW_WDT_DBGFS_REG("version", WDOG_COMP_VERSION_REG_OFFSET),
+	DW_WDT_DBGFS_REG("type", WDOG_COMP_TYPE_REG_OFFSET)
+};
+
+static void dw_wdt_dbgfs_init(struct dw_wdt *dw_wdt)
+{
+	struct device *dev = dw_wdt->wdd.parent;
+	struct debugfs_regset32 *regset;
+
+	regset = devm_kzalloc(dev, sizeof(*regset), GFP_KERNEL);
+	if (!regset)
+		return;
+
+	regset->regs = dw_wdt_dbgfs_regs;
+	regset->nregs = ARRAY_SIZE(dw_wdt_dbgfs_regs);
+	regset->base = dw_wdt->regs;
+
+	dw_wdt->dbgfs_dir = debugfs_create_dir(dev_name(dev), NULL);
+
+	debugfs_create_regset32("registers", 0444, dw_wdt->dbgfs_dir, regset);
+}
+
+static void dw_wdt_dbgfs_clear(struct dw_wdt *dw_wdt)
+{
+	debugfs_remove_recursive(dw_wdt->dbgfs_dir);
+}
+
+#else /* !CONFIG_DEBUG_FS */
+
+static void dw_wdt_dbgfs_init(struct dw_wdt *dw_wdt) {}
+static void dw_wdt_dbgfs_clear(struct dw_wdt *dw_wdt) {}
+
+#endif /* !CONFIG_DEBUG_FS */
+
 static int dw_wdt_drv_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -607,6 +671,8 @@ static int dw_wdt_drv_probe(struct platform_device *pdev)
 	if (ret)
 		goto out_disable_pclk;
 
+	dw_wdt_dbgfs_init(dw_wdt);
+
 	return 0;
 
 out_disable_pclk:
@@ -621,6 +687,8 @@ static int dw_wdt_drv_remove(struct platform_device *pdev)
 {
 	struct dw_wdt *dw_wdt = platform_get_drvdata(pdev);
 
+	dw_wdt_dbgfs_clear(dw_wdt);
+
 	watchdog_unregister_device(&dw_wdt->wdd);
 	reset_control_assert(dw_wdt->rst);
 	clk_disable_unprepare(dw_wdt->pclk);
-- 
2.26.2

Re: [PATCH v3 4/7] watchdog: dw_wdt: Support devices with non-fixed TOP values

[Guenter Roeck]

On Tue, May 26, 2020 at 06:41:20PM +0300, Serge Semin wrote:
> In case if the DW Watchdog IP core is synthesised with
> WDT_USE_FIX_TOP == false, the TOP interval indexes make the device
> to load a custom periods to the counter. These periods are hardwired
> at the IP synthesis stage and can be within [2^8, 2^(WDT_CNT_WIDTH - 1)].
> Alas their values can't be detected at runtime and must be somehow
> supplied to the driver so one could properly determine the watchdog
> timeout intervals. For this purpose we suggest to have a vendor-
> specific dts property "snps,watchdog-tops" utilized, which would
> provide an array of sixteen counter values. At device probe stage they
> will be used to initialize the watchdog device timeouts determined
> from the array values and current clocks source rate.
> 
> In order to have custom TOP values supported the driver must be
> altered in the following way. First of all the fixed-top values
> ready-to-use array must be determined for compatibility with currently
> supported devices, which were synthesised with WDT_USE_FIX_TOP == true.
> It will be used if either fixed TOP feature is detected being enabled or
> no custom TOPs are fetched from the device dt node. Secondly at the probe
> stage we must initialize an array of the watchdog timeouts corresponding
> to the detected TOPs list and the reference clock rate. For generality the
> procedure of initialization is designed in a way to support the TOPs array
> with no limitations on the items order or value. Finally the watchdog
> period search methods should be altered to support the new timeouts data
> structure.
> 
> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
> Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: linux-mips@vger.kernel.org
> Cc: devicetree@vger.kernel.org

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

> ---
> 
> Changelog v2:
> - Rearrange SoBs.
> - Add "ms" suffix to the methods returning msec and convert the methods
>   with no "ms" suffix to return a timeout in sec.
> - Make sure minimum timeout is at least 1 sec.
> - Refactor the timeouts calculation procedure to retain the timeouts in
>   the ascending order.
> - Make sure there is no integer overflow in milliseconds calculation. It
>   is saved in a dedicated uint field of the timeout structure.
> ---
>  drivers/watchdog/dw_wdt.c | 191 +++++++++++++++++++++++++++++++++-----
>  1 file changed, 167 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/watchdog/dw_wdt.c b/drivers/watchdog/dw_wdt.c
> index fba21de2bbad..693c0d1fd796 100644
> --- a/drivers/watchdog/dw_wdt.c
> +++ b/drivers/watchdog/dw_wdt.c
> @@ -13,6 +13,8 @@
>   */
>  
>  #include <linux/bitops.h>
> +#include <linux/limits.h>
> +#include <linux/kernel.h>
>  #include <linux/clk.h>
>  #include <linux/delay.h>
>  #include <linux/err.h>
> @@ -34,21 +36,40 @@
>  #define WDOG_CURRENT_COUNT_REG_OFFSET	    0x08
>  #define WDOG_COUNTER_RESTART_REG_OFFSET     0x0c
>  #define WDOG_COUNTER_RESTART_KICK_VALUE	    0x76
> +#define WDOG_COMP_PARAMS_1_REG_OFFSET       0xf4
> +#define WDOG_COMP_PARAMS_1_USE_FIX_TOP      BIT(6)
>  
> -/* The maximum TOP (timeout period) value that can be set in the watchdog. */
> -#define DW_WDT_MAX_TOP		15
> +/* There are sixteen TOPs (timeout periods) that can be set in the watchdog. */
> +#define DW_WDT_NUM_TOPS		16
> +#define DW_WDT_FIX_TOP(_idx)	(1U << (16 + _idx))
>  
>  #define DW_WDT_DEFAULT_SECONDS	30
>  
> +static const u32 dw_wdt_fix_tops[DW_WDT_NUM_TOPS] = {
> +	DW_WDT_FIX_TOP(0), DW_WDT_FIX_TOP(1), DW_WDT_FIX_TOP(2),
> +	DW_WDT_FIX_TOP(3), DW_WDT_FIX_TOP(4), DW_WDT_FIX_TOP(5),
> +	DW_WDT_FIX_TOP(6), DW_WDT_FIX_TOP(7), DW_WDT_FIX_TOP(8),
> +	DW_WDT_FIX_TOP(9), DW_WDT_FIX_TOP(10), DW_WDT_FIX_TOP(11),
> +	DW_WDT_FIX_TOP(12), DW_WDT_FIX_TOP(13), DW_WDT_FIX_TOP(14),
> +	DW_WDT_FIX_TOP(15)
> +};
> +
>  static bool nowayout = WATCHDOG_NOWAYOUT;
>  module_param(nowayout, bool, 0);
>  MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started "
>  		 "(default=" __MODULE_STRING(WATCHDOG_NOWAYOUT) ")");
>  
> +struct dw_wdt_timeout {
> +	u32 top_val;
> +	unsigned int sec;
> +	unsigned int msec;
> +};
> +
>  struct dw_wdt {
>  	void __iomem		*regs;
>  	struct clk		*clk;
>  	unsigned long		rate;
> +	struct dw_wdt_timeout	timeouts[DW_WDT_NUM_TOPS];
>  	struct watchdog_device	wdd;
>  	struct reset_control	*rst;
>  	/* Save/restore */
> @@ -64,20 +85,66 @@ static inline int dw_wdt_is_enabled(struct dw_wdt *dw_wdt)
>  		WDOG_CONTROL_REG_WDT_EN_MASK;
>  }
>  
> -static inline int dw_wdt_top_in_seconds(struct dw_wdt *dw_wdt, unsigned top)
> +static unsigned int dw_wdt_find_best_top(struct dw_wdt *dw_wdt,
> +					 unsigned int timeout, u32 *top_val)
>  {
> +	int idx;
> +
>  	/*
> -	 * There are 16 possible timeout values in 0..15 where the number of
> -	 * cycles is 2 ^ (16 + i) and the watchdog counts down.
> +	 * Find a TOP with timeout greater or equal to the requested number.
> +	 * Note we'll select a TOP with maximum timeout if the requested
> +	 * timeout couldn't be reached.
>  	 */
> -	return (1U << (16 + top)) / dw_wdt->rate;
> +	for (idx = 0; idx < DW_WDT_NUM_TOPS; ++idx) {
> +		if (dw_wdt->timeouts[idx].sec >= timeout)
> +			break;
> +	}
> +
> +	if (idx == DW_WDT_NUM_TOPS)
> +		--idx;
> +
> +	*top_val = dw_wdt->timeouts[idx].top_val;
> +
> +	return dw_wdt->timeouts[idx].sec;
>  }
>  
> -static int dw_wdt_get_top(struct dw_wdt *dw_wdt)
> +static unsigned int dw_wdt_get_min_timeout(struct dw_wdt *dw_wdt)
>  {
> -	int top = readl(dw_wdt->regs + WDOG_TIMEOUT_RANGE_REG_OFFSET) & 0xF;
> +	int idx;
> +
> +	/*
> +	 * We'll find a timeout greater or equal to one second anyway because
> +	 * the driver probe would have failed if there was none.
> +	 */
> +	for (idx = 0; idx < DW_WDT_NUM_TOPS; ++idx) {
> +		if (dw_wdt->timeouts[idx].sec)
> +			break;
> +	}
>  
> -	return dw_wdt_top_in_seconds(dw_wdt, top);
> +	return dw_wdt->timeouts[idx].sec;
> +}
> +
> +static unsigned int dw_wdt_get_max_timeout_ms(struct dw_wdt *dw_wdt)
> +{
> +	struct dw_wdt_timeout *timeout = &dw_wdt->timeouts[DW_WDT_NUM_TOPS - 1];
> +	u64 msec;
> +
> +	msec = (u64)timeout->sec * MSEC_PER_SEC + timeout->msec;
> +
> +	return msec < UINT_MAX ? msec : UINT_MAX;
> +}
> +
> +static unsigned int dw_wdt_get_timeout(struct dw_wdt *dw_wdt)
> +{
> +	int top_val = readl(dw_wdt->regs + WDOG_TIMEOUT_RANGE_REG_OFFSET) & 0xF;
> +	int idx;
> +
> +	for (idx = 0; idx < DW_WDT_NUM_TOPS; ++idx) {
> +		if (dw_wdt->timeouts[idx].top_val == top_val)
> +			break;
> +	}
> +
> +	return dw_wdt->timeouts[idx].sec;
>  }
>  
>  static int dw_wdt_ping(struct watchdog_device *wdd)
> @@ -93,17 +160,10 @@ static int dw_wdt_ping(struct watchdog_device *wdd)
>  static int dw_wdt_set_timeout(struct watchdog_device *wdd, unsigned int top_s)
>  {
>  	struct dw_wdt *dw_wdt = to_dw_wdt(wdd);
> -	int i, top_val = DW_WDT_MAX_TOP;
> +	unsigned int timeout;
> +	u32 top_val;
>  
> -	/*
> -	 * Iterate over the timeout values until we find the closest match. We
> -	 * always look for >=.
> -	 */
> -	for (i = 0; i <= DW_WDT_MAX_TOP; ++i)
> -		if (dw_wdt_top_in_seconds(dw_wdt, i) >= top_s) {
> -			top_val = i;
> -			break;
> -		}
> +	timeout = dw_wdt_find_best_top(dw_wdt, top_s, &top_val);
>  
>  	/*
>  	 * Set the new value in the watchdog.  Some versions of dw_wdt
> @@ -120,7 +180,7 @@ static int dw_wdt_set_timeout(struct watchdog_device *wdd, unsigned int top_s)
>  	 * wdd->max_hw_heartbeat_ms
>  	 */
>  	if (top_s * 1000 <= wdd->max_hw_heartbeat_ms)
> -		wdd->timeout = dw_wdt_top_in_seconds(dw_wdt, top_val);
> +		wdd->timeout = timeout;
>  	else
>  		wdd->timeout = top_s;
>  
> @@ -238,6 +298,86 @@ static int dw_wdt_resume(struct device *dev)
>  
>  static SIMPLE_DEV_PM_OPS(dw_wdt_pm_ops, dw_wdt_suspend, dw_wdt_resume);
>  
> +/*
> + * In case if DW WDT IP core is synthesized with fixed TOP feature disabled the
> + * TOPs array can be arbitrary ordered with nearly any sixteen uint numbers
> + * depending on the system engineer imagination. The next method handles the
> + * passed TOPs array to pre-calculate the effective timeouts and to sort the
> + * TOP items out in the ascending order with respect to the timeouts.
> + */
> +
> +static void dw_wdt_handle_tops(struct dw_wdt *dw_wdt, const u32 *tops)
> +{
> +	struct dw_wdt_timeout tout, *dst;
> +	int val, tidx;
> +	u64 msec;
> +
> +	/*
> +	 * We walk over the passed TOPs array and calculate corresponding
> +	 * timeouts in seconds and milliseconds. The milliseconds granularity
> +	 * is needed to distinguish the TOPs with very close timeouts and to
> +	 * set the watchdog max heartbeat setting further.
> +	 */
> +	for (val = 0; val < DW_WDT_NUM_TOPS; ++val) {
> +		tout.top_val = val;
> +		tout.sec = tops[val] / dw_wdt->rate;
> +		msec = (u64)tops[val] * MSEC_PER_SEC;
> +		do_div(msec, dw_wdt->rate);
> +		tout.msec = msec - ((u64)tout.sec * MSEC_PER_SEC);
> +
> +		/*
> +		 * Find a suitable place for the current TOP in the timeouts
> +		 * array so that the list is remained in the ascending order.
> +		 */
> +		for (tidx = 0; tidx < val; ++tidx) {
> +			dst = &dw_wdt->timeouts[tidx];
> +			if (tout.sec > dst->sec || (tout.sec == dst->sec &&
> +			    tout.msec >= dst->msec))
> +				continue;
> +			else
> +				swap(*dst, tout);
> +		}
> +
> +		dw_wdt->timeouts[val] = tout;
> +	}
> +}
> +
> +static int dw_wdt_init_timeouts(struct dw_wdt *dw_wdt, struct device *dev)
> +{
> +	u32 data, of_tops[DW_WDT_NUM_TOPS];
> +	const u32 *tops;
> +	int ret;
> +
> +	/*
> +	 * Retrieve custom or fixed counter values depending on the
> +	 * WDT_USE_FIX_TOP flag found in the component specific parameters
> +	 * #1 register.
> +	 */
> +	data = readl(dw_wdt->regs + WDOG_COMP_PARAMS_1_REG_OFFSET);
> +	if (data & WDOG_COMP_PARAMS_1_USE_FIX_TOP) {
> +		tops = dw_wdt_fix_tops;
> +	} else {
> +		ret = of_property_read_variable_u32_array(dev_of_node(dev),
> +			"snps,watchdog-tops", of_tops, DW_WDT_NUM_TOPS,
> +			DW_WDT_NUM_TOPS);
> +		if (ret < 0) {
> +			dev_warn(dev, "No valid TOPs array specified\n");
> +			tops = dw_wdt_fix_tops;
> +		} else {
> +			tops = of_tops;
> +		}
> +	}
> +
> +	/* Convert the specified TOPs into an array of watchdog timeouts. */
> +	dw_wdt_handle_tops(dw_wdt, tops);
> +	if (!dw_wdt->timeouts[DW_WDT_NUM_TOPS - 1].sec) {
> +		dev_err(dev, "No any valid TOP detected\n");
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
>  static int dw_wdt_drv_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
> @@ -275,12 +415,15 @@ static int dw_wdt_drv_probe(struct platform_device *pdev)
>  
>  	reset_control_deassert(dw_wdt->rst);
>  
> +	ret = dw_wdt_init_timeouts(dw_wdt, dev);
> +	if (ret)
> +		goto out_disable_clk;
> +
>  	wdd = &dw_wdt->wdd;
>  	wdd->info = &dw_wdt_ident;
>  	wdd->ops = &dw_wdt_ops;
> -	wdd->min_timeout = 1;
> -	wdd->max_hw_heartbeat_ms =
> -		dw_wdt_top_in_seconds(dw_wdt, DW_WDT_MAX_TOP) * 1000;
> +	wdd->min_timeout = dw_wdt_get_min_timeout(dw_wdt);
> +	wdd->max_hw_heartbeat_ms = dw_wdt_get_max_timeout_ms(dw_wdt);
>  	wdd->parent = dev;
>  
>  	watchdog_set_drvdata(wdd, dw_wdt);
> @@ -293,7 +436,7 @@ static int dw_wdt_drv_probe(struct platform_device *pdev)
>  	 * devicetree.
>  	 */
>  	if (dw_wdt_is_enabled(dw_wdt)) {
> -		wdd->timeout = dw_wdt_get_top(dw_wdt);
> +		wdd->timeout = dw_wdt_get_timeout(dw_wdt);
>  		set_bit(WDOG_HW_RUNNING, &wdd->status);
>  	} else {
>  		wdd->timeout = DW_WDT_DEFAULT_SECONDS;

Re: [PATCH v3 5/7] watchdog: dw_wdt: Support devices with asynch clocks

[Guenter Roeck]

On Tue, May 26, 2020 at 06:41:21PM +0300, Serge Semin wrote:
> DW Watchdog IP core can be synthesised with asynchronous timer/APB
> clocks support (WDT_ASYNC_CLK_MODE_ENABLE == 1). In this case
> separate clock signals are supposed to be used to feed watchdog timer
> and APB interface of the device. Currently the driver supports
> the synchronous mode only. Since there is no way to determine which
> mode was actually activated for device from its registers, we have to
> rely on the platform device configuration data. If optional "pclk"
> clock source is supplied, we consider the device working in asynchronous
> mode, otherwise the driver falls back to the synchronous configuration.
> 
> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
> Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: linux-mips@vger.kernel.org
> Cc: devicetree@vger.kernel.org

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

> ---
> 
> Changelog v2:
> - Rearrange SoBs.
> ---
>  drivers/watchdog/dw_wdt.c | 48 +++++++++++++++++++++++++++++++++++----
>  1 file changed, 43 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/watchdog/dw_wdt.c b/drivers/watchdog/dw_wdt.c
> index 693c0d1fd796..efbc36872670 100644
> --- a/drivers/watchdog/dw_wdt.c
> +++ b/drivers/watchdog/dw_wdt.c
> @@ -68,6 +68,7 @@ struct dw_wdt_timeout {
>  struct dw_wdt {
>  	void __iomem		*regs;
>  	struct clk		*clk;
> +	struct clk		*pclk;
>  	unsigned long		rate;
>  	struct dw_wdt_timeout	timeouts[DW_WDT_NUM_TOPS];
>  	struct watchdog_device	wdd;
> @@ -274,6 +275,7 @@ static int dw_wdt_suspend(struct device *dev)
>  	dw_wdt->control = readl(dw_wdt->regs + WDOG_CONTROL_REG_OFFSET);
>  	dw_wdt->timeout = readl(dw_wdt->regs + WDOG_TIMEOUT_RANGE_REG_OFFSET);
>  
> +	clk_disable_unprepare(dw_wdt->pclk);
>  	clk_disable_unprepare(dw_wdt->clk);
>  
>  	return 0;
> @@ -287,6 +289,12 @@ static int dw_wdt_resume(struct device *dev)
>  	if (err)
>  		return err;
>  
> +	err = clk_prepare_enable(dw_wdt->pclk);
> +	if (err) {
> +		clk_disable_unprepare(dw_wdt->clk);
> +		return err;
> +	}
> +
>  	writel(dw_wdt->timeout, dw_wdt->regs + WDOG_TIMEOUT_RANGE_REG_OFFSET);
>  	writel(dw_wdt->control, dw_wdt->regs + WDOG_CONTROL_REG_OFFSET);
>  
> @@ -393,9 +401,18 @@ static int dw_wdt_drv_probe(struct platform_device *pdev)
>  	if (IS_ERR(dw_wdt->regs))
>  		return PTR_ERR(dw_wdt->regs);
>  
> -	dw_wdt->clk = devm_clk_get(dev, NULL);
> -	if (IS_ERR(dw_wdt->clk))
> -		return PTR_ERR(dw_wdt->clk);
> +	/*
> +	 * Try to request the watchdog dedicated timer clock source. It must
> +	 * be supplied if asynchronous mode is enabled. Otherwise fallback
> +	 * to the common timer/bus clocks configuration, in which the very
> +	 * first found clock supply both timer and APB signals.
> +	 */
> +	dw_wdt->clk = devm_clk_get(dev, "tclk");
> +	if (IS_ERR(dw_wdt->clk)) {
> +		dw_wdt->clk = devm_clk_get(dev, NULL);
> +		if (IS_ERR(dw_wdt->clk))
> +			return PTR_ERR(dw_wdt->clk);
> +	}
>  
>  	ret = clk_prepare_enable(dw_wdt->clk);
>  	if (ret)
> @@ -407,10 +424,27 @@ static int dw_wdt_drv_probe(struct platform_device *pdev)
>  		goto out_disable_clk;
>  	}
>  
> +	/*
> +	 * Request APB clock if device is configured with async clocks mode.
> +	 * In this case both tclk and pclk clocks are supposed to be specified.
> +	 * Alas we can't know for sure whether async mode was really activated,
> +	 * so the pclk phandle reference is left optional. If it couldn't be
> +	 * found we consider the device configured in synchronous clocks mode.
> +	 */
> +	dw_wdt->pclk = devm_clk_get_optional(dev, "pclk");
> +	if (IS_ERR(dw_wdt->pclk)) {
> +		ret = PTR_ERR(dw_wdt->pclk);
> +		goto out_disable_clk;
> +	}
> +
> +	ret = clk_prepare_enable(dw_wdt->pclk);
> +	if (ret)
> +		goto out_disable_clk;
> +
>  	dw_wdt->rst = devm_reset_control_get_optional_shared(&pdev->dev, NULL);
>  	if (IS_ERR(dw_wdt->rst)) {
>  		ret = PTR_ERR(dw_wdt->rst);
> -		goto out_disable_clk;
> +		goto out_disable_pclk;
>  	}
>  
>  	reset_control_deassert(dw_wdt->rst);
> @@ -449,10 +483,13 @@ static int dw_wdt_drv_probe(struct platform_device *pdev)
>  
>  	ret = watchdog_register_device(wdd);
>  	if (ret)
> -		goto out_disable_clk;
> +		goto out_disable_pclk;
>  
>  	return 0;
>  
> +out_disable_pclk:
> +	clk_disable_unprepare(dw_wdt->pclk);
> +
>  out_disable_clk:
>  	clk_disable_unprepare(dw_wdt->clk);
>  	return ret;
> @@ -464,6 +501,7 @@ static int dw_wdt_drv_remove(struct platform_device *pdev)
>  
>  	watchdog_unregister_device(&dw_wdt->wdd);
>  	reset_control_assert(dw_wdt->rst);
> +	clk_disable_unprepare(dw_wdt->pclk);
>  	clk_disable_unprepare(dw_wdt->clk);
>  
>  	return 0;

Re: [PATCH v3 1/7] dt-bindings: watchdog: Convert DW WDT binding to DT schema

[Guenter Roeck]

On Tue, May 26, 2020 at 06:41:17PM +0300, Serge Semin wrote:
> Modern device tree bindings are supposed to be created as YAML-files
> in accordance with dt-schema. This commit replaces the DW Watchdog
> legacy bare text bindings with YAML file. As before the binding states
> that the corresponding dts node is supposed to have a registers
> range, a watchdog timer references clock source, optional reset line and
> pre-timeout interrupt.
> 
> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> Reviewed-by: Rob Herring <robh@kernel.org>
> Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
> Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: linux-mips@vger.kernel.org

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

> ---
> 
> Changelog v2:
> - Rearrange SoBs.
> - Discard BE copyright header.
> - Replace "additionalProperties: false" with "unevaluatedProperties: false"
>   property.
> - Discard interrupts property from the required properties list.
> - Remove a label definition from the binding example.
> - Move the asynchronous APB3 clock support into a dedicated patch.
> ---
>  .../devicetree/bindings/watchdog/dw_wdt.txt   | 24 ---------
>  .../bindings/watchdog/snps,dw-wdt.yaml        | 50 +++++++++++++++++++
>  2 files changed, 50 insertions(+), 24 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/watchdog/dw_wdt.txt
>  create mode 100644 Documentation/devicetree/bindings/watchdog/snps,dw-wdt.yaml
> 
> diff --git a/Documentation/devicetree/bindings/watchdog/dw_wdt.txt b/Documentation/devicetree/bindings/watchdog/dw_wdt.txt
> deleted file mode 100644
> index eb0914420c7c..000000000000
> --- a/Documentation/devicetree/bindings/watchdog/dw_wdt.txt
> +++ /dev/null
> @@ -1,24 +0,0 @@
> -Synopsys Designware Watchdog Timer
> -
> -Required Properties:
> -
> -- compatible	: Should contain "snps,dw-wdt"
> -- reg		: Base address and size of the watchdog timer registers.
> -- clocks	: phandle + clock-specifier for the clock that drives the
> -		watchdog timer.
> -
> -Optional Properties:
> -
> -- interrupts	: The interrupt used for the watchdog timeout warning.
> -- resets	: phandle pointing to the system reset controller with
> -		line index for the watchdog.
> -
> -Example:
> -
> -	watchdog0: wd@ffd02000 {
> -		compatible = "snps,dw-wdt";
> -		reg = <0xffd02000 0x1000>;
> -		interrupts = <0 171 4>;
> -		clocks = <&per_base_clk>;
> -		resets = <&rst WDT0_RESET>;
> -	};
> diff --git a/Documentation/devicetree/bindings/watchdog/snps,dw-wdt.yaml b/Documentation/devicetree/bindings/watchdog/snps,dw-wdt.yaml
> new file mode 100644
> index 000000000000..4f6944756ab4
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/watchdog/snps,dw-wdt.yaml
> @@ -0,0 +1,50 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/watchdog/snps,dw-wdt.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Synopsys Designware Watchdog Timer
> +
> +allOf:
> +  - $ref: "watchdog.yaml#"
> +
> +maintainers:
> +  - Jamie Iles <jamie@jamieiles.com>
> +
> +properties:
> +  compatible:
> +    const: snps,dw-wdt
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    description: DW Watchdog pre-timeout interrupt
> +    maxItems: 1
> +
> +  clocks:
> +    items:
> +      - description: Watchdog timer reference clock
> +
> +  resets:
> +    description: Phandle to the DW Watchdog reset lane
> +    maxItems: 1
> +
> +unevaluatedProperties: false
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +
> +examples:
> +  - |
> +    watchdog@ffd02000 {
> +      compatible = "snps,dw-wdt";
> +      reg = <0xffd02000 0x1000>;
> +      interrupts = <0 171 4>;
> +      clocks = <&per_base_clk>;
> +      resets = <&wdt_rst>;
> +    };
> +...

Re: [PATCH v3 2/7] dt-bindings: watchdog: dw-wdt: Support devices with asynch clocks

[Guenter Roeck]

On Tue, May 26, 2020 at 06:41:18PM +0300, Serge Semin wrote:
> DW Watchdog IP core can be synthesised with asynchronous timer/APB
> clocks support (WDT_ASYNC_CLK_MODE_ENABLE == 1). In this case
> separate clock signals are supposed to be used to feed watchdog timer
> and APB interface of the device. Let's update the DW Watchdog DT node
> schema so it would support the optional APB3 bus clock specified along
> with the mandatory watchdog timer reference clock.
> 
> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> Reviewed-by: Rob Herring <robh@kernel.org>
> Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
> Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: linux-mips@vger.kernel.org

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

> ---
> 
> Changelog v2:
> - It's a new patch unpinned from the previous one.
> ---
>  .../devicetree/bindings/watchdog/snps,dw-wdt.yaml         | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/watchdog/snps,dw-wdt.yaml b/Documentation/devicetree/bindings/watchdog/snps,dw-wdt.yaml
> index 4f6944756ab4..5bf6dc6377f3 100644
> --- a/Documentation/devicetree/bindings/watchdog/snps,dw-wdt.yaml
> +++ b/Documentation/devicetree/bindings/watchdog/snps,dw-wdt.yaml
> @@ -24,8 +24,16 @@ properties:
>      maxItems: 1
>  
>    clocks:
> +    minItems: 1
>      items:
>        - description: Watchdog timer reference clock
> +      - description: APB3 interface clock
> +
> +  clock-names:
> +    minItems: 1
> +    items:
> +      - const: tclk
> +      - const: pclk
>  
>    resets:
>      description: Phandle to the DW Watchdog reset lane

Re: [PATCH v3 3/7] dt-bindings: watchdog: dw-wdt: Add watchdog TOPs array property

[Guenter Roeck]

On Tue, May 26, 2020 at 06:41:19PM +0300, Serge Semin wrote:
> In case if DW Watchdog IP core is built with WDT_USE_FIX_TOP == false,
> a custom timeout periods are used to preset the timer counter. In
> this case that periods should be specified in a new "snps,watchdog-tops"
> property of the DW watchdog dts node.
> 
> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> Reviewed-by: Rob Herring <robh@kernel.org>
> Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
> Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: linux-mips@vger.kernel.org

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

> ---
> 
> Changelog v2:
> - Rearrange SoBs.
> - Move $ref to the root level of the "snps,watchdog-tops" property
>   so does the constraints.
> - Add default TOP values array.
> - Discard the label definition from the new bindings example.
> 
> Changelog v3:
> - Remove items property and move the minItems and maxItems constraints to
>   the root level of the snps,watchdog-tops property.
> ---
>  .../bindings/watchdog/snps,dw-wdt.yaml        | 32 +++++++++++++++++++
>  1 file changed, 32 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/watchdog/snps,dw-wdt.yaml b/Documentation/devicetree/bindings/watchdog/snps,dw-wdt.yaml
> index 5bf6dc6377f3..d9fc7bb851b1 100644
> --- a/Documentation/devicetree/bindings/watchdog/snps,dw-wdt.yaml
> +++ b/Documentation/devicetree/bindings/watchdog/snps,dw-wdt.yaml
> @@ -39,6 +39,23 @@ properties:
>      description: Phandle to the DW Watchdog reset lane
>      maxItems: 1
>  
> +  snps,watchdog-tops:
> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> +    description: |
> +      DW APB Watchdog custom timer intervals - Timeout Period ranges (TOPs).
> +      Each TOP is a number loaded into the watchdog counter at the moment of
> +      the timer restart. The counter decrementing happens each tick of the
> +      reference clock. Therefore the TOPs array is equivalent to an array of
> +      the timer expiration intervals supported by the DW APB Watchdog. Note
> +      DW APB Watchdog IP-core might be synthesized with fixed TOP values,
> +      in which case this property is unnecessary with default TOPs utilized.
> +    default: [0x0001000 0x0002000 0x0004000 0x0008000
> +      0x0010000 0x0020000 0x0040000 0x0080000
> +      0x0100000 0x0200000 0x0400000 0x0800000
> +      0x1000000 0x2000000 0x4000000 0x8000000]
> +    minItems: 16
> +    maxItems: 16
> +
>  unevaluatedProperties: false
>  
>  required:
> @@ -55,4 +72,19 @@ examples:
>        clocks = <&per_base_clk>;
>        resets = <&wdt_rst>;
>      };
> +
> +  - |
> +    watchdog@ffd02000 {
> +      compatible = "snps,dw-wdt";
> +      reg = <0xffd02000 0x1000>;
> +      interrupts = <0 171 4>;
> +      clocks = <&per_base_clk>;
> +      clock-names = "tclk";
> +      snps,watchdog-tops = <0x000000FF 0x000001FF 0x000003FF
> +                            0x000007FF 0x0000FFFF 0x0001FFFF
> +                            0x0003FFFF 0x0007FFFF 0x000FFFFF
> +                            0x001FFFFF 0x003FFFFF 0x007FFFFF
> +                            0x00FFFFFF 0x01FFFFFF 0x03FFFFFF
> +                            0x07FFFFFF>;
> +    };
>  ...

Re: [PATCH v3 6/7] watchdog: dw_wdt: Add pre-timeouts support

[Guenter Roeck]

On Tue, May 26, 2020 at 06:41:22PM +0300, Serge Semin wrote:
> DW Watchdog can rise an interrupt in case if IRQ request mode is enabled
> and timer reaches the zero value. In this case the IRQ lane is left
> pending until either the next watchdog kick event (watchdog restart) or
> until the WDT_EOI register is read or the device/system reset. This
> interface can be used to implement the pre-timeout functionality
> optionally provided by the Linux kernel watchdog devices.
> 
> IRQ mode provides a two stages timeout interface. It means the IRQ is
> raised when the counter reaches zero, while the system reset occurs only
> after subsequent timeout if the timer restart is not performed. Due to
> this peculiarity the pre-timeout value is actually set to the achieved
> hardware timeout, while the real watchdog timeout is considered to be
> twice as much of it. This applies a significant limitation on the
> pre-timeout values, so current implementation supports either zero value,
> which disables the pre-timeout events, or non-zero values, which imply
> the pre-timeout to be at least half of the current watchdog timeout.
> 
> Note that we ask the interrupt controller to detect the rising-edge
> pre-timeout interrupts to prevent the high-level-IRQs flood, since
> if the pre-timeout happens, the IRQ lane will be left pending until
> it's cleared by the timer restart.
> 
> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
> Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: linux-mips@vger.kernel.org
> Cc: devicetree@vger.kernel.org

Nitpick below, but I don't really know what to do about it, so

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

> ---
> 
> Changelog v2:
> - Rearrange SoBs.
> - Make the Pre-timeout IRQ being optionally supported.
> ---
>  drivers/watchdog/dw_wdt.c | 138 +++++++++++++++++++++++++++++++++++---
>  1 file changed, 130 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/watchdog/dw_wdt.c b/drivers/watchdog/dw_wdt.c
> index efbc36872670..3cd7c485cd70 100644
> --- a/drivers/watchdog/dw_wdt.c
> +++ b/drivers/watchdog/dw_wdt.c
> @@ -22,6 +22,7 @@
>  #include <linux/kernel.h>
>  #include <linux/module.h>
>  #include <linux/moduleparam.h>
> +#include <linux/interrupt.h>
>  #include <linux/of.h>
>  #include <linux/pm.h>
>  #include <linux/platform_device.h>
> @@ -36,6 +37,8 @@
>  #define WDOG_CURRENT_COUNT_REG_OFFSET	    0x08
>  #define WDOG_COUNTER_RESTART_REG_OFFSET     0x0c
>  #define WDOG_COUNTER_RESTART_KICK_VALUE	    0x76
> +#define WDOG_INTERRUPT_STATUS_REG_OFFSET    0x10
> +#define WDOG_INTERRUPT_CLEAR_REG_OFFSET     0x14
>  #define WDOG_COMP_PARAMS_1_REG_OFFSET       0xf4
>  #define WDOG_COMP_PARAMS_1_USE_FIX_TOP      BIT(6)
>  
> @@ -59,6 +62,11 @@ module_param(nowayout, bool, 0);
>  MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started "
>  		 "(default=" __MODULE_STRING(WATCHDOG_NOWAYOUT) ")");
>  
> +enum dw_wdt_rmod {
> +	DW_WDT_RMOD_RESET = 1,
> +	DW_WDT_RMOD_IRQ = 2
> +};
> +
>  struct dw_wdt_timeout {
>  	u32 top_val;
>  	unsigned int sec;
> @@ -70,6 +78,7 @@ struct dw_wdt {
>  	struct clk		*clk;
>  	struct clk		*pclk;
>  	unsigned long		rate;
> +	enum dw_wdt_rmod	rmod;
>  	struct dw_wdt_timeout	timeouts[DW_WDT_NUM_TOPS];
>  	struct watchdog_device	wdd;
>  	struct reset_control	*rst;
> @@ -86,6 +95,20 @@ static inline int dw_wdt_is_enabled(struct dw_wdt *dw_wdt)
>  		WDOG_CONTROL_REG_WDT_EN_MASK;
>  }
>  
> +static void dw_wdt_update_mode(struct dw_wdt *dw_wdt, enum dw_wdt_rmod rmod)
> +{
> +	u32 val;
> +
> +	val = readl(dw_wdt->regs + WDOG_CONTROL_REG_OFFSET);
> +	if (rmod == DW_WDT_RMOD_IRQ)
> +		val |= WDOG_CONTROL_REG_RESP_MODE_MASK;
> +	else
> +		val &= ~WDOG_CONTROL_REG_RESP_MODE_MASK;
> +	writel(val, dw_wdt->regs + WDOG_CONTROL_REG_OFFSET);
> +
> +	dw_wdt->rmod = rmod;
> +}
> +
>  static unsigned int dw_wdt_find_best_top(struct dw_wdt *dw_wdt,
>  					 unsigned int timeout, u32 *top_val)
>  {
> @@ -145,7 +168,11 @@ static unsigned int dw_wdt_get_timeout(struct dw_wdt *dw_wdt)
>  			break;
>  	}
>  
> -	return dw_wdt->timeouts[idx].sec;
> +	/*
> +	 * In IRQ mode due to the two stages counter, the actual timeout is
> +	 * twice greater than the TOP setting.
> +	 */
> +	return dw_wdt->timeouts[idx].sec * dw_wdt->rmod;
>  }
>  
>  static int dw_wdt_ping(struct watchdog_device *wdd)
> @@ -164,7 +191,20 @@ static int dw_wdt_set_timeout(struct watchdog_device *wdd, unsigned int top_s)
>  	unsigned int timeout;
>  	u32 top_val;
>  
> -	timeout = dw_wdt_find_best_top(dw_wdt, top_s, &top_val);
> +	/*
> +	 * Note IRQ mode being enabled means having a non-zero pre-timeout
> +	 * setup. In this case we try to find a TOP as close to the half of the
> +	 * requested timeout as possible since DW Watchdog IRQ mode is designed
> +	 * in two stages way - first timeout rises the pre-timeout interrupt,
> +	 * second timeout performs the system reset. So basically the effective
> +	 * watchdog-caused reset happens after two watchdog TOPs elapsed.
> +	 */
> +	timeout = dw_wdt_find_best_top(dw_wdt, DIV_ROUND_UP(top_s, dw_wdt->rmod),
> +				       &top_val);
> +	if (dw_wdt->rmod == DW_WDT_RMOD_IRQ)
> +		wdd->pretimeout = timeout;
> +	else
> +		wdd->pretimeout = 0;
>  
>  	/*
>  	 * Set the new value in the watchdog.  Some versions of dw_wdt
> @@ -175,25 +215,47 @@ static int dw_wdt_set_timeout(struct watchdog_device *wdd, unsigned int top_s)
>  	writel(top_val | top_val << WDOG_TIMEOUT_RANGE_TOPINIT_SHIFT,
>  	       dw_wdt->regs + WDOG_TIMEOUT_RANGE_REG_OFFSET);
>  
> +	/* Kick new TOP value into the watchdog counter if activated. */
> +	if (watchdog_active(wdd))
> +		dw_wdt_ping(wdd);
> +
>  	/*
>  	 * In case users set bigger timeout value than HW can support,
>  	 * kernel(watchdog_dev.c) helps to feed watchdog before
>  	 * wdd->max_hw_heartbeat_ms
>  	 */
>  	if (top_s * 1000 <= wdd->max_hw_heartbeat_ms)
> -		wdd->timeout = timeout;
> +		wdd->timeout = timeout * dw_wdt->rmod;
>  	else
>  		wdd->timeout = top_s;
>  
>  	return 0;
>  }
>  
> +static int dw_wdt_set_pretimeout(struct watchdog_device *wdd, unsigned int req)
> +{
> +	struct dw_wdt *dw_wdt = to_dw_wdt(wdd);
> +
> +	/*
> +	 * We ignore actual value of the timeout passed from user-space
> +	 * using it as a flag whether the pretimeout functionality is intended
> +	 * to be activated.
> +	 */
> +	dw_wdt_update_mode(dw_wdt, req ? DW_WDT_RMOD_IRQ : DW_WDT_RMOD_RESET);
> +	dw_wdt_set_timeout(wdd, wdd->timeout);
> +
> +	return 0;
> +}
> +
>  static void dw_wdt_arm_system_reset(struct dw_wdt *dw_wdt)
>  {
>  	u32 val = readl(dw_wdt->regs + WDOG_CONTROL_REG_OFFSET);
>  
> -	/* Disable interrupt mode; always perform system reset. */
> -	val &= ~WDOG_CONTROL_REG_RESP_MODE_MASK;
> +	/* Disable/enable interrupt mode depending on the RMOD flag. */
> +	if (dw_wdt->rmod == DW_WDT_RMOD_IRQ)
> +		val |= WDOG_CONTROL_REG_RESP_MODE_MASK;
> +	else
> +		val &= ~WDOG_CONTROL_REG_RESP_MODE_MASK;
>  	/* Enable watchdog. */
>  	val |= WDOG_CONTROL_REG_WDT_EN_MASK;
>  	writel(val, dw_wdt->regs + WDOG_CONTROL_REG_OFFSET);
> @@ -231,6 +293,7 @@ static int dw_wdt_restart(struct watchdog_device *wdd,
>  	struct dw_wdt *dw_wdt = to_dw_wdt(wdd);
>  
>  	writel(0, dw_wdt->regs + WDOG_TIMEOUT_RANGE_REG_OFFSET);
> +	dw_wdt_update_mode(dw_wdt, DW_WDT_RMOD_RESET);
>  	if (dw_wdt_is_enabled(dw_wdt))
>  		writel(WDOG_COUNTER_RESTART_KICK_VALUE,
>  		       dw_wdt->regs + WDOG_COUNTER_RESTART_REG_OFFSET);
> @@ -246,9 +309,19 @@ static int dw_wdt_restart(struct watchdog_device *wdd,
>  static unsigned int dw_wdt_get_timeleft(struct watchdog_device *wdd)
>  {
>  	struct dw_wdt *dw_wdt = to_dw_wdt(wdd);
> +	unsigned int sec;
> +	u32 val;
> +
> +	val = readl(dw_wdt->regs + WDOG_CURRENT_COUNT_REG_OFFSET);
> +	sec = val / dw_wdt->rate;
>  
> -	return readl(dw_wdt->regs + WDOG_CURRENT_COUNT_REG_OFFSET) /
> -		dw_wdt->rate;
> +	if (dw_wdt->rmod == DW_WDT_RMOD_IRQ) {
> +		val = readl(dw_wdt->regs + WDOG_INTERRUPT_STATUS_REG_OFFSET);
> +		if (!val)
> +			sec += wdd->pretimeout;
> +	}
> +
> +	return sec;
>  }
>  
>  static const struct watchdog_info dw_wdt_ident = {
> @@ -257,16 +330,41 @@ static const struct watchdog_info dw_wdt_ident = {
>  	.identity	= "Synopsys DesignWare Watchdog",
>  };
>  
> +static const struct watchdog_info dw_wdt_pt_ident = {
> +	.options	= WDIOF_KEEPALIVEPING | WDIOF_SETTIMEOUT |
> +			  WDIOF_PRETIMEOUT | WDIOF_MAGICCLOSE,
> +	.identity	= "Synopsys DesignWare Watchdog",
> +};
> +
>  static const struct watchdog_ops dw_wdt_ops = {
>  	.owner		= THIS_MODULE,
>  	.start		= dw_wdt_start,
>  	.stop		= dw_wdt_stop,
>  	.ping		= dw_wdt_ping,
>  	.set_timeout	= dw_wdt_set_timeout,
> +	.set_pretimeout	= dw_wdt_set_pretimeout,
>  	.get_timeleft	= dw_wdt_get_timeleft,
>  	.restart	= dw_wdt_restart,
>  };
>  
> +static irqreturn_t dw_wdt_irq(int irq, void *devid)
> +{
> +	struct dw_wdt *dw_wdt = devid;
> +	u32 val;
> +
> +	/*
> +	 * We don't clear the IRQ status. It's supposed to be done by the
> +	 * following ping operations.
> +	 */
> +	val = readl(dw_wdt->regs + WDOG_INTERRUPT_STATUS_REG_OFFSET);
> +	if (!val)
> +		return IRQ_NONE;
> +
> +	watchdog_notify_pretimeout(&dw_wdt->wdd);
> +
> +	return IRQ_HANDLED;
> +}
> +
>  #ifdef CONFIG_PM_SLEEP
>  static int dw_wdt_suspend(struct device *dev)
>  {
> @@ -447,6 +545,31 @@ static int dw_wdt_drv_probe(struct platform_device *pdev)
>  		goto out_disable_pclk;
>  	}
>  
> +	/* Enable normal reset without pre-timeout by default. */
> +	dw_wdt_update_mode(dw_wdt, DW_WDT_RMOD_RESET);
> +
> +	/*
> +	 * Pre-timeout IRQ is optional, since some hardware may lack support
> +	 * of it. Note we must request rising-edge IRQ, since the lane is left
> +	 * pending either until the next watchdog kick event or up to the
> +	 * system reset.
> +	 */
> +	ret = platform_get_irq_optional(pdev, 0);
> +	if (ret >= 0) {

I keep seeing notes that an interrupt value of 0 is invalid.

> +		ret = devm_request_irq(dev, ret, dw_wdt_irq,
> +				       IRQF_SHARED | IRQF_TRIGGER_RISING,
> +				       pdev->name, dw_wdt);
> +		if (ret)
> +			goto out_disable_pclk;
> +
> +		dw_wdt->wdd.info = &dw_wdt_pt_ident;
> +	} else {
> +		if (ret == -EPROBE_DEFER)
> +			goto out_disable_pclk;
> +
> +		dw_wdt->wdd.info = &dw_wdt_ident;
> +	}
> +
>  	reset_control_deassert(dw_wdt->rst);
>  
>  	ret = dw_wdt_init_timeouts(dw_wdt, dev);
> @@ -454,7 +577,6 @@ static int dw_wdt_drv_probe(struct platform_device *pdev)
>  		goto out_disable_clk;
>  
>  	wdd = &dw_wdt->wdd;
> -	wdd->info = &dw_wdt_ident;
>  	wdd->ops = &dw_wdt_ops;
>  	wdd->min_timeout = dw_wdt_get_min_timeout(dw_wdt);
>  	wdd->max_hw_heartbeat_ms = dw_wdt_get_max_timeout_ms(dw_wdt);

Re: [PATCH v3 7/7] watchdog: dw_wdt: Add DebugFS files

[Guenter Roeck]

On Tue, May 26, 2020 at 06:41:23PM +0300, Serge Semin wrote:
> For the sake of the easier device-driver debug procedure, we added a
> DebugFS file with the controller registers state. It's available only if
> kernel is configured with DebugFS support.
> 
> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
> Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: linux-mips@vger.kernel.org
> Cc: devicetree@vger.kernel.org

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

> ---
> 
> Changelog v2:
> - Rearrange SoBs.
> - Discard timeout/pretimeout/ping/enable DebugFS nodes. Registers state
>   dump node is only left.
> ---
>  drivers/watchdog/dw_wdt.c | 68 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 68 insertions(+)
> 
> diff --git a/drivers/watchdog/dw_wdt.c b/drivers/watchdog/dw_wdt.c
> index 3cd7c485cd70..012681baaa6d 100644
> --- a/drivers/watchdog/dw_wdt.c
> +++ b/drivers/watchdog/dw_wdt.c
> @@ -28,6 +28,7 @@
>  #include <linux/platform_device.h>
>  #include <linux/reset.h>
>  #include <linux/watchdog.h>
> +#include <linux/debugfs.h>
>  
>  #define WDOG_CONTROL_REG_OFFSET		    0x00
>  #define WDOG_CONTROL_REG_WDT_EN_MASK	    0x01
> @@ -39,8 +40,14 @@
>  #define WDOG_COUNTER_RESTART_KICK_VALUE	    0x76
>  #define WDOG_INTERRUPT_STATUS_REG_OFFSET    0x10
>  #define WDOG_INTERRUPT_CLEAR_REG_OFFSET     0x14
> +#define WDOG_COMP_PARAMS_5_REG_OFFSET       0xe4
> +#define WDOG_COMP_PARAMS_4_REG_OFFSET       0xe8
> +#define WDOG_COMP_PARAMS_3_REG_OFFSET       0xec
> +#define WDOG_COMP_PARAMS_2_REG_OFFSET       0xf0
>  #define WDOG_COMP_PARAMS_1_REG_OFFSET       0xf4
>  #define WDOG_COMP_PARAMS_1_USE_FIX_TOP      BIT(6)
> +#define WDOG_COMP_VERSION_REG_OFFSET        0xf8
> +#define WDOG_COMP_TYPE_REG_OFFSET           0xfc
>  
>  /* There are sixteen TOPs (timeout periods) that can be set in the watchdog. */
>  #define DW_WDT_NUM_TOPS		16
> @@ -85,6 +92,10 @@ struct dw_wdt {
>  	/* Save/restore */
>  	u32			control;
>  	u32			timeout;
> +
> +#ifdef CONFIG_DEBUG_FS
> +	struct dentry		*dbgfs_dir;
> +#endif
>  };
>  
>  #define to_dw_wdt(wdd)	container_of(wdd, struct dw_wdt, wdd)
> @@ -484,6 +495,59 @@ static int dw_wdt_init_timeouts(struct dw_wdt *dw_wdt, struct device *dev)
>  	return 0;
>  }
>  
> +#ifdef CONFIG_DEBUG_FS
> +
> +#define DW_WDT_DBGFS_REG(_name, _off) \
> +{				      \
> +	.name = _name,		      \
> +	.offset = _off		      \
> +}
> +
> +static const struct debugfs_reg32 dw_wdt_dbgfs_regs[] = {
> +	DW_WDT_DBGFS_REG("cr", WDOG_CONTROL_REG_OFFSET),
> +	DW_WDT_DBGFS_REG("torr", WDOG_TIMEOUT_RANGE_REG_OFFSET),
> +	DW_WDT_DBGFS_REG("ccvr", WDOG_CURRENT_COUNT_REG_OFFSET),
> +	DW_WDT_DBGFS_REG("crr", WDOG_COUNTER_RESTART_REG_OFFSET),
> +	DW_WDT_DBGFS_REG("stat", WDOG_INTERRUPT_STATUS_REG_OFFSET),
> +	DW_WDT_DBGFS_REG("param5", WDOG_COMP_PARAMS_5_REG_OFFSET),
> +	DW_WDT_DBGFS_REG("param4", WDOG_COMP_PARAMS_4_REG_OFFSET),
> +	DW_WDT_DBGFS_REG("param3", WDOG_COMP_PARAMS_3_REG_OFFSET),
> +	DW_WDT_DBGFS_REG("param2", WDOG_COMP_PARAMS_2_REG_OFFSET),
> +	DW_WDT_DBGFS_REG("param1", WDOG_COMP_PARAMS_1_REG_OFFSET),
> +	DW_WDT_DBGFS_REG("version", WDOG_COMP_VERSION_REG_OFFSET),
> +	DW_WDT_DBGFS_REG("type", WDOG_COMP_TYPE_REG_OFFSET)
> +};
> +
> +static void dw_wdt_dbgfs_init(struct dw_wdt *dw_wdt)
> +{
> +	struct device *dev = dw_wdt->wdd.parent;
> +	struct debugfs_regset32 *regset;
> +
> +	regset = devm_kzalloc(dev, sizeof(*regset), GFP_KERNEL);
> +	if (!regset)
> +		return;
> +
> +	regset->regs = dw_wdt_dbgfs_regs;
> +	regset->nregs = ARRAY_SIZE(dw_wdt_dbgfs_regs);
> +	regset->base = dw_wdt->regs;
> +
> +	dw_wdt->dbgfs_dir = debugfs_create_dir(dev_name(dev), NULL);
> +
> +	debugfs_create_regset32("registers", 0444, dw_wdt->dbgfs_dir, regset);
> +}
> +
> +static void dw_wdt_dbgfs_clear(struct dw_wdt *dw_wdt)
> +{
> +	debugfs_remove_recursive(dw_wdt->dbgfs_dir);
> +}
> +
> +#else /* !CONFIG_DEBUG_FS */
> +
> +static void dw_wdt_dbgfs_init(struct dw_wdt *dw_wdt) {}
> +static void dw_wdt_dbgfs_clear(struct dw_wdt *dw_wdt) {}
> +
> +#endif /* !CONFIG_DEBUG_FS */
> +
>  static int dw_wdt_drv_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
> @@ -607,6 +671,8 @@ static int dw_wdt_drv_probe(struct platform_device *pdev)
>  	if (ret)
>  		goto out_disable_pclk;
>  
> +	dw_wdt_dbgfs_init(dw_wdt);
> +
>  	return 0;
>  
>  out_disable_pclk:
> @@ -621,6 +687,8 @@ static int dw_wdt_drv_remove(struct platform_device *pdev)
>  {
>  	struct dw_wdt *dw_wdt = platform_get_drvdata(pdev);
>  
> +	dw_wdt_dbgfs_clear(dw_wdt);
> +
>  	watchdog_unregister_device(&dw_wdt->wdd);
>  	reset_control_assert(dw_wdt->rst);
>  	clk_disable_unprepare(dw_wdt->pclk);

Re: [PATCH v3 6/7] watchdog: dw_wdt: Add pre-timeouts support

[Serge Semin]

On Fri, May 29, 2020 at 04:02:19PM -0700, Guenter Roeck wrote:
> On Tue, May 26, 2020 at 06:41:22PM +0300, Serge Semin wrote:
> > DW Watchdog can rise an interrupt in case if IRQ request mode is enabled
> > and timer reaches the zero value. In this case the IRQ lane is left
> > pending until either the next watchdog kick event (watchdog restart) or
> > until the WDT_EOI register is read or the device/system reset. This
> > interface can be used to implement the pre-timeout functionality
> > optionally provided by the Linux kernel watchdog devices.
> > 
> > IRQ mode provides a two stages timeout interface. It means the IRQ is
> > raised when the counter reaches zero, while the system reset occurs only
> > after subsequent timeout if the timer restart is not performed. Due to
> > this peculiarity the pre-timeout value is actually set to the achieved
> > hardware timeout, while the real watchdog timeout is considered to be
> > twice as much of it. This applies a significant limitation on the
> > pre-timeout values, so current implementation supports either zero value,
> > which disables the pre-timeout events, or non-zero values, which imply
> > the pre-timeout to be at least half of the current watchdog timeout.
> > 
> > Note that we ask the interrupt controller to detect the rising-edge
> > pre-timeout interrupts to prevent the high-level-IRQs flood, since
> > if the pre-timeout happens, the IRQ lane will be left pending until
> > it's cleared by the timer restart.
> > 
> > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> > Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
> > Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> > Cc: Arnd Bergmann <arnd@arndb.de>
> > Cc: Rob Herring <robh+dt@kernel.org>
> > Cc: linux-mips@vger.kernel.org
> > Cc: devicetree@vger.kernel.org
> 

> Nitpick below, but I don't really know what to do about it, so
> 
> Reviewed-by: Guenter Roeck <linux@roeck-us.net>

Right. Semantically platform_get_irq_optional() should never return zero even
though the comment above that function definition states otherwise. I'll fix the
conditional statement to check for > 0 you've commented below and resend with
your tag attached. Thanks.

-Sergey

> 
> > ---
> > 
> > Changelog v2:
> > - Rearrange SoBs.
> > - Make the Pre-timeout IRQ being optionally supported.
> > ---
> >  drivers/watchdog/dw_wdt.c | 138 +++++++++++++++++++++++++++++++++++---
> >  1 file changed, 130 insertions(+), 8 deletions(-)
> > 
> > diff --git a/drivers/watchdog/dw_wdt.c b/drivers/watchdog/dw_wdt.c
> > index efbc36872670..3cd7c485cd70 100644
> > --- a/drivers/watchdog/dw_wdt.c
> > +++ b/drivers/watchdog/dw_wdt.c
> > @@ -22,6 +22,7 @@
> >  #include <linux/kernel.h>
> >  #include <linux/module.h>
> >  #include <linux/moduleparam.h>
> > +#include <linux/interrupt.h>
> >  #include <linux/of.h>
> >  #include <linux/pm.h>
> >  #include <linux/platform_device.h>
> > @@ -36,6 +37,8 @@
> >  #define WDOG_CURRENT_COUNT_REG_OFFSET	    0x08
> >  #define WDOG_COUNTER_RESTART_REG_OFFSET     0x0c
> >  #define WDOG_COUNTER_RESTART_KICK_VALUE	    0x76
> > +#define WDOG_INTERRUPT_STATUS_REG_OFFSET    0x10
> > +#define WDOG_INTERRUPT_CLEAR_REG_OFFSET     0x14
> >  #define WDOG_COMP_PARAMS_1_REG_OFFSET       0xf4
> >  #define WDOG_COMP_PARAMS_1_USE_FIX_TOP      BIT(6)
> >  
> > @@ -59,6 +62,11 @@ module_param(nowayout, bool, 0);
> >  MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started "
> >  		 "(default=" __MODULE_STRING(WATCHDOG_NOWAYOUT) ")");
> >  
> > +enum dw_wdt_rmod {
> > +	DW_WDT_RMOD_RESET = 1,
> > +	DW_WDT_RMOD_IRQ = 2
> > +};
> > +
> >  struct dw_wdt_timeout {
> >  	u32 top_val;
> >  	unsigned int sec;
> > @@ -70,6 +78,7 @@ struct dw_wdt {
> >  	struct clk		*clk;
> >  	struct clk		*pclk;
> >  	unsigned long		rate;
> > +	enum dw_wdt_rmod	rmod;
> >  	struct dw_wdt_timeout	timeouts[DW_WDT_NUM_TOPS];
> >  	struct watchdog_device	wdd;
> >  	struct reset_control	*rst;
> > @@ -86,6 +95,20 @@ static inline int dw_wdt_is_enabled(struct dw_wdt *dw_wdt)
> >  		WDOG_CONTROL_REG_WDT_EN_MASK;
> >  }
> >  
> > +static void dw_wdt_update_mode(struct dw_wdt *dw_wdt, enum dw_wdt_rmod rmod)
> > +{
> > +	u32 val;
> > +
> > +	val = readl(dw_wdt->regs + WDOG_CONTROL_REG_OFFSET);
> > +	if (rmod == DW_WDT_RMOD_IRQ)
> > +		val |= WDOG_CONTROL_REG_RESP_MODE_MASK;
> > +	else
> > +		val &= ~WDOG_CONTROL_REG_RESP_MODE_MASK;
> > +	writel(val, dw_wdt->regs + WDOG_CONTROL_REG_OFFSET);
> > +
> > +	dw_wdt->rmod = rmod;
> > +}
> > +
> >  static unsigned int dw_wdt_find_best_top(struct dw_wdt *dw_wdt,
> >  					 unsigned int timeout, u32 *top_val)
> >  {
> > @@ -145,7 +168,11 @@ static unsigned int dw_wdt_get_timeout(struct dw_wdt *dw_wdt)
> >  			break;
> >  	}
> >  
> > -	return dw_wdt->timeouts[idx].sec;
> > +	/*
> > +	 * In IRQ mode due to the two stages counter, the actual timeout is
> > +	 * twice greater than the TOP setting.
> > +	 */
> > +	return dw_wdt->timeouts[idx].sec * dw_wdt->rmod;
> >  }
> >  
> >  static int dw_wdt_ping(struct watchdog_device *wdd)
> > @@ -164,7 +191,20 @@ static int dw_wdt_set_timeout(struct watchdog_device *wdd, unsigned int top_s)
> >  	unsigned int timeout;
> >  	u32 top_val;
> >  
> > -	timeout = dw_wdt_find_best_top(dw_wdt, top_s, &top_val);
> > +	/*
> > +	 * Note IRQ mode being enabled means having a non-zero pre-timeout
> > +	 * setup. In this case we try to find a TOP as close to the half of the
> > +	 * requested timeout as possible since DW Watchdog IRQ mode is designed
> > +	 * in two stages way - first timeout rises the pre-timeout interrupt,
> > +	 * second timeout performs the system reset. So basically the effective
> > +	 * watchdog-caused reset happens after two watchdog TOPs elapsed.
> > +	 */
> > +	timeout = dw_wdt_find_best_top(dw_wdt, DIV_ROUND_UP(top_s, dw_wdt->rmod),
> > +				       &top_val);
> > +	if (dw_wdt->rmod == DW_WDT_RMOD_IRQ)
> > +		wdd->pretimeout = timeout;
> > +	else
> > +		wdd->pretimeout = 0;
> >  
> >  	/*
> >  	 * Set the new value in the watchdog.  Some versions of dw_wdt
> > @@ -175,25 +215,47 @@ static int dw_wdt_set_timeout(struct watchdog_device *wdd, unsigned int top_s)
> >  	writel(top_val | top_val << WDOG_TIMEOUT_RANGE_TOPINIT_SHIFT,
> >  	       dw_wdt->regs + WDOG_TIMEOUT_RANGE_REG_OFFSET);
> >  
> > +	/* Kick new TOP value into the watchdog counter if activated. */
> > +	if (watchdog_active(wdd))
> > +		dw_wdt_ping(wdd);
> > +
> >  	/*
> >  	 * In case users set bigger timeout value than HW can support,
> >  	 * kernel(watchdog_dev.c) helps to feed watchdog before
> >  	 * wdd->max_hw_heartbeat_ms
> >  	 */
> >  	if (top_s * 1000 <= wdd->max_hw_heartbeat_ms)
> > -		wdd->timeout = timeout;
> > +		wdd->timeout = timeout * dw_wdt->rmod;
> >  	else
> >  		wdd->timeout = top_s;
> >  
> >  	return 0;
> >  }
> >  
> > +static int dw_wdt_set_pretimeout(struct watchdog_device *wdd, unsigned int req)
> > +{
> > +	struct dw_wdt *dw_wdt = to_dw_wdt(wdd);
> > +
> > +	/*
> > +	 * We ignore actual value of the timeout passed from user-space
> > +	 * using it as a flag whether the pretimeout functionality is intended
> > +	 * to be activated.
> > +	 */
> > +	dw_wdt_update_mode(dw_wdt, req ? DW_WDT_RMOD_IRQ : DW_WDT_RMOD_RESET);
> > +	dw_wdt_set_timeout(wdd, wdd->timeout);
> > +
> > +	return 0;
> > +}
> > +
> >  static void dw_wdt_arm_system_reset(struct dw_wdt *dw_wdt)
> >  {
> >  	u32 val = readl(dw_wdt->regs + WDOG_CONTROL_REG_OFFSET);
> >  
> > -	/* Disable interrupt mode; always perform system reset. */
> > -	val &= ~WDOG_CONTROL_REG_RESP_MODE_MASK;
> > +	/* Disable/enable interrupt mode depending on the RMOD flag. */
> > +	if (dw_wdt->rmod == DW_WDT_RMOD_IRQ)
> > +		val |= WDOG_CONTROL_REG_RESP_MODE_MASK;
> > +	else
> > +		val &= ~WDOG_CONTROL_REG_RESP_MODE_MASK;
> >  	/* Enable watchdog. */
> >  	val |= WDOG_CONTROL_REG_WDT_EN_MASK;
> >  	writel(val, dw_wdt->regs + WDOG_CONTROL_REG_OFFSET);
> > @@ -231,6 +293,7 @@ static int dw_wdt_restart(struct watchdog_device *wdd,
> >  	struct dw_wdt *dw_wdt = to_dw_wdt(wdd);
> >  
> >  	writel(0, dw_wdt->regs + WDOG_TIMEOUT_RANGE_REG_OFFSET);
> > +	dw_wdt_update_mode(dw_wdt, DW_WDT_RMOD_RESET);
> >  	if (dw_wdt_is_enabled(dw_wdt))
> >  		writel(WDOG_COUNTER_RESTART_KICK_VALUE,
> >  		       dw_wdt->regs + WDOG_COUNTER_RESTART_REG_OFFSET);
> > @@ -246,9 +309,19 @@ static int dw_wdt_restart(struct watchdog_device *wdd,
> >  static unsigned int dw_wdt_get_timeleft(struct watchdog_device *wdd)
> >  {
> >  	struct dw_wdt *dw_wdt = to_dw_wdt(wdd);
> > +	unsigned int sec;
> > +	u32 val;
> > +
> > +	val = readl(dw_wdt->regs + WDOG_CURRENT_COUNT_REG_OFFSET);
> > +	sec = val / dw_wdt->rate;
> >  
> > -	return readl(dw_wdt->regs + WDOG_CURRENT_COUNT_REG_OFFSET) /
> > -		dw_wdt->rate;
> > +	if (dw_wdt->rmod == DW_WDT_RMOD_IRQ) {
> > +		val = readl(dw_wdt->regs + WDOG_INTERRUPT_STATUS_REG_OFFSET);
> > +		if (!val)
> > +			sec += wdd->pretimeout;
> > +	}
> > +
> > +	return sec;
> >  }
> >  
> >  static const struct watchdog_info dw_wdt_ident = {
> > @@ -257,16 +330,41 @@ static const struct watchdog_info dw_wdt_ident = {
> >  	.identity	= "Synopsys DesignWare Watchdog",
> >  };
> >  
> > +static const struct watchdog_info dw_wdt_pt_ident = {
> > +	.options	= WDIOF_KEEPALIVEPING | WDIOF_SETTIMEOUT |
> > +			  WDIOF_PRETIMEOUT | WDIOF_MAGICCLOSE,
> > +	.identity	= "Synopsys DesignWare Watchdog",
> > +};
> > +
> >  static const struct watchdog_ops dw_wdt_ops = {
> >  	.owner		= THIS_MODULE,
> >  	.start		= dw_wdt_start,
> >  	.stop		= dw_wdt_stop,
> >  	.ping		= dw_wdt_ping,
> >  	.set_timeout	= dw_wdt_set_timeout,
> > +	.set_pretimeout	= dw_wdt_set_pretimeout,
> >  	.get_timeleft	= dw_wdt_get_timeleft,
> >  	.restart	= dw_wdt_restart,
> >  };
> >  
> > +static irqreturn_t dw_wdt_irq(int irq, void *devid)
> > +{
> > +	struct dw_wdt *dw_wdt = devid;
> > +	u32 val;
> > +
> > +	/*
> > +	 * We don't clear the IRQ status. It's supposed to be done by the
> > +	 * following ping operations.
> > +	 */
> > +	val = readl(dw_wdt->regs + WDOG_INTERRUPT_STATUS_REG_OFFSET);
> > +	if (!val)
> > +		return IRQ_NONE;
> > +
> > +	watchdog_notify_pretimeout(&dw_wdt->wdd);
> > +
> > +	return IRQ_HANDLED;
> > +}
> > +
> >  #ifdef CONFIG_PM_SLEEP
> >  static int dw_wdt_suspend(struct device *dev)
> >  {
> > @@ -447,6 +545,31 @@ static int dw_wdt_drv_probe(struct platform_device *pdev)
> >  		goto out_disable_pclk;
> >  	}
> >  
> > +	/* Enable normal reset without pre-timeout by default. */
> > +	dw_wdt_update_mode(dw_wdt, DW_WDT_RMOD_RESET);
> > +
> > +	/*
> > +	 * Pre-timeout IRQ is optional, since some hardware may lack support
> > +	 * of it. Note we must request rising-edge IRQ, since the lane is left
> > +	 * pending either until the next watchdog kick event or up to the
> > +	 * system reset.
> > +	 */
> > +	ret = platform_get_irq_optional(pdev, 0);
> > +	if (ret >= 0) {
> 
> I keep seeing notes that an interrupt value of 0 is invalid.
> 
> > +		ret = devm_request_irq(dev, ret, dw_wdt_irq,
> > +				       IRQF_SHARED | IRQF_TRIGGER_RISING,
> > +				       pdev->name, dw_wdt);
> > +		if (ret)
> > +			goto out_disable_pclk;
> > +
> > +		dw_wdt->wdd.info = &dw_wdt_pt_ident;
> > +	} else {
> > +		if (ret == -EPROBE_DEFER)
> > +			goto out_disable_pclk;
> > +
> > +		dw_wdt->wdd.info = &dw_wdt_ident;
> > +	}
> > +
> >  	reset_control_deassert(dw_wdt->rst);
> >  
> >  	ret = dw_wdt_init_timeouts(dw_wdt, dev);
> > @@ -454,7 +577,6 @@ static int dw_wdt_drv_probe(struct platform_device *pdev)
> >  		goto out_disable_clk;
> >  
> >  	wdd = &dw_wdt->wdd;
> > -	wdd->info = &dw_wdt_ident;
> >  	wdd->ops = &dw_wdt_ops;
> >  	wdd->min_timeout = dw_wdt_get_min_timeout(dw_wdt);
> >  	wdd->max_hw_heartbeat_ms = dw_wdt_get_max_timeout_ms(dw_wdt);

Re: [PATCH v3 4/7] watchdog: dw_wdt: Support devices with non-fixed TOP values

[Dan Carpenter]

[-- Attachment #1: Type: text/plain, Size: 4024 bytes --]

Hi Serge,

url:    https://github.com/0day-ci/linux/commits/Serge-Semin/watchdog-dw_wdt-Take-Baikal-T1-DW-WDT-peculiarities-into-account/20200526-234657
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
config: i386-randconfig-m021-20200528 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-13) 9.3.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>

smatch warnings:
drivers/watchdog/dw_wdt.c:124 dw_wdt_get_min_timeout() error: buffer overflow 'dw_wdt->timeouts' 16 <= 16 (assuming for loop doesn't break)
drivers/watchdog/dw_wdt.c:147 dw_wdt_get_timeout() error: buffer overflow 'dw_wdt->timeouts' 16 <= 16 (assuming for loop doesn't break)

# https://github.com/0day-ci/linux/commit/2d6c220c0b6cbc1fd04065b904a6a2f0bfe086e7
git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout 2d6c220c0b6cbc1fd04065b904a6a2f0bfe086e7
vim +124 drivers/watchdog/dw_wdt.c

2d6c220c0b6cbc Serge Semin 2020-05-26  111  static unsigned int dw_wdt_get_min_timeout(struct dw_wdt *dw_wdt)
c9353ae1c69ba1 Jamie Iles  2011-01-24  112  {
2d6c220c0b6cbc Serge Semin 2020-05-26  113  	int idx;
2d6c220c0b6cbc Serge Semin 2020-05-26  114  
2d6c220c0b6cbc Serge Semin 2020-05-26  115  	/*
2d6c220c0b6cbc Serge Semin 2020-05-26  116  	 * We'll find a timeout greater or equal to one second anyway because
2d6c220c0b6cbc Serge Semin 2020-05-26  117  	 * the driver probe would have failed if there was none.
2d6c220c0b6cbc Serge Semin 2020-05-26  118  	 */
2d6c220c0b6cbc Serge Semin 2020-05-26  119  	for (idx = 0; idx < DW_WDT_NUM_TOPS; ++idx) {
2d6c220c0b6cbc Serge Semin 2020-05-26  120  		if (dw_wdt->timeouts[idx].sec)
2d6c220c0b6cbc Serge Semin 2020-05-26  121  			break;
2d6c220c0b6cbc Serge Semin 2020-05-26  122  	}
c9353ae1c69ba1 Jamie Iles  2011-01-24  123  
2d6c220c0b6cbc Serge Semin 2020-05-26 @124  	return dw_wdt->timeouts[idx].sec;
                                                       ^^^^^^^^^^^^^^^^^^^^^
Read overflow if we don't break out of the loop.

2d6c220c0b6cbc Serge Semin 2020-05-26  125  }
2d6c220c0b6cbc Serge Semin 2020-05-26  126  
2d6c220c0b6cbc Serge Semin 2020-05-26  127  static unsigned int dw_wdt_get_max_timeout_ms(struct dw_wdt *dw_wdt)
2d6c220c0b6cbc Serge Semin 2020-05-26  128  {
2d6c220c0b6cbc Serge Semin 2020-05-26  129  	struct dw_wdt_timeout *timeout = &dw_wdt->timeouts[DW_WDT_NUM_TOPS - 1];
2d6c220c0b6cbc Serge Semin 2020-05-26  130  	u64 msec;
2d6c220c0b6cbc Serge Semin 2020-05-26  131  
2d6c220c0b6cbc Serge Semin 2020-05-26  132  	msec = (u64)timeout->sec * MSEC_PER_SEC + timeout->msec;
2d6c220c0b6cbc Serge Semin 2020-05-26  133  
2d6c220c0b6cbc Serge Semin 2020-05-26  134  	return msec < UINT_MAX ? msec : UINT_MAX;
2d6c220c0b6cbc Serge Semin 2020-05-26  135  }
2d6c220c0b6cbc Serge Semin 2020-05-26  136  
2d6c220c0b6cbc Serge Semin 2020-05-26  137  static unsigned int dw_wdt_get_timeout(struct dw_wdt *dw_wdt)
2d6c220c0b6cbc Serge Semin 2020-05-26  138  {
2d6c220c0b6cbc Serge Semin 2020-05-26  139  	int top_val = readl(dw_wdt->regs + WDOG_TIMEOUT_RANGE_REG_OFFSET) & 0xF;
2d6c220c0b6cbc Serge Semin 2020-05-26  140  	int idx;
2d6c220c0b6cbc Serge Semin 2020-05-26  141  
2d6c220c0b6cbc Serge Semin 2020-05-26  142  	for (idx = 0; idx < DW_WDT_NUM_TOPS; ++idx) {
2d6c220c0b6cbc Serge Semin 2020-05-26  143  		if (dw_wdt->timeouts[idx].top_val == top_val)
2d6c220c0b6cbc Serge Semin 2020-05-26  144  			break;
2d6c220c0b6cbc Serge Semin 2020-05-26  145  	}
2d6c220c0b6cbc Serge Semin 2020-05-26  146  
2d6c220c0b6cbc Serge Semin 2020-05-26 @147  	return dw_wdt->timeouts[idx].sec;
                                                       ^^^^^^^^^^^^^^^^^^^^^

c9353ae1c69ba1 Jamie Iles  2011-01-24  148  }

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 36057 bytes --]

Re: [PATCH v3 4/7] watchdog: dw_wdt: Support devices with non-fixed TOP values

[Dan Carpenter]

[-- Attachment #1: Type: text/plain, Size: 4024 bytes --]

Hi Serge,

url:    https://github.com/0day-ci/linux/commits/Serge-Semin/watchdog-dw_wdt-Take-Baikal-T1-DW-WDT-peculiarities-into-account/20200526-234657
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
config: i386-randconfig-m021-20200528 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-13) 9.3.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>

smatch warnings:
drivers/watchdog/dw_wdt.c:124 dw_wdt_get_min_timeout() error: buffer overflow 'dw_wdt->timeouts' 16 <= 16 (assuming for loop doesn't break)
drivers/watchdog/dw_wdt.c:147 dw_wdt_get_timeout() error: buffer overflow 'dw_wdt->timeouts' 16 <= 16 (assuming for loop doesn't break)

# https://github.com/0day-ci/linux/commit/2d6c220c0b6cbc1fd04065b904a6a2f0bfe086e7
git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout 2d6c220c0b6cbc1fd04065b904a6a2f0bfe086e7
vim +124 drivers/watchdog/dw_wdt.c

2d6c220c0b6cbc Serge Semin 2020-05-26  111  static unsigned int dw_wdt_get_min_timeout(struct dw_wdt *dw_wdt)
c9353ae1c69ba1 Jamie Iles  2011-01-24  112  {
2d6c220c0b6cbc Serge Semin 2020-05-26  113  	int idx;
2d6c220c0b6cbc Serge Semin 2020-05-26  114  
2d6c220c0b6cbc Serge Semin 2020-05-26  115  	/*
2d6c220c0b6cbc Serge Semin 2020-05-26  116  	 * We'll find a timeout greater or equal to one second anyway because
2d6c220c0b6cbc Serge Semin 2020-05-26  117  	 * the driver probe would have failed if there was none.
2d6c220c0b6cbc Serge Semin 2020-05-26  118  	 */
2d6c220c0b6cbc Serge Semin 2020-05-26  119  	for (idx = 0; idx < DW_WDT_NUM_TOPS; ++idx) {
2d6c220c0b6cbc Serge Semin 2020-05-26  120  		if (dw_wdt->timeouts[idx].sec)
2d6c220c0b6cbc Serge Semin 2020-05-26  121  			break;
2d6c220c0b6cbc Serge Semin 2020-05-26  122  	}
c9353ae1c69ba1 Jamie Iles  2011-01-24  123  
2d6c220c0b6cbc Serge Semin 2020-05-26 @124  	return dw_wdt->timeouts[idx].sec;
                                                       ^^^^^^^^^^^^^^^^^^^^^
Read overflow if we don't break out of the loop.

2d6c220c0b6cbc Serge Semin 2020-05-26  125  }
2d6c220c0b6cbc Serge Semin 2020-05-26  126  
2d6c220c0b6cbc Serge Semin 2020-05-26  127  static unsigned int dw_wdt_get_max_timeout_ms(struct dw_wdt *dw_wdt)
2d6c220c0b6cbc Serge Semin 2020-05-26  128  {
2d6c220c0b6cbc Serge Semin 2020-05-26  129  	struct dw_wdt_timeout *timeout = &dw_wdt->timeouts[DW_WDT_NUM_TOPS - 1];
2d6c220c0b6cbc Serge Semin 2020-05-26  130  	u64 msec;
2d6c220c0b6cbc Serge Semin 2020-05-26  131  
2d6c220c0b6cbc Serge Semin 2020-05-26  132  	msec = (u64)timeout->sec * MSEC_PER_SEC + timeout->msec;
2d6c220c0b6cbc Serge Semin 2020-05-26  133  
2d6c220c0b6cbc Serge Semin 2020-05-26  134  	return msec < UINT_MAX ? msec : UINT_MAX;
2d6c220c0b6cbc Serge Semin 2020-05-26  135  }
2d6c220c0b6cbc Serge Semin 2020-05-26  136  
2d6c220c0b6cbc Serge Semin 2020-05-26  137  static unsigned int dw_wdt_get_timeout(struct dw_wdt *dw_wdt)
2d6c220c0b6cbc Serge Semin 2020-05-26  138  {
2d6c220c0b6cbc Serge Semin 2020-05-26  139  	int top_val = readl(dw_wdt->regs + WDOG_TIMEOUT_RANGE_REG_OFFSET) & 0xF;
2d6c220c0b6cbc Serge Semin 2020-05-26  140  	int idx;
2d6c220c0b6cbc Serge Semin 2020-05-26  141  
2d6c220c0b6cbc Serge Semin 2020-05-26  142  	for (idx = 0; idx < DW_WDT_NUM_TOPS; ++idx) {
2d6c220c0b6cbc Serge Semin 2020-05-26  143  		if (dw_wdt->timeouts[idx].top_val == top_val)
2d6c220c0b6cbc Serge Semin 2020-05-26  144  			break;
2d6c220c0b6cbc Serge Semin 2020-05-26  145  	}
2d6c220c0b6cbc Serge Semin 2020-05-26  146  
2d6c220c0b6cbc Serge Semin 2020-05-26 @147  	return dw_wdt->timeouts[idx].sec;
                                                       ^^^^^^^^^^^^^^^^^^^^^

c9353ae1c69ba1 Jamie Iles  2011-01-24  148  }

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 36057 bytes --]