public inbox for devicetree@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v1 0/2] watchdog: npcm: Add reset status detection support
@ 2026-02-10 13:38 Tomer Maimon
  2026-02-10 13:38 ` [PATCH v1 1/2] dt-bindings: watchdog: Add NPCM reset status support Tomer Maimon
  2026-02-10 13:38 ` [PATCH v1 2/2] watchdog: npcm: Add " Tomer Maimon
  0 siblings, 2 replies; 10+ messages in thread
From: Tomer Maimon @ 2026-02-10 13:38 UTC (permalink / raw)
  To: andrew, avifishman70, tali.perry1, wim, linux, robh, krzk+dt,
	conor+dt
  Cc: venture, yuenn, benjaminfair, joel, openbmc, linux-watchdog,
	devicetree, linux-kernel, Tomer Maimon

This series adds reset status detection support to the NPCM watchdog
driver for both NPCM7XX (Poleg) and NPCM8XX (Arbel) platforms.

Patch 1 updates the device-tree bindings to support reset status
detection on NPCM750 and NPCM8XX platforms. It introduces the
`nuvoton,card-reset-type`, `nuvoton,ext1-reset-type`, and
`nuvoton,ext2-reset-type` properties, and integrates the syscon
interface required for accessing the reset status registers.

Patch 2 implements the watchdog driver changes, including the reset
status detection infrastructure and platform restart support for both
NPCM7XX and NPCM8XX.

Tomer Maimon (2):
  dt-bindings: watchdog: Add NPCM reset status support
  watchdog: npcm: Add reset status support

 .../watchdog/nuvoton,npcm750-wdt.yaml         |  51 +++++++-
 drivers/watchdog/npcm_wdt.c                   | 110 ++++++++++++++++++
 2 files changed, 159 insertions(+), 2 deletions(-)

-- 
2.34.1


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

* [PATCH v1 1/2] dt-bindings: watchdog: Add NPCM reset status support
  2026-02-10 13:38 [PATCH v1 0/2] watchdog: npcm: Add reset status detection support Tomer Maimon
@ 2026-02-10 13:38 ` Tomer Maimon
  2026-02-10 16:11   ` Krzysztof Kozlowski
  2026-02-10 13:38 ` [PATCH v1 2/2] watchdog: npcm: Add " Tomer Maimon
  1 sibling, 1 reply; 10+ messages in thread
From: Tomer Maimon @ 2026-02-10 13:38 UTC (permalink / raw)
  To: andrew, avifishman70, tali.perry1, wim, linux, robh, krzk+dt,
	conor+dt
  Cc: venture, yuenn, benjaminfair, joel, openbmc, linux-watchdog,
	devicetree, linux-kernel, Tomer Maimon

Add reset status detection for NPCM7XX and NPCM8XX platforms via syscon
integration. Document syscon property and three configurable reset type
properties (nuvoton,card-reset-type, nuvoton,ext1-reset-type,
nuvoton,ext2-reset-type)that map reset signal detection to specific
reset bit positions.

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

diff --git a/Documentation/devicetree/bindings/watchdog/nuvoton,npcm750-wdt.yaml b/Documentation/devicetree/bindings/watchdog/nuvoton,npcm750-wdt.yaml
index 7aa30f5b5c49..054cc0115af2 100644
--- a/Documentation/devicetree/bindings/watchdog/nuvoton,npcm750-wdt.yaml
+++ b/Documentation/devicetree/bindings/watchdog/nuvoton,npcm750-wdt.yaml
@@ -12,7 +12,7 @@ maintainers:
 description:
   Nuvoton NPCM timer module provides five 24-bit timer counters, and a watchdog.
   The watchdog supports a pre-timeout interrupt that fires 10ms before the
-  expiry.
+  expiry and reset status detection via syscon integration.
 
 allOf:
   - $ref: watchdog.yaml#
@@ -40,12 +40,55 @@ properties:
   clock-frequency:
     description: Frequency in Hz of the clock that drives the NPCM timer.
 
+  syscon:
+    description: phandle to the Global Control Register (GCR) syscon node.
+    $ref: /schemas/types.yaml#/definitions/phandle
+
+  nuvoton,card-reset-type:
+    description: Reset type for external card reset signal detection.
+    enum:
+      - porst
+      - corst
+      - wd0
+      - wd1
+      - wd2
+      - sw1
+      - sw2
+      - sw3
+      - sw4
+
+  nuvoton,ext1-reset-type:
+    description: Reset type for external reset signal 1 detection.
+    enum:
+      - porst
+      - corst
+      - wd0
+      - wd1
+      - wd2
+      - sw1
+      - sw2
+      - sw3
+      - sw4
+
+  nuvoton,ext2-reset-type:
+    description: Reset type for external reset signal 2 detection.
+    enum:
+      - porst
+      - corst
+      - wd0
+      - wd1
+      - wd2
+      - sw1
+      - sw2
+      - sw3
+      - sw4
+
 required:
   - compatible
   - reg
   - interrupts
 
-unevaluatedProperties: false
+additionalProperties: false
 
 examples:
   - |
@@ -57,4 +100,8 @@ examples:
         interrupts = <GIC_SPI 47 IRQ_TYPE_LEVEL_HIGH>;
         reg = <0xf000801c 0x4>;
         clocks = <&clk NPCM7XX_CLK_TIMER>;
+        syscon = <&gcr>;
+        nuvoton,card-reset-type = "porst";
+        nuvoton,ext1-reset-type = "wd0";
+        nuvoton,ext2-reset-type = "wd2";
     };
-- 
2.34.1


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

* [PATCH v1 2/2] watchdog: npcm: Add reset status support
  2026-02-10 13:38 [PATCH v1 0/2] watchdog: npcm: Add reset status detection support Tomer Maimon
  2026-02-10 13:38 ` [PATCH v1 1/2] dt-bindings: watchdog: Add NPCM reset status support Tomer Maimon
