diff mbox series

[v6,07/11] arm64: dts: qcom: sdm630: fix gpu's interconnect path

Message ID 20220521152049.1490220-8-dmitry.baryshkov@linaro.org
State Superseded
Headers show
Series arm64: dts: qcom: initial Inforce IFC6560 board support | expand

Commit Message

Dmitry Baryshkov May 21, 2022, 3:20 p.m. UTC
ICC path for the GPU incorrectly states <&gnoc 1 &bimc 5>, which is
a path from SLAVE_GNOC_BIMC to SLAVE_EBI. According to the downstream
GPU uses MASTER_OXILI here, <&bimc 1 ...>.

While we are at it, use defined names instead of the numbers for this
interconnect path.

Fixes: 5cf69dcbec8b ("arm64: dts: qcom: sdm630: Add Adreno 508 GPU configuration")
Reported-by: Marijn Suijten <marijn.suijten@somainline.org>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 arch/arm64/boot/dts/qcom/sdm630.dtsi | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Marijn Suijten May 21, 2022, 4:17 p.m. UTC | #1
On 2022-05-21 18:20:45, Dmitry Baryshkov wrote:
> ICC path for the GPU incorrectly states <&gnoc 1 &bimc 5>, which is
> a path from SLAVE_GNOC_BIMC to SLAVE_EBI. According to the downstream

Note that I suggested to reword this sentence for readablity in v5:

https://lore.kernel.org/linux-arm-msm/20220515145108.zfmlb53vacx7sr33@SoMainline.org/

> GPU uses MASTER_OXILI here, <&bimc 1 ...>.
> 
> While we are at it, use defined names instead of the numbers for this
> interconnect path.
> 
> Fixes: 5cf69dcbec8b ("arm64: dts: qcom: sdm630: Add Adreno 508 GPU configuration")
> Reported-by: Marijn Suijten <marijn.suijten@somainline.org>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

And also added my Reviewed-by there.  I think it is normal to have a
Reviewed-by on top of a Suggested-by/Reported-by, so that I as reviewer
confirm the contents of the patch?

Since this is the third patch missing these, It may just have been an
oversight.

- Marijn

> ---
>  arch/arm64/boot/dts/qcom/sdm630.dtsi | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sdm630.dtsi b/arch/arm64/boot/dts/qcom/sdm630.dtsi
> index 2b5dbc12bdf8..bcda3a1dd249 100644
> --- a/arch/arm64/boot/dts/qcom/sdm630.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sdm630.dtsi
> @@ -8,6 +8,7 @@
>  #include <dt-bindings/clock/qcom,gpucc-sdm660.h>
>  #include <dt-bindings/clock/qcom,mmcc-sdm660.h>
>  #include <dt-bindings/clock/qcom,rpmcc.h>
> +#include <dt-bindings/interconnect/qcom,sdm660.h>
>  #include <dt-bindings/power/qcom-rpmpd.h>
>  #include <dt-bindings/gpio/gpio.h>
>  #include <dt-bindings/interrupt-controller/arm-gic.h>
> @@ -1045,7 +1046,7 @@ adreno_gpu: gpu@5000000 {
>  			nvmem-cells = <&gpu_speed_bin>;
>  			nvmem-cell-names = "speed_bin";
>  
> -			interconnects = <&gnoc 1 &bimc 5>;
> +			interconnects = <&bimc MASTER_OXILI &bimc SLAVE_EBI>;
>  			interconnect-names = "gfx-mem";
>  
>  			operating-points-v2 = <&gpu_sdm630_opp_table>;
> -- 
> 2.35.1
>
Krzysztof Kozlowski May 21, 2022, 4:51 p.m. UTC | #2
On 21/05/2022 18:17, Marijn Suijten wrote:
> 
> And also added my Reviewed-by there.  I think it is normal to have a
> Reviewed-by on top of a Suggested-by/Reported-by, so that I as reviewer
> confirm the contents of the patch?

The review tag is not used to confirm anything like that.

> 
> Since this is the third patch missing these, It may just have been an
> oversight.

If your review was meeting the criteria of "Reviewer's statement of
oversight", then of course should be added here.


Best regards,
Krzysztof
Marijn Suijten May 21, 2022, 6:27 p.m. UTC | #3
On 2022-05-21 18:51:07, Krzysztof Kozlowski wrote:
> On 21/05/2022 18:17, Marijn Suijten wrote:
> > 
> > And also added my Reviewed-by there.  I think it is normal to have a
> > Reviewed-by on top of a Suggested-by/Reported-by, so that I as reviewer
> > confirm the contents of the patch?
> 
> The review tag is not used to confirm anything like that.

It is used to acknowledge that I agree on (confirm) the contents of the
patch as per how a review usually works.  I have reviewed the patch,
double-checked the numbers on my end.  That's the "contents of the
patch"?

> > Since this is the third patch missing these, It may just have been an
> > oversight.
> 
> If your review was meeting the criteria of "Reviewer's statement of
> oversight", then of course should be added here.

That's what I said above, the Reviewed-by goes on top of the Reported-by
as the latter doesn't imply the former at all.

- Marijn
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/qcom/sdm630.dtsi b/arch/arm64/boot/dts/qcom/sdm630.dtsi
index 2b5dbc12bdf8..bcda3a1dd249 100644
--- a/arch/arm64/boot/dts/qcom/sdm630.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm630.dtsi
@@ -8,6 +8,7 @@ 
 #include <dt-bindings/clock/qcom,gpucc-sdm660.h>
 #include <dt-bindings/clock/qcom,mmcc-sdm660.h>
 #include <dt-bindings/clock/qcom,rpmcc.h>
+#include <dt-bindings/interconnect/qcom,sdm660.h>
 #include <dt-bindings/power/qcom-rpmpd.h>
 #include <dt-bindings/gpio/gpio.h>
 #include <dt-bindings/interrupt-controller/arm-gic.h>
@@ -1045,7 +1046,7 @@  adreno_gpu: gpu@5000000 {
 			nvmem-cells = <&gpu_speed_bin>;
 			nvmem-cell-names = "speed_bin";
 
-			interconnects = <&gnoc 1 &bimc 5>;
+			interconnects = <&bimc MASTER_OXILI &bimc SLAVE_EBI>;
 			interconnect-names = "gfx-mem";
 
 			operating-points-v2 = <&gpu_sdm630_opp_table>;