* [PATCH v2 0/2] watchdog: Add Exynos5515 watchdog timer support
@ 2026-08-18 15:44 ` Aiden Isik
0 siblings, 0 replies; 11+ 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] 11+ messages in thread
* [PATCH v2 0/2] watchdog: Add Exynos5515 watchdog timer support
@ 2026-08-18 15:44 ` Aiden Isik
0 siblings, 0 replies; 11+ messages in thread
From: Aiden Isik @ 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] 11+ messages in thread
* [PATCH v2 1/2] dt-bindings: watchdog: samsung-wdt: Add exynos5515-wdt compatible
2026-08-18 15:44 ` Aiden Isik
@ 2026-08-18 15:44 ` Aiden Isik
-1 siblings, 0 replies; 11+ 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] 11+ messages in thread
* [PATCH v2 1/2] dt-bindings: watchdog: samsung-wdt: Add exynos5515-wdt compatible
@ 2026-08-18 15:44 ` Aiden Isik
0 siblings, 0 replies; 11+ messages in thread
From: Aiden Isik @ 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
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] 11+ messages in thread
* [PATCH v2 2/2] watchdog: s3c2410_wdt: Add exynos5515-wdt compatible data
2026-08-18 15:44 ` Aiden Isik
@ 2026-08-18 15:44 ` Aiden Isik
-1 siblings, 0 replies; 11+ 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] 11+ messages in thread
* [PATCH v2 2/2] watchdog: s3c2410_wdt: Add exynos5515-wdt compatible data
@ 2026-08-18 15:44 ` Aiden Isik
0 siblings, 0 replies; 11+ messages in thread
From: Aiden Isik @ 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
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] 11+ messages in thread
* Re: [PATCH v2 2/2] watchdog: s3c2410_wdt: Add exynos5515-wdt compatible data
2026-08-18 15:44 ` Aiden Isik
(?)
@ 2026-08-18 15:53 ` sashiko-bot
2026-08-18 16:43 ` Guenter Roeck
-1 siblings, 1 reply; 11+ 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] 11+ messages in thread
* Re: [PATCH v2 1/2] dt-bindings: watchdog: samsung-wdt: Add exynos5515-wdt compatible
2026-08-18 15:44 ` Aiden Isik
(?)
@ 2026-08-18 15:56 ` sashiko-bot
-1 siblings, 0 replies; 11+ 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] 11+ 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; 11+ 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] 11+ 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; 11+ 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] 11+ 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
0 siblings, 0 replies; 11+ 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] 11+ messages in thread
end of thread, other threads:[~2026-08-18 19:07 UTC | newest]
Thread overview: 11+ 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 ` Aiden Isik
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
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:44 ` Aiden Isik
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
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.