Am Freitag, 1. Februar 2008 16:10:45 schrieb Alan Stern:
> On Fri, 1 Feb 2008, Oliver Neukum wrote:
>
> > Hi,
> >
> > this patch implements autosuspend for USB audio devices.
>
> > @@ -1937,6 +1945,7 @@ static int snd_usb_pcm_close(struct snd_pcm_substream
> > *substream, int direction)
> > subs->interface = -1;
> > }
> > subs->pcm_substream = NULL;
> > + usb_autopm_put_interface(subs->stream->chip->pm_intf);
> > return 0;
> > }
>
> You need to be very careful here. It's illegal to call
> usb_autopm_put_interface() after the disconnect method has returned.
Please apply this additional patch and retest.
Brandon, does this address the issues with locking you saw?
Regards
Oliver
----
commit 8c94621b58984b936f5051a7ea40030658e8d0ea
Author: Oliver Neukum <[EMAIL PROTECTED]>
Date: Fri Feb 1 18:11:29 2008 +0100
locking fix for autosuspend
diff --git a/sound/usb/usbaudio.c b/sound/usb/usbaudio.c
index cdd9c36..d6df063 100644
--- a/sound/usb/usbaudio.c
+++ b/sound/usb/usbaudio.c
@@ -1879,9 +1879,14 @@ static int setup_hw_info(struct snd_pcm_runtime
*runtime, struct snd_usb_substre
}
}
+ mutex_lock(&subs->stream->chip->shutdown_lock);
+ if (subs->stream->chip->shutdown){
+ mutex_unlock(&subs->stream->chip->shutdown_lock);
+ return -ENODEV;
+ }
err = usb_autopm_get_interface(subs->stream->chip->pm_intf);
if (err < 0)
- return err;
+ goto err_lock;
/* set the period time minimum 1ms */
/* FIXME: high-speed mode allows 125us minimum period, but many parts
@@ -1914,10 +1919,13 @@ static int setup_hw_info(struct snd_pcm_runtime
*runtime, struct snd_usb_substre
if ((err = snd_usb_pcm_check_knot(runtime, subs)) < 0)
goto rep_err;
}
+ mutex_unlock(&subs->stream->chip->shutdown_lock);
return 0;
rep_err:
usb_autopm_put_interface(subs->stream->chip->pm_intf);
+err_lock:
+ mutex_unlock(&subs->stream->chip->shutdown_lock);
return err;
}
@@ -1945,7 +1953,10 @@ static int snd_usb_pcm_close(struct snd_pcm_substream
*substream, int direction)
subs->interface = -1;
}
subs->pcm_substream = NULL;
- usb_autopm_put_interface(subs->stream->chip->pm_intf);
+ mutex_lock(&subs->stream->chip->shutdown_lock);
+ if (!subs->stream->chip->shutdown)
+ usb_autopm_put_interface(subs->stream->chip->pm_intf);
+ mutex_unlock(&subs->stream->chip->shutdown_lock);
return 0;
}
@@ -3436,6 +3447,7 @@ static int snd_usb_audio_create(struct usb_device *dev,
int idx,
INIT_LIST_HEAD(&chip->pcm_list);
INIT_LIST_HEAD(&chip->midi_list);
INIT_LIST_HEAD(&chip->mixer_list);
+ mutex_init(&chip->shutdown_lock);
if ((err = snd_device_new(card, SNDRV_DEV_LOWLEVEL, chip, &ops)) < 0) {
snd_usb_audio_free(chip);
@@ -3630,7 +3642,9 @@ static void snd_usb_audio_disconnect(struct usb_device
*dev, void *ptr)
chip = ptr;
card = chip->card;
mutex_lock(®ister_mutex);
+ mutex_lock(&chip->shutdown_lock);
chip->shutdown = 1;
+ mutex_unlock(&chip->shutdown_lock);
chip->num_interfaces--;
if (chip->num_interfaces <= 0) {
snd_card_disconnect(card);
diff --git a/sound/usb/usbaudio.h b/sound/usb/usbaudio.h
index fbd6bdf..746546e 100644
--- a/sound/usb/usbaudio.h
+++ b/sound/usb/usbaudio.h
@@ -124,6 +124,7 @@ struct snd_usb_audio {
struct usb_device *dev;
struct snd_card *card;
struct usb_interface *pm_intf;
+ struct mutex shutdown_lock;
u32 usb_id;
int shutdown;
int num_interfaces;
diff --git a/sound/usb/usbmixer.c b/sound/usb/usbmixer.c
index cbc612b..b7df8b7 100644
--- a/sound/usb/usbmixer.c
+++ b/sound/usb/usbmixer.c
@@ -354,9 +354,16 @@ static int get_ctl_value(struct usb_mixer_elem_info *cval,
int request, int vali
int timeout = 10;
int err;
+ mutex_lock(&cval->mixer->chip->shutdown_lock);
+ if (cval->mixer->chip->shutdown) {
+ mutex_unlock(&cval->mixer->chip->shutdown_lock);
+ return -ENODEV;
+ }
err = usb_autopm_get_interface(cval->mixer->chip->pm_intf);
- if (err < 0)
+ if (err < 0) {
+ mutex_unlock(&cval->mixer->chip->shutdown_lock);
return -EIO;
+ }
while (timeout-- > 0) {
if (snd_usb_ctl_msg(cval->mixer->chip->dev,
usb_rcvctrlpipe(cval->mixer->chip->dev, 0),
@@ -366,10 +373,12 @@ static int get_ctl_value(struct usb_mixer_elem_info
*cval, int request, int vali
buf, val_len, 100) >= val_len) {
*value_ret = convert_signed_value(cval,
snd_usb_combine_bytes(buf, val_len));
usb_autopm_put_interface(cval->mixer->chip->pm_intf);
+ mutex_unlock(&cval->mixer->chip->shutdown_lock);
return 0;
}
}
usb_autopm_put_interface(cval->mixer->chip->pm_intf);
+ mutex_unlock(&cval->mixer->chip->shutdown_lock);
snd_printdd(KERN_ERR "cannot get ctl value: req = %#x, wValue = %#x,
wIndex = %#x, type = %d\n",
request, validx, cval->mixer->ctrlif | (cval->id << 8),
cval->val_type);
return -EINVAL;
@@ -400,9 +409,12 @@ static int set_ctl_value(struct usb_mixer_elem_info *cval,
int request, int vali
value_set = convert_bytes_value(cval, value_set);
buf[0] = value_set & 0xff;
buf[1] = (value_set >> 8) & 0xff;
+ mutex_lock(&cval->mixer->chip->shutdown_lock);
err = usb_autopm_get_interface(cval->mixer->chip->pm_intf);
- if (err < 0)
+ if (err < 0) {
+ mutex_unlock(&cval->mixer->chip->shutdown_lock);
return -EIO;
+ }
while (timeout -- > 0) {
if (snd_usb_ctl_msg(cval->mixer->chip->dev,
@@ -412,10 +424,12 @@ static int set_ctl_value(struct usb_mixer_elem_info
*cval, int request, int vali
validx, cval->mixer->ctrlif | (cval->id <<
8),
buf, val_len, 100) >= 0) {
usb_autopm_put_interface(cval->mixer->chip->pm_intf);
+ mutex_unlock(&cval->mixer->chip->shutdown_lock);
return 0;
}
}
usb_autopm_put_interface(cval->mixer->chip->pm_intf);
+ mutex_unlock(&cval->mixer->chip->shutdown_lock);
snd_printdd(KERN_ERR "cannot set ctl value: req = %#x, wValue = %#x,
wIndex = %#x, type = %d, data = %#x/%#x\n",
request, validx, cval->mixer->ctrlif | (cval->id << 8),
cval->val_type, buf[0], buf[1]);
return -EINVAL;
@@ -1930,8 +1944,14 @@ static int snd_audigy2nx_led_put(struct snd_kcontrol
*kcontrol, struct snd_ctl_e
goto err_out;
}
changed = value != mixer->audigy2nx_leds[index];
+ mutex_lock(&mixer->chip->shutdown_lock);
+ if (mixer->chip->shutdown) {
+ mutex_unlock(&mixer->chip->shutdown_lock);
+ return -ENODEV;
+ }
err = usb_autopm_get_interface(mixer->chip->pm_intf);
if (err < 0) {
+ mutex_unlock(&mixer->chip->shutdown_lock);
changed = -EIO;
goto err_out;
}
@@ -1946,6 +1966,7 @@ static int snd_audigy2nx_led_put(struct snd_kcontrol
*kcontrol, struct snd_ctl_e
mixer->audigy2nx_leds[index] = value;
err:
usb_autopm_put_interface(mixer->chip->pm_intf);
+ mutex_unlock(&mixer->chip->shutdown_lock);
err_out:
return changed;
}
@@ -2025,9 +2046,16 @@ static void snd_audigy2nx_proc_read(struct
snd_info_entry *entry,
else
return;
+ mutex_lock(&mixer->chip->shutdown_lock);
+ if (mixer->chip->shutdown) {
+ mutex_unlock(&mixer->chip->shutdown_lock);
+ return;
+ }
err = usb_autopm_get_interface(mixer->chip->pm_intf);
- if (err < 0)
+ if (err < 0) {
+ mutex_unlock(&mixer->chip->shutdown_lock);
return;
+ }
for (i = 0; jacks[i].name; ++i) {
snd_iprintf(buffer, "%s: ", jacks[i].name);
@@ -2042,6 +2070,7 @@ static void snd_audigy2nx_proc_read(struct snd_info_entry
*entry,
snd_iprintf(buffer, "?\n");
}
usb_autopm_put_interface(mixer->chip->pm_intf);
+ mutex_unlock(&mixer->chip->shutdown_lock);
}
int snd_usb_create_mixer(struct snd_usb_audio *chip, int ctrlif)
-
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
