[PATCH v3 0/3] watchdog: npcm: Add reset status detection support

[PATCH v3 0/3] watchdog: npcm: Add reset status detection support

[Tomer Maimon]

This series documents and implements reset indication reporting for the
NPCM watchdog driver on NPCM7xx and NPCM8xx systems, and documents the
optional GCR syscon property used by that support.

Patch 1 updates the watchdog binding to allow the optional
``nuvoton,sysgcr`` property used for reset-cause reporting.
Patch 2 adds watchdog documentation that describes the bootstatus
mapping.
Patch 3 reads the SoC reset indication bits and maps them into the
existing watchdog bootstatus flags for NPCM750 and NPCM845, while
leaving WPCM450 unchanged.

Addressed comments from:
- Krzysztof Kozlowski: https://patchwork.ozlabs.org/project/openbmc/patch/20260622083046.3189603-2-tmaimon77@gmail.com/

Changes since version 2:
- reword patch 1 to explain the hardware reason for the GCR phandle and
  why it is needed for the reset-status support, per Krzysztof's review
- cache NPCM750 reset status in SCRPAD2 so later watchdog probes can
  report the same boot-time state

Changes since version 1:
- Modify reset detection handle in the watchodg.
- reword patch 1 to use the GCR syscon-property wording from the
  applied NPCM reset binding update and drop the optional property from
  the binding example.
- reword the patch subjects and commit message bodies to match current
  kernel dt-bindings, docs, and watchdog style.

Tomer Maimon (3):
  dt-bindings: watchdog: npcm: add GCR syscon property
  docs: watchdog: npcm: Add reset status description
  watchdog: npcm: add bootstatus support

 .../watchdog/nuvoton,npcm750-wdt.yaml         |   6 +
 Documentation/watchdog/index.rst              |   1 +
 Documentation/watchdog/npcm_wdt.rst           |  71 ++++++
 drivers/watchdog/npcm_wdt.c                   | 217 +++++++++++++++++-
 4 files changed, 293 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/watchdog/npcm_wdt.rst

-- 
2.34.1

[PATCH v3 1/3] dt-bindings: watchdog: npcm: add GCR syscon property

[Tomer Maimon]

NPCM750 and NPCM845 latch watchdog reset indications in the SoC
GCR block rather than in the watchdog block itself.

Add the optional nuvoton,sysgcr phandle so watchdog nodes can
reference the shared GCR reset-status registers that hold those
latched watchdog reset indications.

This is needed by the following reset-status support, which reads
those latches and reports watchdog-caused resets through bootstatus.

Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>
---
 .../devicetree/bindings/watchdog/nuvoton,npcm750-wdt.yaml   | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/watchdog/nuvoton,npcm750-wdt.yaml b/Documentation/devicetree/bindings/watchdog/nuvoton,npcm750-wdt.yaml
index 7aa30f5b5c49..99c225f166db 100644
--- a/Documentation/devicetree/bindings/watchdog/nuvoton,npcm750-wdt.yaml
+++ b/Documentation/devicetree/bindings/watchdog/nuvoton,npcm750-wdt.yaml
@@ -40,6 +40,12 @@ properties:
   clock-frequency:
     description: Frequency in Hz of the clock that drives the NPCM timer.
 
+  nuvoton,sysgcr:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description:
+      a phandle to access the GCR reset-status registers that latch
+      watchdog reset indications on NPCM750 and NPCM845.
+
 required:
   - compatible
   - reg
-- 
2.34.1

[PATCH v3 2/3] docs: watchdog: npcm: Add reset status description

[Tomer Maimon]

Add documentation describing how the NPCM watchdog driver reports reset
causes through bootstatus on NPCM750 and NPCM845 systems.

Document the reset flag mapping, the watchdog instance mapping for
WDIOF_CARDRESET, and the NPCM750/NPCM845 latch handling. Also mention
sysfs bootstatus reporting when watchdog sysfs support is enabled.

Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>
---
 Documentation/watchdog/index.rst    |  1 +
 Documentation/watchdog/npcm_wdt.rst | 71 +++++++++++++++++++++++++++++
 2 files changed, 72 insertions(+)
 create mode 100644 Documentation/watchdog/npcm_wdt.rst

diff --git a/Documentation/watchdog/index.rst b/Documentation/watchdog/index.rst
index dbc702b31a43..eb9e76275fa0 100644
--- a/Documentation/watchdog/index.rst
+++ b/Documentation/watchdog/index.rst
@@ -25,4 +25,5 @@ Driver specific
     hpwdt
     ics-wdt
     mlx-wdt
+    npcm_wdt
     pcwd-watchdog
