| Message ID | 20221210115704.97614-1-krzysztof.kozlowski@linaro.org |
|---|---|
| State | Accepted |
| Commit | 524dfd2ddbd74ed5b4cbb3e002984cf95878c827 |
| Headers | show |
| Series | [1/4] arm64: dts: qcom: sc7180: move QUP and QSPI opp tables out of SoC node | expand |
On 10/12/2022 13:31, Konrad Dybcio wrote: > > > On 10.12.2022 12:57, Krzysztof Kozlowski wrote: >> The sound and codec nodes are not a property of a soc, but rather board >> as it describes the sound configuration. > * in this case, there exist SoC-internal codecs wcd9380 is not SoC internal, so to which codec you refer to? Sound node is for sound configuration, not codec, and sound configuration is board specific. > > It also does not have unit >> address: >> >> sm8250-hdk.dtb: soc@0: sound: {} should not be valid under {'type': 'object'} >> >> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> >> --- > Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org> Best regards, Krzysztof
On 11/12/2022 22:15, Dmitry Baryshkov wrote: > On Sun, 11 Dec 2022 at 22:13, Krzysztof Kozlowski > <krzysztof.kozlowski@linaro.org> wrote: >> >> On 10/12/2022 13:31, Konrad Dybcio wrote: >>> >>> >>> On 10.12.2022 12:57, Krzysztof Kozlowski wrote: >>>> The sound and codec nodes are not a property of a soc, but rather board >>>> as it describes the sound configuration. >>> * in this case, there exist SoC-internal codecs >> >> wcd9380 is not SoC internal, so to which codec you refer to? Sound node >> is for sound configuration, not codec, and sound configuration is board >> specific. > > The platform has several macro 'codec's, which are SoC-internal > devices. On the other hand, these devices also have bus addresses. Ah, so Konrad refers to "codec nodes" being a bit generic because we have them also as part of SoC? These TX/VA macro are named codecs but these are not really audio codecs - they receive already digital signal, AFAIK. They are more like audio mixers and controllers. The codec in traditional meaning is only the wcd9380 on the board. I'll rephrase the commit msg to be clearer here. Best regards, Krzysztof
On 11.12.2022 21:14, Krzysztof Kozlowski wrote: > On 10/12/2022 13:29, Konrad Dybcio wrote: >> >> >> On 10.12.2022 12:57, Krzysztof Kozlowski wrote: >>> The SoC node is a simple-bus and its schema expect to have nodes only >>> with unit addresses: >>> >>> sc7180-trogdor-lazor-r3.dtb: soc@0: opp-table-qspi: {'compatible': ['operating-points-v2'], 'phandle': [[186]], 'opp-75000000': >>> ... 'required-opps': [[47]]}} should not be valid under {'type': 'object'} >>> >>> Move to top-level OPP tables: >>> - QUP which is 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> >>> --- >>> arch/arm64/boot/dts/qcom/sc7180.dtsi | 76 ++++++++++++++-------------- >>> 1 file changed, 38 insertions(+), 38 deletions(-) >>> >>> diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi b/arch/arm64/boot/dts/qcom/sc7180.dtsi >>> index ea886cf08b4d..735581097295 100644 >>> --- a/arch/arm64/boot/dts/qcom/sc7180.dtsi >>> +++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi >>> @@ -538,6 +538,44 @@ cpu6_opp16: opp-2553600000 { >>> }; >>> }; >>> >>> + qspi_opp_table: opp-table-qspi { >>> + compatible = "operating-points-v2"; >>> + >>> + opp-75000000 { >>> + opp-hz = /bits/ 64 <75000000>; >>> + 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-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>; >>> + }; >>> + }; >>> + >>> memory@80000000 { >> Sidenote: memory@ should be moved above opp-*, alphabetically >> >> For this: >> >> Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org> > > You sure? Because before there is already opp for cpu... Which are called opp-table-cpuN and not cpuN-opp-table, 'm' comes before 'o'. Konrad > > Best regards, > Krzysztof >
On 12/12/2022 10:15, Konrad Dybcio wrote: >>>> + >>>> + qup_opp_table: opp-table-qup { >>>> + compatible = "operating-points-v2"; >>>> + >>>> + 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>; >>>> + }; >>>> + }; >>>> + >>>> memory@80000000 { >>> Sidenote: memory@ should be moved above opp-*, alphabetically >>> >>> For this: >>> >>> Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org> >> >> You sure? Because before there is already opp for cpu... > Which are called opp-table-cpuN and not cpuN-opp-table, 'm' comes > before 'o'. So you want to have broken order? opp-table-cpu memory opp-table-qup ? They are at least all together in my patch. Best regards, Krzysztof
On 12.12.2022 10:17, Krzysztof Kozlowski wrote: > On 12/12/2022 10:15, Konrad Dybcio wrote: >>>>> + >>>>> + qup_opp_table: opp-table-qup { >>>>> + compatible = "operating-points-v2"; >>>>> + >>>>> + 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>; >>>>> + }; >>>>> + }; >>>>> + >>>>> memory@80000000 { >>>> Sidenote: memory@ should be moved above opp-*, alphabetically >>>> >>>> For this: >>>> >>>> Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org> >>> >>> You sure? Because before there is already opp for cpu... >> Which are called opp-table-cpuN and not cpuN-opp-table, 'm' comes >> before 'o'. > > So you want to have broken order? > opp-table-cpu > memory > opp-table-qup > ? > > They are at least all together in my patch. No, I meant: memory opp-table-cpu opp-table-qup Konrad > > Best regards, > Krzysztof >
On 12/12/2022 10:18, Konrad Dybcio wrote: > > > On 12.12.2022 10:17, Krzysztof Kozlowski wrote: >> On 12/12/2022 10:15, Konrad Dybcio wrote: >>>>>> + >>>>>> + qup_opp_table: opp-table-qup { >>>>>> + compatible = "operating-points-v2"; >>>>>> + >>>>>> + 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>; >>>>>> + }; >>>>>> + }; >>>>>> + >>>>>> memory@80000000 { >>>>> Sidenote: memory@ should be moved above opp-*, alphabetically >>>>> >>>>> For this: >>>>> >>>>> Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org> >>>> >>>> You sure? Because before there is already opp for cpu... >>> Which are called opp-table-cpuN and not cpuN-opp-table, 'm' comes >>> before 'o'. >> >> So you want to have broken order? >> opp-table-cpu >> memory >> opp-table-qup >> ? >> >> They are at least all together in my patch. > No, I meant: > > memory > opp-table-cpu > opp-table-qup OK, I'll correct the opp-table-cpu/memory order in separate patch. Best regards, Krzysztof
diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi b/arch/arm64/boot/dts/qcom/sc7180.dtsi index ea886cf08b4d..735581097295 100644 --- a/arch/arm64/boot/dts/qcom/sc7180.dtsi +++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi @@ -538,6 +538,44 @@ cpu6_opp16: opp-2553600000 { }; }; + qspi_opp_table: opp-table-qspi { + compatible = "operating-points-v2"; + + opp-75000000 { + opp-hz = /bits/ 64 <75000000>; + 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-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>; + }; + }; + memory@80000000 { device_type = "memory"; /* We expect the bootloader to fill in the size */ @@ -739,25 +777,6 @@ opp-384000000 { }; }; - qup_opp_table: opp-table-qup { - compatible = "operating-points-v2"; - - 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>; - }; - }; - qupv3_id_0: geniqup@8c0000 { compatible = "qcom,geni-se-qup"; reg = <0 0x008c0000 0 0x6000>; @@ -2655,25 +2674,6 @@ opp-202000000 { }; }; - qspi_opp_table: opp-table-qspi { - compatible = "operating-points-v2"; - - opp-75000000 { - opp-hz = /bits/ 64 <75000000>; - 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@88dc000 { compatible = "qcom,sc7180-qspi", "qcom,qspi-v1"; reg = <0 0x088dc000 0 0x600>;
The SoC node is a simple-bus and its schema expect to have nodes only with unit addresses: sc7180-trogdor-lazor-r3.dtb: soc@0: opp-table-qspi: {'compatible': ['operating-points-v2'], 'phandle': [[186]], 'opp-75000000': ... 'required-opps': [[47]]}} should not be valid under {'type': 'object'} Move to top-level OPP tables: - QUP which is 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> --- arch/arm64/boot/dts/qcom/sc7180.dtsi | 76 ++++++++++++++-------------- 1 file changed, 38 insertions(+), 38 deletions(-)