[alsa-devel] [PATCH v2] ALSA: snd-usb-usx2y: remove bogus frame checks
The frame check in i_usX2Y_urb_complete() and i_usX2Y_usbpcm_urb_complete() is bogus and produces false positives as described in this LAU thread:
http://linuxaudio.org/mailarchive/lau/2013/5/20/200177
This patch removes the check code entirely.
Cc: fzu@wemgehoertderstaat.de Reported-by: Dr Nicholas J Bailey nicholas.bailey@glasgow.ac.uk Suggested-by: Takashi Iwai tiwai@suse.de Signed-off-by: Daniel Mack zonque@gmail.com --- sound/usb/usx2y/usbusx2yaudio.c | 22 +++------------------- sound/usb/usx2y/usx2yhwdeppcm.c | 7 +------ 2 files changed, 4 insertions(+), 25 deletions(-)
diff --git a/sound/usb/usx2y/usbusx2yaudio.c b/sound/usb/usx2y/usbusx2yaudio.c index 63fb521..6234a51 100644 --- a/sound/usb/usx2y/usbusx2yaudio.c +++ b/sound/usb/usx2y/usbusx2yaudio.c @@ -299,19 +299,6 @@ static void usX2Y_error_urb_status(struct usX2Ydev *usX2Y, usX2Y_clients_stop(usX2Y); }
-static void usX2Y_error_sequence(struct usX2Ydev *usX2Y, - struct snd_usX2Y_substream *subs, struct urb *urb) -{ - snd_printk(KERN_ERR -"Sequence Error!(hcd_frame=%i ep=%i%s;wait=%i,frame=%i).\n" -"Most probably some urb of usb-frame %i is still missing.\n" -"Cause could be too long delays in usb-hcd interrupt handling.\n", - usb_get_current_frame_number(usX2Y->dev), - subs->endpoint, usb_pipein(urb->pipe) ? "in" : "out", - usX2Y->wait_iso_frame, urb->start_frame, usX2Y->wait_iso_frame); - usX2Y_clients_stop(usX2Y); -} - static void i_usX2Y_urb_complete(struct urb *urb) { struct snd_usX2Y_substream *subs = urb->context; @@ -328,12 +315,9 @@ static void i_usX2Y_urb_complete(struct urb *urb) usX2Y_error_urb_status(usX2Y, subs, urb); return; } - if (likely((urb->start_frame & 0xFFFF) == (usX2Y->wait_iso_frame & 0xFFFF))) - subs->completed_urb = urb; - else { - usX2Y_error_sequence(usX2Y, subs, urb); - return; - } + + subs->completed_urb = urb; + { struct snd_usX2Y_substream *capsubs = usX2Y->subs[SNDRV_PCM_STREAM_CAPTURE], *playbacksubs = usX2Y->subs[SNDRV_PCM_STREAM_PLAYBACK]; diff --git a/sound/usb/usx2y/usx2yhwdeppcm.c b/sound/usb/usx2y/usx2yhwdeppcm.c index f2a1acd..814d0e8 100644 --- a/sound/usb/usx2y/usx2yhwdeppcm.c +++ b/sound/usb/usx2y/usx2yhwdeppcm.c @@ -244,13 +244,8 @@ static void i_usX2Y_usbpcm_urb_complete(struct urb *urb) usX2Y_error_urb_status(usX2Y, subs, urb); return; } - if (likely((urb->start_frame & 0xFFFF) == (usX2Y->wait_iso_frame & 0xFFFF))) - subs->completed_urb = urb; - else { - usX2Y_error_sequence(usX2Y, subs, urb); - return; - }
+ subs->completed_urb = urb; capsubs = usX2Y->subs[SNDRV_PCM_STREAM_CAPTURE]; capsubs2 = usX2Y->subs[SNDRV_PCM_STREAM_CAPTURE + 2]; playbacksubs = usX2Y->subs[SNDRV_PCM_STREAM_PLAYBACK];
I applied the patch by hand to my current source tree (3.10.11) and compiled the kernel from scratch.
I plugged a MIDI cable between midi in and out on the TASCAM US-122. Using QJackCtl, I connected the US-122's MIDI out to fluidsynth, and used amidiplay to send play a MIDI file to the US-122.
While the MIDI was playing, I used Audacity to record the audio via the US-122's mic inputs holding the mic close to the speakers. After about 10 seconds, I rewound audacity and hit record again so that it was both playing and recording through the US-122 while the US-122 was also routing MIDI.
This appears to work fine (at 44100Hz sampling; I've not tried any others), and I reckon this is a pretty severe test.
Thank you for pushing this patch which seems to resolve the original issue. This will be particularly appreciated by many because the US-122, no longer supported for Windows I believe, is currently available on eBay for about £30 in the UK, and represents an inexpensive way for Linux users to make reasonable quality recordings from microphones needing a phantom power source.
On Wednesday 02 October 2013 16:49:50 Daniel Mack wrote:
The frame check in i_usX2Y_urb_complete() and i_usX2Y_usbpcm_urb_complete() is bogus and produces false positives as described in this LAU thread:
http://linuxaudio.org/mailarchive/lau/2013/5/20/200177
This patch removes the check code entirely.
Cc: fzu@wemgehoertderstaat.de Reported-by: Dr Nicholas J Bailey nicholas.bailey@glasgow.ac.uk Suggested-by: Takashi Iwai tiwai@suse.de Signed-off-by: Daniel Mack zonque@gmail.com
sound/usb/usx2y/usbusx2yaudio.c | 22 +++------------------- sound/usb/usx2y/usx2yhwdeppcm.c | 7 +------ 2 files changed, 4 insertions(+), 25 deletions(-)
diff --git a/sound/usb/usx2y/usbusx2yaudio.c b/sound/usb/usx2y/usbusx2yaudio.c index 63fb521..6234a51 100644 --- a/sound/usb/usx2y/usbusx2yaudio.c +++ b/sound/usb/usx2y/usbusx2yaudio.c @@ -299,19 +299,6 @@ static void usX2Y_error_urb_status(struct usX2Ydev *usX2Y, usX2Y_clients_stop(usX2Y); }
-static void usX2Y_error_sequence(struct usX2Ydev *usX2Y,
struct snd_usX2Y_substream *subs, struct urb *urb)-{
- snd_printk(KERN_ERR
-"Sequence Error!(hcd_frame=%i ep=%i%s;wait=%i,frame=%i).\n" -"Most probably some urb of usb-frame %i is still missing.\n" -"Cause could be too long delays in usb-hcd interrupt handling.\n",
usb_get_current_frame_number(usX2Y->dev),subs->endpoint, usb_pipein(urb->pipe) ? "in" : "out",usX2Y->wait_iso_frame, urb->start_frame, usX2Y->wait_iso_frame);- usX2Y_clients_stop(usX2Y);
-}
static void i_usX2Y_urb_complete(struct urb *urb) { struct snd_usX2Y_substream *subs = urb->context; @@ -328,12 +315,9 @@ static void i_usX2Y_urb_complete(struct urb *urb) usX2Y_error_urb_status(usX2Y, subs, urb); return; }
- if (likely((urb->start_frame & 0xFFFF) == (usX2Y->wait_iso_frame &
0xFFFF))) - subs->completed_urb = urb;
- else {
usX2Y_error_sequence(usX2Y, subs, urb);return;- }
- subs->completed_urb = urb;
- { struct snd_usX2Y_substream *capsubs =
usX2Y->subs[SNDRV_PCM_STREAM_CAPTURE], *playbacksubs = usX2Y->subs[SNDRV_PCM_STREAM_PLAYBACK]; diff --git a/sound/usb/usx2y/usx2yhwdeppcm.c b/sound/usb/usx2y/usx2yhwdeppcm.c index f2a1acd..814d0e8 100644 --- a/sound/usb/usx2y/usx2yhwdeppcm.c +++ b/sound/usb/usx2y/usx2yhwdeppcm.c @@ -244,13 +244,8 @@ static void i_usX2Y_usbpcm_urb_complete(struct urb *urb) usX2Y_error_urb_status(usX2Y, subs, urb); return; }
- if (likely((urb->start_frame & 0xFFFF) == (usX2Y->wait_iso_frame &
0xFFFF))) - subs->completed_urb = urb;
- else {
usX2Y_error_sequence(usX2Y, subs, urb);return;- }
- subs->completed_urb = urb; capsubs = usX2Y->subs[SNDRV_PCM_STREAM_CAPTURE]; capsubs2 = usX2Y->subs[SNDRV_PCM_STREAM_CAPTURE + 2]; playbacksubs = usX2Y->subs[SNDRV_PCM_STREAM_PLAYBACK];
At Thu, 3 Oct 2013 11:25:03 +0100, Dr Nicholas J Bailey wrote:
I applied the patch by hand to my current source tree (3.10.11) and compiled the kernel from scratch.
I plugged a MIDI cable between midi in and out on the TASCAM US-122. Using QJackCtl, I connected the US-122's MIDI out to fluidsynth, and used amidiplay to send play a MIDI file to the US-122.
While the MIDI was playing, I used Audacity to record the audio via the US-122's mic inputs holding the mic close to the speakers. After about 10 seconds, I rewound audacity and hit record again so that it was both playing and recording through the US-122 while the US-122 was also routing MIDI.
This appears to work fine (at 44100Hz sampling; I've not tried any others), and I reckon this is a pretty severe test.
Thank you for pushing this patch which seems to resolve the original issue. This will be particularly appreciated by many because the US-122, no longer supported for Windows I believe, is currently available on eBay for about £30 in the UK, and represents an inexpensive way for Linux users to make reasonable quality recordings from microphones needing a phantom power source.
OK, applied the patch now with Cc to stable.
thanks,
Takashi
On Wednesday 02 October 2013 16:49:50 Daniel Mack wrote:
The frame check in i_usX2Y_urb_complete() and i_usX2Y_usbpcm_urb_complete() is bogus and produces false positives as described in this LAU thread:
http://linuxaudio.org/mailarchive/lau/2013/5/20/200177
This patch removes the check code entirely.
Cc: fzu@wemgehoertderstaat.de Reported-by: Dr Nicholas J Bailey nicholas.bailey@glasgow.ac.uk Suggested-by: Takashi Iwai tiwai@suse.de Signed-off-by: Daniel Mack zonque@gmail.com
sound/usb/usx2y/usbusx2yaudio.c | 22 +++------------------- sound/usb/usx2y/usx2yhwdeppcm.c | 7 +------ 2 files changed, 4 insertions(+), 25 deletions(-)
diff --git a/sound/usb/usx2y/usbusx2yaudio.c b/sound/usb/usx2y/usbusx2yaudio.c index 63fb521..6234a51 100644 --- a/sound/usb/usx2y/usbusx2yaudio.c +++ b/sound/usb/usx2y/usbusx2yaudio.c @@ -299,19 +299,6 @@ static void usX2Y_error_urb_status(struct usX2Ydev *usX2Y, usX2Y_clients_stop(usX2Y); }
-static void usX2Y_error_sequence(struct usX2Ydev *usX2Y,
struct snd_usX2Y_substream *subs, struct urb *urb)-{
- snd_printk(KERN_ERR
-"Sequence Error!(hcd_frame=%i ep=%i%s;wait=%i,frame=%i).\n" -"Most probably some urb of usb-frame %i is still missing.\n" -"Cause could be too long delays in usb-hcd interrupt handling.\n",
usb_get_current_frame_number(usX2Y->dev),subs->endpoint, usb_pipein(urb->pipe) ? "in" : "out",usX2Y->wait_iso_frame, urb->start_frame, usX2Y->wait_iso_frame);- usX2Y_clients_stop(usX2Y);
-}
static void i_usX2Y_urb_complete(struct urb *urb) { struct snd_usX2Y_substream *subs = urb->context; @@ -328,12 +315,9 @@ static void i_usX2Y_urb_complete(struct urb *urb) usX2Y_error_urb_status(usX2Y, subs, urb); return; }
- if (likely((urb->start_frame & 0xFFFF) == (usX2Y->wait_iso_frame &
0xFFFF))) - subs->completed_urb = urb;
- else {
usX2Y_error_sequence(usX2Y, subs, urb);return;- }
- subs->completed_urb = urb;
- { struct snd_usX2Y_substream *capsubs =
usX2Y->subs[SNDRV_PCM_STREAM_CAPTURE], *playbacksubs = usX2Y->subs[SNDRV_PCM_STREAM_PLAYBACK]; diff --git a/sound/usb/usx2y/usx2yhwdeppcm.c b/sound/usb/usx2y/usx2yhwdeppcm.c index f2a1acd..814d0e8 100644 --- a/sound/usb/usx2y/usx2yhwdeppcm.c +++ b/sound/usb/usx2y/usx2yhwdeppcm.c @@ -244,13 +244,8 @@ static void i_usX2Y_usbpcm_urb_complete(struct urb *urb) usX2Y_error_urb_status(usX2Y, subs, urb); return; }
- if (likely((urb->start_frame & 0xFFFF) == (usX2Y->wait_iso_frame &
0xFFFF))) - subs->completed_urb = urb;
- else {
usX2Y_error_sequence(usX2Y, subs, urb);return;- }
- subs->completed_urb = urb; capsubs = usX2Y->subs[SNDRV_PCM_STREAM_CAPTURE]; capsubs2 = usX2Y->subs[SNDRV_PCM_STREAM_CAPTURE + 2]; playbacksubs = usX2Y->subs[SNDRV_PCM_STREAM_PLAYBACK];
Next time, when detecting such problems, please consider sending a
patch
to alsa-devel, so fixes make it to the users eventually :)
You are right, but I thought it was a problem with a broken/weird usb controller, because it worked with my old laptop. When I saw another user with the same problem, I sent the patch to LAU (the only mailing list I've subscribed to).
Now I remember why I did that patch: if you see my dmesg output without that patch, you can see that the driver is comparing 1024 to 0. If you check the history of the driver, you can see that this check has already been narrowed with the bit AND operation; I tried to narrow the check a little more, so I chose 0x3ff because 1024 & 0x3ff = 0. But I forgot about my Tascam (because I bought a RME Raydat) and never tested that patch, but I was quite confident that it should work.
[ 669.312765] ALSA sound/usb/usx2y/usbusx2yaudio.c:141 should not be here with counts=42 [ 669.329821] ALSA sound/usb/usx2y/usbusx2yaudio.c:141 should not be here with counts=42 [ 669.345854] ALSA sound/usb/usx2y/usbusx2yaudio.c:141 should not be here with counts=42 [ 669.471627] ALSA sound/usb/usx2y/usbusx2yaudio.c:311 Sequence Error!(hcd_frame=3 ep=10out;wait=1024,frame=0). Most probably some urb of usb-frame 1024 is still missing. Cause could be too long delays in usb-hcd interrupt handling. [ 1203.531392] ALSA sound/usb/usx2y/usbusx2yaudio.c:311 Sequence Error!(hcd_frame=4 ep=8in;wait=1025,frame=1). Most probably some urb of usb-frame 1025 is still missing. Cause could be too long delays in usb-hcd interrupt handling. [ 1218.908160] ALSA sound/usb/usx2y/usbusx2yaudio.c:311 Sequence Error!(hcd_frame=5 ep=8in;wait=1026,frame=2). Most probably some urb of usb-frame 1026 is still missing.
Ciao a tutti Guido
At Sun, 06 Oct 2013 10:29:23 +0200, Guido Aulisi wrote:
Next time, when detecting such problems, please consider sending a
patch
to alsa-devel, so fixes make it to the users eventually :)
You are right, but I thought it was a problem with a broken/weird usb controller, because it worked with my old laptop. When I saw another user with the same problem, I sent the patch to LAU (the only mailing list I've subscribed to).
Now I remember why I did that patch: if you see my dmesg output without that patch, you can see that the driver is comparing 1024 to 0. If you check the history of the driver, you can see that this check has already been narrowed with the bit AND operation; I tried to narrow the check a little more, so I chose 0x3ff because 1024 & 0x3ff = 0. But I forgot about my Tascam (because I bought a RME Raydat) and never tested that patch, but I was quite confident that it should work.
[ 669.312765] ALSA sound/usb/usx2y/usbusx2yaudio.c:141 should not be here with counts=42 [ 669.329821] ALSA sound/usb/usx2y/usbusx2yaudio.c:141 should not be here with counts=42 [ 669.345854] ALSA sound/usb/usx2y/usbusx2yaudio.c:141 should not be here with counts=42
Maybe this is the actual reason? This is a place returning -EPIPE, thus these urbs won't be submitted.
Can anyone test whether the patch below works alone without the previous patch? (Meanwhile, the previous patch to remove the frame check is fine, it's just too strict for controllers like ehci, so it's still fine to apply.)
thanks,
Takashi
--- diff --git a/sound/usb/usx2y/usbusx2yaudio.c b/sound/usb/usx2y/usbusx2yaudio.c index 6234a51..1236ff3 100644 --- a/sound/usb/usx2y/usbusx2yaudio.c +++ b/sound/usb/usx2y/usbusx2yaudio.c @@ -137,10 +137,12 @@ static int usX2Y_urb_play_prepare(struct snd_usX2Y_substream *subs, /* calculate the size of a packet */ counts = cap_urb->iso_frame_desc[pack].actual_length / usX2Y->stride; count += counts; +#if 0 if (counts < 43 || counts > 50) { snd_printk(KERN_ERR "should not be here with counts=%i\n", counts); return -EPIPE; } +#endif /* set up descriptor */ urb->iso_frame_desc[pack].offset = pack ? urb->iso_frame_desc[pack - 1].offset +
On Monday 07 October 2013 08:41:06 you wrote:
Can anyone test whether the patch below works alone without the previous patch? (Meanwhile, the previous patch to remove the frame check is fine, it's just too strict for controllers like ehci, so it's still fine to apply.)
thanks,
Takashi
diff --git a/sound/usb/usx2y/usbusx2yaudio.c b/sound/usb/usx2y/usbusx2yaudio.c index 6234a51..1236ff3 100644 --- a/sound/usb/usx2y/usbusx2yaudio.c +++ b/sound/usb/usx2y/usbusx2yaudio.c @@ -137,10 +137,12 @@ static int usX2Y_urb_play_prepare(struct snd_usX2Y_substream *subs, /* calculate the size of a packet */ counts = cap_urb->iso_frame_desc[pack].actual_length / usX2Y->stride; count += counts; +#if 0 if (counts < 43 || counts > 50) { snd_printk(KERN_ERR "should not be here with counts=%i\n", counts); return -EPIPE; } +#endif /* set up descriptor */ urb->iso_frame_desc[pack].offset = pack ? urb->iso_frame_desc[pack - 1].offset +
For me, this patch alone fails in much the same way (possibly exactly the same way) as before the previous patch.
From audacity (probably not much use unless you know the source code... I
don't):
Expression 'PaAlsaStreamComponent_Initialize( &self->capture, alsaApi, inParams, StreamDirection_In, NULL != callback )' failed in 'src/hostapi/alsa/pa_linux_alsa.c', line: 2092 Expression 'PaAlsaStream_Initialize( stream, alsaHostApi, inputParameters, outputParameters, sampleRate, framesPerBuffer, callback, streamFlags, userData )' failed in 'src/hostapi/alsa/pa_linux_alsa.c', line: 2764 Expression 'stream->playback.pcm' failed in 'src/hostapi/alsa/pa_linux_alsa.c', line: 4541 Expression 'stream->playback.pcm' failed in 'src/hostapi/alsa/pa_linux_alsa.c', line: 4541 ^C
[I hit ^C because recording stalled]
nick@arial:/usr/src$ dmesg | tail [ 111.667015] usb 2-1.6.4: New USB device strings: Mfr=0, Product=0, SerialNumber=0 [ 114.707156] Sequence Error!(hcd_frame=3 ep=8in;wait=1024,frame=0). [ 114.707156] Most probably some urb of usb-frame 1024 is still missing. [ 114.707156] Cause could be too long delays in usb-hcd interrupt handling. [ 574.566748] Sequence Error!(hcd_frame=3 ep=8in;wait=1024,frame=0). [ 574.566748] Most probably some urb of usb-frame 1024 is still missing. [ 574.566748] Cause could be too long delays in usb-hcd interrupt handling. [ 575.593918] Sequence Error!(hcd_frame=6 ep=8in;wait=1027,frame=3). [ 575.593918] Most probably some urb of usb-frame 1027 is still missing. [ 575.593918] Cause could be too long delays in usb-hcd interrupt handling.
Reverting to the previous patch, everything works fine:
[ 170.252226] usb 2-1.6.4: USB disconnect, device number 5 [ 173.012844] usb 2-1.6.4: new full-speed USB device number 7 using ehci-pci [ 173.105451] usb 2-1.6.4: New USB device found, idVendor=1604, idProduct=8006 [ 173.105456] usb 2-1.6.4: New USB device strings: Mfr=0, Product=0, SerialNumber=0 [ 175.372992] usb 2-1.6.4: USB disconnect, device number 7 [ 177.109626] usb 2-1.6.4: new full-speed USB device number 8 using ehci-pci [ 177.202361] usb 2-1.6.4: New USB device found, idVendor=1604, idProduct=8007 [ 177.202372] usb 2-1.6.4: New USB device strings: Mfr=0, Product=0, SerialNumber=0
[no further messages]
Best stick with the original patch then :)
Nick/.
On 07.10.2013 16:58, Dr Nicholas J Bailey wrote:
For me, this patch alone fails in much the same way (possibly exactly the same way) as before the previous patch.
From audacity (probably not much use unless you know the source code... I don't):
[...]
nick@arial:/usr/src$ dmesg | tail [ 111.667015] usb 2-1.6.4: New USB device strings: Mfr=0, Product=0, SerialNumber=0 [ 114.707156] Sequence Error!(hcd_frame=3 ep=8in;wait=1024,frame=0). [ 114.707156] Most probably some urb of usb-frame 1024 is still missing.
You certainly haven't booted a kernel which contains the patch we're talking about here. The only occurance of the error message you quote was removed by this patch, so a patched kernel can't possibly produce what you see in your logs.
Please check your setup again and see what 'uname -a' says.
Daniel
At Mon, 07 Oct 2013 17:11:44 +0200, Daniel Mack wrote:
On 07.10.2013 16:58, Dr Nicholas J Bailey wrote:
For me, this patch alone fails in much the same way (possibly exactly the same way) as before the previous patch.
From audacity (probably not much use unless you know the source code... I don't):
[...]
nick@arial:/usr/src$ dmesg | tail [ 111.667015] usb 2-1.6.4: New USB device strings: Mfr=0, Product=0, SerialNumber=0 [ 114.707156] Sequence Error!(hcd_frame=3 ep=8in;wait=1024,frame=0). [ 114.707156] Most probably some urb of usb-frame 1024 is still missing.
You certainly haven't booted a kernel which contains the patch we're talking about here. The only occurance of the error message you quote was removed by this patch, so a patched kernel can't possibly produce what you see in your logs.
Nicholas booted a kernel without your fix patch but only with my patch to disable the -EPIPE check, in order to see whether the latter alone suffices or not. So he's testing with a right kernel :)
Takashi
On 07.10.2013 17:35, Takashi Iwai wrote:
At Mon, 07 Oct 2013 17:11:44 +0200, Daniel Mack wrote:
On 07.10.2013 16:58, Dr Nicholas J Bailey wrote:
For me, this patch alone fails in much the same way (possibly exactly the same way) as before the previous patch.
From audacity (probably not much use unless you know the source code... I don't):
[...]
nick@arial:/usr/src$ dmesg | tail [ 111.667015] usb 2-1.6.4: New USB device strings: Mfr=0, Product=0, SerialNumber=0 [ 114.707156] Sequence Error!(hcd_frame=3 ep=8in;wait=1024,frame=0). [ 114.707156] Most probably some urb of usb-frame 1024 is still missing.
You certainly haven't booted a kernel which contains the patch we're talking about here. The only occurance of the error message you quote was removed by this patch, so a patched kernel can't possibly produce what you see in your logs.
Nicholas booted a kernel without your fix patch but only with my patch to disable the -EPIPE check, in order to see whether the latter alone suffices or not. So he's testing with a right kernel :)
Oh, so *I* was referring to the wrong patch then. Apologies for causing confusion :)
Daniel
On Monday 07 October 2013 16:34:37 you wrote:
On 07.10.2013 17:35, Takashi Iwai wrote:
At Mon, 07 Oct 2013 17:11:44 +0200,
Daniel Mack wrote:
On 07.10.2013 16:58, Dr Nicholas J Bailey wrote:
For me, this patch alone fails in much the same way (possibly exactly the same way) as before the previous patch.
From audacity (probably not much use unless you know the source code... I
don't):
[...]
nick@arial:/usr/src$ dmesg | tail [ 111.667015] usb 2-1.6.4: New USB device strings: Mfr=0, Product=0, SerialNumber=0 [ 114.707156] Sequence Error!(hcd_frame=3 ep=8in;wait=1024,frame=0). [ 114.707156] Most probably some urb of usb-frame 1024 is still missing.
You certainly haven't booted a kernel which contains the patch we're talking about here. The only occurance of the error message you quote was removed by this patch, so a patched kernel can't possibly produce what you see in your logs.
Nicholas booted a kernel without your fix patch but only with my patch to disable the -EPIPE check, in order to see whether the latter alone suffices or not. So he's testing with a right kernel :)
Oh, so *I* was referring to the wrong patch then. Apologies for causing confusion :)
Daniel
That's cool, Daniel. Actually, I've not written anything for the kernel since 1.x (it was an intravascular ultrasound board my PhD student built - it was a hardware project and the sorfware was never distributed). Things have changed a *lot* since then, and I never used make-dpkg before this, so please keep on checking I do the right thing!
The only thing I *might* have screwed up is that both the kernels are called 3.10.11 as far as uname -a is concerned, but are in separate debs. So the patch which took out
static void usX2Y_error_sequence( struct usX2Ydev *usX2Y, struct snd_usX2Y_substream *subs, struct urb *urb)
and the bit about
if (likely((urb->start_frame & 0xFFFF) == (usX2Y->wait_iso_frame & 0xFFFF))) subs->completed_urb = urb; else { usX2Y_error_sequence(usX2Y, subs, urb); return; }
are in a deb called
linux-image-3.10.11_3.10.11.TASCAM.2_i386.deb
and the one with Takashi's cpp directive /only/ based on a fresh source tree from the debian source package is in
linux-image-3.10.11_3.10.11.TASCAM.3_i386.deb
I'm installing the package I want to play with, and there's a message to say I should reboot ASAP because of potential module conflicts, and that when I do the machine will recompute the mod deps for me, so I do, and that's that.
Probably a bit of a hack, but I don't think it's stupidly wrong (unless you know different).
Bottom line is: TASCAM.2 works with no complaints; TASCAM.3 (with just the #if 0...#endif and no other changes to 3.10.11) gives the stalls and dmesg messages.
The only other thing I changed was the processor. I selected core2 because I thought it would be nice to have a custom kernel for once :)
I hope I've not been wasting your time.
Nick/.
On Monday 07 October 2013 16:07:28 you wrote:
OK, then the sequence error thingy is independent from the bogus overrun check. But I guess you still get the errors about "should not be here with..." even with the sequence-check-removal patch?
thanks,
Takashi
No, because I did the bigger of the two original patches (this is getting difficult!!) as per the other email I just sent, so the whole of the function usX2Y_error_sequence is gone. So no messages.
I did also build a package I called
linux-image-3.10.11_3.10.11.TASCAM.1_i386.deb
which had just the change to the
if (likely((urb->start_frame & 0xFFFF) == (usX2Y->wait_iso_frame & 0xFFFF)))
(0xFFFF to 0X03FF or whatever). That worked too. I can't remember if there were still sequence errors. I've deleted now I'm afraid, having gone for the larger version.
The patch that works well for me modifies sound/usb/usx2y/usx2yhwdeppcm.c as well, replacing a whole similar if statement at line 244 with the line
subs->completed_urb = urb;
I got it from the attached email from Daniel.
Nick/.
participants (4)
-
Daniel Mack -
Dr Nicholas J Bailey -
Guido Aulisi -
Takashi Iwai