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

[Krzysztof Kozlowski <krzk@kernel.org>]

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