On Tue, 12 Feb 2013 15:29:19 +0100 Clemens Ladisch clemens@ladisch.de wrote:
Antonio Ospite wrote:
Clemens Ladisch clemens@ladisch.de wrote:
Antonio Ospite wrote:
[...]
- if (quirk && quirk->device_name)
strcpy(card->shortname, quirk->device_name);- else
strcpy(card->shortname, "M2Tech generic audio");Don't the devices have their name in the device descriptor?
It looks like the iProduct field is not reliable enough, different devices could have the same value there.
The iProduct field is the number of the string descriptor. That string might be useful at least in the non-quirk case (which shouldn't happen in the first case AFAICS).
The original authors of the driver preferred to have all the device names hardcoded in the driver, I guess they had a reason for that; for now I'll leave it this way as I do not have all the supported devices to verify the product strings for all of them. If Pietro can provide this info I will update the driver to rely on info from the devices themselves.
+static void hiface_pcm_out_urb_handler(struct urb *usb_urb) +{
- if (usb_urb->status || rt->panic || rt->stream_state == STREAM_STOPPING)
return;In some error cases, you might consider stopping the stream.
You mean I should check explicitly for some values of usb_urb->status and stop the stream if they happen? Like you do in sound/usb/misc/ua101.c::capture_urb_complete()?
If only one URB doesn't get delivered for whatever reason, how should the driver react? If you just exit from the completion handler, the driver will continue to fill and queue all the other URBs. And what happens when the device is unplugged?
I see, since —as Takashi said— killing URBs is not safe in the complete callback I'll just set rt->panic to true when URBs fail:
https://github.com/panicking/snd-usb-asyncaudio/commit/0b7415e559e1a868240ac...
ret = hiface_pcm_playback(sub, out_urb);if (ret < 0) {spin_unlock_irqrestore(&sub->lock, flags);goto out_fail;}+... +out_fail:
- usb_submit_urb(&out_urb->instance, GFP_ATOMIC);
Same here.
If I remove the check for format in hiface_pcm_playback() then this label would go away too and this won't be an error path anymore, but maybe you meant that I need to check the return value of usb_submit_urb () and stop the stream when the latter fails?
At the moment, you are trying to queue a URB with invalid samples.
I am going to send a version 2 soon.
Thanks again, Antonio