* [PATCH v2] ALSA: snd-usb-usx2y: remove bogus frame checks @ 2013-10-02 15:49 Daniel Mack 2013-10-03 10:25 ` [PATCH v2] ALSA: snd-usb-usx2y: remove bogus frame checks -- SUCCESS Dr Nicholas J Bailey 2013-10-06 8:29 ` [PATCH v2] ALSA: snd-usb-usx2y: remove bogus frame checks Guido Aulisi 0 siblings, 2 replies; 11+ messages in thread From: Daniel Mack @ 2013-10-02 15:49 UTC (permalink / raw) To: alsa-devel; +Cc: tiwai, guido.aulisi, nicholas.bailey, Daniel Mack 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]; -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v2] ALSA: snd-usb-usx2y: remove bogus frame checks -- SUCCESS 2013-10-02 15:49 [PATCH v2] ALSA: snd-usb-usx2y: remove bogus frame checks Daniel Mack @ 2013-10-03 10:25 ` Dr Nicholas J Bailey 2013-10-07 7:35 ` Takashi Iwai 2013-10-06 8:29 ` [PATCH v2] ALSA: snd-usb-usx2y: remove bogus frame checks Guido Aulisi 1 sibling, 1 reply; 11+ messages in thread From: Dr Nicholas J Bailey @ 2013-10-03 10:25 UTC (permalink / raw) To: alsa-devel@alsa-project.org Cc: tiwai@suse.de, Daniel Mack, jennifer.macritchie, graham, guido.aulisi@gmail.com 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]; ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] ALSA: snd-usb-usx2y: remove bogus frame checks -- SUCCESS 2013-10-03 10:25 ` [PATCH v2] ALSA: snd-usb-usx2y: remove bogus frame checks -- SUCCESS Dr Nicholas J Bailey @ 2013-10-07 7:35 ` Takashi Iwai 0 siblings, 0 replies; 11+ messages in thread From: Takashi Iwai @ 2013-10-07 7:35 UTC (permalink / raw) To: Dr Nicholas J Bailey Cc: alsa-devel@alsa-project.org, Daniel Mack, jennifer.macritchie, graham, guido.aulisi@gmail.com 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]; > _______________________________________________ Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] ALSA: snd-usb-usx2y: remove bogus frame checks 2013-10-02 15:49 [PATCH v2] ALSA: snd-usb-usx2y: remove bogus frame checks Daniel Mack 2013-10-03 10:25 ` [PATCH v2] ALSA: snd-usb-usx2y: remove bogus frame checks -- SUCCESS Dr Nicholas J Bailey @ 2013-10-06 8:29 ` Guido Aulisi 2013-10-07 7:41 ` Takashi Iwai 2013-10-07 14:58 ` Dr Nicholas J Bailey 1 sibling, 2 replies; 11+ messages in thread From: Guido Aulisi @ 2013-10-06 8:29 UTC (permalink / raw) To: Daniel Mack; +Cc: tiwai, alsa-devel, nicholas.bailey > 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] ALSA: snd-usb-usx2y: remove bogus frame checks 2013-10-06 8:29 ` [PATCH v2] ALSA: snd-usb-usx2y: remove bogus frame checks Guido Aulisi @ 2013-10-07 7:41 ` Takashi Iwai 2013-10-07 14:58 ` Dr Nicholas J Bailey 1 sibling, 0 replies; 11+ messages in thread From: Takashi Iwai @ 2013-10-07 7:41 UTC (permalink / raw) To: Guido Aulisi; +Cc: alsa-devel, nicholas.bailey, Daniel Mack 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 + ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v2] ALSA: snd-usb-usx2y: remove bogus frame checks 2013-10-06 8:29 ` [PATCH v2] ALSA: snd-usb-usx2y: remove bogus frame checks Guido Aulisi 2013-10-07 7:41 ` Takashi Iwai @ 2013-10-07 14:58 ` Dr Nicholas J Bailey 2013-10-07 15:11 ` Daniel Mack 1 sibling, 1 reply; 11+ messages in thread From: Dr Nicholas J Bailey @ 2013-10-07 14:58 UTC (permalink / raw) To: Guido Aulisi; +Cc: tiwai@suse.de, alsa-devel@alsa-project.org, Daniel Mack 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/. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] ALSA: snd-usb-usx2y: remove bogus frame checks 2013-10-07 14:58 ` Dr Nicholas J Bailey @ 2013-10-07 15:11 ` Daniel Mack 2013-10-07 15:35 ` Takashi Iwai 0 siblings, 1 reply; 11+ messages in thread From: Daniel Mack @ 2013-10-07 15:11 UTC (permalink / raw) To: Dr Nicholas J Bailey Cc: tiwai@suse.de, alsa-devel@alsa-project.org, Guido Aulisi 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] ALSA: snd-usb-usx2y: remove bogus frame checks 2013-10-07 15:11 ` Daniel Mack @ 2013-10-07 15:35 ` Takashi Iwai 2013-10-07 15:34 ` Daniel Mack 0 siblings, 1 reply; 11+ messages in thread From: Takashi Iwai @ 2013-10-07 15:35 UTC (permalink / raw) To: Daniel Mack Cc: alsa-devel@alsa-project.org, Guido Aulisi, Dr Nicholas J Bailey 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] ALSA: snd-usb-usx2y: remove bogus frame checks 2013-10-07 15:35 ` Takashi Iwai @ 2013-10-07 15:34 ` Daniel Mack 2013-10-07 16:20 ` Dr Nicholas J Bailey 2013-10-07 16:31 ` Dr Nicholas J Bailey 0 siblings, 2 replies; 11+ messages in thread From: Daniel Mack @ 2013-10-07 15:34 UTC (permalink / raw) To: Takashi Iwai Cc: alsa-devel@alsa-project.org, Guido Aulisi, Dr Nicholas J Bailey 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] ALSA: snd-usb-usx2y: remove bogus frame checks 2013-10-07 15:34 ` Daniel Mack @ 2013-10-07 16:20 ` Dr Nicholas J Bailey 2013-10-07 16:31 ` Dr Nicholas J Bailey 1 sibling, 0 replies; 11+ messages in thread From: Dr Nicholas J Bailey @ 2013-10-07 16:20 UTC (permalink / raw) To: Daniel Mack; +Cc: Takashi Iwai, alsa-devel@alsa-project.org, Guido Aulisi 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/. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] ALSA: snd-usb-usx2y: remove bogus frame checks 2013-10-07 15:34 ` Daniel Mack 2013-10-07 16:20 ` Dr Nicholas J Bailey @ 2013-10-07 16:31 ` Dr Nicholas J Bailey 1 sibling, 0 replies; 11+ messages in thread From: Dr Nicholas J Bailey @ 2013-10-07 16:31 UTC (permalink / raw) To: Daniel Mack; +Cc: Takashi Iwai, alsa-devel@alsa-project.org, Guido Aulisi [-- Attachment #1: Type: text/plain, Size: 1084 bytes --] 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/. [-- Attachment #2: forwarded message --] [-- Type: text/plain, Size: 4928 bytes --] Content-Type: message/rfc822 Content-Disposition: inline; filename="forwarded message" Content-Description: Daniel Mack <zonque@gmail.com>: [PATCH v2] ALSA: snd-usb-usx2y: remove bogus frame checks Received: from palin.cent.gla.ac.uk (130.209.34.34) by smtp.campus.gla.ac.uk (130.209.14.19) with Microsoft SMTP Server id 8.3.327.1; Wed, 2 Oct 2013 16:50:03 +0100 Received: from svenfoo.org ([82.94.215.22] helo=mail.zonque.de) by palin.cent.gla.ac.uk with esmtp (Exim 4.72) (envelope-from <zonque@gmail.com>) id 1VROgc-00033f-QQ for nicholas.bailey@glasgow.ac.uk; Wed, 02 Oct 2013 16:50:03 +0100 Received: from localhost (localhost [127.0.0.1]) by mail.zonque.de (Postfix) with ESMTP id 817F7C1744; Wed, 2 Oct 2013 17:50:00 +0200 (CEST) Received: from mail.zonque.de ([127.0.0.1]) by localhost (rambrand.bugwerft.de [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id Hk6AJ7ax5yMM; Wed, 2 Oct 2013 17:50:00 +0200 (CEST) Received: from tamtam.taperay.com (unknown [46.115.92.231]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.zonque.de (Postfix) with ESMTPSA id B37B4C1732; Wed, 2 Oct 2013 17:49:58 +0200 (CEST) From: Daniel Mack <zonque@gmail.com> To: "alsa-devel@alsa-project.org" <alsa-devel@alsa-project.org> Cc: "tiwai@suse.de" <tiwai@suse.de>, Nicholas Bailey <Nicholas.Bailey@glasgow.ac.uk>, "guido.aulisi@gmail.com" <guido.aulisi@gmail.com>, Daniel Mack <zonque@gmail.com>, "fzu@wemgehoertderstaat.de" <fzu@wemgehoertderstaat.de> Date: Wed, 02 Oct 2013 16:49:50 +0100 Subject: [PATCH v2] ALSA: snd-usb-usx2y: remove bogus frame checks Thread-Topic: [PATCH v2] ALSA: snd-usb-usx2y: remove bogus frame checks Thread-Index: Ac6/hw9nqvIRMxIZQoqDHPQXxXHLkQ== Message-ID: <1380728990-8443-1-git-send-email-zonque@gmail.com> Accept-Language: en-GB Content-Language: en-GB X-MS-Exchange-Organization-AuthAs: Anonymous X-MS-Exchange-Organization-AuthSource: hub01.campus.gla.ac.uk x-gla-spam-scan: R x-gla-spam-report: _SUMMARY_ x-gla-spam-score: 0.0 (/) Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 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/usbusx2yaudi= o.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 *us= X2Y, usX2Y_clients_stop(usX2Y); } =20 -static void usX2Y_error_sequence(struct usX2Ydev *usX2Y, - struct snd_usX2Y_substream *subs, struct urb *urb) -{ - snd_printk(KERN_ERR -"Sequence Error!(hcd_frame=3D%i ep=3D%i%s;wait=3D%i,frame=3D%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 =3D 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) =3D=3D (usX2Y->wait_iso_frame & 0x= FFFF))) - subs->completed_urb =3D urb; - else { - usX2Y_error_sequence(usX2Y, subs, urb); - return; - } + + subs->completed_urb =3D urb; + { struct snd_usX2Y_substream *capsubs =3D usX2Y->subs[SNDRV_PCM_STREAM_CAP= TURE], *playbacksubs =3D usX2Y->subs[SNDRV_PCM_STREAM_PLAYBACK]; diff --git a/sound/usb/usx2y/usx2yhwdeppcm.c b/sound/usb/usx2y/usx2yhwdeppc= m.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 *ur= b) usX2Y_error_urb_status(usX2Y, subs, urb); return; } - if (likely((urb->start_frame & 0xFFFF) =3D=3D (usX2Y->wait_iso_frame & 0x= FFFF))) - subs->completed_urb =3D urb; - else { - usX2Y_error_sequence(usX2Y, subs, urb); - return; - } =20 + subs->completed_urb =3D urb; capsubs =3D usX2Y->subs[SNDRV_PCM_STREAM_CAPTURE]; capsubs2 =3D usX2Y->subs[SNDRV_PCM_STREAM_CAPTURE + 2]; playbacksubs =3D usX2Y->subs[SNDRV_PCM_STREAM_PLAYBACK]; --=20 1.8.3.1 [-- Attachment #3: Type: text/plain, Size: 0 bytes --] ^ permalink raw reply related [flat|nested] 11+ messages in thread
end of thread, other threads:[~2013-10-07 16:36 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-10-02 15:49 [PATCH v2] ALSA: snd-usb-usx2y: remove bogus frame checks Daniel Mack 2013-10-03 10:25 ` [PATCH v2] ALSA: snd-usb-usx2y: remove bogus frame checks -- SUCCESS Dr Nicholas J Bailey 2013-10-07 7:35 ` Takashi Iwai 2013-10-06 8:29 ` [PATCH v2] ALSA: snd-usb-usx2y: remove bogus frame checks Guido Aulisi 2013-10-07 7:41 ` Takashi Iwai 2013-10-07 14:58 ` Dr Nicholas J Bailey 2013-10-07 15:11 ` Daniel Mack 2013-10-07 15:35 ` Takashi Iwai 2013-10-07 15:34 ` Daniel Mack 2013-10-07 16:20 ` Dr Nicholas J Bailey 2013-10-07 16:31 ` Dr Nicholas J Bailey
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).