Re: [PATCH v8 1/2] dt-bindings: clock: add TI CDCE6214 binding

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [Medium] Conditional constraints in the 'allOf' block target the non-existent property 'io-standard' instead of 'ti,io-standard', bypassing validation.
- [Low] The description for 'ti,cmosp-mode' incorrectly refers to 'CMOSN output' and contains typos.
- [Low] The 'ti,cmosp-mode' and 'ti,cmosn-mode' properties lack a minimum constraint, incorrectly allowing the value 0.
- [Low] The 'clock-names' property incorrectly permits an array with duplicate clock names (e.g., ['secref', 'secref']).
--

commit 3a564f8b01873582c4ee765638671ebe3d3d19b4
Author: Sascha Hauer <s.hauer@pengutronix.de>

dt-bindings: clock: add TI CDCE6214 binding
    
Add device tree binding for the CDCE6214, an Ultra-Low Power Clock
Generator With One PLL, Four Differential Outputs, Two Inputs, and
Internal EEPROM.

> diff --git a/Documentation/devicetree/bindings/clock/ti,cdce6214.yaml b/Documentation/devicetree/bindings/clock/ti,cdce6214.yaml
> new file mode 100644
> index 0000000000000..4a9c9155894af
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/ti,cdce6214.yaml
[ ... ]
> +  clock-names:
> +    minItems: 1
> +    items:
> +      - enum: [ priref, secref ]
> +      - const: secref

[Severity: Low]
Could this allow the clock-names array to have duplicate entries like
"secref", "secref"? Should the first item be restricted to just priref
to prevent defining the same physical clock reference twice?

[ ... ]
> +          ti,cmosp-mode:
> +            description: |
> +              Driving mode for CMOSN output:
> +              1: Low Polarity
> +              2: High Polrity
> +              3: Disable

[Severity: Low]
There appear to be a couple of typos in this description. It refers to
CMOSN output instead of CMOSP, and Polrity instead of Polarity.

> +            $ref: /schemas/types.yaml#/definitions/uint32
> +            maximum: 3

[Severity: Low]
Since this references uint32 with a maximum of 3, does it inadvertently
allow 0 as a valid state?

According to the description and the driver header, 0 is not a valid
mode. Would it be better to use an enum of [1, 2, 3] or define a
minimum of 1?

> +
> +          ti,cmosn-mode:
> +            description: |
> +              Driving mode for CMOSN output:
> +              1: Low Polarity
> +              2: High Polrity
> +              3: Disable
> +            $ref: /schemas/types.yaml#/definitions/uint32
> +            maximum: 3

[Severity: Low]
Similarly, does this property need a minimum constraint to prevent 0
from passing schema validation?

[ ... ]
> +        allOf:
> +          - if:
> +              properties:
> +                pins:
> +                  contains:
> +                    const: priref
> +            then:
> +              properties:
> +                io-standard:
> +                  enum: [ 1, 5 ]

[Severity: Medium]
Do these constraints target the non-existent property io-standard instead
of ti,io-standard?

If so, an invalid ti,io-standard value in a device tree might erroneously
pass schema validation because the constraints aren't applied to it.
This pattern appears in the subsequent conditional blocks as well.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260909-clk-cdce6214-v8-0-4dfa355c1f65@pengutronix.de?part=1