Commit 015618b902ae8e28705b7af9b4668615fea48ddd
Committed by
Takashi Iwai
Takashi Iwai
1 parent
c36b5b054a
Exists in
smarc-l5.0.0_1.0.0-ga
and in
5 other branches
ALSA: snd-usb: Fix URB cancellation at stream start
Commit e9ba389c5 ("ALSA: usb-audio: Fix scheduling-while-atomic bug in PCM capture stream") fixed a scheduling-while-atomic bug that happened when snd_usb_endpoint_start was called from the trigger callback, which is an atmic context. However, the patch breaks the idea of the endpoints reference counting, which is the reason why the driver has been refactored lately. Revert that commit and let snd_usb_endpoint_start() take care of the URB cancellation again. As this function is called from both atomic and non-atomic context, add a flag to denote whether the function may sleep. Signed-off-by: Daniel Mack <zonque@gmail.com> Cc: stable@kernel.org [3.5+] Signed-off-by: Takashi Iwai <tiwai@suse.de>
Showing 3 changed files with 15 additions and 11 deletions Side-by-side Diff
sound/usb/endpoint.c
| ... | ... | @@ -799,7 +799,9 @@ |
| 799 | 799 | /** |
| 800 | 800 | * snd_usb_endpoint_start: start an snd_usb_endpoint |
| 801 | 801 | * |
| 802 | - * @ep: the endpoint to start | |
| 802 | + * @ep: the endpoint to start | |
| 803 | + * @can_sleep: flag indicating whether the operation is executed in | |
| 804 | + * non-atomic context | |
| 803 | 805 | * |
| 804 | 806 | * A call to this function will increment the use count of the endpoint. |
| 805 | 807 | * In case it is not already running, the URBs for this endpoint will be |
| ... | ... | @@ -809,7 +811,7 @@ |
| 809 | 811 | * |
| 810 | 812 | * Returns an error if the URB submission failed, 0 in all other cases. |
| 811 | 813 | */ |
| 812 | -int snd_usb_endpoint_start(struct snd_usb_endpoint *ep) | |
| 814 | +int snd_usb_endpoint_start(struct snd_usb_endpoint *ep, int can_sleep) | |
| 813 | 815 | { |
| 814 | 816 | int err; |
| 815 | 817 | unsigned int i; |
| ... | ... | @@ -820,6 +822,11 @@ |
| 820 | 822 | /* already running? */ |
| 821 | 823 | if (++ep->use_count != 1) |
| 822 | 824 | return 0; |
| 825 | + | |
| 826 | + /* just to be sure */ | |
| 827 | + deactivate_urbs(ep, 0, can_sleep); | |
| 828 | + if (can_sleep) | |
| 829 | + wait_clear_urbs(ep); | |
| 823 | 830 | |
| 824 | 831 | ep->active_mask = 0; |
| 825 | 832 | ep->unlink_mask = 0; |
sound/usb/endpoint.h
| ... | ... | @@ -13,7 +13,7 @@ |
| 13 | 13 | struct audioformat *fmt, |
| 14 | 14 | struct snd_usb_endpoint *sync_ep); |
| 15 | 15 | |
| 16 | -int snd_usb_endpoint_start(struct snd_usb_endpoint *ep); | |
| 16 | +int snd_usb_endpoint_start(struct snd_usb_endpoint *ep, int can_sleep); | |
| 17 | 17 | void snd_usb_endpoint_stop(struct snd_usb_endpoint *ep, |
| 18 | 18 | int force, int can_sleep, int wait); |
| 19 | 19 | int snd_usb_endpoint_activate(struct snd_usb_endpoint *ep); |
sound/usb/pcm.c
| ... | ... | @@ -212,7 +212,7 @@ |
| 212 | 212 | } |
| 213 | 213 | } |
| 214 | 214 | |
| 215 | -static int start_endpoints(struct snd_usb_substream *subs) | |
| 215 | +static int start_endpoints(struct snd_usb_substream *subs, int can_sleep) | |
| 216 | 216 | { |
| 217 | 217 | int err; |
| 218 | 218 | |
| ... | ... | @@ -225,7 +225,7 @@ |
| 225 | 225 | snd_printdd(KERN_DEBUG "Starting data EP @%p\n", ep); |
| 226 | 226 | |
| 227 | 227 | ep->data_subs = subs; |
| 228 | - err = snd_usb_endpoint_start(ep); | |
| 228 | + err = snd_usb_endpoint_start(ep, can_sleep); | |
| 229 | 229 | if (err < 0) { |
| 230 | 230 | clear_bit(SUBSTREAM_FLAG_DATA_EP_STARTED, &subs->flags); |
| 231 | 231 | return err; |
| ... | ... | @@ -239,7 +239,7 @@ |
| 239 | 239 | snd_printdd(KERN_DEBUG "Starting sync EP @%p\n", ep); |
| 240 | 240 | |
| 241 | 241 | ep->sync_slave = subs->data_endpoint; |
| 242 | - err = snd_usb_endpoint_start(ep); | |
| 242 | + err = snd_usb_endpoint_start(ep, can_sleep); | |
| 243 | 243 | if (err < 0) { |
| 244 | 244 | clear_bit(SUBSTREAM_FLAG_SYNC_EP_STARTED, &subs->flags); |
| 245 | 245 | return err; |
| 246 | 246 | |
| ... | ... | @@ -544,13 +544,10 @@ |
| 544 | 544 | subs->last_frame_number = 0; |
| 545 | 545 | runtime->delay = 0; |
| 546 | 546 | |
| 547 | - /* clear the pending deactivation on the target EPs */ | |
| 548 | - deactivate_endpoints(subs); | |
| 549 | - | |
| 550 | 547 | /* for playback, submit the URBs now; otherwise, the first hwptr_done |
| 551 | 548 | * updates for all URBs would happen at the same time when starting */ |
| 552 | 549 | if (subs->direction == SNDRV_PCM_STREAM_PLAYBACK) |
| 553 | - return start_endpoints(subs); | |
| 550 | + return start_endpoints(subs, 1); | |
| 554 | 551 | |
| 555 | 552 | return 0; |
| 556 | 553 | } |
| ... | ... | @@ -1175,7 +1172,7 @@ |
| 1175 | 1172 | |
| 1176 | 1173 | switch (cmd) { |
| 1177 | 1174 | case SNDRV_PCM_TRIGGER_START: |
| 1178 | - err = start_endpoints(subs); | |
| 1175 | + err = start_endpoints(subs, 0); | |
| 1179 | 1176 | if (err < 0) |
| 1180 | 1177 | return err; |
| 1181 | 1178 |