On Wed, 16 May 2007, Oliver Neukum wrote: > Here's part #2, the core autosuspend.
> @@ -230,12 +239,14 @@ static void hid_irq_in(struct urb *urb) > > switch (urb->status) { > case 0: /* success */ > + usbhid_mark_busy(usbhid); > usbhid->retry_delay = 0; > hid_input_report(urb->context, HID_INPUT_REPORT, > urb->transfer_buffer, > urb->actual_length, 1); > break; > case -EPIPE: /* stall */ > + usbhid_mark_busy(usbhid); > clear_bit(HID_IN_RUNNING, &usbhid->iofl); > set_bit(HID_CLEAR_HALT, &usbhid->iofl); > schedule_work(&usbhid->reset_work); > @@ -249,6 +260,7 @@ static void hid_irq_in(struct urb *urb) > case -EPROTO: /* protocol error or unplug */ > case -ETIME: /* protocol error or unplug */ > case -ETIMEDOUT: /* Should never happen, but... */ > + usbhid_mark_busy(usbhid); > clear_bit(HID_IN_RUNNING, &usbhid->iofl); > hid_io_error(hid); > return; At first I thought it would be easier to call usbhid_mark_busy() at every URB completion. But obviously you don't want to do it when an URB is unlinked. > @@ -592,8 +604,21 @@ static int hid_get_class_descriptor(stru > int usbhid_open(struct hid_device *hid) > { > struct usbhid_device *usbhid = hid->driver_data; > + int res; > > - hid->open++; > + mutex_lock(&hid_open_mut); > + if (!hid->open++) { > + res = usb_autopm_get_interface(usbhid->intf); > + /* the device must be awake to reliable request remote wakeup */ > + if (res < 0) { > + hid->open--; > + mutex_unlock(&hid_open_mut); > + return -EIO; > + } > + usbhid->intf->needs_remote_wakeup = 1; > + usb_autopm_put_interface(usbhid->intf); > + } > + mutex_unlock(&hid_open_mut); > if (hid_start_in(hid)) > hid_io_error(hid); > return 0; Don't you need to do usb_autopm_get_interface() before hid_start_in()? > @@ -1154,13 +1184,26 @@ static int hid_suspend(struct usb_interf > struct usbhid_device *usbhid = hid->driver_data; > struct usb_device *udev = interface_to_usbdev(intf); > > - > - spin_lock_irq(&usbhid->inlock); > - set_bit(HID_REPORTED_IDLE, &usbhid->iofl); > - spin_unlock_irq(&usbhid->inlock); > - if (usbhid_wait_io(hid) < 0) > - return -EIO; > - > + if (udev->auto_pm) { > + spin_lock_irq(&usbhid->inlock); /* Sync with error handler */ > + if (!test_bit(HID_RESET_PENDING, &usbhid->iofl) > + && !test_bit(HID_CLEAR_HALT, &usbhid->iofl) > + && !test_bit(HID_OUT_RUNNING, &usbhid->iofl) > + && !test_bit(HID_CTRL_RUNNING, &usbhid->iofl)) > + { > + set_bit(HID_REPORTED_IDLE, &usbhid->iofl); > + spin_unlock_irq(&usbhid->inlock); > + } else { > + spin_unlock_irq(&usbhid->inlock); > + return -EBUSY; > + } > + } else { > + spin_lock_irq(&usbhid->inlock); > + set_bit(HID_REPORTED_IDLE, &usbhid->iofl); > + spin_unlock_irq(&usbhid->inlock); > + if (usbhid_wait_io(hid) < 0) > + return -EIO; > + } > del_timer(&usbhid->io_retry); > usb_kill_urb(usbhid->urbin); > flush_scheduled_work(); This might be simpler if you inverted the order of the tests. That is, first get the spinlock, then test for udev->auto_pm && (test_bit() || test_bit() || ...) to see if you need to fail right away. You probably don't even need to check udev->auto_pm again before calling usbhid_wait_io. > @@ -1175,6 +1218,7 @@ static int hid_resume(struct usb_interfa > int status; > > clear_bit(HID_REPORTED_IDLE, &usbhid->iofl); > + usbhid_mark_busy(usbhid); > usbhid->retry_delay = 0; > status = hid_start_in(hid); > usbhid_restart_queues(usbhid); Again, don't you need to call usb_autopm_get_interface() before hid_start_in()? One last thing, about the race between a last-minute URB completion and autosuspend. The USB autosuspend core routine doesn't check the last_busy value after suspending the interface drivers and before suspending the device. Do we need to? Alan Stern ------------------------------------------------------------------------- This SF.net email is sponsored by DB2 Express Download DB2 Express C - the FREE version of DB2 express and take control of your XML. No limits. Just data. Click to get it now. http://sourceforge.net/powerbar/db2/ _______________________________________________ linux-usb-devel@lists.sourceforge.net To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel