[PATCH 4/4] dt-bindings: watchdog: samsung-wdt: Split if:then: and constrain more

Krzysztof Kozlowski posted 4 patches 1 month ago
[PATCH 4/4] dt-bindings: watchdog: samsung-wdt: Split if:then: and constrain more
Posted by Krzysztof Kozlowski 1 month ago
Binding defined two if:then: blocks covering different conditions but
not fully constraining the properties per each variant:
1. "if:" to require samsung,syscon-phandle,
2. "if:" with "else:" to narrow number of clocks and require or disallow
   samsung,cluster-index.

This still did not cover following cases:
1. Disallow samsung,syscon-phandle when not applicable,
2. Narrow samsung,cluster-index to [0, 1], for SoCs with only two
   clusters.

Solving this in current format would lead to spaghetti code, so re-write
entire "if:then:" approach into mutually exclusive cases so each SoC
appears only in one "if:" block.  This allows to forbid
samsung,syscon-phandle for S3C6410, and narrow samsung,cluster-index
to [0, 1].

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 .../devicetree/bindings/watchdog/samsung-wdt.yaml  | 70 ++++++++++++++++------
 1 file changed, 52 insertions(+), 18 deletions(-)

diff --git a/Documentation/devicetree/bindings/watchdog/samsung-wdt.yaml b/Documentation/devicetree/bindings/watchdog/samsung-wdt.yaml
index 51e597ba7db2615da41f5d3b6dc4e70f6bb72bb6..41aee1655b0c22a6dce212a63fa4849089253f09 100644
--- a/Documentation/devicetree/bindings/watchdog/samsung-wdt.yaml
+++ b/Documentation/devicetree/bindings/watchdog/samsung-wdt.yaml
@@ -74,24 +74,7 @@ allOf:
           contains:
             enum:
               - google,gs101-wdt
-              - samsung,exynos5250-wdt
-              - samsung,exynos5420-wdt
-              - samsung,exynos7-wdt
               - samsung,exynos850-wdt
-              - samsung,exynos990-wdt
-              - samsung,exynosautov9-wdt
-              - samsung,exynosautov920-wdt
-    then:
-      required:
-        - samsung,syscon-phandle
-  - if:
-      properties:
-        compatible:
-          contains:
-            enum:
-              - google,gs101-wdt
-              - samsung,exynos850-wdt
-              - samsung,exynos990-wdt
               - samsung,exynosautov9-wdt
               - samsung,exynosautov920-wdt
     then:
@@ -104,9 +87,41 @@ allOf:
           items:
             - const: watchdog
             - const: watchdog_src
+        samsung,cluster-index:
+          enum: [0, 1]
       required:
         - samsung,cluster-index
-    else:
+        - samsung,syscon-phandle
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - samsung,exynos990-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
+      required:
+        - samsung,cluster-index
+        - samsung,syscon-phandle
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - samsung,exynos5250-wdt
+              - samsung,exynos5420-wdt
+              - samsung,exynos7-wdt
+    then:
       properties:
         clocks:
           items:
@@ -115,6 +130,25 @@ allOf:
           items:
             - const: watchdog
         samsung,cluster-index: false
+      required:
+        - samsung,syscon-phandle
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - samsung,s3c6410-wdt
+    then:
+      properties:
+        clocks:
+          items:
+            - description: Bus clock, which is also a source clock
+        clock-names:
+          items:
+            - const: watchdog
+        samsung,cluster-index: false
+        samsung,syscon-phandle: false
 
 unevaluatedProperties: false
 

-- 
2.48.1
Re: [PATCH 4/4] dt-bindings: watchdog: samsung-wdt: Split if:then: and constrain more
Posted by Guenter Roeck 1 month ago
On Sat, Aug 30, 2025 at 12:19:00PM +0200, Krzysztof Kozlowski wrote:
> Binding defined two if:then: blocks covering different conditions but
> not fully constraining the properties per each variant:
> 1. "if:" to require samsung,syscon-phandle,
> 2. "if:" with "else:" to narrow number of clocks and require or disallow
>    samsung,cluster-index.
> 
> This still did not cover following cases:
> 1. Disallow samsung,syscon-phandle when not applicable,
> 2. Narrow samsung,cluster-index to [0, 1], for SoCs with only two
>    clusters.
> 
> Solving this in current format would lead to spaghetti code, so re-write
> entire "if:then:" approach into mutually exclusive cases so each SoC
> appears only in one "if:" block.  This allows to forbid
> samsung,syscon-phandle for S3C6410, and narrow samsung,cluster-index
> to [0, 1].
> 
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Acked-by: Guenter Roeck <linux@roeck-us.net>
Re: [PATCH 4/4] dt-bindings: watchdog: samsung-wdt: Split if:then: and constrain more
Posted by Rob Herring (Arm) 1 month ago
On Sat, 30 Aug 2025 12:19:00 +0200, Krzysztof Kozlowski wrote:
> Binding defined two if:then: blocks covering different conditions but
> not fully constraining the properties per each variant:
> 1. "if:" to require samsung,syscon-phandle,
> 2. "if:" with "else:" to narrow number of clocks and require or disallow
>    samsung,cluster-index.
> 
> This still did not cover following cases:
> 1. Disallow samsung,syscon-phandle when not applicable,
> 2. Narrow samsung,cluster-index to [0, 1], for SoCs with only two
>    clusters.
> 
> Solving this in current format would lead to spaghetti code, so re-write
> entire "if:then:" approach into mutually exclusive cases so each SoC
> appears only in one "if:" block.  This allows to forbid
> samsung,syscon-phandle for S3C6410, and narrow samsung,cluster-index
> to [0, 1].
> 
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
>  .../devicetree/bindings/watchdog/samsung-wdt.yaml  | 70 ++++++++++++++++------
>  1 file changed, 52 insertions(+), 18 deletions(-)
> 

