Re: [PATCH 4/5] ASoC: dapm: Add support for autodisable mux controls

[Charles Keepax <ckeepax@opensource.wolfsonmicro.com>]

On Thu, Apr 30, 2015 at 02:44:40PM +0200, Lars-Peter Clausen wrote:
> On 04/30/2015 12:38 PM, Charles Keepax wrote:
>> Commit 57295073b6ac ("ASoC: dapm: Implement mixer input auto-disable")
>> added support for autodisable controls, controls whose values are only
>> written to the hardware when their respective widgets are powered up.
>> But it only added support for controls based on the mixer abstraction.
>>
>> This patch add support for mux controls (DAPM controls based on the
>> enum abstraction) to be auto-disabled as well. As each mux can only have
>> a single control, there is no need to tie the autodisable widget to the
>> inputs (as is done for the mixer controls) it can be tided directly to
>> the mux widget itself.
>>
>> Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
>
> Looks pretty good.
>
> [...]
>> @@ -354,6 +355,34 @@ static int dapm_kcontrol_data_alloc(struct snd_soc_dapm_widget *widget,
>>   			}
>>   		}
>>   		break;
>> +	case snd_soc_dapm_mux:
>> +		e = (struct soc_enum *)kcontrol->private_value;
>> +
>> +		if (e->autodisable) {
>> +			struct snd_soc_dapm_widget template;
>> +
>> +			memset(&template, 0, sizeof(template));
>> +			template.reg = e->reg;
>> +			template.mask = e->mask << e->shift_l;
>> +			template.shift = e->shift_l;
>> +			template.off_val = 0;
>
> I've though about adding a auto-disable MUX type as well and the chip 
> were I wanted to use it has a non-zero power-off value. So I'd like to be 
> able to somehow specify this. We could for example put it at the end of 
> the values array. E.g.
>
> if (e->values)
> 	template.off_val = e->values[e->items].
> else
> 	template.off_val = 0;
>

Thats a good point something I hadn't thought of. It seems
reasonable that if the mux is autodisable that one of the states
in the values array should be the disabled state. Either the
first or last value would seem the reasonable choices. I would
lean towards the first value, as this is already the case in the
Arizona code :-).

I will do a respin with that included.

Thanks,
Charles