[alsa-devel] [PATCH] usb: add USB_QUIRK_RESET_RESUME for M-Audio 49
Some USB MIDI keyboards fail to operate after a USB autosuspend. The device is recognized by ALSA, but no events are received and the device goes quiet.
$ amidi -l Dir Device Name IO hw:1,0,0 Oxygen 49 MIDI 1 $ amidi -p hw:1,0,0 -d <... play some notes ...> ^C 0 bytes read
A workaround is to disable USB autosuspend for these devices by putting AUTOSUSPEND_USBID_BLACKLIST="0763:2027" (resp. 0763:019b) in /etc/laptop-mode/conf.d/usb-autosuspend.conf. In the spirit of commit 166cb70e97bd ("usb: add USB_QUIRK_RESET_RESUME for M-Audio 88es"), reset the device on resume so this workaround is not needed any more.
Noticed on a kernel close to 3.2.10. Tested against 3.6.5. Debian's 2.6.32.54-based kernel did not seem to be affected, though it's not clear whether that is due to code or configuration.
Addresses http://bugs.debian.org/664068
Reported-and-tested-by: David Banks amoebae@gmail.com # Oxygen 49 Reported-and-tested-by: Olivier MATZ zer0@droids-corp.org # KeyRig 49 Signed-off-by: Jonathan Nieder jrnieder@gmail.com Cc: stable@vger.kernel.org --- Thoughts?
drivers/usb/core/quirks.c | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/drivers/usb/core/quirks.c b/drivers/usb/core/quirks.c index fdefd9c7f7af..998c6a8290f6 100644 --- a/drivers/usb/core/quirks.c +++ b/drivers/usb/core/quirks.c @@ -105,6 +105,12 @@ static const struct usb_device_id usb_quirk_list[] = { /* Midiman M-Audio Keystation 88es */ { USB_DEVICE(0x0763, 0x0192), .driver_info = USB_QUIRK_RESET_RESUME },
+ /* Midiman M-Audio KeyRig 49 */ + { USB_DEVICE(0x0763, 0x019b), .driver_info = USB_QUIRK_RESET_RESUME }, + + /* Midiman M-Audio Oxygen 49 */ + { USB_DEVICE(0x0763, 0x2027), .driver_info = USB_QUIRK_RESET_RESUME }, + /* M-Systems Flash Disk Pioneers */ { USB_DEVICE(0x08ec, 0x1000), .driver_info = USB_QUIRK_RESET_RESUME },
Jonathan Nieder wrote:
Some USB MIDI keyboards fail to operate after a USB autosuspend.
Make that *all* USB MIDI devices with input ports.
This is not a bug in the device, but one of the many bugs introduced with the autosuspend code in http://git.kernel.org/linus/88a8516a2128.
That patch does not handle input at all, i.e., when the driver wants to read from the device, it just doesn't take it out of suspend mode.
A workaround is to disable USB autosuspend for these devices by putting AUTOSUSPEND_USBID_BLACKLIST="0763:2027" (resp. 0763:019b) in /etc/laptop-mode/conf.d/usb-autosuspend.conf. In the spirit of commit 166cb70e97bd ("usb: add USB_QUIRK_RESET_RESUME for M-Audio 88es"), reset the device on resume so this workaround is not needed any more.
It is not feasible to add the IDs of all USB MIDI devices.
I'm working on a fix that adds proper power management for input ports, but this requires the driver to be reorganized a little ...
Regards, Clemens
At Sun, 25 Nov 2012 23:01:27 +0100, Clemens Ladisch wrote:
Jonathan Nieder wrote:
Some USB MIDI keyboards fail to operate after a USB autosuspend.
Make that *all* USB MIDI devices with input ports.
This is not a bug in the device, but one of the many bugs introduced with the autosuspend code in http://git.kernel.org/linus/88a8516a2128.
That patch does not handle input at all, i.e., when the driver wants to read from the device, it just doesn't take it out of suspend mode.
A workaround is to disable USB autosuspend for these devices by putting AUTOSUSPEND_USBID_BLACKLIST="0763:2027" (resp. 0763:019b) in /etc/laptop-mode/conf.d/usb-autosuspend.conf. In the spirit of commit 166cb70e97bd ("usb: add USB_QUIRK_RESET_RESUME for M-Audio 88es"), reset the device on resume so this workaround is not needed any more.
It is not feasible to add the IDs of all USB MIDI devices.
I'm working on a fix that adds proper power management for input ports, but this requires the driver to be reorganized a little ...
Doesn't a simple patch like below work? (It even reduces more lines! :)
Takashi
--- diff --git a/sound/usb/midi.c b/sound/usb/midi.c index eeefbce..2e0fabc 100644 --- a/sound/usb/midi.c +++ b/sound/usb/midi.c @@ -148,7 +148,6 @@ struct snd_usb_midi_out_endpoint { struct snd_usb_midi_out_endpoint* ep; struct snd_rawmidi_substream *substream; int active; - bool autopm_reference; uint8_t cable; /* cable number << 4 */ uint8_t state; #define STATE_UNKNOWN 0 @@ -1033,29 +1032,35 @@ static void update_roland_altsetting(struct snd_usb_midi* umidi) snd_usbmidi_input_start(&umidi->list); }
-static void substream_open(struct snd_rawmidi_substream *substream, int open) +static int substream_open(struct snd_rawmidi_substream *substream, int open) { struct snd_usb_midi* umidi = substream->rmidi->private_data; struct snd_kcontrol *ctl; + int err = 0;
mutex_lock(&umidi->mutex); - if (open) { - if (umidi->opened++ == 0 && umidi->roland_load_ctl) { + if (open && umidi->opened++ == 0) { + err = usb_autopm_get_interface(umidi->iface); + if (err == -EACCES) + err = 0; + if (!err && umidi->roland_load_ctl) { ctl = umidi->roland_load_ctl; ctl->vd[0].access |= SNDRV_CTL_ELEM_ACCESS_INACTIVE; snd_ctl_notify(umidi->card, SNDRV_CTL_EVENT_MASK_INFO, &ctl->id); update_roland_altsetting(umidi); } - } else { - if (--umidi->opened == 0 && umidi->roland_load_ctl) { + } else if (!open && --umidi->opened == 0) { + if (umidi->roland_load_ctl) { ctl = umidi->roland_load_ctl; ctl->vd[0].access &= ~SNDRV_CTL_ELEM_ACCESS_INACTIVE; snd_ctl_notify(umidi->card, SNDRV_CTL_EVENT_MASK_INFO, &ctl->id); } + usb_autopm_put_interface(umidi->iface); } mutex_unlock(&umidi->mutex); + return err; }
static int snd_usbmidi_output_open(struct snd_rawmidi_substream *substream) @@ -1076,25 +1081,17 @@ static int snd_usbmidi_output_open(struct snd_rawmidi_substream *substream) snd_BUG(); return -ENXIO; } - err = usb_autopm_get_interface(umidi->iface); - port->autopm_reference = err >= 0; - if (err < 0 && err != -EACCES) - return -EIO; + err = substream_open(substream, 1); + if (err < 0) + return err; substream->runtime->private_data = port; port->state = STATE_UNKNOWN; - substream_open(substream, 1); return 0; }
static int snd_usbmidi_output_close(struct snd_rawmidi_substream *substream) { - struct snd_usb_midi* umidi = substream->rmidi->private_data; - struct usbmidi_out_port *port = substream->runtime->private_data; - - substream_open(substream, 0); - if (port->autopm_reference) - usb_autopm_put_interface(umidi->iface); - return 0; + return substream_open(substream, 0); }
static void snd_usbmidi_output_trigger(struct snd_rawmidi_substream *substream, int up) @@ -1147,14 +1144,12 @@ static void snd_usbmidi_output_drain(struct snd_rawmidi_substream *substream)
static int snd_usbmidi_input_open(struct snd_rawmidi_substream *substream) { - substream_open(substream, 1); - return 0; + return substream_open(substream, 1); }
static int snd_usbmidi_input_close(struct snd_rawmidi_substream *substream) { - substream_open(substream, 0); - return 0; + return substream_open(substream, 0); }
static void snd_usbmidi_input_trigger(struct snd_rawmidi_substream *substream, int up)
Takashi Iwai wrote:> At Sun, 25 Nov 2012 23:01:27 +0100,
Clemens Ladisch wrote:
Jonathan Nieder wrote:
Some USB MIDI keyboards fail to operate after a USB autosuspend.
Make that *all* USB MIDI devices with input ports.
This is not a bug in the device, but one of the many bugs introduced with the autosuspend code in http://git.kernel.org/linus/88a8516a2128.
That patch does not handle input at all, i.e., when the driver wants to read from the device, it just doesn't take it out of suspend mode.
A workaround is to disable USB autosuspend for these devices by putting AUTOSUSPEND_USBID_BLACKLIST="0763:2027" (resp. 0763:019b) in /etc/laptop-mode/conf.d/usb-autosuspend.conf. In the spirit of commit 166cb70e97bd ("usb: add USB_QUIRK_RESET_RESUME for M-Audio 88es"), reset the device on resume so this workaround is not needed any more.
It is not feasible to add the IDs of all USB MIDI devices.
I'm working on a fix that adds proper power management for input ports, but this requires the driver to be reorganized a little ...
Doesn't a simple patch like below work?
+static int substream_open(struct snd_rawmidi_substream *substream, int open) {
- if (open && umidi->opened++ == 0) {
err = usb_autopm_get_interface(umidi->iface);static int snd_usbmidi_input_open(struct snd_rawmidi_substream *substream) {
- return substream_open(substream, 1);
No, because the input URBs are submitted before the userspace device is opened. (And usb_autopm_get_interface() cannot be called from the USB probe callback.)
Regards, Clemens
At Mon, 26 Nov 2012 13:35:31 +0100, Clemens Ladisch wrote:
Takashi Iwai wrote:> At Sun, 25 Nov 2012 23:01:27 +0100,
Clemens Ladisch wrote:
Jonathan Nieder wrote:
Some USB MIDI keyboards fail to operate after a USB autosuspend.
Make that *all* USB MIDI devices with input ports.
This is not a bug in the device, but one of the many bugs introduced with the autosuspend code in http://git.kernel.org/linus/88a8516a2128.
That patch does not handle input at all, i.e., when the driver wants to read from the device, it just doesn't take it out of suspend mode.
A workaround is to disable USB autosuspend for these devices by putting AUTOSUSPEND_USBID_BLACKLIST="0763:2027" (resp. 0763:019b) in /etc/laptop-mode/conf.d/usb-autosuspend.conf. In the spirit of commit 166cb70e97bd ("usb: add USB_QUIRK_RESET_RESUME for M-Audio 88es"), reset the device on resume so this workaround is not needed any more.
It is not feasible to add the IDs of all USB MIDI devices.
I'm working on a fix that adds proper power management for input ports, but this requires the driver to be reorganized a little ...
Doesn't a simple patch like below work?
+static int substream_open(struct snd_rawmidi_substream *substream, int open) {
- if (open && umidi->opened++ == 0) {
err = usb_autopm_get_interface(umidi->iface);static int snd_usbmidi_input_open(struct snd_rawmidi_substream *substream) {
- return substream_open(substream, 1);
No, because the input URBs are submitted before the userspace device is opened. (And usb_autopm_get_interface() cannot be called from the USB probe callback.)
Ah, right. What's the reason of submitting input urbs for the all time from the beginning? For loopback?
If it has to be running, the easiest fix would be the patch like below. This will turn off the autopm essentially, but better than breakage.
Takashi
--- diff --git a/sound/usb/midi.c b/sound/usb/midi.c index eeefbce..66acccb 100644 --- a/sound/usb/midi.c +++ b/sound/usb/midi.c @@ -175,6 +175,7 @@ struct snd_usb_midi_in_endpoint { u8 seen_f5; u8 error_resubmit; int current_port; + bool autopm_reference; };
static void snd_usbmidi_do_output(struct snd_usb_midi_out_endpoint* ep); @@ -2065,6 +2066,8 @@ void snd_usbmidi_input_stop(struct list_head* p) if (ep->in) for (j = 0; j < INPUT_URBS; ++j) usb_kill_urb(ep->in->urbs[j]); + if (ep->autopm_reference) + usb_autopm_put_interface(umidi->iface); } }
@@ -2074,6 +2077,8 @@ static void snd_usbmidi_input_start_ep(struct snd_usb_midi_in_endpoint* ep)
if (!ep) return; + ep->autopm_reference = + usb_autopm_get_interface(ep->umidi->iface) >= 0; for (i = 0; i < INPUT_URBS; ++i) { struct urb* urb = ep->urbs[i]; urb->dev = ep->umidi->dev;
Takashi Iwai wrote:
Clemens Ladisch wrote:
Takashi Iwai wrote:
Clemens Ladisch wrote:
I'm working on a fix that adds proper power management for input ports, but this requires the driver to be reorganized a little ...
Doesn't a simple patch like below work?
+static int substream_open(struct snd_rawmidi_substream *substream, int open) {
- if (open && umidi->opened++ == 0) {
err = usb_autopm_get_interface(umidi->iface);static int snd_usbmidi_input_open(struct snd_rawmidi_substream *substream) {
- return substream_open(substream, 1);
No, because the input URBs are submitted before the userspace device is opened.
Ah, right. What's the reason of submitting input urbs for the all time from the beginning? For loopback?
For not needing to count open input ports.
If it has to be running, the easiest fix would be the patch like below. This will turn off the autopm essentially, but better than breakage.
@@ -2074,6 +2077,8 @@ static void snd_usbmidi_input_start_ep(struct snd_usb_midi_in_endpoint* ep)
- ep->autopm_reference =
usb_autopm_get_interface(ep->umidi->iface) >= 0;
usb_autopm_get_interface() cannot be called from the USB probe callback.
Regards, Clemens
On Monday 26 November 2012 14:43:13 Clemens Ladisch wrote:
If it has to be running, the easiest fix would be the patch like below. This will turn off the autopm essentially, but better than breakage.
@@ -2074,6 +2077,8 @@ static void snd_usbmidi_input_start_ep(struct snd_usb_midi_in_endpoint* ep)
- ep->autopm_reference =
usb_autopm_get_interface(ep->umidi->iface) >= 0;usb_autopm_get_interface() cannot be called from the USB probe callback.
You can use usb_autopm_get_interface_no_resume() During probe() the device is known to not be suspended.
Regards Oliver
At Mon, 26 Nov 2012 14:43:13 +0100, Clemens Ladisch wrote:
Takashi Iwai wrote:
Clemens Ladisch wrote:
Takashi Iwai wrote:
Clemens Ladisch wrote:
I'm working on a fix that adds proper power management for input ports, but this requires the driver to be reorganized a little ...
Doesn't a simple patch like below work?
+static int substream_open(struct snd_rawmidi_substream *substream, int open) {
- if (open && umidi->opened++ == 0) {
err = usb_autopm_get_interface(umidi->iface);static int snd_usbmidi_input_open(struct snd_rawmidi_substream *substream) {
- return substream_open(substream, 1);
No, because the input URBs are submitted before the userspace device is opened.
Ah, right. What's the reason of submitting input urbs for the all time from the beginning? For loopback?
For not needing to count open input ports.
So we've spun urbs for all the time just for reducing refcount? That's bad.
If it has to be running, the easiest fix would be the patch like below. This will turn off the autopm essentially, but better than breakage.
@@ -2074,6 +2077,8 @@ static void snd_usbmidi_input_start_ep(struct snd_usb_midi_in_endpoint* ep)
- ep->autopm_reference =
usb_autopm_get_interface(ep->umidi->iface) >= 0;usb_autopm_get_interface() cannot be called from the USB probe callback.
OK, it can call no_resume version by passing an argument.
But judging from your comment above, we should fix the free wheel MIDI input urbs at first indeed, not only for autopm but in general.
Takashi
participants (4)
-
Clemens Ladisch -
Jonathan Nieder -
Oliver Neukum -
Takashi Iwai