From: Pierluigi Passaro <[email protected]>
The WM8904 codec supports both ADC and DMIC inputs.
Add dedicated controls to support the additional routing.
Signed-off-by: Pierluigi Passaro <[email protected]>
Signed-off by: Alifer Moraes <[email protected]>
---
sound/soc/codecs/wm8904.c | 34 ++++++++++++++++++++++++++++++++++
1 file changed, 34 insertions(+)
diff --git a/sound/soc/codecs/wm8904.c b/sound/soc/codecs/wm8904.c
index 4121771db104..c7987dc81e4d 100644
--- a/sound/soc/codecs/wm8904.c
+++ b/sound/soc/codecs/wm8904.c
@@ -837,6 +837,26 @@ static int out_pga_event(struct snd_soc_dapm_widget *w,
return 0;
}
+static const char *dmic_text[] = {
+ "DMIC1", "DMIC2"
+};
+
+static SOC_ENUM_SINGLE_DECL(dmic_enum,
+ WM8904_DIGITAL_MICROPHONE_0, 11, dmic_text);
+
+static const struct snd_kcontrol_new dmic_mux =
+ SOC_DAPM_ENUM("DMIC Mux", dmic_enum);
+
+static const char *cin_text[] = {
+ "ADC", "DMIC"
+};
+
+static SOC_ENUM_SINGLE_DECL(cin_enum,
+ WM8904_DIGITAL_MICROPHONE_0, 12, cin_text);
+
+static const struct snd_kcontrol_new cin_mux =
+ SOC_DAPM_ENUM("Capture Input", cin_enum);
+
static const char *input_mode_text[] = {
"Single-Ended", "Differential Line", "Differential Mic"
};
@@ -930,6 +950,10 @@ SND_SOC_DAPM_INPUT("IN2R"),
SND_SOC_DAPM_INPUT("IN3L"),
SND_SOC_DAPM_INPUT("IN3R"),
+SND_SOC_DAPM_MUX("DMIC Mux", SND_SOC_NOPM, 0, 0, &dmic_mux),
+SND_SOC_DAPM_MUX("Left Capture Input", SND_SOC_NOPM, 0, 0, &cin_mux),
+SND_SOC_DAPM_MUX("Right Capture Input", SND_SOC_NOPM, 0, 0, &cin_mux),
+
SND_SOC_DAPM_SUPPLY("MICBIAS", WM8904_MIC_BIAS_CONTROL_0, 0, 0, NULL, 0),
SND_SOC_DAPM_MUX("Left Capture Mux", SND_SOC_NOPM, 0, 0, &lin_mux),
@@ -1093,11 +1117,21 @@ static const struct snd_soc_dapm_route adc_intercon[] = {
{ "AIFOUTL", NULL, "AIFOUTL Mux" },
{ "AIFOUTR", NULL, "AIFOUTR Mux" },
+ { "DMIC Mux", "DMIC1", "IN1L" },
+ { "DMIC Mux", "DMIC2", "IN1R" },
+
+ { "Left Capture Input", "ADC", "Left Capture PGA" },
+ { "Left Capture Input", "DMIC", "DMIC Mux" },
+ { "Right Capture Input", "ADC", "Right Capture PGA" },
+ { "Right Capture Input", "DMIC", "DMIC Mux" },
+
{ "ADCL", NULL, "CLK_DSP" },
{ "ADCL", NULL, "Left Capture PGA" },
+ { "ADCL", NULL, "Left Capture Input" },
{ "ADCR", NULL, "CLK_DSP" },
{ "ADCR", NULL, "Right Capture PGA" },
+ { "ADCR", NULL, "Right Capture Input" },
};
static const struct snd_soc_dapm_route dac_intercon[] = {
--
2.25.1
On Mon, Mar 07, 2022 at 11:10:41AM -0300, Alifer Moraes wrote:
> +static const char *cin_text[] = {
> + "ADC", "DMIC"
> +};
> +
> +static SOC_ENUM_SINGLE_DECL(cin_enum,
> + WM8904_DIGITAL_MICROPHONE_0, 12, cin_text);
Why would this be runtime selectable? I'd expect the decision to use
an analogue or digital microphone to be made in the hardware design.
Hi All,
> > +static const char *cin_text[] = {
> > +???? "ADC", "DMIC"
> > +};
> > +
> > +static SOC_ENUM_SINGLE_DECL(cin_enum,
> > +???????????????????????? WM8904_DIGITAL_MICROPHONE_0, 12, cin_text);
>
> Why would this be runtime selectable?? I'd expect the decision to use
> an analogue or digital microphone to be made in the hardware design.
I agree that dedicated HW is required, but currently SW side there's no support at all.
This patch is aiming to provide a way to enable DMIC on boards using it.
Is this supposed to be managed in a different way ?
Thanks
Best Regards
Pier
On Mon, Jun 20, 2022 at 02:49:51PM +0000, Pierluigi Passaro wrote:
> > > +static const char *cin_text[] = {
> > > +???? "ADC", "DMIC"
> > > +};
> > > +static SOC_ENUM_SINGLE_DECL(cin_enum,
> > > +???????????????????????? WM8904_DIGITAL_MICROPHONE_0, 12, cin_text);
> > Why would this be runtime selectable?? I'd expect the decision to use
> > an analogue or digital microphone to be made in the hardware design.
> I agree that dedicated HW is required, but currently SW side there's no support at all.
> This patch is aiming to provide a way to enable DMIC on boards using it.
> Is this supposed to be managed in a different way ?
Via firmware description.
On Mon, Jun 20, 2022 at 03:03:50PM +0000, Pierluigi Passaro wrote:
> > Via firmware description.
> Can you please provide any reference approach in the kernel code ?
git grep of_
git grep fwnode_
and I don't immediately remember what the prefix is for ACPI functions.
> > > Via firmware description.
> > Can you please provide any reference approach in the kernel code ?
> git grep of_
> git grep fwnode_
> and I don't immediately remember what the prefix is for ACPI functions.
Just for my understanding, are you suggesting to set a device tree property to force a fixed behavior in the driver ?
WM8904 allows using both a DMIC and LINEIN, switching between one or the other and this is how we currently use it.
Why the user should not be allowed to switch between DMIC and LINEIN ?
> > > > +static const char *cin_text[] = {
> > > > +???? "ADC", "DMIC"
> > > > +};
> > > > +static SOC_ENUM_SINGLE_DECL(cin_enum,
> > > > +???????????????????????? WM8904_DIGITAL_MICROPHONE_0, 12, cin_text);
> > > Why would this be runtime selectable?? I'd expect the decision to use
> > > an analogue or digital microphone to be made in the hardware design.
> > I agree that dedicated HW is required, but currently SW side there's no support at all.
> > This patch is aiming to provide a way to enable DMIC on boards using it.
> > Is this supposed to be managed in a different way ?
> Via firmware description.
Can you please provide any reference approach in the kernel code ?
On Mon, Jun 20, 2022 at 03:30:45PM +0000, Pierluigi Passaro wrote:
> Just for my understanding, are you suggesting to set a device tree property to force a fixed behavior in the driver ?
Yes.
> WM8904 allows using both a DMIC and LINEIN, switching between one or the other and this is how we currently use it.
> Why the user should not be allowed to switch between DMIC and LINEIN ?
The device shares pins between the line inputs and the DMIC inputs so at
least some of the configuration is going to be determinted at system
design time, that will fix the usable values of at least one of the
controls which ought to be reflected in the runtime behaviour.
Please fix your mail client to word wrap within paragraphs at something
substantially less than 80 columns. Doing this makes your messages much
easier to read and reply to.
> > Just for my understanding, are you suggesting to set a device tree
> > property to force a fixed behavior in the driver ?
> Yes.
Why should we use a fixed behavior ?
> > WM8904 allows using both a DMIC and LINEIN, switching between one or
> > the other and this is how we currently use it.
> > Why the user should not be allowed to switch between DMIC and LINEIN ?
> The device shares pins between the line inputs and the DMIC inputs so at
> least some of the configuration is going to be determinted at system
> design time, that will fix the usable values of at least one of the
> controls which ought to be reflected in the runtime behaviour.
In our design we use:
- pin 1: DMIC_CLK
- pin 24: LINEIN2R
- pin 26: LINEIN2L
- pin 27: DMIC_DATA
we have no pins shared among DMIC and LINEIN.
We have several customer switching between DMIC and LINEIN at runtime.
> Please fix your mail client to word wrap within paragraphs at something
> substantially less than 80 columns. Doing this makes your messages much
> easier to read and reply to.
I beg your pardon: it should be fixed now.
On Mon, Jun 20, 2022 at 05:52:43PM +0000, Pierluigi Passaro wrote:
> > > Just for my understanding, are you suggesting to set a device tree
> > > property to force a fixed behavior in the driver ?
> > Yes.
> Why should we use a fixed behavior ?
The things that are fixed by the design should be fixed.
> > The device shares pins between the line inputs and the DMIC inputs so at
> > least some of the configuration is going to be determinted at system
> > design time, that will fix the usable values of at least one of the
> > controls which ought to be reflected in the runtime behaviour.
> In our design we use:
> - pin 1: DMIC_CLK
> - pin 24: LINEIN2R
> - pin 26: LINEIN2L
> - pin 27: DMIC_DATA
> we have no pins shared among DMIC and LINEIN.
This means that DMICDAT2 is not usefully selectable at runtime, you've
got IN1 as digital and IN2 as analogue, so while the DMIC/ADC switch is
useful the DMIC1/2 switch is not.
> > > > Just for my understanding, are you suggesting to set a device tree
> > > > property to force a fixed behavior in the driver ?
> > > Yes.
> > Why should we use a fixed behavior ?
> The things that are fixed by the design should be fixed.
> > > The device shares pins between the line inputs and the DMIC inputs so at
> > > least some of the configuration is going to be determinted at system
> > > design time, that will fix the usable values of at least one of the
> > > controls which ought to be reflected in the runtime behaviour.
> > In our design we use:
> > - pin 1: DMIC_CLK
> > - pin 24: LINEIN2R
> > - pin 26: LINEIN2L
> > - pin 27: DMIC_DATA
> > we have no pins shared among DMIC and LINEIN.
> This means that DMICDAT2 is not usefully selectable at runtime, you've
> got IN1 as digital and IN2 as analogue, so while the DMIC/ADC switch is
> useful the DMIC1/2 switch is not.
A customer could have the following working configuration
- pin 1: DMIC_CLK
- pin 24: LINEIN2R
- pin 25: DMICDAT2
- pin 26: LINEIN2L
- pin 27: DMICDAT1
with no shared pins: here there's the chance to select DMIC1, DMIC2 and
LINEIN2 at runtime: I can't find a reason for a fixed behavior.
Can you please elaborate ?
On Mon, Jun 20, 2022 at 07:53:56PM +0000, Pierluigi Passaro wrote:
> > This means that DMICDAT2 is not usefully selectable at runtime, you've
> > got IN1 as digital and IN2 as analogue, so while the DMIC/ADC switch is
> > useful the DMIC1/2 switch is not.
> A customer could have the following working configuration
> - pin 1: DMIC_CLK
> - pin 24: LINEIN2R
> - pin 25: DMICDAT2
> - pin 26: LINEIN2L
> - pin 27: DMICDAT1
> with no shared pins: here there's the chance to select DMIC1, DMIC2 and
> LINEIN2 at runtime: I can't find a reason for a fixed behavior.
> Can you please elaborate ?
So in that case the driver should offer the DMIC1/2 selection. The
driver should be looking at which pins are wired up as DMICs and only
registering controls that can actually be used in the system based on
the pins that are wired up.