2023-09-26 20:52:49

by Stephan Gerhold

[permalink] [raw]
Subject: Re: [PATCH 05/13] arm64: dts: qcom: msm8916-samsung-serranove: Add sound and modem

On Tue, Sep 26, 2023 at 08:55:14PM +0200, Konrad Dybcio wrote:
> On 26.09.2023 18:51, Stephan Gerhold wrote:
> > Enable sound and modem for the Samsung S4 Mini Value Edition. The setup
> > is similar to most MSM8916 devices, i.e.:
> >
> > - QDSP6 audio
> > - Speaker/earpiece/headphones/microphones via digital/analog codec in
> > MSM8916/PM8916
> > - WWAN Internet via BAM-DMUX
> >
> > except:
> >
> > - Samsung-specific audio jack detection (not supported yet)
> >
> > Signed-off-by: Stephan Gerhold <[email protected]>
> > ---
> > arch/arm64/boot/dts/qcom/msm8916-samsung-serranove.dts | 14 ++++++++++++++
> > 1 file changed, 14 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/qcom/msm8916-samsung-serranove.dts b/arch/arm64/boot/dts/qcom/msm8916-samsung-serranove.dts
> > index 68da2a2d3077..5f33aa0ad7b5 100644
> > --- a/arch/arm64/boot/dts/qcom/msm8916-samsung-serranove.dts
> > +++ b/arch/arm64/boot/dts/qcom/msm8916-samsung-serranove.dts
> > @@ -6,6 +6,8 @@
> > /dts-v1/;
> >
> > #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>
> > @@ -319,6 +321,10 @@ &blsp_uart2 {
> > status = "okay";
> > };
> >
> > +&mpss_mem {
> > + reg = <0x0 0x86800000 0x0 0x5a00000>;
> > +};
> > +
> > &pm8916_resin {
> > status = "okay";
> > linux,code = <KEY_VOLUMEDOWN>;
> > @@ -350,6 +356,14 @@ &sdhc_2 {
> > no-1-8-v;
> > };
> >
> > +&sound {
> > + status = "okay";
> > + audio-routing =
> > + "AMIC1", "MIC BIAS External1",
> > + "AMIC2", "MIC BIAS Internal2",
> > + "AMIC3", "MIC BIAS External1";
> > +};
> I *think* we should be able to harmlessly enable &audio globally?
>

What about boards that do not have/use audio at all? (see
msm8916-ufi.dtsi). We don't even want to load the kernel modules on
those.

IMO the SoC dtsi should always be minimal by default. This also
guarantees that you don't run into trouble because of half- or
incorrectly configured components during early bring-up, especially if
you don't have serial and are hoping to get the device booting far
enough to debug it.

Thanks,
Stephan


2023-09-26 23:29:03

by Konrad Dybcio

[permalink] [raw]
Subject: Re: [PATCH 05/13] arm64: dts: qcom: msm8916-samsung-serranove: Add sound and modem

[...]

>>>
>>> +&sound {
>>> + status = "okay";
>>> + audio-routing =
>>> + "AMIC1", "MIC BIAS External1",
>>> + "AMIC2", "MIC BIAS Internal2",
>>> + "AMIC3", "MIC BIAS External1";
>>> +};
>> I *think* we should be able to harmlessly enable &audio globally?
>>
>
> What about boards that do not have/use audio at all? (see
> msm8916-ufi.dtsi). We don't even want to load the kernel modules on
> those.
Is it really an issue other than losing a few kb of memory?

>
> IMO the SoC dtsi should always be minimal by default. This also
> guarantees that you don't run into trouble because of half- or
> incorrectly configured components during early bring-up, especially if
> you don't have serial and are hoping to get the device booting far
> enough to debug it.
Generally I'd agree, but if the audio machine driver cannot NOP
successfully without a topology configuration, that's a problem.

Konrad