Linux Watchdog driver development
 help / color / mirror / Atom feed
* [PATCH v2 0/2] watchdog: Add Exynos5515 watchdog timer support
@ 2026-08-18 15:44 Aiden Isik via B4 Relay
  2026-08-18 15:44 ` [PATCH v2 1/2] dt-bindings: watchdog: samsung-wdt: Add exynos5515-wdt compatible Aiden Isik via B4 Relay
  2026-08-18 15:44 ` [PATCH v2 2/2] watchdog: s3c2410_wdt: Add exynos5515-wdt compatible data Aiden Isik via B4 Relay
  0 siblings, 2 replies; 9+ messages in thread
From: Aiden Isik via B4 Relay @ 2026-08-18 15:44 UTC (permalink / raw)
  To: Wim Van Sebroeck, Guenter Roeck, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Peter Griffin, Alim Akhtar
  Cc: Krzysztof Kozlowski, linux-watchdog, devicetree, linux-kernel,
	linux-arm-kernel, linux-samsung-soc, Aiden Isik

Hi there,

This patchset adds support for the Exynos5515 SoC's watchdog timer
to the S3C2410 watchdog timer driver.

There is one checkpatch warning for this patchset:

> -------------------------------------------------------------------------
> Commit 8f4ec3a9b89b ("watchdog: s3c2410_wdt: Add exynos5515-wdt compatibl
> -------------------------------------------------------------------------
> WARNING: Avoid unnecessary line continuations
> #26: FILE: drivers/watchdog/s3c2410_wdt.c:240:
> +       .quirks = QUIRK_HAS_WTCLRINT_REG | QUIRK_HAS_PMU_MASK_RESET | \

Which seems to be erroneous: this line continuation is necessary
(and is done by every other existing entry).

Signed-off-by: Aiden Isik <aidenisik@member.fsf.org>
---
Changes in v2:
- Add samsung,cluster-index: false to documentation
- Remove SoC comment from documentation
- Correct signed-off-by tag in commits
- Link to v1: https://patch.msgid.link/20260817-for-next-lucky7-watchdog-v1-0-a14370e20b92@member.fsf.org

To: Wim Van Sebroeck <wim@linux-watchdog.org>
To: Guenter Roeck <linux@roeck-us.net>
To: Rob Herring <robh@kernel.org>
To: Krzysztof Kozlowski <krzk+dt@kernel.org>
To: Conor Dooley <conor+dt@kernel.org>
To: Peter Griffin <peter.griffin@linaro.org>
To: Alim Akhtar <alim.akhtar@samsung.com>
Cc: Krzysztof Kozlowski <krzk@kernel.org>
Cc: linux-watchdog@vger.kernel.org
Cc: devicetree@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-samsung-soc@vger.kernel.org

---
Aiden Isik (2):
      dt-bindings: watchdog: samsung-wdt: Add exynos5515-wdt compatible
      watchdog: s3c2410_wdt: Add exynos5515-wdt compatible data

 .../devicetree/bindings/watchdog/samsung-wdt.yaml  | 23 +++++++++++++++++++++-
 drivers/watchdog/s3c2410_wdt.c                     | 12 +++++++++++
 2 files changed, 34 insertions(+), 1 deletion(-)
---
base-commit: acde40ba506aae78633e080b5704d33cf4bc13c6
change-id: 20260815-for-next-lucky7-watchdog-32bfb51191f1

Best regards,
--  
Aiden Isik <aidenisik@member.fsf.org>



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

* [PATCH v2 1/2] dt-bindings: watchdog: samsung-wdt: Add exynos5515-wdt compatible
  2026-08-18 15:44 [PATCH v2 0/2] watchdog: Add Exynos5515 watchdog timer support Aiden Isik via B4 Relay
@ 2026-08-18 15:44 ` Aiden Isik via B4 Relay
  2026-08-18 15:56   ` sashiko-bot
  2026-08-18 15:44 ` [PATCH v2 2/2] watchdog: s3c2410_wdt: Add exynos5515-wdt compatible data Aiden Isik via B4 Relay
  1 sibling, 1 reply; 9+ messages in thread
From: Aiden Isik via B4 Relay @ 2026-08-18 15:44 UTC (permalink / raw)
  To: Wim Van Sebroeck, Guenter Roeck, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Peter Griffin, Alim Akhtar
  Cc: Krzysztof Kozlowski, linux-watchdog, devicetree, linux-kernel,
	linux-arm-kernel, linux-samsung-soc, Aiden Isik

From: Aiden Isik <aidenisik@member.fsf.org>

Add a dt-binding compatible for the Exynos5515 watchdog timer.

This watchdog requires a syscon phandle, and the cluster index should
*not* be specified, as that does not make sense on the Exynos5515 SoC
(due to it only having a single core cluster).

Signed-off-by: Aiden Isik <aidenisik@member.fsf.org>
---
 .../devicetree/bindings/watchdog/samsung-wdt.yaml  | 23 +++++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/watchdog/samsung-wdt.yaml b/Documentation/devicetree/bindings/watchdog/samsung-wdt.yaml
index 41aee1655b0c..a32c47831577 100644
--- a/Documentation/devicetree/bindings/watchdog/samsung-wdt.yaml
+++ b/Documentation/devicetree/bindings/watchdog/samsung-wdt.yaml
@@ -22,6 +22,7 @@ properties:
           - samsung,s3c6410-wdt                   # for S3C6410, S5PV210 and Exynos4
           - samsung,exynos5250-wdt                # for Exynos5250
           - samsung,exynos5420-wdt                # for Exynos5420
+          - samsung,exynos5515-wdt
           - samsung,exynos7-wdt                   # for Exynos7
           - samsung,exynos850-wdt                 # for Exynos850
           - samsung,exynos990-wdt                 # for Exynos990
@@ -57,7 +58,7 @@ properties:
     $ref: /schemas/types.yaml#/definitions/phandle
     description:
       Phandle to the PMU system controller node (in case of Exynos5250,
-      Exynos5420, Exynos7, Exynos850, Exynos990 and gs101).
+      Exynos5420, Exynos5515, Exynos7, Exynos850, Exynos990 and gs101).
 
 required:
   - compatible
@@ -93,6 +94,26 @@ allOf:
         - samsung,cluster-index
         - samsung,syscon-phandle
 
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - samsung,exynos5515-wdt
+    then:
+      properties:
+        clocks:
+          items:
+            - description: Bus clock, used for register interface
+            - description: Source clock (driving watchdog counter)
+        clock-names:
+          items:
+            - const: watchdog
+            - const: watchdog_src
+        samsung,cluster-index: false
+      required:
+        - samsung,syscon-phandle
+
   - if:
       properties:
         compatible:

-- 
2.54.0



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

* [PATCH v2 2/2] watchdog: s3c2410_wdt: Add exynos5515-wdt compatible data
  2026-08-18 15:44 [PATCH v2 0/2] watchdog: Add Exynos5515 watchdog timer support Aiden Isik via B4 Relay
  2026-08-18 15:44 ` [PATCH v2 1/2] dt-bindings: watchdog: samsung-wdt: Add exynos5515-wdt compatible Aiden Isik via B4 Relay
@ 2026-08-18 15:44 ` Aiden Isik via B4 Relay
  2026-08-18 15:53   ` sashiko-bot
  1 sibling, 1 reply; 9+ messages in thread
From: Aiden Isik via B4 Relay @ 2026-08-18 15:44 UTC (permalink / raw)
  To: Wim Van Sebroeck, Guenter Roeck, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Peter Griffin, Alim Akhtar
  Cc: Krzysztof Kozlowski, linux-watchdog, devicetree, linux-kernel,
	linux-arm-kernel, linux-samsung-soc, Aiden Isik

From: Aiden Isik <aidenisik@member.fsf.org>

Add driver data for the Exynos5515 SoC's watchdog timer.

Signed-off-by: Aiden Isik <aidenisik@member.fsf.org>
---
 drivers/watchdog/s3c2410_wdt.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/watchdog/s3c2410_wdt.c b/drivers/watchdog/s3c2410_wdt.c
index e31f93db0509..7299dfdb0c09 100644
--- a/drivers/watchdog/s3c2410_wdt.c
+++ b/drivers/watchdog/s3c2410_wdt.c
@@ -232,6 +232,16 @@ static const struct s3c2410_wdt_variant drv_data_exynos5420 = {
 		  QUIRK_HAS_PMU_RST_STAT | QUIRK_HAS_PMU_AUTO_DISABLE,
 };
 
+static const struct s3c2410_wdt_variant drv_data_exynos5515 = {
+	.mask_reset_reg = EXYNOSAUTOV920_CLUSTER0_NONCPU_INT_EN,
+	.mask_bit = 2,
+	.rst_stat_reg = EXYNOS5_RST_STAT_REG_OFFSET,
+	.rst_stat_bit = 24,
+	.quirks = QUIRK_HAS_WTCLRINT_REG | QUIRK_HAS_PMU_MASK_RESET | \
+		  QUIRK_HAS_PMU_RST_STAT | QUIRK_HAS_PMU_AUTO_DISABLE | \
+		  QUIRK_HAS_DBGACK_BIT,
+};
+
 static const struct s3c2410_wdt_variant drv_data_exynos7 = {
 	.disable_reg = EXYNOS5_WDT_DISABLE_REG_OFFSET,
 	.mask_reset_reg = EXYNOS5_WDT_MASK_RESET_REG_OFFSET,
@@ -379,6 +389,8 @@ static const struct of_device_id s3c2410_wdt_match[] = {
 	  .data = &drv_data_exynos5250 },
 	{ .compatible = "samsung,exynos5420-wdt",
 	  .data = &drv_data_exynos5420 },
+	{ .compatible = "samsung,exynos5515-wdt",
+	  .data = &drv_data_exynos5515 },
 	{ .compatible = "samsung,exynos7-wdt",
 	  .data = &drv_data_exynos7 },
 	{ .compatible = "samsung,exynos850-wdt",

-- 
2.54.0



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

* Re: [PATCH v2 2/2] watchdog: s3c2410_wdt: Add exynos5515-wdt compatible data
  2026-08-18 15:44 ` [PATCH v2 2/2] watchdog: s3c2410_wdt: Add exynos5515-wdt compatible data Aiden Isik via B4 Relay
@ 2026-08-18 15:53   ` sashiko-bot
  2026-08-18 16:43     ` Guenter Roeck
  0 siblings, 1 reply; 9+ messages in thread
From: sashiko-bot @ 2026-08-18 15:53 UTC (permalink / raw)
  To: Aiden Isik; +Cc: devicetree, robh, conor+dt, linux-watchdog

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The `drv_data_exynos5515` variant includes `QUIRK_HAS_PMU_AUTO_DISABLE` but fails to define `.disable_reg`, resulting in PMU register corruption at offset 0x0.
- [High] The `drv_data_exynos5515` variant uses an INT_EN register for masking but fails to set `.mask_reset_inv = true`, inverting the watchdog masking logic.
--

--- Patch [2]: [PATCH v2 2/2] watchdog: s3c2410_wdt: Add exynos5515-wdt compatible data ---
commit fd223d0c16451bec63089d85a77c3b012b8f28ed
Author: Aiden Isik <aidenisik@member.fsf.org>

watchdog: s3c2410_wdt: Add exynos5515-wdt compatible data
    
Add driver data for the Exynos5515 SoC's watchdog timer.

> diff --git a/drivers/watchdog/s3c2410_wdt.c b/drivers/watchdog/s3c2410_wdt.c
> index e31f93db05096..7299dfdb0c099 100644
> --- a/drivers/watchdog/s3c2410_wdt.c
> +++ b/drivers/watchdog/s3c2410_wdt.c
> @@ -232,6 +232,16 @@ static const struct s3c2410_wdt_variant drv_data_exynos5420 = {
>  		  QUIRK_HAS_PMU_RST_STAT | QUIRK_HAS_PMU_AUTO_DISABLE,
>  };
>  
> +static const struct s3c2410_wdt_variant drv_data_exynos5515 = {
> +	.mask_reset_reg = EXYNOSAUTOV920_CLUSTER0_NONCPU_INT_EN,

[Severity: High]
This variant uses an INT_EN register for masking but omits .mask_reset_inv.
Could this cause the watchdog masking logic to be inverted?

Looking at s3c2410wdt_mask_wdt_reset(), the logic evaluates to false when
unmasking if .mask_reset_inv is not set to true:

s3c2410wdt_mask_wdt_reset() {
	...
	const bool val_inv = wdt->drv_data->mask_reset_inv;
	const u32 val = (mask ^ val_inv) ? mask_val : 0;
	...
	ret = regmap_update_bits(wdt->pmureg, wdt->drv_data->mask_reset_reg,
				 mask_val, val);

Does this mean it will write a 0 when unmasking, disabling the reset output
instead of enabling it?

> +	.mask_bit = 2,
> +	.rst_stat_reg = EXYNOS5_RST_STAT_REG_OFFSET,
> +	.rst_stat_bit = 24,
> +	.quirks = QUIRK_HAS_WTCLRINT_REG | QUIRK_HAS_PMU_MASK_RESET | \
> +		  QUIRK_HAS_PMU_RST_STAT | QUIRK_HAS_PMU_AUTO_DISABLE | \

[Severity: High]
Since QUIRK_HAS_PMU_AUTO_DISABLE is set but .disable_reg is omitted, will
.disable_reg default to 0x0?

During initialization and suspend, s3c2410wdt_disable_wdt_reset() applies
updates to wdt->drv_data->disable_reg:

s3c2410wdt_disable_wdt_reset() {
	...
	ret = regmap_update_bits(wdt->pmureg, wdt->drv_data->disable_reg,
				 mask_val, val);

Could this unintentionally corrupt bit 2 at PMU offset 0x0?

> +		  QUIRK_HAS_DBGACK_BIT,
> +};
> +

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260818-for-next-lucky7-watchdog-v2-0-6743f479a47f@member.fsf.org?part=2

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

* Re: [PATCH v2 1/2] dt-bindings: watchdog: samsung-wdt: Add exynos5515-wdt compatible
  2026-08-18 15:44 ` [PATCH v2 1/2] dt-bindings: watchdog: samsung-wdt: Add exynos5515-wdt compatible Aiden Isik via B4 Relay
@ 2026-08-18 15:56   ` sashiko-bot
  0 siblings, 0 replies; 9+ messages in thread
From: sashiko-bot @ 2026-08-18 15:56 UTC (permalink / raw)
  To: Aiden Isik; +Cc: conor+dt, linux-watchdog, robh, devicetree

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The `samsung,exynos5515-wdt` compatible string is defined as a standalone string without a fallback, but corresponding support is entirely missing from the watchdog driver.
--

--- Patch [1]: [PATCH v2 1/2] dt-bindings: watchdog: samsung-wdt: Add exynos5515-wdt compatible ---
commit 1111ae2e418dde480b9a0bc5f9cb740eb0c3a137
Author: Aiden Isik <aidenisik@member.fsf.org>

dt-bindings: watchdog: samsung-wdt: Add exynos5515-wdt compatible

Add a dt-binding compatible for the Exynos5515 watchdog timer.

> diff --git a/Documentation/devicetree/bindings/watchdog/samsung-wdt.yaml b/Documentation/devicetree/bindings/watchdog/samsung-wdt.yaml
> index 41aee1655b0c2..a32c478315779 100644
> --- a/Documentation/devicetree/bindings/watchdog/samsung-wdt.yaml
> +++ b/Documentation/devicetree/bindings/watchdog/samsung-wdt.yaml
> @@ -22,6 +22,7 @@ properties:
>            - samsung,s3c6410-wdt                   # for S3C6410, S5PV210 and Exynos4
>            - samsung,exynos5250-wdt                # for Exynos5250
>            - samsung,exynos5420-wdt                # for Exynos5420
> +          - samsung,exynos5515-wdt

[Severity: Medium]
Does this standalone compatible string need a corresponding entry in the
watchdog driver to function?

Because there is no fallback compatible string provided in this binding, it
looks like the driver lacks a matching entry to probe the device. Looking at
the device ID table in drivers/watchdog/s3c2410_wdt.c:

drivers/watchdog/s3c2410_wdt.c
static const struct of_device_id s3c2410_wdt_match[] = {
    ...
	{ .compatible = "google,gs101-wdt",
	  .data = &drv_data_gs101_cl0 },
    ...
};

Without an addition to s3c2410_wdt_match or a suitable fallback string in
the device tree, will the Exynos5515 watchdog silently fail to probe and
leave the hardware unsupported at runtime?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260818-for-next-lucky7-watchdog-v2-0-6743f479a47f@member.fsf.org?part=1

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

* Re: [PATCH v2 2/2] watchdog: s3c2410_wdt: Add exynos5515-wdt compatible data
  2026-08-18 15:53   ` sashiko-bot
@ 2026-08-18 16:43     ` Guenter Roeck
  2026-08-18 17:56       ` Aiden Isik
  0 siblings, 1 reply; 9+ messages in thread
From: Guenter Roeck @ 2026-08-18 16:43 UTC (permalink / raw)
  To: sashiko-reviews, Aiden Isik; +Cc: devicetree, robh, conor+dt, linux-watchdog

Aiden,

On 8/18/26 08:53, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] The `drv_data_exynos5515` variant includes `QUIRK_HAS_PMU_AUTO_DISABLE` but fails to define `.disable_reg`, resulting in PMU register corruption at offset 0x0.
> - [High] The `drv_data_exynos5515` variant uses an INT_EN register for masking but fails to set `.mask_reset_inv = true`, inverting the watchdog masking logic.
> --
> 
Please either address or explain why those are non-issues.

Guenter

> --- Patch [2]: [PATCH v2 2/2] watchdog: s3c2410_wdt: Add exynos5515-wdt compatible data ---
> commit fd223d0c16451bec63089d85a77c3b012b8f28ed
> Author: Aiden Isik <aidenisik@member.fsf.org>
> 
> watchdog: s3c2410_wdt: Add exynos5515-wdt compatible data
>      
> Add driver data for the Exynos5515 SoC's watchdog timer.
> 
>> diff --git a/drivers/watchdog/s3c2410_wdt.c b/drivers/watchdog/s3c2410_wdt.c
>> index e31f93db05096..7299dfdb0c099 100644
>> --- a/drivers/watchdog/s3c2410_wdt.c
>> +++ b/drivers/watchdog/s3c2410_wdt.c
>> @@ -232,6 +232,16 @@ static const struct s3c2410_wdt_variant drv_data_exynos5420 = {
>>   		  QUIRK_HAS_PMU_RST_STAT | QUIRK_HAS_PMU_AUTO_DISABLE,
>>   };
>>   
>> +static const struct s3c2410_wdt_variant drv_data_exynos5515 = {
>> +	.mask_reset_reg = EXYNOSAUTOV920_CLUSTER0_NONCPU_INT_EN,
> 
> [Severity: High]
> This variant uses an INT_EN register for masking but omits .mask_reset_inv.
> Could this cause the watchdog masking logic to be inverted?
> 
> Looking at s3c2410wdt_mask_wdt_reset(), the logic evaluates to false when
> unmasking if .mask_reset_inv is not set to true:
> 
> s3c2410wdt_mask_wdt_reset() {
> 	...
> 	const bool val_inv = wdt->drv_data->mask_reset_inv;
> 	const u32 val = (mask ^ val_inv) ? mask_val : 0;
> 	...
> 	ret = regmap_update_bits(wdt->pmureg, wdt->drv_data->mask_reset_reg,
> 				 mask_val, val);
> 
> Does this mean it will write a 0 when unmasking, disabling the reset output
> instead of enabling it?
> 
>> +	.mask_bit = 2,
>> +	.rst_stat_reg = EXYNOS5_RST_STAT_REG_OFFSET,
>> +	.rst_stat_bit = 24,
>> +	.quirks = QUIRK_HAS_WTCLRINT_REG | QUIRK_HAS_PMU_MASK_RESET | \
>> +		  QUIRK_HAS_PMU_RST_STAT | QUIRK_HAS_PMU_AUTO_DISABLE | \
> 
> [Severity: High]
> Since QUIRK_HAS_PMU_AUTO_DISABLE is set but .disable_reg is omitted, will
> .disable_reg default to 0x0?
> 
> During initialization and suspend, s3c2410wdt_disable_wdt_reset() applies
> updates to wdt->drv_data->disable_reg:
> 
> s3c2410wdt_disable_wdt_reset() {
> 	...
> 	ret = regmap_update_bits(wdt->pmureg, wdt->drv_data->disable_reg,
> 				 mask_val, val);
> 
> Could this unintentionally corrupt bit 2 at PMU offset 0x0?
> 
>> +		  QUIRK_HAS_DBGACK_BIT,
>> +};
>> +
> 
> [ ... ]
> 


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

* Re: [PATCH v2 2/2] watchdog: s3c2410_wdt: Add exynos5515-wdt compatible data
  2026-08-18 16:43     ` Guenter Roeck
@ 2026-08-18 17:56       ` Aiden Isik
  2026-08-18 19:07         ` Guenter Roeck
  0 siblings, 1 reply; 9+ messages in thread
From: Aiden Isik @ 2026-08-18 17:56 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: sashiko-reviews, Aiden Isik, devicetree, robh, conor+dt,
	linux-watchdog

Hi Guenter,

> Aiden,
> 
> On 8/18/26 08:53, sashiko-bot@kernel.org wrote:
> > Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> > - [High] The `drv_data_exynos5515` variant includes `QUIRK_HAS_PMU_AUTO_DISABLE` but fails to define `.disable_reg`, resulting in PMU register corruption at offset 0x0.
> > - [High] The `drv_data_exynos5515` variant uses an INT_EN register for masking but fails to set `.mask_reset_inv = true`, inverting the watchdog masking logic.
> > --
> >
> Please either address or explain why those are non-issues.
> 

For the 'disable_reg' issue Sashiko flagged, downstream says this:

> /* PMU registers are changed.
>  * MASK_RESET register was replaced by CLUSTERx_NONCPU_INT_TYPE register.
>  * DISABLE register was removed and its value was always fixed by 0.
>  */

Which is why I omitted disable_reg. It no longer exists.

Looking closer just now, A few months ago when I was implementing this
I mistook pmu_reset_func for auto_disable_func. There is no
auto_disable_func set in the struct corresponding to exynos5515 and so
the QUIRK_HAS_PMU_AUTO_DISABLE flag should be removed. I'm not sure how
I did not catch that. Will fix in v3.

As for the mask inversion issue, similar story. Will also fix in v3.

I've been using this driver for the past few months and it has been
working, I assumed it poked the watchdog every so often but I suppose
with these problems it disables it completely by default. I will keep
that in mind when testing future contributions to this driver.

> Guenter
> 
> > --- Patch [2]: [PATCH v2 2/2] watchdog: s3c2410_wdt: Add exynos5515-wdt compatible data ---
> > commit fd223d0c16451bec63089d85a77c3b012b8f28ed
> > Author: Aiden Isik <aidenisik@member.fsf.org>
> >
> > watchdog: s3c2410_wdt: Add exynos5515-wdt compatible data
> >
> > Add driver data for the Exynos5515 SoC's watchdog timer.
> >
> >> diff --git a/drivers/watchdog/s3c2410_wdt.c b/drivers/watchdog/s3c2410_wdt.c
> >> index e31f93db05096..7299dfdb0c099 100644
> >> --- a/drivers/watchdog/s3c2410_wdt.c
> >> +++ b/drivers/watchdog/s3c2410_wdt.c
> >> @@ -232,6 +232,16 @@ static const struct s3c2410_wdt_variant drv_data_exynos5420 = {
> >>   		  QUIRK_HAS_PMU_RST_STAT | QUIRK_HAS_PMU_AUTO_DISABLE,
> >>   };
> >>
> >> +static const struct s3c2410_wdt_variant drv_data_exynos5515 = {
> >> +	.mask_reset_reg = EXYNOSAUTOV920_CLUSTER0_NONCPU_INT_EN,
> >
> > [Severity: High]
> > This variant uses an INT_EN register for masking but omits .mask_reset_inv.
> > Could this cause the watchdog masking logic to be inverted?
> >
> > Looking at s3c2410wdt_mask_wdt_reset(), the logic evaluates to false when
> > unmasking if .mask_reset_inv is not set to true:
> >
> > s3c2410wdt_mask_wdt_reset() {
> > 	...
> > 	const bool val_inv = wdt->drv_data->mask_reset_inv;
> > 	const u32 val = (mask ^ val_inv) ? mask_val : 0;
> > 	...
> > 	ret = regmap_update_bits(wdt->pmureg, wdt->drv_data->mask_reset_reg,
> > 				 mask_val, val);
> >
> > Does this mean it will write a 0 when unmasking, disabling the reset output
> > instead of enabling it?
> >
> >> +	.mask_bit = 2,
> >> +	.rst_stat_reg = EXYNOS5_RST_STAT_REG_OFFSET,
> >> +	.rst_stat_bit = 24,
> >> +	.quirks = QUIRK_HAS_WTCLRINT_REG | QUIRK_HAS_PMU_MASK_RESET | \
> >> +		  QUIRK_HAS_PMU_RST_STAT | QUIRK_HAS_PMU_AUTO_DISABLE | \
> >
> > [Severity: High]
> > Since QUIRK_HAS_PMU_AUTO_DISABLE is set but .disable_reg is omitted, will
> > .disable_reg default to 0x0?
> >
> > During initialization and suspend, s3c2410wdt_disable_wdt_reset() applies
> > updates to wdt->drv_data->disable_reg:
> >
> > s3c2410wdt_disable_wdt_reset() {
> > 	...
> > 	ret = regmap_update_bits(wdt->pmureg, wdt->drv_data->disable_reg,
> > 				 mask_val, val);
> >
> > Could this unintentionally corrupt bit 2 at PMU offset 0x0?
> >
> >> +		  QUIRK_HAS_DBGACK_BIT,
> >> +};
> >> +
> >
> > [ ... ]
> >
> 
> 

Kind regards,
- Aiden Isik

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

* Re: [PATCH v2 2/2] watchdog: s3c2410_wdt: Add exynos5515-wdt compatible data
  2026-08-18 17:56       ` Aiden Isik
@ 2026-08-18 19:07         ` Guenter Roeck
  2026-08-18 19:32           ` Aiden Isik
  0 siblings, 1 reply; 9+ messages in thread
From: Guenter Roeck @ 2026-08-18 19:07 UTC (permalink / raw)
  To: Aiden Isik
  Cc: sashiko-reviews, Aiden Isik, devicetree, robh, conor+dt,
	linux-watchdog

On 8/18/26 10:56, Aiden Isik wrote:
> Hi Guenter,
> 
>> Aiden,
>>
>> On 8/18/26 08:53, sashiko-bot@kernel.org wrote:
>>> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
>>> - [High] The `drv_data_exynos5515` variant includes `QUIRK_HAS_PMU_AUTO_DISABLE` but fails to define `.disable_reg`, resulting in PMU register corruption at offset 0x0.
>>> - [High] The `drv_data_exynos5515` variant uses an INT_EN register for masking but fails to set `.mask_reset_inv = true`, inverting the watchdog masking logic.
>>> --
>>>
>> Please either address or explain why those are non-issues.
>>
> 
> For the 'disable_reg' issue Sashiko flagged, downstream says this:
> 
>> /* PMU registers are changed.
>>   * MASK_RESET register was replaced by CLUSTERx_NONCPU_INT_TYPE register.
>>   * DISABLE register was removed and its value was always fixed by 0.
>>   */
> 
> Which is why I omitted disable_reg. It no longer exists.
> 

What is "downstream" ? I do not see this comment in the patch, not
in linux-next as of next-20260817.

Guenter


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

* Re: [PATCH v2 2/2] watchdog: s3c2410_wdt: Add exynos5515-wdt compatible data
  2026-08-18 19:07         ` Guenter Roeck
@ 2026-08-18 19:32           ` Aiden Isik
  0 siblings, 0 replies; 9+ messages in thread
From: Aiden Isik @ 2026-08-18 19:32 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: sashiko-reviews, Aiden Isik, devicetree, robh, conor+dt,
	linux-watchdog

Hi Guenter,

> On 8/18/26 10:56, Aiden Isik wrote:
> > Hi Guenter,
> >
> >> Aiden,
> >>
> >> On 8/18/26 08:53, sashiko-bot@kernel.org wrote:
> >>> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> >>> - [High] The `drv_data_exynos5515` variant includes `QUIRK_HAS_PMU_AUTO_DISABLE` but fails to define `.disable_reg`, resulting in PMU register corruption at offset 0x0.
> >>> - [High] The `drv_data_exynos5515` variant uses an INT_EN register for masking but fails to set `.mask_reset_inv = true`, inverting the watchdog masking logic.
> >>> --
> >>>
> >> Please either address or explain why those are non-issues.
> >>
> >
> > For the 'disable_reg' issue Sashiko flagged, downstream says this:
> >
> >> /* PMU registers are changed.
> >>   * MASK_RESET register was replaced by CLUSTERx_NONCPU_INT_TYPE register.
> >>   * DISABLE register was removed and its value was always fixed by 0.
> >>   */
> >
> > Which is why I omitted disable_reg. It no longer exists.
> >
>
> What is "downstream" ? I do not see this comment in the patch, not
> in linux-next as of next-20260817.

Samsung's Linux kernel fork for Exynos5515. I host a mirror of it at
https://github.com/Aiden-Isik/linux-android-lucky7.

>
> Guenter
>
>

Kind regards,
- Aiden Isik

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

end of thread, other threads:[~2026-08-18 19:32 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-18 15:44 [PATCH v2 0/2] watchdog: Add Exynos5515 watchdog timer support Aiden Isik via B4 Relay
2026-08-18 15:44 ` [PATCH v2 1/2] dt-bindings: watchdog: samsung-wdt: Add exynos5515-wdt compatible Aiden Isik via B4 Relay
2026-08-18 15:56   ` sashiko-bot
2026-08-18 15:44 ` [PATCH v2 2/2] watchdog: s3c2410_wdt: Add exynos5515-wdt compatible data Aiden Isik via B4 Relay
2026-08-18 15:53   ` sashiko-bot
2026-08-18 16:43     ` Guenter Roeck
2026-08-18 17:56       ` Aiden Isik
2026-08-18 19:07         ` Guenter Roeck
2026-08-18 19:32           ` Aiden Isik

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