[v2,4/6] arm64: dts: qcom: sdm845: move DSI/QUP/QSPI opp tables out of SoC node

  The SoC node is a simple-bus and its schema expect to have nodes only
with unit addresses:

  sdm850-lenovo-yoga-c630.dtb: soc@0: opp-table-qup: {'compatible': ['operating-points-v2'], 'phandle': [[60]], 'opp-50000000':
  ... 'required-opps': [[55]]}} should not be valid under {'type': 'object'}

Move to top-level OPP tables:
 - DSI and QUP which are shared between multiple nodes,
 - QSPI which cannot be placed in its node due to address/size cells.

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>

---

Changes since v1:
1. Only rebase due to node reorderings.
2. Add Rb tag.
---
 arch/arm64/boot/dts/qcom/sdm845.dtsi | 154 +++++++++++++--------------
 1 file changed, 77 insertions(+), 77 deletions(-)
  

On 12 December 2022 13:02:30 GMT+03:00, Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:
>The SoC node is a simple-bus and its schema expect to have nodes only
>with unit addresses:
>
>  sdm850-lenovo-yoga-c630.dtb: soc@0: opp-table-qup: {'compatible': ['operating-points-v2'], 'phandle': [[60]], 'opp-50000000':
>  ... 'required-opps': [[55]]}} should not be valid under {'type': 'object'}
>
>Move to top-level OPP tables:
> - DSI and QUP which are shared between multiple nodes,

This makes me rise a question: on other platforms we have been placing the shared dsi opp table into one of DSI nodes (usually into the second one). Should we also move such tables up to the top level?


> - QSPI which cannot be placed in its node due to address/size cells.
>
>Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>

Reviewed-by: Dmitry Baryshkov<dmitry.baryshkov@linaro.org>

