[PATCH v2 3/3] dt-bindings: ufs: qcom: Split SM8650 and similar

Krzysztof Kozlowski posted 3 patches 2 months ago
[PATCH v2 3/3] dt-bindings: ufs: qcom: Split SM8650 and similar
Posted by Krzysztof Kozlowski 2 months ago
The binding for Qualcomm SoC UFS controllers grew and it will grow
further.  Split SM8650 and SM8750 UFS controllers which:
1. Do not reference ICE as IO address space, but as phandle,
2. Have same order of clocks.
3. Have MCQ IO address space. Document that MCQ address space as
   optional to maintain backwards compatibility and because Linux
   drivers can operate perfectly fine without it (thus without MCQ
   feature).  Linux driver already uses "mcq" as possible name for
   "reg-names" property.

The split allows easier review and maintenance of the binding.

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 .../devicetree/bindings/ufs/qcom,sm8650-ufshc.yaml | 178 +++++++++++++++++++++
 .../devicetree/bindings/ufs/qcom,ufs.yaml          |  32 ----
 2 files changed, 178 insertions(+), 32 deletions(-)

diff --git a/Documentation/devicetree/bindings/ufs/qcom,sm8650-ufshc.yaml b/Documentation/devicetree/bindings/ufs/qcom,sm8650-ufshc.yaml
new file mode 100644
index 0000000000000000000000000000000000000000..aaa0bbb5bfe1673e3e0d25812c2829350b137abb
--- /dev/null
+++ b/Documentation/devicetree/bindings/ufs/qcom,sm8650-ufshc.yaml
@@ -0,0 +1,178 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/ufs/qcom,sm8650-ufshc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm SM8650 and Other SoCs UFS Controllers
+
+maintainers:
+  - Bjorn Andersson <bjorn.andersson@linaro.org>
+
+# Select only our matches, not all jedec,ufs-2.0
+select:
+  properties:
+    compatible:
+      contains:
+        enum:
+          - qcom,sm8650-ufshc
+          - qcom,sm8750-ufshc
+  required:
+    - compatible
+
+properties:
+  compatible:
+    items:
+      - enum:
+          - qcom,sm8650-ufshc
+          - qcom,sm8750-ufshc
+      - const: qcom,ufshc
+      - const: jedec,ufs-2.0
+
+  reg:
+    minItems: 1
+    maxItems: 2
+
+  reg-names:
+    minItems: 1
+    items:
+      - const: std
+      - const: mcq
+
+  clocks:
+    minItems: 8
+    maxItems: 8
+
+  clock-names:
+    items:
+      - const: core_clk
+      - const: bus_aggr_clk
+      - const: iface_clk
+      - const: core_clk_unipro
+      - const: ref_clk
+      - const: tx_lane0_sync_clk
+      - const: rx_lane0_sync_clk
+      - const: rx_lane1_sync_clk
+
+  qcom,ice:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description: phandle to the Inline Crypto Engine node
+
+required:
+  - compatible
+  - reg
+
+allOf:
+  - $ref: qcom,ufs-common.yaml
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/qcom,sm8650-gcc.h>
+    #include <dt-bindings/clock/qcom,sm8650-tcsr.h>
+    #include <dt-bindings/clock/qcom,rpmh.h>
+    #include <dt-bindings/gpio/gpio.h>
+    #include <dt-bindings/interconnect/qcom,icc.h>
+    #include <dt-bindings/interconnect/qcom,sm8650-rpmh.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+    soc {
+        #address-cells = <2>;
+        #size-cells = <2>;
+
+        ufshc@1d84000 {
+            compatible = "qcom,sm8650-ufshc", "qcom,ufshc", "jedec,ufs-2.0";
+            reg = <0x0 0x01d84000 0x0 0x3000>;
+
+            interrupts = <GIC_SPI 265 IRQ_TYPE_LEVEL_HIGH 0>;
+
+            clocks = <&gcc GCC_UFS_PHY_AXI_CLK>,
+                     <&gcc GCC_AGGRE_UFS_PHY_AXI_CLK>,
+                     <&gcc GCC_UFS_PHY_AHB_CLK>,
+                     <&gcc GCC_UFS_PHY_UNIPRO_CORE_CLK>,
+                     <&tcsr TCSR_UFS_PAD_CLKREF_EN>,
+                     <&gcc GCC_UFS_PHY_TX_SYMBOL_0_CLK>,
+                     <&gcc GCC_UFS_PHY_RX_SYMBOL_0_CLK>,
+                     <&gcc GCC_UFS_PHY_RX_SYMBOL_1_CLK>;
+            clock-names = "core_clk",
+                          "bus_aggr_clk",
+                          "iface_clk",
+                          "core_clk_unipro",
+                          "ref_clk",
+                          "tx_lane0_sync_clk",
+                          "rx_lane0_sync_clk",
+                          "rx_lane1_sync_clk";
+
+            resets = <&gcc GCC_UFS_PHY_BCR>;
+            reset-names = "rst";
+            reset-gpios = <&tlmm 210 GPIO_ACTIVE_LOW>;
+
+            interconnects = <&aggre1_noc MASTER_UFS_MEM QCOM_ICC_TAG_ALWAYS
+                             &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>,
+                            <&gem_noc MASTER_APPSS_PROC QCOM_ICC_TAG_ACTIVE_ONLY
+                             &config_noc SLAVE_UFS_MEM_CFG QCOM_ICC_TAG_ACTIVE_ONLY>;
+            interconnect-names = "ufs-ddr",
+                         "cpu-ufs";
+
+            power-domains = <&gcc UFS_PHY_GDSC>;
+            required-opps = <&rpmhpd_opp_nom>;
+
+            operating-points-v2 = <&ufs_opp_table>;
+
+            iommus = <&apps_smmu 0x60 0>;
+
+            lanes-per-direction = <2>;
+            qcom,ice = <&ice>;
+
+            phys = <&ufs_mem_phy>;
+            phy-names = "ufsphy";
+
+            #reset-cells = <1>;
+
+            vcc-supply = <&vreg_l7b_2p5>;
+            vcc-max-microamp = <1100000>;
+            vccq-supply = <&vreg_l9b_1p2>;
+            vccq-max-microamp = <1200000>;
+
+            ufs_opp_table: opp-table {
+                compatible = "operating-points-v2";
+
+                opp-100000000 {
+                    opp-hz = /bits/ 64 <100000000>,
+                             /bits/ 64 <0>,
+                             /bits/ 64 <0>,
+                             /bits/ 64 <100000000>,
+                             /bits/ 64 <0>,
+                             /bits/ 64 <0>,
+                             /bits/ 64 <0>,
+                             /bits/ 64 <0>;
+                    required-opps = <&rpmhpd_opp_low_svs>;
+                };
+
+                opp-201500000 {
+                    opp-hz = /bits/ 64 <201500000>,
+                             /bits/ 64 <0>,
+                             /bits/ 64 <0>,
+                             /bits/ 64 <201500000>,
+                             /bits/ 64 <0>,
+                             /bits/ 64 <0>,
+                             /bits/ 64 <0>,
+                             /bits/ 64 <0>;
+                    required-opps = <&rpmhpd_opp_svs>;
+                };
+
+                opp-403000000 {
+                    opp-hz = /bits/ 64 <403000000>,
+                             /bits/ 64 <0>,
+                             /bits/ 64 <0>,
+                             /bits/ 64 <403000000>,
+                             /bits/ 64 <0>,
+                             /bits/ 64 <0>,
+                             /bits/ 64 <0>,
+                             /bits/ 64 <0>;
+                    required-opps = <&rpmhpd_opp_nom>;
+                };
+            };
+        };
+    };
diff --git a/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml b/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml
index 191b88120d139a47632e3dce3d3f3a37d7a55c72..1dd41f6d5258014d59c8c8005bc54f7994351a52 100644
--- a/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml
+++ b/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml
@@ -24,8 +24,6 @@ select:
           - qcom,sm6125-ufshc
           - qcom,sm6350-ufshc
           - qcom,sm8150-ufshc
