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

["Alim Akhtar" <alim.akhtar@samsung.com>]

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