Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:5 as permitted sender) client-ip=2620:137:e000::3:5;
Date:   Tue, 26 Sep 2023 21:26:45 +0200
From:   Stephan Gerhold <stephan@gerhold.net>
To:     Konrad Dybcio <konrad.dybcio@linaro.org>
Cc:     Bjorn Andersson <andersson@kernel.org>,
        Andy Gross <agross@kernel.org>, linux-arm-msm@vger.kernel.org,
        devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
        phone-devel@vger.kernel.org, ~postmarketos/upstreaming@lists.sr.ht,
        Bryan O'Donoghue <bryan.odonoghue@linaro.org>,
        "Lin, Meng-Bo" <linmengbo0689@protonmail.com>
Subject: Re: [PATCH 04/13] arm64: dts: qcom: msm8916-samsung-a2015: Add sound
 and modem
Message-ID: <ZRMwdRo9hAm4BO5E@gerhold.net>
References: <20230926-msm8916-modem-v1-0-398eec74bac9@gerhold.net>
 <20230926-msm8916-modem-v1-4-398eec74bac9@gerhold.net>
 <2c9eda54-a90e-483a-abc2-865d82854b80@linaro.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <2c9eda54-a90e-483a-abc2-865d82854b80@linaro.org>
Content-Transfer-Encoding: 7bit
Precedence: bulk

On Tue, Sep 26, 2023 at 08:54:29PM +0200, Konrad Dybcio wrote:
> On 26.09.2023 18:51, Stephan Gerhold wrote:
> > Enable sound and modem for the Samsung A2015 based devices (A3, A5, E5,
> > E7, Grand Max). The setup is similar to most MSM8916 devices, i.e.:
> > 
> >  - QDSP6 audio
> >  - Earpiece/headphones/microphones via digital/analog codec in
> >    MSM8916/PM8916
> >  - WWAN Internet via BAM-DMUX
> > 
> > except:
> > 
> >  - NXP TFA9895 codec for speaker on Quaternary MI2S
> >  - Samsung-specific audio jack detection (not supported yet)
> > 
> > [Lin: Add e2015 and grandmax]
> > Co-developed-by: "Lin, Meng-Bo" <linmengbo0689@protonmail.com>
> > Signed-off-by: "Lin, Meng-Bo" <linmengbo0689@protonmail.com>
> > Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
> > ---
> >  .../dts/qcom/msm8916-samsung-a2015-common.dtsi     | 55 ++++++++++++++++++++++
> >  .../dts/qcom/msm8916-samsung-e2015-common.dtsi     |  4 ++
> >  .../boot/dts/qcom/msm8916-samsung-grandmax.dts     |  4 ++
> >  3 files changed, 63 insertions(+)
> > 
> > diff --git a/arch/arm64/boot/dts/qcom/msm8916-samsung-a2015-common.dtsi b/arch/arm64/boot/dts/qcom/msm8916-samsung-a2015-common.dtsi
> > index 0b29132b74e1..f71b18d89bf9 100644
> > --- a/arch/arm64/boot/dts/qcom/msm8916-samsung-a2015-common.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/msm8916-samsung-a2015-common.dtsi
> > @@ -1,10 +1,13 @@
> >  // SPDX-License-Identifier: GPL-2.0-only
> >  
> >  #include "msm8916-pm8916.dtsi"
> > +#include "msm8916-modem-qdsp6.dtsi"
> > +
> >  #include <dt-bindings/gpio/gpio.h>
> >  #include <dt-bindings/input/input.h>
> >  #include <dt-bindings/interrupt-controller/irq.h>
> >  #include <dt-bindings/pinctrl/qcom,pmic-gpio.h>
> > +#include <dt-bindings/sound/apq8016-lpass.h>
> >  
> >  / {
> >  	aliases {
> > @@ -196,6 +199,18 @@ vibrator: vibrator {
> >  	};
> >  };
> >  
> > +&blsp_i2c1 {
> > +	status = "okay";
> > +
> > +	speaker_codec: audio-codec@34 {
> > +		compatible = "nxp,tfa9895";
> > +		reg = <0x34>;
> > +		vddd-supply = <&pm8916_l5>;
> > +		sound-name-prefix = "Speaker";
> > +		#sound-dai-cells = <0>;
> > +	};
> > +};
> > +
> >  &blsp_i2c2 {
> >  	status = "okay";
> >  
> > @@ -243,6 +258,13 @@ &gpu {
> >  	status = "okay";
> >  };
> >  
> > +&lpass {
> > +	dai-link@3 {
> > +		reg = <MI2S_QUATERNARY>;
> > +		qcom,playback-sd-lines = <1>;
> > +	};
> > +};
> Is that not status = reserved?
> 

Correct. This is here to simplify switching to the modem-bypass audio
routing if someone does not need (or want) the modem. The direct audio
path with the LPASS drivers tends to be more reliable and configurable
(especially wrt bit formats, sampling rates, latency, channels etc).
I know that at some point this helped someone who tried to use an old
phone as some kind of portable musical instrument / synthesizer.

It's not too obvious that these definitions would be needed when making
those changes (because devices using the standard SD lines (i.e <0>) do
not need it). If you forget about this you get non-functional audio with
no error or any hint what could be wrong.

To simplify switching between the different audio routing options, the
lk2nd bootloader actually has an option to do this transformation in the
DTB automagically during boot. It's sort of like a DTB overlay that
disables the QDSP6 audio path and enables this node instead. The DAI
links are also adjusted where necessary.

Do you think a comment would help here?

Thanks,
Stephan