diff --git a/Documentation/watchdog/npcm_wdt.rst b/Documentation/watchdog/npcm_wdt.rst
new file mode 100644
index 000000000000..cdc45ade9e0d
--- /dev/null
+++ b/Documentation/watchdog/npcm_wdt.rst
@@ -0,0 +1,71 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+=============
+NPCM Watchdog
+=============
+
+The NPCM watchdog driver can report reset-cause information on
+``nuvoton,npcm750-wdt`` and ``nuvoton,npcm845-wdt`` systems.
+
+Userspace can read the latched reset cause through
+``WDIOC_GETBOOTSTATUS``. When ``CONFIG_WATCHDOG_SYSFS`` is enabled, the
+same value is also visible through ``/sys/class/watchdog/watchdogN/bootstatus``.
+
+The mapping is fixed in the driver. It exposes the SoC reset indications
+through the generic watchdog bootstatus flags and is not configurable from
+Device Tree.
+
+.. list-table:: Reset-cause mapping
+   :header-rows: 1
+
+   * - Platform
+     - Reset indication
+     - Bootstatus flag
+     - Reported meaning
+   * - NPCM750 and NPCM845
+     - ``PORST``
+     - ``WDIOF_OVERHEAT``
+     - power-on reset
+   * - NPCM750 and NPCM845
+     - ``CORST``
+     - ``WDIOF_FANFAULT``
+     - core reset
+   * - NPCM750 and NPCM845
+     - ``SWR1RST``
+     - ``WDIOF_EXTERN1``
+     - software reset source 1
+   * - NPCM750 and NPCM845
+     - ``SWR2RST``
+     - ``WDIOF_EXTERN2``
+     - software reset source 2
+   * - NPCM750 and NPCM845
+     - ``SWR3RST``
+     - ``WDIOF_POWERUNDER``
+     - software reset source 3
+   * - NPCM750
+     - ``SWR4RST``
+     - ``WDIOF_POWEROVER``
+     - software reset source 4
+   * - NPCM845
+     - ``TIP reset`` (``INTCR2[25]``)
+     - ``WDIOF_POWEROVER``
+     - TIP reset
+
+``WDIOF_CARDRESET`` is reported only for the watchdog instance whose own
+reset-status bit is latched. On systems with three watchdog instances, this
+maps ``WD0RST``, ``WD1RST``, and ``WD2RST`` to ``watchdog0``, ``watchdog1``,
+and ``watchdog2`` respectively.
+
+The driver may report ``WDIOF_CARDRESET`` together with one or more of the
+reset-cause flags listed above.
+
+On NPCM750, the driver samples ``RESSR``. When reset bits are still latched,
+it clears them and stores the sampled value in ``SCRPAD2`` so later watchdog
+probes can report the same boot-time state.
+
+On NPCM845, the driver samples ``INTCR2``. When reset bits are still latched,
+it clears them and stores the sampled value in ``SCRPAD10`` so later watchdog
+probes can report the same boot-time state.
+
+The WPCM450 watchdog continues to operate without this reset-indication
+mapping.
-- 
2.34.1

[PATCH v3 3/3] watchdog: npcm: add bootstatus support

[Tomer Maimon]

The NPCM750 uses RESSR and the NPCM845 uses INTCR2 to latch reset
indications. Read those bits during probe and map them into watchdog
bootstatus flags.

For NPCM845, cache the sampled INTCR2 state in SCRPAD10 after the reset
status bits are cleared so later probes can report the same boot-time
state. Also report WDIOF_CARDRESET for the watchdog instance whose reset
bit is latched, while leaving WPCM450 behavior unchanged.

Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>
---
 drivers/watchdog/npcm_wdt.c | 217 +++++++++++++++++++++++++++++++++++-
 1 file changed, 215 insertions(+), 2 deletions(-)

diff --git a/drivers/watchdog/npcm_wdt.c b/drivers/watchdog/npcm_wdt.c
index e62ea054bc61..cecfd6e574d8 100644
--- a/drivers/watchdog/npcm_wdt.c
+++ b/drivers/watchdog/npcm_wdt.c
@@ -7,14 +7,52 @@
 #include <linux/delay.h>
 #include <linux/interrupt.h>
 #include <linux/kernel.h>
+#include <linux/mfd/syscon.h>
 #include <linux/module.h>
 #include <linux/of_irq.h>
 #include <linux/platform_device.h>
+#include <linux/regmap.h>
 #include <linux/slab.h>
 #include <linux/watchdog.h>
 
 #define NPCM_WTCR	0x1C
 
