[PATCH] ASoC: max98088: add support for reg_4a_cfg_bypass reg

[PATCH] ASoC: max98088: add support for reg_4a_cfg_bypass reg

[Tommaso Merciai]

Add mixer controls support for M98088_REG_4A_CFG_BYPASS register

References:
 - https://datasheets.maximintegrated.com/en/ds/MAX98089.pdf p71, p113

Signed-off-by: Tommaso Merciai <tommaso.merciai@amarulasolutions.com>
---
 sound/soc/codecs/max98088.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/sound/soc/codecs/max98088.c b/sound/soc/codecs/max98088.c
index 429717d4ac5a..f8ec2f164e08 100644
--- a/sound/soc/codecs/max98088.c
+++ b/sound/soc/codecs/max98088.c
@@ -486,6 +486,11 @@ static const struct snd_kcontrol_new max98088_snd_controls[] = {
        SOC_SINGLE("EQ1 Switch", M98088_REG_49_CFG_LEVEL, 0, 1, 0),
        SOC_SINGLE("EQ2 Switch", M98088_REG_49_CFG_LEVEL, 1, 1, 0),
 
+       SOC_SINGLE("SPK Bypass Switch", M98088_REG_4A_CFG_BYPASS, 0, 1, 0),
+       SOC_SINGLE("REC Bypass Switch", M98088_REG_4A_CFG_BYPASS, 1, 1, 0),
+       SOC_SINGLE("MIC2 Bypass Switch", M98088_REG_4A_CFG_BYPASS, 4, 1, 0),
+       SOC_SINGLE("INA Bypass Switch", M98088_REG_4A_CFG_BYPASS, 7, 1, 0),
+
        SOC_ENUM("EX Limiter Mode", max98088_exmode_enum),
        SOC_ENUM("EX Limiter Threshold", max98088_ex_thresh_enum),
 
-- 
2.25.1

Re: [PATCH] ASoC: max98088: add support for reg_4a_cfg_bypass reg

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 862 bytes --]

On Thu, May 12, 2022 at 09:43:58AM +0200, Tommaso Merciai wrote:

> Add mixer controls support for M98088_REG_4A_CFG_BYPASS register

