[PATCH V1 0/3] Enable UFS MCQ support for SM8650 and SM8750

[PATCH V1 0/3] Enable UFS MCQ support for SM8650 and SM8750

[Ram Kumar Dwivedi]

This patch series enables Multi-Circular Queue (MCQ) support for the UFS
host controller on Qualcomm SM8650 and SM8750 platforms. MCQ is a modern
queuing model that improves performance and scalability by allowing
multiple hardware queues.

Although MCQ support has been present in the UFS driver for several years,
this is the first time it is being enabled via Device Tree for these
platforms.

Patch 1 updates the device tree bindings to allow the additional register
regions and reg-names required for MCQ operation.

Patches 2 and 3 update the device trees for SM8650 and SM8750 respectively
to enable MCQ by adding the necessary register mappings and MSI parent.

Tested on internal hardware for both platforms.

Palash Kambar (1):
  arm64: dts: qcom: sm8750: Enable MCQ support for UFS controller

Ram Kumar Dwivedi (2):
  dt-bindings: ufs: qcom: Add MCQ support to reg and reg-names
  arm64: dts: qcom: sm8650: Enable MCQ support for UFS controller

 .../devicetree/bindings/ufs/qcom,ufs.yaml     | 21 ++++++++++++-------
 arch/arm64/boot/dts/qcom/sm8650.dtsi          |  9 +++++++-
 arch/arm64/boot/dts/qcom/sm8750.dtsi          | 10 +++++++--
 3 files changed, 29 insertions(+), 11 deletions(-)

-- 
2.50.1

[PATCH V1 1/3] dt-bindings: ufs: qcom: Add reg and reg-names

[Ram Kumar Dwivedi]

Update the Qualcomm UFS device tree bindings to support Multi-Circular
Queue (MCQ) operation. This includes increasing the maximum number of
register entries from 2 to 3 and extending the accepted values for
reg-names to include "mcq_sqd" and "mcq_vs".

These changes are required to enable MCQ support via Device Tree for
platforms such as SM8650 and SM8750.

Signed-off-by: Ram Kumar Dwivedi <quic_rdwivedi@quicinc.com>
---
 .../devicetree/bindings/ufs/qcom,ufs.yaml     | 21 ++++++++++++-------
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml b/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml
index 6c6043d9809e..de263118b552 100644
--- a/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml
+++ b/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml
@@ -86,12 +86,17 @@ properties:
 
   reg:
     minItems: 1
-    maxItems: 2
+    maxItems: 3
 
   reg-names:
-    items:
-      - const: std
-      - const: ice
+    oneOf:
+      - items:
+          - const: std
+          - const: ice
+      - items:
+          - const: ufs_mem
+          - const: mcq_sqd
+          - const: mcq_vs
 
   required-opps:
     maxItems: 1
@@ -177,9 +182,9 @@ allOf:
             - const: rx_lane1_sync_clk
         reg:
           minItems: 1
-          maxItems: 1
+          maxItems: 3
         reg-names:
-          maxItems: 1
+          maxItems: 3
 
   - if:
       properties:
@@ -280,7 +285,7 @@ allOf:
     then:
       properties:
         reg:
-          maxItems: 1
+          maxItems: 3
         clocks:
           minItems: 7
           maxItems: 8
@@ -288,7 +293,7 @@ allOf:
       properties:
         reg:
           minItems: 1
-          maxItems: 2
+          maxItems: 3
         clocks:
           minItems: 7
           maxItems: 9
-- 
2.50.1

[PATCH V1 2/3] arm64: dts: qcom: sm8650: Enable MCQ support for UFS controller

[Ram Kumar Dwivedi]

Enable Multi-Circular Queue (MCQ) support for the UFS host controller
on the Qualcomm SM8650 platform by updating the device tree node. This
includes adding new register regions and specifying the MSI parent
required for MCQ operation.

MCQ is a modern queuing model for UFS that improves performance and
scalability by allowing multiple hardware queues. 

Changes:
- Add reg entries for mcq_sqd and mcq_vs regions.
- Define reg-names for the new regions.
- Specify msi-parent for interrupt routing.

Signed-off-by: Ram Kumar Dwivedi <quic_rdwivedi@quicinc.com>
---
 arch/arm64/boot/dts/qcom/sm8650.dtsi | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/qcom/sm8650.dtsi b/arch/arm64/boot/dts/qcom/sm8650.dtsi
