[PATCH v2] arm64: dts: qcom: sdm630: move DSI opp-table out of soc node

[PATCH v2] arm64: dts: qcom: sdm630: move DSI opp-table out of soc node

[Krzysztof Kozlowski]

The soc node is supposed to have only device nodes with MMIO addresses,
so move the DSI OPP out of it (it is used also by second DSI1 on
SDM660):

  sda660-inforce-ifc6560.dtb: soc: opp-table-dsi: {'compatible': ['operating-points-v2'], ... should not be valid under {'type': 'object'}
    From schema: dtschema/schemas/simple-bus.yaml

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

---

Changes since v1:
1. Move the node out of soc. Don't add Konrad's review tag.
---
 arch/arm64/boot/dts/qcom/sdm630.dtsi | 38 ++++++++++++++--------------
 1 file changed, 19 insertions(+), 19 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sdm630.dtsi b/arch/arm64/boot/dts/qcom/sdm630.dtsi
index 72d9a12b5e9c..b91e423a3cfc 100644
--- a/arch/arm64/boot/dts/qcom/sdm630.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm630.dtsi
@@ -328,6 +328,25 @@ memory@80000000 {
 		reg = <0x0 0x80000000 0x0 0x0>;
 	};
 
+	dsi_opp_table: opp-table-dsi {
+		compatible = "operating-points-v2";
+
+		opp-131250000 {
+			opp-hz = /bits/ 64 <131250000>;
+			required-opps = <&rpmpd_opp_svs>;
+		};
+
+		opp-210000000 {
+			opp-hz = /bits/ 64 <210000000>;
+			required-opps = <&rpmpd_opp_svs_plus>;
+		};
+
+		opp-262500000 {
+			opp-hz = /bits/ 64 <262500000>;
+			required-opps = <&rpmpd_opp_nom>;
+		};
+	};
+
 	pmu {
 		compatible = "arm,armv8-pmuv3";
 		interrupts = <GIC_PPI 6 IRQ_TYPE_LEVEL_HIGH>;
@@ -1450,25 +1469,6 @@ mmcc: clock-controller@c8c0000 {
 					<0>;
 		};
 
-		dsi_opp_table: opp-table-dsi {
-			compatible = "operating-points-v2";
-
-			opp-131250000 {
-				opp-hz = /bits/ 64 <131250000>;
-				required-opps = <&rpmpd_opp_svs>;
-			};
-
-			opp-210000000 {
-				opp-hz = /bits/ 64 <210000000>;
-				required-opps = <&rpmpd_opp_svs_plus>;
-			};
-
-			opp-262500000 {
-				opp-hz = /bits/ 64 <262500000>;
-				required-opps = <&rpmpd_opp_nom>;
-			};
-		};
-
 		mdss: display-subsystem@c900000 {
 			compatible = "qcom,mdss";
 			reg = <0x0c900000 0x1000>,
-- 
2.34.1

Re: [PATCH v2] arm64: dts: qcom: sdm630: move DSI opp-table out of soc node

[Dmitry Baryshkov]

On Sun, 26 Mar 2023 at 12:16, Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> The soc node is supposed to have only device nodes with MMIO addresses,
> so move the DSI OPP out of it (it is used also by second DSI1 on
> SDM660):

This raises a question: would it make sense to add /opps to handle all
opp tables?

>
>   sda660-inforce-ifc6560.dtb: soc: opp-table-dsi: {'compatible': ['operating-points-v2'], ... should not be valid under {'type': 'object'}
>     From schema: dtschema/schemas/simple-bus.yaml
>
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>
> ---
>
> Changes since v1:
> 1. Move the node out of soc. Don't add Konrad's review tag.
> ---
>  arch/arm64/boot/dts/qcom/sdm630.dtsi | 38 ++++++++++++++--------------
>  1 file changed, 19 insertions(+), 19 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/qcom/sdm630.dtsi b/arch/arm64/boot/dts/qcom/sdm630.dtsi
> index 72d9a12b5e9c..b91e423a3cfc 100644
> --- a/arch/arm64/boot/dts/qcom/sdm630.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sdm630.dtsi
> @@ -328,6 +328,25 @@ memory@80000000 {
>                 reg = <0x0 0x80000000 0x0 0x0>;
>         };
>
> +       dsi_opp_table: opp-table-dsi {
> +               compatible = "operating-points-v2";
> +
> +               opp-131250000 {
> +                       opp-hz = /bits/ 64 <131250000>;
> +                       required-opps = <&rpmpd_opp_svs>;
> +               };
> +
> +               opp-210000000 {
> +                       opp-hz = /bits/ 64 <210000000>;
> +                       required-opps = <&rpmpd_opp_svs_plus>;
> +               };
> +
> +               opp-262500000 {
> +                       opp-hz = /bits/ 64 <262500000>;
> +                       required-opps = <&rpmpd_opp_nom>;
> +               };
> +       };
> +
>         pmu {
>                 compatible = "arm,armv8-pmuv3";
>                 interrupts = <GIC_PPI 6 IRQ_TYPE_LEVEL_HIGH>;
> @@ -1450,25 +1469,6 @@ mmcc: clock-controller@c8c0000 {
>                                         <0>;
>                 };
>
> -               dsi_opp_table: opp-table-dsi {
> -                       compatible = "operating-points-v2";
> -
> -                       opp-131250000 {
> -                               opp-hz = /bits/ 64 <131250000>;
> -                               required-opps = <&rpmpd_opp_svs>;
> -                       };
> -
> -                       opp-210000000 {
> -                               opp-hz = /bits/ 64 <210000000>;
> -                               required-opps = <&rpmpd_opp_svs_plus>;
> -                       };
> -
> -                       opp-262500000 {
> -                               opp-hz = /bits/ 64 <262500000>;
> -                               required-opps = <&rpmpd_opp_nom>;
> -                       };
> -               };
> -
>                 mdss: display-subsystem@c900000 {
>                         compatible = "qcom,mdss";
>                         reg = <0x0c900000 0x1000>,
> --
> 2.34.1
>


-- 
With best wishes
Dmitry

Re: [PATCH v2] arm64: dts: qcom: sdm630: move DSI opp-table out of soc node

[Krzysztof Kozlowski]

On 26/03/2023 11:21, Dmitry Baryshkov wrote:
> On Sun, 26 Mar 2023 at 12:16, Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
>>
>> The soc node is supposed to have only device nodes with MMIO addresses,
>> so move the DSI OPP out of it (it is used also by second DSI1 on
>> SDM660):
> 
> This raises a question: would it make sense to add /opps to handle all
> opp tables?

We didn't add it to any other cases like this (and we already fixed all
other boards), so why now? We can but it is a bit late for it.

Best regards,
Krzysztof

Re: [PATCH v2] arm64: dts: qcom: sdm630: move DSI opp-table out of soc node

[Dmitry Baryshkov]

On Sun, 26 Mar 2023 at 12:22, Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 26/03/2023 11:21, Dmitry Baryshkov wrote:
> > On Sun, 26 Mar 2023 at 12:16, Krzysztof Kozlowski
> > <krzysztof.kozlowski@linaro.org> wrote:
> >>
> >> The soc node is supposed to have only device nodes with MMIO addresses,
> >> so move the DSI OPP out of it (it is used also by second DSI1 on
> >> SDM660):
> >
> > This raises a question: would it make sense to add /opps to handle all
> > opp tables?
>
> We didn't add it to any other cases like this (and we already fixed all
> other boards), so why now? We can but it is a bit late for it.

Because nobody expressed this idea beforehand? I'm not insisting here,
you have a better understanding of DT. Just wondering if it makes
sense.

-- 
With best wishes
Dmitry

Re: [PATCH v2] arm64: dts: qcom: sdm630: move DSI opp-table out of soc node

[Krzysztof Kozlowski]

On 26/03/2023 12:03, Dmitry Baryshkov wrote:
> On Sun, 26 Mar 2023 at 12:22, Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
>>
>> On 26/03/2023 11:21, Dmitry Baryshkov wrote:
>>> On Sun, 26 Mar 2023 at 12:16, Krzysztof Kozlowski
>>> <krzysztof.kozlowski@linaro.org> wrote:
>>>>
>>>> The soc node is supposed to have only device nodes with MMIO addresses,
>>>> so move the DSI OPP out of it (it is used also by second DSI1 on
>>>> SDM660):
>>>
>>> This raises a question: would it make sense to add /opps to handle all
>>> opp tables?
>>
>> We didn't add it to any other cases like this (and we already fixed all
>> other boards), so why now? We can but it is a bit late for it.
> 
> Because nobody expressed this idea beforehand? I'm not insisting here,
> you have a better understanding of DT. Just wondering if it makes
> sense.

It will not change much of ordering - all nodes will be close to each
other anyway (opp-table-XYZ), thus is rather a matter of readability and
subjective preference. No other platforms have "opps" or "opp-tables".

Best regards,
Krzysztof

Re: [PATCH v2] arm64: dts: qcom: sdm630: move DSI opp-table out of soc node

[Dmitry Baryshkov]

On Sun, 26 Mar 2023 at 13:13, Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 26/03/2023 12:03, Dmitry Baryshkov wrote:
> > On Sun, 26 Mar 2023 at 12:22, Krzysztof Kozlowski
> > <krzysztof.kozlowski@linaro.org> wrote:
> >>
> >> On 26/03/2023 11:21, Dmitry Baryshkov wrote:
> >>> On Sun, 26 Mar 2023 at 12:16, Krzysztof Kozlowski
> >>> <krzysztof.kozlowski@linaro.org> wrote:
> >>>>
> >>>> The soc node is supposed to have only device nodes with MMIO addresses,
> >>>> so move the DSI OPP out of it (it is used also by second DSI1 on
> >>>> SDM660):
> >>>
> >>> This raises a question: would it make sense to add /opps to handle all
> >>> opp tables?
> >>
> >> We didn't add it to any other cases like this (and we already fixed all
> >> other boards), so why now? We can but it is a bit late for it.
> >
> > Because nobody expressed this idea beforehand? I'm not insisting here,
> > you have a better understanding of DT. Just wondering if it makes
> > sense.
>
> It will not change much of ordering - all nodes will be close to each
> other anyway (opp-table-XYZ), thus is rather a matter of readability and
> subjective preference. No other platforms have "opps" or "opp-tables".

Ack, thanks for the explanation.

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



-- 
With best wishes
Dmitry

Re: [PATCH v2] arm64: dts: qcom: sdm630: move DSI opp-table out of soc node

[Konrad Dybcio]

On 26.03.2023 11:16, Krzysztof Kozlowski wrote:
> The soc node is supposed to have only device nodes with MMIO addresses,
> so move the DSI OPP out of it (it is used also by second DSI1 on
> SDM660):
> 
>   sda660-inforce-ifc6560.dtb: soc: opp-table-dsi: {'compatible': ['operating-points-v2'], ... should not be valid under {'type': 'object'}
>     From schema: dtschema/schemas/simple-bus.yaml
> 
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> 
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>

Konrad
> 
> Changes since v1:
> 1. Move the node out of soc. Don't add Konrad's review tag.
> ---
>  arch/arm64/boot/dts/qcom/sdm630.dtsi | 38 ++++++++++++++--------------
>  1 file changed, 19 insertions(+), 19 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sdm630.dtsi b/arch/arm64/boot/dts/qcom/sdm630.dtsi
> index 72d9a12b5e9c..b91e423a3cfc 100644
> --- a/arch/arm64/boot/dts/qcom/sdm630.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sdm630.dtsi
> @@ -328,6 +328,25 @@ memory@80000000 {
>  		reg = <0x0 0x80000000 0x0 0x0>;
>  	};
>  
> +	dsi_opp_table: opp-table-dsi {
> +		compatible = "operating-points-v2";
> +
> +		opp-131250000 {
> +			opp-hz = /bits/ 64 <131250000>;
> +			required-opps = <&rpmpd_opp_svs>;
> +		};
> +
> +		opp-210000000 {
> +			opp-hz = /bits/ 64 <210000000>;
> +			required-opps = <&rpmpd_opp_svs_plus>;
> +		};
> +
> +		opp-262500000 {
> +			opp-hz = /bits/ 64 <262500000>;
> +			required-opps = <&rpmpd_opp_nom>;
> +		};
> +	};
> +
>  	pmu {
>  		compatible = "arm,armv8-pmuv3";
>  		interrupts = <GIC_PPI 6 IRQ_TYPE_LEVEL_HIGH>;
> @@ -1450,25 +1469,6 @@ mmcc: clock-controller@c8c0000 {
>  					<0>;
>  		};
>  
> -		dsi_opp_table: opp-table-dsi {
> -			compatible = "operating-points-v2";
> -
> -			opp-131250000 {
> -				opp-hz = /bits/ 64 <131250000>;
> -				required-opps = <&rpmpd_opp_svs>;
> -			};
> -
> -			opp-210000000 {
> -				opp-hz = /bits/ 64 <210000000>;
> -				required-opps = <&rpmpd_opp_svs_plus>;
> -			};
> -
> -			opp-262500000 {
> -				opp-hz = /bits/ 64 <262500000>;
> -				required-opps = <&rpmpd_opp_nom>;
> -			};
> -		};
> -
>  		mdss: display-subsystem@c900000 {
>  			compatible = "qcom,mdss";
>  			reg = <0x0c900000 0x1000>,

Re: [PATCH v2] arm64: dts: qcom: sdm630: move DSI opp-table out of soc node

[Krzysztof Kozlowski]

On 27/03/2023 21:39, Bjorn Andersson wrote:
> On Sun, Mar 26, 2023 at 11:16:05AM +0200, Krzysztof Kozlowski wrote:
>> The soc node is supposed to have only device nodes with MMIO addresses,
>> so move the DSI OPP out of it (it is used also by second DSI1 on
>> SDM660):
>>
> 
> This node has been moved into the dsi node, so if we still want this,
> could you please update the commit message.

The OPP table has been moved *out of* DSI node. The v1 was moving
inside, but this was not good approach, thus v2 moves it out.

I don't understand what shall be updated here.



Best regards,
Krzysztof

Re: [PATCH v2] arm64: dts: qcom: sdm630: move DSI opp-table out of soc node

[Bjorn Andersson]

On Sun, Mar 26, 2023 at 11:16:05AM +0200, Krzysztof Kozlowski wrote:
> The soc node is supposed to have only device nodes with MMIO addresses,
> so move the DSI OPP out of it (it is used also by second DSI1 on
> SDM660):
> 

This node has been moved into the dsi node, so if we still want this,
could you please update the commit message.

Regards,
Bjorn

>   sda660-inforce-ifc6560.dtb: soc: opp-table-dsi: {'compatible': ['operating-points-v2'], ... should not be valid under {'type': 'object'}
>     From schema: dtschema/schemas/simple-bus.yaml
> 
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> 
> ---
> 
> Changes since v1:
> 1. Move the node out of soc. Don't add Konrad's review tag.
> ---
>  arch/arm64/boot/dts/qcom/sdm630.dtsi | 38 ++++++++++++++--------------
>  1 file changed, 19 insertions(+), 19 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sdm630.dtsi b/arch/arm64/boot/dts/qcom/sdm630.dtsi
> index 72d9a12b5e9c..b91e423a3cfc 100644
> --- a/arch/arm64/boot/dts/qcom/sdm630.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sdm630.dtsi
> @@ -328,6 +328,25 @@ memory@80000000 {
>  		reg = <0x0 0x80000000 0x0 0x0>;
>  	};
>  
> +	dsi_opp_table: opp-table-dsi {
> +		compatible = "operating-points-v2";
> +
> +		opp-131250000 {
> +			opp-hz = /bits/ 64 <131250000>;
> +			required-opps = <&rpmpd_opp_svs>;
> +		};
> +
> +		opp-210000000 {
> +			opp-hz = /bits/ 64 <210000000>;
> +			required-opps = <&rpmpd_opp_svs_plus>;
> +		};
> +
> +		opp-262500000 {
> +			opp-hz = /bits/ 64 <262500000>;
> +			required-opps = <&rpmpd_opp_nom>;
> +		};
> +	};
> +
>  	pmu {
>  		compatible = "arm,armv8-pmuv3";
>  		interrupts = <GIC_PPI 6 IRQ_TYPE_LEVEL_HIGH>;
> @@ -1450,25 +1469,6 @@ mmcc: clock-controller@c8c0000 {
>  					<0>;
>  		};
>  
> -		dsi_opp_table: opp-table-dsi {
> -			compatible = "operating-points-v2";
> -
> -			opp-131250000 {
> -				opp-hz = /bits/ 64 <131250000>;
> -				required-opps = <&rpmpd_opp_svs>;
> -			};
> -
> -			opp-210000000 {
> -				opp-hz = /bits/ 64 <210000000>;
> -				required-opps = <&rpmpd_opp_svs_plus>;
> -			};
> -
> -			opp-262500000 {
> -				opp-hz = /bits/ 64 <262500000>;
> -				required-opps = <&rpmpd_opp_nom>;
> -			};
> -		};
> -
>  		mdss: display-subsystem@c900000 {
>  			compatible = "qcom,mdss";
>  			reg = <0x0c900000 0x1000>,
> -- 
> 2.34.1
> 

Re: [PATCH v2] arm64: dts: qcom: sdm630: move DSI opp-table out of soc node

[Bjorn Andersson]

On Mon, Mar 27, 2023 at 09:39:00PM +0200, Krzysztof Kozlowski wrote:
> On 27/03/2023 21:39, Bjorn Andersson wrote:
> > On Sun, Mar 26, 2023 at 11:16:05AM +0200, Krzysztof Kozlowski wrote:
> >> The soc node is supposed to have only device nodes with MMIO addresses,
> >> so move the DSI OPP out of it (it is used also by second DSI1 on
> >> SDM660):
> >>
> > 
> > This node has been moved into the dsi node, so if we still want this,
> > could you please update the commit message.
> 
> The OPP table has been moved *out of* DSI node. The v1 was moving
> inside, but this was not good approach, thus v2 moves it out.
> 
> I don't understand what shall be updated here.
> 

The commit message doesn't reflect what's in linux-next today and the
patch doesn't apply.

Regards,
Bjorn

Re: [PATCH v2] arm64: dts: qcom: sdm630: move DSI opp-table out of soc node

[Krzysztof Kozlowski]

On 07/04/2023 18:34, Bjorn Andersson wrote:
> On Mon, Mar 27, 2023 at 09:39:00PM +0200, Krzysztof Kozlowski wrote:
>> On 27/03/2023 21:39, Bjorn Andersson wrote:
>>> On Sun, Mar 26, 2023 at 11:16:05AM +0200, Krzysztof Kozlowski wrote:
>>>> The soc node is supposed to have only device nodes with MMIO addresses,
>>>> so move the DSI OPP out of it (it is used also by second DSI1 on
>>>> SDM660):
>>>>
>>>
>>> This node has been moved into the dsi node, so if we still want this,
>>> could you please update the commit message.
>>
>> The OPP table has been moved *out of* DSI node. The v1 was moving
>> inside, but this was not good approach, thus v2 moves it out.
>>
>> I don't understand what shall be updated here.
>>
> 
> The commit message doesn't reflect what's in linux-next today and the
> patch doesn't apply.
> 

It seems you applied v1. It's okay, yet would be nice to clean up so I
will send a follow-up patch.

Best regards,
Krzysztof