Re: [PATCH v8 01/10] dt-bindings: display: rockchip: analogix-dp: Fix hclk as third clock for RK3588

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The schema fails to enforce the per-SoC clock names for the third clock as claimed in the commit message.
--

commit 610de2ebf53101c37f8a390596d7553b1a60ba5c
Author: Damon Ding <damon.ding@rock-chips.com>

dt-bindings: display: rockchip: analogix-dp: Fix hclk as third clock for RK3588

This commit changes the analogix-dp binding to require the hclk clock
for the RK3588 eDP controller to make the dependency explicit. It attempts
to define per-SoC clock requirements, specifying 2 clocks for RK3288,
and 3 clocks for RK3399 (grf) and RK3588 (hclk).

> diff --git a/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml b/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml
> index d99b23b88cc59..a1ab7a77bdd35 100644
> --- a/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml
> +++ b/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml
> @@ -26,7 +26,9 @@ properties:
>      items:
>        - const: dp
>        - const: pclk
> -      - const: grf
> +      - enum:
> +          - grf
> +          - hclk
>  
>    power-domains:
>      maxItems: 1

[ ... ]

> @@ -60,6 +62,32 @@ required:
>  allOf:
>    - $ref: /schemas/display/bridge/analogix,dp.yaml#

[ ... ]

> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - rockchip,rk3399-edp
> +    then:
> +      properties:
> +        clocks:
> +          minItems: 3
> +        clock-names:
> +          minItems: 3
> +
>    - if:
>        properties:
>          compatible:
> @@ -68,6 +96,10 @@ allOf:
>                - rockchip,rk3588-edp
>      then:
>        properties:
> +        clocks:
> +          minItems: 3
> +        clock-names:
> +          minItems: 3

[Severity: Medium]
The commit message explicitly states the intent to enforce per-SoC clock-names
requirements, specifying that RK3399 requires grf as the third clock while
RK3588 requires hclk.

Does this implementation actually enforce the per-SoC clock names for the third
clock?

The conditional blocks only add minItems: 3 to the clock-names property without
explicitly defining the sequence of allowed values. Since the top-level
clock-names definition allows an enum of grf and hclk, could an
RK3399 node successfully validate while using hclk, or an RK3588 node
validate while using grf?

Should the then clauses explicitly define clock-names: items with the exact
sequence of const strings required for each specific SoC to fully enforce
these constraints?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260525125331.140059-1-damon.ding@rock-chips.com?part=1