The MediaTek MT8196 SoC contains the same UFS host controller interface
hardware as the MT8195 SoC. Add it as a variant of MT8195, and extend
its list of allowed clocks, as well as give it the previously absent
resets property.

Also add examples for both MT8195 and the new MT8196, so that the
binding can be verified against examples for these two variants.

Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 .../devicetree/bindings/ufs/mediatek,ufs.yaml      | 134 +++++++++++++++++++--
 1 file changed, 123 insertions(+), 11 deletions(-)

diff --git a/Documentation/devicetree/bindings/ufs/mediatek,ufs.yaml b/Documentation/devicetree/bindings/ufs/mediatek,ufs.yaml
index 1dec54fb00f3..070ae0982591 100644
--- a/Documentation/devicetree/bindings/ufs/mediatek,ufs.yaml
+++ b/Documentation/devicetree/bindings/ufs/mediatek,ufs.yaml
@@ -11,18 +11,30 @@ maintainers:
 
 properties:
   compatible:
-    enum:
-      - mediatek,mt8183-ufshci
-      - mediatek,mt8192-ufshci
-      - mediatek,mt8195-ufshci
+    oneOf:
+      - enum:
+          - mediatek,mt8183-ufshci
+          - mediatek,mt8195-ufshci
+      - items:
+          - enum:
+              - mediatek,mt8192-ufshci
+          - const: mediatek,mt8183-ufshci
+      - items:
+          - enum:
+              - mediatek,mt8196-ufshci
+          - const: mediatek,mt8195-ufshci
 
   clocks:
     minItems: 1
-    maxItems: 8
+    maxItems: 16
 
   clock-names:
     minItems: 1
-    maxItems: 8
+    maxItems: 16
+
+  freq-table-hz: true
+
+  interrupts: true
 
   phys:
     maxItems: 1
@@ -30,7 +42,15 @@ properties:
   reg:
     maxItems: 1
 
+  resets:
+    maxItems: 3
+
+  reset-names:
+    maxItems: 3
+
   vcc-supply: true
+  vccq-supply: true
+  vccq2-supply: true
 
   mediatek,ufs-disable-mcq:
     $ref: /schemas/types.yaml#/definitions/flag
@@ -44,22 +64,19 @@ required:
   - reg
   - vcc-supply
 
-unevaluatedProperties: false
-
 allOf:
   - $ref: ufs-common.yaml
-
   - if:
       properties:
         compatible:
           contains:
-            enum:
-              - mediatek,mt8195-ufshci
+            const: mediatek,mt8195-ufshci
     then:
       properties:
         clocks:
           minItems: 8
         clock-names:
+          minItems: 8
           items:
             - const: ufs
             - const: ufs_aes
@@ -69,6 +86,19 @@ allOf:
             - const: unipro_mp_bclk
             - const: ufs_tx_symbol
             - const: ufs_mem_sub
+            - const: crypt_mux
+            - const: crypt_lp
+            - const: crypt_perf
+            - const: ufs_sel
+            - const: ufs_sel_min_src
+            - const: ufs_sel_max_src
+            - const: ufs_rx_symbol0
+            - const: ufs_rx_symbol1
+        reset-names:
+          items:
+            - const: unipro_rst
+            - const: crypto_rst
+            - const: hci_rst
     else:
       properties:
         clocks:
@@ -76,6 +106,10 @@ allOf:
         clock-names:
           items:
             - const: ufs
+        resets: false
+        reset-names: false
+
+unevaluatedProperties: false
 
 examples:
   - |
@@ -99,3 +133,81 @@ examples:
             vcc-supply = <&mt_pmic_vemc_ldo_reg>;
         };
     };
+  - |
+    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>;
+        clock-names = "ufs", "ufs_aes", "ufs_tick",
+                      "unipro_sysclk", "unipro_tick",
+                      "unipro_mp_bclk", "ufs_tx_symbol",
+                      "ufs_mem_sub";
+        freq-table-hz = <0 0>, <0 0>, <0 0>,
+                        <0 0>, <0 0>, <0 0>,
+                        <0 0>, <0 0>;
+        vcc-supply = <&mt6359_vemc_1_ldo_reg>;
+        mediatek,ufs-disable-mcq;
+    };
+  - |
+    #include <dt-bindings/reset/mediatek,mt8196-resets.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+    ufshci@16810000 {
+        compatible = "mediatek,mt8196-ufshci", "mediatek,mt8195-ufshci";
+        reg = <0x16810000 0x2a00>;
+        interrupts = <GIC_SPI 320 IRQ_TYPE_LEVEL_HIGH>;
+
+        clocks = <&ufs_ao_clk 6>,
+                 <&ufs_ao_clk 7>,
+                 <&clk26m>,
+                 <&ufs_ao_clk 3>,
+                 <&clk26m>,
+                 <&ufs_ao_clk 4>,
+                 <&ufs_ao_clk 0>,
+                 <&topckgen 7>,
+                 <&topckgen 41>,
+                 <&topckgen 105>,
+                 <&topckgen 83>,
+                 <&topckgen 42>,
+                 <&topckgen 84>,
+                 <&topckgen 102>,
+                 <&ufs_ao_clk 1>,
+                 <&ufs_ao_clk 2>;
+        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_sel",
+                      "ufs_sel_min_src",
+                      "ufs_sel_max_src",
+                      "ufs_rx_symbol0",
+                      "ufs_rx_symbol1";
+
+        freq-table-hz = <273000000 499200000>, <0 0>, <0 0>, <0 0>, <0 0>,
+                        <0 0>, <0 0>, <0 0>, <0 0>, <0 0>, <0 0>, <0 0>, <0 0>,
+                        <0 0>;
+
+        phys = <&ufsphy>;
+
+        vcc-supply = <&mt6363_vemc>;
+        vccq-supply = <&mt6363_vufs12>;
+        vccq2-supply = <&mt6363_vufs18>;
+
+        resets = <&ufs_ao_clk MT8196_UFSAO_RST1_UFS_UNIPRO>,
+                 <&ufs_ao_clk MT8196_UFSAO_RST1_UFS_CRYPTO>,
+                 <&ufs_ao_clk MT8196_UFSAO_RST1_UFSHCI>;
+        reset-names = "unipro_rst", "crypto_rst", "hci_rst";
+        mediatek,ufs-disable-mcq;
+    };

-- 
2.51.0

Hey,

On Tue, Oct 14, 2025 at 05:10:05PM +0200, Nicolas Frattaroli wrote:
> The MediaTek MT8196 SoC contains the same UFS host controller interface
> hardware as the MT8195 SoC. Add it as a variant of MT8195, and extend
> its list of allowed clocks, as well as give it the previously absent
> resets property.
> 
> Also add examples for both MT8195 and the new MT8196, so that the
> binding can be verified against examples for these two variants.
> 
> Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> ---
>  .../devicetree/bindings/ufs/mediatek,ufs.yaml      | 134 +++++++++++++++++++--
>  1 file changed, 123 insertions(+), 11 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/ufs/mediatek,ufs.yaml b/Documentation/devicetree/bindings/ufs/mediatek,ufs.yaml
> index 1dec54fb00f3..070ae0982591 100644
> --- a/Documentation/devicetree/bindings/ufs/mediatek,ufs.yaml
> +++ b/Documentation/devicetree/bindings/ufs/mediatek,ufs.yaml
> @@ -11,18 +11,30 @@ maintainers:
>  
>  properties:
>    compatible:
> -    enum:
> -      - mediatek,mt8183-ufshci
> -      - mediatek,mt8192-ufshci
> -      - mediatek,mt8195-ufshci
> +    oneOf:
> +      - enum:
> +          - mediatek,mt8183-ufshci
> +          - mediatek,mt8195-ufshci
> +      - items:
> +          - enum:
> +              - mediatek,mt8192-ufshci
> +          - const: mediatek,mt8183-ufshci

It's hard to follow what's going on in this commit.
Firstly, this seems to be some sort of unrelated change that isn't
mentioned in the commit message.

