Messages in this thread | | | Subject | Re: [PATCH v4 1/9] ASoC: dt-bindings: wcd938x: add bindings for wcd938x | From | Srinivas Kandagatla <> | Date | Fri, 30 Apr 2021 09:20:17 +0100 |
| |
Hi Rob,
On 15/04/2021 17:53, Srinivas Kandagatla wrote: > Thanks Rob for quick review, > > On 15/04/2021 17:29, Rob Herring wrote: >> On Wed, Apr 14, 2021 at 04:48:37PM +0100, Srinivas Kandagatla wrote: >>> Qualcomm WCD9380/WCD9385 Codec is a standalone Hi-Fi audio codec IC >>> connected over SoundWire. This device has two SoundWire device RX and >>> TX respectively, supporting 4 x ADCs, ClassH, Ear, Aux PA, 2xHPH, >>> 7 x TX diff inputs, 8 DMICs, MBHC. >>> >>> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> >>> --- >>> .../bindings/sound/qcom,wcd938x.yaml | 176 ++++++++++++++++++ >>> 1 file changed, 176 insertions(+) >>> create mode 100644 >>> Documentation/devicetree/bindings/sound/qcom,wcd938x.yaml >>> >>> diff --git >>> a/Documentation/devicetree/bindings/sound/qcom,wcd938x.yaml >>> b/Documentation/devicetree/bindings/sound/qcom,wcd938x.yaml >>> new file mode 100644 >>> index 000000000000..4c8fa8290af0 >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/sound/qcom,wcd938x.yaml >>> @@ -0,0 +1,176 @@ > > ... > >>> +additionalProperties: false >>> + >>> +examples: >>> + - | >>> + codec { >>> + compatible = "qcom,wcd9380-codec"; >>> + reset-gpios = <&tlmm 32 0>; >>> + #sound-dai-cells = <1>; >>> + qcom,tx-device = <&wcd938x_tx>; >>> + qcom,rx-device = <&wcd938x_rx>; >>> + qcom,micbias1-microvolt = <1800000>; >>> + qcom,micbias2-microvolt = <1800000>; >>> + qcom,micbias3-microvolt = <1800000>; >>> + qcom,micbias4-microvolt = <1800000>; >>> + qcom,mbhc-hphl-switch; >>> + qcom,mbhc-ground-switch; >>> + qcom,mbhc-button0-vthreshold-microvolt = <75000>; >>> + qcom,mbhc-button1-vthreshold-microvolt = <150000>; >>> + qcom,mbhc-button2-vthreshold-microvolt = <237000>; >>> + qcom,mbhc-button3-vthreshold-microvolt = <500000>; >>> + qcom,mbhc-button5-vthreshold-microvolt = <500000>; >>> + qcom,mbhc-button6-vthreshold-microvolt = <500000>; >>> + qcom,mbhc-button7-vthreshold-microvolt = <500000>; >>> + }; >>> + >>> + /* ... */ >>> + >>> + soundwire@3230000 { >>> + #address-cells = <2>; >>> + #size-cells = <0>; >>> + reg = <0x03230000 0x2000>; >>> + wcd938x_tx: codec@0,3 { >>> + compatible = "sdw20217010d00"; >>> + reg = <0 3>; >>> + qcom,direction = "tx"; >>> + qcom,port-mapping = <2 3 4 5>; >>> + }; >>> + >>> + wcd938x_rx: codec@0,4 { >>> + compatible = "sdw20217010d00"; >>> + reg = <0 4>; >>> + qcom,direction = "rx"; >>> + qcom,port-mapping = <1 2 3 4 5>; >>> + }; >> >> This is a single device, right? We shouldn't need 3 nodes to describe >> it. I think this should all be a single node like this: >> > No, WCD938x is a Audio Codec which has two SoundWire Slave device (TX > and RX). WCD938X reset lines and supplies are common for both TX and RX > SoundWire devices. > > However TX SoundWire device only has register access to codec > CSR(Control Status registers). > > So there are two SoundWire devices and a WCD938X common parts. Now > making the common Codec part as a separate device made more sense here. > So we ended with total 3 devices. > > 1 . WCD938x Codec which deals with all the codec side including Common > parts. > 2. TX SoundWire device to configure TX SoundWire ports/interface and > provide CSR access. > 3. RX SoundWire device to configure RX Soundwire ports/interface >
Are you okay with the existing device layout after providing the above information?
codec { compatible = "qcom,wcd9380-codec"; reset-gpios = <&tlmm 32 0>; #sound-dai-cells = <1>; qcom,tx-device = <&wcd938x_tx>; qcom,rx-device = <&wcd938x_rx>; qcom,micbias1-microvolt = <1800000>; qcom,micbias2-microvolt = <1800000>; qcom,micbias3-microvolt = <1800000>; qcom,micbias4-microvolt = <1800000>; qcom,mbhc-hphl-switch; qcom,mbhc-ground-switch; qcom,mbhc-button0-vthreshold-microvolt = <75000>; qcom,mbhc-button1-vthreshold-microvolt = <150000>; qcom,mbhc-button2-vthreshold-microvolt = <237000>; qcom,mbhc-button3-vthreshold-microvolt = <500000>; qcom,mbhc-button5-vthreshold-microvolt = <500000>; qcom,mbhc-button6-vthreshold-microvolt = <500000>; qcom,mbhc-button7-vthreshold-microvolt = <500000>; };
soundwire-controller@3230000 { reg = <0 0x3230000 0 0x2000>; compatible = "qcom,soundwire-v1.5.1"; wcd938x_tx: codec@0,3 { compatible = "sdw20217010d00"; reg = <0 3>; qcom,direction = "tx"; qcom,port-mapping = <2 3 4 5>; }; };
soundwire-controller@3210000 { reg = <0 0x3210000 0 0x2000>; compatible = "qcom,soundwire-v1.5.1"; wcd938x_rx: codec@0,4 { compatible = "sdw20217010d00"; reg = <0 4>; qcom,direction = "rx"; qcom,port-mapping = <1 2 3 4 5>; }; };
thanks, srini
> >> codec@0,3 { >> reg = <0 3>, <0 4>; > > We can't have this, as these two SoundWire devices hang on different > SoundWire bus instances. > >> compatible = "sdw20217010d00"; >> >> reset-gpios = <&tlmm 32 0>; >> #sound-dai-cells = <1>; >> qcom,micbias1-microvolt = <1800000>; >> qcom,micbias2-microvolt = <1800000>; >> qcom,micbias3-microvolt = <1800000>; >> qcom,micbias4-microvolt = <1800000>; >> qcom,mbhc-hphl-switch; >> qcom,mbhc-ground-switch; >> qcom,mbhc-button0-vthreshold-microvolt = <75000>; >> qcom,mbhc-button1-vthreshold-microvolt = <150000>; >> qcom,mbhc-button2-vthreshold-microvolt = <237000>; >> qcom,mbhc-button3-vthreshold-microvolt = <500000>; >> qcom,mbhc-button5-vthreshold-microvolt = <500000>; >> qcom,mbhc-button6-vthreshold-microvolt = <500000>; >> qcom,mbhc-button7-vthreshold-microvolt = <500000>; >> }; >> >> You'll have to figure out the qcom,direction and qcom,port-mapping parts >> though. > > That is the reason why we ended up with 3 devices here. > > --srini >> >> Rob >>
| |