-          - qcom,sm8650-ufshc
-          - qcom,sm8750-ufshc
   required:
     - compatible
 
@@ -41,8 +39,6 @@ properties:
           - qcom,sm6125-ufshc
           - qcom,sm6350-ufshc
           - qcom,sm8150-ufshc
-          - qcom,sm8650-ufshc
-          - qcom,sm8750-ufshc
       - const: qcom,ufshc
       - const: jedec,ufs-2.0
 
@@ -66,34 +62,6 @@ required:
 allOf:
   - $ref: qcom,ufs-common.yaml
 
-  - if:
-      properties:
-        compatible:
-          contains:
-            enum:
-              - qcom,sm8650-ufshc
-              - qcom,sm8750-ufshc
-    then:
-      properties:
-        clocks:
-          minItems: 8
-          maxItems: 8
-        clock-names:
-          items:
-            - const: core_clk
-            - const: bus_aggr_clk
-            - const: iface_clk
-            - const: core_clk_unipro
-            - const: ref_clk
-            - const: tx_lane0_sync_clk
-            - const: rx_lane0_sync_clk
-            - const: rx_lane1_sync_clk
-        reg:
-          minItems: 1
-          maxItems: 1
-        reg-names:
-          maxItems: 1
-
   - if:
       properties:
         compatible:

-- 
2.48.1
Re: [PATCH v2 3/3] dt-bindings: ufs: qcom: Split SM8650 and similar
Posted by Manivannan Sadhasivam 2 months ago
On Thu, Jul 31, 2025 at 09:15:54AM GMT, Krzysztof Kozlowski wrote:
> The binding for Qualcomm SoC UFS controllers grew and it will grow
> further.  Split SM8650 and SM8750 UFS controllers which:
> 1. Do not reference ICE as IO address space, but as phandle,
> 2. Have same order of clocks.
> 3. Have MCQ IO address space. Document that MCQ address space as
>    optional to maintain backwards compatibility and because Linux
>    drivers can operate perfectly fine without it (thus without MCQ
>    feature).  Linux driver already uses "mcq" as possible name for
>    "reg-names" property.

Since Qcom SoC memory maps have holes and shared registers in the whole 'mcq'
region, it is preferred to map only the required parts. So please drop 'mcq' and
add 'mcq_sqd', 'mcq_vs' regions.

With the above change, 

Acked-by: Manivannan Sadhasivam <mani@kernel.org>

- Mani

> 
> The split allows easier review and maintenance of the binding.
> 
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
>  .../devicetree/bindings/ufs/qcom,sm8650-ufshc.yaml | 178 +++++++++++++++++++++
>  .../devicetree/bindings/ufs/qcom,ufs.yaml          |  32 ----
>  2 files changed, 178 insertions(+), 32 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/ufs/qcom,sm8650-ufshc.yaml b/Documentation/devicetree/bindings/ufs/qcom,sm8650-ufshc.yaml
> new file mode 100644
> index 0000000000000000000000000000000000000000..aaa0bbb5bfe1673e3e0d25812c2829350b137abb
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/ufs/qcom,sm8650-ufshc.yaml
> @@ -0,0 +1,178 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/ufs/qcom,sm8650-ufshc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Qualcomm SM8650 and Other SoCs UFS Controllers
> +
> +maintainers:
> +  - Bjorn Andersson <bjorn.andersson@linaro.org>
> +
> +# Select only our matches, not all jedec,ufs-2.0
> +select:
> +  properties:
> +    compatible:
> +      contains:
> +        enum:
> +          - qcom,sm8650-ufshc
> +          - qcom,sm8750-ufshc
> +  required:
> +    - compatible
> +
> +properties:
> +  compatible:
> +    items:
> +      - enum:
> +          - qcom,sm8650-ufshc
> +          - qcom,sm8750-ufshc
> +      - const: qcom,ufshc
> +      - const: jedec,ufs-2.0
> +
> +  reg:
> +    minItems: 1
> +    maxItems: 2
> +
> +  reg-names:
> +    minItems: 1
> +    items:
> +      - const: std
> +      - const: mcq
> +
> +  clocks:
> +    minItems: 8
> +    maxItems: 8
> +
> +  clock-names:
> +    items:
> +      - const: core_clk
> +      - const: bus_aggr_clk
> +      - const: iface_clk
> +      - const: core_clk_unipro
> +      - const: ref_clk
> +      - const: tx_lane0_sync_clk
> +      - const: rx_lane0_sync_clk
> +      - const: rx_lane1_sync_clk
> +
> +  qcom,ice:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description: phandle to the Inline Crypto Engine node
> +
> +required:
> +  - compatible
> +  - reg
> +
> +allOf:
> +  - $ref: qcom,ufs-common.yaml
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/qcom,sm8650-gcc.h>
> +    #include <dt-bindings/clock/qcom,sm8650-tcsr.h>
> +    #include <dt-bindings/clock/qcom,rpmh.h>
> +    #include <dt-bindings/gpio/gpio.h>
> +    #include <dt-bindings/interconnect/qcom,icc.h>
> +    #include <dt-bindings/interconnect/qcom,sm8650-rpmh.h>
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> +    soc {
> +        #address-cells = <2>;
> +        #size-cells = <2>;
> +
> +        ufshc@1d84000 {
> +            compatible = "qcom,sm8650-ufshc", "qcom,ufshc", "jedec,ufs-2.0";
> +            reg = <0x0 0x01d84000 0x0 0x3000>;
> +
> +            interrupts = <GIC_SPI 265 IRQ_TYPE_LEVEL_HIGH 0>;
> +
> +            clocks = <&gcc GCC_UFS_PHY_AXI_CLK>,
> +                     <&gcc GCC_AGGRE_UFS_PHY_AXI_CLK>,
> +                     <&gcc GCC_UFS_PHY_AHB_CLK>,
> +                     <&gcc GCC_UFS_PHY_UNIPRO_CORE_CLK>,
> +                     <&tcsr TCSR_UFS_PAD_CLKREF_EN>,
> +                     <&gcc GCC_UFS_PHY_TX_SYMBOL_0_CLK>,
> +                     <&gcc GCC_UFS_PHY_RX_SYMBOL_0_CLK>,
> +                     <&gcc GCC_UFS_PHY_RX_SYMBOL_1_CLK>;
> +            clock-names = "core_clk",
> +                          "bus_aggr_clk",
> +                          "iface_clk",
> +                          "core_clk_unipro",
> +                          "ref_clk",
> +                          "tx_lane0_sync_clk",
> +                          "rx_lane0_sync_clk",
> +                          "rx_lane1_sync_clk";
> +
> +            resets = <&gcc GCC_UFS_PHY_BCR>;
> +            reset-names = "rst";
> +            reset-gpios = <&tlmm 210 GPIO_ACTIVE_LOW>;
> +
> +            interconnects = <&aggre1_noc MASTER_UFS_MEM QCOM_ICC_TAG_ALWAYS
> +                             &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>,
> +                            <&gem_noc MASTER_APPSS_PROC QCOM_ICC_TAG_ACTIVE_ONLY
> +                             &config_noc SLAVE_UFS_MEM_CFG QCOM_ICC_TAG_ACTIVE_ONLY>;
> +            interconnect-names = "ufs-ddr",
> +                         "cpu-ufs";
> +
> +            power-domains = <&gcc UFS_PHY_GDSC>;
> +            required-opps = <&rpmhpd_opp_nom>;
> +
> +            operating-points-v2 = <&ufs_opp_table>;
> +
> +            iommus = <&apps_smmu 0x60 0>;
> +
> +            lanes-per-direction = <2>;
> +            qcom,ice = <&ice>;
> +
> +            phys = <&ufs_mem_phy>;
> +            phy-names = "ufsphy";
> +
> +            #reset-cells = <1>;
> +
> +            vcc-supply = <&vreg_l7b_2p5>;
> +            vcc-max-microamp = <1100000>;
> +            vccq-supply = <&vreg_l9b_1p2>;
> +            vccq-max-microamp = <1200000>;
> +
> +            ufs_opp_table: opp-table {
> +                compatible = "operating-points-v2";
> +
> +                opp-100000000 {
> +                    opp-hz = /bits/ 64 <100000000>,
> +                             /bits/ 64 <0>,
> +                             /bits/ 64 <0>,
> +                             /bits/ 64 <100000000>,
> +                             /bits/ 64 <0>,
> +                             /bits/ 64 <0>,
> +                             /bits/ 64 <0>,
> +                             /bits/ 64 <0>;
> +                    required-opps = <&rpmhpd_opp_low_svs>;
> +                };
> +
> +                opp-201500000 {
> +                    opp-hz = /bits/ 64 <201500000>,
> +                             /bits/ 64 <0>,
> +                             /bits/ 64 <0>,
> +                             /bits/ 64 <201500000>,
> +                             /bits/ 64 <0>,
> +                             /bits/ 64 <0>,
> +                             /bits/ 64 <0>,
> +                             /bits/ 64 <0>;
> +                    required-opps = <&rpmhpd_opp_svs>;
> +                };
> +
> +                opp-403000000 {
> +                    opp-hz = /bits/ 64 <403000000>,
> +                             /bits/ 64 <0>,
> +                             /bits/ 64 <0>,
> +                             /bits/ 64 <403000000>,
> +                             /bits/ 64 <0>,
> +                             /bits/ 64 <0>,
> +                             /bits/ 64 <0>,
> +                             /bits/ 64 <0>;
> +                    required-opps = <&rpmhpd_opp_nom>;
> +                };
> +            };
> +        };
> +    };
> diff --git a/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml b/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml
> index 191b88120d139a47632e3dce3d3f3a37d7a55c72..1dd41f6d5258014d59c8c8005bc54f7994351a52 100644
> --- a/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml
> +++ b/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml
> @@ -24,8 +24,6 @@ select:
>            - qcom,sm6125-ufshc
>            - qcom,sm6350-ufshc
>            - qcom,sm8150-ufshc
> -          - qcom,sm8650-ufshc
> -          - qcom,sm8750-ufshc
>    required:
>      - compatible
>  
> @@ -41,8 +39,6 @@ properties:
>            - qcom,sm6125-ufshc
>            - qcom,sm6350-ufshc
>            - qcom,sm8150-ufshc
> -          - qcom,sm8650-ufshc
> -          - qcom,sm8750-ufshc
>        - const: qcom,ufshc
>        - const: jedec,ufs-2.0
>  
> @@ -66,34 +62,6 @@ required:
>  allOf:
>    - $ref: qcom,ufs-common.yaml
>  
> -  - if:
> -      properties:
> -        compatible:
> -          contains:
> -            enum:
> -              - qcom,sm8650-ufshc
> -              - qcom,sm8750-ufshc
> -    then:
> -      properties:
> -        clocks:
> -          minItems: 8
> -          maxItems: 8
> -        clock-names:
> -          items:
> -            - const: core_clk
> -            - const: bus_aggr_clk
> -            - const: iface_clk
> -            - const: core_clk_unipro
> -            - const: ref_clk
> -            - const: tx_lane0_sync_clk
> -            - const: rx_lane0_sync_clk
> -            - const: rx_lane1_sync_clk
> -        reg:
> -          minItems: 1
> -          maxItems: 1
> -        reg-names:
> -          maxItems: 1
> -
>    - if:
>        properties:
>          compatible:
> 
> -- 
> 2.48.1
> 