> +      - items:
> +          - enum:
> +              - mediatek,mt8196-ufshci
> +          - const: mediatek,mt8195-ufshci
>  
>    clocks:
>      minItems: 1
> -    maxItems: 8
> +    maxItems: 16
>  
>    clock-names:
>      minItems: 1
> -    maxItems: 8
> +    maxItems: 16

Then all devices grow 8 more permitted clocks, despite the wording in
the commit message being 8195 specific. (Hint: you missed maxItems: 8 in
the else)

> +
> +  freq-table-hz: true

Then you add this deprecated property, which isn't mentioned in the
commit message and I don't see why a deprecated property is needed.

> +
> +  interrupts: true
>  
>    phys:
>      maxItems: 1
> @@ -30,7 +42,15 @@ properties:
>    reg:
>      maxItems: 1
>  
> +  resets:
> +    maxItems: 3
> +
> +  reset-names:
> +    maxItems: 3

You cannot use reset-names if you don't define what the names are.
Please provide a items list with descriptions in resets and some
names in reset-names.

>    vcc-supply: true
> +  vccq-supply: true
> +  vccq2-supply: true

And then two new supplies that are not mentioned in the commit message,
and again are allowed for all variants. The commit message talks about
extended 8195 features, so this is starting to look like there was some
sort of squashing accident.

>  
>    mediatek,ufs-disable-mcq:
>      $ref: /schemas/types.yaml#/definitions/flag
> @@ -44,22 +64,19 @@ required:
>    - reg
>    - vcc-supply
>  
> -unevaluatedProperties: false
> -
>  allOf:
>    - $ref: ufs-common.yaml
> -
>    - if:
>        properties:
>          compatible:
>            contains:
> -            enum:
> -              - mediatek,mt8195-ufshci
> +            const: mediatek,mt8195-ufshci

The commit message says:
| hardware as the MT8195 SoC. Add it as a variant of MT8195, and extend
| its list of allowed clocks, as well as give it the previously absent
| resets property.

I don't know if that's meant to mean that only the new device has 16 and
the 8195 only has 8, or if the 8195 should have had 16 possible clocks
too.

>      then:
>        properties:
>          clocks:
>            minItems: 8
>          clock-names:
> +          minItems: 8
>            items:
>              - const: ufs
>              - const: ufs_aes
> @@ -69,6 +86,19 @@ allOf:
>              - const: unipro_mp_bclk
>              - const: ufs_tx_symbol
>              - const: ufs_mem_sub
> +            - const: crypt_mux
> +            - const: crypt_lp
> +            - const: crypt_perf
> +            - const: ufs_sel
> +            - const: ufs_sel_min_src
> +            - const: ufs_sel_max_src
> +            - const: ufs_rx_symbol0
> +            - const: ufs_rx_symbol1
> +        reset-names:
> +          items:
> +            - const: unipro_rst
> +            - const: crypto_rst
> +            - const: hci_rst
>      else:
>        properties:
>          clocks:
> @@ -76,6 +106,10 @@ allOf:
>          clock-names:
>            items:
>              - const: ufs
> +        resets: false
> +        reset-names: false
> +
> +unevaluatedProperties: false
>  
>  examples:
>    - |
> @@ -99,3 +133,81 @@ examples:
>              vcc-supply = <&mt_pmic_vemc_ldo_reg>;
>          };
>      };
> +  - |
> +    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>;
> +        clock-names = "ufs", "ufs_aes", "ufs_tick",
> +                      "unipro_sysclk", "unipro_tick",
> +                      "unipro_mp_bclk", "ufs_tx_symbol",
> +                      "ufs_mem_sub";
> +        freq-table-hz = <0 0>, <0 0>, <0 0>,
> +                        <0 0>, <0 0>, <0 0>,
> +                        <0 0>, <0 0>;
> +        vcc-supply = <&mt6359_vemc_1_ldo_reg>;
> +        mediatek,ufs-disable-mcq;
> +    };
> +  - |
> +    #include <dt-bindings/reset/mediatek,mt8196-resets.h>
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> +    ufshci@16810000 {
> +        compatible = "mediatek,mt8196-ufshci", "mediatek,mt8195-ufshci";
> +        reg = <0x16810000 0x2a00>;
> +        interrupts = <GIC_SPI 320 IRQ_TYPE_LEVEL_HIGH>;
> +
> +        clocks = <&ufs_ao_clk 6>,
> +                 <&ufs_ao_clk 7>,
> +                 <&clk26m>,
> +                 <&ufs_ao_clk 3>,
> +                 <&clk26m>,
> +                 <&ufs_ao_clk 4>,
> +                 <&ufs_ao_clk 0>,
> +                 <&topckgen 7>,
> +                 <&topckgen 41>,
> +                 <&topckgen 105>,
> +                 <&topckgen 83>,
> +                 <&topckgen 42>,
> +                 <&topckgen 84>,
> +                 <&topckgen 102>,
> +                 <&ufs_ao_clk 1>,
> +                 <&ufs_ao_clk 2>;
> +        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_sel",
> +                      "ufs_sel_min_src",
> +                      "ufs_sel_max_src",
> +                      "ufs_rx_symbol0",
> +                      "ufs_rx_symbol1";
> +
> +        freq-table-hz = <273000000 499200000>, <0 0>, <0 0>, <0 0>, <0 0>,
> +                        <0 0>, <0 0>, <0 0>, <0 0>, <0 0>, <0 0>, <0 0>, <0 0>,
> +                        <0 0>;
> +
> +        phys = <&ufsphy>;
> +
> +        vcc-supply = <&mt6363_vemc>;
> +        vccq-supply = <&mt6363_vufs12>;
> +        vccq2-supply = <&mt6363_vufs18>;
> +
> +        resets = <&ufs_ao_clk MT8196_UFSAO_RST1_UFS_UNIPRO>,
> +                 <&ufs_ao_clk MT8196_UFSAO_RST1_UFS_CRYPTO>,
> +                 <&ufs_ao_clk MT8196_UFSAO_RST1_UFSHCI>;
> +        reset-names = "unipro_rst", "crypto_rst", "hci_rst";

Putting _rst in the name of a reset is redundant.

pw-bot: changes-requested

Thanks,
Conor.
> +        mediatek,ufs-disable-mcq;
> +    };
> 
> -- 
> 2.51.0
> 

