Re: [PATCH v4] dt-bindings: clock: via,vt8500: Convert to DT Schema

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] The `patternProperties` regex enforces strictly lowercase node names, inadvertently rejecting standard `fixed-clock` nodes unless they violate DT naming conventions by including uppercase letters to bypass validation.
- [Medium] The patch defines a new schema for `via,vt8500-pmc` but fails to delete the existing legacy text binding for the same compatible string.
- [Low] The `divisor-mask` property lacks a `dependentRequired` constraint to enforce that it can only be used when `divisor-reg` is also present.
--

commit f63cd62ab71a54ca61f786bce852286bf98a7852
Author: Udaya Kiran Challa <challauday369@gmail.com>

dt-bindings: clock: via,vt8500: Convert to DT Schema
    
Convert the VIA/Wondermedia VT8500 and Wondermedia WM8xxx series SoCs clock
controller binding from the legacy text format to DT schema.

> diff --git a/Documentation/devicetree/bindings/clock/via,vt8500-clock.yaml b/Documentation/devicetree/bindings/clock/via,vt8500-clock.yaml
> new file mode 100644
> index 0000000000000..51a68df6c2f30
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/via,vt8500-clock.yaml

[Severity: Medium]
Should this patch also delete the existing text binding for this compatible
located at Documentation/devicetree/bindings/arm/vt8500/via,vt8500-pmc.txt?
Leaving the old text binding intact causes duplicate and fragmented
documentation for the exact same compatible string.

[ ... ]
> +    patternProperties:
> +      "^[a-z0-9-]+(@[0-9a-f]+)?$":
> +        type: object
> +
> +        properties:
> +          compatible:
> +            enum:
> +              - via,vt8500-pll-clock
> +              - wm,wm8650-pll-clock
> +              - wm,wm8750-pll-clock
> +              - wm,wm8850-pll-clock
> +              - via,vt8500-device-clock

[Severity: High]
Does this regex unintentionally restrict compliant lowercase node names?

Any standard child node with a compliant lowercase name (like "ref24") will
match this pattern and be strictly restricted to the compatibles above, failing
if it contains properties like clock-frequency.

It appears the fixed-clock examples in this patch only pass validation because
their node names include uppercase letters (e.g., "ref24M"), which evades the
regex entirely and falls through to additionalProperties.

[ ... ]
> +          divisor-reg:
> +            $ref: /schemas/types.yaml#/definitions/uint32
> +            description:
> +              Offset of the clock divisor register within the PMC
> +              register space.
> +
> +          divisor-mask:
> +            $ref: /schemas/types.yaml#/definitions/uint32
> +            default: 0x1f
> +            description:
> +              Bitmask describing the divisor field inside divisor-reg.
> +
> +        dependentRequired:
> +          enable-reg:
> +            - enable-bit
> +
> +          enable-bit:
> +            - enable-reg

[Severity: Low]
Should this section also include a constraint to enforce that divisor-mask
can only be used when divisor-reg is present? The legacy text binding indicates
the mask is semantically tied to the divisor register.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260524151110.21277-1-challauday369@gmail.com?part=1