[PATCH 0/5] MediaTek UFS Cleanup and MT8196 Enablement

[PATCH 0/5] MediaTek UFS Cleanup and MT8196 Enablement

[Nicolas Frattaroli]

In this series, the existing MediaTek UFS binding is expanded and
completed to correctly describe not just the existing compatibles, but
also to introduce a new compatible in the from of the MT8196 SoC.

The resets, which until now were completely absent from both the UFS
host controller binding and the UFS PHY binding, are introduced to both.
This also means the driver's undocumented and, in mainline, unused reset
logic is reworked. In particular, the PHY reset is no longer a reset of
the host controller node, but of the PHY node.

This means the host controller can reset the PHY through the common PHY
framework.

The resets remain optional.

Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
Nicolas Frattaroli (5):
      dt-bindings: ufs: mediatek,ufs: Add mt8196-ufshci variant
      dt-bindings: phy: Add mediatek,mt8196-ufsphy variant
      scsi: ufs: mediatek: Move MTK_SIP_UFS_CONTROL to mtk_sip_svc.h
      phy: mediatek: ufs: Add support for resets
      scsi: ufs: mediatek: Rework resets

 .../devicetree/bindings/phy/mediatek,ufs-phy.yaml  |  16 +++
 .../devicetree/bindings/ufs/mediatek,ufs.yaml      | 134 +++++++++++++++++++--
 drivers/phy/mediatek/phy-mtk-ufs.c                 |  71 +++++++++++
 drivers/ufs/host/ufs-mediatek-sip.h                |   9 --
 drivers/ufs/host/ufs-mediatek.c                    |  67 ++++++-----
 drivers/ufs/host/ufs-mediatek.h                    |   1 -
 include/linux/soc/mediatek/mtk_sip_svc.h           |   3 +
 7 files changed, 251 insertions(+), 50 deletions(-)
---
base-commit: 40a3abb0f3e5229996c8ef0498fc8d8a0c2bd64f
change-id: 20251014-mt8196-ufs-cec4b9a97e53

Best regards,
-- 
Nicolas Frattaroli <nicolas.frattaroli@collabora.com>

[PATCH 1/5] dt-bindings: ufs: mediatek,ufs: Add mt8196-ufshci variant

[Nicolas Frattaroli]

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

Re: [PATCH 1/5] dt-bindings: ufs: mediatek,ufs: Add mt8196-ufshci variant

[Peter Wang (王信友)]

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

Re: [PATCH 1/5] dt-bindings: ufs: mediatek,ufs: Add mt8196-ufshci variant

[Peter Wang (王信友)]

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

Re: [PATCH 1/5] dt-bindings: ufs: mediatek,ufs: Add mt8196-ufshci variant

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 8204 bytes --]

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
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 1/5] dt-bindings: ufs: mediatek,ufs: Add mt8196-ufshci variant

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 662 bytes --]

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[PATCH 2/5] dt-bindings: phy: Add mediatek,mt8196-ufsphy variant

[Nicolas Frattaroli]

The MediaTek MT8196 SoC includes an M-PHY compatible with the already
existing mt8183 binding.

However, one omission from the original binding was that all of these
variants may have an optional reset.

Add the new compatible, and also the resets property, with an example.

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

diff --git a/Documentation/devicetree/bindings/phy/mediatek,ufs-phy.yaml b/Documentation/devicetree/bindings/phy/mediatek,ufs-phy.yaml
index 3e62b5d4da61..f414aaa18997 100644
--- a/Documentation/devicetree/bindings/phy/mediatek,ufs-phy.yaml
+++ b/Documentation/devicetree/bindings/phy/mediatek,ufs-phy.yaml
@@ -26,6 +26,7 @@ properties:
       - items:
           - enum:
               - mediatek,mt8195-ufsphy
+              - mediatek,mt8196-ufsphy
           - const: mediatek,mt8183-ufsphy
       - const: mediatek,mt8183-ufsphy
 
@@ -42,6 +43,10 @@ properties:
       - const: unipro
       - const: mp
 
+  resets:
+    items:
+      - description: Optional UFS M-PHY reset.
+
   "#phy-cells":
     const: 0
 
