[Sound-open-firmware] [PATCH 2/5] Volume: Replace SOF_CTRL_CMD_MUTE/UNMUTE with SOF_CTRL_CMD_SWITCH

[Pierre-Louis Bossart]

On 10/20/17 12:15 PM, Seppo Ingalsuo wrote:
> Due to previous patch the volume mute is updated to be controlled with
> SOF_CTRL_CMD_SWITCH that maps to ALSA switch style control. Mute for a
> channel is set with an (unsigned) value larger than zero. Zero value
> unmutes the channel.

Mute and switch are not synonyms - the results are inverted.
ALSA switches set to 1 means enable audio (unmute), 0 means mute.
we've had this confusion in the past with inverted logic for the SST 
driver.
We should also treat switch as a boolean, it's zero or one so we can 
test with switch or !switch.

> 
> In both mute and volume set code the if statement to compare component
> channel map is changed to compare to chanv[j].channel instead of used
> chanv[j].value that looks incorrect.
> 
> Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo at linux.intel.com>
> ---
>   src/audio/volume.c | 29 ++++++++++++++---------------
>   1 file changed, 14 insertions(+), 15 deletions(-)
> 
> diff --git a/src/audio/volume.c b/src/audio/volume.c
> index 137be3c..28d3225 100644
> --- a/src/audio/volume.c
> +++ b/src/audio/volume.c
> @@ -449,36 +449,35 @@ static int volume_ctrl_set_cmd(struct comp_dev *dev, struct sof_ipc_ctrl_data *c
>   
>   	switch (cdata->cmd) {
>   	case SOF_CTRL_CMD_VOLUME:
> -
> +		trace_volume("vst");
>   		for (i = 0; i < SOF_IPC_MAX_CHANNELS; i++) {
>   			for (j = 0; j < cdata->num_elems; j++) {
> -				if (cdata->chanv[j].value == cd->chan[i])
> +				tracev_value(cdata->chanv[j].channel);
> +				tracev_value(cdata->chanv[j].value);
> +				if (cdata->chanv[j].channel == cd->chan[i])
>   					volume_set_chan(dev, i, cdata->chanv[j].value);
>   			}
>   		}
> -
>   		work_schedule_default(&cd->volwork, VOL_RAMP_US);
>   		break;
> -	case SOF_CTRL_CMD_MUTE:
>   
> +	case SOF_CTRL_CMD_SWITCH:
> +		trace_volume("mst");
>   		for (i = 0; i < SOF_IPC_MAX_CHANNELS; i++) {
>   			for (j = 0; j < cdata->num_elems; j++) {
> -				if (cdata->chanv[j].value == cd->chan[i])
> -					volume_set_chan_mute(dev, i);
> +				tracev_value(cdata->chanv[j].channel);
> +				tracev_value(cdata->chanv[j].value);
> +				if (cdata->chanv[j].channel == cd->chan[i]) {
> +					if (cdata->chanv[j].value > 0)
> +						volume_set_chan_mute(dev, i);
> +					else
> +						volume_set_chan_unmute(dev, i);
> +				}
>   			}
>   		}
>   		work_schedule_default(&cd->volwork, VOL_RAMP_US);
>   		break;
> -	case SOF_CTRL_CMD_UNMUTE:
>   
> -		for (i = 0; i < SOF_IPC_MAX_CHANNELS; i++) {
> -			for (j = 0; j < cdata->num_elems; j++) {
> -				if (cdata->chanv[j].value == cd->chan[i])
> -					volume_set_chan_unmute(dev, i);
> -			}
> -		}
> -		work_schedule_default(&cd->volwork, VOL_RAMP_US);
> -		break;
>   	default:
>   		trace_volume_error("gs1");
>   		return -EINVAL;
>