index e14d3d778b71..5d164fe511ba 100644
--- a/arch/arm64/boot/dts/qcom/sm8650.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8650.dtsi
@@ -3982,7 +3982,12 @@ ufs_mem_phy: phy@1d80000 {
 
 		ufs_mem_hc: ufshc@1d84000 {
 			compatible = "qcom,sm8650-ufshc", "qcom,ufshc", "jedec,ufs-2.0";
-			reg = <0 0x01d84000 0 0x3000>;
+			reg = <0 0x01d84000 0 0x3000>,
+			      <0 0x01da5000 0 0x2000>,
+			      <0 0x01da4000 0 0x0010>;
+			reg-names = "ufs_mem",
+				    "mcq_sqd",
+				    "mcq_vs";
 
 			interrupts = <GIC_SPI 265 IRQ_TYPE_LEVEL_HIGH 0>;
 
@@ -4020,6 +4025,8 @@ &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>,
 
 			iommus = <&apps_smmu 0x60 0>;
 
+			msi-parent = <&gic_its 0x60>;
+
 			lanes-per-direction = <2>;
 			qcom,ice = <&ice>;
 
-- 
2.50.1

[PATCH V1 3/3] arm64: dts: qcom: sm8750: Enable MCQ support for UFS controller

[Ram Kumar Dwivedi]

From: Palash Kambar <quic_pkambar@quicinc.com>

Enable Multi-Circular Queue (MCQ) support for the UFS host controller
on the Qualcomm SM8750 platform by updating the device tree node. This
includes adding new register regions and specifying the MSI parent
required for MCQ operation.

MCQ is a modern queuing model for UFS that improves performance and
scalability by allowing multiple hardware queues. Although MCQ support
has existed in the UFS driver for several years, this patch enables it
via Device Tree for SM8750.

Changes:
- Add reg entries for mcq_sqd and mcq_vs regions.
- Define reg-names for the new regions.
- Specify msi-parent for interrupt routing.

Signed-off-by: Palash Kambar <quic_pkambar@quicinc.com>
---
 arch/arm64/boot/dts/qcom/sm8750.dtsi | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sm8750.dtsi b/arch/arm64/boot/dts/qcom/sm8750.dtsi
index 4643705021c6..401e510ee738 100644
--- a/arch/arm64/boot/dts/qcom/sm8750.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8750.dtsi
@@ -3329,7 +3329,12 @@ ufs_mem_phy: phy@1d80000 {
 
 		ufs_mem_hc: ufs@1d84000 {
 			compatible = "qcom,sm8750-ufshc", "qcom,ufshc", "jedec,ufs-2.0";
-			reg = <0x0 0x01d84000 0x0 0x3000>;
+			reg = <0x0 0x01d84000 0x0 0x3000>,
+			      <0x0 0x1da5000 0x0 0x2000>,
+			      <0x0 0x1da4000 0x0 0x10>;
+			reg-names = "ufs_mem",
+				    "mcq_sqd",
+				    "mcq_vs";
 
 			interrupts = <GIC_SPI 265 IRQ_TYPE_LEVEL_HIGH>;
 
@@ -3363,11 +3368,12 @@ &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>,
 					     "cpu-ufs";
 
 			power-domains = <&gcc GCC_UFS_PHY_GDSC>;
+
 			required-opps = <&rpmhpd_opp_nom>;
 
 			iommus = <&apps_smmu 0x60 0>;
 			dma-coherent;
-
+			msi-parent = <&gic_its 0x60>;
 			lanes-per-direction = <2>;
 
 			phys = <&ufs_mem_phy>;
-- 
2.50.1

Re: [PATCH V1 1/3] dt-bindings: ufs: qcom: Add reg and reg-names

[Krzysztof Kozlowski]

On 30/07/2025 10:22, Ram Kumar Dwivedi wrote:
> Update the Qualcomm UFS device tree bindings to support Multi-Circular
> Queue (MCQ) operation. This includes increasing the maximum number of
> register entries from 2 to 3 and extending the accepted values for
> reg-names to include "mcq_sqd" and "mcq_vs".
> 
> These changes are required to enable MCQ support via Device Tree for
> platforms such as SM8650 and SM8750.
> 
> Signed-off-by: Ram Kumar Dwivedi <quic_rdwivedi@quicinc.com>
> ---
>  .../devicetree/bindings/ufs/qcom,ufs.yaml     | 21 ++++++++++++-------
>  1 file changed, 13 insertions(+), 8 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml b/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml
> index 6c6043d9809e..de263118b552 100644
> --- a/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml
> +++ b/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml
> @@ -86,12 +86,17 @@ properties:
>  
>    reg:
>      minItems: 1
> -    maxItems: 2
> +    maxItems: 3
>  
>    reg-names:
> -    items:
> -      - const: std
> -      - const: ice
> +    oneOf:
> +      - items:
> +          - const: std
> +          - const: ice
> +      - items:
> +          - const: ufs_mem
> +          - const: mcq_sqd
> +          - const: mcq_vs

This is incompatible change and commit msg is inaccurate here. It says
"extending" but you are not extending at all.

Recent qcom patches love to break ABI and impact users. No.

Best regards,
Krzysztof

Re: [PATCH V1 2/3] arm64: dts: qcom: sm8650: Enable MCQ support for UFS controller

[Krzysztof Kozlowski]

On 30/07/2025 10:22, Ram Kumar Dwivedi wrote:
> Enable Multi-Circular Queue (MCQ) support for the UFS host controller
> on the Qualcomm SM8650 platform by updating the device tree node. This
> includes adding new register regions and specifying the MSI parent
> required for MCQ operation.
> 
> MCQ is a modern queuing model for UFS that improves performance and
> scalability by allowing multiple hardware queues. 
> 
> Changes:
> - Add reg entries for mcq_sqd and mcq_vs regions.
> - Define reg-names for the new regions.
> - Specify msi-parent for interrupt routing.


We see that from the diff. Drop redundant description, your first
paragraph already said this.

Best regards,
Krzysztof

Re: [PATCH V1 1/3] dt-bindings: ufs: qcom: Add reg and reg-names

[Ram Kumar Dwivedi]

On 30-Jul-25 2:41 PM, Krzysztof Kozlowski wrote:
> On 30/07/2025 10:22, Ram Kumar Dwivedi wrote:
>> Update the Qualcomm UFS device tree bindings to support Multi-Circular
>> Queue (MCQ) operation. This includes increasing the maximum number of
>> register entries from 2 to 3 and extending the accepted values for
>> reg-names to include "mcq_sqd" and "mcq_vs".
>>
>> These changes are required to enable MCQ support via Device Tree for
>> platforms such as SM8650 and SM8750.
>>
>> Signed-off-by: Ram Kumar Dwivedi <quic_rdwivedi@quicinc.com>
>> ---
>>  .../devicetree/bindings/ufs/qcom,ufs.yaml     | 21 ++++++++++++-------
>>  1 file changed, 13 insertions(+), 8 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml b/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml
>> index 6c6043d9809e..de263118b552 100644
>> --- a/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml
>> +++ b/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml
>> @@ -86,12 +86,17 @@ properties:
>>  
>>    reg:
>>      minItems: 1
>> -    maxItems: 2
>> +    maxItems: 3
>>  
>>    reg-names:
>> -    items:
>> -      - const: std
>> -      - const: ice
>> +    oneOf:
>> +      - items:
>> +          - const: std
>> +          - const: ice
>> +      - items:
>> +          - const: ufs_mem
>> +          - const: mcq_sqd
>> +          - const: mcq_vs
> 
> This is incompatible change and commit msg is inaccurate here. It says
> "extending" but you are not extending at all.
> 
> Recent qcom patches love to break ABI and impact users. No.
> 
> Best regards,
> Krzysztof


Hi Krzysztof,

Thanks for your feedback.

Regarding your concern about this being an incompatible change — could you please clarify what specific aspect you believe breaks compatibility? 
From my side, I’ve carefully tested the patch and verified that it does not break any existing DTs. I ran the following command to validate against the schema:

make -j32 dt_binding_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/ufs/qcom,ufs.yaml
make -j32 dtbs_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/ufs/qcom,ufs.yaml

There were no errors reported for any target DTB.

As for the commit message, I acknowledge your point and will revise it in the next patchset to more accurately reflect the nature of the change.

Best regards,
Ram.

Re: [PATCH V1 2/3] arm64: dts: qcom: sm8650: Enable MCQ support for UFS controller

[Ram Kumar Dwivedi]

On 30-Jul-25 2:42 PM, Krzysztof Kozlowski wrote:
> On 30/07/2025 10:22, Ram Kumar Dwivedi wrote:
>> Enable Multi-Circular Queue (MCQ) support for the UFS host controller
>> on the Qualcomm SM8650 platform by updating the device tree node. This
>> includes adding new register regions and specifying the MSI parent
>> required for MCQ operation.
>>
>> MCQ is a modern queuing model for UFS that improves performance and
>> scalability by allowing multiple hardware queues. 
>>
>> Changes:
>> - Add reg entries for mcq_sqd and mcq_vs regions.
>> - Define reg-names for the new regions.
>> - Specify msi-parent for interrupt routing.
> 
> 
> We see that from the diff. Drop redundant description, your first
> paragraph already said this.
> 
> Best regards,
> Krzysztof


Hi Krzysztof,

I will update it in the next patchset.

Thanks,
Ram.

Re: [PATCH V1 1/3] dt-bindings: ufs: qcom: Add reg and reg-names

[Krzysztof Kozlowski]

On 30/07/2025 12:27, Ram Kumar Dwivedi wrote:
> 
> 
> On 30-Jul-25 2:41 PM, Krzysztof Kozlowski wrote:
>> On 30/07/2025 10:22, Ram Kumar Dwivedi wrote:
>>> Update the Qualcomm UFS device tree bindings to support Multi-Circular
>>> Queue (MCQ) operation. This includes increasing the maximum number of
>>> register entries from 2 to 3 and extending the accepted values for
>>> reg-names to include "mcq_sqd" and "mcq_vs".
>>>
>>> These changes are required to enable MCQ support via Device Tree for
>>> platforms such as SM8650 and SM8750.
>>>
>>> Signed-off-by: Ram Kumar Dwivedi <quic_rdwivedi@quicinc.com>
>>> ---
>>>  .../devicetree/bindings/ufs/qcom,ufs.yaml     | 21 ++++++++++++-------
>>>  1 file changed, 13 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml b/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml
>>> index 6c6043d9809e..de263118b552 100644
>>> --- a/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml
>>> +++ b/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml
>>> @@ -86,12 +86,17 @@ properties:
>>>  
>>>    reg:
>>>      minItems: 1
>>> -    maxItems: 2
>>> +    maxItems: 3
>>>  
>>>    reg-names:
>>> -    items:
>>> -      - const: std
>>> -      - const: ice
>>> +    oneOf:
>>> +      - items:
>>> +          - const: std
>>> +          - const: ice
>>> +      - items:
>>> +          - const: ufs_mem
>>> +          - const: mcq_sqd
>>> +          - const: mcq_vs
>>
>> This is incompatible change and commit msg is inaccurate here. It says
>> "extending" but you are not extending at all.
>>
>> Recent qcom patches love to break ABI and impact users. No.
>>
>> Best regards,
>> Krzysztof
> 
> 
> Hi Krzysztof,
> 
> Thanks for your feedback.
> 
> Regarding your concern about this being an incompatible change — could you please clarify what specific aspect you believe breaks compatibility? 
> From my side, I’ve carefully tested the patch and verified that it does not break any existing DTs. I ran the following command to validate against the schema:


I missed that earlier list is not actually used for SM8550 and SM8650.
The syntax is a bit confusing after looking only at diff, which probably
means this binding is getting messy.

I think binding should be just split the constraints are easier to follow.

Best regards,
Krzysztof

Re: [PATCH V1 1/3] dt-bindings: ufs: qcom: Add reg and reg-names

[Krzysztof Kozlowski]

On 30/07/2025 13:33, Krzysztof Kozlowski wrote:
>>> "extending" but you are not extending at all.
>>>
>>> Recent qcom patches love to break ABI and impact users. No.
>>>
>>> Best regards,
>>> Krzysztof
>>
>>
>> Hi Krzysztof,
>>
>> Thanks for your feedback.
>>
>> Regarding your concern about this being an incompatible change — could you please clarify what specific aspect you believe breaks compatibility? 
>> From my side, I’ve carefully tested the patch and verified that it does not break any existing DTs. I ran the following command to validate against the schema:
> 
> 
> I missed that earlier list is not actually used for SM8550 and SM8650.
> The syntax is a bit confusing after looking only at diff, which probably
> means this binding is getting messy.
> 
> I think binding should be just split the constraints are easier to follow.

I sent a patch for splitting the bindings.

Best regards,
Krzysztof

Re: [PATCH V1 2/3] arm64: dts: qcom: sm8650: Enable MCQ support for UFS controller

[Krzysztof Kozlowski]

On 30/07/2025 10:22, Ram Kumar Dwivedi wrote:
> Enable Multi-Circular Queue (MCQ) support for the UFS host controller
> on the Qualcomm SM8650 platform by updating the device tree node. This
> includes adding new register regions and specifying the MSI parent
> required for MCQ operation.
> 
> MCQ is a modern queuing model for UFS that improves performance and
> scalability by allowing multiple hardware queues. 
> 
> Changes:
> - Add reg entries for mcq_sqd and mcq_vs regions.
> - Define reg-names for the new regions.
> - Specify msi-parent for interrupt routing.
> 
> Signed-off-by: Ram Kumar Dwivedi <quic_rdwivedi@quicinc.com>
> ---
>  arch/arm64/boot/dts/qcom/sm8650.dtsi | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sm8650.dtsi b/arch/arm64/boot/dts/qcom/sm8650.dtsi
> index e14d3d778b71..5d164fe511ba 100644
> --- a/arch/arm64/boot/dts/qcom/sm8650.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8650.dtsi
> @@ -3982,7 +3982,12 @@ ufs_mem_phy: phy@1d80000 {
>  
>  		ufs_mem_hc: ufshc@1d84000 {
>  			compatible = "qcom,sm8650-ufshc", "qcom,ufshc", "jedec,ufs-2.0";
> -			reg = <0 0x01d84000 0 0x3000>;
> +			reg = <0 0x01d84000 0 0x3000>,
> +			      <0 0x01da5000 0 0x2000>,
> +			      <0 0x01da4000 0 0x0010>;


These are wrong address spaces. Open your datasheet and look there.


Best regards,
Krzysztof

Re: [PATCH V1 2/3] arm64: dts: qcom: sm8650: Enable MCQ support for UFS controller

[Ram Kumar Dwivedi]

On 31-Jul-25 12:15 PM, Krzysztof Kozlowski wrote:
> On 30/07/2025 10:22, Ram Kumar Dwivedi wrote:
>> Enable Multi-Circular Queue (MCQ) support for the UFS host controller
>> on the Qualcomm SM8650 platform by updating the device tree node. This
>> includes adding new register regions and specifying the MSI parent
>> required for MCQ operation.
>>
>> MCQ is a modern queuing model for UFS that improves performance and
>> scalability by allowing multiple hardware queues. 
>>
>> Changes:
>> - Add reg entries for mcq_sqd and mcq_vs regions.
>> - Define reg-names for the new regions.
>> - Specify msi-parent for interrupt routing.
>>
>> Signed-off-by: Ram Kumar Dwivedi <quic_rdwivedi@quicinc.com>
>> ---
>>  arch/arm64/boot/dts/qcom/sm8650.dtsi | 9 ++++++++-
>>  1 file changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/sm8650.dtsi b/arch/arm64/boot/dts/qcom/sm8650.dtsi
>> index e14d3d778b71..5d164fe511ba 100644
>> --- a/arch/arm64/boot/dts/qcom/sm8650.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/sm8650.dtsi
>> @@ -3982,7 +3982,12 @@ ufs_mem_phy: phy@1d80000 {
>>  
>>  		ufs_mem_hc: ufshc@1d84000 {
>>  			compatible = "qcom,sm8650-ufshc", "qcom,ufshc", "jedec,ufs-2.0";
>> -			reg = <0 0x01d84000 0 0x3000>;
>> +			reg = <0 0x01d84000 0 0x3000>,
>> +			      <0 0x01da5000 0 0x2000>,
>> +			      <0 0x01da4000 0 0x0010>;
> 
> 
> These are wrong address spaces. Open your datasheet and look there.
>
Hi Krzysztof,

I’ve reviewed it again, and it is correct and functioning as expected both on our upstream and downstream codebase.
I think it is probably overlooked by you. Can you please double check from your end?

Thanks,
Ram.
 
> 
> Best regards,
> Krzysztof

Re: [PATCH V1 2/3] arm64: dts: qcom: sm8650: Enable MCQ support for UFS controller

[Krzysztof Kozlowski]

On 31/07/2025 10:34, Ram Kumar Dwivedi wrote:
> 
> 
> On 31-Jul-25 12:15 PM, Krzysztof Kozlowski wrote:
>> On 30/07/2025 10:22, Ram Kumar Dwivedi wrote:
>>> Enable Multi-Circular Queue (MCQ) support for the UFS host controller
>>> on the Qualcomm SM8650 platform by updating the device tree node. This
>>> includes adding new register regions and specifying the MSI parent
>>> required for MCQ operation.
>>>
>>> MCQ is a modern queuing model for UFS that improves performance and
>>> scalability by allowing multiple hardware queues. 
>>>
>>> Changes:
>>> - Add reg entries for mcq_sqd and mcq_vs regions.
>>> - Define reg-names for the new regions.
>>> - Specify msi-parent for interrupt routing.
>>>
>>> Signed-off-by: Ram Kumar Dwivedi <quic_rdwivedi@quicinc.com>
>>> ---
>>>  arch/arm64/boot/dts/qcom/sm8650.dtsi | 9 ++++++++-
>>>  1 file changed, 8 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/arch/arm64/boot/dts/qcom/sm8650.dtsi b/arch/arm64/boot/dts/qcom/sm8650.dtsi
>>> index e14d3d778b71..5d164fe511ba 100644
>>> --- a/arch/arm64/boot/dts/qcom/sm8650.dtsi
>>> +++ b/arch/arm64/boot/dts/qcom/sm8650.dtsi
>>> @@ -3982,7 +3982,12 @@ ufs_mem_phy: phy@1d80000 {
>>>  
>>>  		ufs_mem_hc: ufshc@1d84000 {
>>>  			compatible = "qcom,sm8650-ufshc", "qcom,ufshc", "jedec,ufs-2.0";
>>> -			reg = <0 0x01d84000 0 0x3000>;
>>> +			reg = <0 0x01d84000 0 0x3000>,
>>> +			      <0 0x01da5000 0 0x2000>,
>>> +			      <0 0x01da4000 0 0x0010>;
>>
>>
>> These are wrong address spaces. Open your datasheet and look there.
>>
> Hi Krzysztof,
> 
> I’ve reviewed it again, and it is correct and functioning as expected both on our upstream and downstream codebase.
> I think it is probably overlooked by you. Can you please double check from your end?
> 

No, it is not overlooked. There is no address space of length 0x10 at
0x01da4000 in qcom doc/datasheet system. Just open the doc and look
there by yourself. The size is 0x15000.

You are talking now about driver and this proves my point here:
https://lore.kernel.org/linux-devicetree/1547e339-5be2-4d87-ab35-98a9be0d250e@kernel.org/



Best regards,
Krzysztof

Re: [PATCH V1 0/3] Enable UFS MCQ support for SM8650 and SM8750

[neil.armstrong]

Hi,

On 30/07/2025 10:22, Ram Kumar Dwivedi wrote:
> This patch series enables Multi-Circular Queue (MCQ) support for the UFS
> host controller on Qualcomm SM8650 and SM8750 platforms. MCQ is a modern
> queuing model that improves performance and scalability by allowing
> multiple hardware queues.
> 
> Although MCQ support has been present in the UFS driver for several years,
> this is the first time it is being enabled via Device Tree for these
> platforms.
> 
> Patch 1 updates the device tree bindings to allow the additional register
> regions and reg-names required for MCQ operation.
> 
> Patches 2 and 3 update the device trees for SM8650 and SM8750 respectively
> to enable MCQ by adding the necessary register mappings and MSI parent.
> 
> Tested on internal hardware for both platforms.
> 
> Palash Kambar (1):
>    arm64: dts: qcom: sm8750: Enable MCQ support for UFS controller
> 
> Ram Kumar Dwivedi (2):
>    dt-bindings: ufs: qcom: Add MCQ support to reg and reg-names
>    arm64: dts: qcom: sm8650: Enable MCQ support for UFS controller
> 
>   .../devicetree/bindings/ufs/qcom,ufs.yaml     | 21 ++++++++++++-------
>   arch/arm64/boot/dts/qcom/sm8650.dtsi          |  9 +++++++-
>   arch/arm64/boot/dts/qcom/sm8750.dtsi          | 10 +++++++--
>   3 files changed, 29 insertions(+), 11 deletions(-)
> 

I ran some tests on the SM8650-QRD, and it works so please add my:
Tested-by: Neil Armstrong <neil.armstrong@linaro.org> # on SM8650-QRD

I ran some fio tests, comparing the v6.15, v6.16 (with threaded irqs)
and next + mcq support, and here's the analysis on the results:

Significant Performance Gains in Write Operations with Multiple Jobs:
The "mcq" change shows a substantial improvement in both IOPS and bandwidth for write operations with 8 jobs.
Moderate Improvement in Single Job Operations (Read and Write):
For single job operations (read and write), the "mcq" change generally leads to positive, albeit less dramatic, improvements in IOPS and bandwidth.
Slight Decrease in Read Operations with Multiple Jobs:
Interestingly, for read operations with 8 jobs, there's a slight decrease in both IOPS and bandwidth with the "mcq" kernel.

The raw results are:
Board: sm8650-qrd

read / 1 job
                v6.15     v6.16  next+mcq
iops (min)  3,996.00  5,921.60  4,661.20
iops (max)  4,772.80  6,491.20  5,027.60
iops (avg)  4,526.25  6,295.31  4,979.81
cpu % usr       4.62      2.96      5.68
cpu % sys      21.45     17.88     25.58
bw (MB/s)      18.54     25.78     20.40

read / 8 job
                 v6.15      v6.16   next+mcq
iops (min)  51,867.60  51,575.40  56,818.40
iops (max)  67,513.60  64,456.40  65,379.60
iops (avg)  64,314.80  62,136.76  63,016.07
cpu % usr        3.98       3.72       3.85
cpu % sys       16.70      17.16      14.87
bw (MB/s)      263.60     254.40     258.20

write / 1 job
                v6.15     v6.16  next+mcq
iops (min)  5,654.80  8,060.00  7,117.20
iops (max)  6,720.40  8,852.00  7,706.80
iops (avg)  6,576.91  8,579.81  7,459.97
cpu % usr       7.48      3.79      6.73
cpu % sys      41.09     23.27     30.66
bw (MB/s)      26.96     35.16     30.56

write / 8 job
                  v6.15       v6.16    next+mcq
iops (min)   84,687.80   95,043.40  114,054.00
iops (max)  107,620.80  113,572.00  164,526.00
iops (avg)   97,910.86  105,927.38  149,071.43
cpu % usr         5.43        4.38        2.88
cpu % sys        21.73       20.29       16.09
bw (MB/s)       400.80      433.80      610.40

The test suite is:
for rw in read write ; do
     echo "rw: ${rw}"
     for jobs in 1 8 ; do
         echo "jobs: ${jobs}"
         for it in $(seq 1 5) ; do
             fio --name=rand${rw} --rw=rand${rw} \
                 --ioengine=libaio --direct=1 \
                 --bs=4k --numjobs=${jobs} --size=32m \
                 --runtime=30 --time_based --end_fsync=1 \
                 --group_reporting --filename=/dev/disk/by-partlabel/super \
             | grep -E '(iops|sys=|READ:|WRITE:)'
             sleep 5
         done
     done
done

Thanks,
Neil

Re: [PATCH V1 2/3] arm64: dts: qcom: sm8650: Enable MCQ support for UFS controller

[Manivannan Sadhasivam]

On Thu, Jul 31, 2025 at 10:38:56AM GMT, Krzysztof Kozlowski wrote:
> On 31/07/2025 10:34, Ram Kumar Dwivedi wrote:
> > 
> > 
> > On 31-Jul-25 12:15 PM, Krzysztof Kozlowski wrote:
> >> On 30/07/2025 10:22, Ram Kumar Dwivedi wrote:
> >>> Enable Multi-Circular Queue (MCQ) support for the UFS host controller
> >>> on the Qualcomm SM8650 platform by updating the device tree node. This
> >>> includes adding new register regions and specifying the MSI parent
> >>> required for MCQ operation.
> >>>
> >>> MCQ is a modern queuing model for UFS that improves performance and
> >>> scalability by allowing multiple hardware queues. 
> >>>
> >>> Changes:
> >>> - Add reg entries for mcq_sqd and mcq_vs regions.
> >>> - Define reg-names for the new regions.
> >>> - Specify msi-parent for interrupt routing.
> >>>
> >>> Signed-off-by: Ram Kumar Dwivedi <quic_rdwivedi@quicinc.com>
> >>> ---
> >>>  arch/arm64/boot/dts/qcom/sm8650.dtsi | 9 ++++++++-
> >>>  1 file changed, 8 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/arch/arm64/boot/dts/qcom/sm8650.dtsi b/arch/arm64/boot/dts/qcom/sm8650.dtsi
> >>> index e14d3d778b71..5d164fe511ba 100644
> >>> --- a/arch/arm64/boot/dts/qcom/sm8650.dtsi
> >>> +++ b/arch/arm64/boot/dts/qcom/sm8650.dtsi
> >>> @@ -3982,7 +3982,12 @@ ufs_mem_phy: phy@1d80000 {
> >>>  
> >>>  		ufs_mem_hc: ufshc@1d84000 {
> >>>  			compatible = "qcom,sm8650-ufshc", "qcom,ufshc", "jedec,ufs-2.0";
> >>> -			reg = <0 0x01d84000 0 0x3000>;
> >>> +			reg = <0 0x01d84000 0 0x3000>,
> >>> +			      <0 0x01da5000 0 0x2000>,
> >>> +			      <0 0x01da4000 0 0x0010>;
> >>
> >>
> >> These are wrong address spaces. Open your datasheet and look there.
> >>
> > Hi Krzysztof,
> > 
> > I’ve reviewed it again, and it is correct and functioning as expected both on our upstream and downstream codebase.
> > I think it is probably overlooked by you. Can you please double check from your end?
> > 
> 
> No, it is not overlooked. There is no address space of length 0x10 at
> 0x01da4000 in qcom doc/datasheet system. Just open the doc and look
> there by yourself. The size is 0x15000.
> 

The whole UFS MCQ region is indeed of size 0x15000, but the SQD and VS registers
are at random offsets, not fixed across the SoC revisions. And there are some
big holes within the whole region for things like ICE and all.

So it makes sense to map only the part of these regions and leave the unused
ones.

But again, this should've been clearly explained in the patch description. I
hope it will be taken care in the next version.

- Mani

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH V1 0/3] Enable UFS MCQ support for SM8650 and SM8750

[Manivannan Sadhasivam]

On Thu, Jul 31, 2025 at 10:50:21AM GMT, neil.armstrong@linaro.org wrote:
> Hi,
> 
> On 30/07/2025 10:22, Ram Kumar Dwivedi wrote:
> > This patch series enables Multi-Circular Queue (MCQ) support for the UFS
> > host controller on Qualcomm SM8650 and SM8750 platforms. MCQ is a modern
> > queuing model that improves performance and scalability by allowing
> > multiple hardware queues.
> > 
> > Although MCQ support has been present in the UFS driver for several years,
> > this is the first time it is being enabled via Device Tree for these
> > platforms.
> > 
> > Patch 1 updates the device tree bindings to allow the additional register
> > regions and reg-names required for MCQ operation.
> > 
> > Patches 2 and 3 update the device trees for SM8650 and SM8750 respectively
> > to enable MCQ by adding the necessary register mappings and MSI parent.
> > 
> > Tested on internal hardware for both platforms.
> > 
> > Palash Kambar (1):
> >    arm64: dts: qcom: sm8750: Enable MCQ support for UFS controller
> > 
> > Ram Kumar Dwivedi (2):
> >    dt-bindings: ufs: qcom: Add MCQ support to reg and reg-names
> >    arm64: dts: qcom: sm8650: Enable MCQ support for UFS controller
> > 
> >   .../devicetree/bindings/ufs/qcom,ufs.yaml     | 21 ++++++++++++-------
> >   arch/arm64/boot/dts/qcom/sm8650.dtsi          |  9 +++++++-
> >   arch/arm64/boot/dts/qcom/sm8750.dtsi          | 10 +++++++--
> >   3 files changed, 29 insertions(+), 11 deletions(-)
> > 
> 
> I ran some tests on the SM8650-QRD, and it works so please add my:
> Tested-by: Neil Armstrong <neil.armstrong@linaro.org> # on SM8650-QRD
> 

Thanks Neil for testing it out!

> I ran some fio tests, comparing the v6.15, v6.16 (with threaded irqs)
> and next + mcq support, and here's the analysis on the results:
> 
> Significant Performance Gains in Write Operations with Multiple Jobs:
> The "mcq" change shows a substantial improvement in both IOPS and bandwidth for write operations with 8 jobs.
> Moderate Improvement in Single Job Operations (Read and Write):
> For single job operations (read and write), the "mcq" change generally leads to positive, albeit less dramatic, improvements in IOPS and bandwidth.
> Slight Decrease in Read Operations with Multiple Jobs:
> Interestingly, for read operations with 8 jobs, there's a slight decrease in both IOPS and bandwidth with the "mcq" kernel.
> 
> The raw results are:
> Board: sm8650-qrd
> 
> read / 1 job
>                v6.15     v6.16  next+mcq
> iops (min)  3,996.00  5,921.60  4,661.20
> iops (max)  4,772.80  6,491.20  5,027.60
> iops (avg)  4,526.25  6,295.31  4,979.81
> cpu % usr       4.62      2.96      5.68
> cpu % sys      21.45     17.88     25.58
> bw (MB/s)      18.54     25.78     20.40
> 

It is interesting to note the % of CPU time spent with MCQ in the 1 job case.
Looks like it is spending more time here. I'm wondering if it is the ESI
limitation/overhead.

- Mani

> read / 8 job
>                 v6.15      v6.16   next+mcq
> iops (min)  51,867.60  51,575.40  56,818.40
> iops (max)  67,513.60  64,456.40  65,379.60
> iops (avg)  64,314.80  62,136.76  63,016.07
> cpu % usr        3.98       3.72       3.85
> cpu % sys       16.70      17.16      14.87
> bw (MB/s)      263.60     254.40     258.20
> 
> write / 1 job
>                v6.15     v6.16  next+mcq
> iops (min)  5,654.80  8,060.00  7,117.20
> iops (max)  6,720.40  8,852.00  7,706.80
> iops (avg)  6,576.91  8,579.81  7,459.97
> cpu % usr       7.48      3.79      6.73
> cpu % sys      41.09     23.27     30.66
> bw (MB/s)      26.96     35.16     30.56
> 
> write / 8 job
>                  v6.15       v6.16    next+mcq
> iops (min)   84,687.80   95,043.40  114,054.00
> iops (max)  107,620.80  113,572.00  164,526.00
> iops (avg)   97,910.86  105,927.38  149,071.43
> cpu % usr         5.43        4.38        2.88
> cpu % sys        21.73       20.29       16.09
> bw (MB/s)       400.80      433.80      610.40
> 
> The test suite is:
> for rw in read write ; do
>     echo "rw: ${rw}"
>     for jobs in 1 8 ; do
>         echo "jobs: ${jobs}"
>         for it in $(seq 1 5) ; do
>             fio --name=rand${rw} --rw=rand${rw} \
>                 --ioengine=libaio --direct=1 \
>                 --bs=4k --numjobs=${jobs} --size=32m \
>                 --runtime=30 --time_based --end_fsync=1 \
>                 --group_reporting --filename=/dev/disk/by-partlabel/super \
>             | grep -E '(iops|sys=|READ:|WRITE:)'
>             sleep 5
>         done
>     done
> done
> 
> Thanks,
> Neil

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH V1 2/3] arm64: dts: qcom: sm8650: Enable MCQ support for UFS controller

[Krzysztof Kozlowski]

On 01/08/2025 14:24, Manivannan Sadhasivam wrote:
> On Thu, Jul 31, 2025 at 10:38:56AM GMT, Krzysztof Kozlowski wrote:
>> On 31/07/2025 10:34, Ram Kumar Dwivedi wrote:
>>>
>>>
>>> On 31-Jul-25 12:15 PM, Krzysztof Kozlowski wrote:
>>>> On 30/07/2025 10:22, Ram Kumar Dwivedi wrote:
>>>>> Enable Multi-Circular Queue (MCQ) support for the UFS host controller
>>>>> on the Qualcomm SM8650 platform by updating the device tree node. This
>>>>> includes adding new register regions and specifying the MSI parent
>>>>> required for MCQ operation.
>>>>>
>>>>> MCQ is a modern queuing model for UFS that improves performance and
>>>>> scalability by allowing multiple hardware queues. 
>>>>>
>>>>> Changes:
>>>>> - Add reg entries for mcq_sqd and mcq_vs regions.
>>>>> - Define reg-names for the new regions.
>>>>> - Specify msi-parent for interrupt routing.
>>>>>
>>>>> Signed-off-by: Ram Kumar Dwivedi <quic_rdwivedi@quicinc.com>
>>>>> ---
>>>>>  arch/arm64/boot/dts/qcom/sm8650.dtsi | 9 ++++++++-
>>>>>  1 file changed, 8 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/arch/arm64/boot/dts/qcom/sm8650.dtsi b/arch/arm64/boot/dts/qcom/sm8650.dtsi
>>>>> index e14d3d778b71..5d164fe511ba 100644
>>>>> --- a/arch/arm64/boot/dts/qcom/sm8650.dtsi
>>>>> +++ b/arch/arm64/boot/dts/qcom/sm8650.dtsi
>>>>> @@ -3982,7 +3982,12 @@ ufs_mem_phy: phy@1d80000 {
>>>>>  
>>>>>  		ufs_mem_hc: ufshc@1d84000 {
>>>>>  			compatible = "qcom,sm8650-ufshc", "qcom,ufshc", "jedec,ufs-2.0";
>>>>> -			reg = <0 0x01d84000 0 0x3000>;
>>>>> +			reg = <0 0x01d84000 0 0x3000>,
>>>>> +			      <0 0x01da5000 0 0x2000>,
>>>>> +			      <0 0x01da4000 0 0x0010>;
>>>>
>>>>
>>>> These are wrong address spaces. Open your datasheet and look there.
>>>>
>>> Hi Krzysztof,
>>>
>>> I’ve reviewed it again, and it is correct and functioning as expected both on our upstream and downstream codebase.
>>> I think it is probably overlooked by you. Can you please double check from your end?
>>>
>>
>> No, it is not overlooked. There is no address space of length 0x10 at
>> 0x01da4000 in qcom doc/datasheet system. Just open the doc and look
>> there by yourself. The size is 0x15000.
>>
> 
> The whole UFS MCQ region is indeed of size 0x15000, but the SQD and VS registers
> are at random offsets, not fixed across the SoC revisions. And there are some
> big holes within the whole region for things like ICE and all.
> 
> So it makes sense to map only the part of these regions and leave the unused
> ones.
Each item in the reg represents some continuous, dedicated address
space, not individual registers or artificially decided subsection. The
holes in such address space is not a problem, we do it all the time for
all other devices as well.

You need to use the definition of that address space.

Best regards,
Krzysztof

Re: [PATCH V1 2/3] arm64: dts: qcom: sm8650: Enable MCQ support for UFS controller

[Manivannan Sadhasivam]

On Fri, Aug 01, 2025 at 04:20:37PM GMT, Krzysztof Kozlowski wrote:
> On 01/08/2025 14:24, Manivannan Sadhasivam wrote:
> > On Thu, Jul 31, 2025 at 10:38:56AM GMT, Krzysztof Kozlowski wrote:
> >> On 31/07/2025 10:34, Ram Kumar Dwivedi wrote:
> >>>
> >>>
> >>> On 31-Jul-25 12:15 PM, Krzysztof Kozlowski wrote:
> >>>> On 30/07/2025 10:22, Ram Kumar Dwivedi wrote:
> >>>>> Enable Multi-Circular Queue (MCQ) support for the UFS host controller
> >>>>> on the Qualcomm SM8650 platform by updating the device tree node. This
> >>>>> includes adding new register regions and specifying the MSI parent
> >>>>> required for MCQ operation.
> >>>>>
> >>>>> MCQ is a modern queuing model for UFS that improves performance and
> >>>>> scalability by allowing multiple hardware queues. 
> >>>>>
> >>>>> Changes:
> >>>>> - Add reg entries for mcq_sqd and mcq_vs regions.
> >>>>> - Define reg-names for the new regions.
> >>>>> - Specify msi-parent for interrupt routing.
> >>>>>
> >>>>> Signed-off-by: Ram Kumar Dwivedi <quic_rdwivedi@quicinc.com>
> >>>>> ---
> >>>>>  arch/arm64/boot/dts/qcom/sm8650.dtsi | 9 ++++++++-
> >>>>>  1 file changed, 8 insertions(+), 1 deletion(-)
> >>>>>
> >>>>> diff --git a/arch/arm64/boot/dts/qcom/sm8650.dtsi b/arch/arm64/boot/dts/qcom/sm8650.dtsi
> >>>>> index e14d3d778b71..5d164fe511ba 100644
> >>>>> --- a/arch/arm64/boot/dts/qcom/sm8650.dtsi
> >>>>> +++ b/arch/arm64/boot/dts/qcom/sm8650.dtsi
> >>>>> @@ -3982,7 +3982,12 @@ ufs_mem_phy: phy@1d80000 {
> >>>>>  
> >>>>>  		ufs_mem_hc: ufshc@1d84000 {
> >>>>>  			compatible = "qcom,sm8650-ufshc", "qcom,ufshc", "jedec,ufs-2.0";
> >>>>> -			reg = <0 0x01d84000 0 0x3000>;
> >>>>> +			reg = <0 0x01d84000 0 0x3000>,
> >>>>> +			      <0 0x01da5000 0 0x2000>,
> >>>>> +			      <0 0x01da4000 0 0x0010>;
> >>>>
> >>>>
> >>>> These are wrong address spaces. Open your datasheet and look there.
> >>>>
> >>> Hi Krzysztof,
> >>>
> >>> I’ve reviewed it again, and it is correct and functioning as expected both on our upstream and downstream codebase.
> >>> I think it is probably overlooked by you. Can you please double check from your end?
> >>>
> >>
> >> No, it is not overlooked. There is no address space of length 0x10 at
> >> 0x01da4000 in qcom doc/datasheet system. Just open the doc and look
> >> there by yourself. The size is 0x15000.
> >>
> > 
> > The whole UFS MCQ region is indeed of size 0x15000, but the SQD and VS registers
> > are at random offsets, not fixed across the SoC revisions. And there are some
> > big holes within the whole region for things like ICE and all.
> > 
> > So it makes sense to map only the part of these regions and leave the unused
> > ones.
> Each item in the reg represents some continuous, dedicated address
> space, not individual registers or artificially decided subsection. The
> holes in such address space is not a problem, we do it all the time for
> all other devices as well.
> 
> You need to use the definition of that address space.
> 

What if some of the registers in that whole address space is shared with other
peripherals such as ICE?

I agree with the fact that artifically creating separate register spaces leads
to issues, but here I'm worried about hardcoding the offsets in the driver which
can change between SoCs and also the shared address space with ICE.

- Mani

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH V1 2/3] arm64: dts: qcom: sm8650: Enable MCQ support for UFS controller

[Krzysztof Kozlowski]

On 01/08/2025 17:33, Manivannan Sadhasivam wrote:
> On Fri, Aug 01, 2025 at 04:20:37PM GMT, Krzysztof Kozlowski wrote:
>> On 01/08/2025 14:24, Manivannan Sadhasivam wrote:
>>> On Thu, Jul 31, 2025 at 10:38:56AM GMT, Krzysztof Kozlowski wrote:
>>>> On 31/07/2025 10:34, Ram Kumar Dwivedi wrote:
>>>>>
>>>>>
>>>>> On 31-Jul-25 12:15 PM, Krzysztof Kozlowski wrote:
>>>>>> On 30/07/2025 10:22, Ram Kumar Dwivedi wrote:
>>>>>>> Enable Multi-Circular Queue (MCQ) support for the UFS host controller
>>>>>>> on the Qualcomm SM8650 platform by updating the device tree node. This
>>>>>>> includes adding new register regions and specifying the MSI parent
>>>>>>> required for MCQ operation.
>>>>>>>
>>>>>>> MCQ is a modern queuing model for UFS that improves performance and
>>>>>>> scalability by allowing multiple hardware queues. 
>>>>>>>
>>>>>>> Changes:
>>>>>>> - Add reg entries for mcq_sqd and mcq_vs regions.
>>>>>>> - Define reg-names for the new regions.
>>>>>>> - Specify msi-parent for interrupt routing.
>>>>>>>
>>>>>>> Signed-off-by: Ram Kumar Dwivedi <quic_rdwivedi@quicinc.com>
>>>>>>> ---
>>>>>>>  arch/arm64/boot/dts/qcom/sm8650.dtsi | 9 ++++++++-
>>>>>>>  1 file changed, 8 insertions(+), 1 deletion(-)
>>>>>>>
>>>>>>> diff --git a/arch/arm64/boot/dts/qcom/sm8650.dtsi b/arch/arm64/boot/dts/qcom/sm8650.dtsi
>>>>>>> index e14d3d778b71..5d164fe511ba 100644
>>>>>>> --- a/arch/arm64/boot/dts/qcom/sm8650.dtsi
>>>>>>> +++ b/arch/arm64/boot/dts/qcom/sm8650.dtsi
>>>>>>> @@ -3982,7 +3982,12 @@ ufs_mem_phy: phy@1d80000 {
>>>>>>>  
>>>>>>>  		ufs_mem_hc: ufshc@1d84000 {
>>>>>>>  			compatible = "qcom,sm8650-ufshc", "qcom,ufshc", "jedec,ufs-2.0";
>>>>>>> -			reg = <0 0x01d84000 0 0x3000>;
>>>>>>> +			reg = <0 0x01d84000 0 0x3000>,
>>>>>>> +			      <0 0x01da5000 0 0x2000>,
>>>>>>> +			      <0 0x01da4000 0 0x0010>;
>>>>>>
>>>>>>
>>>>>> These are wrong address spaces. Open your datasheet and look there.
>>>>>>
>>>>> Hi Krzysztof,
>>>>>
>>>>> I’ve reviewed it again, and it is correct and functioning as expected both on our upstream and downstream codebase.
>>>>> I think it is probably overlooked by you. Can you please double check from your end?
>>>>>
>>>>
>>>> No, it is not overlooked. There is no address space of length 0x10 at
>>>> 0x01da4000 in qcom doc/datasheet system. Just open the doc and look
>>>> there by yourself. The size is 0x15000.
>>>>
>>>
>>> The whole UFS MCQ region is indeed of size 0x15000, but the SQD and VS registers
>>> are at random offsets, not fixed across the SoC revisions. And there are some
>>> big holes within the whole region for things like ICE and all.
>>>
>>> So it makes sense to map only the part of these regions and leave the unused
>>> ones.
>> Each item in the reg represents some continuous, dedicated address
>> space, not individual registers or artificially decided subsection. The
>> holes in such address space is not a problem, we do it all the time for
>> all other devices as well.
>>
>> You need to use the definition of that address space.
>>
> 
> What if some of the registers in that whole address space is shared with other
> peripherals such as ICE?


It will be a different address space. We don't talk about imaginary
3rd-party SoC. Qualcomm datasheet lists address spaces in very precise
way. We were recently fixing all address spaces for remoterpocs based on
that.

> 
> I agree with the fact that artifically creating separate register spaces leads
> to issues, but here I'm worried about hardcoding the offsets in the driver which
> can change between SoCs and also the shared address space with ICE.

Drivers are expected to hard-code offsets and all drivers do it. Look at
display, sound codecs (both SoC and soundwire devices). Everything
hard-coded offsets internal to address space.

What you essentially want is (making it border case) "reg" per register.

Best regards,
Krzysztof

Re: [PATCH V1 2/3] arm64: dts: qcom: sm8650: Enable MCQ support for UFS controller

[Manivannan Sadhasivam]

On Fri, Aug 01, 2025 at 06:09:18PM GMT, Krzysztof Kozlowski wrote:
> On 01/08/2025 17:33, Manivannan Sadhasivam wrote:
> > On Fri, Aug 01, 2025 at 04:20:37PM GMT, Krzysztof Kozlowski wrote:
> >> On 01/08/2025 14:24, Manivannan Sadhasivam wrote:
> >>> On Thu, Jul 31, 2025 at 10:38:56AM GMT, Krzysztof Kozlowski wrote:
> >>>> On 31/07/2025 10:34, Ram Kumar Dwivedi wrote:
> >>>>>
> >>>>>
> >>>>> On 31-Jul-25 12:15 PM, Krzysztof Kozlowski wrote:
> >>>>>> On 30/07/2025 10:22, Ram Kumar Dwivedi wrote:
> >>>>>>> Enable Multi-Circular Queue (MCQ) support for the UFS host controller
> >>>>>>> on the Qualcomm SM8650 platform by updating the device tree node. This
> >>>>>>> includes adding new register regions and specifying the MSI parent
> >>>>>>> required for MCQ operation.
> >>>>>>>
> >>>>>>> MCQ is a modern queuing model for UFS that improves performance and
> >>>>>>> scalability by allowing multiple hardware queues. 
> >>>>>>>
> >>>>>>> Changes:
> >>>>>>> - Add reg entries for mcq_sqd and mcq_vs regions.
> >>>>>>> - Define reg-names for the new regions.
> >>>>>>> - Specify msi-parent for interrupt routing.
> >>>>>>>
> >>>>>>> Signed-off-by: Ram Kumar Dwivedi <quic_rdwivedi@quicinc.com>
> >>>>>>> ---
> >>>>>>>  arch/arm64/boot/dts/qcom/sm8650.dtsi | 9 ++++++++-
> >>>>>>>  1 file changed, 8 insertions(+), 1 deletion(-)
> >>>>>>>
> >>>>>>> diff --git a/arch/arm64/boot/dts/qcom/sm8650.dtsi b/arch/arm64/boot/dts/qcom/sm8650.dtsi
> >>>>>>> index e14d3d778b71..5d164fe511ba 100644
> >>>>>>> --- a/arch/arm64/boot/dts/qcom/sm8650.dtsi
> >>>>>>> +++ b/arch/arm64/boot/dts/qcom/sm8650.dtsi
> >>>>>>> @@ -3982,7 +3982,12 @@ ufs_mem_phy: phy@1d80000 {
> >>>>>>>  
> >>>>>>>  		ufs_mem_hc: ufshc@1d84000 {
> >>>>>>>  			compatible = "qcom,sm8650-ufshc", "qcom,ufshc", "jedec,ufs-2.0";
> >>>>>>> -			reg = <0 0x01d84000 0 0x3000>;
> >>>>>>> +			reg = <0 0x01d84000 0 0x3000>,
> >>>>>>> +			      <0 0x01da5000 0 0x2000>,
> >>>>>>> +			      <0 0x01da4000 0 0x0010>;
> >>>>>>
> >>>>>>
> >>>>>> These are wrong address spaces. Open your datasheet and look there.
> >>>>>>
> >>>>> Hi Krzysztof,
> >>>>>
> >>>>> I’ve reviewed it again, and it is correct and functioning as expected both on our upstream and downstream codebase.
> >>>>> I think it is probably overlooked by you. Can you please double check from your end?
> >>>>>
> >>>>
> >>>> No, it is not overlooked. There is no address space of length 0x10 at
> >>>> 0x01da4000 in qcom doc/datasheet system. Just open the doc and look
> >>>> there by yourself. The size is 0x15000.
> >>>>
> >>>
> >>> The whole UFS MCQ region is indeed of size 0x15000, but the SQD and VS registers
> >>> are at random offsets, not fixed across the SoC revisions. And there are some
> >>> big holes within the whole region for things like ICE and all.
> >>>
> >>> So it makes sense to map only the part of these regions and leave the unused
> >>> ones.
> >> Each item in the reg represents some continuous, dedicated address
> >> space, not individual registers or artificially decided subsection. The
> >> holes in such address space is not a problem, we do it all the time for
> >> all other devices as well.
> >>
> >> You need to use the definition of that address space.
> >>
> > 
> > What if some of the registers in that whole address space is shared with other
> > peripherals such as ICE?
> 
> 
> It will be a different address space. We don't talk about imaginary
> 3rd-party SoC. Qualcomm datasheet lists address spaces in very precise
> way. We were recently fixing all address spaces for remoterpocs based on
> that.
> 
> > 
> > I agree with the fact that artifically creating separate register spaces leads
> > to issues, but here I'm worried about hardcoding the offsets in the driver which
> > can change between SoCs and also the shared address space with ICE.
> 
> Drivers are expected to hard-code offsets and all drivers do it. Look at
> display, sound codecs (both SoC and soundwire devices). Everything
> hard-coded offsets internal to address space.
> 
> What you essentially want is (making it border case) "reg" per register.
> 

I was worried about the ICE overlap, but I got access to the documentation and
verified myself (also with Nitin) that there is no ICE overlap. So yes, we can
map the entire MCQ region and live with the hardcoded offsets.

- Mani

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH V1 2/3] arm64: dts: qcom: sm8650: Enable MCQ support for UFS controller

[Ram Kumar Dwivedi]

On 11-Aug-25 2:57 PM, Manivannan Sadhasivam wrote:
> On Fri, Aug 01, 2025 at 06:09:18PM GMT, Krzysztof Kozlowski wrote:
>> On 01/08/2025 17:33, Manivannan Sadhasivam wrote:
>>> On Fri, Aug 01, 2025 at 04:20:37PM GMT, Krzysztof Kozlowski wrote:
>>>> On 01/08/2025 14:24, Manivannan Sadhasivam wrote:
>>>>> On Thu, Jul 31, 2025 at 10:38:56AM GMT, Krzysztof Kozlowski wrote:
>>>>>> On 31/07/2025 10:34, Ram Kumar Dwivedi wrote:
>>>>>>>
>>>>>>>
>>>>>>> On 31-Jul-25 12:15 PM, Krzysztof Kozlowski wrote:
>>>>>>>> On 30/07/2025 10:22, Ram Kumar Dwivedi wrote:
>>>>>>>>> Enable Multi-Circular Queue (MCQ) support for the UFS host controller
>>>>>>>>> on the Qualcomm SM8650 platform by updating the device tree node. This
>>>>>>>>> includes adding new register regions and specifying the MSI parent
>>>>>>>>> required for MCQ operation.
>>>>>>>>>
>>>>>>>>> MCQ is a modern queuing model for UFS that improves performance and
>>>>>>>>> scalability by allowing multiple hardware queues. 
>>>>>>>>>
>>>>>>>>> Changes:
>>>>>>>>> - Add reg entries for mcq_sqd and mcq_vs regions.
>>>>>>>>> - Define reg-names for the new regions.
>>>>>>>>> - Specify msi-parent for interrupt routing.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Ram Kumar Dwivedi <quic_rdwivedi@quicinc.com>
>>>>>>>>> ---
>>>>>>>>>  arch/arm64/boot/dts/qcom/sm8650.dtsi | 9 ++++++++-
>>>>>>>>>  1 file changed, 8 insertions(+), 1 deletion(-)
>>>>>>>>>
>>>>>>>>> diff --git a/arch/arm64/boot/dts/qcom/sm8650.dtsi b/arch/arm64/boot/dts/qcom/sm8650.dtsi
>>>>>>>>> index e14d3d778b71..5d164fe511ba 100644
>>>>>>>>> --- a/arch/arm64/boot/dts/qcom/sm8650.dtsi
>>>>>>>>> +++ b/arch/arm64/boot/dts/qcom/sm8650.dtsi
>>>>>>>>> @@ -3982,7 +3982,12 @@ ufs_mem_phy: phy@1d80000 {
>>>>>>>>>  
>>>>>>>>>  		ufs_mem_hc: ufshc@1d84000 {
>>>>>>>>>  			compatible = "qcom,sm8650-ufshc", "qcom,ufshc", "jedec,ufs-2.0";
>>>>>>>>> -			reg = <0 0x01d84000 0 0x3000>;
>>>>>>>>> +			reg = <0 0x01d84000 0 0x3000>,
>>>>>>>>> +			      <0 0x01da5000 0 0x2000>,
>>>>>>>>> +			      <0 0x01da4000 0 0x0010>;
>>>>>>>>
>>>>>>>>
>>>>>>>> These are wrong address spaces. Open your datasheet and look there.
>>>>>>>>
>>>>>>> Hi Krzysztof,
>>>>>>>
>>>>>>> I’ve reviewed it again, and it is correct and functioning as expected both on our upstream and downstream codebase.
>>>>>>> I think it is probably overlooked by you. Can you please double check from your end?
>>>>>>>
>>>>>>
>>>>>> No, it is not overlooked. There is no address space of length 0x10 at
>>>>>> 0x01da4000 in qcom doc/datasheet system. Just open the doc and look
>>>>>> there by yourself. The size is 0x15000.
>>>>>>
>>>>>
>>>>> The whole UFS MCQ region is indeed of size 0x15000, but the SQD and VS registers
>>>>> are at random offsets, not fixed across the SoC revisions. And there are some
>>>>> big holes within the whole region for things like ICE and all.
Hi Krzysztof,

I have addressed your comment by using single MCQ region mapping and accordingly updated dt bindings.

Thanks,
Ram.
>>>>>
>>>>> So it makes sense to map only the part of these regions and leave the unused
>>>>> ones.
>>>> Each item in the reg represents some continuous, dedicated address
>>>> space, not individual registers or artificially decided subsection. The
>>>> holes in such address space is not a problem, we do it all the time for
>>>> all other devices as well.
>>>>
>>>> You need to use the definition of that address space.
>>>>
>>>
>>> What if some of the registers in that whole address space is shared with other
>>> peripherals such as ICE?
>>
>>
>> It will be a different address space. We don't talk about imaginary
>> 3rd-party SoC. Qualcomm datasheet lists address spaces in very precise
>> way. We were recently fixing all address spaces for remoterpocs based on
>> that.
>>
>>>
>>> I agree with the fact that artifically creating separate register spaces leads
>>> to issues, but here I'm worried about hardcoding the offsets in the driver which
>>> can change between SoCs and also the shared address space with ICE.
>>
>> Drivers are expected to hard-code offsets and all drivers do it. Look at
>> display, sound codecs (both SoC and soundwire devices). Everything
>> hard-coded offsets internal to address space.
>>
>> What you essentially want is (making it border case) "reg" per register.
>>
> 
> I was worried about the ICE overlap, but I got access to the documentation and
> verified myself (also with Nitin) that there is no ICE overlap. So yes, we can
> map the entire MCQ region and live with the hardcoded offsets.
> 
> - Mani
> 

Re: [PATCH V1 2/3] arm64: dts: qcom: sm8650: Enable MCQ support for UFS controller

[Konrad Dybcio]

On 8/11/25 11:27 AM, Manivannan Sadhasivam wrote:
> On Fri, Aug 01, 2025 at 06:09:18PM GMT, Krzysztof Kozlowski wrote:
>> On 01/08/2025 17:33, Manivannan Sadhasivam wrote:
>>> On Fri, Aug 01, 2025 at 04:20:37PM GMT, Krzysztof Kozlowski wrote:
>>>> On 01/08/2025 14:24, Manivannan Sadhasivam wrote:
>>>>> On Thu, Jul 31, 2025 at 10:38:56AM GMT, Krzysztof Kozlowski wrote:
>>>>>> On 31/07/2025 10:34, Ram Kumar Dwivedi wrote:
>>>>>>>
>>>>>>>
>>>>>>> On 31-Jul-25 12:15 PM, Krzysztof Kozlowski wrote:
>>>>>>>> On 30/07/2025 10:22, Ram Kumar Dwivedi wrote:
>>>>>>>>> Enable Multi-Circular Queue (MCQ) support for the UFS host controller
>>>>>>>>> on the Qualcomm SM8650 platform by updating the device tree node. This
>>>>>>>>> includes adding new register regions and specifying the MSI parent
>>>>>>>>> required for MCQ operation.
>>>>>>>>>
>>>>>>>>> MCQ is a modern queuing model for UFS that improves performance and
>>>>>>>>> scalability by allowing multiple hardware queues. 
>>>>>>>>>
>>>>>>>>> Changes:
>>>>>>>>> - Add reg entries for mcq_sqd and mcq_vs regions.
>>>>>>>>> - Define reg-names for the new regions.
>>>>>>>>> - Specify msi-parent for interrupt routing.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Ram Kumar Dwivedi <quic_rdwivedi@quicinc.com>
>>>>>>>>> ---
>>>>>>>>>  arch/arm64/boot/dts/qcom/sm8650.dtsi | 9 ++++++++-
>>>>>>>>>  1 file changed, 8 insertions(+), 1 deletion(-)
>>>>>>>>>
>>>>>>>>> diff --git a/arch/arm64/boot/dts/qcom/sm8650.dtsi b/arch/arm64/boot/dts/qcom/sm8650.dtsi
>>>>>>>>> index e14d3d778b71..5d164fe511ba 100644
>>>>>>>>> --- a/arch/arm64/boot/dts/qcom/sm8650.dtsi
>>>>>>>>> +++ b/arch/arm64/boot/dts/qcom/sm8650.dtsi
>>>>>>>>> @@ -3982,7 +3982,12 @@ ufs_mem_phy: phy@1d80000 {
>>>>>>>>>  
>>>>>>>>>  		ufs_mem_hc: ufshc@1d84000 {
>>>>>>>>>  			compatible = "qcom,sm8650-ufshc", "qcom,ufshc", "jedec,ufs-2.0";
>>>>>>>>> -			reg = <0 0x01d84000 0 0x3000>;
>>>>>>>>> +			reg = <0 0x01d84000 0 0x3000>,
>>>>>>>>> +			      <0 0x01da5000 0 0x2000>,
>>>>>>>>> +			      <0 0x01da4000 0 0x0010>;
>>>>>>>>
>>>>>>>>
>>>>>>>> These are wrong address spaces. Open your datasheet and look there.
>>>>>>>>
>>>>>>> Hi Krzysztof,
>>>>>>>
>>>>>>> I’ve reviewed it again, and it is correct and functioning as expected both on our upstream and downstream codebase.
>>>>>>> I think it is probably overlooked by you. Can you please double check from your end?
>>>>>>>
>>>>>>
>>>>>> No, it is not overlooked. There is no address space of length 0x10 at
>>>>>> 0x01da4000 in qcom doc/datasheet system. Just open the doc and look
>>>>>> there by yourself. The size is 0x15000.
>>>>>>
>>>>>
>>>>> The whole UFS MCQ region is indeed of size 0x15000, but the SQD and VS registers
>>>>> are at random offsets, not fixed across the SoC revisions. And there are some
>>>>> big holes within the whole region for things like ICE and all.
>>>>>
>>>>> So it makes sense to map only the part of these regions and leave the unused
>>>>> ones.
>>>> Each item in the reg represents some continuous, dedicated address
>>>> space, not individual registers or artificially decided subsection. The
>>>> holes in such address space is not a problem, we do it all the time for
>>>> all other devices as well.
>>>>
>>>> You need to use the definition of that address space.
>>>>
>>>
>>> What if some of the registers in that whole address space is shared with other
>>> peripherals such as ICE?
>>
>>
>> It will be a different address space. We don't talk about imaginary
>> 3rd-party SoC. Qualcomm datasheet lists address spaces in very precise
>> way. We were recently fixing all address spaces for remoterpocs based on
>> that.
>>
>>>
>>> I agree with the fact that artifically creating separate register spaces leads
>>> to issues, but here I'm worried about hardcoding the offsets in the driver which
>>> can change between SoCs and also the shared address space with ICE.
>>
>> Drivers are expected to hard-code offsets and all drivers do it. Look at
>> display, sound codecs (both SoC and soundwire devices). Everything
>> hard-coded offsets internal to address space.
>>
>> What you essentially want is (making it border case) "reg" per register.
>>
> 
> I was worried about the ICE overlap, but I got access to the documentation and
> verified myself (also with Nitin) that there is no ICE overlap. So yes, we can
> map the entire MCQ region and live with the hardcoded offsets.

(that means we can look into ripping out lots of code from the ufs driver
as I attempted to before, feel free to take the wheel on that though)

Konrad