Re: [PATCH 05/15] dt-bindings: clock: mediatek: Add MT8189 clocks

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The `else` clause meant to disallow `#reset-cells` contains a typo, specifying `reset-cells: false` instead of `'#reset-cells': false`.
- [Medium] The conditional blocks for `#clock-cells` and `#reset-cells` incorrectly use `enum:` instead of `contains: enum:` to match the `compatible` property.
--

commit 0801644b64f2bb225fa62c5b58a27baad9d2d00e
Author: Louis-Alexis Eyraud <louisalexis.eyraud@collabora.com>

dt-bindings: clock: mediatek: Add MT8189 clocks
    
Add dt schema and IDs for the clocks of MediaTek MT8189 SoC.
The MT8189 clock IP provide clock control for main system
(apmixedsys, topcksys and vlpcksys) and subsys (eg. peri, scp,
ufs...).
    
Also, add compatible for frequency hopping and spread spectrum clock
functionality and reset controller header file for MT8189 UFS reset
controller support.

> diff --git a/Documentation/devicetree/bindings/clock/mediatek,mt8186-clock.yaml b/Documentation/devicetree/bindings/clock/mediatek,mt8186-clock.yaml
> index 84e602c7d3266..e30ed16f321d7 100644
> --- a/Documentation/devicetree/bindings/clock/mediatek,mt8186-clock.yaml
> +++ b/Documentation/devicetree/bindings/clock/mediatek,mt8186-clock.yaml
[ ... ]
> @@ -145,6 +158,19 @@ allOf:
>              - mediatek,mt8188-vencsys
>              - mediatek,mt8188-wpesys
>              - mediatek,mt8188-wpesys-vpp0
> +            - mediatek,mt8189-dbg-ao
> +            - mediatek,mt8189-dem
> +            - mediatek,mt8189-dvfsrc-top
> +            - mediatek,mt8189-iic-wrap-e
> +            - mediatek,mt8189-iic-wrap-en
> +            - mediatek,mt8189-iic-wrap-s
> +            - mediatek,mt8189-iic-wrap-ws
> +            - mediatek,mt8189-scp-clk
> +            - mediatek,mt8189-scp-i2c-clk
> +            - mediatek,mt8189-ufscfg-ao
> +            - mediatek,mt8189-ufscfg-pdn
> +            - mediatek,mt8189-vlpcfg
> +            - mediatek,mt8189-vlpcfg-ao
>      then:
>        required:
>          - '#clock-cells'

[Severity: Medium]
Does the if condition for this block incorrectly use enum: instead of
contains: enum: to match the compatible property?

Because the device tree compatible property is an array of strings, enum:
will attempt to match the entire array against a single string, which fails.
This causes the schema to silently skip enforcing the required #clock-cells
property for the listed compatibles, including the newly added mt8189 variants.

[ ... ]
> @@ -163,6 +189,8 @@ allOf:
>              - mediatek,mt8188-imgsys1-dip-nr
>              - mediatek,mt8188-imgsys1-dip-top
>              - mediatek,mt8188-ipesys
> +            - mediatek,mt8189-ufscfg-ao
> +            - mediatek,mt8189-ufscfg-pdn
>      then:
>        required:
>          - '#reset-cells'

[Severity: Medium]
This block has the same matching issue for #reset-cells. Should this also use
contains: enum: instead of enum:?

Additionally, looking just below this block at the else clause:

    else:
      properties:
        reset-cells: false

Is there a typo in the property name here? It specifies reset-cells: false
instead of '#reset-cells': false.

This allows schema validation to pass even if device nodes incorrectly define
#reset-cells when they are not reset providers, because the constraint
targets a non-existent property.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260701-mt8189-clocks-system-base-v1-0-2b048feea50a@collabora.com?part=5