2023-03-26 09:23:13

by Krzysztof Kozlowski

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

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 <[email protected]>

---

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


2023-03-26 09:23:18

by Dmitry Baryshkov

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

On Sun, 26 Mar 2023 at 12:16, Krzysztof Kozlowski
<[email protected]> 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 <[email protected]>
>
> ---
>
> 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

2023-03-26 09:24:01

by Krzysztof Kozlowski

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

On 26/03/2023 11:21, Dmitry Baryshkov wrote:
> On Sun, 26 Mar 2023 at 12:16, Krzysztof Kozlowski
> <[email protected]> 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

2023-03-26 10:26:12

by Dmitry Baryshkov

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

On Sun, 26 Mar 2023 at 12:22, Krzysztof Kozlowski
<[email protected]> wrote:
>
> On 26/03/2023 11:21, Dmitry Baryshkov wrote:
> > On Sun, 26 Mar 2023 at 12:16, Krzysztof Kozlowski
> > <[email protected]> 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

2023-03-26 11:10:52

by Krzysztof Kozlowski

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

On 26/03/2023 12:03, Dmitry Baryshkov wrote:
> On Sun, 26 Mar 2023 at 12:22, Krzysztof Kozlowski
> <[email protected]> wrote:
>>
>> On 26/03/2023 11:21, Dmitry Baryshkov wrote:
>>> On Sun, 26 Mar 2023 at 12:16, Krzysztof Kozlowski
>>> <[email protected]> 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

2023-03-26 22:06:29

by Dmitry Baryshkov

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

On Sun, 26 Mar 2023 at 13:13, Krzysztof Kozlowski
<[email protected]> wrote:
>
> On 26/03/2023 12:03, Dmitry Baryshkov wrote:
> > On Sun, 26 Mar 2023 at 12:22, Krzysztof Kozlowski
> > <[email protected]> wrote:
> >>
> >> On 26/03/2023 11:21, Dmitry Baryshkov wrote:
> >>> On Sun, 26 Mar 2023 at 12:16, Krzysztof Kozlowski
> >>> <[email protected]> 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 <[email protected]>



--
With best wishes
Dmitry

2023-03-27 17:50:13

by Konrad Dybcio

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



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 <[email protected]>
>
> ---
Reviewed-by: Konrad Dybcio <[email protected]>

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>,

2023-03-27 19:37:23

by Bjorn Andersson

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

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 <[email protected]>
>
> ---
>
> 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
>

2023-03-27 19:48:39

by Krzysztof Kozlowski

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

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

2023-04-07 16:34:31

by Bjorn Andersson

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

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

2023-04-07 19:20:03

by Krzysztof Kozlowski

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

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