| 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 |
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
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",
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
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
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
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
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..
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.
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 --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",
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(-)