@@ -65,5 +70,16 @@ examples:
         clock-names = "unipro", "mp";
         #phy-cells = <0>;
     };
+  - |
+    #include <dt-bindings/reset/mediatek,mt8196-resets.h>
+    ufs-phy@16800000 {
+        compatible = "mediatek,mt8196-ufsphy", "mediatek,mt8183-ufsphy";
+        reg = <0x16800000 0x10000>;
+        clocks = <&ufs_ao_clk 3>,
+                 <&ufs_ao_clk 5>;
+        clock-names = "unipro", "mp";
+        resets = <&ufs_ao_clk MT8196_UFSAO_RST0_UFS_MPHY>;
+        #phy-cells = <0>;
+    };
 
 ...

-- 
2.51.0

Re: [PATCH 2/5] dt-bindings: phy: Add mediatek,mt8196-ufsphy variant

[Peter Wang (王信友)]

On Tue, 2025-10-14 at 17:10 +0200, Nicolas Frattaroli wrote:
> The MediaTek MT8196 SoC includes an M-PHY compatible with the already
> existing mt8183 binding.
> 
> However, one omission from the original binding was that all of these
> variants may have an optional reset.
> 
> Add the new compatible, and also the resets property, with an
> example.
> 
> Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>

Reviewed-by: Peter Wang <peter.wang@mediatek.com>

[PATCH 3/5] scsi: ufs: mediatek: Move MTK_SIP_UFS_CONTROL to mtk_sip_svc.h

[Nicolas Frattaroli]

SMC commands used by multiple drivers need to live in a shared header
file somewhere to avoid code duplication. In order to rework the MPHY
reset control to be in the phy-mtk-ufs.c driver, both ufs-mediatek and
the phy driver need access to this command.

Move it to mtk_sip_svc.h, where other such command definitions already
live.

Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 drivers/ufs/host/ufs-mediatek-sip.h      | 1 -
 include/linux/soc/mediatek/mtk_sip_svc.h | 3 +++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/ufs/host/ufs-mediatek-sip.h b/drivers/ufs/host/ufs-mediatek-sip.h
index 7d17aedf6fb8..d627dfb4a766 100644
--- a/drivers/ufs/host/ufs-mediatek-sip.h
+++ b/drivers/ufs/host/ufs-mediatek-sip.h
@@ -11,7 +11,6 @@
 /*
  * SiP (Slicon Partner) commands
  */
-#define MTK_SIP_UFS_CONTROL               MTK_SIP_SMC_CMD(0x276)
 #define UFS_MTK_SIP_VA09_PWR_CTRL         BIT(0)
 #define UFS_MTK_SIP_DEVICE_RESET          BIT(1)
 #define UFS_MTK_SIP_CRYPTO_CTRL           BIT(2)
diff --git a/include/linux/soc/mediatek/mtk_sip_svc.h b/include/linux/soc/mediatek/mtk_sip_svc.h
index abe24a73ee19..9a6866912e81 100644
--- a/include/linux/soc/mediatek/mtk_sip_svc.h
+++ b/include/linux/soc/mediatek/mtk_sip_svc.h
@@ -28,4 +28,7 @@
 /* IOMMU related SMC call */
 #define MTK_SIP_KERNEL_IOMMU_CONTROL	MTK_SIP_SMC_CMD(0x514)
 
+/* UFS related SMC call */
+#define MTK_SIP_UFS_CONTROL		MTK_SIP_SMC_CMD(0x276)
+
 #endif

-- 
2.51.0

Re: [PATCH 3/5] scsi: ufs: mediatek: Move MTK_SIP_UFS_CONTROL to mtk_sip_svc.h

[Peter Wang (王信友)]

On Tue, 2025-10-14 at 17:10 +0200, Nicolas Frattaroli wrote:
> diff --git a/include/linux/soc/mediatek/mtk_sip_svc.h
> b/include/linux/soc/mediatek/mtk_sip_svc.h
> index abe24a73ee19..9a6866912e81 100644
> --- a/include/linux/soc/mediatek/mtk_sip_svc.h
> +++ b/include/linux/soc/mediatek/mtk_sip_svc.h
> @@ -28,4 +28,7 @@
>  /* IOMMU related SMC call */
>  #define MTK_SIP_KERNEL_IOMMU_CONTROL   MTK_SIP_SMC_CMD(0x514)
> 
> +/* UFS related SMC call */
> +#define MTK_SIP_UFS_CONTROL            MTK_SIP_SMC_CMD(0x276)
> +
>  #endif
> 
> --
> 2.51.0
> 


