| Message ID | 20221210115704.97614-4-krzysztof.kozlowski@linaro.org (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | e5b8c08245307a82cdf180cd5d385a34ba1cfd9d |
| Headers | show |
| Series | [1/4] arm64: dts: qcom: sc7180: move QUP and QSPI opp tables out of SoC node | expand |
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 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> Konrad > arch/arm64/boot/dts/qcom/sm8250-mtp.dts | 40 ++++++++++++------------- > arch/arm64/boot/dts/qcom/sm8250.dtsi | 6 ++-- > 2 files changed, 22 insertions(+), 24 deletions(-) > > diff --git a/arch/arm64/boot/dts/qcom/sm8250-mtp.dts b/arch/arm64/boot/dts/qcom/sm8250-mtp.dts > index 3ed8c84e25b8..b741b7da1afc 100644 > --- a/arch/arm64/boot/dts/qcom/sm8250-mtp.dts > +++ b/arch/arm64/boot/dts/qcom/sm8250-mtp.dts > @@ -27,6 +27,25 @@ chosen { > stdout-path = "serial0:115200n8"; > }; > > + wcd938x: codec { > + compatible = "qcom,wcd9380-codec"; > + #sound-dai-cells = <1>; > + reset-gpios = <&tlmm 32 GPIO_ACTIVE_LOW>; > + vdd-buck-supply = <&vreg_s4a_1p8>; > + vdd-rxtx-supply = <&vreg_s4a_1p8>; > + vdd-io-supply = <&vreg_s4a_1p8>; > + vdd-mic-bias-supply = <&vreg_bob>; > + qcom,micbias1-microvolt = <1800000>; > + qcom,micbias2-microvolt = <1800000>; > + qcom,micbias3-microvolt = <1800000>; > + qcom,micbias4-microvolt = <1800000>; > + qcom,mbhc-buttons-vthreshold-microvolt = <75000 150000 237000 500000 500000 500000 500000 500000>; > + qcom,mbhc-headset-vthreshold-microvolt = <1700000>; > + qcom,mbhc-headphone-vthreshold-microvolt = <50000>; > + qcom,rx-device = <&wcd_rx>; > + qcom,tx-device = <&wcd_tx>; > + }; > + > thermal-zones { > camera-thermal { > polling-delay-passive = <0>; > @@ -631,27 +650,6 @@ &slpi { > firmware-name = "qcom/sm8250/slpi.mbn"; > }; > > -&soc { > - wcd938x: codec { > - compatible = "qcom,wcd9380-codec"; > - #sound-dai-cells = <1>; > - reset-gpios = <&tlmm 32 GPIO_ACTIVE_LOW>; > - vdd-buck-supply = <&vreg_s4a_1p8>; > - vdd-rxtx-supply = <&vreg_s4a_1p8>; > - vdd-io-supply = <&vreg_s4a_1p8>; > - vdd-mic-bias-supply = <&vreg_bob>; > - qcom,micbias1-microvolt = <1800000>; > - qcom,micbias2-microvolt = <1800000>; > - qcom,micbias3-microvolt = <1800000>; > - qcom,micbias4-microvolt = <1800000>; > - qcom,mbhc-buttons-vthreshold-microvolt = <75000 150000 237000 500000 500000 500000 500000 500000>; > - qcom,mbhc-headset-vthreshold-microvolt = <1700000>; > - qcom,mbhc-headphone-vthreshold-microvolt = <50000>; > - qcom,rx-device = <&wcd_rx>; > - qcom,tx-device = <&wcd_tx>; > - }; > -}; > - > &sound { > compatible = "qcom,sm8250-sndcard"; > model = "SM8250-MTP-WCD9380-WSA8810-VA-DMIC"; > diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi b/arch/arm64/boot/dts/qcom/sm8250.dtsi > index d517d6a80bdc..fbbbae29e0c2 100644 > --- a/arch/arm64/boot/dts/qcom/sm8250.dtsi > +++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi > @@ -2826,9 +2826,6 @@ compute-cb@8 { > }; > }; > > - sound: sound { > - }; > - > usb_1_hsphy: phy@88e3000 { > compatible = "qcom,sm8250-usb-hs-phy", > "qcom,usb-snps-hs-7nm-phy"; > @@ -4910,6 +4907,9 @@ cpufreq_hw: cpufreq@18591000 { > }; > }; > > + sound: sound { > + }; > + > timer { > compatible = "arm,armv8-timer"; > interrupts = <GIC_PPI 13
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 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. > > > > > 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>
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:13, Krzysztof Kozlowski 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. Your patch is correct, this was a nit pertaining to the commit message, as it could suggest that all codecs should be moved out of /soc, which would not be the case for MMIO-mapped ones. Konrad > >> >> 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 >
diff --git a/arch/arm64/boot/dts/qcom/sm8250-mtp.dts b/arch/arm64/boot/dts/qcom/sm8250-mtp.dts index 3ed8c84e25b8..b741b7da1afc 100644 --- a/arch/arm64/boot/dts/qcom/sm8250-mtp.dts +++ b/arch/arm64/boot/dts/qcom/sm8250-mtp.dts @@ -27,6 +27,25 @@ chosen { stdout-path = "serial0:115200n8"; }; + wcd938x: codec { + compatible = "qcom,wcd9380-codec"; + #sound-dai-cells = <1>; + reset-gpios = <&tlmm 32 GPIO_ACTIVE_LOW>; + vdd-buck-supply = <&vreg_s4a_1p8>; + vdd-rxtx-supply = <&vreg_s4a_1p8>; + vdd-io-supply = <&vreg_s4a_1p8>; + vdd-mic-bias-supply = <&vreg_bob>; + qcom,micbias1-microvolt = <1800000>; + qcom,micbias2-microvolt = <1800000>; + qcom,micbias3-microvolt = <1800000>; + qcom,micbias4-microvolt = <1800000>; + qcom,mbhc-buttons-vthreshold-microvolt = <75000 150000 237000 500000 500000 500000 500000 500000>; + qcom,mbhc-headset-vthreshold-microvolt = <1700000>; + qcom,mbhc-headphone-vthreshold-microvolt = <50000>; + qcom,rx-device = <&wcd_rx>; + qcom,tx-device = <&wcd_tx>; + }; + thermal-zones { camera-thermal { polling-delay-passive = <0>; @@ -631,27 +650,6 @@ &slpi { firmware-name = "qcom/sm8250/slpi.mbn"; }; -&soc { - wcd938x: codec { - compatible = "qcom,wcd9380-codec"; - #sound-dai-cells = <1>; - reset-gpios = <&tlmm 32 GPIO_ACTIVE_LOW>; - vdd-buck-supply = <&vreg_s4a_1p8>; - vdd-rxtx-supply = <&vreg_s4a_1p8>; - vdd-io-supply = <&vreg_s4a_1p8>; - vdd-mic-bias-supply = <&vreg_bob>; - qcom,micbias1-microvolt = <1800000>; - qcom,micbias2-microvolt = <1800000>; - qcom,micbias3-microvolt = <1800000>; - qcom,micbias4-microvolt = <1800000>; - qcom,mbhc-buttons-vthreshold-microvolt = <75000 150000 237000 500000 500000 500000 500000 500000>; - qcom,mbhc-headset-vthreshold-microvolt = <1700000>; - qcom,mbhc-headphone-vthreshold-microvolt = <50000>; - qcom,rx-device = <&wcd_rx>; - qcom,tx-device = <&wcd_tx>; - }; -}; - &sound { compatible = "qcom,sm8250-sndcard"; model = "SM8250-MTP-WCD9380-WSA8810-VA-DMIC"; diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi b/arch/arm64/boot/dts/qcom/sm8250.dtsi index d517d6a80bdc..fbbbae29e0c2 100644 --- a/arch/arm64/boot/dts/qcom/sm8250.dtsi +++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi @@ -2826,9 +2826,6 @@ compute-cb@8 { }; }; - sound: sound { - }; - usb_1_hsphy: phy@88e3000 { compatible = "qcom,sm8250-usb-hs-phy", "qcom,usb-snps-hs-7nm-phy"; @@ -4910,6 +4907,9 @@ cpufreq_hw: cpufreq@18591000 { }; }; + sound: sound { + }; + timer { compatible = "arm,armv8-timer"; interrupts = <GIC_PPI 13
The sound and codec nodes are not a property of a soc, but rather board as it describes the sound configuration. 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> --- arch/arm64/boot/dts/qcom/sm8250-mtp.dts | 40 ++++++++++++------------- arch/arm64/boot/dts/qcom/sm8250.dtsi | 6 ++-- 2 files changed, 22 insertions(+), 24 deletions(-)