[PATCH] ASoC: Add external amplifier controls for Visstrim_M10.

[PATCH] ASoC: Add external amplifier controls for Visstrim_M10.

[Javier Martin]

Visstrim_M10 has an external class D amplifier.
This patch provides support for controlling the 4
possible gain levels and per channel muting.

Signed-off-by: Javier Martin <javier.martin@vista-silicon.com>
---
 sound/soc/imx/mx27vis-aic32x4.c |   72 +++++++++++++++++++++++++++++++++++++++
 1 files changed, 72 insertions(+), 0 deletions(-)

diff --git a/sound/soc/imx/mx27vis-aic32x4.c b/sound/soc/imx/mx27vis-aic32x4.c
index d37e23c..47a7bf9 100644
--- a/sound/soc/imx/mx27vis-aic32x4.c
+++ b/sound/soc/imx/mx27vis-aic32x4.c
@@ -25,16 +25,36 @@
 #include <linux/moduleparam.h>
 #include <linux/device.h>
 #include <linux/i2c.h>
+#include <linux/gpio.h>
 #include <sound/core.h>
 #include <sound/pcm.h>
 #include <sound/soc.h>
 #include <sound/soc-dapm.h>
 #include <asm/mach-types.h>
 #include <mach/audmux.h>
+#include <mach/iomux-mx27.h>
 
 #include "../codecs/tlv320aic32x4.h"
 #include "imx-ssi.h"
 
+#define MX27VIS_AMP_GAIN	0
+#define MX27VIS_AMP_MUTE	1
+
+#define MX27VIS_PIN_G0		(GPIO_PORTF + 9)
+#define MX27VIS_PIN_G1		(GPIO_PORTF + 8)
+#define MX27VIS_PIN_SDL		(GPIO_PORTE + 5)
+#define MX27VIS_PIN_SDR		(GPIO_PORTF + 7)
+
+static int mx27vis_amp_gain;
+static int mx27vis_amp_mute;
+
+static const int mx27vis_amp_pins[] = {
+	MX27VIS_PIN_G0 | GPIO_GPIO | GPIO_OUT,
+	MX27VIS_PIN_G1 | GPIO_GPIO | GPIO_OUT,
+	MX27VIS_PIN_SDL | GPIO_GPIO | GPIO_OUT,
+	MX27VIS_PIN_SDR | GPIO_GPIO | GPIO_OUT,
+};
+
 static int mx27vis_aic32x4_hw_params(struct snd_pcm_substream *substream,
 			    struct snd_pcm_hw_params *params)
 {
@@ -74,8 +94,53 @@ static struct snd_soc_ops mx27vis_aic32x4_snd_ops = {
 	.hw_params	= mx27vis_aic32x4_hw_params,
 };
 
+static int mx27vis_amp_set(struct snd_kcontrol *kcontrol,
+			    struct snd_ctl_elem_value *ucontrol)
+{
+	struct soc_mixer_control *mc =
+		(struct soc_mixer_control *)kcontrol->private_value;
+	int value = ucontrol->value.integer.value[0];
+	unsigned int reg = mc->reg;
+
+	switch (reg) {
+	case MX27VIS_AMP_GAIN:
+		gpio_set_value(MX27VIS_PIN_G0, value & 1);
+		gpio_set_value(MX27VIS_PIN_G1, value >> 1);
+		mx27vis_amp_gain = value;
+		break;
+	case MX27VIS_AMP_MUTE:
+		gpio_set_value(MX27VIS_PIN_SDL, value & 1);
+		gpio_set_value(MX27VIS_PIN_SDR, value >> 1);
+		mx27vis_amp_mute = value;
+		break;
+	}
+	return 0;
+}
+
+static int mx27vis_amp_get(struct snd_kcontrol *kcontrol,
+			    struct snd_ctl_elem_value *ucontrol)
+{
+	struct soc_mixer_control *mc =
+		(struct soc_mixer_control *)kcontrol->private_value;
+	unsigned int reg = mc->reg;
+
+	switch (reg) {
+	case MX27VIS_AMP_GAIN:
+		ucontrol->value.integer.value[0] = mx27vis_amp_gain;
+		break;
+	case MX27VIS_AMP_MUTE:
+		ucontrol->value.integer.value[0] = mx27vis_amp_mute;
+		break;
+	}
+	return 0;
+}
+
 static const struct snd_kcontrol_new mx27vis_aic32x4_controls[] = {
 	SOC_DAPM_PIN_SWITCH("External Mic"),
+	SOC_SINGLE_EXT("LO Ext Boost", MX27VIS_AMP_GAIN, 0, 3, 0,
+		       mx27vis_amp_get, mx27vis_amp_set),
+	SOC_DOUBLE_EXT("LO Ext Mute Switch", MX27VIS_AMP_MUTE, 0, 1, 1, 0,
+		       mx27vis_amp_get, mx27vis_amp_set),
 };
 
 static const struct snd_soc_dapm_widget aic32x4_dapm_widgets[] = {
@@ -146,6 +211,13 @@ static int __init mx27vis_aic32x4_init(void)
 			MXC_AUDMUX_V1_PCR_RXDSEL(MX27_AUDMUX_HPCR1_SSI0)
 	);
 
+	ret = mxc_gpio_setup_multiple_pins(mx27vis_amp_pins,
+			ARRAY_SIZE(mx27vis_amp_pins), "MX27VIS_AMP");
+	if (ret) {
+		printk(KERN_ERR "ASoC: unable to setup gpios\n");
+		platform_device_put(mx27vis_aic32x4_snd_device);
+	}
+
 	return ret;
 }
 
