Re: [PATCH v2 2/8] dt-bindings: devfreq: event: rockchip,dfi: add clocks to rk3588

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] dt-bindings: missing clock constraints for rk3568-dfi
- [Medium] dt-bindings: variant conditionals fail to restrict clock-names exact strings
--

commit 88c291fbc8ab68b7cfb2fa8d4a230dd27bd0f853
Author: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>

dt-bindings: devfreq: event: rockchip,dfi: add clocks to rk3588

This commit updates the Rockchip DFI binding to add clock definitions for
the rk3588 SoC. It modifies the global clock limits and introduces
conditional blocks to handle the differences between rk3399 and rk3588.

> diff --git a/Documentation/devicetree/bindings/devfreq/event/rockchip,dfi.yaml b/Documentation/devicetree/bindings/devfreq/event/rockchip,dfi.yaml
> index 50d3fabe958d5..3450e28b40208 100644
> --- a/Documentation/devicetree/bindings/devfreq/event/rockchip,dfi.yaml
> +++ b/Documentation/devicetree/bindings/devfreq/event/rockchip,dfi.yaml
> @@ -17,11 +17,18 @@ properties:
>        - rockchip,rk3588-dfi
>  
>    clocks:
> -    maxItems: 1
> +    minItems: 1
> +    maxItems: 4
>  
>    clock-names:
> +    minItems: 1
>      items:
> -      - const: pclk_ddr_mon
> +      - enum:
> +          - pclk_ddr_mon
> +          - pclk_ddr_mon_ch0
> +      - const: pclk_ddr_mon_ch1
> +      - const: pclk_ddr_mon_ch2
> +      - const: pclk_ddr_mon_ch3

[ ... ]

> -if:
> -  properties:
> -    compatible:
> -      contains:
> -        enum:
> -          - rockchip,rk3399-dfi
> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - rockchip,rk3399-dfi

[Severity: Medium]
Will this omission cause schema validation to be incomplete for rk3568-dfi?

The global clocks constraint was relaxed to maxItems: 4 to support rk3588,
but rk3568-dfi is omitted from these conditional blocks. Doesn't this mean
device trees for rk3568-dfi specifying up to 4 clocks will inadvertently pass
dtbs_check validation instead of being restricted to the historic maxItems: 1
limit?

> +    then:
> +      properties:
> +        clocks:
> +          maxItems: 1
> +        clock-names:
> +          maxItems: 1
> +      required:
> +        - clocks
> +        - clock-names

[Severity: Medium]
Does this setup allow incorrectly named clocks to pass validation for specific
variants?

Since the global clock-names definition uses an enum for the first item,
and this conditional block only restricts maxItems: 1, a device tree using
clock-names = "pclk_ddr_mon_ch0" will erroneously pass validation for
rk3399-dfi. The same applies for rk3588-dfi passing with "pclk_ddr_mon".

Should the variant-specific conditional blocks override the items array
to enforce the correct fixed string (const) for that specific variant?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260921-rockchip-dfi-cleanup-v2-0-43ff34ac8599@collabora.com?part=2