@ 2026-02-10 13:38 ` Tomer Maimon
  2026-02-10 16:02   ` Guenter Roeck
  1 sibling, 1 reply; 10+ messages in thread
From: Tomer Maimon @ 2026-02-10 13:38 UTC (permalink / raw)
  To: andrew, avifishman70, tali.perry1, wim, linux, robh, krzk+dt,
	conor+dt
  Cc: venture, yuenn, benjaminfair, joel, openbmc, linux-watchdog,
	devicetree, linux-kernel, Tomer Maimon

Add reset status detection for NPCM watchdog driver on both NPCM7XX and
NPCM8XX platforms. Implement GCR register integration via syscon for
reset status detection and configurable reset type mapping via device
tree properties.

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

diff --git a/drivers/watchdog/npcm_wdt.c b/drivers/watchdog/npcm_wdt.c
index e62ea054bc61..ebece5d6240a 100644
--- a/drivers/watchdog/npcm_wdt.c
+++ b/drivers/watchdog/npcm_wdt.c
@@ -12,9 +12,25 @@
 #include <linux/platform_device.h>
 #include <linux/slab.h>
 #include <linux/watchdog.h>
+#include <linux/regmap.h>
+#include <linux/mfd/syscon.h>
+
+#define NPCM7XX_RESSR_OFFSET	0x6C
+#define NPCM7XX_INTCR2_OFFSET	0x60
 
 #define NPCM_WTCR	0x1C
 
+#define NPCM7XX_PORST	BIT(31)
+#define NPCM7XX_CORST	BIT(30)
+#define NPCM7XX_WD0RST	BIT(29)
+#define NPCM7XX_WD1RST	BIT(24)
+#define NPCM7XX_WD2RST	BIT(23)
+#define NPCM7XX_SWR1RST	BIT(28)
+#define NPCM7XX_SWR2RST	BIT(27)
+#define NPCM7XX_SWR3RST	BIT(26)
+#define NPCM7XX_SWR4RST	BIT(25)
+#define NPCM8XX_RST	(GENMASK(31, 23) | GENMASK(15, 12))
+
 #define NPCM_WTCLK	(BIT(10) | BIT(11))	/* Clock divider */
 #define NPCM_WTE	BIT(7)			/* Enable */
 #define NPCM_WTIE	BIT(6)			/* Enable irq */
@@ -45,6 +61,9 @@ struct npcm_wdt {
 	struct watchdog_device  wdd;
 	void __iomem		*reg;
 	struct clk		*clk;
+	u32			card_reset;
+	u32			ext1_reset;
+	u32			ext2_reset;
 };
 
 static inline struct npcm_wdt *to_npcm_wdt(struct watchdog_device *wdd)
@@ -185,6 +204,95 @@ static const struct watchdog_ops npcm_wdt_ops = {
 	.restart = npcm_wdt_restart,
 };
 
+static u32 npcm_wdt_reset_type(const char *reset_type)
+{
+	if (!strcmp(reset_type, "porst"))
+		return NPCM7XX_PORST;
+	else if (!strcmp(reset_type, "corst"))
+		return NPCM7XX_CORST;
+	else if (!strcmp(reset_type, "wd0"))
+		return NPCM7XX_WD0RST;
+	else if (!strcmp(reset_type, "wd1"))
+		return NPCM7XX_WD1RST;
+	else if (!strcmp(reset_type, "wd2"))
+		return NPCM7XX_WD2RST;
+	else if (!strcmp(reset_type, "sw1"))
+		return NPCM7XX_SWR1RST;
+	else if (!strcmp(reset_type, "sw2"))
+		return NPCM7XX_SWR2RST;
+	else if (!strcmp(reset_type, "sw3"))
+		return NPCM7XX_SWR3RST;
+	else if (!strcmp(reset_type, "sw4"))
+		return NPCM7XX_SWR4RST;
+
+	return 0;
+}
+
+static void npcm_get_reset_status(struct npcm_wdt *wdt, struct device *dev)
+{
+	const char *card_reset_type;
+	const char *ext1_reset_type;
+	const char *ext2_reset_type;
+	struct regmap *gcr_regmap;
+	u32 rstval, ressrval;
+	int ret;
+
+	gcr_regmap = syscon_regmap_lookup_by_phandle(dev->of_node, "syscon");
+	if (IS_ERR(gcr_regmap)) {
+		dev_warn(dev, "Failed to find gcr syscon, WD reset status not supported\n");
+		return;
+	}
+
+	ret = of_property_read_string(dev->of_node,
+				      "nuvoton,card-reset-type",
+				      &card_reset_type);
+	if (ret)
+		wdt->card_reset = NPCM7XX_PORST;
+	else
+		wdt->card_reset = npcm_wdt_reset_type(card_reset_type);
+
+	ret = of_property_read_string(dev->of_node,
+				      "nuvoton,ext1-reset-type",
+				      &ext1_reset_type);
+	if (ret)
+		wdt->ext1_reset = 0;
+	else
+		wdt->ext1_reset = npcm_wdt_reset_type(ext1_reset_type);
+
+	ret = of_property_read_string(dev->of_node,
+				      "nuvoton,ext2-reset-type",
+				      &ext2_reset_type);
+	if (ret)
+		wdt->ext2_reset = 0;
+	else
+		wdt->ext2_reset = npcm_wdt_reset_type(ext2_reset_type);
+
+	regmap_read(gcr_regmap, NPCM7XX_INTCR2_OFFSET, &rstval);
+	/* prefer the most specific SoC first */
+	if (of_device_is_compatible(dev->of_node, "nuvoton,npcm845-wdt")) {
+		regmap_write(gcr_regmap, NPCM7XX_INTCR2_OFFSET,
+			     rstval & ~NPCM8XX_RST);
+	} else if (of_device_is_compatible(dev->of_node, "nuvoton,npcm750-wdt")) {
+		if ((rstval & NPCM7XX_PORST) == 0) {
+			rstval = NPCM7XX_PORST;
+			regmap_write(gcr_regmap, NPCM7XX_INTCR2_OFFSET,
+				     rstval | NPCM7XX_PORST);
+		} else {
+			rstval = 0;
+		}
+		regmap_read(gcr_regmap, NPCM7XX_RESSR_OFFSET, &ressrval);
+		rstval |= ressrval;
+		regmap_write(gcr_regmap, NPCM7XX_RESSR_OFFSET, ressrval);
+	}
+
+	if (rstval & wdt->card_reset)
+		wdt->wdd.bootstatus |= WDIOF_CARDRESET;
+	if (rstval & wdt->ext1_reset)
+		wdt->wdd.bootstatus |= WDIOF_EXTERN1;
+	if (rstval & wdt->ext2_reset)
+		wdt->wdd.bootstatus |= WDIOF_EXTERN2;
+}
+
 static int npcm_wdt_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -208,6 +316,8 @@ static int npcm_wdt_probe(struct platform_device *pdev)
 	if (irq < 0)
 		return irq;
 
