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.1On 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
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
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
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
--
மணிவண்ணன் சதாசிவம்
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
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
--
மணிவண்ணன் சதாசிவம்
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
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
--
மணிவண்ணன் சதாசிவம்
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
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
>
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
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.
© 2016 - 2025 Red Hat, Inc.