Re: [PATCH v13 02/24] dt-bindings: ufs: mediatek,ufs: Complete the binding

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] dt-bindings: ufs: mediatek: brittle schema constraint misrepresents hardware clocks
--

commit 52701a7e111f7103b8f873c8ff58dfce872a6586
Author: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>

dt-bindings: ufs: mediatek,ufs: Complete the binding

This commit updates the Mediatek UFS binding to make it more complete,
removing deprecated properties and updating the clocks and resets lists.
It attempts to set clock constraints for different SoC generations such as
MT8183, MT8192, and MT8195.

> diff --git a/Documentation/devicetree/bindings/ufs/mediatek,ufs.yaml b/Documentation/devicetree/bindings/ufs/mediatek,ufs.yaml
> index 15c347f5e6604..945cb986b06b7 100644
> --- a/Documentation/devicetree/bindings/ufs/mediatek,ufs.yaml
> +++ b/Documentation/devicetree/bindings/ufs/mediatek,ufs.yaml

[ ... ]

> @@ -54,29 +96,41 @@ allOf:
>        properties:
>          compatible:
>            contains:
> -            enum:
> -              - mediatek,mt8195-ufshci
> +            const: mediatek,mt8183-ufshci
>      then:
>        properties:
>          clocks:
> -          minItems: 8
> +          maxItems: 1

[Severity: Low]
Does this arbitrarily restrict MT8183 to a single clock, ignoring its UniPro
and AES clocks? Because the global items list enforces positional ordering,
if a device tree attempts to add the physically present UFS-related clocks
for this SoC, schema validation will reject the changes due to this maxItems
limit.

>          clock-names:
>            items:
> -            - const: ufs
> -            - const: ufs_aes
> -            - const: ufs_tick
> -            - const: unipro_sysclk
> -            - const: unipro_tick
> -            - const: unipro_mp_bclk
> -            - const: ufs_tx_symbol
> -            - const: ufs_mem_sub
> -    else:
> +            - const: main
> +        avdd12-ckbuf-supply: false
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: mediatek,mt8192-ufshci
> +    then:
>        properties:
>          clocks:
> -          maxItems: 1
> +          minItems: 3
> +          maxItems: 3

[Severity: Low]
Does this restrict MT8192 to 3 clocks, explicitly blocking proper hardware
representation? Since the newly introduced global items list strictly
enforces positional ordering, a device with maxItems: 3 is forced to provide
specifically "main", "aes", and "tick".

If MT8192 hardware actually implements UFS UniPro clocks, as evidenced by
this clock definition in drivers/clk/mediatek/clk-mt8192.c:

infra_clks:
    GATE_INFRA2(CLK_INFRA_UNIPRO_SYS, "infra_unipro_sys", "ufs_sel", 11),

wouldn't this rule prevent accurately describing these clocks in the device
tree?

>          clock-names:
> -          items:
> -            - const: ufs
> +          minItems: 3
> +          maxItems: 3

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261001-mt8196-ufs-v13-0-60854adab6d9@collabora.com?part=2