+	npcm_get_reset_status(wdt, dev);
+
 	wdt->wdd.info = &npcm_wdt_info;
 	wdt->wdd.ops = &npcm_wdt_ops;
 	wdt->wdd.min_timeout = 1;
-- 
2.34.1


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

* Re: [PATCH v1 2/2] watchdog: npcm: Add reset status support
  2026-02-10 13:38 ` [PATCH v1 2/2] watchdog: npcm: Add " Tomer Maimon
@ 2026-02-10 16:02   ` Guenter Roeck
       [not found]     ` <CAP6Zq1gGVB+hk+=xSRyPgddq07F_B+oE-dc246JRW2_waoe_bg@mail.gmail.com>
  0 siblings, 1 reply; 10+ messages in thread
From: Guenter Roeck @ 2026-02-10 16:02 UTC (permalink / raw)
  To: Tomer Maimon, andrew, avifishman70, tali.perry1, wim, robh,
	krzk+dt, conor+dt
  Cc: venture, yuenn, benjaminfair, joel, openbmc, linux-watchdog,
	devicetree, linux-kernel

On 2/10/26 05:38, Tomer Maimon wrote:
> Add reset status detection for NPCM watchdog driver on both NPCM7XX and
> NPCM8XX platforms. Implement GCR register integration via syscon for
> reset status detection and configurable reset type mapping via device
> tree properties.
> 
> Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>
> ---
>   drivers/watchdog/npcm_wdt.c | 110 ++++++++++++++++++++++++++++++++++++
>   1 file changed, 110 insertions(+)
> 
> diff --git a/drivers/watchdog/npcm_wdt.c b/drivers/watchdog/npcm_wdt.c
> index e62ea054bc61..ebece5d6240a 100644
> --- a/drivers/watchdog/npcm_wdt.c
> +++ b/drivers/watchdog/npcm_wdt.c
> @@ -12,9 +12,25 @@
>   #include <linux/platform_device.h>
>   #include <linux/slab.h>
>   #include <linux/watchdog.h>
> +#include <linux/regmap.h>
> +#include <linux/mfd/syscon.h>
> +
> +#define NPCM7XX_RESSR_OFFSET	0x6C
> +#define NPCM7XX_INTCR2_OFFSET	0x60
>   
>   #define NPCM_WTCR	0x1C
>   
> +#define NPCM7XX_PORST	BIT(31)
> +#define NPCM7XX_CORST	BIT(30)
> +#define NPCM7XX_WD0RST	BIT(29)
> +#define NPCM7XX_WD1RST	BIT(24)
> +#define NPCM7XX_WD2RST	BIT(23)
> +#define NPCM7XX_SWR1RST	BIT(28)
> +#define NPCM7XX_SWR2RST	BIT(27)
> +#define NPCM7XX_SWR3RST	BIT(26)
> +#define NPCM7XX_SWR4RST	BIT(25)
> +#define NPCM8XX_RST	(GENMASK(31, 23) | GENMASK(15, 12))
> +
>   #define NPCM_WTCLK	(BIT(10) | BIT(11))	/* Clock divider */
>   #define NPCM_WTE	BIT(7)			/* Enable */
>   #define NPCM_WTIE	BIT(6)			/* Enable irq */
> @@ -45,6 +61,9 @@ struct npcm_wdt {
>   	struct watchdog_device  wdd;
>   	void __iomem		*reg;
>   	struct clk		*clk;
> +	u32			card_reset;
> +	u32			ext1_reset;
> +	u32			ext2_reset;
>   };
>   
>   static inline struct npcm_wdt *to_npcm_wdt(struct watchdog_device *wdd)
> @@ -185,6 +204,95 @@ static const struct watchdog_ops npcm_wdt_ops = {
>   	.restart = npcm_wdt_restart,
>   };
>   
> +static u32 npcm_wdt_reset_type(const char *reset_type)
> +{
> +	if (!strcmp(reset_type, "porst"))
> +		return NPCM7XX_PORST;
> +	else if (!strcmp(reset_type, "corst"))
> +		return NPCM7XX_CORST;
> +	else if (!strcmp(reset_type, "wd0"))
> +		return NPCM7XX_WD0RST;
> +	else if (!strcmp(reset_type, "wd1"))
> +		return NPCM7XX_WD1RST;
> +	else if (!strcmp(reset_type, "wd2"))
> +		return NPCM7XX_WD2RST;
> +	else if (!strcmp(reset_type, "sw1"))
> +		return NPCM7XX_SWR1RST;
> +	else if (!strcmp(reset_type, "sw2"))
> +		return NPCM7XX_SWR2RST;
> +	else if (!strcmp(reset_type, "sw3"))
> +		return NPCM7XX_SWR3RST;
> +	else if (!strcmp(reset_type, "sw4"))
> +		return NPCM7XX_SWR4RST;
> +
> +	return 0;
> +}
> +
> +static void npcm_get_reset_status(struct npcm_wdt *wdt, struct device *dev)
> +{
> +	const char *card_reset_type;
> +	const char *ext1_reset_type;
> +	const char *ext2_reset_type;
> +	struct regmap *gcr_regmap;
> +	u32 rstval, ressrval;
> +	int ret;
> +
> +	gcr_regmap = syscon_regmap_lookup_by_phandle(dev->of_node, "syscon");
> +	if (IS_ERR(gcr_regmap)) {
> +		dev_warn(dev, "Failed to find gcr syscon, WD reset status not supported\n");

A warning is quite strong here, given that this is new code and the
syscon reference may not exist in existing devicetree files. notice
should be good enough.

> +		return;
> +	}
> +
> +	ret = of_property_read_string(dev->of_node,
> +				      "nuvoton,card-reset-type",
> +				      &card_reset_type);
> +	if (ret)
> +		wdt->card_reset = NPCM7XX_PORST;
> +	else
> +		wdt->card_reset = npcm_wdt_reset_type(card_reset_type);
> +
> +	ret = of_property_read_string(dev->of_node,
> +				      "nuvoton,ext1-reset-type",
> +				      &ext1_reset_type);
> +	if (ret)
> +		wdt->ext1_reset = 0;

wdt is zero-allocated, so setting those variables to 0 is not necessary.

> +	else
> +		wdt->ext1_reset = npcm_wdt_reset_type(ext1_reset_type);
> +
> +	ret = of_property_read_string(dev->of_node,
> +				      "nuvoton,ext2-reset-type",
> +				      &ext2_reset_type);
> +	if (ret)
> +		wdt->ext2_reset = 0;
> +	else
> +		wdt->ext2_reset = npcm_wdt_reset_type(ext2_reset_type);
> +
> +	regmap_read(gcr_regmap, NPCM7XX_INTCR2_OFFSET, &rstval);

This warrants an explanation/comment: Why is it not necessary to check
the return value of the regmap operations ?

> +	/* prefer the most specific SoC first */
> +	if (of_device_is_compatible(dev->of_node, "nuvoton,npcm845-wdt")) {
> +		regmap_write(gcr_regmap, NPCM7XX_INTCR2_OFFSET,
> +			     rstval & ~NPCM8XX_RST);
> +	} else if (of_device_is_compatible(dev->of_node, "nuvoton,npcm750-wdt")) {
> +		if ((rstval & NPCM7XX_PORST) == 0) {
> +			rstval = NPCM7XX_PORST;
> +			regmap_write(gcr_regmap, NPCM7XX_INTCR2_OFFSET,
> +				     rstval | NPCM7XX_PORST);

That "| NPCM7XX_PORST" is pretty pointless here since rstval was
just set to that value.

> +		} else {
> +			rstval = 0;
> +		}

Another comment needed: This negates NPCM7XX_PORST and otherwise clear
rstval. The reason is not immediately (or, rather, at all) obvious.

> +		regmap_read(gcr_regmap, NPCM7XX_RESSR_OFFSET, &ressrval);
> +		rstval |= ressrval;
> +		regmap_write(gcr_regmap, NPCM7XX_RESSR_OFFSET, ressrval);
> +	}

If the device is not compatible to either chip, retval is just passed
on and nothing is written to the chip. That warrants another comment.

[ Yes, I see that the driver does not currently support another chip.

> +
> +	if (rstval & wdt->card_reset)
> +		wdt->wdd.bootstatus |= WDIOF_CARDRESET;
> +	if (rstval & wdt->ext1_reset)
> +		wdt->wdd.bootstatus |= WDIOF_EXTERN1;
> +	if (rstval & wdt->ext2_reset)
> +		wdt->wdd.bootstatus |= WDIOF_EXTERN2;
> +}
> +
>   static int npcm_wdt_probe(struct platform_device *pdev)
>   {
>   	struct device *dev = &pdev->dev;
> @@ -208,6 +316,8 @@ static int npcm_wdt_probe(struct platform_device *pdev)
>   	if (irq < 0)
>   		return irq;
>   
> +	npcm_get_reset_status(wdt, dev);
> +
>   	wdt->wdd.info = &npcm_wdt_info;
>   	wdt->wdd.ops = &npcm_wdt_ops;
>   	wdt->wdd.min_timeout = 1;


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

* Re: [PATCH v1 1/2] dt-bindings: watchdog: Add NPCM reset status support
  2026-02-10 13:38 ` [PATCH v1 1/2] dt-bindings: watchdog: Add NPCM reset status support Tomer Maimon
