Re: [PATCH] ASoC: qcom: audioreach: support multichannel PCM formats

[Prasad Kumpatla]

>   void audioreach_set_default_channel_mapping(u8 *ch_map, int num_channels)
>   {
> -	if (num_channels == 1) {
> -		ch_map[0] =  PCM_CHANNEL_FL;
> -	} else if (num_channels == 2) {
> -		ch_map[0] =  PCM_CHANNEL_FL;
> -		ch_map[1] =  PCM_CHANNEL_FR;
> -	} else if (num_channels == 4) {
> -		ch_map[0] =  PCM_CHANNEL_FL;
> -		ch_map[1] =  PCM_CHANNEL_FR;
> -		ch_map[2] =  PCM_CHANNEL_LS;
> -		ch_map[3] =  PCM_CHANNEL_RS;
> +	memset(ch_map, 0, AR_PCM_MAX_NUM_CHANNEL);
> +
> +	switch (num_channels) {
> +	case 1:
> +		ch_map[0] = PCM_CHANNEL_FL;
> +		break;
> +	case 2:
> +		ch_map[0] = PCM_CHANNEL_FL;
> +		ch_map[1] = PCM_CHANNEL_FR;
> +		break;
> +	case 3:
> +		ch_map[0] = PCM_CHANNEL_FL;
> +		ch_map[1] = PCM_CHANNEL_FR;
> +		ch_map[2] = PCM_CHANNEL_FC;
> +		break;
> +	case 4:
> +		ch_map[0] = PCM_CHANNEL_FL;
> +		ch_map[1] = PCM_CHANNEL_FR;
> +		ch_map[2] = PCM_CHANNEL_LS;
> +		ch_map[3] = PCM_CHANNEL_RS;
> +		break;
> +	case 5:
> +		ch_map[0] = PCM_CHANNEL_FL;
> +		ch_map[1] = PCM_CHANNEL_FR;
> +		ch_map[2] = PCM_CHANNEL_FC;
> +		ch_map[3] = PCM_CHANNEL_LS;
> +		ch_map[4] = PCM_CHANNEL_RS;
> +		break;
> +	case 6:
> +		ch_map[0] = PCM_CHANNEL_FL;
> +		ch_map[1] = PCM_CHANNEL_FR;
> +		ch_map[2] = PCM_CHANNEL_LFE;
> +		ch_map[3] = PCM_CHANNEL_FC;
> +		ch_map[4] = PCM_CHANNEL_LS;
> +		ch_map[5] = PCM_CHANNEL_RS;
> +		break;
> +	case 7:
> +		ch_map[0] = PCM_CHANNEL_FL;
> +		ch_map[1] = PCM_CHANNEL_FR;
> +		ch_map[2] = PCM_CHANNEL_LFE;
> +		ch_map[3] = PCM_CHANNEL_FC;
> +		ch_map[4] = PCM_CHANNEL_LS;
> +		ch_map[5] = PCM_CHANNEL_RS;
> +		ch_map[6] = PCM_CHANNEL_CS;
> +		break;
> +	case 8:
> +		ch_map[0] = PCM_CHANNEL_FL;
> +		ch_map[1] = PCM_CHANNEL_FR;
> +		ch_map[2] = PCM_CHANNEL_LFE;
> +		ch_map[3] = PCM_CHANNEL_FC;
> +		ch_map[4] = PCM_CHANNEL_LS;
> +		ch_map[5] = PCM_CHANNEL_RS;
> +		ch_map[6] = PCM_CHANNEL_LB;
> +		ch_map[7] = PCM_CHANNEL_RB;
> +		break;
>   	}
>   }
>   EXPORT_SYMBOL_GPL(audioreach_set_default_channel_mapping);

audioreach_set_default_channel_mapping() silently leaves an all-zero channel map for unsupported channel counts.
Since this helper is exported and callers such as q6apm_dai_prepare/q6dma_hw_params/q6hdmi_hw_params() don't validate the range.

would it make sense to return -EINVAL for unsupported values and have callers propagate the error?

Thanks,
Prasad

>