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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Changing clock names and constraints breaks DT validation for the existing in-tree user `mt8195.dtsi` and omits crypt clocks expected by the driver.
- [Low] Missing `#include <dt-bindings/interrupt-controller/arm-gic.h>` in the newly added device tree example.
--

commit 043addd7d18e6896e3ee286c9ea8d5a48e6e340a
Author: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>

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

This commit updates the mediatek,ufs.yaml binding to make it more complete.
It disallows the deprecated freq-table-hz property, adds reset controls and
power supplies, modifies clock names by removing the ufs_ prefix, and
updates constraints for MT8183, MT8192, and MT8195. It also adds a second
example for 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

[ ... ]

> @@ -19,11 +19,25 @@ properties:
>  
>    clocks:
>      minItems: 1
> -    maxItems: 8
> +    maxItems: 10
>  
>    clock-names:
>      minItems: 1
> -    maxItems: 8
> +    items:
> +      - const: main
> +      - const: aes
> +      - const: tick
> +      - const: unipro_sysclk
> +      - const: unipro_tick
> +      - const: unipro_mp_bclk
> +      - const: tx_symbol
> +      - const: mem_sub
> +      - const: rx_symbol0
> +      - const: rx_symbol1
> +
> +  operating-points-v2: true
> +
> +  freq-table-hz: false

[Severity: Medium]
Will this change cause dtbs_check validation failures for existing in-tree
device trees?

The in-tree arch/arm64/boot/dts/mediatek/mt8195.dtsi still uses the old
clock names and the freq-table-hz property:

arch/arm64/boot/dts/mediatek/mt8195.dtsi:ufshci() {
    ...
    compatible = "mediatek,mt8195-ufshci";
    ...
    freq-table-hz = <0 0>, <0 0>, <0 0>;
    ...
}

Since this patch explicitly disallows freq-table-hz and renames the clocks
without updating the dtsi, running make dtbs_check against the unchanged
dtsi will fail.

Additionally, does this list of clock names omit the crypt clocks? The
driver actively requests the crypt_mux, crypt_lp, and crypt_perf clocks
(along with the mediatek,ufs-boost-crypt property) in
ufs_mtk_init_boost_crypt().

[ ... ]

> @@ -94,9 +148,32 @@ examples:
>              phys = <&ufsphy>;
>  
>              clocks = <&infracfg_ao CLK_INFRA_UFS>;
> -            clock-names = "ufs";
> -            freq-table-hz = <0 0>;
> +            clock-names = "main";
>  
>              vcc-supply = <&mt_pmic_vemc_ldo_reg>;
>          };
>      };
> +  - |
> +    ufshci@11270000 {
> +        compatible = "mediatek,mt8195-ufshci";
> +        reg = <0x11270000 0x2300>;
> +        interrupts = <GIC_SPI 137 IRQ_TYPE_LEVEL_HIGH>;

[Severity: Low]
Does this new example fail to compile with dt_binding_check?

The example uses the C preprocessor macros GIC_SPI and IRQ_TYPE_LEVEL_HIGH
without including the header file that defines them. Since dt-schema parses
each example block independently, a missing
#include <dt-bindings/interrupt-controller/arm-gic.h>
will cause dtc compilation to fail during make dt_binding_check.

> +        phys = <&ufsphy>;
> +        clocks = <&infracfg_ao 63>, <&infracfg_ao 64>, <&infracfg_ao 65>,
> +                 <&infracfg_ao 54>, <&infracfg_ao 55>, <&infracfg_ao 56>,
> +                 <&infracfg_ao 90>, <&infracfg_ao 93>, <&topckgen 212>,
> +                 <&topckgen 215>;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260901-mt8196-ufs-v11-0-dbd007704490@collabora.com?part=2