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

["Chaotian Jing (井朝天)" <Chaotian.Jing@mediatek.com>]

On Thu, 2025-10-23 at 21:49 +0200, Nicolas Frattaroli wrote:
> As it stands, the mediatek,ufs.yaml binding is startlingly
> incomplete.
> Its one example, which is the only real "user" of this binding in
> mainline, uses the deprecated freq-table-hz property.
> 
> The resets, of which there are three optional ones, are completely
> absent.
> 
> The clock description for MT8195 is incomplete, as is the one for
> MT8192. It's not known if the one clock binding for MT8183 is even
> correct, but I do not have access to the necessary code and
> documentation to find this out myself.
> 
> The power supply situation is not much better; the binding describes
> one
> required power supply, but uses a supply property from ufs-
> common.yaml
> that can be either 1.8V or 3.3V.
> 
> No second example is present in the binding, making verification
> difficult.
> 
> Disallow freq-table-hz and move to operating-points-v2. It's fine to
> break compatibility here, as the binding is currently unused and
> would
> be impossible to correctly use in its current state.
> 
> Add the three resets and the corresponding reset-names property.
> These
> resets appear to be optional, i.e. not required for the functioning
> of
> the device.
> 
> Move the list of clock names out of the if condition, and expand it
> for
> the confirmed clocks I could find by cross-referencing several clock
> drivers. For MT8195, increase the minimum number of clocks to include
> the crypt and rx_symbol ones, as they're internal to the SoC and
> should
> always be present, and should therefore not be omitted.
> 
> MT8192 gets to have at least 3 clocks, as these were the ones I could
> quickly confirm from a glance at various trees. I can't say this was
> an
> exhaustive search though, but it's better than the current situation.
> 
> Properly document all supplies, with which pin name on the SoCs they
> supply, and what voltage we understand them as. Mandate vcc-supply-
> 1p8,
> as vcc-supply appears to always be describing a 1.8V supply. The
> ufs-common.yaml vccq/vccq2 supplies are used for this purpose, so
> that
> common UFS implementations which do power management for these don't
> have to treat MediaTek's 1.2V supplies in a special way.
> 
> Add the missing avdd09-supply, which so far only mt8183 uses.
> 
> Also add a MT8195 example to the binding, using supply labels that I
> am
> pretty sure would be the right ones for e.g. the Radxa NIO 12L.
> 
> Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> ---
>  .../devicetree/bindings/ufs/mediatek,ufs.yaml      | 115
> +++++++++++++++++----
>  1 file changed, 97 insertions(+), 18 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/ufs/mediatek,ufs.yaml
> b/Documentation/devicetree/bindings/ufs/mediatek,ufs.yaml
> index 1dec54fb00f3..364672bc65b1 100644
> --- a/Documentation/devicetree/bindings/ufs/mediatek,ufs.yaml
> +++ b/Documentation/devicetree/bindings/ufs/mediatek,ufs.yaml
> @@ -18,11 +18,28 @@ properties:
>  
>    clocks:
>      minItems: 1
> -    maxItems: 8
> +    maxItems: 13
>  
>    clock-names:
>      minItems: 1
> -    maxItems: 8
> +    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
> +      - const: crypt_mux
> +      - const: crypt_lp
> +      - const: crypt_perf
> +      - const: ufs_rx_symbol0
> +      - const: ufs_rx_symbol1
> +
> +  operating-points-v2: true
> +
> +  freq-table-hz: false
>  
>    phys:
>      maxItems: 1
> @@ -30,7 +47,31 @@ properties:
>    reg:
>      maxItems: 1
>  
> -  vcc-supply: true
> +  resets:
> +    items:
> +      - description: reset for the UniPro layer
> +      - description: reset for the cryptography engine
> +      - description: reset for the host controller
> +
> +  reset-names:
> +    items:
> +      - const: unipro
> +      - const: crypto
> +      - const: hci
> +
> +  avdd09-supply:
> +    description: Phandle to the 0.9V supply powering the AVDD09_UFS
> pin
> +
> +  vcc-supply:
> +    description: Phandle to the 1.8V supply powering the AVDD18_UFS
> pin
> +
> +  vcc-supply-1p8: true
> +
> +  vccq-supply:
> +    description: Phandle to the 1.2V supply powering the AVDD12_UFS
> pin
> +
> +  vccq2-supply:
> +    description: Phandle to the 1.2V supply powering the
> AVDD12_CKBUF_UFS pin

The AVDD_xxx LDO is used for IC internally, and the vcc-supply/vccq-
supply/vccq2-supply are used for UFS device's power. so that it is
wrong setting the AVDD_xxx to these suppliers.
and, the vcc is 2.5V or 3.3V, not 1.8V.
>  
>    mediatek,ufs-disable-mcq:
>      $ref: /schemas/types.yaml#/definitions/flag
> @@ -43,6 +84,7 @@ required:
>    - phys
>    - reg
>    - vcc-supply
> +  - vcc-supply-1p8
>  
>  unevaluatedProperties: false
>  
> @@ -53,29 +95,41 @@ allOf:
>        properties:
>          compatible:
>            contains:
> -            enum:
> -              - mediatek,mt8195-ufshci
> +            const: mediatek,mt8183-ufshci
>      then:
>        properties:
>          clocks:
> -          minItems: 8
> +          maxItems: 1
>          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:
> +        vccq2-supply: false
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: mediatek,mt8192-ufshci
> +    then:
>        properties:
>          clocks:
> -          maxItems: 1
> +          minItems: 3
> +          maxItems: 3
> +        clocks-names:
> +          minItems: 3
> +          maxItems: 3
> +        avdd09-supply: false
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: mediatek,mt8195-ufshci
> +    then:
> +      properties:
> +        clocks:
> +          minItems: 13
>          clock-names:
> -          items:
> -            - const: ufs
> +          minItems: 13
> +        avdd09-supply: false
>  
>  examples:
>    - |
> @@ -94,8 +148,33 @@ examples:
>  
>              clocks = <&infracfg_ao CLK_INFRA_UFS>;
>              clock-names = "ufs";
> -            freq-table-hz = <0 0>;
>  
>              vcc-supply = <&mt_pmic_vemc_ldo_reg>;
> +            vcc-supply-1p8;
>          };
>      };
> +  - |
> +    ufshci@11270000 {
> +        compatible = "mediatek,mt8195-ufshci";
> +        reg = <0x11270000 0x2300>;
> +        interrupts = <GIC_SPI 137 IRQ_TYPE_LEVEL_HIGH>;
> +        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 60>, <&topckgen 152>,
> +                 <&topckgen 125>, <&topckgen 212>, <&topckgen 215>;
> +        clock-names = "ufs", "ufs_aes", "ufs_tick",
> +                      "unipro_sysclk", "unipro_tick",
> +                      "unipro_mp_bclk", "ufs_tx_symbol",
> +                      "ufs_mem_sub", "crypt_mux", "crypt_lp",
> +                      "crypt_perf", "ufs_rx_symbol0",
> "ufs_rx_symbol1";
> +
> +        operating-points-v2 = <&ufs_opp_table>;
> +
> +        vcc-supply = <&mt6359_vufs_ldo_reg>;
> +        vcc-supply-1p8;
> +        vccq-supply = <&mt6359_vrf12_ldo_reg>;
> +        vccq2-supply = <&mt6359_vbbck_ldo_reg>;
> +        mediatek,ufs-disable-mcq;
> +    };
>