Hi Nicolas,

Please sort them according to the SIP command number sequence.

Thanks
Peter

[PATCH 4/5] phy: mediatek: ufs: Add support for resets

[Nicolas Frattaroli]

The MediaTek UFS PHY supports PHY resets. Until now, they've been
implemented in the UFS host driver. Since they were never documented in
the UFS HCI node's DT bindings, and no mainline DT uses it, it's fine if
it's moved to the correct location, which is the PHY driver.

Implement the MPHY reset logic in this driver and expose it through the
phy subsystem's reset op. The reset itself is optional, as judging by
other mainline devices that use this hardware, it's not required for the
device to function.

If no reset is present, the reset op returns -EOPNOTSUPP, which means
that the ufshci driver can detect it's present and not double sleep in
its own reset function, where it will call the phy reset.

Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 drivers/phy/mediatek/phy-mtk-ufs.c | 71 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 71 insertions(+)

diff --git a/drivers/phy/mediatek/phy-mtk-ufs.c b/drivers/phy/mediatek/phy-mtk-ufs.c
index 0cb5a25b1b7a..d77ba689ebc8 100644
--- a/drivers/phy/mediatek/phy-mtk-ufs.c
+++ b/drivers/phy/mediatek/phy-mtk-ufs.c
@@ -4,6 +4,7 @@
  * Author: Stanley Chu <stanley.chu@mediatek.com>
  */
 
+#include <linux/arm-smccc.h>
 #include <linux/clk.h>
 #include <linux/delay.h>
 #include <linux/io.h>
@@ -11,6 +12,8 @@
 #include <linux/module.h>
 #include <linux/phy/phy.h>
 #include <linux/platform_device.h>
+#include <linux/reset.h>
+#include <linux/soc/mediatek/mtk_sip_svc.h>
 
 #include "phy-mtk-io.h"
 
@@ -36,9 +39,17 @@
 
 #define UFSPHY_CLKS_CNT    2
 
+#define UFS_MTK_SIP_MPHY_CTRL       BIT(8)
+
+enum ufs_mtk_mphy_op {
+	UFS_MPHY_BACKUP = 0,
+	UFS_MPHY_RESTORE
+};
+
 struct ufs_mtk_phy {
 	struct device *dev;
 	void __iomem *mmio;
+	struct reset_control *reset;
 	struct clk_bulk_data clks[UFSPHY_CLKS_CNT];
 };
 
@@ -141,9 +152,59 @@ static int ufs_mtk_phy_power_off(struct phy *generic_phy)
 	return 0;
 }
 
+static int ufs_mtk_phy_ctrl(struct ufs_mtk_phy *phy, enum ufs_mtk_mphy_op op)
+{
+	struct arm_smccc_res res;
+
+	arm_smccc_smc(MTK_SIP_UFS_CONTROL, UFS_MTK_SIP_MPHY_CTRL, op,
+		      0, 0, 0, 0, 0, &res);
+
+	switch (res.a0) {
+	case SMCCC_RET_NOT_SUPPORTED:
+		return -EOPNOTSUPP;
+	case SMCCC_RET_INVALID_PARAMETER:
+		return -EINVAL;
+	default:
+		return 0;
+	}
+}
+
+static int ufs_mtk_phy_reset(struct phy *generic_phy)
+{
+	struct ufs_mtk_phy *phy = get_ufs_mtk_phy(generic_phy);
+	int ret;
+
+	if (!phy->reset)
+		return -EOPNOTSUPP;
+
+	ret = reset_control_assert(phy->reset);
+	if (ret)
+		return ret;
+
+	usleep_range(100, 110);
+
+	ret = reset_control_deassert(phy->reset);
+	if (ret)
+		return ret;
+
+	/*
+	 * To avoid double-sleep and other unintended side-effects in the ufshci
+	 * driver, don't return the phy_ctrl retval here, but just return -EPROTO.
+	 */
+	ret = ufs_mtk_phy_ctrl(phy, UFS_MPHY_RESTORE);
+	if (ret) {
+		dev_err(phy->dev, "UFS_MPHY_RESTORE SMC command failed: %pe\n",
+			ERR_PTR(ret));
+		return -EPROTO;
+	}
+
+	return 0;
+}
+
 static const struct phy_ops ufs_mtk_phy_ops = {
 	.power_on       = ufs_mtk_phy_power_on,
 	.power_off      = ufs_mtk_phy_power_off,
+	.reset          = ufs_mtk_phy_reset,
 	.owner          = THIS_MODULE,
 };
 
