From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 914A0C433FE for ; Mon, 18 Apr 2022 16:45:20 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1346373AbiDRQr6 (ORCPT ); Mon, 18 Apr 2022 12:47:58 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:50238 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1346403AbiDRQr5 (ORCPT ); Mon, 18 Apr 2022 12:47:57 -0400 Received: from mail-pg1-x530.google.com (mail-pg1-x530.google.com [IPv6:2607:f8b0:4864:20::530]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id CE79E30F49 for ; Mon, 18 Apr 2022 09:45:17 -0700 (PDT) Received: by mail-pg1-x530.google.com with SMTP id j71so6322635pge.11 for ; Mon, 18 Apr 2022 09:45:17 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=4xwTXGyMiRwQRbK7umG3ZynWZ3Dw576AsZDOR95/gXU=; b=B2EWZ80+i+OxKDK4aG1UbaOkuqHEb3Qp0nagwb7VyMcVLJCoOV5edrOdNc4hq5gp/4 37HAOe2WOYlwTQPlhCzMHONQzGpLuG6MpmUTAIXlmeuWi+TEP1aE9eZNDeVnq8ekL9Vh ky6BvKBDjOwM3WC59Xcofqh6srmpHLzTtdDVA= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=4xwTXGyMiRwQRbK7umG3ZynWZ3Dw576AsZDOR95/gXU=; b=22jtJl+Sqfu4MxnE4vLaSOvOZz8NFfDsW/aO8iyw7feKv1mACN6rsJ+TgNJazw+zWL iCDU5G9e1DP5Q/XWpxtqeW1eLKsfQ/9gP8+BSEYx96u6u6AOsbr7uXILxw4dsj7IPrMb aLdm+PUWzUIXZLZzAj2rFmcyzThb8k2h3MYfVY2iHPC9sZa6uhENYhBYur8CJvtIJHNk uxlSBqK8jd6kfpqcnDbReq12p5u0f0PzJan1osw6ferSrox4ot3vb8eY+3qcCn9aKBkk 7T73Oh+il7IxK4QK9c6yMkQpoGEt7k0A0f4k+cemAmj0gqqL4uLOQYq3GQxPeeCKSAq0 Sl6Q== X-Gm-Message-State: AOAM530TTZ2gM87b/d3QhvVhLoAbv9FqBhTqyLEaDpCuRyhu6dutIAdf skY5982gQZZAncx67L9FkkGwAw== X-Google-Smtp-Source: ABdhPJwRJZtekdiefz/R1D51gc7pQIKgOwr5/nKXzzfb1qXbeZEcEiLIiitOAbOMJnmAR2Tk0AGPbQ== X-Received: by 2002:a63:d915:0:b0:39d:3f7d:1850 with SMTP id r21-20020a63d915000000b0039d3f7d1850mr10870714pgg.10.1650300315780; Mon, 18 Apr 2022 09:45:15 -0700 (PDT) Received: from localhost ([2620:15c:202:201:6b32:a0a5:ec32:c287]) by smtp.gmail.com with UTF8SMTPSA id r33-20020a63fc61000000b003a9f98905e9sm3995484pgk.76.2022.04.18.09.45.14 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 18 Apr 2022 09:45:15 -0700 (PDT) Date: Mon, 18 Apr 2022 09:45:13 -0700 From: Matthias Kaehlcke To: Srinivasa Rao Mandadapu Cc: agross@kernel.org, bjorn.andersson@linaro.org, robh+dt@kernel.org, linux-arm-msm@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, quic_rohkumar@quicinc.com, srinivas.kandagatla@linaro.org, dianders@chromium.org, swboyd@chromium.org, judyhsiao@chromium.org, Venkata Prasad Potturu Subject: Re: [PATCH v8 2/4] arm64: dts: qcom: sc7280: Add nodes for wcd9385 and max98360a codec Message-ID: References: <1650291252-30398-1-git-send-email-quic_srivasam@quicinc.com> <1650291252-30398-3-git-send-email-quic_srivasam@quicinc.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <1650291252-30398-3-git-send-email-quic_srivasam@quicinc.com> Precedence: bulk List-ID: X-Mailing-List: devicetree@vger.kernel.org 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. > Signed-off-by: Srinivasa Rao Mandadapu > Co-developed-by: Venkata Prasad Potturu > Signed-off-by: Venkata Prasad Potturu > --- > 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 > + > &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'? > + 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? > + > 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. > + > &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 > + 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 > + 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? > + > &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.