[05/13] arm64: dts: qcom: msm8916-samsung-serranove: Add sound and modem

Message ID 20230926-msm8916-modem-v1-5-398eec74bac9@gerhold.net
State New
Headers
Series arm64: dts: qcom: msm8916/39: Enable sound and modem with QDSP6 |

Commit Message

Stephan Gerhold Sept. 26, 2023, 4:51 p.m. UTC
  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 <stephan@gerhold.net>
---
 arch/arm64/boot/dts/qcom/msm8916-samsung-serranove.dts | 14 ++++++++++++++
 1 file changed, 14 insertions(+)
  

Comments

Konrad Dybcio Sept. 26, 2023, 6:55 p.m. UTC | #1
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 <stephan@gerhold.net>
> ---
>  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?

Konrad
  
Stephan Gerhold Sept. 26, 2023, 7:29 p.m. UTC | #2
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 <stephan@gerhold.net>
> > ---
> >  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
  
Konrad Dybcio Sept. 26, 2023, 7:57 p.m. UTC | #3
[...]

>>>  
>>> +&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
  
Stephan Gerhold Sept. 26, 2023, 8:04 p.m. UTC | #4
On Tue, Sep 26, 2023 at 09:57:51PM +0200, Konrad Dybcio wrote:
> [...]
> 
> >>>  
> >>> +&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?
> 

Well, the msm8916-ufi.dtsi boards (basically USB modem/WiFi sticks)
have 512 MiB of RAM, with 85 MiB reserved for modem firmware, plus more
for TZ, HYP etc etc. That's not too much :D

> > 
> > 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.
> 

I think it will effectively fail to probe because there are no DAI links
and no "model". I guess you could consider this to be a NOP but it's
confusing and takes away the attention from the actual errors in dmesg.

I would say it's disabled by default for the same reason that WiFi,
SDHCI, UFS etc etc are disabled by default. They simply don't do
anything useful without additional configuration & hardware support.

Thanks,
Stephan
  

Patch

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";
+};
+
 &usb {
 	status = "okay";
 	extcon = <&muic>, <&muic>;