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

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

[Sebastian Reimers]

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-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 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

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [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-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

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-l3A5Bk7waGM@public.gmane.org>


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
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Daniel Mack]

On 07/04/2014 07:43 AM, Takashi Iwai wrote:
> 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-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> 
> 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-l3A5Bk7waGM@public.gmane.org>

Felipe, are you fine with this one?


Thanks,
Daniel

>> ---
>>  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
>>
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw@public.gmane.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 1216 bytes --]

On Tue, Jul 22, 2014 at 07:09:51PM +0200, Daniel Mack wrote:
> On 07/04/2014 07:43 AM, Takashi Iwai wrote:
> > 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-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> > 
> > 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-l3A5Bk7waGM@public.gmane.org>
> 
> Felipe, are you fine with this one?

seems like I missed this one, do you mind if we take it during v3.17-rc
and add Cc: stable ?

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

[Daniel Mack]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 07/22/2014 07:44 PM, Felipe Balbi wrote:
> On Tue, Jul 22, 2014 at 07:09:51PM +0200, Daniel Mack wrote:
>> On 07/04/2014 07:43 AM, Takashi Iwai wrote:
>>> 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-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>> 
>>> 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-l3A5Bk7waGM@public.gmane.org>
>> 
>> Felipe, are you fine with this one?
> 
> seems like I missed this one, do you mind if we take it during
> v3.17-rc and add Cc: stable ?

I guess that was what Sebastian intended :)

Thanks for taking care of it!


Daniel

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQIcBAEBAgAGBQJTzqckAAoJELphMRr8Y1QkzXYP/iWtHkczgDurT5s/6JNGJSv1
UsVTueRVepMLfVVFRW0mMiHHVfZVD/JoLMTAl0LkODBf95Vxgd0OR9WmHm1FVYdW
oJ80LTCkGEEjeZlZTcHJFIqfgwB6ryUsPoC+BAWvWpVSjvp2uHME8JmipYiNrwLS
fJNJSwL4lT5PFESQ27seonkDneAUk8FQObZCSOnqJsqqZpAWHmRQOPTQSyRsJ/fA
wOj/jo9gWJXqMtow2mfDteM/doiblBJaquKoQDIlj1UuEv4Ta97a1f/0N/O0q8kP
+4VqynEgCOhr8TNR8x7a6YTlzYQkonddFUGZuuJLLf5//Z5SA1zGdBl+bnZ3OPVV
7oVJZPakVaG/1ImkkPh+yUmPg6jRjaEbW6f2Y9p1t2iitnEGsi/doQpBS7xEf3+n
kHehgcKBUXCGzrJ25vxu9Q1/wUQXDXqsb3Q+PorcbhSF86OOe9pll9GvGjJbc7VY
zEitPPPKMmmLVUgyfFcsyz94NkZeAYMOCPZIBIZXtshoQGMvujUKB/qRbK5tO7x3
Y7cUnb5rSlfSuNCuRz4kKGIVK+ULr8XtdT6sSzm5kmNnqwYlxh2zt4XEPflIiRg9
4Z0MMHW4mRN0rUikfJofjsHObx/B6BvbFj7EBhyFipRZZW32b5jEn7kL5pOQRq1K
pANkTdVOnKGmgJ1VI6x8
=3cTI
-----END PGP SIGNATURE-----
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html