Re: [PATCH] arm64: dts: qcom: sm8450: fix interconnects property of UFS node

From: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
To: Bjorn Andersson <bjorn.andersson@linaro.org>,
	Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Cc: Vinod Koul <vkoul@kernel.org>, Andy Gross <agross@kernel.org>,
	Rob Herring <robh+dt@kernel.org>,
	linux-arm-msm@vger.kernel.org, devicetree@vger.kernel.org
Subject: Re: [PATCH] arm64: dts: qcom: sm8450: fix interconnects property of UFS node
Date: Tue, 12 Apr 2022 09:34:31 +0300	[thread overview]
Message-ID: <2af9a8a7-0904-df31-7c1a-21705bcda8d6@linaro.org> (raw)
In-Reply-To: <YlTlZa35A4lu02oI@builder.lan>

Hi Bjorn,

On 4/12/22 05:35, Bjorn Andersson wrote:
> On Tue 05 Apr 12:38 CDT 2022, Dmitry Baryshkov wrote:
> 
>> On Tue, 5 Apr 2022 at 20:17, Bjorn Andersson <bjorn.andersson@linaro.org> wrote:
>>>
>>> On Tue 05 Apr 08:38 PDT 2022, Dmitry Baryshkov wrote:
>>>
>>>> On 11/03/2022 01:19, Vladimir Zapolskiy wrote:
>>>>> All interconnect device tree nodes on sm8450 are 2-cells, however in
>>>>> UFS node they are handled as 1-cells, fix it.
>>>>>
>>>>> Fixes: aa2d0bf04a3c ("arm64: dts: qcom: sm8450: add interconnect nodes")
>>>>> Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
>>>>
>>>> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>>>>
>>>> Bjorn, could you please this pick for the -rc kernel?
>>>>
>>>
>>> The change is obviously correct, but what difference does this change
>>> make with the current implementation?
>>
>> it makes interconnect paths probe correctly. All NoC have
>> #interconnec-cells = <2> now.
>>
> 
> But there's no code in the UFS driver that calls of_icc_get(), so what
> does this actually do? (Other than correcting the dtb for the day when
> we add that support to the driver).

FWIW the change also has a runtime effect, it fixes a parsing of the board dtb,
otherwise a warning in the kernel log appears:

   OF: /soc@0/ufshc@1d84000: could not get #interconnect-cells for /clocks/sleep-clk

Why /clocks/sleep-clk is mentioned here at all??
Its phandle value is 0x26, which is equal to SLAVE_UFS_MEM_CFG from the array.

--
Best wishes,
Vladimir

>>>>> ---
>>>>>    arch/arm64/boot/dts/qcom/sm8450.dtsi | 4 ++--
>>>>>    1 file changed, 2 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/arch/arm64/boot/dts/qcom/sm8450.dtsi b/arch/arm64/boot/dts/qcom/sm8450.dtsi
>>>>> index 0cd5af8c03bd..bbd38b55e976 100644
>>>>> --- a/arch/arm64/boot/dts/qcom/sm8450.dtsi
>>>>> +++ b/arch/arm64/boot/dts/qcom/sm8450.dtsi
>>>>> @@ -1376,8 +1376,8 @@ ufs_mem_hc: ufshc@1d84000 {
>>>>>                      iommus = <&apps_smmu 0xe0 0x0>;
>>>>> -                   interconnects = <&aggre1_noc MASTER_UFS_MEM &mc_virt SLAVE_EBI1>,
>>>>> -                                   <&gem_noc MASTER_APPSS_PROC &config_noc SLAVE_UFS_MEM_CFG>;
>>>>> +                   interconnects = <&aggre1_noc MASTER_UFS_MEM 0 &mc_virt SLAVE_EBI1 0>,
>>>>> +                                   <&gem_noc MASTER_APPSS_PROC 0 &config_noc SLAVE_UFS_MEM_CFG 0>;
>>>>>                      interconnect-names = "ufs-ddr", "cpu-ufs";
>>>>>                      clock-names =
>>>>>                              "core_clk",
>>>>
>>>>
>>>> --
>>>> With best wishes
>>>> Dmitry
>>
>>
>>
>> -- 
>> With best wishes
>> Dmitry