>
>---
>
>Changes since v1:
>1. Only rebase due to node reorderings.
>2. Add Rb tag.
>---
> arch/arm64/boot/dts/qcom/sdm845.dtsi | 154 +++++++++++++--------------
> 1 file changed, 77 insertions(+), 77 deletions(-)
>
>diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
>index 88e7d4061aae..8eeb3aa261d5 100644
>--- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
>+++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
>@@ -604,6 +604,83 @@ cpu4_opp32: opp-2803200000 {
> 		};
> 	};
> 
>+	dsi_opp_table: opp-table-dsi {
>+		compatible = "operating-points-v2";
>+
>+		opp-19200000 {
>+			opp-hz = /bits/ 64 <19200000>;
>+			required-opps = <&rpmhpd_opp_min_svs>;
>+		};
>+
>+		opp-180000000 {
>+			opp-hz = /bits/ 64 <180000000>;
>+			required-opps = <&rpmhpd_opp_low_svs>;
>+		};
>+
>+		opp-275000000 {
>+			opp-hz = /bits/ 64 <275000000>;
>+			required-opps = <&rpmhpd_opp_svs>;
>+		};
>+
>+		opp-328580000 {
>+			opp-hz = /bits/ 64 <328580000>;
>+			required-opps = <&rpmhpd_opp_svs_l1>;
>+		};
>+
>+		opp-358000000 {
>+			opp-hz = /bits/ 64 <358000000>;
>+			required-opps = <&rpmhpd_opp_nom>;
>+		};
>+	};
>+
>+	qspi_opp_table: opp-table-qspi {
>+		compatible = "operating-points-v2";
>+
>+		opp-19200000 {
>+			opp-hz = /bits/ 64 <19200000>;
>+			required-opps = <&rpmhpd_opp_min_svs>;
>+		};
>+
>+		opp-100000000 {
>+			opp-hz = /bits/ 64 <100000000>;
>+			required-opps = <&rpmhpd_opp_low_svs>;
>+		};
>+
>+		opp-150000000 {
>+			opp-hz = /bits/ 64 <150000000>;
>+			required-opps = <&rpmhpd_opp_svs>;
>+		};
>+
>+		opp-300000000 {
>+			opp-hz = /bits/ 64 <300000000>;
>+			required-opps = <&rpmhpd_opp_nom>;
>+		};
>+	};
>+
>+	qup_opp_table: opp-table-qup {
>+		compatible = "operating-points-v2";
>+
>+		opp-50000000 {
>+			opp-hz = /bits/ 64 <50000000>;
>+			required-opps = <&rpmhpd_opp_min_svs>;
>+		};
>+
>+		opp-75000000 {
>+			opp-hz = /bits/ 64 <75000000>;
>+			required-opps = <&rpmhpd_opp_low_svs>;
>+		};
>+
>+		opp-100000000 {
>+			opp-hz = /bits/ 64 <100000000>;
>+			required-opps = <&rpmhpd_opp_svs>;
>+		};
>+
>+		opp-128000000 {
>+			opp-hz = /bits/ 64 <128000000>;
>+			required-opps = <&rpmhpd_opp_nom>;
>+		};
>+	};
>+
> 	pmu {
> 		compatible = "arm,armv8-pmuv3";
> 		interrupts = <GIC_PPI 5 IRQ_TYPE_LEVEL_HIGH>;
>@@ -1117,30 +1194,6 @@ rng: rng@793000 {
> 			clock-names = "core";
> 		};
> 
>-		qup_opp_table: opp-table-qup {
>-			compatible = "operating-points-v2";
>-
>-			opp-50000000 {
>-				opp-hz = /bits/ 64 <50000000>;
>-				required-opps = <&rpmhpd_opp_min_svs>;
>-			};
>-
>-			opp-75000000 {
>-				opp-hz = /bits/ 64 <75000000>;
>-				required-opps = <&rpmhpd_opp_low_svs>;
>-			};
>-
>-			opp-100000000 {
>-				opp-hz = /bits/ 64 <100000000>;
>-				required-opps = <&rpmhpd_opp_svs>;
>-			};
>-
>-			opp-128000000 {
>-				opp-hz = /bits/ 64 <128000000>;
>-				required-opps = <&rpmhpd_opp_nom>;
>-			};
>-		};
>-
> 		gpi_dma0: dma-controller@800000 {
> 			#dma-cells = <3>;
> 			compatible = "qcom,sdm845-gpi-dma";
>@@ -3799,30 +3852,6 @@ opp-201500000 {
> 			};
> 		};
> 
>-		qspi_opp_table: opp-table-qspi {
>-			compatible = "operating-points-v2";
>-
>-			opp-19200000 {
>-				opp-hz = /bits/ 64 <19200000>;
>-				required-opps = <&rpmhpd_opp_min_svs>;
>-			};
>-
>-			opp-100000000 {
>-				opp-hz = /bits/ 64 <100000000>;
>-				required-opps = <&rpmhpd_opp_low_svs>;
>-			};
>-
>-			opp-150000000 {
>-				opp-hz = /bits/ 64 <150000000>;
>-				required-opps = <&rpmhpd_opp_svs>;
>-			};
>-
>-			opp-300000000 {
>-				opp-hz = /bits/ 64 <300000000>;
>-				required-opps = <&rpmhpd_opp_nom>;
>-			};
>-		};
>-
> 		qspi: spi@88df000 {
> 			compatible = "qcom,sdm845-qspi", "qcom,qspi-v1";
> 			reg = <0 0x088df000 0 0x600>;
>@@ -4420,35 +4449,6 @@ clock_camcc: clock-controller@ad00000 {
> 			clock-names = "bi_tcxo";
> 		};
> 
>-		dsi_opp_table: opp-table-dsi {
>-			compatible = "operating-points-v2";
>-
>-			opp-19200000 {
>-				opp-hz = /bits/ 64 <19200000>;
>-				required-opps = <&rpmhpd_opp_min_svs>;
>-			};
>-
>-			opp-180000000 {
>-				opp-hz = /bits/ 64 <180000000>;
>-				required-opps = <&rpmhpd_opp_low_svs>;
>-			};
>-
>-			opp-275000000 {
>-				opp-hz = /bits/ 64 <275000000>;
>-				required-opps = <&rpmhpd_opp_svs>;
>-			};
>-
>-			opp-328580000 {
>-				opp-hz = /bits/ 64 <328580000>;
>-				required-opps = <&rpmhpd_opp_svs_l1>;
>-			};
>-
>-			opp-358000000 {
>-				opp-hz = /bits/ 64 <358000000>;
>-				required-opps = <&rpmhpd_opp_nom>;
>-			};
>-		};
>-
> 		mdss: mdss@ae00000 {
> 			compatible = "qcom,sdm845-mdss";
> 			reg = <0 0x0ae00000 0 0x1000>;
  

On 12/12/2022 14:46, Dmitry Baryshkov wrote:
> 
> 
> On 12 December 2022 13:02:30 GMT+03:00, Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:
>> The SoC node is a simple-bus and its schema expect to have nodes only
>> with unit addresses:
>>
>>  sdm850-lenovo-yoga-c630.dtb: soc@0: opp-table-qup: {'compatible': ['operating-points-v2'], 'phandle': [[60]], 'opp-50000000':
>>  ... 'required-opps': [[55]]}} should not be valid under {'type': 'object'}
>>
>> Move to top-level OPP tables:
>> - DSI and QUP which are shared between multiple nodes,
> 
> This makes me rise a question: on other platforms we have been placing the shared dsi opp table into one of DSI nodes (usually into the second one). Should we also move such tables up to the top level?

Hmm, indeed I see SM8250. The location of the table from
shared/non-shared point of view is one, but second problem is that DSI
has address/size cells. DTC has additional checks for certain buses thus
the QSPI table cannot be inside such node with address/size cells:

Warning (spi_bus_reg): /soc@0/spi@88dc000/opp-table: missing or empty
reg property

This check is not triggered for DSI, but the concept is the same here -
nodes with address/size cells are expected to have only properties, not
other objects. See simple-bus:
https://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/simple-bus.yaml#L59


Best regards,
Krzysztof