Skip to contentSkip to conversation

hci_usb: implement suspend/resume

PATCH In review Johannes Berg 5 participants 11 messages Jan 18, 2006 – Apr 16, 2006 Bluetooth drivers

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 hci_usb: implement suspend/resume

The attached patch implements suspend/resume for the hci_usb bluetooth driver by simply killing all outstanding urbs on suspend, and re-issuing them on resume.

This allows me to actually use the internal bluetooth "dongle" in my powerbook after suspend-to-ram without taking down all userland programs (sdpd, ...) and the hci device and reloading the module.

drivers/bluetooth/hci_usb.c +44 -0
@@ -1043,11 +1043,55 @@
 	hci_free_dev(hdev);
 }
 
+static int hci_usb_suspend(struct usb_interface *intf, pm_message_t message)
+{
+	struct hci_usb *husb = usb_get_intfdata(intf);
+	int i;
+	unsigned long flags;
+	if (!husb || intf == husb->isoc_iface)
+		return 0;
+	
+	for (i = 0; i < 4; i++) {
+		struct _urb_queue *q = &husb->pending_q[i];
+		struct _urb *_urb;
+		spin_lock_irqsave(&q->lock, flags);
+		list_for_each_entry(_urb, &q->head, list)
+			usb_kill_urb(&_urb->urb);
+		spin_unlock_irqrestore(&q->lock, flags);
+	}
+	return 0;
+}
+
+static int hci_usb_resume(struct usb_interface *intf)
+{
+	struct hci_usb *husb = usb_get_intfdata(intf);
+	int i, err;
+	unsigned long flags;
+	if (!husb || intf == husb->isoc_iface)
+		return 0;
+	
+	for (i = 0; i < 4; i++) {
+		struct _urb_queue *q = &husb->pending_q[i];
+		struct _urb *_urb;
+		spin_lock_irqsave(&q->lock, flags);
+		list_for_each_entry(_urb, &q->head, list) {
+			err = usb_submit_urb(&_urb->urb, GFP_ATOMIC);
+			if (err) break;
+		}
+		spin_unlock_irqrestore(&q->lock, flags);
+		if (err)
+			return -EIO;
+	}
+	return 0;
+}
+
 static struct usb_driver hci_usb_driver = {
 	.owner		= THIS_MODULE,
 	.name		= "hci_usb",
 	.probe		= hci_usb_probe,
 	.disconnect	= hci_usb_disconnect,
+	.suspend	= hci_usb_suspend,
+	.resume		= hci_usb_resume,
 	.id_table	= bluetooth_ids,
 };
 
maintainer
#1 lore raw

Hi Johannes,

Quoted 7 lines

The attached patch implements suspend/resume for the hci_usb bluetooth driver by simply killing all outstanding urbs on suspend, and re-issuing them on resume.

This allows me to actually use the internal bluetooth "dongle" in my powerbook after suspend-to-ram without taking down all userland programs (sdpd, ...) and the hci device and reloading the module.

thanks for the patch. Due to the removed owner field it won't apply cleanly to 2.6.16-rc1, but I can fix this easily by myself.

Quoted 12 lines
drivers/bluetooth/hci_usb.c +12 -0
+static int hci_usb_resume(struct usb_interface *intf)
+{
+	struct hci_usb *husb = usb_get_intfdata(intf);
+	int i, err;
+	unsigned long flags;
+	if (!husb || intf == husb->isoc_iface)
+		return 0;
+	
+	for (i = 0; i < 4; i++) {
+		struct _urb_queue *q = &husb->pending_q[i];
+		struct _urb *_urb;
+		spin_lock_irqsave(&q->lock, flags);

drivers/bluetooth/hci_usb.c line 1086

Quoted 10 lines
drivers/bluetooth/hci_usb.c +10 -0
+		list_for_each_entry(_urb, &q->head, list) {
+			err = usb_submit_urb(&_urb->urb, GFP_ATOMIC);
+			if (err) break;
+		}
+		spin_unlock_irqrestore(&q->lock, flags);
+		if (err)
+			return -EIO;
+	}
+	return 0;
+}

