* [PATCH] ALSA: usb-audio: do not trust too-big wMaxPacketSize values
@ 2013-08-08 9:24 Clemens Ladisch
2013-08-08 9:37 ` Takashi Iwai
2013-08-08 18:07 ` Eldad Zack
0 siblings, 2 replies; 3+ messages in thread
From: Clemens Ladisch @ 2013-08-08 9:24 UTC (permalink / raw)
To: Takashi Iwai; +Cc: James Stone, alsa-devel
The driver used to assume that the streaming endpoint's wMaxPacketSize
value would be an indication of how much data the endpoint expects or
sends, and compute the number of packets per URB using this value.
However, the Focusrite Scarlett 2i4 declares a value of 1024 bytes,
while only about 88 or 44 bytes are be actually used. This discrepancy
would result in URBs with far too few packets, which would not work
correctly on the EHCI driver.
To get correct URBs, use wMaxPacketSize only as an upper limit on the
packet size.
Reported-by: James Stone <jamesmstone@gmail.com>
Tested-by: James Stone <jamesmstone@gmail.com>
Cc: <stable@vger.kernel.org> # 2.6.35+
Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
---
sound/usb/endpoint.c | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)
--- a/sound/usb/endpoint.c
+++ b/sound/usb/endpoint.c
@@ -591,17 +591,16 @@ static int data_ep_set_params(struct snd_usb_endpoint *ep,
ep->stride = frame_bits >> 3;
ep->silence_value = pcm_format == SNDRV_PCM_FORMAT_U8 ? 0x80 : 0;
- /* calculate max. frequency */
- if (ep->maxpacksize) {
+ /* assume max. frequency is 25% higher than nominal */
+ ep->freqmax = ep->freqn + (ep->freqn >> 2);
+ maxsize = ((ep->freqmax + 0xffff) * (frame_bits >> 3))
+ >> (16 - ep->datainterval);
+ /* but wMaxPacketSize might reduce this */
+ if (ep->maxpacksize && ep->maxpacksize < maxsize) {
/* whatever fits into a max. size packet */
maxsize = ep->maxpacksize;
ep->freqmax = (maxsize / (frame_bits >> 3))
<< (16 - ep->datainterval);
- } else {
- /* no max. packet size: just take 25% higher than nominal */
- ep->freqmax = ep->freqn + (ep->freqn >> 2);
- maxsize = ((ep->freqmax + 0xffff) * (frame_bits >> 3))
- >> (16 - ep->datainterval);
}
if (ep->fill_max)
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] ALSA: usb-audio: do not trust too-big wMaxPacketSize values
2013-08-08 9:24 [PATCH] ALSA: usb-audio: do not trust too-big wMaxPacketSize values Clemens Ladisch
@ 2013-08-08 9:37 ` Takashi Iwai
2013-08-08 18:07 ` Eldad Zack
1 sibling, 0 replies; 3+ messages in thread
From: Takashi Iwai @ 2013-08-08 9:37 UTC (permalink / raw)
To: Clemens Ladisch; +Cc: James Stone, alsa-devel
At Thu, 08 Aug 2013 11:24:55 +0200,
Clemens Ladisch wrote:
>
> The driver used to assume that the streaming endpoint's wMaxPacketSize
> value would be an indication of how much data the endpoint expects or
> sends, and compute the number of packets per URB using this value.
>
> However, the Focusrite Scarlett 2i4 declares a value of 1024 bytes,
> while only about 88 or 44 bytes are be actually used. This discrepancy
> would result in URBs with far too few packets, which would not work
> correctly on the EHCI driver.
>
> To get correct URBs, use wMaxPacketSize only as an upper limit on the
> packet size.
>
> Reported-by: James Stone <jamesmstone@gmail.com>
> Tested-by: James Stone <jamesmstone@gmail.com>
> Cc: <stable@vger.kernel.org> # 2.6.35+
> Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
Thanks, applied.
Takashi
> ---
> sound/usb/endpoint.c | 13 ++++++-------
> 1 file changed, 6 insertions(+), 7 deletions(-)
>
> --- a/sound/usb/endpoint.c
> +++ b/sound/usb/endpoint.c
> @@ -591,17 +591,16 @@ static int data_ep_set_params(struct snd_usb_endpoint *ep,
> ep->stride = frame_bits >> 3;
> ep->silence_value = pcm_format == SNDRV_PCM_FORMAT_U8 ? 0x80 : 0;
>
> - /* calculate max. frequency */
> - if (ep->maxpacksize) {
> + /* assume max. frequency is 25% higher than nominal */
> + ep->freqmax = ep->freqn + (ep->freqn >> 2);
> + maxsize = ((ep->freqmax + 0xffff) * (frame_bits >> 3))
> + >> (16 - ep->datainterval);
> + /* but wMaxPacketSize might reduce this */
> + if (ep->maxpacksize && ep->maxpacksize < maxsize) {
> /* whatever fits into a max. size packet */
> maxsize = ep->maxpacksize;
> ep->freqmax = (maxsize / (frame_bits >> 3))
> << (16 - ep->datainterval);
> - } else {
> - /* no max. packet size: just take 25% higher than nominal */
> - ep->freqmax = ep->freqn + (ep->freqn >> 2);
> - maxsize = ((ep->freqmax + 0xffff) * (frame_bits >> 3))
> - >> (16 - ep->datainterval);
> }
>
> if (ep->fill_max)
>
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] ALSA: usb-audio: do not trust too-big wMaxPacketSize values
2013-08-08 9:24 [PATCH] ALSA: usb-audio: do not trust too-big wMaxPacketSize values Clemens Ladisch
2013-08-08 9:37 ` Takashi Iwai
@ 2013-08-08 18:07 ` Eldad Zack
1 sibling, 0 replies; 3+ messages in thread
From: Eldad Zack @ 2013-08-08 18:07 UTC (permalink / raw)
To: Clemens Ladisch; +Cc: Takashi Iwai, James Stone, alsa-devel
On Thu, 8 Aug 2013, Clemens Ladisch wrote:
> The driver used to assume that the streaming endpoint's wMaxPacketSize
> value would be an indication of how much data the endpoint expects or
> sends, and compute the number of packets per URB using this value.
>
> However, the Focusrite Scarlett 2i4 declares a value of 1024 bytes,
> while only about 88 or 44 bytes are be actually used. This discrepancy
> would result in URBs with far too few packets, which would not work
> correctly on the EHCI driver.
>
> To get correct URBs, use wMaxPacketSize only as an upper limit on the
> packet size.
Works good on here too (M-Audio C400) - and maxsize does gets smaller
for 48/44.1KHz.
>
> Reported-by: James Stone <jamesmstone@gmail.com>
> Tested-by: James Stone <jamesmstone@gmail.com>
> Cc: <stable@vger.kernel.org> # 2.6.35+
> Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
> ---
> sound/usb/endpoint.c | 13 ++++++-------
> 1 file changed, 6 insertions(+), 7 deletions(-)
>
> --- a/sound/usb/endpoint.c
> +++ b/sound/usb/endpoint.c
> @@ -591,17 +591,16 @@ static int data_ep_set_params(struct snd_usb_endpoint *ep,
> ep->stride = frame_bits >> 3;
> ep->silence_value = pcm_format == SNDRV_PCM_FORMAT_U8 ? 0x80 : 0;
>
> - /* calculate max. frequency */
> - if (ep->maxpacksize) {
> + /* assume max. frequency is 25% higher than nominal */
> + ep->freqmax = ep->freqn + (ep->freqn >> 2);
> + maxsize = ((ep->freqmax + 0xffff) * (frame_bits >> 3))
> + >> (16 - ep->datainterval);
> + /* but wMaxPacketSize might reduce this */
> + if (ep->maxpacksize && ep->maxpacksize < maxsize) {
> /* whatever fits into a max. size packet */
> maxsize = ep->maxpacksize;
> ep->freqmax = (maxsize / (frame_bits >> 3))
> << (16 - ep->datainterval);
> - } else {
> - /* no max. packet size: just take 25% higher than nominal */
> - ep->freqmax = ep->freqn + (ep->freqn >> 2);
> - maxsize = ((ep->freqmax + 0xffff) * (frame_bits >> 3))
> - >> (16 - ep->datainterval);
> }
>
> if (ep->fill_max)
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2013-08-08 18:08 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-08-08 9:24 [PATCH] ALSA: usb-audio: do not trust too-big wMaxPacketSize values Clemens Ladisch
2013-08-08 9:37 ` Takashi Iwai
2013-08-08 18:07 ` Eldad Zack
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.