| Message ID | 20220521152049.1490220-8-dmitry.baryshkov@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | arm64: dts: qcom: initial Inforce IFC6560 board support | expand |
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 >
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
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 --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>;
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(-)