@ 2026-02-10 16:11   ` Krzysztof Kozlowski
       [not found]     ` <CAP6Zq1jZorLxXQYqm5KzcYdoRzcFtD1KQqzmgaa6KKy-+Tpv+Q@mail.gmail.com>
  0 siblings, 1 reply; 10+ messages in thread
From: Krzysztof Kozlowski @ 2026-02-10 16:11 UTC (permalink / raw)
  To: Tomer Maimon, andrew, avifishman70, tali.perry1, wim, linux, robh,
	krzk+dt, conor+dt
  Cc: venture, yuenn, benjaminfair, joel, openbmc, linux-watchdog,
	devicetree, linux-kernel

On 10/02/2026 14:38, Tomer Maimon wrote:
> Add reset status detection for NPCM7XX and NPCM8XX platforms via syscon
> integration. Document syscon property and three configurable reset type
> properties (nuvoton,card-reset-type, nuvoton,ext1-reset-type,
> nuvoton,ext2-reset-type)that map reset signal detection to specific
> reset bit positions.
> 
> Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>
> ---
>  .../watchdog/nuvoton,npcm750-wdt.yaml         | 51 ++++++++++++++++++-
>  1 file changed, 49 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/watchdog/nuvoton,npcm750-wdt.yaml b/Documentation/devicetree/bindings/watchdog/nuvoton,npcm750-wdt.yaml
> index 7aa30f5b5c49..054cc0115af2 100644
> --- a/Documentation/devicetree/bindings/watchdog/nuvoton,npcm750-wdt.yaml
> +++ b/Documentation/devicetree/bindings/watchdog/nuvoton,npcm750-wdt.yaml
> @@ -12,7 +12,7 @@ maintainers:
>  description:
>    Nuvoton NPCM timer module provides five 24-bit timer counters, and a watchdog.
>    The watchdog supports a pre-timeout interrupt that fires 10ms before the
> -  expiry.
> +  expiry and reset status detection via syscon integration.
>  
>  allOf:
>    - $ref: watchdog.yaml#
> @@ -40,12 +40,55 @@ properties:
>    clock-frequency:
>      description: Frequency in Hz of the clock that drives the NPCM timer.
>  
> +  syscon:

First iteration. See "How to Get Your DT Schema Bindings Accepted in
Less Than 10 Iterations"

or just read the docs please.

> +    description: phandle to the Global Control Register (GCR) syscon node.
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +
> +  nuvoton,card-reset-type:
> +    description: Reset type for external card reset signal detection.
> +    enum:
> +      - porst
> +      - corst
> +      - wd0
> +      - wd1
> +      - wd2
> +      - sw1
> +      - sw2
> +      - sw3
> +      - sw4

You want it to be static configuration, so cannot be changed runtime? Why?

> +
> +  nuvoton,ext1-reset-type:
> +    description: Reset type for external reset signal 1 detection.
> +    enum:
> +      - porst
> +      - corst
> +      - wd0
> +      - wd1
> +      - wd2
> +      - sw1
> +      - sw2
> +      - sw3
> +      - sw4
> +
> +  nuvoton,ext2-reset-type:
> +    description: Reset type for external reset signal 2 detection.
> +    enum:
> +      - porst
> +      - corst
> +      - wd0
> +      - wd1
> +      - wd2
> +      - sw1
> +      - sw2
> +      - sw3
> +      - sw4
> +
>  required:
>    - compatible
>    - reg
>    - interrupts
>  
> -unevaluatedProperties: false
> +additionalProperties: false

Why? Nothing explains this in the commit msg.


>  
>  examples:
>    - |
> @@ -57,4 +100,8 @@ examples:
>          interrupts = <GIC_SPI 47 IRQ_TYPE_LEVEL_HIGH>;
>          reg = <0xf000801c 0x4>;
>          clocks = <&clk NPCM7XX_CLK_TIMER>;
> +        syscon = <&gcr>;
> +        nuvoton,card-reset-type = "porst";
> +        nuvoton,ext1-reset-type = "wd0";
> +        nuvoton,ext2-reset-type = "wd2";
>      };


Best regards,
Krzysztof

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

