[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <77a94d4c-c79e-dede-f365-769f0fae883c@quicinc.com>
Date: Mon, 18 Apr 2022 19:36:34 +0530
From: Srinivasa Rao Mandadapu <quic_srivasam@...cinc.com>
To: Matthias Kaehlcke <mka@...omium.org>
CC: <agross@...nel.org>, <bjorn.andersson@...aro.org>,
<robh+dt@...nel.org>, <linux-arm-msm@...r.kernel.org>,
<devicetree@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
<quic_rohkumar@...cinc.com>, <srinivas.kandagatla@...aro.org>,
<dianders@...omium.org>, <swboyd@...omium.org>,
<judyhsiao@...omium.org>,
Venkata Prasad Potturu <quic_potturu@...cinc.com>
Subject: Re: [PATCH v7 2/4] arm64: dts: qcom: sc7280: Add nodes for wcd9385
and max98360a codec
On 4/14/2022 1:05 AM, Matthias Kaehlcke wrote:
Thanks for your time Matthias!!!
> On Wed, Apr 13, 2022 at 08:51:15PM +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.
>>
>> Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@...cinc.com>
>> Co-developed-by: Venkata Prasad Potturu <quic_potturu@...cinc.com>
>> Signed-off-by: Venkata Prasad Potturu <quic_potturu@...cinc.com>
>> ---
> Applicable to all you patches: a per-patch change log would be really
> nice for reviewers. It can help them to focus on the parts that have
> actually changed with respect to earlier revisions they might have
> already reviewed.
Okay. Understood. But we have been updating change log in cover letter,
it would be difficult to segregate now.
Next time onward will take care.
>
>> arch/arm64/boot/dts/qcom/sc7280-crd.dts | 6 ++
>> arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi | 8 +++
>> arch/arm64/boot/dts/qcom/sc7280-idp.dtsi | 94 ++++++++++++++++++++++++++
>> 3 files changed, 108 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/sc7280-crd.dts b/arch/arm64/boot/dts/qcom/sc7280-crd.dts
>> index e2efbdd..b944366 100644
>> --- a/arch/arm64/boot/dts/qcom/sc7280-crd.dts
>> +++ b/arch/arm64/boot/dts/qcom/sc7280-crd.dts
> Your branch needs a refresh, this file doesn't exist anymore.
>
> You'll want to change sc7280-crd-r3.dts for the CRD <= 2.x and
> sc7280-herobrine-crd.dts for the CRD >= 3.x
Okay. Will do accordingly.
>
>> @@ -84,6 +84,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 dc17f20..8e4f822 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>;
>> + };
>> +
> I agreed earlier that sorting by node name is correct, but that wasn't
> entirely true. The nodes should be sorted by name within the 'device'
> section, i.e. after the comment "/* BOARD-SPECIFIC TOP LEVEL NODES */".
Okay. will sort accordingly.
>
>> 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 110d9e9..6004c08 100644
>> --- a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
>> @@ -20,6 +20,41 @@
>> 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";
>> + #sound-dai-cells = <1>;
>> +
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&wcd_reset_n>, <&wcd_reset_n_sleep>;
> Shouldn't this be:
>
> pinctrl-names = "default", "sleep";
> pinctrl-0 = <&wcd_reset_n>;
> pinctrl-1 = <&wcd_reset_n_sleep>;
>
> ?
Okay. Will update accordingly.
>
> nit: make the ordering of properties (pinctrl, #sound-dai-cells) and
> spacing within the node consistent for max98360a and wcd938x? Not
> super-important, but since both are codecs and added by the same
> patch, why not :)
Okay. will maintain accordingly.
>
>> + 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>;
>> + };
>> +
>> gpio-keys {
>> compatible = "gpio-keys";
>> label = "gpio-keys";
>> @@ -238,6 +273,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 +346,24 @@
>> cd-gpios = <&tlmm 91 GPIO_ACTIVE_LOW>;
>> };
>>
>> +&swr0 {
>> + wcd_rx: codec@0,4 {
>> + compatible = "sdw20217010d00";
>> + reg = <0 4>;
>> + #sound-dai-cells = <1>;
>> + qcom,rx-port-mapping = <1 2 3 4 5>;
>> + };
>> +};
>> +
>> +&swr1 {
>> + 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 +627,12 @@
>> };
>>
>> &tlmm {
>> + amp_en: amp-en {
>> + pins = "gpio63";
>> + bias-pull-down;
>> + drive-strength = <2>;
>> + };
>> +
>> bt_en: bt-en {
>> pins = "gpio85";
>> function = "gpio";
>> @@ -643,5 +715,27 @@
>> 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;
>> + };
>> };
>>
>> --
>> 2.7.4
>>
Powered by blists - more mailing lists