What happens if hci_usb_resume() really returns -EIO? Do we have to kill the URBs again or does the USB subsystems disconnect the device in this case?

Regards

Marcel

-------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc. Do you grep through log files for problems? Stop! Download the new AJAX search engine that makes searching your log files as easy as surfing the web. DOWNLOAD SPLUNK!
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=103432&bid=230486&dat=121642
_______________________________________________
Bluez-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/bluez-devel

#7 lore raw
In reply to Johannes Berg

Am Mittwoch, 18. Januar 2006 00:21 schrieb Johannes Berg:

Quoted 23 lines

The attached patch implements suspend/resume for the hci_usb bluetooth driver by simply killing all outstanding urbs on suspend, and re-issuing them on resume.

This allows me to actually use the internal bluetooth "dongle" in my powerbook after suspend-to-ram without taking down all userland programs (sdpd, ...) and the hci device and reloading the module.

drivers/bluetooth/hci_usb.c +7 -0
@@ -1043,11 +1043,55 @@
 	hci_free_dev(hdev);
 }
 
+static int hci_usb_suspend(struct usb_interface *intf, pm_message_t message)
+{
+	struct hci_usb *husb = usb_get_intfdata(intf);
+	int i;
+	unsigned long flags;
+	if (!husb || intf == husb->isoc_iface)
+		return 0;

drivers/bluetooth/hci_usb.c line 1062

Quoted 10 lines
drivers/bluetooth/hci_usb.c +10 -0
+	
+	for (i = 0; i < 4; i++) {
+		struct _urb_queue *q = &husb->pending_q[i];
+		struct _urb *_urb;
+		spin_lock_irqsave(&q->lock, flags);
+		list_for_each_entry(_urb, &q->head, list)
+			usb_kill_urb(&_urb->urb);
+		spin_unlock_irqrestore(&q->lock, flags);
+	}
+	return 0;

This patch is wrong. usb_kill_urb() will sleep. You must not use it under a spinlock.

Regards
Oliver

#8 lore raw
Quoted 2 lines · Oliver Neukum

This patch is wrong. usb_kill_urb() will sleep. You must not use it under a spinlock.

Whoops. Good catch. I'll have to analyse the logic with the lists being used here (and probably add a temporary list). Will try to get a new patch until tomorrow.

[side note: how about adding might_sleep() to usb_kill_urb? Then I'd at least have noticed this right away]

johannes

#9 lore raw

Am Mittwoch, 18. Januar 2006 15:13 schrieb Johannes Berg:

Quoted 11 lines · Oliver Neukum
Quoted 3 lines

This patch is wrong. usb_kill_urb() will sleep. You must not use it under a spinlock.

Whoops. Good catch. I'll have to analyse the logic with the lists being used here (and probably add a temporary list). Will try to get a new patch until tomorrow.

[side note: how about adding might_sleep() to usb_kill_urb? Then I'd at least have noticed this right away]

Good idea.

Regards
Oliver

#2 lore raw
In reply to Marcel Holtmann

The attached patch implements suspend/resume for the hci_usb bluetooth driver by simply killing all outstanding urbs on suspend, and re-issuing them on resume.

This allows me to actually use the internal bluetooth "dongle" in my powerbook after suspend-to-ram without taking down all userland programs (sdpd, ...) and the hci device and reloading the module.

drivers/bluetooth/hci_usb.c +55 -0
@@ -1043,10 +1043,65 @@
 	hci_free_dev(hdev);
 }
 
+static int hci_usb_suspend(struct usb_interface *intf, pm_message_t message)
+{
+	struct hci_usb *husb = usb_get_intfdata(intf);
+	int i;
+	unsigned long flags;
+	struct list_head killed;
+
+	if (!husb || intf == husb->isoc_iface)
+		return 0;
+
+	INIT_LIST_HEAD(&killed);
+
+	for (i = 0; i < 4; i++) {
+		struct _urb_queue *q = &husb->pending_q[i];
+		struct _urb *_urb, *_tmp;
+		while ((_urb = _urb_dequeue(q))) {
+			/* reset queue since _urb_dequeue sets it to NULL */
+			_urb->queue = q;
+			usb_kill_urb(&_urb->urb);
+			list_add(&_urb->list, &killed);
+		}
+		spin_lock_irqsave(&q->lock, flags);
+		list_for_each_entry_safe(_urb, _tmp, &killed, list) {
+			list_move_tail(&_urb->list, &q->head);
+		}
+		spin_unlock_irqrestore(&q->lock, flags);
+	}
+	return 0;
+}
+
+static int hci_usb_resume(struct usb_interface *intf)
+{
+	struct hci_usb *husb = usb_get_intfdata(intf);
+	int i, err = 0;
+	unsigned long flags;
+	if (!husb || intf == husb->isoc_iface)
+		return 0;
+	
+	for (i = 0; i < 4; i++) {
+		struct _urb_queue *q = &husb->pending_q[i];
+		struct _urb *_urb;
+		spin_lock_irqsave(&q->lock, flags);
+		list_for_each_entry(_urb, &q->head, list) {
+			err = usb_submit_urb(&_urb->urb, GFP_ATOMIC);
+			if (err) break;
+		}
+		spin_unlock_irqrestore(&q->lock, flags);
+		if (err)
+			return -EIO;
+	}
+	return 0;
+}
+
 static struct usb_driver hci_usb_driver = {
 	.name		= "hci_usb",
 	.probe		= hci_usb_probe,
 	.disconnect	= hci_usb_disconnect,
+	.suspend	= hci_usb_suspend,
+	.resume		= hci_usb_resume,
 	.id_table	= bluetooth_ids,
 };
 
#10 lore raw
In reply to Oliver Neukum
Quoted 6 lines · Oliver Neukum

Am Mittwoch, 18. Januar 2006 15:13 schrieb Johannes Berg:

Quoted 5 lines · Oliver Neukum
Quoted 3 lines

This patch is wrong. usb_kill_urb() will sleep. You must not use it under a spinlock.

Quoted 8 lines
Quoted 6 lines

Whoops. Good catch. I'll have to analyse the logic with the lists being used here (and probably add a temporary list). Will try to get a new patch until tomorrow.

[side note: how about adding might_sleep() to usb_kill_urb? Then I'd at least have noticed this right away]

Good idea.

Done.

thanks,

greg k-h

#3 lore raw
In reply to Johannes Berg
Quoted 7 lines · Johannes Berg

The attached patch implements suspend/resume for the hci_usb bluetooth driver by simply killing all outstanding urbs on suspend, and re-issuing them on resume.

This allows me to actually use the internal bluetooth "dongle" in my powerbook after suspend-to-ram without taking down all userland programs (sdpd, ...) and the hci device and reloading the module.

Anybody have success using this patch with suspend2?

I get:

Feb  9 13:48:32 localhost kernel: [17193418.644000] uhci_hcd
0000:00:1f.2: USB bus 1 deregistered
Feb  9 13:48:32 localhost kernel: [17193418.660000] ACPI: PCI
interrupt for device 0000:00:1f.2 disabled
Feb  9 13:48:39 localhost kernel: [17193425.268000] Suspend2 2.2:
Initiating a software suspend cycle.
Feb  9 13:48:46 localhost kernel: [17193427.272000] userspace ui:
Failed to contact userspace process.
Feb  9 13:48:46 localhost kernel: [17193427.272000] Freezing processes
Feb  9 13:48:47 localhost kernel: [17193432.404000] Stopping tasks failed.
Feb  9 13:48:47 localhost kernel: [17193432.404000] Tasks that refused
to be refrigerated and haven't since exited:
Feb  9 13:48:47 localhost kernel: [17193432.404000]  - snd-bt-scod
(#2206) signalled and todo list empty.
Feb  9 13:48:47 localhost kernel: [17193432.428000] Suspend2 debugging info=
:
Feb  9 13:48:47 localhost kernel: [17193432.428000] - SUSPEND core   : 2.2
Feb  9 13:48:47 localhost kernel: [17193432.428000] - Kernel Version :
2.6.15.1-p4-s2-ll
Feb  9 13:48:47 localhost kernel: [17193432.428000] - Compiler vers. : 4.0
Feb  9 13:48:47 localhost kernel: [17193432.428000] - Attempt number : 2
Feb  9 13:48:47 localhost kernel: [17193432.428000] - Parameters     :
17 32 0 1 525 0
Feb  9 13:48:47 localhost kernel: [17193432.428000] - Overall expected
compression percentage: 50.
Feb  9 13:48:47 localhost kernel: [17193432.428000] - Compressor lzf enable=
d.
Feb  9 13:48:47 localhost kernel: [17193432.428000] - Swapwriter active.
Feb  9 13:48:47 localhost kernel: [17193432.428000]   Swap available
for image: 307241 pages.
Feb  9 13:48:47 localhost kernel: [17193432.428000] - Filewriter inactive.
Feb  9 13:48:47 localhost kernel: [17193432.428000] - No I/O speed
stats available.
Feb  9 13:48:47 localhost kernel: [17193433.656000] USB Universal Host
Controller Interface driver v2.3
Feb  9 13:48:47 localhost kernel: [17193433.668000] ACPI: PCI
Interrupt 0000:00:1f.2[D] -> GSI 19 (level, low) -> IRQ 16
Feb  9 13:48:47 localhost kernel: [17193433.672000] PCI: Setting

latency timer of device 0000:00:1f.2 to 64

--
John C. McCabe-Dansted
Master's Student
94709339 (In Perth)

-------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc. Do you grep through log files for problems? Stop! Download the new AJAX search engine that makes searching your log files as easy as surfing the web. DOWNLOAD SPLUNK!
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=103432&bid=230486&dat=121642
_______________________________________________
Bluez-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/bluez-devel

#4 lore raw
In reply to Johannes Berg
Quoted 7 lines · Johannes Berg

The attached patch implements suspend/resume for the hci_usb bluetooth driver by simply killing all outstanding urbs on suspend, and re-issuing them on resume.

This allows me to actually use the internal bluetooth "dongle" in my powerbook after suspend-to-ram without taking down all userland programs (sdpd, ...) and the hci device and reloading the module.

Can someone push this patch for 2.6.17 now that 2.6.16 is out? Or is there still anything fundamentally wrong with it? I've been waiting for it forever now ;)

johannes

maintainer
#5 lore raw

Hi Johannes,

Quoted 11 lines
Quoted 7 lines

The attached patch implements suspend/resume for the hci_usb bluetooth driver by simply killing all outstanding urbs on suspend, and re-issuing them on resume.

This allows me to actually use the internal bluetooth "dongle" in my powerbook after suspend-to-ram without taking down all userland programs (sdpd, ...) and the hci device and reloading the module.

Can someone push this patch for 2.6.17 now that 2.6.16 is out? Or is there still anything fundamentally wrong with it? I've been waiting for it forever now ;)

I will push it with some other small changes in the next few days.

Regards

Marcel

-------------------------------------------------------
This SF.Net email is sponsored by xPML, a groundbreaking scripting language that extends applications into web and mobile media. Attend the live webcast and join the prime developer group breaking into this new coding territory!
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=110944&bid=241720&dat=121642
_______________________________________________
Bluez-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/bluez-devel

#6 lore raw
Quoted 1 line · Marcel Holtmann

I will push it with some other small changes in the next few days.

I don't see it yet, should I just post it instead?

johannes