| Message ID | 1650291252-30398-3-git-send-email-quic_srivasam@quicinc.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v8,1/4] arm64: dts: qcom: sc7280: Add nodes for soundwire and va tx rx digital macro codecs | expand |
On 4/19/2022 4:54 PM, Srinivasa Rao Mandadapu wrote: > > On 4/18/2022 10:15 PM, Matthias Kaehlcke wrote: > Thanks for your time and valuable inputs Matthias!!! >> On Mon, Apr 18, 2022 at 07:44:10PM +0530, Srinivasa Rao Mandadapu wrote: >>> Add wcd938x and max98360a codecs for audio use case on >>> sc7280 based platforms. >>> Add tlmm gpio property in wcd938x node for switching CTIA/OMTP Headset. >>> Add amp_en node for max98360a codec. >> General note: I don't think it's a good practice to add stuff like >> this to >> multiple boards in a single patch. Why? >> >> First the subject of such a patch tends to be vague ("arm64: dts: qcom: >> sc7280: Add nodes for wcd9385 and max98360a codec"), in this case it >> gives >> no hint about the boards. If someone was interested in picking changes >> for a given board they can't easily identify from the subject that the >> change is relevant for them. >> >> Changes touching multiple boards are more likely to cause conflicts when >> being picked (or reverted), both upstream and in downstream trees (which >> unfortunately have to exist for product development). Downstream trees >> might only pick changes for the board(s) they target, patches that touch >> mutiple boards often cause conflicts due to context deltas in the >> 'irrelevant' boards. >> >> Lastly it's usually easier to get a patch reviewed (in the sense of >> getting a 'Reviewed-by' tag) and landed that does a single thing. > > Yes, agree to your opinion. In a nutshell, we will include board > name(ex: herobrine) > > in commit message and split the patches per external codec. > > Actually, in Initial herobrine boards, EVT and IDP, has both maxim > speaker and WCD codec, > > hence we included in same patch. > >>> Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com> >>> Co-developed-by: Venkata Prasad Potturu <quic_potturu@quicinc.com> >>> Signed-off-by: Venkata Prasad Potturu <quic_potturu@quicinc.com> >>> --- >>> arch/arm64/boot/dts/qcom/sc7280-crd-r3.dts | 6 ++ >>> arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi | 8 +++ >>> arch/arm64/boot/dts/qcom/sc7280-idp.dtsi | 97 >>> ++++++++++++++++++++++++++ >>> 3 files changed, 111 insertions(+) >>> >>> diff --git a/arch/arm64/boot/dts/qcom/sc7280-crd-r3.dts >>> b/arch/arm64/boot/dts/qcom/sc7280-crd-r3.dts >>> index 344338a..aa0bf6e2 100644 >>> --- a/arch/arm64/boot/dts/qcom/sc7280-crd-r3.dts >>> +++ b/arch/arm64/boot/dts/qcom/sc7280-crd-r3.dts >>> @@ -87,6 +87,12 @@ ap_ts_pen_1v8: &i2c13 { >>> pins = "gpio51"; >>> }; >>> +&wcd938x { >>> + pinctrl-names = "default"; >>> + pinctrl-0 = <&us_euro_hs_sel>; >>> + us-euro-gpios = <&tlmm 81 GPIO_ACTIVE_HIGH>; >>> +}; >> Since this is added for the CRD rev3 it probably should also be added to >> sc7280-herobrine-crd.dts > Okay. Will add in corresponding latest herobrine CRD dts file also. >> >>> + >>> &tlmm { >>> tp_int_odl: tp-int-odl { >>> pins = "gpio7"; >>> diff --git a/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi >>> b/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi >>> index d58045d..f247403 100644 >>> --- a/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi >>> +++ b/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi >>> @@ -20,6 +20,14 @@ >>> #include "sc7280-chrome-common.dtsi" >>> / { >>> + max98360a: audio-codec-0 { >>> + compatible = "maxim,max98360a"; >>> + pinctrl-names = "default"; >>> + pinctrl-0 = <&_en>; >>> + sdmode-gpios = <&tlmm 63 GPIO_ACTIVE_HIGH>; >>> + #sound-dai-cells = <0>; >>> + }; >>> + >>> chosen { >>> stdout-path = "serial0:115200n8"; >>> }; >>> diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi >>> b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi >>> index 2f863c0..8dad599 100644 >>> --- a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi >>> +++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi >>> @@ -20,6 +20,42 @@ >>> serial1 = &uart7; >>> }; >>> + max98360a: audio-codec-0 { >>> + compatible = "maxim,max98360a"; >>> + pinctrl-names = "default"; >>> + pinctrl-0 = <&_en>; >>> + sdmode-gpios = <&tlmm 63 GPIO_ACTIVE_HIGH>; >>> + #sound-dai-cells = <0>; >>> + }; >>> + >>> + wcd938x: audio-codec-1 { >> Why 'wcd938x' and not 'wcd9385'? > > Actually same driver is used for both wcd9380 and wcd9385. Here we can > use specific name as per board. > > Will change accordingly. At present, dt-bindgs also has wcd938x. So will update the name in bindings and here post this series. is it okay? > >> >>> + compatible = "qcom,wcd9385-codec"; >>> + pinctrl-names = "default", "sleep"; >>> + pinctrl-0 = <&wcd_reset_n>; >>> + pinctrl-1 = <&wcd_reset_n_sleep>; >>> + >>> + reset-gpios = <&tlmm 83 GPIO_ACTIVE_HIGH>; >>> + >>> + qcom,rx-device = <&wcd_rx>; >>> + qcom,tx-device = <&wcd_tx>; >>> + >>> + vdd-rxtx-supply = <&vreg_l18b_1p8>; >>> + vdd-io-supply = <&vreg_l18b_1p8>; >>> + vdd-buck-supply = <&vreg_l17b_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>; >>> + #sound-dai-cells = <1>; >>> + }; >> Also add this node to sc7280-herobrine-crd.dts? > Okay. will do accordingly. >> >>> + >>> gpio-keys { >>> compatible = "gpio-keys"; >>> label = "gpio-keys"; >>> @@ -238,6 +274,19 @@ >>> modem-init; >>> }; >>> +&lpass_rx_macro { >>> + status = "okay"; >>> +}; >>> + >>> +&lpass_tx_macro { >>> + status = "okay"; >>> +}; >>> + >>> +&lpass_va_macro { >>> + status = "okay"; >>> + vdd-micb-supply = <&vreg_bob>; >>> +}; >> Enable these also in sc7280-herobrine.dtsi if other nodes are added to >> sc7280-herobrine-crd.dts. > Okay. >> >>> + >>> &pcie1 { >>> status = "okay"; >>> perst-gpio = <&tlmm 2 GPIO_ACTIVE_LOW>; >>> @@ -298,6 +347,26 @@ >>> cd-gpios = <&tlmm 91 GPIO_ACTIVE_LOW>; >>> }; >>> +&swr0 { >>> + status = "okay"; >> nit: add an empty line between properties and nodes > Okay. >> >>> + wcd_rx: codec@0,4 { >>> + compatible = "sdw20217010d00"; >>> + reg = <0 4>; >>> + #sound-dai-cells = <1>; >>> + qcom,rx-port-mapping = <1 2 3 4 5>; >>> + }; >>> +}; >>> + >>> +&swr1 { >>> + status = "okay"; >> ditto > Okay. >> >>> + wcd_tx: codec@0,3 { >>> + compatible = "sdw20217010d00"; >>> + reg = <0 3>; >>> + #sound-dai-cells = <1>; >>> + qcom,tx-port-mapping = <1 2 3 4>; >>> + }; >>> +}; >> Also add these to sc7280-herobrine-crd.dts? > Okay. >> >>> + >>> &uart5 { >>> compatible = "qcom,geni-debug-uart"; >>> status = "okay"; >>> @@ -561,6 +630,12 @@ >>> }; >>> &tlmm { >>> + amp_en: amp-en { >>> + pins = "gpio63"; >>> + bias-pull-down; >>> + drive-strength = <2>; >>> + }; >>> + >>> bt_en: bt-en { >>> pins = "gpio85"; >>> function = "gpio"; >>> @@ -643,6 +718,28 @@ >>> function = "gpio"; >>> bias-pull-down; >>> }; >>> + >>> + us_euro_hs_sel: us-euro-hs-sel { >>> + pins = "gpio81"; >>> + function = "gpio"; >>> + bias-pull-down; >>> + drive-strength = <2>; >>> + }; >>> + >>> + wcd_reset_n: wcd-reset-n { >>> + pins = "gpio83"; >>> + function = "gpio"; >>> + drive-strength = <8>; >>> + output-high; >>> + }; >>> + >>> + wcd_reset_n_sleep: wcd-reset-n-sleep { >>> + pins = "gpio83"; >>> + function = "gpio"; >>> + drive-strength = <8>; >>> + bias-disable; >>> + output-low; >>> + }; >> Also add to sc7280-herobrine-crd.dts if the other nodes are added. > Okay.
diff --git a/arch/arm64/boot/dts/qcom/sc7280-crd-r3.dts b/arch/arm64/boot/dts/qcom/sc7280-crd-r3.dts index 344338a..aa0bf6e2 100644 --- a/arch/arm64/boot/dts/qcom/sc7280-crd-r3.dts +++ b/arch/arm64/boot/dts/qcom/sc7280-crd-r3.dts @@ -87,6 +87,12 @@ ap_ts_pen_1v8: &i2c13 { pins = "gpio51"; }; +&wcd938x { + pinctrl-names = "default"; + pinctrl-0 = <&us_euro_hs_sel>; + us-euro-gpios = <&tlmm 81 GPIO_ACTIVE_HIGH>; +}; + &tlmm { tp_int_odl: tp-int-odl { pins = "gpio7"; diff --git a/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi b/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi index d58045d..f247403 100644 --- a/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi +++ b/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi @@ -20,6 +20,14 @@ #include "sc7280-chrome-common.dtsi" / { + max98360a: audio-codec-0 { + compatible = "maxim,max98360a"; + pinctrl-names = "default"; + pinctrl-0 = <&_en>; + sdmode-gpios = <&tlmm 63 GPIO_ACTIVE_HIGH>; + #sound-dai-cells = <0>; + }; + chosen { stdout-path = "serial0:115200n8"; }; diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi index 2f863c0..8dad599 100644 --- a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi +++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi @@ -20,6 +20,42 @@ serial1 = &uart7; }; + max98360a: audio-codec-0 { + compatible = "maxim,max98360a"; + pinctrl-names = "default"; + pinctrl-0 = <&_en>; + sdmode-gpios = <&tlmm 63 GPIO_ACTIVE_HIGH>; + #sound-dai-cells = <0>; + }; + + wcd938x: audio-codec-1 { + compatible = "qcom,wcd9385-codec"; + pinctrl-names = "default", "sleep"; + pinctrl-0 = <&wcd_reset_n>; + pinctrl-1 = <&wcd_reset_n_sleep>; + + reset-gpios = <&tlmm 83 GPIO_ACTIVE_HIGH>; + + qcom,rx-device = <&wcd_rx>; + qcom,tx-device = <&wcd_tx>; + + vdd-rxtx-supply = <&vreg_l18b_1p8>; + vdd-io-supply = <&vreg_l18b_1p8>; + vdd-buck-supply = <&vreg_l17b_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>; + #sound-dai-cells = <1>; + }; + gpio-keys { compatible = "gpio-keys"; label = "gpio-keys"; @@ -238,6 +274,19 @@ modem-init; }; +&lpass_rx_macro { + status = "okay"; +}; + +&lpass_tx_macro { + status = "okay"; +}; + +&lpass_va_macro { + status = "okay"; + vdd-micb-supply = <&vreg_bob>; +}; + &pcie1 { status = "okay"; perst-gpio = <&tlmm 2 GPIO_ACTIVE_LOW>; @@ -298,6 +347,26 @@ cd-gpios = <&tlmm 91 GPIO_ACTIVE_LOW>; }; +&swr0 { + status = "okay"; + wcd_rx: codec@0,4 { + compatible = "sdw20217010d00"; + reg = <0 4>; + #sound-dai-cells = <1>; + qcom,rx-port-mapping = <1 2 3 4 5>; + }; +}; + +&swr1 { + status = "okay"; + wcd_tx: codec@0,3 { + compatible = "sdw20217010d00"; + reg = <0 3>; + #sound-dai-cells = <1>; + qcom,tx-port-mapping = <1 2 3 4>; + }; +}; + &uart5 { compatible = "qcom,geni-debug-uart"; status = "okay"; @@ -561,6 +630,12 @@ }; &tlmm { + amp_en: amp-en { + pins = "gpio63"; + bias-pull-down; + drive-strength = <2>; + }; + bt_en: bt-en { pins = "gpio85"; function = "gpio"; @@ -643,6 +718,28 @@ function = "gpio"; bias-pull-down; }; + + us_euro_hs_sel: us-euro-hs-sel { + pins = "gpio81"; + function = "gpio"; + bias-pull-down; + drive-strength = <2>; + }; + + wcd_reset_n: wcd-reset-n { + pins = "gpio83"; + function = "gpio"; + drive-strength = <8>; + output-high; + }; + + wcd_reset_n_sleep: wcd-reset-n-sleep { + pins = "gpio83"; + function = "gpio"; + drive-strength = <8>; + bias-disable; + output-low; + }; }; &remoteproc_wpss {