On Thu, Oct 16, 2025 at 05:53:01PM +0100, Conor Dooley wrote:
> On Tue, Oct 14, 2025 at 05:10:05PM +0200, Nicolas Frattaroli wrote:

> > @@ -30,7 +42,15 @@ properties:
> >    reg:
> >      maxItems: 1
> >  
> > +  resets:
> > +    maxItems: 3
> > +
> > +  reset-names:
> > +    maxItems: 3
> 
> You cannot use reset-names if you don't define what the names are.
> Please provide a items list with descriptions in resets and some
> names in reset-names.

I missed that the names were provided in the if/then/else. Please just
define them here, since there's currently only one possible set and use
the if/then/else to block their use on !8195

On Tue, 2025-10-14 at 17:10 +0200, Nicolas Frattaroli wrote:
> +  resets:
> +    maxItems: 3
> +
> +  reset-names:
> +    maxItems: 3
> +

Hi, Nicolas,

The maximum value should be 4, as there may be an mphy_reset.
For reference, please see:
https://elixir.bootlin.com/linux/v6.17.1/source/drivers/ufs/host/ufs-mediatek.c#L211

> 
> +        reset-names:
> +          items:
> +            - const: unipro_rst
> +            - const: crypto_rst
> +            - const: hci_rst
> 

Add mphy_reset too.


Thanks
Peter

On Thu, 2025-10-16 at 10:59 +0800, peter.wang wrote:
> Hi, Nicolas,
> 
> The maximum value should be 4, as there may be an mphy_reset.
> For reference, please see:
> https://elixir.bootlin.com/linux/v6.17.1/source/drivers/ufs/host/ufs-mediatek.c#L211
> 
> > 
> > +        reset-names:
> > +          items:
> > +            - const: unipro_rst
> > +            - const: crypto_rst
> > +            - const: hci_rst
> > 
> 
> Add mphy_reset too.
> 
> 
> Thanks
> Peter

Hi, Nicolas,

Please ignore this email, as I noticed that you added 
mphy_reset in the next patch.

Thanks
Peter