[alsa-devel] usb: add USB_QUIRK_RESET_RESUME for M-Audio 49

Commit Message

Takashi Iwai Nov. 26, 2012, 9:35 a.m. UTC

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

---
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Clemens Ladisch Nov. 26, 2012, 12:35 p.m. UTC | #1

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
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

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)