[alsa-devel] [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 at 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