-- 
1.7.0.4

Re: [PATCH] ASoC: Add external amplifier controls for Visstrim_M10.

[Mark Brown]

On Thu, Jan 19, 2012 at 03:53:27PM +0100, Javier Martin wrote:
> Visstrim_M10 has an external class D amplifier.
> This patch provides support for controlling the 4
> possible gain levels and per channel muting.

It does seem like there could usefully be a generic driver but it's so
little code...

> +	switch (reg) {
> +	case MX27VIS_AMP_GAIN:
> +		gpio_set_value(MX27VIS_PIN_G0, value & 1);
> +		gpio_set_value(MX27VIS_PIN_G1, value >> 1);
> +		mx27vis_amp_gain = value;
> +		break;
> +	case MX27VIS_AMP_MUTE:
> +		gpio_set_value(MX27VIS_PIN_SDL, value & 1);
> +		gpio_set_value(MX27VIS_PIN_SDR, value >> 1);
> +		mx27vis_amp_mute = value;
> +		break;

These should validate that value is in range.

> +	ret = mxc_gpio_setup_multiple_pins(mx27vis_amp_pins,
> +			ARRAY_SIZE(mx27vis_amp_pins), "MX27VIS_AMP");
> +	if (ret) {
> +		printk(KERN_ERR "ASoC: unable to setup gpios\n");
> +		platform_device_put(mx27vis_aic32x4_snd_device);
> +	}

This looks awfully like it should be arch/arm code except it looks like
it also munges in the gpio_request.  Odd.

Re: [PATCH] ASoC: Add external amplifier controls for Visstrim_M10.

[javier Martin]

Hi Mark,
thank you for your review.

On 19 January 2012 16:45, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Thu, Jan 19, 2012 at 03:53:27PM +0100, Javier Martin wrote:
>> Visstrim_M10 has an external class D amplifier.
>> This patch provides support for controlling the 4
>> possible gain levels and per channel muting.
>
> It does seem like there could usefully be a generic driver but it's so
> little code...
>
>> +     switch (reg) {
>> +     case MX27VIS_AMP_GAIN:
>> +             gpio_set_value(MX27VIS_PIN_G0, value & 1);
>> +             gpio_set_value(MX27VIS_PIN_G1, value >> 1);
>> +             mx27vis_amp_gain = value;
>> +             break;
>> +     case MX27VIS_AMP_MUTE:
>> +             gpio_set_value(MX27VIS_PIN_SDL, value & 1);
>> +             gpio_set_value(MX27VIS_PIN_SDR, value >> 1);
>> +             mx27vis_amp_mute = value;
>> +             break;
>
> These should validate that value is in range.

Sure. Sorry.

>> +     ret = mxc_gpio_setup_multiple_pins(mx27vis_amp_pins,
>> +                     ARRAY_SIZE(mx27vis_amp_pins), "MX27VIS_AMP");
>> +     if (ret) {
>> +             printk(KERN_ERR "ASoC: unable to setup gpios\n");
>> +             platform_device_put(mx27vis_aic32x4_snd_device);
>> +     }
>
> This looks awfully like it should be arch/arm code except it looks like
> it also munges in the gpio_request.  Odd.

This is a doubt I had. I could also use board specific file to request
these gpios and then use some platform data to tell the driver what
GPIOS must be used.
However, as those gpios are fixed for all Visstrim_M10 boards I
considered it didn't worth it.

On the other hand, if you think it's cleaner that way I can do that.

Regards.
-- 
Javier Martin
Vista Silicon S.L.
CDTUC - FASE C - Oficina S-345
Avda de los Castros s/n
39005- Santander. Cantabria. Spain
+34 942 25 32 60
www.vista-silicon.com

Re: [PATCH] ASoC: Add external amplifier controls for Visstrim_M10.

[Lars-Peter Clausen]

On 01/19/2012 03:53 PM, Javier Martin wrote:
> Visstrim_M10 has an external class D amplifier.
> This patch provides support for controlling the 4
> possible gain levels and per channel muting.
> 
> Signed-off-by: Javier Martin <javier.martin@vista-silicon.com>
> ---
>  sound/soc/imx/mx27vis-aic32x4.c |   72 +++++++++++++++++++++++++++++++++++++++
>  1 files changed, 72 insertions(+), 0 deletions(-)
> 
> diff --git a/sound/soc/imx/mx27vis-aic32x4.c b/sound/soc/imx/mx27vis-aic32x4.c
> index d37e23c..47a7bf9 100644
> --- a/sound/soc/imx/mx27vis-aic32x4.c
> +++ b/sound/soc/imx/mx27vis-aic32x4.c
> @@ -25,16 +25,36 @@
> [...]
> +
>  static const struct snd_kcontrol_new mx27vis_aic32x4_controls[] = {
>  	SOC_DAPM_PIN_SWITCH("External Mic"),
> +	SOC_SINGLE_EXT("LO Ext Boost", MX27VIS_AMP_GAIN, 0, 3, 0,
> +		       mx27vis_amp_get, mx27vis_amp_set),

Do you know the dB gain of the amplifier? If yes you should provide it using
SOC_SINGLE_EXT_TLV.

> [...]
>  };
>  
> [...]

Re: [PATCH] ASoC: Add external amplifier controls for Visstrim_M10.

[Mark Brown]

On Thu, Jan 19, 2012 at 05:37:46PM +0100, javier Martin wrote:
> On 19 January 2012 16:45, Mark Brown

> >> +     ret = mxc_gpio_setup_multiple_pins(mx27vis_amp_pins,
> >> +                     ARRAY_SIZE(mx27vis_amp_pins), "MX27VIS_AMP");
> >> +     if (ret) {
> >> +             printk(KERN_ERR "ASoC: unable to setup gpios\n");
> >> +             platform_device_put(mx27vis_aic32x4_snd_device);
> >> +     }

> > This looks awfully like it should be arch/arm code except it looks like
> > it also munges in the gpio_request.  Odd.

> This is a doubt I had. I could also use board specific file to request
> these gpios and then use some platform data to tell the driver what
> GPIOS must be used.
> However, as those gpios are fixed for all Visstrim_M10 boards I
> considered it didn't worth it.

> On the other hand, if you think it's cleaner that way I can do that.

I think this is just the arch being non-standard.