* Re: [PATCH v1 1/2] dt-bindings: watchdog: Add NPCM reset status support
       [not found]     ` <CAP6Zq1jZorLxXQYqm5KzcYdoRzcFtD1KQqzmgaa6KKy-+Tpv+Q@mail.gmail.com>
@ 2026-02-16 11:15       ` Krzysztof Kozlowski
       [not found]         ` <CAP6Zq1hLkT-xMwV99yVE-hLsf_nT+V_3v7sEshfqEkkRCkEevA@mail.gmail.com>
  0 siblings, 1 reply; 10+ messages in thread
From: Krzysztof Kozlowski @ 2026-02-16 11:15 UTC (permalink / raw)
  To: Tomer Maimon
  Cc: andrew, avifishman70, tali.perry1, wim, linux, robh, krzk+dt,
	conor+dt, venture, yuenn, benjaminfair, joel, openbmc,
	linux-watchdog, devicetree, linux-kernel

On 16/02/2026 12:10, Tomer Maimon wrote:
> Hi Krzysztof,
> 
> Thanks for your review.
> 
> On Tue, 10 Feb 2026 at 18:11, Krzysztof Kozlowski <krzk@kernel.org> wrote:
> 
>> On 10/02/2026 14:38, Tomer Maimon wrote:
>>> Add reset status detection for NPCM7XX and NPCM8XX platforms via syscon
>>> integration. Document syscon property and three configurable reset type
>>> properties (nuvoton,card-reset-type, nuvoton,ext1-reset-type,
>>> nuvoton,ext2-reset-type)that map reset signal detection to specific
>>> reset bit positions.
>>>
>>> Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>
>>> ---
>>>  .../watchdog/nuvoton,npcm750-wdt.yaml         | 51 ++++++++++++++++++-
>>>  1 file changed, 49 insertions(+), 2 deletions(-)
>>>
>>> diff --git
>> a/Documentation/devicetree/bindings/watchdog/nuvoton,npcm750-wdt.yaml
>> b/Documentation/devicetree/bindings/watchdog/nuvoton,npcm750-wdt.yaml
>>> index 7aa30f5b5c49..054cc0115af2 100644
>>> --- a/Documentation/devicetree/bindings/watchdog/nuvoton,npcm750-wdt.yaml
>>> +++ b/Documentation/devicetree/bindings/watchdog/nuvoton,npcm750-wdt.yaml
>>> @@ -12,7 +12,7 @@ maintainers:
>>>  description:
>>>    Nuvoton NPCM timer module provides five 24-bit timer counters, and a
>> watchdog.
>>>    The watchdog supports a pre-timeout interrupt that fires 10ms before
>> the
>>> -  expiry.
>>> +  expiry and reset status detection via syscon integration.
>>>
>>>  allOf:
>>>    - $ref: watchdog.yaml#
>>> @@ -40,12 +40,55 @@ properties:
>>>    clock-frequency:
>>>      description: Frequency in Hz of the clock that drives the NPCM
>> timer.
>>>
>>> +  syscon:
>>
>> First iteration. See "How to Get Your DT Schema Bindings Accepted in
>> Less Than 10 Iterations"
>>
> Thanks, it was very helpful.
> the syscon property is already found in the WD node
> in nuvoton-common-npcm8xx.dtsi file, what should I do:

How is that file related to this binding?

Either you document existing ABI or you add new (for new device). Commit
msg MUST be explicit about it and provide the reasons. If wrong (e.g.
discouraged) ABI was already used then it depends how and when it got
into the kernel, e.g. if someone bypassed DT completely just to get it
inside.

> 1. Modify the syson to nuvoton,sys-gcr like in the dtsi?
> 2. Still use the syscon property in the dtsi file, therefore stick with the
> syscon add.
> 

>>
>> or just read the docs please.
>>
>>> +    description: phandle to the Global Control Register (GCR) syscon
>> node.
>>> +    $ref: /schemas/types.yaml#/definitions/phandle
>>> +
>>> +  nuvoton,card-reset-type:
>>> +    description: Reset type for external card reset signal detection.
>>> +    enum:
>>> +      - porst
>>> +      - corst
>>> +      - wd0
>>> +      - wd1
>>> +      - wd2
>>> +      - sw1
>>> +      - sw2
>>> +      - sw3
>>> +      - sw4
>>
>> You want it to be static configuration, so cannot be changed runtime? Why?
>>
> Yes, it is only an indication of the most recent reset in the BMC. In
> addition:
> 1. The driver reads the reset register during the probe. After this read,
> the reset register should be cleared so it’s ready for the next system
> reset.
> 2. I’m not aware of any service function that allows changing the reset
> indication at runtime.

Huh? If the driver reads the reset you do not need this property at all.

Best regards,
Krzysztof

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

* Re: [PATCH v1 1/2] dt-bindings: watchdog: Add NPCM reset status support
       [not found]         ` <CAP6Zq1hLkT-xMwV99yVE-hLsf_nT+V_3v7sEshfqEkkRCkEevA@mail.gmail.com>
@ 2026-02-16 14:48           ` Krzysztof Kozlowski
       [not found]             ` <CAP6Zq1iWpHc-Rsq62iBN0VtYmYS2=KhU12TE_5nxztr+HbB+tA@mail.gmail.com>
  0 siblings, 1 reply; 10+ messages in thread
From: Krzysztof Kozlowski @ 2026-02-16 14:48 UTC (permalink / raw)
  To: Tomer Maimon
  Cc: andrew, avifishman70, tali.perry1, wim, linux, robh, krzk+dt,
	conor+dt, venture, yuenn, benjaminfair, joel, openbmc,
	linux-watchdog, devicetree, linux-kernel

On 16/02/2026 15:37, Tomer Maimon wrote:
> Hi Krzysztof,
> 
> Thanks for your comments.
> 
> On Mon, 16 Feb 2026 at 13:16, Krzysztof Kozlowski <krzk@kernel.org> wrote:
> 
>> On 16/02/2026 12:10, Tomer Maimon wrote:
>>> Hi Krzysztof,
>>>
>>> Thanks for your review.
>>>
>>> On Tue, 10 Feb 2026 at 18:11, Krzysztof Kozlowski <krzk@kernel.org>
>> wrote:
>>>
>>>> On 10/02/2026 14:38, Tomer Maimon wrote:
>>>>> Add reset status detection for NPCM7XX and NPCM8XX platforms via syscon
>>>>> integration. Document syscon property and three configurable reset type
>>>>> properties (nuvoton,card-reset-type, nuvoton,ext1-reset-type,
>>>>> nuvoton,ext2-reset-type)that map reset signal detection to specific
>>>>> reset bit positions.
>>>>>
>>>>> Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>
>>>>> ---
>>>>>  .../watchdog/nuvoton,npcm750-wdt.yaml         | 51 ++++++++++++++++++-
>>>>>  1 file changed, 49 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git
>>>> a/Documentation/devicetree/bindings/watchdog/nuvoton,npcm750-wdt.yaml
>>>> b/Documentation/devicetree/bindings/watchdog/nuvoton,npcm750-wdt.yaml
>>>>> index 7aa30f5b5c49..054cc0115af2 100644
>>>>> ---
>> a/Documentation/devicetree/bindings/watchdog/nuvoton,npcm750-wdt.yaml
>>>>> +++
>> b/Documentation/devicetree/bindings/watchdog/nuvoton,npcm750-wdt.yaml
>>>>> @@ -12,7 +12,7 @@ maintainers:
>>>>>  description:
>>>>>    Nuvoton NPCM timer module provides five 24-bit timer counters, and a
>>>> watchdog.
>>>>>    The watchdog supports a pre-timeout interrupt that fires 10ms before
>>>> the
>>>>> -  expiry.
>>>>> +  expiry and reset status detection via syscon integration.
>>>>>
>>>>>  allOf:
>>>>>    - $ref: watchdog.yaml#
>>>>> @@ -40,12 +40,55 @@ properties:
>>>>>    clock-frequency:
>>>>>      description: Frequency in Hz of the clock that drives the NPCM
>>>> timer.
>>>>>
>>>>> +  syscon:
>>>>
>>>> First iteration. See "How to Get Your DT Schema Bindings Accepted in
>>>> Less Than 10 Iterations"
>>>>
>>> Thanks, it was very helpful.
>>> the syscon property is already found in the WD node
>>> in nuvoton-common-npcm8xx.dtsi file, what should I do:
>>
>> How is that file related to this binding?
>>
>> Either you document existing ABI or you add new (for new device). Commit
>> msg MUST be explicit about it and provide the reasons. If wrong (e.g.
>> discouraged) ABI was already used then it depends how and when it got
>> into the kernel, e.g. if someone bypassed DT completely just to get it
>> inside.
>>
> The syscon property is already used in the upstream NPCM8xx DTSI watchdog
> node, so I will document it as existing ABI and mark it deprecated. I will

