| Message ID | s5hwqx8h5w1.wl%tiwai@suse.de (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
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 -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 26 Nov 2012, Oliver Neukum wrote: > 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. In fact, as far as I know you _can_ use usb_autopm_get_interface() from within a probe routine. Is there some problem with it I'm not aware of? Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Alan Stern wrote: > On Mon, 26 Nov 2012, Oliver Neukum wrote: >> On Monday 26 November 2012 14:43:13 Clemens Ladisch wrote: >>> 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. > > In fact, as far as I know you _can_ use usb_autopm_get_interface() from > within a probe routine. Is there some problem with it I'm not aware of? It returns -EACCES. But _no_resume() appears to work just fine. Regards, Clemens -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 26 Nov 2012, Clemens Ladisch wrote: > Alan Stern wrote: > > On Mon, 26 Nov 2012, Oliver Neukum wrote: > >> On Monday 26 November 2012 14:43:13 Clemens Ladisch wrote: > >>> 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. > > > > In fact, as far as I know you _can_ use usb_autopm_get_interface() from > > within a probe routine. Is there some problem with it I'm not aware of? > > It returns -EACCES. That's odd. -EACCES occurs only when the interface or the device it belongs to is disabled for runtime PM. But the USB core enables interfaces before probing, provided the driver's supports_autosuspend flag is set. Furthermore, -EACCES can occur in usb_autopm_get_interface() only when the interface or its device is not currently resumed -- but USB devices and interfaces are always resumed during a probe. Do you have any idea about what's going wrong here? Or do I need to investigate more closely? Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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;