@@ -163,8 +224,18 @@ static int ufs_mtk_phy_probe(struct platform_device *pdev)
 	if (IS_ERR(phy->mmio))
 		return PTR_ERR(phy->mmio);
 
+	phy->reset = devm_reset_control_get_optional(dev, NULL);
+	if (IS_ERR(phy->reset))
+		return dev_err_probe(dev, PTR_ERR(phy->reset), "Failed to get reset\n");
+
 	phy->dev = dev;
 
+	if (phy->reset) {
+		ret = ufs_mtk_phy_ctrl(phy, UFS_MPHY_BACKUP);
+		if (ret)
+			return dev_err_probe(dev, ret, "Failed to back up MPHY\n");
+	}
+
 	ret = ufs_mtk_phy_clk_init(phy);
 	if (ret)
 		return ret;

-- 
2.51.0

Re: [PATCH 4/5] phy: mediatek: ufs: Add support for resets

[Philipp Zabel]

On Di, 2025-10-14 at 17:10 +0200, Nicolas Frattaroli wrote:
> The MediaTek UFS PHY supports PHY resets. Until now, they've been
> implemented in the UFS host driver. Since they were never documented in
> the UFS HCI node's DT bindings, and no mainline DT uses it, it's fine if
> it's moved to the correct location, which is the PHY driver.
> 
> Implement the MPHY reset logic in this driver and expose it through the
> phy subsystem's reset op. The reset itself is optional, as judging by
> other mainline devices that use this hardware, it's not required for the
> device to function.
> 
> If no reset is present, the reset op returns -EOPNOTSUPP, which means
> that the ufshci driver can detect it's present and not double sleep in
> its own reset function, where it will call the phy reset.
> 
> Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>

Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>

regards
Philipp

[PATCH 5/5] scsi: ufs: mediatek: Rework resets

[Nicolas Frattaroli]

Rework the reset control getting in the driver's probe function to use
the "_optional" function instead of defaulting to NULL on IS_ERR, so
that actual real errors (as opposed to missing resets) can be handled as
errors in the probe function.

Also move the MPHY reset into the PHY driver, where it should live, and
remove all remnants of it ever having been in this driver.

Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 drivers/ufs/host/ufs-mediatek-sip.h |  8 -----
 drivers/ufs/host/ufs-mediatek.c     | 67 +++++++++++++++++++++----------------
 drivers/ufs/host/ufs-mediatek.h     |  1 -
 3 files changed, 38 insertions(+), 38 deletions(-)

diff --git a/drivers/ufs/host/ufs-mediatek-sip.h b/drivers/ufs/host/ufs-mediatek-sip.h
index d627dfb4a766..256598cc3b5b 100644
--- a/drivers/ufs/host/ufs-mediatek-sip.h
+++ b/drivers/ufs/host/ufs-mediatek-sip.h
@@ -31,11 +31,6 @@ enum ufs_mtk_vcc_num {
 	UFS_VCC_MAX
 };
 
-enum ufs_mtk_mphy_op {
-	UFS_MPHY_BACKUP = 0,
-	UFS_MPHY_RESTORE
-};
-
 /*
  * SMC call wrapper function
  */
@@ -84,9 +79,6 @@ static inline void _ufs_mtk_smc(struct ufs_mtk_smc_arg s)
 #define ufs_mtk_device_pwr_ctrl(on, ufs_version, res) \
 	ufs_mtk_smc(UFS_MTK_SIP_DEVICE_PWR_CTRL, &(res), on, ufs_version)
 