And how it is used? I cannot find its usage, so I do not agree on
documenting it. Property should be removed or at least provide the
justification/impact of removal, if you need it to stay.

> add a new vendor‑specific property (nuvoton,sysgcr) as the preferred one,
> and explain this clearly in the commit message.
> 
>>
>>> 1. Modify the syson to nuvoton,sys-gcr like in the dtsi?
>>> 2. Still use the syscon property in the dtsi file, therefore stick with
>> the
>>> syscon add.


Best regards,
Krzysztof

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

* Re: [PATCH v1 2/2] watchdog: npcm: Add reset status support
       [not found]     ` <CAP6Zq1gGVB+hk+=xSRyPgddq07F_B+oE-dc246JRW2_waoe_bg@mail.gmail.com>
@ 2026-02-16 15:26       ` Guenter Roeck
  0 siblings, 0 replies; 10+ messages in thread
From: Guenter Roeck @ 2026-02-16 15:26 UTC (permalink / raw)
  To: Tomer Maimon
  Cc: andrew, avifishman70, tali.perry1, wim, robh, krzk+dt, conor+dt,
	venture, yuenn, benjaminfair, joel, openbmc, linux-watchdog,
	devicetree, linux-kernel

On Mon, Feb 16, 2026 at 04:30:46PM +0200, Tomer Maimon wrote:
> Hi Guenter,
> 
> Thanks for your review.
> 
> From your comments and from Krzysztof’s earlier feedback, I understand that
> I cannot use Device Tree properties to describe software behavior, and DT
> should only describe hardware.
> 
> Given that, I am trying to understand what would be the correct upstream
> way to expose the different reset causes that the GCR reports. The watchdog
> framework provides only a few standardized bootstatus flags, and I would
> like to check whether it is acceptable to map the different reset causes
> into these existing flags.
> 
> For example, conceptually:
> 
>    - WDIOF_CARDRESET → power‑on reset
>    - WDIOF_OVERHEAT → core reset
>    - WDIOF_FANFAULT → watchdog reset
>    - WDIOF_EXTERN1 → SW0 reset
>    - WDIOF_EXTERN2 → SW1 reset
>    - WDIOF_POWERUNDER → SW2 reset
>    - WDIOF_POWEROVER → SW3 reset
> 
> Is such a mapping acceptable?
> 

Ok with me as long as it is well documented (i.e., in
Documentation/watchdog/npcm_wdt.rst or similar).

Guenter

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

* Re: [PATCH v1 1/2] dt-bindings: watchdog: Add NPCM reset status support
       [not found]             ` <CAP6Zq1iWpHc-Rsq62iBN0VtYmYS2=KhU12TE_5nxztr+HbB+tA@mail.gmail.com>
@ 2026-02-16 18:03               ` Krzysztof Kozlowski
       [not found]                 ` <CAP6Zq1gRd8zDi1rOkX+2x3WP0ZGnULaJ0cGS6bwpRpcmQNmRCA@mail.gmail.com>
  0 siblings, 1 reply; 10+ messages in thread
From: Krzysztof Kozlowski @ 2026-02-16 18:03 UTC (permalink / raw)
  To: Tomer Maimon
  Cc: andrew, avifishman70, tali.perry1, wim, linux, robh, krzk+dt,
	conor+dt, venture, yuenn, benjaminfair, joel, openbmc,
	linux-watchdog, devicetree, linux-kernel

On Mon, Feb 16, 2026 at 04:59:18PM +0200, Tomer Maimon wrote:
> On Mon, 16 Feb 2026 at 16:48, Krzysztof Kozlowski <krzk@kernel.org> wrote:
> > >>>>> +  syscon:
> > >>>>
> > >>>> First iteration. See "How to Get Your DT Schema Bindings Accepted in
> > >>>> Less Than 10 Iterations"
> > >>>>
> > >>> Thanks, it was very helpful.
> > >>> the syscon property is already found in the WD node
> > >>> in nuvoton-common-npcm8xx.dtsi file, what should I do:
> > >>
> > >> How is that file related to this binding?
> > >>
> > >> Either you document existing ABI or you add new (for new device). Commit
> > >> msg MUST be explicit about it and provide the reasons. If wrong (e.g.
> > >> discouraged) ABI was already used then it depends how and when it got
> > >> into the kernel, e.g. if someone bypassed DT completely just to get it
> > >> inside.
> > >>
> > > The syscon property is already used in the upstream NPCM8xx DTSI watchdog
> > > node, so I will document it as existing ABI and mark it deprecated. I
> > will
> >
> > And how it is used? I cannot find its usage, so I do not agree on
> > documenting it. Property should be removed or at least provide the
> > justification/impact of removal, if you need it to stay.
> >
> > Understood. The syscon phandle is used by the watchdog driver to read and

You messed up quotes.

Can you point me to the line? I REALLY want to be sure that we are not
wasting each other time, e.g. me looking at wrong code or you telling me
some bollocks from downstream.

> clear the GCR reset‑status registers and then report the reset cause
> through the watchdog bootstatus bits.
> Therefore, this property should appear in the binding only because the
> watchdog driver actually uses it — which I am implementing in this patch
> set.
> I will document it accordingly, and also add the new nuvoton,sysgcr phandle
> as the preferred name.

Best regards,
Krzysztof


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

* Re: [PATCH v1 1/2] dt-bindings: watchdog: Add NPCM reset status support
       [not found]                 ` <CAP6Zq1gRd8zDi1rOkX+2x3WP0ZGnULaJ0cGS6bwpRpcmQNmRCA@mail.gmail.com>