> +++ b/sound/soc/codecs/max98088.c
> @@ -486,6 +486,11 @@ static const struct snd_kcontrol_new max98088_snd_controls[] = {
>         SOC_SINGLE("EQ1 Switch", M98088_REG_49_CFG_LEVEL, 0, 1, 0),
>         SOC_SINGLE("EQ2 Switch", M98088_REG_49_CFG_LEVEL, 1, 1, 0),
>  
> +       SOC_SINGLE("SPK Bypass Switch", M98088_REG_4A_CFG_BYPASS, 0, 1, 0),
> +       SOC_SINGLE("REC Bypass Switch", M98088_REG_4A_CFG_BYPASS, 1, 1, 0),
> +       SOC_SINGLE("MIC2 Bypass Switch", M98088_REG_4A_CFG_BYPASS, 4, 1, 0),
> +       SOC_SINGLE("INA Bypass Switch", M98088_REG_4A_CFG_BYPASS, 7, 1, 0),

These look like they should be DAPM controls since they're controlling
audio routing but they're being added as regular controls.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] ASoC: max98088: add support for reg_4a_cfg_bypass reg

[Tommaso Merciai]

On Thu, May 12, 2022 at 11:12:02AM +0100, Mark Brown wrote:
> On Thu, May 12, 2022 at 09:43:58AM +0200, Tommaso Merciai wrote:
> 
> > Add mixer controls support for M98088_REG_4A_CFG_BYPASS register
> 
> > +++ b/sound/soc/codecs/max98088.c
> > @@ -486,6 +486,11 @@ static const struct snd_kcontrol_new max98088_snd_controls[] = {
> >         SOC_SINGLE("EQ1 Switch", M98088_REG_49_CFG_LEVEL, 0, 1, 0),
> >         SOC_SINGLE("EQ2 Switch", M98088_REG_49_CFG_LEVEL, 1, 1, 0),
> >  
> > +       SOC_SINGLE("SPK Bypass Switch", M98088_REG_4A_CFG_BYPASS, 0, 1, 0),
> > +       SOC_SINGLE("REC Bypass Switch", M98088_REG_4A_CFG_BYPASS, 1, 1, 0),
> > +       SOC_SINGLE("MIC2 Bypass Switch", M98088_REG_4A_CFG_BYPASS, 4, 1, 0),
> > +       SOC_SINGLE("INA Bypass Switch", M98088_REG_4A_CFG_BYPASS, 7, 1, 0),
> 
> These look like they should be DAPM controls since they're controlling
> audio routing but they're being added as regular controls.

Hi Mark,
Thanks for the review, I'll send v2.

Tommaso

-- 
Tommaso Merciai
Embedded Linux Engineer
tommaso.merciai@amarulasolutions.com
__________________________________

Amarula Solutions SRL
Via Le Canevare 30, 31100 Treviso, Veneto, IT
T. +39 042 243 5310
info@amarulasolutions.com
www.amarulasolutions.com

Re: [PATCH] ASoC: max98088: add support for reg_4a_cfg_bypass reg

[Tommaso Merciai]

On Thu, May 12, 2022 at 11:12:02AM +0100, Mark Brown wrote:
> On Thu, May 12, 2022 at 09:43:58AM +0200, Tommaso Merciai wrote:
> 
> > Add mixer controls support for M98088_REG_4A_CFG_BYPASS register
> 
> > +++ b/sound/soc/codecs/max98088.c
> > @@ -486,6 +486,11 @@ static const struct snd_kcontrol_new max98088_snd_controls[] = {
> >         SOC_SINGLE("EQ1 Switch", M98088_REG_49_CFG_LEVEL, 0, 1, 0),
> >         SOC_SINGLE("EQ2 Switch", M98088_REG_49_CFG_LEVEL, 1, 1, 0),
> >  
> > +       SOC_SINGLE("SPK Bypass Switch", M98088_REG_4A_CFG_BYPASS, 0, 1, 0),
> > +       SOC_SINGLE("REC Bypass Switch", M98088_REG_4A_CFG_BYPASS, 1, 1, 0),
> > +       SOC_SINGLE("MIC2 Bypass Switch", M98088_REG_4A_CFG_BYPASS, 4, 1, 0),
> > +       SOC_SINGLE("INA Bypass Switch", M98088_REG_4A_CFG_BYPASS, 7, 1, 0),
> 
> These look like they should be DAPM controls since they're controlling
> audio routing but they're being added as regular controls.

Hi Mark,
Sorry again. You suggest to create a new structure for these entries,
for example:

/* Out Bypass mixer switch */
static const struct snd_kcontrol_new max98088_out_bypass_mixer_controls[] = {
       SOC_DAPM_SINGLE("INA Switch", M98088_REG_4A_CFG_BYPASS, 7, 1, 0),
       SOC_DAPM_SINGLE("MIC2 Switch", M98088_REG_4A_CFG_BYPASS, 4, 1, 0),
       SOC_DAPM_SINGLE("REC Switch", M98088_REG_4A_CFG_BYPASS, 1, 1, 0),
       SOC_DAPM_SINGLE("SPK Switch", M98088_REG_4A_CFG_BYPASS, 0, 1, 0),
};

Let me know.

Thanks in advance.
Tommaso

-- 
Tommaso Merciai
Embedded Linux Engineer
tommaso.merciai@amarulasolutions.com
__________________________________

Amarula Solutions SRL
Via Le Canevare 30, 31100 Treviso, Veneto, IT
T. +39 042 243 5310
info@amarulasolutions.com
www.amarulasolutions.com

Re: [PATCH] ASoC: max98088: add support for reg_4a_cfg_bypass reg

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 913 bytes --]

On Thu, May 12, 2022 at 12:46:42PM +0200, Tommaso Merciai wrote:
> On Thu, May 12, 2022 at 11:12:02AM +0100, Mark Brown wrote:

> > These look like they should be DAPM controls since they're controlling
> > audio routing but they're being added as regular controls.

> Sorry again. You suggest to create a new structure for these entries,
> for example:

> /* Out Bypass mixer switch */
> static const struct snd_kcontrol_new max98088_out_bypass_mixer_controls[] = {
>        SOC_DAPM_SINGLE("INA Switch", M98088_REG_4A_CFG_BYPASS, 7, 1, 0),
>        SOC_DAPM_SINGLE("MIC2 Switch", M98088_REG_4A_CFG_BYPASS, 4, 1, 0),
>        SOC_DAPM_SINGLE("REC Switch", M98088_REG_4A_CFG_BYPASS, 1, 1, 0),
>        SOC_DAPM_SINGLE("SPK Switch", M98088_REG_4A_CFG_BYPASS, 0, 1, 0),
> };

If that's how they fit into the routing for the device, yes - you'd need
to define the bypass mixer as well and set up appropraite routes.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] ASoC: max98088: add support for reg_4a_cfg_bypass reg

[Tommaso Merciai]

On Thu, May 12, 2022 at 11:53:07AM +0100, Mark Brown wrote:
> On Thu, May 12, 2022 at 12:46:42PM +0200, Tommaso Merciai wrote:
> > On Thu, May 12, 2022 at 11:12:02AM +0100, Mark Brown wrote:
> 
> > > These look like they should be DAPM controls since they're controlling
> > > audio routing but they're being added as regular controls.
> 
> > Sorry again. You suggest to create a new structure for these entries,
> > for example:
> 
> > /* Out Bypass mixer switch */
> > static const struct snd_kcontrol_new max98088_out_bypass_mixer_controls[] = {
> >        SOC_DAPM_SINGLE("INA Switch", M98088_REG_4A_CFG_BYPASS, 7, 1, 0),
> >        SOC_DAPM_SINGLE("MIC2 Switch", M98088_REG_4A_CFG_BYPASS, 4, 1, 0),
> >        SOC_DAPM_SINGLE("REC Switch", M98088_REG_4A_CFG_BYPASS, 1, 1, 0),
> >        SOC_DAPM_SINGLE("SPK Switch", M98088_REG_4A_CFG_BYPASS, 0, 1, 0),
> > };
> 
> If that's how they fit into the routing for the device, yes - you'd need
> to define the bypass mixer as well and set up appropraite routes.

Hi,
I added this reg as regular controls because this reg is pretty generic
as you can see this controll bypass of some output, not all. 
What do you think about?

Thanks,
Tommaso
-- 
Tommaso Merciai
Embedded Linux Engineer
tommaso.merciai@amarulasolutions.com
__________________________________

Amarula Solutions SRL
Via Le Canevare 30, 31100 Treviso, Veneto, IT
T. +39 042 243 5310
info@amarulasolutions.com
www.amarulasolutions.com

Re: [PATCH] ASoC: max98088: add support for reg_4a_cfg_bypass reg

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 912 bytes --]

On Thu, May 12, 2022 at 01:09:59PM +0200, Tommaso Merciai wrote:
> On Thu, May 12, 2022 at 11:53:07AM +0100, Mark Brown wrote:
> > On Thu, May 12, 2022 at 12:46:42PM +0200, Tommaso Merciai wrote:
> > > On Thu, May 12, 2022 at 11:12:02AM +0100, Mark Brown wrote:

> > > > These look like they should be DAPM controls since they're controlling
> > > > audio routing but they're being added as regular controls.

> > > Sorry again. You suggest to create a new structure for these entries,
> > > for example:

> > If that's how they fit into the routing for the device, yes - you'd need
> > to define the bypass mixer as well and set up appropraite routes.

> I added this reg as regular controls because this reg is pretty generic
> as you can see this controll bypass of some output, not all. 
> What do you think about?

That sounds exactly like a DAPM control, please make them DAPM controls.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] ASoC: max98088: add support for reg_4a_cfg_bypass reg

[Tommaso Merciai]

On Thu, May 12, 2022 at 12:19:03PM +0100, Mark Brown wrote:
> On Thu, May 12, 2022 at 01:09:59PM +0200, Tommaso Merciai wrote:
> > On Thu, May 12, 2022 at 11:53:07AM +0100, Mark Brown wrote:
> > > On Thu, May 12, 2022 at 12:46:42PM +0200, Tommaso Merciai wrote:
> > > > On Thu, May 12, 2022 at 11:12:02AM +0100, Mark Brown wrote:
> 
> > > > > These look like they should be DAPM controls since they're controlling
> > > > > audio routing but they're being added as regular controls.
> 
> > > > Sorry again. You suggest to create a new structure for these entries,
> > > > for example:
> 
> > > If that's how they fit into the routing for the device, yes - you'd need
> > > to define the bypass mixer as well and set up appropraite routes.
> 
> > I added this reg as regular controls because this reg is pretty generic
> > as you can see this controll bypass of some output, not all. 
> > What do you think about?
> 
> That sounds exactly like a DAPM control, please make them DAPM controls.

Hi Mark,
Perfect, thanks for your suggestion.
I'll do it in V2.

Tommaso

-- 
Tommaso Merciai
Embedded Linux Engineer
tommaso.merciai@amarulasolutions.com
__________________________________

Amarula Solutions SRL
Via Le Canevare 30, 31100 Treviso, Veneto, IT
T. +39 042 243 5310
info@amarulasolutions.com
www.amarulasolutions.com

Re: [PATCH] ASoC: max98088: add support for reg_4a_cfg_bypass reg

[Tommaso Merciai]

On Thu, May 12, 2022 at 12:19:03PM +0100, Mark Brown wrote:
> On Thu, May 12, 2022 at 01:09:59PM +0200, Tommaso Merciai wrote:
> > On Thu, May 12, 2022 at 11:53:07AM +0100, Mark Brown wrote:
> > > On Thu, May 12, 2022 at 12:46:42PM +0200, Tommaso Merciai wrote:
> > > > On Thu, May 12, 2022 at 11:12:02AM +0100, Mark Brown wrote:
> 
> > > > > These look like they should be DAPM controls since they're controlling
> > > > > audio routing but they're being added as regular controls.
> 
> > > > Sorry again. You suggest to create a new structure for these entries,
> > > > for example:
> 
> > > If that's how they fit into the routing for the device, yes - you'd need
> > > to define the bypass mixer as well and set up appropraite routes.
> 
> > I added this reg as regular controls because this reg is pretty generic
> > as you can see this controll bypass of some output, not all. 
> > What do you think about?
> 
> That sounds exactly like a DAPM control, please make them DAPM controls.

Hi Mark,
Sorry again, but I'm quite new on alsa subsystem. I need an help on figuring out
on how to implements your solution. From what you suggest I got that I need to create
a bypass mixer for every switch (4 -> SPK, REC, MIC2, INA):

/* Out Mixer SPK */
static const struct snd_kcontrol_new max98088_output_bypass_spk_mixer_controls[] = {
       SOC_DAPM_SINGLE("SPK Bypass Switch", M98088_REG_4A_CFG_BYPASS, 0, 1, 0),
};

/* Out Mixer REC */
static const struct snd_kcontrol_new max98088_output_bypass_rec_mixer_controls[] = {
       SOC_DAPM_SINGLE("REC Bypass Switch", M98088_REG_4A_CFG_BYPASS, 1, 1, 0),
};

/* Out Mixer MIC */
static const struct snd_kcontrol_new max98088_output_bypass_mic_mixer_controls[] = {
       SOC_DAPM_SINGLE("MIC2 Bypass Switch", M98088_REG_4A_CFG_BYPASS, 4, 1, 0),
};

/* Out Mixer INA */
static const struct snd_kcontrol_new max98088_output_bypass_ina_mixer_controls[] = {
       SOC_DAPM_SINGLE("INA Bypass Switch", M98088_REG_4A_CFG_BYPASS, 7, 1, 0),
};

After that, I need to route the new control mixers on the switch:

 {"Out Mixer SPK", "SPK Bypass Switch", "RECN"},
 {"Out Mixer REC", "REC Bypass Switch", "RECP"},
 {"Out Mixer MIC", "MIC2 Bypass Switch", "MIC1"},
 {"Out Mixer INA", "INA Bypass Switch", "INA"},

Then route the bypass switch to the new output:

 {"SPKL", NULL, "SPK Bypass Switch"},
 {"RECN", NULL, "REC Bypass Switch"},
 {"MIC2", NULL, "MIC2 Bypass Switch"},
 {"MIC1", NULL, "INA Bypass Switch"},

I'm in the right way? What do you think about?
Can you point me a similar bypass switch into the kernel to take as reference?
Thanks in advance

Regards,
Tommmaso




-- 
Tommaso Merciai
Embedded Linux Engineer
tommaso.merciai@amarulasolutions.com
__________________________________

Amarula Solutions SRL
Via Le Canevare 30, 31100 Treviso, Veneto, IT
T. +39 042 243 5310
info@amarulasolutions.com
www.amarulasolutions.com

Re: [PATCH] ASoC: max98088: add support for reg_4a_cfg_bypass reg

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1718 bytes --]

On Fri, May 13, 2022 at 05:20:55PM +0200, Tommaso Merciai wrote:
> On Thu, May 12, 2022 at 12:19:03PM +0100, Mark Brown wrote:

> > That sounds exactly like a DAPM control, please make them DAPM controls.

> Sorry again, but I'm quite new on alsa subsystem. I need an help on figuring out
> on how to implements your solution. From what you suggest I got that I need to create
> a bypass mixer for every switch (4 -> SPK, REC, MIC2, INA):

It depends how the audio is routed - that would be a fairly unusual
structure for hardware but it's possible.  Often bypass paths feed into
mixers that have other, non-bypass paths.

> After that, I need to route the new control mixers on the switch:

>  {"Out Mixer SPK", "SPK Bypass Switch", "RECN"},
>  {"Out Mixer REC", "REC Bypass Switch", "RECP"},
>  {"Out Mixer MIC", "MIC2 Bypass Switch", "MIC1"},
>  {"Out Mixer INA", "INA Bypass Switch", "INA"},

> Then route the bypass switch to the new output:
> 
>  {"SPKL", NULL, "SPK Bypass Switch"},
>  {"RECN", NULL, "REC Bypass Switch"},
>  {"MIC2", NULL, "MIC2 Bypass Switch"},
>  {"MIC1", NULL, "INA Bypass Switch"},

> I'm in the right way? What do you think about?

That's pretty much it if they're a bunch of separate things.

> Can you point me a similar bypass switch into the kernel to take as reference?

If you search for "Bypass" in sound/soc/codecs you'll see a bunch of
examples - a lot of the Wolfson devices have bypass paths for example.
You'll see that for example with wm9713 the bypass paths go into mixers
that have other inputs rather than being totally separate things - I see
that this device has things like "Right SPK Mixer" which look like they
might fit here.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]