-#define ufs_mtk_mphy_ctrl(op, res) \
-	ufs_mtk_smc(UFS_MTK_SIP_MPHY_CTRL, &(res), op)
-
 #define ufs_mtk_mtcmos_ctrl(op, res) \
 	ufs_mtk_smc(UFS_MTK_SIP_MTCMOS_CTRL, &(res), op)
 
diff --git a/drivers/ufs/host/ufs-mediatek.c b/drivers/ufs/host/ufs-mediatek.c
index 758a393a9de1..ac40d4a3a800 100644
--- a/drivers/ufs/host/ufs-mediatek.c
+++ b/drivers/ufs/host/ufs-mediatek.c
@@ -204,49 +204,60 @@ static void ufs_mtk_crypto_enable(struct ufs_hba *hba)
 static void ufs_mtk_host_reset(struct ufs_hba *hba)
 {
 	struct ufs_mtk_host *host = ufshcd_get_variant(hba);
-	struct arm_smccc_res res;
+	int ret;
 
 	reset_control_assert(host->hci_reset);
 	reset_control_assert(host->crypto_reset);
 	reset_control_assert(host->unipro_reset);
-	reset_control_assert(host->mphy_reset);
 
-	usleep_range(100, 110);
+	ret = phy_reset(host->mphy);
+
+	/*
+	 * Only sleep if MPHY doesn't have a reset implemented (which already
+	 * sleeps) or the PHY reset function failed somehow, just to be safe
+	 */
+	if (ret) {
+		usleep_range(100, 110);
+		if (ret != -EOPNOTSUPP)
+			dev_warn(hba->dev, "PHY reset failed: %pe\n", ERR_PTR(ret));
+	}
 
 	reset_control_deassert(host->unipro_reset);
 	reset_control_deassert(host->crypto_reset);
 	reset_control_deassert(host->hci_reset);
-	reset_control_deassert(host->mphy_reset);
-
-	/* restore mphy setting aftre mphy reset */
-	if (host->mphy_reset)
-		ufs_mtk_mphy_ctrl(UFS_MPHY_RESTORE, res);
 }
 
-static void ufs_mtk_init_reset_control(struct ufs_hba *hba,
-				       struct reset_control **rc,
-				       char *str)
+static int ufs_mtk_init_reset_control(struct ufs_hba *hba,
+				      struct reset_control **rc,
+				      const char *str)
 {
-	*rc = devm_reset_control_get(hba->dev, str);
+	*rc = devm_reset_control_get_optional(hba->dev, str);
 	if (IS_ERR(*rc)) {
-		dev_info(hba->dev, "Failed to get reset control %s: %ld\n",
-			 str, PTR_ERR(*rc));
-		*rc = NULL;
+		dev_err(hba->dev, "Failed to get reset control %s: %pe\n", str, *rc);
+		return PTR_ERR(*rc);
 	}
+
+	return 0;
 }
 
