[alsa-devel] [PATCH 1/1] usb: gadget: f_uac2: Fix pcm sample size selection

[Takashi Iwai]

At Thu,  3 Jul 2014 20:15:28 +0200,
Sebastian Reimers wrote:
> 
> The pcm playback and capture sample size format was fixed
> SNDRV_PCM_FMTBIT_S16_LE.
> 
> This patch respects also 16, 24 and 32 bit p_ssize and c_ssize values.
> 
> Signed-off-by: Sebastian Reimers <sebastian.reimers at gmail.com>

In general, this driver should check more on p_ssize.  This can be any
integer value passed as a module parameter.  It should be cut to a
sane value or give an error in the probe function when an invalid
value is given.  Ditto for p_srate and other parameters.

Also, I don't understand the logic of the code
	runtime->hw.period_bytes_min = 2 * uac2->p_prm.max_psize
					/ runtime->hw.periods_min;

But this is mostly irrelevant with this patch, so for this patch:
   Reviewed-by: Takashi Iwai <tiwai at suse.de>


thanks,

Takashi


> ---
>  drivers/usb/gadget/f_uac2.c | 24 ++++++++++++++++++++++--
>  1 file changed, 22 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/gadget/f_uac2.c b/drivers/usb/gadget/f_uac2.c
> index 6261db4a..3ed89ec 100644
> --- a/drivers/usb/gadget/f_uac2.c
> +++ b/drivers/usb/gadget/f_uac2.c
> @@ -348,14 +348,34 @@ static int uac2_pcm_open(struct snd_pcm_substream *substream)
>  	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
>  		spin_lock_init(&uac2->p_prm.lock);
>  		runtime->hw.rate_min = p_srate;
> -		runtime->hw.formats = SNDRV_PCM_FMTBIT_S16_LE; /* ! p_ssize ! */
> +		switch (p_ssize) {
> +		case 3:
> +			runtime->hw.formats = SNDRV_PCM_FMTBIT_S24_3LE;
> +			break;
> +		case 4:
> +			runtime->hw.formats = SNDRV_PCM_FMTBIT_S32_LE;
> +			break;
> +		default:
> +			runtime->hw.formats = SNDRV_PCM_FMTBIT_S16_LE;
> +			break;
> +		}
>  		runtime->hw.channels_min = num_channels(p_chmask);
>  		runtime->hw.period_bytes_min = 2 * uac2->p_prm.max_psize
>  						/ runtime->hw.periods_min;
>  	} else {
>  		spin_lock_init(&uac2->c_prm.lock);
>  		runtime->hw.rate_min = c_srate;
> -		runtime->hw.formats = SNDRV_PCM_FMTBIT_S16_LE; /* ! c_ssize ! */
> +		switch (c_ssize) {
> +		case 3:
> +			runtime->hw.formats = SNDRV_PCM_FMTBIT_S24_3LE;
> +			break;
> +		case 4:
> +			runtime->hw.formats = SNDRV_PCM_FMTBIT_S32_LE;
> +			break;
> +		default:
> +			runtime->hw.formats = SNDRV_PCM_FMTBIT_S16_LE;
> +			break;
> +		}
>  		runtime->hw.channels_min = num_channels(c_chmask);
>  		runtime->hw.period_bytes_min = 2 * uac2->c_prm.max_psize
>  						/ runtime->hw.periods_min;
> -- 
> 2.0.1
>