Re: [PATCH v4 2/5] dt-bindings: clock: versal-clk: Fix mio_clk index range in clock-names pattern

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The `xlnx,versal-net-clk` compatible string matches multiple contradictory `if` blocks, making valid device tree nodes mathematically impossible to validate.
--

--- Patch [2]: [PATCH v4 2/5] dt-bindings: clock: versal-clk: Fix mio_clk index range in clock-names pattern ---
commit bbeb0fbfb689e023d79c16fd79850f4f854a36ee
Author: Michal Simek <michal.simek@amd.com>

    dt-bindings: clock: versal-clk: Fix mio_clk index range in clock-names pattern
    
    The clock-names pattern "^mio_clk[00-77]+.*$" was intended to constrain
    the MIO index to the valid range 00..77 (ZynqMP has 78 MIO pins),
    but a regex character class cannot express a multi-digit decimal range.
    Replace the bogus character class with an explicit alternation that
    enumerates the two-digit decimal values 00..77.
    
    Fixes: 03d4a1004053 ("dt-bindings: clock: versal: Convert the xlnx,zynqmp-clk.txt to yaml")
    Signed-off-by: Michal Simek <michal.simek@amd.com>
    Acked-by: Conor Dooley <conor.dooley@microchip.com>

> diff --git a/Documentation/devicetree/bindings/clock/xlnx,versal-clk.yaml b/Documentation/devicetree/bindings/clock/xlnx,versal-clk.yaml
> index bef109d163a82..b1623c2ab0f69 100644
> --- a/Documentation/devicetree/bindings/clock/xlnx,versal-clk.yaml
> +++ b/Documentation/devicetree/bindings/clock/xlnx,versal-clk.yaml
> @@ -116,7 +116,7 @@ allOf:
>              - const: pss_alt_ref_clk
>              - const: aux_ref_clk
>              - const: gt_crx_ref_clk
> -            - pattern: "^mio_clk[00-77]+.*$"
> +            - pattern: "^mio_clk([0-6][0-9]|7[0-7])+.*$"
>              - pattern: "gem[0-3]+_emio_clk.*$"
>              - pattern: "swdt[0-1]+_ext_clk.*$"

[Severity: High]
This is a pre-existing issue and was not introduced by the patch under review,
but does the current allOf schema make it impossible to validate
xlnx,versal-net-clk nodes?

Since xlnx,versal-net-clk uses xlnx,versal-clk as a fallback string, this
first conditional in Documentation/devicetree/bindings/clock/xlnx,versal-clk.yaml
will match:

  allOf:
    - if:
        properties:
          compatible:
            contains:
              enum:
                - xlnx,versal-clk
      then:
        properties:
          clocks:
            items:
              - description: reference clock
              - description: alternate reference clock for programmable logic

This restricts the node to exactly 2 clocks. However, because both conditions
are processed, the following conditional for xlnx,versal-net-clk also matches:

    - if:
        properties:
          compatible:
            contains:
              enum:
                - xlnx,versal-net-clk
      then:
        properties:
          clocks:
            items:
              - description: reference clock
              - description: alternate reference clock for programmable logic
              - description: alternate reference clock

This applies a 3-item length constraint. Could this cause the schema to
unconditionally reject valid xlnx,versal-net-clk nodes since it forces the
node to have exactly 2 and exactly 3 clocks simultaneously?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1783495122.git.michal.simek@amd.com?part=2