diff mbox series

arm64: dts: qcom: sm8450: fix interconnects property of UFS node

Message ID 20220310221934.1560729-1-vladimir.zapolskiy@linaro.org (mailing list archive)
State Accepted
Headers show
Series arm64: dts: qcom: sm8450: fix interconnects property of UFS node | expand

Commit Message

Vladimir Zapolskiy March 10, 2022, 10:19 p.m. UTC
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>
---
 arch/arm64/boot/dts/qcom/sm8450.dtsi | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Vinod Koul March 11, 2022, 4:55 a.m. UTC | #1
On 11-03-22, 00: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.

Reviewed-by: Vinod Koul <vkoul@kernel.org>

> 
> Fixes: aa2d0bf04a3c ("arm64: dts: qcom: sm8450: add interconnect nodes")
> Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
> ---
>  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",
> -- 
> 2.33.0
Dmitry Baryshkov April 5, 2022, 3:38 p.m. UTC | #2
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?

> ---
>   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",
Bjorn Andersson April 5, 2022, 5:19 p.m. UTC | #3
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?

Regards,
Bjorn

> > ---
> >   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
Dmitry Baryshkov April 5, 2022, 5:38 p.m. UTC | #4
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.

>
> Regards,
> Bjorn
>
> > > ---
> > >   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
Bjorn Andersson April 12, 2022, 2:35 a.m. UTC | #5
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).

Regards,
Bjorn

> >
> > Regards,
> > Bjorn
> >
> > > > ---
> > > >   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
Vladimir Zapolskiy April 12, 2022, 6:34 a.m. UTC | #6
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
Vinod Koul April 12, 2022, 11:17 a.m. UTC | #7
On 12-04-22, 09:34, Vladimir Zapolskiy wrote:
> 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.

We should either apply this fix or a patch to drop this line from dts.
Either would be apt and latter would make more sense..
Dmitry Baryshkov June 6, 2022, 3:57 p.m. UTC | #8
On 12/04/2022 14:17, Vinod Koul wrote:
> On 12-04-22, 09:34, Vladimir Zapolskiy wrote:
>> 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.
> 
> We should either apply this fix or a patch to drop this line from dts.
> Either would be apt and latter would make more sense..

So, neither of the patches were applied. I'd suggest to apply this one 
now during the -rc stage.
Bjorn Andersson June 27, 2022, 8:03 p.m. UTC | #9
On Fri, 11 Mar 2022 00:19:34 +0200, 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.
> 
> 

Applied, thanks!

[1/1] arm64: dts: qcom: sm8450: fix interconnects property of UFS node
      commit: de9b3d9616078f1d1d0d51b01cdafa101733f935

Best regards,
diff mbox series

Patch

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",