-static void ufs_mtk_init_reset(struct ufs_hba *hba)
+static int ufs_mtk_init_reset(struct ufs_hba *hba)
 {
 	struct ufs_mtk_host *host = ufshcd_get_variant(hba);
+	int ret;
+
+	ret = ufs_mtk_init_reset_control(hba, &host->hci_reset, "hci_rst");
+	if (ret)
+		return ret;
+
+	ret = ufs_mtk_init_reset_control(hba, &host->unipro_reset, "unipro_rst");
+	if (ret)
+		return ret;
+
+	ret = ufs_mtk_init_reset_control(hba, &host->crypto_reset, "crypto_rst");
+	if (ret)
+		return ret;
 
-	ufs_mtk_init_reset_control(hba, &host->hci_reset,
-				   "hci_rst");
-	ufs_mtk_init_reset_control(hba, &host->unipro_reset,
-				   "unipro_rst");
-	ufs_mtk_init_reset_control(hba, &host->crypto_reset,
-				   "crypto_rst");
-	ufs_mtk_init_reset_control(hba, &host->mphy_reset,
-				   "mphy_rst");
+	return 0;
 }
 
 static int ufs_mtk_hce_enable_notify(struct ufs_hba *hba,
@@ -1238,11 +1249,9 @@ static int ufs_mtk_init(struct ufs_hba *hba)
 	if (err)
 		goto out_variant_clear;
 
-	ufs_mtk_init_reset(hba);
-
-	/* backup mphy setting if mphy can reset */
-	if (host->mphy_reset)
-		ufs_mtk_mphy_ctrl(UFS_MPHY_BACKUP, res);
+	err = ufs_mtk_init_reset(hba);
+	if (err)
+		goto out_variant_clear;
 
 	/* Enable runtime autosuspend */
 	hba->caps |= UFSHCD_CAP_RPM_AUTOSUSPEND;
diff --git a/drivers/ufs/host/ufs-mediatek.h b/drivers/ufs/host/ufs-mediatek.h
index dfbf78bd8664..4a8a8dc2ab1e 100644
--- a/drivers/ufs/host/ufs-mediatek.h
+++ b/drivers/ufs/host/ufs-mediatek.h
@@ -174,7 +174,6 @@ struct ufs_mtk_host {
 	struct reset_control *hci_reset;
 	struct reset_control *unipro_reset;
 	struct reset_control *crypto_reset;
-	struct reset_control *mphy_reset;
 	struct ufs_hba *hba;
 	struct ufs_mtk_crypt_cfg *crypt;
 	struct ufs_mtk_clk mclk;

-- 
2.51.0

Re: [PATCH 5/5] scsi: ufs: mediatek: Rework resets

[Philipp Zabel]

Hi Nicolas,

On Di, 2025-10-14 at 17:10 +0200, Nicolas Frattaroli wrote:
> Rework the reset control getting in the driver's probe function to use
> the "_optional" function instead of defaulting to NULL on IS_ERR, so
> that actual real errors (as opposed to missing resets) can be handled as
> errors in the probe function.
> 
> Also move the MPHY reset into the PHY driver, where it should live, and
> remove all remnants of it ever having been in this driver.

Part of that happened in the previous patch.

> Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> ---
>  drivers/ufs/host/ufs-mediatek-sip.h |  8 -----
>  drivers/ufs/host/ufs-mediatek.c     | 67 +++++++++++++++++++++----------------
>  drivers/ufs/host/ufs-mediatek.h     |  1 -
>  3 files changed, 38 insertions(+), 38 deletions(-)
> 
> diff --git a/drivers/ufs/host/ufs-mediatek-sip.h b/drivers/ufs/host/ufs-mediatek-sip.h
> index d627dfb4a766..256598cc3b5b 100644
> --- a/drivers/ufs/host/ufs-mediatek-sip.h
> +++ b/drivers/ufs/host/ufs-mediatek-sip.h
> @@ -31,11 +31,6 @@ enum ufs_mtk_vcc_num {
>  	UFS_VCC_MAX
>  };
>  
> -enum ufs_mtk_mphy_op {
> -	UFS_MPHY_BACKUP = 0,
> -	UFS_MPHY_RESTORE
> -};
> -
>  /*
>   * SMC call wrapper function
>   */
> @@ -84,9 +79,6 @@ static inline void _ufs_mtk_smc(struct ufs_mtk_smc_arg s)
>  #define ufs_mtk_device_pwr_ctrl(on, ufs_version, res) \
>  	ufs_mtk_smc(UFS_MTK_SIP_DEVICE_PWR_CTRL, &(res), on, ufs_version)
>  
> -#define ufs_mtk_mphy_ctrl(op, res) \
> -	ufs_mtk_smc(UFS_MTK_SIP_MPHY_CTRL, &(res), op)
> -
>  #define ufs_mtk_mtcmos_ctrl(op, res) \
>  	ufs_mtk_smc(UFS_MTK_SIP_MTCMOS_CTRL, &(res), op)
>  
> diff --git a/drivers/ufs/host/ufs-mediatek.c b/drivers/ufs/host/ufs-mediatek.c
> index 758a393a9de1..ac40d4a3a800 100644
> --- a/drivers/ufs/host/ufs-mediatek.c
> +++ b/drivers/ufs/host/ufs-mediatek.c
> @@ -204,49 +204,60 @@ static void ufs_mtk_crypto_enable(struct ufs_hba *hba)
>  static void ufs_mtk_host_reset(struct ufs_hba *hba)
>  {
>  	struct ufs_mtk_host *host = ufshcd_get_variant(hba);
> -	struct arm_smccc_res res;
> +	int ret;
>  
>  	reset_control_assert(host->hci_reset);
>  	reset_control_assert(host->crypto_reset);
>  	reset_control_assert(host->unipro_reset);
> -	reset_control_assert(host->mphy_reset);
>  
> -	usleep_range(100, 110);
> +	ret = phy_reset(host->mphy);
> +
> +	/*
> +	 * Only sleep if MPHY doesn't have a reset implemented (which already
> +	 * sleeps) or the PHY reset function failed somehow, just to be safe
> +	 */
> +	if (ret) {
> +		usleep_range(100, 110);
> +		if (ret != -EOPNOTSUPP)
> +			dev_warn(hba->dev, "PHY reset failed: %pe\n", ERR_PTR(ret));
> +	}
>  
>  	reset_control_deassert(host->unipro_reset);
>  	reset_control_deassert(host->crypto_reset);
>  	reset_control_deassert(host->hci_reset);
> -	reset_control_deassert(host->mphy_reset);
> -
> -	/* restore mphy setting aftre mphy reset */
> -	if (host->mphy_reset)
> -		ufs_mtk_mphy_ctrl(UFS_MPHY_RESTORE, res);
>  }
>  
> -static void ufs_mtk_init_reset_control(struct ufs_hba *hba,
> -				       struct reset_control **rc,
> -				       char *str)
> +static int ufs_mtk_init_reset_control(struct ufs_hba *hba,
> +				      struct reset_control **rc,
> +				      const char *str)
>  {
> -	*rc = devm_reset_control_get(hba->dev, str);
> +	*rc = devm_reset_control_get_optional(hba->dev, str);

Please use

	devm_reset_control_get_optional_exclusive()

directly, or see below for an alternative suggestion.

>  	if (IS_ERR(*rc)) {
> -		dev_info(hba->dev, "Failed to get reset control %s: %ld\n",
> -			 str, PTR_ERR(*rc));
> -		*rc = NULL;
> +		dev_err(hba->dev, "Failed to get reset control %s: %pe\n", str, *rc);
> +		return PTR_ERR(*rc);
>  	}
> +
> +	return 0;
>  }
>  
> -static void ufs_mtk_init_reset(struct ufs_hba *hba)
> +static int ufs_mtk_init_reset(struct ufs_hba *hba)
>  {
>  	struct ufs_mtk_host *host = ufshcd_get_variant(hba);
> +	int ret;
> +
> +	ret = ufs_mtk_init_reset_control(hba, &host->hci_reset, "hci_rst");
> +	if (ret)
> +		return ret;
> +
> +	ret = ufs_mtk_init_reset_control(hba, &host->unipro_reset, "unipro_rst");
> +	if (ret)
> +		return ret;
> +
> +	ret = ufs_mtk_init_reset_control(hba, &host->crypto_reset, "crypto_rst");
> +	if (ret)
> +		return ret;

It looks to me like you could combine these into a

	devm_reset_control_bulk_get_optional_exclusive()

and operate the three resets with reset_control_bulk_assert/deassert().


regards
Philipp

Re: [PATCH 0/5] MediaTek UFS Cleanup and MT8196 Enablement

[AngeloGioacchino Del Regno]

Il 14/10/25 17:10, Nicolas Frattaroli ha scritto:
> In this series, the existing MediaTek UFS binding is expanded and
> completed to correctly describe not just the existing compatibles, but
> also to introduce a new compatible in the from of the MT8196 SoC.
> 
> The resets, which until now were completely absent from both the UFS
> host controller binding and the UFS PHY binding, are introduced to both.
> This also means the driver's undocumented and, in mainline, unused reset
> logic is reworked. In particular, the PHY reset is no longer a reset of
> the host controller node, but of the PHY node.
> 
> This means the host controller can reset the PHY through the common PHY
> framework.
> 
> The resets remain optional.
> 
> Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>

While series is

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

Cheers,
Angelo