[PATCH] ALSA: usb-audio: disable autopm for MIDI devices

[PATCH] ALSA: usb-audio: disable autopm for MIDI devices

[Clemens Ladisch]

Commit 88a8516a2128 (ALSA: usbaudio: implement USB autosuspend)
introduced autopm for all USB audio/MIDI devices.  However, many MIDI
devices, such as synthesizers, do not merely transmit MIDI messages but
use their MIDI inputs to control other functions.  With autopm, these
devices would get powered down as soon as the last MIDI port device is
closed on the host.

Even some plain MIDI interfaces could get broken: they automatically
send Active Sensing messages while powered up, but as soon as these
messages cease, the receiving device would interpret this as an
accidental disconnection.

Commit f5f165418cab (ALSA: usb-audio: Fix missing autopm for MIDI input)
introduced another regression: some devices (e.g. the Roland GAIA SH-01)
are self-powered but do a reset whenever the USB interface's power state
changes.

To work around all this, just disable autopm for all USB MIDI devices.

Reported-by: Laurens Holst
Cc: <stable@vger.kernel.org>
Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
---
 sound/usb/midi.c |   13 ++-----------
 1 file changed, 2 insertions(+), 11 deletions(-)

diff --git a/sound/usb/midi.c b/sound/usb/midi.c
index 34b9bb7..e5fee18 100644
--- a/sound/usb/midi.c
+++ b/sound/usb/midi.c
@@ -126,7 +126,6 @@ struct snd_usb_midi {
 		struct snd_usb_midi_in_endpoint *in;
 	} endpoints[MIDI_MAX_ENDPOINTS];
 	unsigned long input_triggered;
-	bool autopm_reference;
 	unsigned int opened[2];
 	unsigned char disconnected;
 	unsigned char input_running;
@@ -1040,7 +1039,6 @@ static int substream_open(struct snd_rawmidi_substream *substream, int dir,
 {
 	struct snd_usb_midi* umidi = substream->rmidi->private_data;
 	struct snd_kcontrol *ctl;
-	int err;

 	down_read(&umidi->disc_rwsem);
 	if (umidi->disconnected) {
@@ -1051,13 +1049,6 @@ static int substream_open(struct snd_rawmidi_substream *substream, int dir,
 	mutex_lock(&umidi->mutex);
 	if (open) {
 		if (!umidi->opened[0] && !umidi->opened[1]) {
-			err = usb_autopm_get_interface(umidi->iface);
-			umidi->autopm_reference = err >= 0;
-			if (err < 0 && err != -EACCES) {
-				mutex_unlock(&umidi->mutex);
-				up_read(&umidi->disc_rwsem);
-				return -EIO;
-			}
 			if (umidi->roland_load_ctl) {
 				ctl = umidi->roland_load_ctl;
 				ctl->vd[0].access |= SNDRV_CTL_ELEM_ACCESS_INACTIVE;
@@ -1080,8 +1071,6 @@ static int substream_open(struct snd_rawmidi_substream *substream, int dir,
 				snd_ctl_notify(umidi->card,
 				       SNDRV_CTL_EVENT_MASK_INFO, &ctl->id);
 			}
-			if (umidi->autopm_reference)
-				usb_autopm_put_interface(umidi->iface);
 		}
 	}
 	mutex_unlock(&umidi->mutex);
@@ -2256,6 +2245,8 @@ int snd_usbmidi_create(struct snd_card *card,
 		return err;
 	}

+	usb_autopm_get_interface_no_resume(umidi->iface);
+
 	list_add_tail(&umidi->list, midi_list);
 	return 0;
 }

Re: [PATCH] ALSA: usb-audio: disable autopm for MIDI devices

[Takashi Iwai]

At Mon, 15 Apr 2013 15:59:51 +0200,
Clemens Ladisch wrote:
> 
> Commit 88a8516a2128 (ALSA: usbaudio: implement USB autosuspend)
> introduced autopm for all USB audio/MIDI devices.  However, many MIDI
> devices, such as synthesizers, do not merely transmit MIDI messages but
> use their MIDI inputs to control other functions.  With autopm, these
> devices would get powered down as soon as the last MIDI port device is
> closed on the host.
> 
> Even some plain MIDI interfaces could get broken: they automatically
> send Active Sensing messages while powered up, but as soon as these
> messages cease, the receiving device would interpret this as an
> accidental disconnection.
> 
> Commit f5f165418cab (ALSA: usb-audio: Fix missing autopm for MIDI input)
> introduced another regression: some devices (e.g. the Roland GAIA SH-01)
> are self-powered but do a reset whenever the USB interface's power state
> changes.
> 
> To work around all this, just disable autopm for all USB MIDI devices.
> 
> Reported-by: Laurens Holst
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Clemens Ladisch <clemens@ladisch.de>

Thanks, applied now, but queued for 3.10 since this is no new
regression in 3.9, and we're so close to 3.9 release.


Takashi


> ---
>  sound/usb/midi.c |   13 ++-----------
>  1 file changed, 2 insertions(+), 11 deletions(-)
> 
> diff --git a/sound/usb/midi.c b/sound/usb/midi.c
> index 34b9bb7..e5fee18 100644
> --- a/sound/usb/midi.c
> +++ b/sound/usb/midi.c
> @@ -126,7 +126,6 @@ struct snd_usb_midi {
>  		struct snd_usb_midi_in_endpoint *in;
>  	} endpoints[MIDI_MAX_ENDPOINTS];
>  	unsigned long input_triggered;
> -	bool autopm_reference;
>  	unsigned int opened[2];
>  	unsigned char disconnected;
>  	unsigned char input_running;
> @@ -1040,7 +1039,6 @@ static int substream_open(struct snd_rawmidi_substream *substream, int dir,
>  {
>  	struct snd_usb_midi* umidi = substream->rmidi->private_data;
>  	struct snd_kcontrol *ctl;
> -	int err;
> 
>  	down_read(&umidi->disc_rwsem);
>  	if (umidi->disconnected) {
> @@ -1051,13 +1049,6 @@ static int substream_open(struct snd_rawmidi_substream *substream, int dir,
>  	mutex_lock(&umidi->mutex);
>  	if (open) {
>  		if (!umidi->opened[0] && !umidi->opened[1]) {
> -			err = usb_autopm_get_interface(umidi->iface);
> -			umidi->autopm_reference = err >= 0;
> -			if (err < 0 && err != -EACCES) {
> -				mutex_unlock(&umidi->mutex);
> -				up_read(&umidi->disc_rwsem);
> -				return -EIO;
> -			}
>  			if (umidi->roland_load_ctl) {
>  				ctl = umidi->roland_load_ctl;
>  				ctl->vd[0].access |= SNDRV_CTL_ELEM_ACCESS_INACTIVE;
> @@ -1080,8 +1071,6 @@ static int substream_open(struct snd_rawmidi_substream *substream, int dir,
>  				snd_ctl_notify(umidi->card,
>  				       SNDRV_CTL_EVENT_MASK_INFO, &ctl->id);
>  			}
> -			if (umidi->autopm_reference)
> -				usb_autopm_put_interface(umidi->iface);
>  		}
>  	}
>  	mutex_unlock(&umidi->mutex);
> @@ -2256,6 +2245,8 @@ int snd_usbmidi_create(struct snd_card *card,
>  		return err;
>  	}
> 
> +	usb_autopm_get_interface_no_resume(umidi->iface);
> +
>  	list_add_tail(&umidi->list, midi_list);
>  	return 0;
>  }
>