Hi, this is autosuspend for HID devices. It uses the new last_busy facility for USB devices. And for a few functions the pm counter.
The main problem implementing this is LEDs. HID has the very though property of initiating output to them without user space's involvement from interrupt context to all attached devices. Doing this with suspended devices is problematic. Output requests now go through different code paths depending on whether the device they are intended for is asleep or not. In the suspended case they are merely queued and the queue processed on resume. Upon every input recieved, whether faulty or not, the device is marked busy. The race with output requests is handled with a spinlock. The suspend handler will report a failure if autosuspend wins the race against output, thus solving the problem. Some races opened by sleeping in the open path were fixed with a mutex. Tested: - basic functionality - devices with output devices (a keyboard) - multiple devices - devices without remote wakeup - suspend to ram known limitations: - will crash with PID devices - too many queued requests should resume - they don't, cause unknown needed testing: - stress testing - this is the only USB driver whose suspend() method is designed to fail at times. This code path is nearly untested - exotic devices - hiddev - reading from a joystick - suspend to disk - cooperation with input devices on other busses (eg. bluetooth) Any comment on the code is appreciated (though I know it doesn't yet conform to coding style) Blast away Oliver ------ --- linux-2.6.22-rc1/drivers/hid/usbhid/usbhid.h 2007-05-13 03:45:56.000000000 +0200 +++ linux-2.6.22-rc1-autosuspend/drivers/hid/usbhid/usbhid.h 2007-05-15 09:53:12.000000000 +0200 @@ -63,21 +63,24 @@ struct usbhid_device { unsigned char ctrlhead, ctrltail; /* Control fifo head & tail */ char *ctrlbuf; /* Control buffer */ dma_addr_t ctrlbuf_dma; /* Control buffer dma */ - spinlock_t ctrllock; /* Control fifo spinlock */ struct urb *urbout; /* Output URB */ struct hid_report *out[HID_CONTROL_FIFO_SIZE]; /* Output pipe fifo */ unsigned char outhead, outtail; /* Output pipe fifo head & tail */ char *outbuf; /* Output buffer */ dma_addr_t outbuf_dma; /* Output buffer dma */ - spinlock_t outlock; /* Output fifo spinlock */ - unsigned long iofl; /* I/O flags (CTRL_RUNNING, OUT_RUNNING) */ - struct timer_list io_retry; /* Retry timer */ - unsigned long stop_retry; /* Time to give up, in jiffies */ - unsigned int retry_delay; /* Delay length in ms */ - struct work_struct reset_work; /* Task context for resets */ + spinlock_t outputlock; /* Output fifo spinlock */ + unsigned long iofl; /* I/O flags (CTRL_RUNNING, OUT_RUNNING) */ + struct timer_list io_retry; /* Retry timer */ + struct timer_list idle_timer; /* timer to determine idleness */ + unsigned long stop_retry; /* Time to give up, in jiffies */ + unsigned int idle_time; /* Time to determine idleness, in seconds */ + unsigned int retry_delay; /* Delay length in ms */ + struct work_struct reset_work; /* Task context for resets */ + struct work_struct idle_work; /* Task context for reporting idleness */ + struct work_struct restart_work; /* waking up for output to be done in task context */ }; #define hid_to_usb_dev(hid_dev) \ --- linux-2.6.22-rc1/drivers/hid/usbhid/hid-core.c 2007-05-13 03:45:56.000000000 +0200 +++ linux-2.6.22-rc1-autosuspend/drivers/hid/usbhid/hid-core.c 2007-05-15 08:57:38.000000000 +0200 @@ -5,6 +5,7 @@ * Copyright (c) 2000-2005 Vojtech Pavlik <[EMAIL PROTECTED]> * Copyright (c) 2005 Michael Haboustak <[EMAIL PROTECTED]> for Concept2, Inc * Copyright (c) 2006-2007 Jiri Kosina + * Copyright (c) 2007 Oliver Neukum */ /* @@ -63,8 +64,11 @@ MODULE_PARM_DESC(quirks, "Add/modify USB /* * Input submission and I/O error handler. */ +static DEFINE_MUTEX(hid_open_mut); static void hid_io_error(struct hid_device *hid); +static int hid_submit_out(struct hid_device *hid); +static int hid_submit_ctrl(struct hid_device *hid); /* Start up the input URB */ static int hid_start_in(struct hid_device *hid) @@ -74,7 +78,7 @@ static int hid_start_in(struct hid_devic struct usbhid_device *usbhid = hid->driver_data; spin_lock_irqsave(&usbhid->inlock, flags); - if (hid->open > 0 && !test_bit(HID_SUSPENDED, &usbhid->iofl) && + if (hid->open > 0 && !test_bit(HID_REPORTED_IDLE, &usbhid->iofl) && !test_and_set_bit(HID_IN_RUNNING, &usbhid->iofl)) { rc = usb_submit_urb(usbhid->urbin, GFP_ATOMIC); if (rc != 0) @@ -178,6 +182,51 @@ done: spin_unlock_irqrestore(&usbhid->inlock, flags); } +static void usbhid_mark_busy(struct usbhid_device *usbhid) +{ + struct usb_interface *intf = usbhid->intf; + + usb_mark_last_busy(interface_to_usbdev(intf)); +} + +static int usbhid_restart_out_queue(struct usbhid_device *usbhid) +{ + struct hid_device *hid = usb_get_intfdata(usbhid->intf); + int kicked; + + WARN_ON(hid == NULL); + if (!hid) + return 0; + + if ((kicked = (usbhid->outhead != usbhid->outtail))) { + dbg("Kicking head %d tail %d", usbhid->outhead, usbhid->outtail); + if (hid_submit_out(hid)) { + clear_bit(HID_OUT_RUNNING, &usbhid->iofl); + wake_up(&hid->wait); + } + } + return kicked; +} + +static int usbhid_restart_ctrl_queue(struct usbhid_device *usbhid) +{ + struct hid_device *hid = usb_get_intfdata(usbhid->intf); + int kicked; + + WARN_ON(hid == NULL); + if (!hid) + return 0; + + if ((kicked = (usbhid->ctrlhead != usbhid->ctrltail))) { + dbg("Kicking head %d tail %d", usbhid->ctrlhead, usbhid->ctrltail); + if (hid_submit_ctrl(hid)) { + clear_bit(HID_CTRL_RUNNING, &usbhid->iofl); + wake_up(&hid->wait); + } + } + return kicked; +} + /* * Input interrupt completion handler. */ @@ -190,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); @@ -209,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; @@ -234,9 +286,11 @@ static int hid_submit_out(struct hid_dev struct hid_report *report; struct usbhid_device *usbhid = hid->driver_data; + WARN_ON(usbhid == NULL); report = usbhid->out[usbhid->outtail]; - + WARN_ON(report == NULL); hid_output_report(report, usbhid->outbuf); + BUG_ON(usbhid->urbout == NULL); usbhid->urbout->transfer_buffer_length = ((report->size - 1) >> 3) + 1 + (report->id > 0); usbhid->urbout->dev = hid_to_usb_dev(hid); @@ -324,24 +378,20 @@ static void hid_irq_out(struct urb *urb) warn("output irq status %d received", urb->status); } - spin_lock_irqsave(&usbhid->outlock, flags); + spin_lock_irqsave(&usbhid->outputlock, flags); if (unplug) usbhid->outtail = usbhid->outhead; else usbhid->outtail = (usbhid->outtail + 1) & (HID_OUTPUT_FIFO_SIZE - 1); - if (usbhid->outhead != usbhid->outtail) { - if (hid_submit_out(hid)) { - clear_bit(HID_OUT_RUNNING, &usbhid->iofl); - wake_up(&hid->wait); - } - spin_unlock_irqrestore(&usbhid->outlock, flags); + if (usbhid_restart_out_queue(usbhid)) { + spin_unlock_irqrestore(&usbhid->outputlock, flags); return; } clear_bit(HID_OUT_RUNNING, &usbhid->iofl); - spin_unlock_irqrestore(&usbhid->outlock, flags); + spin_unlock_irqrestore(&usbhid->outputlock, flags); wake_up(&hid->wait); } @@ -356,7 +406,7 @@ static void hid_ctrl(struct urb *urb) unsigned long flags; int unplug = 0; - spin_lock_irqsave(&usbhid->ctrllock, flags); + spin_lock_irqsave(&usbhid->outputlock, flags); switch (urb->status) { case 0: /* success */ @@ -381,24 +431,55 @@ static void hid_ctrl(struct urb *urb) else usbhid->ctrltail = (usbhid->ctrltail + 1) & (HID_CONTROL_FIFO_SIZE - 1); - if (usbhid->ctrlhead != usbhid->ctrltail) { - if (hid_submit_ctrl(hid)) { - clear_bit(HID_CTRL_RUNNING, &usbhid->iofl); - wake_up(&hid->wait); - } - spin_unlock_irqrestore(&usbhid->ctrllock, flags); + if (usbhid_restart_ctrl_queue(usbhid)) { + spin_unlock_irqrestore(&usbhid->outputlock, flags); return; } clear_bit(HID_CTRL_RUNNING, &usbhid->iofl); - spin_unlock_irqrestore(&usbhid->ctrllock, flags); + spin_unlock_irqrestore(&usbhid->outputlock, flags); wake_up(&hid->wait); } -void usbhid_submit_report(struct hid_device *hid, struct hid_report *report, unsigned char dir) +static int usbhid_queue_report_out(struct hid_device *hid, struct hid_report *report) { + struct usbhid_device *usbhid = hid->driver_data; int head; - unsigned long flags; + + if ((head = (usbhid->outhead + 1) & (HID_OUTPUT_FIFO_SIZE - 1)) == usbhid->outtail) { + warn("output queue full"); + return 0; + } + + usbhid->out[usbhid->outhead] = report; + usbhid->outhead = head; + + return usbhid->outhead < usbhid->outtail ? + usbhid->outtail - usbhid->outhead : + HID_OUTPUT_FIFO_SIZE - usbhid->outhead + usbhid->outtail; +} + +static int usbhid_queue_report_ctrl(struct hid_device *hid, struct hid_report *report, unsigned char dir) +{ + struct usbhid_device *usbhid = hid->driver_data; + int head; + + if ((head = (usbhid->ctrlhead + 1) & (HID_CONTROL_FIFO_SIZE - 1)) == usbhid->ctrltail) { + warn("control queue full"); + return 0; + } + + usbhid->ctrl[usbhid->ctrlhead].report = report; + usbhid->ctrl[usbhid->ctrlhead].dir = dir; + usbhid->ctrlhead = head; + + return usbhid->ctrlhead < usbhid->ctrltail ? + usbhid->ctrltail - usbhid->ctrlhead : + HID_CONTROL_FIFO_SIZE - usbhid->ctrlhead + usbhid->ctrltail; +} + +static void __usbhid_submit_report(struct hid_device *hid, struct hid_report *report, unsigned char dir) +{ struct usbhid_device *usbhid = hid->driver_data; if ((hid->quirks & HID_QUIRK_NOGET) && dir == USB_DIR_IN) @@ -406,49 +487,53 @@ void usbhid_submit_report(struct hid_dev if (usbhid->urbout && dir == USB_DIR_OUT && report->type == HID_OUTPUT_REPORT) { - spin_lock_irqsave(&usbhid->outlock, flags); - - if ((head = (usbhid->outhead + 1) & (HID_OUTPUT_FIFO_SIZE - 1)) == usbhid->outtail) { - spin_unlock_irqrestore(&usbhid->outlock, flags); - warn("output queue full"); - return; - } - - usbhid->out[usbhid->outhead] = report; - usbhid->outhead = head; + usbhid_queue_report_out(hid, report); if (!test_and_set_bit(HID_OUT_RUNNING, &usbhid->iofl)) if (hid_submit_out(hid)) clear_bit(HID_OUT_RUNNING, &usbhid->iofl); - spin_unlock_irqrestore(&usbhid->outlock, flags); - return; - } + } else { - spin_lock_irqsave(&usbhid->ctrllock, flags); + usbhid_queue_report_ctrl(hid, report, dir); + + if (!test_and_set_bit(HID_CTRL_RUNNING, &usbhid->iofl)) + if (hid_submit_ctrl(hid)) + clear_bit(HID_CTRL_RUNNING, &usbhid->iofl); - if ((head = (usbhid->ctrlhead + 1) & (HID_CONTROL_FIFO_SIZE - 1)) == usbhid->ctrltail) { - spin_unlock_irqrestore(&usbhid->ctrllock, flags); - warn("control queue full"); - return; } +} - usbhid->ctrl[usbhid->ctrlhead].report = report; - usbhid->ctrl[usbhid->ctrlhead].dir = dir; - usbhid->ctrlhead = head; +void usbhid_submit_report(struct hid_device *hid, struct hid_report *report, unsigned char dir) +{ + struct usbhid_device *usbhid = hid->driver_data; + unsigned long flags; - if (!test_and_set_bit(HID_CTRL_RUNNING, &usbhid->iofl)) - if (hid_submit_ctrl(hid)) - clear_bit(HID_CTRL_RUNNING, &usbhid->iofl); + spin_lock_irqsave(&usbhid->outputlock, flags); + __usbhid_submit_report(hid, report, dir); + spin_unlock_irqrestore(&usbhid->outputlock, flags); +} - spin_unlock_irqrestore(&usbhid->ctrllock, flags); +static int usbhid_queue_report(struct hid_device *hid, struct hid_report *report, unsigned char dir) +{ + struct usbhid_device *usbhid = hid->driver_data; + int rv; + + if (usbhid->urbout && dir == USB_DIR_OUT && report->type == HID_OUTPUT_REPORT) { + rv = usbhid_queue_report_out(hid, report); + } else { + rv = usbhid_queue_report_ctrl(hid, report, dir); + } + return rv; } static int usb_hidinput_input_event(struct input_dev *dev, unsigned int type, unsigned int code, int value) { struct hid_device *hid = input_get_drvdata(dev); + struct usbhid_device *usbhid = hid->driver_data; struct hid_field *field; - int offset; + unsigned long flags; + int offset, used; if (type == EV_FF) return input_ff_event(dev, type, code, value); @@ -461,9 +546,21 @@ static int usb_hidinput_input_event(stru return -1; } + spin_lock_irqsave(&usbhid->outputlock, flags); + hid_set_field(field, offset, value); - usbhid_submit_report(hid, field->report, USB_DIR_OUT); + if (!test_bit(HID_REPORTED_IDLE, &usbhid->iofl)) { + /* the device isn't idle, we do the output now*/ + __usbhid_submit_report(hid, field->report, USB_DIR_OUT); + } else { + /* the device is idle, queue the request + and wake it up if the queue fills up too much*/ + used = usbhid_queue_report(hid, field->report, USB_DIR_OUT); + if ( 3 < HID_OUTPUT_FIFO_SIZE - used) + schedule_work(&usbhid->restart_work); + } + spin_unlock_irqrestore(&usbhid->outputlock, flags); return 0; } @@ -506,7 +603,22 @@ static int hid_get_class_descriptor(stru int usbhid_open(struct hid_device *hid) { - ++hid->open; + struct usbhid_device *usbhid = hid->driver_data; + int res; + + 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; @@ -516,8 +628,13 @@ void usbhid_close(struct hid_device *hid { struct usbhid_device *usbhid = hid->driver_data; - if (!--hid->open) + mutex_lock(&hid_open_mut); + if (!--hid->open) { usb_kill_urb(usbhid->urbin); + flush_scheduled_work(); + usbhid->intf->needs_remote_wakeup = 0; + } + mutex_unlock(&hid_open_mut); } /* @@ -527,14 +644,21 @@ void usbhid_close(struct hid_device *hid void usbhid_init_reports(struct hid_device *hid) { struct hid_report *report; - struct usbhid_device *usbhid = hid->driver_data; + unsigned long flags; int err, ret; + struct usbhid_device *usbhid = hid->driver_data; - list_for_each_entry(report, &hid->report_enum[HID_INPUT_REPORT].report_list, list) - usbhid_submit_report(hid, report, USB_DIR_IN); + list_for_each_entry(report, &hid->report_enum[HID_INPUT_REPORT].report_list, list) { + spin_lock_irqsave(&usbhid->outputlock, flags); + __usbhid_submit_report(hid, report, USB_DIR_IN); + spin_unlock_irqrestore(&usbhid->outputlock, flags); + } - list_for_each_entry(report, &hid->report_enum[HID_FEATURE_REPORT].report_list, list) - usbhid_submit_report(hid, report, USB_DIR_IN); + list_for_each_entry(report, &hid->report_enum[HID_FEATURE_REPORT].report_list, list) { + spin_lock_irqsave(&usbhid->outputlock, flags); + __usbhid_submit_report(hid, report, USB_DIR_IN); + spin_unlock_irqrestore(&usbhid->outputlock, flags); + } err = 0; ret = usbhid_wait_io(hid); @@ -622,6 +746,23 @@ static int hid_alloc_buffers(struct usb_ return 0; } +static void usbhid_restart_queues(struct usbhid_device *usbhid) +{ + if (usbhid->urbout) + usbhid_restart_out_queue(usbhid); + usbhid_restart_ctrl_queue(usbhid); +} + +static void __usbhid_restart_queues(struct work_struct *work) +{ + struct usbhid_device *usbhid = + container_of(work, struct usbhid_device, restart_work); + + usb_autopm_get_interface(usbhid->intf); + usbhid_restart_queues(usbhid); + usb_autopm_put_interface(usbhid->intf); +} + static void hid_free_buffers(struct usb_device *dev, struct hid_device *hid) { struct usbhid_device *usbhid = hid->driver_data; @@ -868,17 +1009,18 @@ static struct hid_device *usb_hid_config init_waitqueue_head(&hid->wait); INIT_WORK(&usbhid->reset_work, hid_reset); + INIT_WORK(&usbhid->restart_work, __usbhid_restart_queues); setup_timer(&usbhid->io_retry, hid_retry_timeout, (unsigned long) hid); spin_lock_init(&usbhid->inlock); - spin_lock_init(&usbhid->outlock); - spin_lock_init(&usbhid->ctrllock); + spin_lock_init(&usbhid->outputlock); hid->version = le16_to_cpu(hdesc->bcdHID); hid->country = hdesc->bCountryCode; hid->dev = &intf->dev; usbhid->intf = intf; usbhid->ifnum = interface->desc.bInterfaceNumber; + usbhid->idle_time = 10; //FIXME hid->name[0] = 0; @@ -984,6 +1126,8 @@ static int hid_probe(struct usb_interfac if (!(hid = usb_hid_configure(intf))) return -ENODEV; + usb_set_intfdata(intf, hid); + usbhid_init_reports(hid); hid_dump_device(hid); if (hid->quirks & HID_QUIRK_RESET_LEDS) @@ -994,7 +1138,6 @@ static int hid_probe(struct usb_interfac if (!hiddev_connect(hid)) hid->claimed |= HID_CLAIMED_HIDDEV; - usb_set_intfdata(intf, hid); if (!hid->claimed) { printk ("HID device not claimed by input or hiddev\n"); @@ -1038,14 +1181,24 @@ static int hid_probe(struct usb_interfac static int hid_suspend(struct usb_interface *intf, pm_message_t message) { - struct hid_device *hid = usb_get_intfdata (intf); + struct hid_device *hid = usb_get_intfdata(intf); struct usbhid_device *usbhid = hid->driver_data; spin_lock_irq(&usbhid->inlock); /* Sync with error handler */ - set_bit(HID_SUSPENDED, &usbhid->iofl); - spin_unlock_irq(&usbhid->inlock); + 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; + } del_timer(&usbhid->io_retry); usb_kill_urb(usbhid->urbin); + flush_scheduled_work(); dev_dbg(&intf->dev, "suspend\n"); return 0; } @@ -1056,9 +1209,11 @@ static int hid_resume(struct usb_interfa struct usbhid_device *usbhid = hid->driver_data; int status; - clear_bit(HID_SUSPENDED, &usbhid->iofl); + clear_bit(HID_REPORTED_IDLE, &usbhid->iofl); + usbhid_mark_busy(usbhid); usbhid->retry_delay = 0; status = hid_start_in(hid); + usbhid_restart_queues(usbhid); dev_dbg(&intf->dev, "resume status %d\n", status); return status; } @@ -1097,6 +1252,7 @@ static struct usb_driver hid_driver = { .pre_reset = hid_pre_reset, .post_reset = hid_post_reset, .id_table = hid_usb_ids, + .supports_autosuspend = 1, }; static int __init hid_init(void) --- linux-2.6.22-rc1/include/linux/hid.h 2007-05-13 03:45:56.000000000 +0200 +++ linux-2.6.22-rc1-autosuspend/include/linux/hid.h 2007-05-15 10:03:12.000000000 +0200 @@ -401,6 +401,7 @@ struct hid_control_fifo { #define HID_RESET_PENDING 4 #define HID_SUSPENDED 5 #define HID_CLEAR_HALT 6 +#define HID_REPORTED_IDLE 7 struct hid_input { struct list_head list; ------------------------------------------------------------------------- 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