-- 
மணிவண்ணன் சதாசிவம்
Re: [PATCH v2 3/3] dt-bindings: ufs: qcom: Split SM8650 and similar
Posted by Krzysztof Kozlowski 2 months ago
On 01/08/2025 14:28, Manivannan Sadhasivam wrote:
> On Thu, Jul 31, 2025 at 09:15:54AM GMT, Krzysztof Kozlowski wrote:
>> The binding for Qualcomm SoC UFS controllers grew and it will grow
>> further.  Split SM8650 and SM8750 UFS controllers which:
>> 1. Do not reference ICE as IO address space, but as phandle,
>> 2. Have same order of clocks.
>> 3. Have MCQ IO address space. Document that MCQ address space as
>>    optional to maintain backwards compatibility and because Linux
>>    drivers can operate perfectly fine without it (thus without MCQ
>>    feature).  Linux driver already uses "mcq" as possible name for
>>    "reg-names" property.
> 
> Since Qcom SoC memory maps have holes and shared registers in the whole 'mcq'
> region, it is preferred to map only the required parts. So please drop 'mcq' and
> add 'mcq_sqd', 'mcq_vs' regions.


No, it is not preferred. The docs are clearly stating that there is oone
address space for MCQ and continuous.

We have been fixing above approach you propose also for other devices,
so this should not go to the broken part.


Best regards,
Krzysztof
Re: [PATCH v2 3/3] dt-bindings: ufs: qcom: Split SM8650 and similar
Posted by Rob Herring (Arm) 2 months ago
On Thu, 31 Jul 2025 09:15:54 +0200, Krzysztof Kozlowski wrote:
> The binding for Qualcomm SoC UFS controllers grew and it will grow
> further.  Split SM8650 and SM8750 UFS controllers which:
> 1. Do not reference ICE as IO address space, but as phandle,
> 2. Have same order of clocks.
> 3. Have MCQ IO address space. Document that MCQ address space as
>    optional to maintain backwards compatibility and because Linux
>    drivers can operate perfectly fine without it (thus without MCQ
>    feature).  Linux driver already uses "mcq" as possible name for
>    "reg-names" property.
> 
> The split allows easier review and maintenance of the binding.
> 
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
>  .../devicetree/bindings/ufs/qcom,sm8650-ufshc.yaml | 178 +++++++++++++++++++++
>  .../devicetree/bindings/ufs/qcom,ufs.yaml          |  32 ----
>  2 files changed, 178 insertions(+), 32 deletions(-)
> 

Reviewed-by: Rob Herring (Arm) <robh@kernel.org>