[PATCH v3 0/2] watchdog: Add Exynos5515 watchdog timer support

[PATCH v3 0/2] watchdog: Add Exynos5515 watchdog timer support

[Aiden Isik via B4 Relay]

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 v3:
- Set reset_mask_inv to true
- Remove erroneous QUIRK_HAS_PMU_AUTO_DISABLE flag
- Link to v2: https://patch.msgid.link/20260818-for-next-lucky7-watchdog-v2-0-6743f479a47f@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>

[PATCH v3 1/2] dt-bindings: watchdog: samsung-wdt: Add exynos5515-wdt compatible

[Aiden Isik via B4 Relay]

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

Re: [PATCH v3 1/2] dt-bindings: watchdog: samsung-wdt: Add exynos5515-wdt compatible

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The new `samsung,exynos5515-wdt` conditional block may incorrectly enforce a 2-clock constraint, potentially violating the subsystem generation grouping rule.
--

--- Patch [1]: [PATCH v3 1/2] dt-bindings: watchdog: samsung-wdt: Add exynos5515-wdt compatible ---
commit 751ef37a6e709e26029d210213d732f6299f833b
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.
    
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).

> 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

[ ... ]

> @@ -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

[Severity: Medium]
Does the Exynos 5515 hardware actually require two clocks?

Other Exynos 5 hardware, like the exynos5250 and exynos5420, typically
use a single clock constraint.

If the Exynos 5515 only has a single clock, this two-clock constraint will
unintentionally force device tree authors to provide a non-existent second
clock to pass schema validation.

> +        samsung,cluster-index: false
> +      required:
> +        - samsung,syscon-phandle
> +

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

Re: [PATCH v3 1/2] dt-bindings: watchdog: samsung-wdt: Add exynos5515-wdt compatible

[Aiden Isik]

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] The new `samsung,exynos5515-wdt` conditional block may incorrectly enforce a 2-clock constraint, potentially violating the subsystem generation grouping rule.
> --
>
> --- Patch [1]: [PATCH v3 1/2] dt-bindings: watchdog: samsung-wdt: Add exynos5515-wdt compatible ---
> commit 751ef37a6e709e26029d210213d732f6299f833b
> 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.
>
> 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).
>
> > 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
>
> [ ... ]
>
> > @@ -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
>
> [Severity: Medium]
> Does the Exynos 5515 hardware actually require two clocks?
>
> Other Exynos 5 hardware, like the exynos5250 and exynos5420, typically
> use a single clock constraint.
>
> If the Exynos 5515 only has a single clock, this two-clock constraint will
> unintentionally force device tree authors to provide a non-existent second
> clock to pass schema validation.

Exynos5515 is more similar to Exynos990 and Exynos2200 than Exynos5.
Two clocks is correct.

>
> > +        samsung,cluster-index: false
> > +      required:
> > +        - samsung,syscon-phandle
> > +
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260818-for-next-lucky7-watchdog-v3-0-2912bd122c57@member.fsf.org?part=1
>

[PATCH v3 2/2] watchdog: s3c2410_wdt: Add exynos5515-wdt compatible data

[Aiden Isik via B4 Relay]

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..5592f25fe6c8 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,
+	.mask_reset_inv = true,
+	.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_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

Re: [PATCH v3 2/2] watchdog: s3c2410_wdt: Add exynos5515-wdt compatible data

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Missing PMU counter enable configuration for Exynos5515 watchdog.
--

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

watchdog: s3c2410_wdt: Add exynos5515-wdt compatible data
    
This commit adds 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..5592f25fe6c83 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,
> +	.mask_reset_inv = true,
> +	.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_DBGACK_BIT,
> +};

[Severity: Medium]
Does the Exynos5515 PMU require explicit counter enablement like other recent
Exynos SoCs?

Other variants in this driver (like Exynos850, AutoV920, and GS101) that use
the NONCPU_INT_EN PMU registers also pair it with a NONCPU_OUT register and
the QUIRK_HAS_PMU_CNT_EN flag to ensure the watchdog counter ticks.

If the Exynos5515 hardware works similarly, omitting cnt_en_reg, cnt_en_bit,
and QUIRK_HAS_PMU_CNT_EN here might leave the counter disabled and the
watchdog silently non-functional. Could you confirm if this variant needs the
counter enable logic?

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

Re: [PATCH v3 2/2] watchdog: s3c2410_wdt: Add exynos5515-wdt compatible data

[Aiden Isik]

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] Missing PMU counter enable configuration for Exynos5515 watchdog.
> --
> 
> --- Patch [2]: [PATCH v3 2/2] watchdog: s3c2410_wdt: Add exynos5515-wdt compatible data ---
> commit 9ce360e3c933b7447ba5da9ba1b7ae3fee989703
> Author: Aiden Isik <aidenisik@member.fsf.org>
> 
> watchdog: s3c2410_wdt: Add exynos5515-wdt compatible data
> 
> This commit adds 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..5592f25fe6c83 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,
> > +	.mask_reset_inv = true,
> > +	.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_DBGACK_BIT,
> > +};
> 
> [Severity: Medium]
> Does the Exynos5515 PMU require explicit counter enablement like other recent
> Exynos SoCs?
> 
> Other variants in this driver (like Exynos850, AutoV920, and GS101) that use
> the NONCPU_INT_EN PMU registers also pair it with a NONCPU_OUT register and
> the QUIRK_HAS_PMU_CNT_EN flag to ensure the watchdog counter ticks.
> 
> If the Exynos5515 hardware works similarly, omitting cnt_en_reg, cnt_en_bit,
> and QUIRK_HAS_PMU_CNT_EN here might leave the counter disabled and the
> watchdog silently non-functional. Could you confirm if this variant needs the
> counter enable logic?

Exynos5515 does not require this.

Downstream on GS101, as an example, has this struct member set:

> pmu_count_en_func

This is not set downstream for Exynos5515, so it is not used for
Exynos5515.

> 
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260818-for-next-lucky7-watchdog-v3-0-2912bd122c57@member.fsf.org?part=2
>

Re: [PATCH v3 2/2] watchdog: s3c2410_wdt: Add exynos5515-wdt compatible data

[Guenter Roeck]

On 8/18/26 11:33, Aiden Isik wrote:
>> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>> - [Medium] Missing PMU counter enable configuration for Exynos5515 watchdog.
>> --
>>
>> --- Patch [2]: [PATCH v3 2/2] watchdog: s3c2410_wdt: Add exynos5515-wdt compatible data ---
>> commit 9ce360e3c933b7447ba5da9ba1b7ae3fee989703
>> Author: Aiden Isik <aidenisik@member.fsf.org>
>>
>> watchdog: s3c2410_wdt: Add exynos5515-wdt compatible data
>>
>> This commit adds 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..5592f25fe6c83 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,
>>> +	.mask_reset_inv = true,
>>> +	.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_DBGACK_BIT,
>>> +};
>>
>> [Severity: Medium]
>> Does the Exynos5515 PMU require explicit counter enablement like other recent
>> Exynos SoCs?
>>
>> Other variants in this driver (like Exynos850, AutoV920, and GS101) that use
>> the NONCPU_INT_EN PMU registers also pair it with a NONCPU_OUT register and
>> the QUIRK_HAS_PMU_CNT_EN flag to ensure the watchdog counter ticks.
>>
>> If the Exynos5515 hardware works similarly, omitting cnt_en_reg, cnt_en_bit,
>> and QUIRK_HAS_PMU_CNT_EN here might leave the counter disabled and the
>> watchdog silently non-functional. Could you confirm if this variant needs the
>> counter enable logic?
> 
> Exynos5515 does not require this.
> 
> Downstream on GS101, as an example, has this struct member set:
> 
>> pmu_count_en_func
> 
> This is not set downstream for Exynos5515, so it is not used for
> Exynos5515.
> 

You keep talking about "downstream". What is that ?

Guenter

Re: [PATCH v3 2/2] watchdog: s3c2410_wdt: Add exynos5515-wdt compatible data

[Aiden Isik]

Hi Guenter,

> On 8/18/26 11:33, Aiden Isik wrote:
> >> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> >> - [Medium] Missing PMU counter enable configuration for Exynos5515 watchdog.
> >> --
> >>
> >> --- Patch [2]: [PATCH v3 2/2] watchdog: s3c2410_wdt: Add exynos5515-wdt compatible data ---
> >> commit 9ce360e3c933b7447ba5da9ba1b7ae3fee989703
> >> Author: Aiden Isik <aidenisik@member.fsf.org>
> >>
> >> watchdog: s3c2410_wdt: Add exynos5515-wdt compatible data
> >>
> >> This commit adds 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..5592f25fe6c83 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,
> >>> +	.mask_reset_inv = true,
> >>> +	.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_DBGACK_BIT,
> >>> +};
> >>
> >> [Severity: Medium]
> >> Does the Exynos5515 PMU require explicit counter enablement like other recent
> >> Exynos SoCs?
> >>
> >> Other variants in this driver (like Exynos850, AutoV920, and GS101) that use
> >> the NONCPU_INT_EN PMU registers also pair it with a NONCPU_OUT register and
> >> the QUIRK_HAS_PMU_CNT_EN flag to ensure the watchdog counter ticks.
> >>
> >> If the Exynos5515 hardware works similarly, omitting cnt_en_reg, cnt_en_bit,
> >> and QUIRK_HAS_PMU_CNT_EN here might leave the counter disabled and the
> >> watchdog silently non-functional. Could you confirm if this variant needs the
> >> counter enable logic?
> >
> > Exynos5515 does not require this.
> >
> > Downstream on GS101, as an example, has this struct member set:
> >
> >> pmu_count_en_func
> >
> > This is not set downstream for Exynos5515, so it is not used for
> > Exynos5515.
> >
> 
> You keep talking about "downstream". What is that ?

By "downstream" I am referring specifically to Samsung's Linux kernel
fork for Exynos5515.

I host a copy here: https://github.com/Aiden-Isik/linux-android-lucky7

Also in this message I refer to Google's GS101 kernel fork, which can
be found here:
https://github.com/LineageOS/android_kernel_google_gs101

> 
> Guenter
> 
> 

Kind regards,
- Aiden Isik

Re: [PATCH v3 2/2] watchdog: s3c2410_wdt: Add exynos5515-wdt compatible data

[Guenter Roeck]

On 8/18/26 12:35, Aiden Isik wrote:
> Hi Guenter,
> 
>> On 8/18/26 11:33, Aiden Isik wrote:
>>>> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>>>> - [Medium] Missing PMU counter enable configuration for Exynos5515 watchdog.
>>>> --
>>>>
>>>> --- Patch [2]: [PATCH v3 2/2] watchdog: s3c2410_wdt: Add exynos5515-wdt compatible data ---
>>>> commit 9ce360e3c933b7447ba5da9ba1b7ae3fee989703
>>>> Author: Aiden Isik <aidenisik@member.fsf.org>
>>>>
>>>> watchdog: s3c2410_wdt: Add exynos5515-wdt compatible data
>>>>
>>>> This commit adds 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..5592f25fe6c83 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,
>>>>> +	.mask_reset_inv = true,
>>>>> +	.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_DBGACK_BIT,
>>>>> +};
>>>>
>>>> [Severity: Medium]
>>>> Does the Exynos5515 PMU require explicit counter enablement like other recent
>>>> Exynos SoCs?
>>>>
>>>> Other variants in this driver (like Exynos850, AutoV920, and GS101) that use
>>>> the NONCPU_INT_EN PMU registers also pair it with a NONCPU_OUT register and
>>>> the QUIRK_HAS_PMU_CNT_EN flag to ensure the watchdog counter ticks.
>>>>
>>>> If the Exynos5515 hardware works similarly, omitting cnt_en_reg, cnt_en_bit,
>>>> and QUIRK_HAS_PMU_CNT_EN here might leave the counter disabled and the
>>>> watchdog silently non-functional. Could you confirm if this variant needs the
>>>> counter enable logic?
>>>
>>> Exynos5515 does not require this.
>>>
>>> Downstream on GS101, as an example, has this struct member set:
>>>
>>>> pmu_count_en_func
>>>
>>> This is not set downstream for Exynos5515, so it is not used for
>>> Exynos5515.
>>>
>>
>> You keep talking about "downstream". What is that ?
> 
> By "downstream" I am referring specifically to Samsung's Linux kernel
> fork for Exynos5515.
> 
> I host a copy here: https://github.com/Aiden-Isik/linux-android-lucky7
> 
> Also in this message I refer to Google's GS101 kernel fork, which can
> be found here:
> https://github.com/LineageOS/android_kernel_google_gs101
> 

Both are completely irrelevant for the upstream kernel. I am most definitely
not going to look into that code. If there is anything relevant - such as
the comment you pointed to earlier - you'll need to provide that as independent
comment(s), not just refer to some downstream code as explanation. The
explanation needs to be provided in the _upstream_ kernel code.

Guenter

>>
>> Guenter
>>
>>
> 
> Kind regards,
> - Aiden Isik

Re: [PATCH v3 2/2] watchdog: s3c2410_wdt: Add exynos5515-wdt compatible data

[Aiden Isik]

Hi Guenter,

> On 8/18/26 12:35, Aiden Isik wrote:
> > Hi Guenter,
> >
> >> On 8/18/26 11:33, Aiden Isik wrote:
> >>>> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> >>>> - [Medium] Missing PMU counter enable configuration for Exynos5515 watchdog.
> >>>> --
> >>>>
> >>>> --- Patch [2]: [PATCH v3 2/2] watchdog: s3c2410_wdt: Add exynos5515-wdt compatible data ---
> >>>> commit 9ce360e3c933b7447ba5da9ba1b7ae3fee989703
> >>>> Author: Aiden Isik <aidenisik@member.fsf.org>
> >>>>
> >>>> watchdog: s3c2410_wdt: Add exynos5515-wdt compatible data
> >>>>
> >>>> This commit adds 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..5592f25fe6c83 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,
> >>>>> +	.mask_reset_inv = true,
> >>>>> +	.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_DBGACK_BIT,
> >>>>> +};
> >>>>
> >>>> [Severity: Medium]
> >>>> Does the Exynos5515 PMU require explicit counter enablement like other recent
> >>>> Exynos SoCs?
> >>>>
> >>>> Other variants in this driver (like Exynos850, AutoV920, and GS101) that use
> >>>> the NONCPU_INT_EN PMU registers also pair it with a NONCPU_OUT register and
> >>>> the QUIRK_HAS_PMU_CNT_EN flag to ensure the watchdog counter ticks.
> >>>>
> >>>> If the Exynos5515 hardware works similarly, omitting cnt_en_reg, cnt_en_bit,
> >>>> and QUIRK_HAS_PMU_CNT_EN here might leave the counter disabled and the
> >>>> watchdog silently non-functional. Could you confirm if this variant needs the
> >>>> counter enable logic?
> >>>
> >>> Exynos5515 does not require this.
> >>>
> >>> Downstream on GS101, as an example, has this struct member set:
> >>>
> >>>> pmu_count_en_func
> >>>
> >>> This is not set downstream for Exynos5515, so it is not used for
> >>> Exynos5515.
> >>>
> >>
> >> You keep talking about "downstream". What is that ?
> >
> > By "downstream" I am referring specifically to Samsung's Linux kernel
> > fork for Exynos5515.
> >
> > I host a copy here: https://github.com/Aiden-Isik/linux-android-lucky7
> >
> > Also in this message I refer to Google's GS101 kernel fork, which can
> > be found here:
> > https://github.com/LineageOS/android_kernel_google_gs101
> >
> 
> Both are completely irrelevant for the upstream kernel. I am most definitely
> not going to look into that code. If there is anything relevant - such as
> the comment you pointed to earlier - you'll need to provide that as independent
> comment(s), not just refer to some downstream code as explanation. The
> explanation needs to be provided in the _upstream_ kernel code.
> 

There's not really any explanation for it other than "that is the way it is".
Samsung designed the Exynos5515 watchdog timer to not have that quirk,
I read the downstream code and learnt that fact, so we don't enable
that quirk.

> Guenter
> 
> >>
> >> Guenter
> >>
> >>
> >
> > Kind regards,
> > - Aiden Isik
> 
> 

Kind regards,
- Aiden Isik

Re: [PATCH v3 2/2] watchdog: s3c2410_wdt: Add exynos5515-wdt compatible data

[Aiden Isik]

Hi again Guenter,

> Hi Guenter,
> 
> > On 8/18/26 12:35, Aiden Isik wrote:
> > > Hi Guenter,
> > >
> > >> On 8/18/26 11:33, Aiden Isik wrote:
> > >>>> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> > >>>> - [Medium] Missing PMU counter enable configuration for Exynos5515 watchdog.
> > >>>> --
> > >>>>
> > >>>> --- Patch [2]: [PATCH v3 2/2] watchdog: s3c2410_wdt: Add exynos5515-wdt compatible data ---
> > >>>> commit 9ce360e3c933b7447ba5da9ba1b7ae3fee989703
> > >>>> Author: Aiden Isik <aidenisik@member.fsf.org>
> > >>>>
> > >>>> watchdog: s3c2410_wdt: Add exynos5515-wdt compatible data
> > >>>>
> > >>>> This commit adds 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..5592f25fe6c83 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,
> > >>>>> +	.mask_reset_inv = true,
> > >>>>> +	.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_DBGACK_BIT,
> > >>>>> +};
> > >>>>
> > >>>> [Severity: Medium]
> > >>>> Does the Exynos5515 PMU require explicit counter enablement like other recent
> > >>>> Exynos SoCs?
> > >>>>
> > >>>> Other variants in this driver (like Exynos850, AutoV920, and GS101) that use
> > >>>> the NONCPU_INT_EN PMU registers also pair it with a NONCPU_OUT register and
> > >>>> the QUIRK_HAS_PMU_CNT_EN flag to ensure the watchdog counter ticks.
> > >>>>
> > >>>> If the Exynos5515 hardware works similarly, omitting cnt_en_reg, cnt_en_bit,
> > >>>> and QUIRK_HAS_PMU_CNT_EN here might leave the counter disabled and the
> > >>>> watchdog silently non-functional. Could you confirm if this variant needs the
> > >>>> counter enable logic?
> > >>>
> > >>> Exynos5515 does not require this.
> > >>>
> > >>> Downstream on GS101, as an example, has this struct member set:
> > >>>
> > >>>> pmu_count_en_func
> > >>>
> > >>> This is not set downstream for Exynos5515, so it is not used for
> > >>> Exynos5515.
> > >>>
> > >>
> > >> You keep talking about "downstream". What is that ?
> > >
> > > By "downstream" I am referring specifically to Samsung's Linux kernel
> > > fork for Exynos5515.
> > >
> > > I host a copy here: https://github.com/Aiden-Isik/linux-android-lucky7
> > >
> > > Also in this message I refer to Google's GS101 kernel fork, which can
> > > be found here:
> > > https://github.com/LineageOS/android_kernel_google_gs101
> > >
> >
> > Both are completely irrelevant for the upstream kernel. I am most definitely
> > not going to look into that code. If there is anything relevant - such as
> > the comment you pointed to earlier - you'll need to provide that as independent
> > comment(s), not just refer to some downstream code as explanation. The
> > explanation needs to be provided in the _upstream_ kernel code.
> >
> 
> There's not really any explanation for it other than "that is the way it is".
> Samsung designed the Exynos5515 watchdog timer to not have that quirk,
> I read the downstream code and learnt that fact, so we don't enable
> that quirk.
> 

Just in case I am misunderstanding what you are asking for and you want
examples of how I know this is correct to be posted on the mailing list,
here you go.

Unfortunately Samsung doesn't publish technical specifications for their
modern SoCs, so the best documentation we have is their own source code,
which doesn't really explain the *why*, but it does explain the *what*
and the *how*.

I'll do my best to explain it concisely and how we drive the hardware
in the same way upstream:

Google (and/or Samsung since GS101 is Exynos-based) define the struct
for the watchdog timer like this:

> static const struct s3c2410_wdt_variant drv_data_gs101_cl0 = {
> 	.noncpu_int_en = EXYNOS_CLUSTER0_NONCPU_INT_EN,
> 	.noncpu_out = EXYNOS_CLUSTER0_NONCPU_OUT,
> 	.mask_bit = 2,
> 	.cnt_en_bit = 8,
> 	.rst_stat_reg = EXYNOS_RST_STAT_REG_OFFSET,
> 	.rst_stat_bit = 0,      /* CLUSTER0 WDTRESET */
> 	.pmu_reset_func = s3c2410wdt_noncpu_int_en,
> 	.pmu_count_en_func = s3c2410wdt_noncpu_out,
> 	.quirks = QUIRK_HAS_PMU_CONFIG | QUIRK_HAS_RST_STAT | QUIRK_HAS_WTCLRINT_REG |
> 		  QUIRK_HAS_DBGACK_BIT | QUIRK_HAS_WTMINCNT_REG,
> };

Note 'pmu_count_en_func' is set.

Later on in the file, we have this code:

> if (wdt->drv_data->pmu_count_en_func) {
> 	ret = wdt->drv_data->pmu_count_en_func(wdt, true);
> 	if (ret < 0) {
> 		dev_info(wdt->dev, "wdt count enable fail\n");
> 		return ret;
> 	}
> }

Which will call the chosen counter enable function if that field is set,
which will, well, enable the counter.

Now, for the struct corresponding to Exynos5515 in the same driver:

> static const struct s3c2410_wdt_variant drv_data_exynos9_v4 = {
> 	.noncpu_int_en = EXYNOS_CLUSTER2_NONCPU_INT_EN,
> 	.mask_bit = 2,
> 	.rst_stat_reg = EXYNOS5_RST_STAT_REG_OFFSET,
> 	.rst_stat_bit = 23,	/* CLUSTER2 WDTRESET */
> 	.pmu_reset_func = s3c2410wdt_noncpu_int_en,
> 	.quirks = QUIRK_HAS_PMU_CONFIG | QUIRK_HAS_RST_STAT | QUIRK_HAS_WTCLRINT_REG |
> 		  QUIRK_HAS_DBGACK_BIT | QUIRK_HAS_WTMINCNT_REG,
};

We can see that 'pmu_count_en_func' is not set, meaning that code within
the if statement shown earlier is not called, and the counter is not manually
enabled.

Translating this to upstream, if QUIRK_HAS_PMU_CNT_EN is set as a flag,
the following code within the if statement will execute:

> if (wdt->drv_data->quirks & QUIRK_HAS_PMU_CNT_EN) {
> 	ret = s3c2410wdt_enable_counter(wdt, en);
> 		if (ret < 0)
> 			return ret;
> 	}

Which as we can see does the same thing and enables the counter.

So, since 'pmu_count_en_func' is enabled on GS101 in the downstream vendor
code, the QUIRK_HAS_PMU_CNT_EN flag is set, as the Sashiko AI points out.
However, since it is *not* set for Exynos5515, that means Exynos5515 does
not require manual counter enablement and that flag should not be set,
which is why the AI's concern is wrong.

I also tested this (with the watchdog actually enabled this time), and it
behaves as expected.

> > Guenter
> >
> > >>
> > >> Guenter
> > >>
> > >>
> > >
> > > Kind regards,
> > > - Aiden Isik
> >
> >
> 
> Kind regards,
> - Aiden Isik
> 

Kind regards,
- Aiden Isik

Re: [PATCH v3 2/2] watchdog: s3c2410_wdt: Add exynos5515-wdt compatible data

[Guenter Roeck]

On 8/19/26 16:36, Aiden Isik wrote:
> Hi again Guenter,
> 
>> Hi Guenter,
>>
>>> On 8/18/26 12:35, Aiden Isik wrote:
>>>> Hi Guenter,
>>>>
>>>>> On 8/18/26 11:33, Aiden Isik wrote:
>>>>>>> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>>>>>>> - [Medium] Missing PMU counter enable configuration for Exynos5515 watchdog.
>>>>>>> --
>>>>>>>
>>>>>>> --- Patch [2]: [PATCH v3 2/2] watchdog: s3c2410_wdt: Add exynos5515-wdt compatible data ---
>>>>>>> commit 9ce360e3c933b7447ba5da9ba1b7ae3fee989703
>>>>>>> Author: Aiden Isik <aidenisik@member.fsf.org>
>>>>>>>
>>>>>>> watchdog: s3c2410_wdt: Add exynos5515-wdt compatible data
>>>>>>>
>>>>>>> This commit adds 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..5592f25fe6c83 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,
>>>>>>>> +	.mask_reset_inv = true,
>>>>>>>> +	.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_DBGACK_BIT,
>>>>>>>> +};
>>>>>>>
>>>>>>> [Severity: Medium]
>>>>>>> Does the Exynos5515 PMU require explicit counter enablement like other recent
>>>>>>> Exynos SoCs?
>>>>>>>
>>>>>>> Other variants in this driver (like Exynos850, AutoV920, and GS101) that use
>>>>>>> the NONCPU_INT_EN PMU registers also pair it with a NONCPU_OUT register and
>>>>>>> the QUIRK_HAS_PMU_CNT_EN flag to ensure the watchdog counter ticks.
>>>>>>>
>>>>>>> If the Exynos5515 hardware works similarly, omitting cnt_en_reg, cnt_en_bit,
>>>>>>> and QUIRK_HAS_PMU_CNT_EN here might leave the counter disabled and the
>>>>>>> watchdog silently non-functional. Could you confirm if this variant needs the
>>>>>>> counter enable logic?
>>>>>>
>>>>>> Exynos5515 does not require this.
>>>>>>
>>>>>> Downstream on GS101, as an example, has this struct member set:
>>>>>>
>>>>>>> pmu_count_en_func
>>>>>>
>>>>>> This is not set downstream for Exynos5515, so it is not used for
>>>>>> Exynos5515.
>>>>>>
>>>>>
>>>>> You keep talking about "downstream". What is that ?
>>>>
>>>> By "downstream" I am referring specifically to Samsung's Linux kernel
>>>> fork for Exynos5515.
>>>>
>>>> I host a copy here: https://github.com/Aiden-Isik/linux-android-lucky7
>>>>
>>>> Also in this message I refer to Google's GS101 kernel fork, which can
>>>> be found here:
>>>> https://github.com/LineageOS/android_kernel_google_gs101
>>>>
>>>
>>> Both are completely irrelevant for the upstream kernel. I am most definitely
>>> not going to look into that code. If there is anything relevant - such as
>>> the comment you pointed to earlier - you'll need to provide that as independent
>>> comment(s), not just refer to some downstream code as explanation. The
>>> explanation needs to be provided in the _upstream_ kernel code.
>>>
>>
>> There's not really any explanation for it other than "that is the way it is".
>> Samsung designed the Exynos5515 watchdog timer to not have that quirk,
>> I read the downstream code and learnt that fact, so we don't enable
>> that quirk.
>>
> 
> Just in case I am misunderstanding what you are asking for and you want
> examples of how I know this is correct to be posted on the mailing list,
> here you go.
> 
> Unfortunately Samsung doesn't publish technical specifications for their
> modern SoCs, so the best documentation we have is their own source code,
> which doesn't really explain the *why*, but it does explain the *what*
> and the *how*.
> 
> I'll do my best to explain it concisely and how we drive the hardware
> in the same way upstream:
> 
> Google (and/or Samsung since GS101 is Exynos-based) define the struct
> for the watchdog timer like this:
> 
>> static const struct s3c2410_wdt_variant drv_data_gs101_cl0 = {
>> 	.noncpu_int_en = EXYNOS_CLUSTER0_NONCPU_INT_EN,
>> 	.noncpu_out = EXYNOS_CLUSTER0_NONCPU_OUT,
>> 	.mask_bit = 2,
>> 	.cnt_en_bit = 8,
>> 	.rst_stat_reg = EXYNOS_RST_STAT_REG_OFFSET,
>> 	.rst_stat_bit = 0,      /* CLUSTER0 WDTRESET */
>> 	.pmu_reset_func = s3c2410wdt_noncpu_int_en,
>> 	.pmu_count_en_func = s3c2410wdt_noncpu_out,
>> 	.quirks = QUIRK_HAS_PMU_CONFIG | QUIRK_HAS_RST_STAT | QUIRK_HAS_WTCLRINT_REG |
>> 		  QUIRK_HAS_DBGACK_BIT | QUIRK_HAS_WTMINCNT_REG,
>> };
> 
> Note 'pmu_count_en_func' is set.
> 
> Later on in the file, we have this code:
> 
>> if (wdt->drv_data->pmu_count_en_func) {
>> 	ret = wdt->drv_data->pmu_count_en_func(wdt, true);
>> 	if (ret < 0) {
>> 		dev_info(wdt->dev, "wdt count enable fail\n");
>> 		return ret;
>> 	}
>> }
> 
> Which will call the chosen counter enable function if that field is set,
> which will, well, enable the counter.
> 
> Now, for the struct corresponding to Exynos5515 in the same driver:
> 
>> static const struct s3c2410_wdt_variant drv_data_exynos9_v4 = {
>> 	.noncpu_int_en = EXYNOS_CLUSTER2_NONCPU_INT_EN,
>> 	.mask_bit = 2,
>> 	.rst_stat_reg = EXYNOS5_RST_STAT_REG_OFFSET,
>> 	.rst_stat_bit = 23,	/* CLUSTER2 WDTRESET */
>> 	.pmu_reset_func = s3c2410wdt_noncpu_int_en,
>> 	.quirks = QUIRK_HAS_PMU_CONFIG | QUIRK_HAS_RST_STAT | QUIRK_HAS_WTCLRINT_REG |
>> 		  QUIRK_HAS_DBGACK_BIT | QUIRK_HAS_WTMINCNT_REG,
> };
> 
> We can see that 'pmu_count_en_func' is not set, meaning that code within
> the if statement shown earlier is not called, and the counter is not manually
> enabled.
> 
> Translating this to upstream, if QUIRK_HAS_PMU_CNT_EN is set as a flag,
> the following code within the if statement will execute:
> 
>> if (wdt->drv_data->quirks & QUIRK_HAS_PMU_CNT_EN) {
>> 	ret = s3c2410wdt_enable_counter(wdt, en);
>> 		if (ret < 0)
>> 			return ret;
>> 	}
> 
> Which as we can see does the same thing and enables the counter.
> 
> So, since 'pmu_count_en_func' is enabled on GS101 in the downstream vendor
> code, the QUIRK_HAS_PMU_CNT_EN flag is set, as the Sashiko AI points out.
> However, since it is *not* set for Exynos5515, that means Exynos5515 does
> not require manual counter enablement and that flag should not be set,
> which is why the AI's concern is wrong.
> 
> I also tested this (with the watchdog actually enabled this time), and it
> behaves as expected.
> 

All ok, but you do not explain all this as comment in your patches, you just
point to downstream code. At least some of that downstream code, if I recall
your previous feedback correctly, actually _did_ explain at least some of
this.

Example: A comment along the line of "According to <place source here>,
Exynos5515 PMU does not require explicit counter enablement" would do.
Sashiko does not know the context. I don't either, and neither does
anyone else.

So why exactly are you not willing to add such comments ? Please explain.

Guenter

Re: [PATCH v3 2/2] watchdog: s3c2410_wdt: Add exynos5515-wdt compatible data

[Aiden Isik]

> On 8/19/26 16:36, Aiden Isik wrote:
> > Hi again Guenter,
> >
> >> Hi Guenter,
> >>
> >>> On 8/18/26 12:35, Aiden Isik wrote:
> >>>> Hi Guenter,
> >>>>
> >>>>> On 8/18/26 11:33, Aiden Isik wrote:
> >>>>>>> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> >>>>>>> - [Medium] Missing PMU counter enable configuration for Exynos5515 watchdog.
> >>>>>>> --
> >>>>>>>
> >>>>>>> --- Patch [2]: [PATCH v3 2/2] watchdog: s3c2410_wdt: Add exynos5515-wdt compatible data ---
> >>>>>>> commit 9ce360e3c933b7447ba5da9ba1b7ae3fee989703
> >>>>>>> Author: Aiden Isik <aidenisik@member.fsf.org>
> >>>>>>>
> >>>>>>> watchdog: s3c2410_wdt: Add exynos5515-wdt compatible data
> >>>>>>>
> >>>>>>> This commit adds 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..5592f25fe6c83 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,
> >>>>>>>> +	.mask_reset_inv = true,
> >>>>>>>> +	.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_DBGACK_BIT,
> >>>>>>>> +};
> >>>>>>>
> >>>>>>> [Severity: Medium]
> >>>>>>> Does the Exynos5515 PMU require explicit counter enablement like other recent
> >>>>>>> Exynos SoCs?
> >>>>>>>
> >>>>>>> Other variants in this driver (like Exynos850, AutoV920, and GS101) that use
> >>>>>>> the NONCPU_INT_EN PMU registers also pair it with a NONCPU_OUT register and
> >>>>>>> the QUIRK_HAS_PMU_CNT_EN flag to ensure the watchdog counter ticks.
> >>>>>>>
> >>>>>>> If the Exynos5515 hardware works similarly, omitting cnt_en_reg, cnt_en_bit,
> >>>>>>> and QUIRK_HAS_PMU_CNT_EN here might leave the counter disabled and the
> >>>>>>> watchdog silently non-functional. Could you confirm if this variant needs the
> >>>>>>> counter enable logic?
> >>>>>>
> >>>>>> Exynos5515 does not require this.
> >>>>>>
> >>>>>> Downstream on GS101, as an example, has this struct member set:
> >>>>>>
> >>>>>>> pmu_count_en_func
> >>>>>>
> >>>>>> This is not set downstream for Exynos5515, so it is not used for
> >>>>>> Exynos5515.
> >>>>>>
> >>>>>
> >>>>> You keep talking about "downstream". What is that ?
> >>>>
> >>>> By "downstream" I am referring specifically to Samsung's Linux kernel
> >>>> fork for Exynos5515.
> >>>>
> >>>> I host a copy here: https://github.com/Aiden-Isik/linux-android-lucky7
> >>>>
> >>>> Also in this message I refer to Google's GS101 kernel fork, which can
> >>>> be found here:
> >>>> https://github.com/LineageOS/android_kernel_google_gs101
> >>>>
> >>>
> >>> Both are completely irrelevant for the upstream kernel. I am most definitely
> >>> not going to look into that code. If there is anything relevant - such as
> >>> the comment you pointed to earlier - you'll need to provide that as independent
> >>> comment(s), not just refer to some downstream code as explanation. The
> >>> explanation needs to be provided in the _upstream_ kernel code.
> >>>
> >>
> >> There's not really any explanation for it other than "that is the way it is".
> >> Samsung designed the Exynos5515 watchdog timer to not have that quirk,
> >> I read the downstream code and learnt that fact, so we don't enable
> >> that quirk.
> >>
> >
> > Just in case I am misunderstanding what you are asking for and you want
> > examples of how I know this is correct to be posted on the mailing list,
> > here you go.
> >
> > Unfortunately Samsung doesn't publish technical specifications for their
> > modern SoCs, so the best documentation we have is their own source code,
> > which doesn't really explain the *why*, but it does explain the *what*
> > and the *how*.
> >
> > I'll do my best to explain it concisely and how we drive the hardware
> > in the same way upstream:
> >
> > Google (and/or Samsung since GS101 is Exynos-based) define the struct
> > for the watchdog timer like this:
> >
> >> static const struct s3c2410_wdt_variant drv_data_gs101_cl0 = {
> >> 	.noncpu_int_en = EXYNOS_CLUSTER0_NONCPU_INT_EN,
> >> 	.noncpu_out = EXYNOS_CLUSTER0_NONCPU_OUT,
> >> 	.mask_bit = 2,
> >> 	.cnt_en_bit = 8,
> >> 	.rst_stat_reg = EXYNOS_RST_STAT_REG_OFFSET,
> >> 	.rst_stat_bit = 0,      /* CLUSTER0 WDTRESET */
> >> 	.pmu_reset_func = s3c2410wdt_noncpu_int_en,
> >> 	.pmu_count_en_func = s3c2410wdt_noncpu_out,
> >> 	.quirks = QUIRK_HAS_PMU_CONFIG | QUIRK_HAS_RST_STAT | QUIRK_HAS_WTCLRINT_REG |
> >> 		  QUIRK_HAS_DBGACK_BIT | QUIRK_HAS_WTMINCNT_REG,
> >> };
> >
> > Note 'pmu_count_en_func' is set.
> >
> > Later on in the file, we have this code:
> >
> >> if (wdt->drv_data->pmu_count_en_func) {
> >> 	ret = wdt->drv_data->pmu_count_en_func(wdt, true);
> >> 	if (ret < 0) {
> >> 		dev_info(wdt->dev, "wdt count enable fail\n");
> >> 		return ret;
> >> 	}
> >> }
> >
> > Which will call the chosen counter enable function if that field is set,
> > which will, well, enable the counter.
> >
> > Now, for the struct corresponding to Exynos5515 in the same driver:
> >
> >> static const struct s3c2410_wdt_variant drv_data_exynos9_v4 = {
> >> 	.noncpu_int_en = EXYNOS_CLUSTER2_NONCPU_INT_EN,
> >> 	.mask_bit = 2,
> >> 	.rst_stat_reg = EXYNOS5_RST_STAT_REG_OFFSET,
> >> 	.rst_stat_bit = 23,	/* CLUSTER2 WDTRESET */
> >> 	.pmu_reset_func = s3c2410wdt_noncpu_int_en,
> >> 	.quirks = QUIRK_HAS_PMU_CONFIG | QUIRK_HAS_RST_STAT | QUIRK_HAS_WTCLRINT_REG |
> >> 		  QUIRK_HAS_DBGACK_BIT | QUIRK_HAS_WTMINCNT_REG,
> > };
> >
> > We can see that 'pmu_count_en_func' is not set, meaning that code within
> > the if statement shown earlier is not called, and the counter is not manually
> > enabled.
> >
> > Translating this to upstream, if QUIRK_HAS_PMU_CNT_EN is set as a flag,
> > the following code within the if statement will execute:
> >
> >> if (wdt->drv_data->quirks & QUIRK_HAS_PMU_CNT_EN) {
> >> 	ret = s3c2410wdt_enable_counter(wdt, en);
> >> 		if (ret < 0)
> >> 			return ret;
> >> 	}
> >
> > Which as we can see does the same thing and enables the counter.
> >
> > So, since 'pmu_count_en_func' is enabled on GS101 in the downstream vendor
> > code, the QUIRK_HAS_PMU_CNT_EN flag is set, as the Sashiko AI points out.
> > However, since it is *not* set for Exynos5515, that means Exynos5515 does
> > not require manual counter enablement and that flag should not be set,
> > which is why the AI's concern is wrong.
> >
> > I also tested this (with the watchdog actually enabled this time), and it
> > behaves as expected.
> >
> 
> All ok, but you do not explain all this as comment in your patches, you just
> point to downstream code. At least some of that downstream code, if I recall
> your previous feedback correctly, actually _did_ explain at least some of
> this.

For this specific quirk there are no comments whatsoever. That comment I
sent is one of the only comments in that file.

> 
> Example: A comment along the line of "According to <place source here>,
> Exynos5515 PMU does not require explicit counter enablement" would do.
> Sashiko does not know the context. I don't either, and neither does
> anyone else.
> 
> So why exactly are you not willing to add such comments ? Please explain.

Initially I did not add a comment before I sent the modifications to
the mailing list because nothing else seems to have comments justifying
the flags set or not set.

After you requested an explanation in the code, I thought you wanted
a full explanation like what I sent above, which would be very unwieldy.
Which is why I then also sent you that explanation here, because I wasn't
sure what exactly you were asking for.

But if it's just a comment like the example you described, I can do that.
Do you want the source to be "Samsung's downstream code", or a link to
it?

> 
> Guenter
> 
> 

Kind regards,
- Aiden Isik

Re: [PATCH v3 2/2] watchdog: s3c2410_wdt: Add exynos5515-wdt compatible data

[Guenter Roeck]

On 8/20/26 03:18, Aiden Isik wrote:

> But if it's just a comment like the example you described, I can do that.
> Do you want the source to be "Samsung's downstream code", or a link to
> it?
> 

Links may not be available forever. Just a brief explanation, please.

Thanks,
Guenter