Acked-by: Rob Herring (Arm) <robh@kernel.org>
RE: [PATCH 4/4] dt-bindings: watchdog: samsung-wdt: Split if:then: and constrain more
Posted by Alim Akhtar 1 month ago

> -----Original Message-----
> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> Sent: Saturday, August 30, 2025 3:49 PM
> To: Wim Van Sebroeck <wim@linux-watchdog.org>; Guenter Roeck
> <linux@roeck-us.net>; Rob Herring <robh@kernel.org>; Krzysztof Kozlowski
> <krzk+dt@kernel.org>; Conor Dooley <conor+dt@kernel.org>; Alim Akhtar
> <alim.akhtar@samsung.com>
> Cc: Krzysztof Kozlowski <krzk@kernel.org>; linux-
> watchdog@vger.kernel.org; devicetree@vger.kernel.org; linux-
> kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
> samsung-soc@vger.kernel.org; Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org>
> Subject: [PATCH 4/4] dt-bindings: watchdog: samsung-wdt: Split if:then: and
> constrain more
> 
> Binding defined two if:then: blocks covering different conditions but not fully
> constraining the properties per each variant:
> 1. "if:" to require samsung,syscon-phandle, 2. "if:" with "else:" to narrow
> number of clocks and require or disallow
>    samsung,cluster-index.
> 
> This still did not cover following cases:
> 1. Disallow samsung,syscon-phandle when not applicable, 2. Narrow
> samsung,cluster-index to [0, 1], for SoCs with only two
>    clusters.
> 
> Solving this in current format would lead to spaghetti code, so re-write entire
> "if:then:" approach into mutually exclusive cases so each SoC appears only in
> one "if:" block.  This allows to forbid samsung,syscon-phandle for S3C6410,
> and narrow samsung,cluster-index to [0, 1].
> 
This looks much cleaner. 
On a side note, may be you can add an example of latest SoC binding 
for the documentation purpose as current example in this file is pretty old and simple one. 
(I know one can always look into dtsi/dts for the example, but updating here won't harm)

> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
In anycase,

Reviewed-by: Alim Akhtar <alim.akhtar@samsung.com>

>  .../devicetree/bindings/watchdog/samsung-wdt.yaml  | 70
> ++++++++++++++++------
>  1 file changed, 52 insertions(+), 18 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/watchdog/samsung-
> wdt.yaml b/Documentation/devicetree/bindings/watchdog/samsung-
> wdt.yaml
> index
> 51e597ba7db2615da41f5d3b6dc4e70f6bb72bb6..41aee1655b0c22a6dce212a6
> 3fa4849089253f09 100644
> --- a/Documentation/devicetree/bindings/watchdog/samsung-wdt.yaml
> +++ b/Documentation/devicetree/bindings/watchdog/samsung-wdt.yaml
> @@ -74,24 +74,7 @@ allOf:
>            contains:
>              enum:
>                - google,gs101-wdt
> -              - samsung,exynos5250-wdt
> -              - samsung,exynos5420-wdt
> -              - samsung,exynos7-wdt
>                - samsung,exynos850-wdt
> -              - samsung,exynos990-wdt
> -              - samsung,exynosautov9-wdt
> -              - samsung,exynosautov920-wdt
> -    then:
> -      required:
> -        - samsung,syscon-phandle
> -  - if:
> -      properties:
> -        compatible:
> -          contains:
> -            enum:
> -              - google,gs101-wdt
> -              - samsung,exynos850-wdt
> -              - samsung,exynos990-wdt
>                - samsung,exynosautov9-wdt
>                - samsung,exynosautov920-wdt
>      then:
> @@ -104,9 +87,41 @@ allOf:
>            items:
>              - const: watchdog
>              - const: watchdog_src
> +        samsung,cluster-index:
> +          enum: [0, 1]
>        required:
>          - samsung,cluster-index
> -    else:
> +        - samsung,syscon-phandle
> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - samsung,exynos990-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
> +      required:
> +        - samsung,cluster-index
> +        - samsung,syscon-phandle
> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - samsung,exynos5250-wdt
> +              - samsung,exynos5420-wdt
> +              - samsung,exynos7-wdt
> +    then:
>        properties:
>          clocks:
>            items:
> @@ -115,6 +130,25 @@ allOf:
>            items:
>              - const: watchdog
>          samsung,cluster-index: false
> +      required:
> +        - samsung,syscon-phandle
> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - samsung,s3c6410-wdt
> +    then:
> +      properties:
> +        clocks:
> +          items:
> +            - description: Bus clock, which is also a source clock
> +        clock-names:
> +          items:
> +            - const: watchdog
> +        samsung,cluster-index: false
> +        samsung,syscon-phandle: false
> 
>  unevaluatedProperties: false
> 
> 
> --
> 2.48.1