+/* NPCM GCR module */
+#define NPCM_RESSR_OFFSET		0x6C
+#define NPCM_INTCR2_OFFSET		0x60
+#define NPCM7XX_SCRPAD2_OFFSET		0x84
+#define NPCM8XX_SCRPAD10_OFFSET		0xE28
+
+#define NPCM_PORST			BIT(31)
+#define NPCM_CORST			BIT(30)
+#define NPCM_WD0RST			BIT(29)
+#define NPCM_SWR1RST			BIT(28)
+#define NPCM_SWR2RST			BIT(27)
+#define NPCM_SWR3RST			BIT(26)
+#define NPCM_SWR4RST			BIT(25)
+#define NPCM_WD1RST			BIT(24)
+#define NPCM_WD2RST			BIT(23)
+#define NPCM_RST			GENMASK(31, 23)
+#define NPCM8XX_TIP_RESET		BIT(25) /* Replaces SWRST4 on NPCM8xx */
+
+/* Per-instance mapping of MMIO base address to its RESSR/INTCR2 reset bit. */
+struct npcm_wdt_rst_map {
+	phys_addr_t	base;
+	u32		rst_bit;
+};
+
+struct npcm_wdt_status_map {
+	u32	rst_bit;
+	u32	wdiof_flag;
+};
+
+struct npcm_wdt_data {
+	const struct npcm_wdt_rst_map		*rst_map;
+	unsigned int				rst_map_size;
+	const struct npcm_wdt_status_map	*status_map;
+	unsigned int				status_map_size;
+};
+
 #define NPCM_WTCLK	(BIT(10) | BIT(11))	/* Clock divider */
 #define NPCM_WTE	BIT(7)			/* Enable */
 #define NPCM_WTIE	BIT(6)			/* Enable irq */
@@ -47,6 +85,50 @@ struct npcm_wdt {
 	struct clk		*clk;
 };
 
+static const struct npcm_wdt_rst_map npcm750_rst_map[] = {
+	{ 0xf000801c, NPCM_WD0RST },
+	{ 0xf000901c, NPCM_WD1RST },
+	{ 0xf000a01c, NPCM_WD2RST },
+};
+
+static const struct npcm_wdt_status_map npcm750_status_map[] = {
+	{ NPCM_PORST, WDIOF_OVERHEAT },
+	{ NPCM_CORST, WDIOF_FANFAULT },
+	{ NPCM_SWR1RST, WDIOF_EXTERN1 },
+	{ NPCM_SWR2RST, WDIOF_EXTERN2 },
+	{ NPCM_SWR3RST, WDIOF_POWERUNDER },
+	{ NPCM_SWR4RST, WDIOF_POWEROVER },
+};
+
+static const struct npcm_wdt_data npcm750_data = {
+	.rst_map = npcm750_rst_map,
+	.rst_map_size = ARRAY_SIZE(npcm750_rst_map),
+	.status_map = npcm750_status_map,
+	.status_map_size = ARRAY_SIZE(npcm750_status_map),
+};
+
+static const struct npcm_wdt_rst_map npcm845_rst_map[] = {
+	{ 0xf000801c, NPCM_WD0RST },
+	{ 0xf000901c, NPCM_WD1RST },
+	{ 0xf000a01c, NPCM_WD2RST },
+};
+
+static const struct npcm_wdt_status_map npcm845_status_map[] = {
+	{ NPCM_PORST, WDIOF_OVERHEAT },
+	{ NPCM_CORST, WDIOF_FANFAULT },
+	{ NPCM_SWR1RST, WDIOF_EXTERN1 },
+	{ NPCM_SWR2RST, WDIOF_EXTERN2 },
+	{ NPCM_SWR3RST, WDIOF_POWERUNDER },
+	{ NPCM8XX_TIP_RESET, WDIOF_POWEROVER },
+};
+
+static const struct npcm_wdt_data npcm845_data = {
+	.rst_map = npcm845_rst_map,
+	.rst_map_size = ARRAY_SIZE(npcm845_rst_map),
+	.status_map = npcm845_status_map,
+	.status_map_size = ARRAY_SIZE(npcm845_status_map),
+};
+
 static inline struct npcm_wdt *to_npcm_wdt(struct watchdog_device *wdd)
 {
 	return container_of(wdd, struct npcm_wdt, wdd);
@@ -169,6 +251,111 @@ static bool npcm_is_running(struct watchdog_device *wdd)
 	return readl(wdt->reg) & NPCM_WTE;
 }
 
+static void npcm_get_reset_status(struct npcm_wdt *wdt, struct device *dev,
+				  const struct npcm_wdt_data *data,
+				  resource_size_t start)
+{
+	struct regmap *gcr_regmap;
+	u32 rstval = 0;
+	unsigned int i;
+	int ret;
+
+	if (!data)
+		return;
+
+	gcr_regmap = syscon_regmap_lookup_by_phandle(dev->of_node,
+						     "nuvoton,sysgcr");
+	if (IS_ERR(gcr_regmap)) {
+		dev_warn(dev,
+			 "Failed to find nuvoton,sysgcr, WD reset status not supported\n");
+		return;
+	}
+
+	if (of_device_is_compatible(dev->of_node, "nuvoton,npcm845-wdt")) {
+		ret = regmap_read(gcr_regmap, NPCM_INTCR2_OFFSET, &rstval);
+		if (ret) {
+			dev_warn(dev, "Failed to read INTCR2 reset status: %d\n",
+				 ret);
+			return;
+		}
+
+		if (rstval & NPCM_RST) {
+			ret = regmap_write(gcr_regmap, NPCM_INTCR2_OFFSET,
+					   rstval & ~NPCM_RST);
+			if (ret) {
+				dev_warn(dev,
+					 "Failed to clear INTCR2 reset status: %d\n",
+					 ret);
+				return;
+			}
+
+			ret = regmap_write(gcr_regmap, NPCM8XX_SCRPAD10_OFFSET,
+					   rstval);
+			if (ret) {
+				dev_warn(dev,
+					 "Failed to cache reset status in SCRPAD10: %d\n",
+					 ret);
+				return;
+			}
+		} else {
+			ret = regmap_read(gcr_regmap, NPCM8XX_SCRPAD10_OFFSET,
+					  &rstval);
+			if (ret) {
+				dev_warn(dev,
+					 "Failed to read cached reset status from SCRPAD10: %d\n",
+					 ret);
+				return;
+			}
+		}
+	} else if (of_device_is_compatible(dev->of_node, "nuvoton,npcm750-wdt")) {
+		ret = regmap_read(gcr_regmap, NPCM_RESSR_OFFSET, &rstval);
+		if (ret) {
+			dev_warn(dev, "Failed to read RESSR reset status: %d\n",
+				 ret);
+			return;
+		}
+
+		if (rstval & NPCM_RST) {
+			ret = regmap_write(gcr_regmap, NPCM_RESSR_OFFSET, 
+					   rstval & ~NPCM_RST);
+			if (ret) {
+				dev_warn(dev, "Failed to clear RESSR reset status: %d\n", ret);
+				return;
+			}
+
+			ret = regmap_write(gcr_regmap, NPCM7XX_SCRPAD2_OFFSET,
+					   rstval);
+			if (ret) {
+				dev_warn(dev,
+					 "Failed to cache reset status in SCRPAD2: %d\n", ret);
+				return;
+			}
+		} else {
+			ret = regmap_read(gcr_regmap, NPCM7XX_SCRPAD2_OFFSET,
+					  &rstval);
+			if (ret) {
+				dev_warn(dev,
+					 "Failed to read cached reset status from SCRPAD2: %d\n",
+					 ret);
+				return;
+			}
+		}
+	}
+
+	for (i = 0; i < data->status_map_size; i++) {
+		if (rstval & data->status_map[i].rst_bit)
+			wdt->wdd.bootstatus |= data->status_map[i].wdiof_flag;
+	}
+
+	for (i = 0; i < data->rst_map_size; i++) {
+		if (data->rst_map[i].base == start &&
+		    rstval & data->rst_map[i].rst_bit) {
+			wdt->wdd.bootstatus |= WDIOF_CARDRESET;
+			break;
+		}
+	}
+}
+
 static const struct watchdog_info npcm_wdt_info = {
 	.identity	= KBUILD_MODNAME,
 	.options	= WDIOF_SETTIMEOUT
@@ -176,6 +363,20 @@ static const struct watchdog_info npcm_wdt_info = {
 			| WDIOF_MAGICCLOSE,
 };
 
+static const struct watchdog_info npcm_wdt_rst_info = {
+	.identity	= KBUILD_MODNAME,
+	.options	= WDIOF_SETTIMEOUT
+			| WDIOF_KEEPALIVEPING
+			| WDIOF_MAGICCLOSE
+			| WDIOF_CARDRESET
+			| WDIOF_OVERHEAT
+			| WDIOF_FANFAULT
+			| WDIOF_EXTERN1
+			| WDIOF_EXTERN2
+			| WDIOF_POWERUNDER
+			| WDIOF_POWEROVER,
+};
+
 static const struct watchdog_ops npcm_wdt_ops = {
 	.owner = THIS_MODULE,
 	.start = npcm_wdt_start,
@@ -188,7 +389,10 @@ static const struct watchdog_ops npcm_wdt_ops = {
 static int npcm_wdt_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
+	const struct npcm_wdt_data *data = device_get_match_data(dev);
+	struct resource *res;
 	struct npcm_wdt *wdt;
+	resource_size_t start;
 	int irq;
 	int ret;
 
@@ -196,10 +400,16 @@ static int npcm_wdt_probe(struct platform_device *pdev)
 	if (!wdt)
 		return -ENOMEM;
 
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res)
+		return -EINVAL;
+
 	wdt->reg = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(wdt->reg))
 		return PTR_ERR(wdt->reg);
 
+	start = res->start;
+
 	wdt->clk = devm_clk_get_optional(&pdev->dev, NULL);
 	if (IS_ERR(wdt->clk))
 		return PTR_ERR(wdt->clk);
@@ -208,7 +418,7 @@ static int npcm_wdt_probe(struct platform_device *pdev)
 	if (irq < 0)
 		return irq;
 
-	wdt->wdd.info = &npcm_wdt_info;
+	wdt->wdd.info = data ? &npcm_wdt_rst_info : &npcm_wdt_info;
 	wdt->wdd.ops = &npcm_wdt_ops;
 	wdt->wdd.min_timeout = 1;
 	wdt->wdd.max_timeout = 2750;
@@ -220,6 +430,8 @@ static int npcm_wdt_probe(struct platform_device *pdev)
 	/* Ensure timeout is able to be represented by the hardware */
 	npcm_wdt_set_timeout(&wdt->wdd, wdt->wdd.timeout);
 
+	npcm_get_reset_status(wdt, dev, data, start);
+
 	if (npcm_is_running(&wdt->wdd)) {
 		/* Restart with the default or device-tree specified timeout */
 		npcm_wdt_start(&wdt->wdd);
@@ -243,7 +455,8 @@ static int npcm_wdt_probe(struct platform_device *pdev)
 #ifdef CONFIG_OF
 static const struct of_device_id npcm_wdt_match[] = {
 	{.compatible = "nuvoton,wpcm450-wdt"},
-	{.compatible = "nuvoton,npcm750-wdt"},
+	{.compatible = "nuvoton,npcm750-wdt", .data = &npcm750_data},
+	{.compatible = "nuvoton,npcm845-wdt", .data = &npcm845_data},
 	{},
 };
 MODULE_DEVICE_TABLE(of, npcm_wdt_match);
-- 
2.34.1

Re: [PATCH v3 2/3] docs: watchdog: npcm: Add reset status description

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The watchdog driver abuses standard Watchdog UAPI constants to report non-watchdog platform reset causes, violating the userspace ABI contract.
--

commit 72fd6c8b87d96376853f3dcd5ef1597495cf9817
Author: Tomer Maimon <tmaimon77@gmail.com>

docs: watchdog: npcm: Add reset status description

Add documentation describing how the NPCM watchdog driver reports reset
causes through bootstatus on NPCM750 and NPCM845 systems.

> diff --git a/Documentation/watchdog/npcm_wdt.rst b/Documentation/watchdog/npcm_wdt.rst
> new file mode 100644
> index 0000000000000..cdc45ade9e0d9
> --- /dev/null
> +++ b/Documentation/watchdog/npcm_wdt.rst

[ ... ]

> +.. list-table:: Reset-cause mapping
> +   :header-rows: 1
> +
> +   * - Platform
> +     - Reset indication
> +     - Bootstatus flag
> +     - Reported meaning
> +   * - NPCM750 and NPCM845
> +     - ``PORST``
> +     - ``WDIOF_OVERHEAT``
> +     - power-on reset
> +   * - NPCM750 and NPCM845
> +     - ``CORST``
> +     - ``WDIOF_FANFAULT``
> +     - core reset

[Severity: High]
Is it correct to map generic Watchdog UAPI thermal and fan failure flags to
standard platform reset indications?

The generic Watchdog API defines strict semantics for its status flags.
For instance, WDIOF_OVERHEAT means a reset due to CPU overheat, and
WDIOF_FANFAULT means a fan failed. By mapping standard SoC reset causes like
Power-On Reset (PORST) to WDIOF_OVERHEAT and Core Reset (CORST) to
WDIOF_FANFAULT, might this break the userspace ABI contract?

During a normal cold boot, reading the SoC latch would unconditionally expose
this to userspace via the WDIOC_GETBOOTSTATUS ioctl or sysfs. Standard
userspace monitoring daemons (like systemd or wdctl) reading bootstatus
might erroneously conclude the system suffered a thermal event or fan failure,
potentially triggering unintended emergency actions or false alarms.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260705140100.1543486-1-tmaimon77@gmail.com?part=2

Re: [PATCH v3 3/3] watchdog: npcm: add bootstatus support

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:

New issues:
- [High] Non-atomic read-modify-write on shared syscon registers corrupts concurrent updates or incorrectly clears W1C bits.
- [High] The driver hijacks standardized watchdog bootstatus flags to report unrelated SoC reset reasons.
- [Low] The driver hardcodes physical MMIO base addresses to identify driver instances.

Pre-existing issues:
- [High] The driver leaks a clock enable reference on every timeout change and fails to clean up on probe errors.
--

commit 3d82c9ef067a4770a83258e13cbb0ed566d4998b
Author: Tomer Maimon <tmaimon77@gmail.com>

watchdog: npcm: add bootstatus support

The NPCM750 uses RESSR and the NPCM845 uses INTCR2 to latch reset indications.
Read those bits during probe and map them into watchdog bootstatus flags. For
NPCM845, cache the sampled INTCR2 state in SCRPAD10 after the reset status
bits are cleared.

> diff --git a/drivers/watchdog/npcm_wdt.c b/drivers/watchdog/npcm_wdt.c
> index e62ea054bc618..82ff3956dcf21 100644
> --- a/drivers/watchdog/npcm_wdt.c
> +++ b/drivers/watchdog/npcm_wdt.c

[ ... ]

> +static const struct npcm_wdt_rst_map npcm750_rst_map[] = {
> +	{ 0xf000801c, NPCM_WD0RST },
> +	{ 0xf000901c, NPCM_WD1RST },
> +	{ 0xf000a01c, NPCM_WD2RST },
> +};

[Severity: Low]
Does this code hardcode physical MMIO base addresses to identify driver
instances?

Matching the resource start address against hardcoded values in
npcm_get_reset_status() can be brittle if the memory map changes in future
SoC revisions or alternative device tree mappings. Is there a way to use
device tree aliases or instance IDs to identify the hardware block instead?

> +
> +static const struct npcm_wdt_status_map npcm750_status_map[] = {
> +	{ NPCM_PORST, WDIOF_OVERHEAT },
> +	{ NPCM_CORST, WDIOF_FANFAULT },
> +	{ NPCM_SWR1RST, WDIOF_EXTERN1 },
> +	{ NPCM_SWR2RST, WDIOF_EXTERN2 },
> +	{ NPCM_SWR3RST, WDIOF_POWERUNDER },
> +	{ NPCM_SWR4RST, WDIOF_POWEROVER },
> +};

[Severity: High]
Does this mapping hijack standardized watchdog bootstatus flags to report
unrelated SoC reset reasons?

Translating NPCM SoC-specific reset reasons into standard UAPI flags might
cause issues in userspace. For example, WDIOF_OVERHEAT and WDIOF_FANFAULT
are strictly defined in the UAPI to indicate thermal overheating and fan
failures.

Could system monitoring tools misinterpret a normal cold boot (NPCM_CORST)
as a severe hardware failure and trigger unintended alerts or shutdowns?

[ ... ]

> +static void npcm_get_reset_status(struct npcm_wdt *wdt, struct device *dev,
> +				  const struct npcm_wdt_data *data,
> +				  resource_size_t start)
> +{

[ ... ]

> +	if (of_device_is_compatible(dev->of_node, "nuvoton,npcm845-wdt")) {
> +		ret = regmap_read(gcr_regmap, NPCM_INTCR2_OFFSET, &rstval);
> +		if (ret) {
> +			dev_warn(dev, "Failed to read INTCR2 reset status: %d\n",
> +				 ret);
> +			return;
> +		}
> +
> +		if (rstval & NPCM_RST) {
> +			ret = regmap_write(gcr_regmap, NPCM_INTCR2_OFFSET,
> +					   rstval & ~NPCM_RST);

[Severity: High]
Can this non-atomic read-modify-write sequence on the shared syscon
register corrupt concurrent updates?

If INTCR2 (and RESSR below) are standard read-write registers, this
sequence could silently overwrite modifications made by other drivers
sharing the syscon.

Alternatively, if these are Write-1-to-Clear (W1C) registers, writing
rstval & ~NPCM_RST will write 1s to all other active status bits,
inadvertently clearing them, while writing 0 to the watchdog bits,
failing to clear the intended reset flags.

Would it be safer to use regmap_update_bits() for atomic updates, or if
W1C, write only the target bits without a read-modify-write?

[ ... ]

> @@ -220,6 +430,8 @@ static int npcm_wdt_probe(struct platform_device *pdev)
>  	/* Ensure timeout is able to be represented by the hardware */
>  	npcm_wdt_set_timeout(&wdt->wdd, wdt->wdd.timeout);
>  
> +	npcm_get_reset_status(wdt, dev, data, start);
> +
>  	if (npcm_is_running(&wdt->wdd)) {
>  		/* Restart with the default or device-tree specified timeout */
>  		npcm_wdt_start(&wdt->wdd);
>  		set_bit(WDOG_HW_RUNNING, &wdt->wdd.status);
>  	}
>  
>  	ret = devm_request_irq(dev, irq, npcm_wdt_interrupt, 0, "watchdog",
>  			       wdt);
>  	if (ret)
>  		return ret;

[Severity: High]
This is a pre-existing issue, but does this code leak a clock enable
reference?

In npcm_wdt_probe(), if the hardware is running, npcm_wdt_start() is called:

npcm_wdt_start()
    clk_prepare_enable(wdt->clk);

If subsequent probe steps fail (e.g., devm_request_irq() returns an error),
probe returns without unwinding the clock enable.

Additionally, npcm_wdt_set_timeout() calls npcm_wdt_start() when the watchdog
is active. Since there is no matching disable on timeout change, every
WDIOC_SETTIMEOUT ioctl increments the clock's refcount. This prevents
the clock from ever being disabled when the watchdog is stopped, leading
to a resource leak.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260705140100.1543486-1-tmaimon77@gmail.com?part=3

Re: [PATCH v3 1/3] dt-bindings: watchdog: npcm: add GCR syscon property

[Conor Dooley]

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

On Sun, Jul 05, 2026 at 05:00:58PM +0300, Tomer Maimon wrote:
> NPCM750 and NPCM845 latch watchdog reset indications in the SoC
> GCR block rather than in the watchdog block itself.
> 
> Add the optional nuvoton,sysgcr phandle so watchdog nodes can
> reference the shared GCR reset-status registers that hold those
> latched watchdog reset indications.
> 
> This is needed by the following reset-status support, which reads
> those latches and reports watchdog-caused resets through bootstatus.
> 
> Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>

Since the 450 doesn't support this, please make it conditional.
pw-bot: changes-requested

Thanks,
Conor.

> ---
>  .../devicetree/bindings/watchdog/nuvoton,npcm750-wdt.yaml   | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/watchdog/nuvoton,npcm750-wdt.yaml b/Documentation/devicetree/bindings/watchdog/nuvoton,npcm750-wdt.yaml
> index 7aa30f5b5c49..99c225f166db 100644
> --- a/Documentation/devicetree/bindings/watchdog/nuvoton,npcm750-wdt.yaml
> +++ b/Documentation/devicetree/bindings/watchdog/nuvoton,npcm750-wdt.yaml
> @@ -40,6 +40,12 @@ properties:
>    clock-frequency:
>      description: Frequency in Hz of the clock that drives the NPCM timer.
>  
> +  nuvoton,sysgcr:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description:
> +      a phandle to access the GCR reset-status registers that latch
> +      watchdog reset indications on NPCM750 and NPCM845.
> +
>  required:
>    - compatible
>    - reg
> -- 
> 2.34.1
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v3 2/3] docs: watchdog: npcm: Add reset status description

[Tomer Maimon]

Hi,

On Sun, 5 Jul 2026 at 17:06, <sashiko-bot@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] The watchdog driver abuses standard Watchdog UAPI constants to report non-watchdog platform reset causes, violating the userspace ABI contract.
> --
>
> commit 72fd6c8b87d96376853f3dcd5ef1597495cf9817
> Author: Tomer Maimon <tmaimon77@gmail.com>
>
> docs: watchdog: npcm: Add reset status description
>
> Add documentation describing how the NPCM watchdog driver reports reset
> causes through bootstatus on NPCM750 and NPCM845 systems.
>
> > diff --git a/Documentation/watchdog/npcm_wdt.rst b/Documentation/watchdog/npcm_wdt.rst
> > new file mode 100644
> > index 0000000000000..cdc45ade9e0d9
> > --- /dev/null
> > +++ b/Documentation/watchdog/npcm_wdt.rst
>
> [ ... ]
>
> > +.. list-table:: Reset-cause mapping
> > +   :header-rows: 1
> > +
> > +   * - Platform
> > +     - Reset indication
> > +     - Bootstatus flag
> > +     - Reported meaning
> > +   * - NPCM750 and NPCM845
> > +     - ``PORST``
> > +     - ``WDIOF_OVERHEAT``
> > +     - power-on reset
> > +   * - NPCM750 and NPCM845
> > +     - ``CORST``
> > +     - ``WDIOF_FANFAULT``
> > +     - core reset
>
> [Severity: High]
> Is it correct to map generic Watchdog UAPI thermal and fan failure flags to
> standard platform reset indications?
>
Yes, it is something that was already discussed with the WD
maintainer; This is why it is mentioned in the rst document
> The generic Watchdog API defines strict semantics for its status flags.
> For instance, WDIOF_OVERHEAT means a reset due to CPU overheat, and
> WDIOF_FANFAULT means a fan failed. By mapping standard SoC reset causes like
> Power-On Reset (PORST) to WDIOF_OVERHEAT and Core Reset (CORST) to
> WDIOF_FANFAULT, might this break the userspace ABI contract?
>
> During a normal cold boot, reading the SoC latch would unconditionally expose
> this to userspace via the WDIOC_GETBOOTSTATUS ioctl or sysfs. Standard
> userspace monitoring daemons (like systemd or wdctl) reading bootstatus
> might erroneously conclude the system suffered a thermal event or fan failure,
> potentially triggering unintended emergency actions or false alarms.
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260705140100.1543486-1-tmaimon77@gmail.com?part=2

Thanks,

Tomer

Re: [PATCH v3 3/3] watchdog: npcm: add bootstatus support

[Tomer Maimon]

Hi,

On Sun, 5 Jul 2026 at 17:10, <sashiko-bot@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
>
> New issues:
> - [High] Non-atomic read-modify-write on shared syscon registers corrupts concurrent updates or incorrectly clears W1C bits.
Not needed, only the WD using this register.
> - [High] The driver hijacks standardized watchdog bootstatus flags to report unrelated SoC reset reasons.
Approved by the maintainer.
> - [Low] The driver hardcodes physical MMIO base addresses to identify driver instances.
>
> Pre-existing issues:
> - [High] The driver leaks a clock enable reference on every timeout change and fails to clean up on probe errors.
> --
>
> commit 3d82c9ef067a4770a83258e13cbb0ed566d4998b
> Author: Tomer Maimon <tmaimon77@gmail.com>
>
> watchdog: npcm: add bootstatus support
>
> The NPCM750 uses RESSR and the NPCM845 uses INTCR2 to latch reset indications.
> Read those bits during probe and map them into watchdog bootstatus flags. For
> NPCM845, cache the sampled INTCR2 state in SCRPAD10 after the reset status
> bits are cleared.
>
> > diff --git a/drivers/watchdog/npcm_wdt.c b/drivers/watchdog/npcm_wdt.c
> > index e62ea054bc618..82ff3956dcf21 100644
> > --- a/drivers/watchdog/npcm_wdt.c
> > +++ b/drivers/watchdog/npcm_wdt.c
>
> [ ... ]
>
> > +static const struct npcm_wdt_rst_map npcm750_rst_map[] = {
> > +     { 0xf000801c, NPCM_WD0RST },
> > +     { 0xf000901c, NPCM_WD1RST },
> > +     { 0xf000a01c, NPCM_WD2RST },
> > +};
>
> [Severity: Low]
> Does this code hardcode physical MMIO base addresses to identify driver
> instances?
It is hardcoded.
>
> Matching the resource start address against hardcoded values in
> npcm_get_reset_status() can be brittle if the memory map changes in future
> SoC revisions or alternative device tree mappings. Is there a way to use
> device tree aliases or instance IDs to identify the hardware block instead?
>
> > +
> > +static const struct npcm_wdt_status_map npcm750_status_map[] = {
> > +     { NPCM_PORST, WDIOF_OVERHEAT },
> > +     { NPCM_CORST, WDIOF_FANFAULT },
> > +     { NPCM_SWR1RST, WDIOF_EXTERN1 },
> > +     { NPCM_SWR2RST, WDIOF_EXTERN2 },
> > +     { NPCM_SWR3RST, WDIOF_POWERUNDER },
> > +     { NPCM_SWR4RST, WDIOF_POWEROVER },
> > +};
>
> [Severity: High]
> Does this mapping hijack standardized watchdog bootstatus flags to report
> unrelated SoC reset reasons?
>
> Translating NPCM SoC-specific reset reasons into standard UAPI flags might
> cause issues in userspace. For example, WDIOF_OVERHEAT and WDIOF_FANFAULT
> are strictly defined in the UAPI to indicate thermal overheating and fan
> failures.
>
> Could system monitoring tools misinterpret a normal cold boot (NPCM_CORST)
> as a severe hardware failure and trigger unintended alerts or shutdowns?
>
> [ ... ]
>
> > +static void npcm_get_reset_status(struct npcm_wdt *wdt, struct device *dev,
> > +                               const struct npcm_wdt_data *data,
> > +                               resource_size_t start)
> > +{
>
> [ ... ]
>
> > +     if (of_device_is_compatible(dev->of_node, "nuvoton,npcm845-wdt")) {
> > +             ret = regmap_read(gcr_regmap, NPCM_INTCR2_OFFSET, &rstval);
> > +             if (ret) {
> > +                     dev_warn(dev, "Failed to read INTCR2 reset status: %d\n",
> > +                              ret);
> > +                     return;
> > +             }
> > +
> > +             if (rstval & NPCM_RST) {
> > +                     ret = regmap_write(gcr_regmap, NPCM_INTCR2_OFFSET,
> > +                                        rstval & ~NPCM_RST);
>
> [Severity: High]
> Can this non-atomic read-modify-write sequence on the shared syscon
> register corrupt concurrent updates?
>
> If INTCR2 (and RESSR below) are standard read-write registers, this
> sequence could silently overwrite modifications made by other drivers
> sharing the syscon.
>
> Alternatively, if these are Write-1-to-Clear (W1C) registers, writing
> rstval & ~NPCM_RST will write 1s to all other active status bits,
> inadvertently clearing them, while writing 0 to the watchdog bits,
> failing to clear the intended reset flags.
>
> Would it be safer to use regmap_update_bits() for atomic updates, or if
> W1C, write only the target bits without a read-modify-write?
>
> [ ... ]
>
> > @@ -220,6 +430,8 @@ static int npcm_wdt_probe(struct platform_device *pdev)
> >       /* Ensure timeout is able to be represented by the hardware */
> >       npcm_wdt_set_timeout(&wdt->wdd, wdt->wdd.timeout);
> >
> > +     npcm_get_reset_status(wdt, dev, data, start);
> > +
> >       if (npcm_is_running(&wdt->wdd)) {
> >               /* Restart with the default or device-tree specified timeout */
> >               npcm_wdt_start(&wdt->wdd);
> >               set_bit(WDOG_HW_RUNNING, &wdt->wdd.status);
> >       }
> >
> >       ret = devm_request_irq(dev, irq, npcm_wdt_interrupt, 0, "watchdog",
> >                              wdt);
> >       if (ret)
> >               return ret;
>
> [Severity: High]
> This is a pre-existing issue, but does this code leak a clock enable
> reference?
>
> In npcm_wdt_probe(), if the hardware is running, npcm_wdt_start() is called:
>
> npcm_wdt_start()
>     clk_prepare_enable(wdt->clk);
>
> If subsequent probe steps fail (e.g., devm_request_irq() returns an error),
> probe returns without unwinding the clock enable.
>
> Additionally, npcm_wdt_set_timeout() calls npcm_wdt_start() when the watchdog
> is active. Since there is no matching disable on timeout change, every
> WDIOC_SETTIMEOUT ioctl increments the clock's refcount. This prevents
> the clock from ever being disabled when the watchdog is stopped, leading
> to a resource leak.
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260705140100.1543486-1-tmaimon77@gmail.com?part=3

Thanks,

Tomer