@ 2026-02-17  7:12                   ` Krzysztof Kozlowski
  0 siblings, 0 replies; 10+ messages in thread
From: Krzysztof Kozlowski @ 2026-02-17  7:12 UTC (permalink / raw)
  To: Tomer Maimon
  Cc: andrew, avifishman70, tali.perry1, wim, linux, robh, krzk+dt,
	conor+dt, venture, yuenn, benjaminfair, joel, openbmc,
	linux-watchdog, devicetree, linux-kernel

On 17/02/2026 07:53, Tomer Maimon wrote:
> On Mon, 16 Feb 2026 at 20:03, Krzysztof Kozlowski <krzk@kernel.org> wrote:
> 
>> On Mon, Feb 16, 2026 at 04:59:18PM +0200, Tomer Maimon wrote:
>>> On Mon, 16 Feb 2026 at 16:48, Krzysztof Kozlowski <krzk@kernel.org>
>> wrote:
>>>>>>>>> +  syscon:
>>>>>>>>
>>>>>>>> First iteration. See "How to Get Your DT Schema Bindings Accepted
>> in
>>>>>>>> Less Than 10 Iterations"
>>>>>>>>
>>>>>>> Thanks, it was very helpful.
>>>>>>> the syscon property is already found in the WD node
>>>>>>> in nuvoton-common-npcm8xx.dtsi file, what should I do:
>>>>>>
>>>>>> How is that file related to this binding?
>>>>>>
>>>>>> Either you document existing ABI or you add new (for new device).
>> Commit
>>>>>> msg MUST be explicit about it and provide the reasons. If wrong
>> (e.g.
>>>>>> discouraged) ABI was already used then it depends how and when it
>> got
>>>>>> into the kernel, e.g. if someone bypassed DT completely just to get
>> it
>>>>>> inside.
>>>>>>
>>>>> The syscon property is already used in the upstream NPCM8xx DTSI
>> watchdog
>>>>> node, so I will document it as existing ABI and mark it deprecated. I
>>>> will
>>>>
>>>> And how it is used? I cannot find its usage, so I do not agree on
>>>> documenting it. Property should be removed or at least provide the
>>>> justification/impact of removal, if you need it to stay.
>>>>
>>>> Understood. The syscon phandle is used by the watchdog driver to read
>> and
>>
>> You messed up quotes.
>>
>> Can you point me to the line? I REALLY want to be sure that we are not
>> wasting each other time, e.g. me looking at wrong code or you telling me
>> some bollocks from downstream.
>>
> The syscon property is not used in the current upstream npcm_wdt driver.
> It is used *in this patch set*, which introduces the function

Why would that matter for the ABI?

So no, your buggy DTS sneaked into the kernel before submitting bindings
is not acceptable thus ABI which was NEVER reviewed must be removed.


Best regards,
Krzysztof

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

end of thread, other threads:[~2026-02-17  7:12 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-02-10 13:38 [PATCH v1 0/2] watchdog: npcm: Add reset status detection support Tomer Maimon
2026-02-10 13:38 ` [PATCH v1 1/2] dt-bindings: watchdog: Add NPCM reset status support Tomer Maimon
2026-02-10 16:11   ` Krzysztof Kozlowski
     [not found]     ` <CAP6Zq1jZorLxXQYqm5KzcYdoRzcFtD1KQqzmgaa6KKy-+Tpv+Q@mail.gmail.com>
2026-02-16 11:15       ` Krzysztof Kozlowski
     [not found]         ` <CAP6Zq1hLkT-xMwV99yVE-hLsf_nT+V_3v7sEshfqEkkRCkEevA@mail.gmail.com>
2026-02-16 14:48           ` Krzysztof Kozlowski
     [not found]             ` <CAP6Zq1iWpHc-Rsq62iBN0VtYmYS2=KhU12TE_5nxztr+HbB+tA@mail.gmail.com>
2026-02-16 18:03               ` Krzysztof Kozlowski
     [not found]                 ` <CAP6Zq1gRd8zDi1rOkX+2x3WP0ZGnULaJ0cGS6bwpRpcmQNmRCA@mail.gmail.com>
2026-02-17  7:12                   ` Krzysztof Kozlowski
2026-02-10 13:38 ` [PATCH v1 2/2] watchdog: npcm: Add " Tomer Maimon
2026-02-10 16:02   ` Guenter Roeck
     [not found]     ` <CAP6Zq1gGVB+hk+=xSRyPgddq07F_B+oE-dc246JRW2_waoe_bg@mail.gmail.com>
2026-02-16 15:26       ` Guenter Roeck

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox