Am Montag 25 August 2008 13:54:31 schrieb Marcel Holtmann:
> Oliver, can you come up with a small test patch, that just kills all =A0
> URB when suspend and submits the interrupt ones at resume. Such a =A0
> patch might have to be merged for 2.6.27 if it fixes this problem.
Rafael,
this patch implemts suspend/resume for btusb and fixes a disconnect
problem. Does it help you?
Regards
Oliver
=2D--
=2D-- linux-2.6.27-rc4/drivers/usb/core/urb.c 2008-08-21 10:03:44.000000000=
+0200
+++ linux-2.6.27-rc3/drivers/usb/core/urb.c 2008-08-22 17:25:49.000000000 +=
0200
@@ -601,15 +601,20 @@ EXPORT_SYMBOL_GPL(usb_kill_anchored_urbs
void usb_unlink_anchored_urbs(struct usb_anchor *anchor)
{
struct urb *victim;
+ unsigned long flags;
=20
=2D spin_lock_irq(&anchor->lock);
+ spin_lock_irqsave(&anchor->lock, flags);
while (!list_empty(&anchor->urb_list)) {
victim =3D list_entry(anchor->urb_list.prev, struct urb,
anchor_list);
+ usb_get_urb(victim);
+ spin_unlock_irqrestore(&anchor->lock, flags);
/* this will unanchor the URB */
usb_unlink_urb(victim);
+ usb_put_urb(victim);
+ spin_lock_irqsave(&anchor->lock, flags);
}
=2D spin_unlock_irq(&anchor->lock);
+ spin_unlock_irqrestore(&anchor->lock, flags);
}
EXPORT_SYMBOL_GPL(usb_unlink_anchored_urbs);
=20
=2D-- linux-2.6.27-rc4/drivers/bluetooth/btusb.c.alt 2008-08-25 15:02:14.00=
0000000 +0200
+++ linux-2.6.27-rc4/drivers/bluetooth/btusb.c 2008-08-25 15:44:25.00000000=
0 +0200
@@ -169,6 +169,7 @@ static struct usb_device_id blacklist_ta
struct btusb_data {
struct hci_dev *hdev;
struct usb_device *udev;
+ struct usb_interface *acl;
struct usb_interface *isoc;
=20
spinlock_t lock;
@@ -176,6 +177,7 @@ struct btusb_data {
unsigned long flags;
=20
struct work_struct work;
+ struct work_struct waker;
=20
struct usb_anchor tx_anchor;
struct usb_anchor intr_anchor;
@@ -189,6 +191,7 @@ struct btusb_data {
struct usb_endpoint_descriptor *isoc_rx_ep;
=20
int isoc_altsetting;
+ int susp_count;
};
=20
static void btusb_intr_complete(struct urb *urb)
@@ -227,7 +230,7 @@ static void btusb_intr_complete(struct u
}
}
=20
=2Dstatic int btusb_submit_intr_urb(struct hci_dev *hdev)
+static int btusb_submit_intr_urb(struct hci_dev *hdev, gfp_t gfp)
{
struct btusb_data *data =3D hdev->driver_data;
struct urb *urb;
@@ -240,13 +243,13 @@ static int btusb_submit_intr_urb(struct
if (!data->intr_ep)
return -ENODEV;
=20
=2D urb =3D usb_alloc_urb(0, GFP_ATOMIC);
+ urb =3D usb_alloc_urb(0, gfp);
if (!urb)
return -ENOMEM;
=20
size =3D le16_to_cpu(data->intr_ep->wMaxPacketSize);
=20
=2D buf =3D kmalloc(size, GFP_ATOMIC);
+ buf =3D kmalloc(size, gfp);
if (!buf) {
usb_free_urb(urb);
return -ENOMEM;
@@ -262,7 +265,7 @@ static int btusb_submit_intr_urb(struct
=20
usb_anchor_urb(urb, &data->intr_anchor);
=20
=2D err =3D usb_submit_urb(urb, GFP_ATOMIC);
+ err =3D usb_submit_urb(urb, gfp);
if (err < 0) {
BT_ERR("%s urb %p submission failed (%d)",
hdev->name, urb, -err);
@@ -311,7 +314,7 @@ static void btusb_bulk_complete(struct u
}
}
=20
=2Dstatic int btusb_submit_bulk_urb(struct hci_dev *hdev)
+static int btusb_submit_bulk_urb(struct hci_dev *hdev, gfp_t gfp)
{
struct btusb_data *data =3D hdev->driver_data;
struct urb *urb;
@@ -324,18 +327,19 @@ static int btusb_submit_bulk_urb(struct
if (!data->bulk_rx_ep)
return -ENODEV;
=20
=2D urb =3D usb_alloc_urb(0, GFP_KERNEL);
+ urb =3D usb_alloc_urb(0, gfp);
if (!urb)
return -ENOMEM;
=20
size =3D le16_to_cpu(data->bulk_rx_ep->wMaxPacketSize);
=20
=2D buf =3D kmalloc(size, GFP_KERNEL);
+ buf =3D kmalloc(size, gfp);
if (!buf) {
usb_free_urb(urb);
return -ENOMEM;
}
=20
+ usb_mark_last_busy(data->udev);
pipe =3D usb_rcvbulkpipe(data->udev, data->bulk_rx_ep->bEndpointAddress);
=20
usb_fill_bulk_urb(urb, data->udev, pipe,
@@ -345,7 +349,7 @@ static int btusb_submit_bulk_urb(struct
=20
usb_anchor_urb(urb, &data->bulk_anchor);
=20
=2D err =3D usb_submit_urb(urb, GFP_KERNEL);
+ err =3D usb_submit_urb(urb, gfp);
if (err < 0) {
BT_ERR("%s urb %p submission failed (%d)",
hdev->name, urb, -err);
@@ -514,7 +518,7 @@ static int btusb_open(struct hci_dev *hd
if (test_and_set_bit(BTUSB_INTR_RUNNING, &data->flags))
return 0;
=20
=2D err =3D btusb_submit_intr_urb(hdev);
+ err =3D btusb_submit_intr_urb(hdev, GFP_KERNEL);
if (err < 0) {
clear_bit(BTUSB_INTR_RUNNING, &hdev->flags);
clear_bit(HCI_RUNNING, &hdev->flags);
@@ -523,6 +527,13 @@ static int btusb_open(struct hci_dev *hd
return err;
}
=20
+static void btusb_stop_traffic(struct btusb_data *data)
+{
+ usb_kill_anchored_urbs(&data->intr_anchor);
+ usb_kill_anchored_urbs(&data->bulk_anchor);
+ usb_kill_anchored_urbs(&data->isoc_anchor);
+}
+
static int btusb_close(struct hci_dev *hdev)
{
struct btusb_data *data =3D hdev->driver_data;
@@ -532,14 +543,12 @@ static int btusb_close(struct hci_dev *h
if (!test_and_clear_bit(HCI_RUNNING, &hdev->flags))
return 0;
=20
=2D clear_bit(BTUSB_ISOC_RUNNING, &data->flags);
=2D usb_kill_anchored_urbs(&data->intr_anchor);
+ flush_work(&data->work);
=20
+ clear_bit(BTUSB_ISOC_RUNNING, &data->flags);
clear_bit(BTUSB_BULK_RUNNING, &data->flags);
=2D usb_kill_anchored_urbs(&data->bulk_anchor);
=2D
clear_bit(BTUSB_INTR_RUNNING, &data->flags);
=2D usb_kill_anchored_urbs(&data->intr_anchor);
+ btusb_stop_traffic(data);
=20
return 0;
}
@@ -672,8 +681,19 @@ static void btusb_notify(struct hci_dev
=20
BT_DBG("%s evt %d", hdev->name, evt);
=20
=2D if (evt =3D=3D HCI_NOTIFY_CONN_ADD || evt =3D=3D HCI_NOTIFY_CONN_DEL)
=2D schedule_work(&data->work);
+ if (hdev->conn_hash.acl_num > 0) {
+ if (!test_and_set_bit(BTUSB_BULK_RUNNING, &data->flags)) {
+ if (btusb_submit_bulk_urb(hdev, GFP_ATOMIC) < 0)
+ clear_bit(BTUSB_BULK_RUNNING, &data->flags);
+ else
+ btusb_submit_bulk_urb(hdev, GFP_ATOMIC);
+ }
+ } else {
+ clear_bit(BTUSB_BULK_RUNNING, &data->flags);
+ usb_unlink_anchored_urbs(&data->bulk_anchor);
+ }
+
+ schedule_work(&data->work);
}
=20
static int inline __set_isoc_interface(struct hci_dev *hdev, int altsettin=
g)
@@ -724,18 +744,6 @@ static void btusb_work(struct work_struc
struct btusb_data *data =3D container_of(work, struct btusb_data, work);
struct hci_dev *hdev =3D data->hdev;
=20
=2D if (hdev->conn_hash.acl_num > 0) {
=2D if (!test_and_set_bit(BTUSB_BULK_RUNNING, &data->flags)) {
=2D if (btusb_submit_bulk_urb(hdev) < 0)
=2D clear_bit(BTUSB_BULK_RUNNING, &data->flags);
=2D else
=2D btusb_submit_bulk_urb(hdev);
=2D }
=2D } else {
=2D clear_bit(BTUSB_BULK_RUNNING, &data->flags);
=2D usb_kill_anchored_urbs(&data->bulk_anchor);
=2D }
=2D
if (hdev->conn_hash.sco_num > 0) {
if (data->isoc_altsetting !=3D 2) {
clear_bit(BTUSB_ISOC_RUNNING, &data->flags);
@@ -821,6 +829,7 @@ static int btusb_probe(struct usb_interf
}
=20
data->udev =3D interface_to_usbdev(intf);
+ data->acl =3D intf;
=20
spin_lock_init(&data->lock);
=20
@@ -889,7 +898,7 @@ static int btusb_probe(struct usb_interf
=20
if (data->isoc) {
err =3D usb_driver_claim_interface(&btusb_driver,
=2D data->isoc, NULL);
+ data->isoc, data);
if (err < 0) {
hci_free_dev(hdev);
kfree(data);
@@ -921,20 +930,92 @@ static void btusb_disconnect(struct usb_
=20
hdev =3D data->hdev;
=20
=2D if (data->isoc)
=2D usb_driver_release_interface(&btusb_driver, data->isoc);
+ /* make sure we have a reference */
+ __hci_dev_hold(hdev);
=20
=2D usb_set_intfdata(intf, NULL);
+ usb_set_intfdata(data->acl, NULL);
+ if (data->isoc)
+ usb_set_intfdata(data->isoc, NULL);
=20
+ /* unregister before releasing any interface */
hci_unregister_dev(hdev);
=20
+ if (intf =3D=3D data->isoc)
+ usb_driver_release_interface(&btusb_driver, data->acl);
+ else if (data->isoc)
+ usb_driver_release_interface(&btusb_driver, data->isoc);
+
+ /* release the reference */
+ __hci_dev_put(hdev);
hci_free_dev(hdev);
}
=20
+static int btusb_suspend(struct usb_interface *intf, pm_message_t message)
+{
+ struct btusb_data *data =3D usb_get_intfdata(intf);
+
+ BT_DBG("%s called\n", __func__);
+
+ if (data->susp_count++)
+ return 0;
+
+ cancel_work_sync(&data->work);
+ btusb_stop_traffic(data);
+ usb_kill_anchored_urbs(&data->tx_anchor);
+ return 0;
+}
+
+static int btusb_resume(struct usb_interface *intf)
+{
+ struct btusb_data *data =3D usb_get_intfdata(intf);
+ struct hci_dev *hdev =3D data->hdev;
+ int ret;
+
+ if (--data->susp_count)
+ return 0;
+ if (test_bit(HCI_RUNNING, &hdev->flags)) {
+ ret =3D btusb_submit_intr_urb(hdev, GFP_NOIO);
+ if (ret < 0) {
+ clear_bit(HCI_RUNNING, &hdev->flags);
+ return ret;
+ }
+ }
+
+ if (hdev->conn_hash.acl_num > 0) {
+ ret =3D btusb_submit_bulk_urb(hdev, GFP_NOIO);
+ if (ret < 0) {
+ clear_bit(BTUSB_BULK_RUNNING, &data->flags);
+ return ret;
+ } else {
+ ret =3D btusb_submit_bulk_urb(hdev, GFP_NOIO);
+ if (ret < 0) {
+ clear_bit(BTUSB_BULK_RUNNING, &data->flags);
+ usb_kill_anchored_urbs(&data->bulk_anchor);
+ return ret;
+ }
+ }
+ }
+
+ if (data->isoc) {
+ if (test_bit(BTUSB_ISOC_RUNNING, &data->flags)) {
+ ret =3D btusb_submit_isoc_urb(hdev);
+ if (ret < 0)
+ clear_bit(BTUSB_ISOC_RUNNING, &data->flags);
+ else
+ btusb_submit_isoc_urb(hdev);
+ }
+ }
+
+ schedule_work(&data->work);
+ return 0;
+}
+
static struct usb_driver btusb_driver =3D {
.name =3D "btusb",
.probe =3D btusb_probe,
.disconnect =3D btusb_disconnect,
+ .suspend =3D btusb_suspend,
+ .resume =3D btusb_resume,
.id_table =3D btusb_table,
};
=20
Am Donnerstag 28 August 2008 00:33:06 schrieb Rafael J. Wysocki:
> On Wednesday, 27 of August 2008, Rafael J. Wysocki wrote:
> > On Wednesday, 27 of August 2008, Oliver Neukum wrote:
> > > Am Mittwoch 27 August 2008 15:04:55 schrieb Rafael J. Wysocki:
> > > > I'm quite confident it's specific to bluetooth, because I only need to turn
> > > > the bluetooth user space off, most importantly hcid, to make the problem go
> > > > away even without the patch (that is, without the patch I don't even have to
> > > > uload btusb before suspend if the bluetoot user land is not running).
> > >
> > > But do you use any other usb driver without support for suspend/resume?
> >
> > Well, probably not.
> >
> > I'll check the hid, then.
>
> I couldn't reproduce the breakage with suspend/resume support removed from
> usbhid.
>
> Let's just say it's related to BT, since your patch evidently fixes the problem
> for me and I have just no reason to believe it's anything else than BT.
>
> Thanks,
> Rafael
>
Very good.
Marcel, here it is again. You may need to cut the USB bits depending
on which tree you want to apply it to.
Signed-off-by: Oliver Neukum <[email protected]>
Regards
Oliver
---
--- linux-2.6.27-rc4/drivers/usb/core/urb.c 2008-08-21 10:03:44.000000000 +0200
+++ linux-2.6.27-rc3/drivers/usb/core/urb.c 2008-08-22 17:25:49.000000000 +0200
@@ -601,15 +601,20 @@ EXPORT_SYMBOL_GPL(usb_kill_anchored_urbs
void usb_unlink_anchored_urbs(struct usb_anchor *anchor)
{
struct urb *victim;
+ unsigned long flags;
- spin_lock_irq(&anchor->lock);
+ spin_lock_irqsave(&anchor->lock, flags);
while (!list_empty(&anchor->urb_list)) {
victim = list_entry(anchor->urb_list.prev, struct urb,
anchor_list);
+ usb_get_urb(victim);
+ spin_unlock_irqrestore(&anchor->lock, flags);
/* this will unanchor the URB */
usb_unlink_urb(victim);
+ usb_put_urb(victim);
+ spin_lock_irqsave(&anchor->lock, flags);
}
- spin_unlock_irq(&anchor->lock);
+ spin_unlock_irqrestore(&anchor->lock, flags);
}
EXPORT_SYMBOL_GPL(usb_unlink_anchored_urbs);
--- linux-2.6.27-rc4/drivers/bluetooth/btusb.c.alt 2008-08-25 15:02:14.000000000 +0200
+++ linux-2.6.27-rc4/drivers/bluetooth/btusb.c 2008-08-25 15:44:25.000000000 +0200
@@ -169,6 +169,7 @@ static struct usb_device_id blacklist_ta
struct btusb_data {
struct hci_dev *hdev;
struct usb_device *udev;
+ struct usb_interface *acl;
struct usb_interface *isoc;
spinlock_t lock;
@@ -176,6 +177,7 @@ struct btusb_data {
unsigned long flags;
struct work_struct work;
+ struct work_struct waker;
struct usb_anchor tx_anchor;
struct usb_anchor intr_anchor;
@@ -189,6 +191,7 @@ struct btusb_data {
struct usb_endpoint_descriptor *isoc_rx_ep;
int isoc_altsetting;
+ int susp_count;
};
static void btusb_intr_complete(struct urb *urb)
@@ -227,7 +230,7 @@ static void btusb_intr_complete(struct u
}
}
-static int btusb_submit_intr_urb(struct hci_dev *hdev)
+static int btusb_submit_intr_urb(struct hci_dev *hdev, gfp_t gfp)
{
struct btusb_data *data = hdev->driver_data;
struct urb *urb;
@@ -240,13 +243,13 @@ static int btusb_submit_intr_urb(struct
if (!data->intr_ep)
return -ENODEV;
- urb = usb_alloc_urb(0, GFP_ATOMIC);
+ urb = usb_alloc_urb(0, gfp);
if (!urb)
return -ENOMEM;
size = le16_to_cpu(data->intr_ep->wMaxPacketSize);
- buf = kmalloc(size, GFP_ATOMIC);
+ buf = kmalloc(size, gfp);
if (!buf) {
usb_free_urb(urb);
return -ENOMEM;
@@ -262,7 +265,7 @@ static int btusb_submit_intr_urb(struct
usb_anchor_urb(urb, &data->intr_anchor);
- err = usb_submit_urb(urb, GFP_ATOMIC);
+ err = usb_submit_urb(urb, gfp);
if (err < 0) {
BT_ERR("%s urb %p submission failed (%d)",
hdev->name, urb, -err);
@@ -311,7 +314,7 @@ static void btusb_bulk_complete(struct u
}
}
-static int btusb_submit_bulk_urb(struct hci_dev *hdev)
+static int btusb_submit_bulk_urb(struct hci_dev *hdev, gfp_t gfp)
{
struct btusb_data *data = hdev->driver_data;
struct urb *urb;
@@ -324,18 +327,19 @@ static int btusb_submit_bulk_urb(struct
if (!data->bulk_rx_ep)
return -ENODEV;
- urb = usb_alloc_urb(0, GFP_KERNEL);
+ urb = usb_alloc_urb(0, gfp);
if (!urb)
return -ENOMEM;
size = le16_to_cpu(data->bulk_rx_ep->wMaxPacketSize);
- buf = kmalloc(size, GFP_KERNEL);
+ buf = kmalloc(size, gfp);
if (!buf) {
usb_free_urb(urb);
return -ENOMEM;
}
+ usb_mark_last_busy(data->udev);
pipe = usb_rcvbulkpipe(data->udev, data->bulk_rx_ep->bEndpointAddress);
usb_fill_bulk_urb(urb, data->udev, pipe,
@@ -345,7 +349,7 @@ static int btusb_submit_bulk_urb(struct
usb_anchor_urb(urb, &data->bulk_anchor);
- err = usb_submit_urb(urb, GFP_KERNEL);
+ err = usb_submit_urb(urb, gfp);
if (err < 0) {
BT_ERR("%s urb %p submission failed (%d)",
hdev->name, urb, -err);
@@ -514,7 +518,7 @@ static int btusb_open(struct hci_dev *hd
if (test_and_set_bit(BTUSB_INTR_RUNNING, &data->flags))
return 0;
- err = btusb_submit_intr_urb(hdev);
+ err = btusb_submit_intr_urb(hdev, GFP_KERNEL);
if (err < 0) {
clear_bit(BTUSB_INTR_RUNNING, &hdev->flags);
clear_bit(HCI_RUNNING, &hdev->flags);
@@ -523,6 +527,13 @@ static int btusb_open(struct hci_dev *hd
return err;
}
+static void btusb_stop_traffic(struct btusb_data *data)
+{
+ usb_kill_anchored_urbs(&data->intr_anchor);
+ usb_kill_anchored_urbs(&data->bulk_anchor);
+ usb_kill_anchored_urbs(&data->isoc_anchor);
+}
+
static int btusb_close(struct hci_dev *hdev)
{
struct btusb_data *data = hdev->driver_data;
@@ -532,14 +543,12 @@ static int btusb_close(struct hci_dev *h
if (!test_and_clear_bit(HCI_RUNNING, &hdev->flags))
return 0;
- clear_bit(BTUSB_ISOC_RUNNING, &data->flags);
- usb_kill_anchored_urbs(&data->intr_anchor);
+ flush_work(&data->work);
+ clear_bit(BTUSB_ISOC_RUNNING, &data->flags);
clear_bit(BTUSB_BULK_RUNNING, &data->flags);
- usb_kill_anchored_urbs(&data->bulk_anchor);
-
clear_bit(BTUSB_INTR_RUNNING, &data->flags);
- usb_kill_anchored_urbs(&data->intr_anchor);
+ btusb_stop_traffic(data);
return 0;
}
@@ -672,8 +681,19 @@ static void btusb_notify(struct hci_dev
BT_DBG("%s evt %d", hdev->name, evt);
- if (evt == HCI_NOTIFY_CONN_ADD || evt == HCI_NOTIFY_CONN_DEL)
- schedule_work(&data->work);
+ if (hdev->conn_hash.acl_num > 0) {
+ if (!test_and_set_bit(BTUSB_BULK_RUNNING, &data->flags)) {
+ if (btusb_submit_bulk_urb(hdev, GFP_ATOMIC) < 0)
+ clear_bit(BTUSB_BULK_RUNNING, &data->flags);
+ else
+ btusb_submit_bulk_urb(hdev, GFP_ATOMIC);
+ }
+ } else {
+ clear_bit(BTUSB_BULK_RUNNING, &data->flags);
+ usb_unlink_anchored_urbs(&data->bulk_anchor);
+ }
+
+ schedule_work(&data->work);
}
static int inline __set_isoc_interface(struct hci_dev *hdev, int altsetting)
@@ -724,18 +744,6 @@ static void btusb_work(struct work_struc
struct btusb_data *data = container_of(work, struct btusb_data, work);
struct hci_dev *hdev = data->hdev;
- if (hdev->conn_hash.acl_num > 0) {
- if (!test_and_set_bit(BTUSB_BULK_RUNNING, &data->flags)) {
- if (btusb_submit_bulk_urb(hdev) < 0)
- clear_bit(BTUSB_BULK_RUNNING, &data->flags);
- else
- btusb_submit_bulk_urb(hdev);
- }
- } else {
- clear_bit(BTUSB_BULK_RUNNING, &data->flags);
- usb_kill_anchored_urbs(&data->bulk_anchor);
- }
-
if (hdev->conn_hash.sco_num > 0) {
if (data->isoc_altsetting != 2) {
clear_bit(BTUSB_ISOC_RUNNING, &data->flags);
@@ -821,6 +829,7 @@ static int btusb_probe(struct usb_interf
}
data->udev = interface_to_usbdev(intf);
+ data->acl = intf;
spin_lock_init(&data->lock);
@@ -889,7 +898,7 @@ static int btusb_probe(struct usb_interf
if (data->isoc) {
err = usb_driver_claim_interface(&btusb_driver,
- data->isoc, NULL);
+ data->isoc, data);
if (err < 0) {
hci_free_dev(hdev);
kfree(data);
@@ -921,20 +930,92 @@ static void btusb_disconnect(struct usb_
hdev = data->hdev;
- if (data->isoc)
- usb_driver_release_interface(&btusb_driver, data->isoc);
+ /* make sure we have a reference */
+ __hci_dev_hold(hdev);
- usb_set_intfdata(intf, NULL);
+ usb_set_intfdata(data->acl, NULL);
+ if (data->isoc)
+ usb_set_intfdata(data->isoc, NULL);
+ /* unregister before releasing any interface */
hci_unregister_dev(hdev);
+ if (intf == data->isoc)
+ usb_driver_release_interface(&btusb_driver, data->acl);
+ else if (data->isoc)
+ usb_driver_release_interface(&btusb_driver, data->isoc);
+
+ /* release the reference */
+ __hci_dev_put(hdev);
hci_free_dev(hdev);
}
+static int btusb_suspend(struct usb_interface *intf, pm_message_t message)
+{
+ struct btusb_data *data = usb_get_intfdata(intf);
+
+ BT_DBG("%s called\n", __func__);
+
+ if (data->susp_count++)
+ return 0;
+
+ cancel_work_sync(&data->work);
+ btusb_stop_traffic(data);
+ usb_kill_anchored_urbs(&data->tx_anchor);
+ return 0;
+}
+
+static int btusb_resume(struct usb_interface *intf)
+{
+ struct btusb_data *data = usb_get_intfdata(intf);
+ struct hci_dev *hdev = data->hdev;
+ int ret;
+
+ if (--data->susp_count)
+ return 0;
+ if (test_bit(HCI_RUNNING, &hdev->flags)) {
+ ret = btusb_submit_intr_urb(hdev, GFP_NOIO);
+ if (ret < 0) {
+ clear_bit(HCI_RUNNING, &hdev->flags);
+ return ret;
+ }
+ }
+
+ if (hdev->conn_hash.acl_num > 0) {
+ ret = btusb_submit_bulk_urb(hdev, GFP_NOIO);
+ if (ret < 0) {
+ clear_bit(BTUSB_BULK_RUNNING, &data->flags);
+ return ret;
+ } else {
+ ret = btusb_submit_bulk_urb(hdev, GFP_NOIO);
+ if (ret < 0) {
+ clear_bit(BTUSB_BULK_RUNNING, &data->flags);
+ usb_kill_anchored_urbs(&data->bulk_anchor);
+ return ret;
+ }
+ }
+ }
+
+ if (data->isoc) {
+ if (test_bit(BTUSB_ISOC_RUNNING, &data->flags)) {
+ ret = btusb_submit_isoc_urb(hdev);
+ if (ret < 0)
+ clear_bit(BTUSB_ISOC_RUNNING, &data->flags);
+ else
+ btusb_submit_isoc_urb(hdev);
+ }
+ }
+
+ schedule_work(&data->work);
+ return 0;
+}
+
static struct usb_driver btusb_driver = {
.name = "btusb",
.probe = btusb_probe,
.disconnect = btusb_disconnect,
+ .suspend = btusb_suspend,
+ .resume = btusb_resume,
.id_table = btusb_table,
};
On Wednesday, 27 of August 2008, Rafael J. Wysocki wrote:
> On Wednesday, 27 of August 2008, Oliver Neukum wrote:
> > Am Mittwoch 27 August 2008 15:04:55 schrieb Rafael J. Wysocki:
> > > I'm quite confident it's specific to bluetooth, because I only need to turn
> > > the bluetooth user space off, most importantly hcid, to make the problem go
> > > away even without the patch (that is, without the patch I don't even have to
> > > uload btusb before suspend if the bluetoot user land is not running).
> >
> > But do you use any other usb driver without support for suspend/resume?
>
> Well, probably not.
>
> I'll check the hid, then.
I couldn't reproduce the breakage with suspend/resume support removed from
usbhid.
Let's just say it's related to BT, since your patch evidently fixes the problem
for me and I have just no reason to believe it's anything else than BT.
Thanks,
Rafael
On Wednesday, 27 of August 2008, Oliver Neukum wrote:
> Am Mittwoch 27 August 2008 15:04:55 schrieb Rafael J. Wysocki:
> > I'm quite confident it's specific to bluetooth, because I only need to turn
> > the bluetooth user space off, most importantly hcid, to make the problem go
> > away even without the patch (that is, without the patch I don't even have to
> > uload btusb before suspend if the bluetoot user land is not running).
>
> But do you use any other usb driver without support for suspend/resume?
Well, probably not.
I'll check the hid, then.
BTW, 2.6.26 appears to be fine.
Thanks,
Rafael
Am Mittwoch 27 August 2008 15:04:55 schrieb Rafael J. Wysocki:
> I'm quite confident it's specific to bluetooth, because I only need to turn
> the bluetooth user space off, most importantly hcid, to make the problem go
> away even without the patch (that is, without the patch I don't even have to
> uload btusb before suspend if the bluetoot user land is not running).
But do you use any other usb driver without support for suspend/resume?
Regards
Oliver
On Wednesday, 27 of August 2008, Oliver Neukum wrote:
> Am Mittwoch 27 August 2008 01:28:15 schrieb Rafael J. Wysocki:
> > > This doesn't explain the original failure. Can you comment out the su=
pport
> > > for suspend/resume in the driver and try again?
> >=20
> > With that commented out, I'm able to reproduce the failure. =A0With the=
original
> > patch, I'm not.
>=20
> Now we know a driver without support for suspend/resume breaks suspend and
> hibernate. We need to find out whether this is limited to btusb or everyt=
hing.
I'm quite confident it's specific to bluetooth, because I only need to turn
the bluetooth user space off, most importantly hcid, to make the problem go
away even without the patch (that is, without the patch I don't even have to
uload btusb before suspend if the bluetoot user land is not running).
> Can you test with anotherrrrr driver, like a mouse and remove suspend/res=
ume
> from usbhid? If this is specific to btusb we have a fix, if not, I am afr=
aid
> this has to be bisected.
Is that really necessary?
I can do that, but first I'll check if 2.6.26 is fine.
Thanks,
Rafael
Hi Rafael,
> > > > > > Good. Can you test what happens if you unplug the device while suspended
> > > > > > and hibernated?
> > > > >
> > > > > It's built-in, I can't unplug it. :-)
> > > >
> > > > Maybe you can disable it in the BIOS, but this might change the DSDT / other
> > > > system configuration, so it might break resume in other ways :-(
> > >
> > > There is a switch that's supposed to disable the radio (rfkill or something).
> > > I used it to switch the radio off while the box was waking up from hibernation
> > > and kbluetooth didn't find the adapter after the resume. After I've pressed
> > > the "radio off" button again, the bluetooth appears to be functional again.
> > >
> > > However, this "radio off" button is shared between bluetooth and wireless
> > > (b43) and there are some surprising interactions. Nothing seems to be broken,
> > > though.
> >
> > This doesn't explain the original failure. Can you comment out the support
> > for suspend/resume in the driver and try again?
>
> With that commented out, I'm able to reproduce the failure. With the original
> patch, I'm not.
I've never seen any issues with the suspend/resume and btusb, but I
must admit that I am using an X61 and in that case pm-utils has a magic
hack to disable Bluetooth before suspend and this means a clean
disconnect from the USB bus.
Anyway, killing all URBs in-fly on suspend and bringing up the interrupt
one on resume should do the right thing. However we have to check if we
not just better resume all URBs and let the Bluetooth core handle lost
connection during the suspended time.
Regards
Marcel
Am Mittwoch 27 August 2008 01:28:15 schrieb Rafael J. Wysocki:
> > This doesn't explain the original failure. Can you comment out the supp=
ort
> > for suspend/resume in the driver and try again?
>=20
> With that commented out, I'm able to reproduce the failure. =A0With the o=
riginal
> patch, I'm not.
Now we know a driver without support for suspend/resume breaks suspend and
hibernate. We need to find out whether this is limited to btusb or everythi=
ng.
Can you test with anotherrrrr driver, like a mouse and remove suspend/resume
from usbhid? If this is specific to btusb we have a fix, if not, I am afraid
this has to be bisected.
Regards
Oliver
On Tuesday, 26 of August 2008, Oliver Neukum wrote:
> Am Dienstag 26 August 2008 20:44:53 schrieb Rafael J. Wysocki:
> > On Tuesday, 26 of August 2008, Stefan Seyfried wrote:
> > > On Tue, Aug 26, 2008 at 12:10:08PM +0200, Rafael J. Wysocki wrote:
> > > > > Good. Can you test what happens if you unplug the device while suspended
> > > > > and hibernated?
> > > >
> > > > It's built-in, I can't unplug it. :-)
> > >
> > > Maybe you can disable it in the BIOS, but this might change the DSDT / other
> > > system configuration, so it might break resume in other ways :-(
> >
> > There is a switch that's supposed to disable the radio (rfkill or something).
> > I used it to switch the radio off while the box was waking up from hibernation
> > and kbluetooth didn't find the adapter after the resume. After I've pressed
> > the "radio off" button again, the bluetooth appears to be functional again.
> >
> > However, this "radio off" button is shared between bluetooth and wireless
> > (b43) and there are some surprising interactions. Nothing seems to be broken,
> > though.
>
> This doesn't explain the original failure. Can you comment out the support
> for suspend/resume in the driver and try again?
With that commented out, I'm able to reproduce the failure. With the original
patch, I'm not.
Thanks,
Rafael
Am Dienstag 26 August 2008 20:44:53 schrieb Rafael J. Wysocki:
> On Tuesday, 26 of August 2008, Stefan Seyfried wrote:
> > On Tue, Aug 26, 2008 at 12:10:08PM +0200, Rafael J. Wysocki wrote:
> > > > Good. Can you test what happens if you unplug the device while suspended
> > > > and hibernated?
> > >
> > > It's built-in, I can't unplug it. :-)
> >
> > Maybe you can disable it in the BIOS, but this might change the DSDT / other
> > system configuration, so it might break resume in other ways :-(
>
> There is a switch that's supposed to disable the radio (rfkill or something).
> I used it to switch the radio off while the box was waking up from hibernation
> and kbluetooth didn't find the adapter after the resume. After I've pressed
> the "radio off" button again, the bluetooth appears to be functional again.
>
> However, this "radio off" button is shared between bluetooth and wireless
> (b43) and there are some surprising interactions. Nothing seems to be broken,
> though.
This doesn't explain the original failure. Can you comment out the support
for suspend/resume in the driver and try again? The patch also fixes a
race in disconnect that you may be hitting.
Regards
Oliver
On Tuesday, 26 of August 2008, Stefan Seyfried wrote:
> On Tue, Aug 26, 2008 at 12:10:08PM +0200, Rafael J. Wysocki wrote:
> > > Good. Can you test what happens if you unplug the device while suspended
> > > and hibernated?
> >
> > It's built-in, I can't unplug it. :-)
>
> Maybe you can disable it in the BIOS, but this might change the DSDT / other
> system configuration, so it might break resume in other ways :-(
There is a switch that's supposed to disable the radio (rfkill or something).
I used it to switch the radio off while the box was waking up from hibernation
and kbluetooth didn't find the adapter after the resume. After I've pressed
the "radio off" button again, the bluetooth appears to be functional again.
However, this "radio off" button is shared between bluetooth and wireless
(b43) and there are some surprising interactions. Nothing seems to be broken,
though.
Thanks,
Rafael
On Tue, Aug 26, 2008 at 12:10:08PM +0200, Rafael J. Wysocki wrote:
> > Good. Can you test what happens if you unplug the device while suspended
> > and hibernated?
>
> It's built-in, I can't unplug it. :-)
Maybe you can disable it in the BIOS, but this might change the DSDT / other
system configuration, so it might break resume in other ways :-(
--
Stefan Seyfried
R&D Team Mobile Devices | "Any ideas, John?"
SUSE LINUX Products GmbH, N?rnberg | "Well, surrounding them's out."
This footer brought to you by insane German lawmakers:
SUSE Linux Products GmbH, GF: Markus Rex, HRB 16746 (AG N?rnberg)
On Tuesday, 26 of August 2008, Oliver Neukum wrote:
> Am Dienstag 26 August 2008 11:36:04 schrieb Rafael J. Wysocki:
> > [Sorry for the delayed reply.]
> >=20
> > On Monday, 25 of August 2008, Oliver Neukum wrote:
> > > Am Montag 25 August 2008 13:54:31 schrieb Marcel Holtmann:
> > > > Oliver, can you come up with a small test patch, that just kills al=
l =A0
> > > > URB when suspend and submits the interrupt ones at resume. Such a =
=A0
> > > > patch might have to be merged for 2.6.27 if it fixes this problem.
> > >=20
> > > Rafael,
> > >=20
> > > this patch implemts suspend/resume for btusb and fixes a disconnect
> > > problem. Does it help you?
> >=20
> > Yes, the patch appears to help.
> >=20
> > I haven't had a single crash since I applied it. I'm going to test it =
a bit
> > more today, though.
>=20
> Good. Can you test what happens if you unplug the device while suspended
> and hibernated?
It's built-in, I can't unplug it. :-)
Thanks,
Rafael
Am Dienstag 26 August 2008 11:36:04 schrieb Rafael J. Wysocki:
> [Sorry for the delayed reply.]
>=20
> On Monday, 25 of August 2008, Oliver Neukum wrote:
> > Am Montag 25 August 2008 13:54:31 schrieb Marcel Holtmann:
> > > Oliver, can you come up with a small test patch, that just kills all =
=A0
> > > URB when suspend and submits the interrupt ones at resume. Such a =A0
> > > patch might have to be merged for 2.6.27 if it fixes this problem.
> >=20
> > Rafael,
> >=20
> > this patch implemts suspend/resume for btusb and fixes a disconnect
> > problem. Does it help you?
>=20
> Yes, the patch appears to help.
>=20
> I haven't had a single crash since I applied it. I'm going to test it a =
bit
> more today, though.
Good. Can you test what happens if you unplug the device while suspended
and hibernated?
Regards
Oliver
[Sorry for the delayed reply.]
On Monday, 25 of August 2008, Oliver Neukum wrote:
> Am Montag 25 August 2008 13:54:31 schrieb Marcel Holtmann:
> > Oliver, can you come up with a small test patch, that just kills all ?
> > URB when suspend and submits the interrupt ones at resume. Such a ?
> > patch might have to be merged for 2.6.27 if it fixes this problem.
>
> Rafael,
>
> this patch implemts suspend/resume for btusb and fixes a disconnect
> problem. Does it help you?
Yes, the patch appears to help.
I haven't had a single crash since I applied it. I'm going to test it a bit
more today, though.
> ---
BTW, this change:
> --- linux-2.6.27-rc4/drivers/usb/core/urb.c 2008-08-21 10:03:44.000000000 +0200
> +++ linux-2.6.27-rc3/drivers/usb/core/urb.c 2008-08-22 17:25:49.000000000 +0200
> @@ -601,15 +601,20 @@ EXPORT_SYMBOL_GPL(usb_kill_anchored_urbs
> void usb_unlink_anchored_urbs(struct usb_anchor *anchor)
> {
> struct urb *victim;
> + unsigned long flags;
>
> - spin_lock_irq(&anchor->lock);
> + spin_lock_irqsave(&anchor->lock, flags);
> while (!list_empty(&anchor->urb_list)) {
> victim = list_entry(anchor->urb_list.prev, struct urb,
> anchor_list);
> + usb_get_urb(victim);
> + spin_unlock_irqrestore(&anchor->lock, flags);
> /* this will unanchor the URB */
> usb_unlink_urb(victim);
> + usb_put_urb(victim);
> + spin_lock_irqsave(&anchor->lock, flags);
> }
> - spin_unlock_irq(&anchor->lock);
> + spin_unlock_irqrestore(&anchor->lock, flags);
> }
> EXPORT_SYMBOL_GPL(usb_unlink_anchored_urbs);
>
is apparently already in the mainline.
Thanks,
Rafael
> --- linux-2.6.27-rc4/drivers/bluetooth/btusb.c.alt 2008-08-25 15:02:14.000000000 +0200
> +++ linux-2.6.27-rc4/drivers/bluetooth/btusb.c 2008-08-25 15:44:25.000000000 +0200
> @@ -169,6 +169,7 @@ static struct usb_device_id blacklist_ta
> struct btusb_data {
> struct hci_dev *hdev;
> struct usb_device *udev;
> + struct usb_interface *acl;
> struct usb_interface *isoc;
>
> spinlock_t lock;
> @@ -176,6 +177,7 @@ struct btusb_data {
> unsigned long flags;
>
> struct work_struct work;
> + struct work_struct waker;
>
> struct usb_anchor tx_anchor;
> struct usb_anchor intr_anchor;
> @@ -189,6 +191,7 @@ struct btusb_data {
> struct usb_endpoint_descriptor *isoc_rx_ep;
>
> int isoc_altsetting;
> + int susp_count;
> };
>
> static void btusb_intr_complete(struct urb *urb)
> @@ -227,7 +230,7 @@ static void btusb_intr_complete(struct u
> }
> }
>
> -static int btusb_submit_intr_urb(struct hci_dev *hdev)
> +static int btusb_submit_intr_urb(struct hci_dev *hdev, gfp_t gfp)
> {
> struct btusb_data *data = hdev->driver_data;
> struct urb *urb;
> @@ -240,13 +243,13 @@ static int btusb_submit_intr_urb(struct
> if (!data->intr_ep)
> return -ENODEV;
>
> - urb = usb_alloc_urb(0, GFP_ATOMIC);
> + urb = usb_alloc_urb(0, gfp);
> if (!urb)
> return -ENOMEM;
>
> size = le16_to_cpu(data->intr_ep->wMaxPacketSize);
>
> - buf = kmalloc(size, GFP_ATOMIC);
> + buf = kmalloc(size, gfp);
> if (!buf) {
> usb_free_urb(urb);
> return -ENOMEM;
> @@ -262,7 +265,7 @@ static int btusb_submit_intr_urb(struct
>
> usb_anchor_urb(urb, &data->intr_anchor);
>
> - err = usb_submit_urb(urb, GFP_ATOMIC);
> + err = usb_submit_urb(urb, gfp);
> if (err < 0) {
> BT_ERR("%s urb %p submission failed (%d)",
> hdev->name, urb, -err);
> @@ -311,7 +314,7 @@ static void btusb_bulk_complete(struct u
> }
> }
>
> -static int btusb_submit_bulk_urb(struct hci_dev *hdev)
> +static int btusb_submit_bulk_urb(struct hci_dev *hdev, gfp_t gfp)
> {
> struct btusb_data *data = hdev->driver_data;
> struct urb *urb;
> @@ -324,18 +327,19 @@ static int btusb_submit_bulk_urb(struct
> if (!data->bulk_rx_ep)
> return -ENODEV;
>
> - urb = usb_alloc_urb(0, GFP_KERNEL);
> + urb = usb_alloc_urb(0, gfp);
> if (!urb)
> return -ENOMEM;
>
> size = le16_to_cpu(data->bulk_rx_ep->wMaxPacketSize);
>
> - buf = kmalloc(size, GFP_KERNEL);
> + buf = kmalloc(size, gfp);
> if (!buf) {
> usb_free_urb(urb);
> return -ENOMEM;
> }
>
> + usb_mark_last_busy(data->udev);
> pipe = usb_rcvbulkpipe(data->udev, data->bulk_rx_ep->bEndpointAddress);
>
> usb_fill_bulk_urb(urb, data->udev, pipe,
> @@ -345,7 +349,7 @@ static int btusb_submit_bulk_urb(struct
>
> usb_anchor_urb(urb, &data->bulk_anchor);
>
> - err = usb_submit_urb(urb, GFP_KERNEL);
> + err = usb_submit_urb(urb, gfp);
> if (err < 0) {
> BT_ERR("%s urb %p submission failed (%d)",
> hdev->name, urb, -err);
> @@ -514,7 +518,7 @@ static int btusb_open(struct hci_dev *hd
> if (test_and_set_bit(BTUSB_INTR_RUNNING, &data->flags))
> return 0;
>
> - err = btusb_submit_intr_urb(hdev);
> + err = btusb_submit_intr_urb(hdev, GFP_KERNEL);
> if (err < 0) {
> clear_bit(BTUSB_INTR_RUNNING, &hdev->flags);
> clear_bit(HCI_RUNNING, &hdev->flags);
> @@ -523,6 +527,13 @@ static int btusb_open(struct hci_dev *hd
> return err;
> }
>
> +static void btusb_stop_traffic(struct btusb_data *data)
> +{
> + usb_kill_anchored_urbs(&data->intr_anchor);
> + usb_kill_anchored_urbs(&data->bulk_anchor);
> + usb_kill_anchored_urbs(&data->isoc_anchor);
> +}
> +
> static int btusb_close(struct hci_dev *hdev)
> {
> struct btusb_data *data = hdev->driver_data;
> @@ -532,14 +543,12 @@ static int btusb_close(struct hci_dev *h
> if (!test_and_clear_bit(HCI_RUNNING, &hdev->flags))
> return 0;
>
> - clear_bit(BTUSB_ISOC_RUNNING, &data->flags);
> - usb_kill_anchored_urbs(&data->intr_anchor);
> + flush_work(&data->work);
>
> + clear_bit(BTUSB_ISOC_RUNNING, &data->flags);
> clear_bit(BTUSB_BULK_RUNNING, &data->flags);
> - usb_kill_anchored_urbs(&data->bulk_anchor);
> -
> clear_bit(BTUSB_INTR_RUNNING, &data->flags);
> - usb_kill_anchored_urbs(&data->intr_anchor);
> + btusb_stop_traffic(data);
>
> return 0;
> }
> @@ -672,8 +681,19 @@ static void btusb_notify(struct hci_dev
>
> BT_DBG("%s evt %d", hdev->name, evt);
>
> - if (evt == HCI_NOTIFY_CONN_ADD || evt == HCI_NOTIFY_CONN_DEL)
> - schedule_work(&data->work);
> + if (hdev->conn_hash.acl_num > 0) {
> + if (!test_and_set_bit(BTUSB_BULK_RUNNING, &data->flags)) {
> + if (btusb_submit_bulk_urb(hdev, GFP_ATOMIC) < 0)
> + clear_bit(BTUSB_BULK_RUNNING, &data->flags);
> + else
> + btusb_submit_bulk_urb(hdev, GFP_ATOMIC);
> + }
> + } else {
> + clear_bit(BTUSB_BULK_RUNNING, &data->flags);
> + usb_unlink_anchored_urbs(&data->bulk_anchor);
> + }
> +
> + schedule_work(&data->work);
> }
>
> static int inline __set_isoc_interface(struct hci_dev *hdev, int altsetting)
> @@ -724,18 +744,6 @@ static void btusb_work(struct work_struc
> struct btusb_data *data = container_of(work, struct btusb_data, work);
> struct hci_dev *hdev = data->hdev;
>
> - if (hdev->conn_hash.acl_num > 0) {
> - if (!test_and_set_bit(BTUSB_BULK_RUNNING, &data->flags)) {
> - if (btusb_submit_bulk_urb(hdev) < 0)
> - clear_bit(BTUSB_BULK_RUNNING, &data->flags);
> - else
> - btusb_submit_bulk_urb(hdev);
> - }
> - } else {
> - clear_bit(BTUSB_BULK_RUNNING, &data->flags);
> - usb_kill_anchored_urbs(&data->bulk_anchor);
> - }
> -
> if (hdev->conn_hash.sco_num > 0) {
> if (data->isoc_altsetting != 2) {
> clear_bit(BTUSB_ISOC_RUNNING, &data->flags);
> @@ -821,6 +829,7 @@ static int btusb_probe(struct usb_interf
> }
>
> data->udev = interface_to_usbdev(intf);
> + data->acl = intf;
>
> spin_lock_init(&data->lock);
>
> @@ -889,7 +898,7 @@ static int btusb_probe(struct usb_interf
>
> if (data->isoc) {
> err = usb_driver_claim_interface(&btusb_driver,
> - data->isoc, NULL);
> + data->isoc, data);
> if (err < 0) {
> hci_free_dev(hdev);
> kfree(data);
> @@ -921,20 +930,92 @@ static void btusb_disconnect(struct usb_
>
> hdev = data->hdev;
>
> - if (data->isoc)
> - usb_driver_release_interface(&btusb_driver, data->isoc);
> + /* make sure we have a reference */
> + __hci_dev_hold(hdev);
>
> - usb_set_intfdata(intf, NULL);
> + usb_set_intfdata(data->acl, NULL);
> + if (data->isoc)
> + usb_set_intfdata(data->isoc, NULL);
>
> + /* unregister before releasing any interface */
> hci_unregister_dev(hdev);
>
> + if (intf == data->isoc)
> + usb_driver_release_interface(&btusb_driver, data->acl);
> + else if (data->isoc)
> + usb_driver_release_interface(&btusb_driver, data->isoc);
> +
> + /* release the reference */
> + __hci_dev_put(hdev);
> hci_free_dev(hdev);
> }
>
> +static int btusb_suspend(struct usb_interface *intf, pm_message_t message)
> +{
> + struct btusb_data *data = usb_get_intfdata(intf);
> +
> + BT_DBG("%s called\n", __func__);
> +
> + if (data->susp_count++)
> + return 0;
> +
> + cancel_work_sync(&data->work);
> + btusb_stop_traffic(data);
> + usb_kill_anchored_urbs(&data->tx_anchor);
> + return 0;
> +}
> +
> +static int btusb_resume(struct usb_interface *intf)
> +{
> + struct btusb_data *data = usb_get_intfdata(intf);
> + struct hci_dev *hdev = data->hdev;
> + int ret;
> +
> + if (--data->susp_count)
> + return 0;
> + if (test_bit(HCI_RUNNING, &hdev->flags)) {
> + ret = btusb_submit_intr_urb(hdev, GFP_NOIO);
> + if (ret < 0) {
> + clear_bit(HCI_RUNNING, &hdev->flags);
> + return ret;
> + }
> + }
> +
> + if (hdev->conn_hash.acl_num > 0) {
> + ret = btusb_submit_bulk_urb(hdev, GFP_NOIO);
> + if (ret < 0) {
> + clear_bit(BTUSB_BULK_RUNNING, &data->flags);
> + return ret;
> + } else {
> + ret = btusb_submit_bulk_urb(hdev, GFP_NOIO);
> + if (ret < 0) {
> + clear_bit(BTUSB_BULK_RUNNING, &data->flags);
> + usb_kill_anchored_urbs(&data->bulk_anchor);
> + return ret;
> + }
> + }
> + }
> +
> + if (data->isoc) {
> + if (test_bit(BTUSB_ISOC_RUNNING, &data->flags)) {
> + ret = btusb_submit_isoc_urb(hdev);
> + if (ret < 0)
> + clear_bit(BTUSB_ISOC_RUNNING, &data->flags);
> + else
> + btusb_submit_isoc_urb(hdev);
> + }
> + }
> +
> + schedule_work(&data->work);
> + return 0;
> +}
> +
> static struct usb_driver btusb_driver = {
> .name = "btusb",
> .probe = btusb_probe,
> .disconnect = btusb_disconnect,
> + .suspend = btusb_suspend,
> + .resume = btusb_resume,
> .id_table = btusb_table,
> };
>
>
>
>
On Monday, 8 of September 2008, Marcel Holtmann wrote:
> Hi Oliver,
>
> > > I couldn't reproduce the breakage with suspend/resume support removed from
> > > usbhid.
> > >
> > > Let's just say it's related to BT, since your patch evidently fixes the problem
> > > for me and I have just no reason to believe it's anything else than BT.
> >
> > Very good.
> >
> > here it is again. You may need to cut the USB bits depending
> > on which tree you want to apply it to.
>
> do we have a version that could be applied to 2.6.27-rc5, because the
> patch depends on unmerged USB stuff. I tend to leave this for 2.6.28 for
> now.
I've been using the one below recently. It doesn't seem to depend on anything
unmerged.
Thanks,
Rafael
---
drivers/bluetooth/btusb.c | 145 +++++++++++++++++++++++++++++++++++-----------
1 file changed, 113 insertions(+), 32 deletions(-)
Index: linux-2.6/drivers/bluetooth/btusb.c
===================================================================
--- linux-2.6.orig/drivers/bluetooth/btusb.c
+++ linux-2.6/drivers/bluetooth/btusb.c
@@ -169,6 +169,7 @@ static struct usb_device_id blacklist_ta
struct btusb_data {
struct hci_dev *hdev;
struct usb_device *udev;
+ struct usb_interface *acl;
struct usb_interface *isoc;
spinlock_t lock;
@@ -176,6 +177,7 @@ struct btusb_data {
unsigned long flags;
struct work_struct work;
+ struct work_struct waker;
struct usb_anchor tx_anchor;
struct usb_anchor intr_anchor;
@@ -189,6 +191,7 @@ struct btusb_data {
struct usb_endpoint_descriptor *isoc_rx_ep;
int isoc_altsetting;
+ int susp_count;
};
static void btusb_intr_complete(struct urb *urb)
@@ -227,7 +230,7 @@ static void btusb_intr_complete(struct u
}
}
-static int btusb_submit_intr_urb(struct hci_dev *hdev)
+static int btusb_submit_intr_urb(struct hci_dev *hdev, gfp_t gfp)
{
struct btusb_data *data = hdev->driver_data;
struct urb *urb;
@@ -240,13 +243,13 @@ static int btusb_submit_intr_urb(struct
if (!data->intr_ep)
return -ENODEV;
- urb = usb_alloc_urb(0, GFP_ATOMIC);
+ urb = usb_alloc_urb(0, gfp);
if (!urb)
return -ENOMEM;
size = le16_to_cpu(data->intr_ep->wMaxPacketSize);
- buf = kmalloc(size, GFP_ATOMIC);
+ buf = kmalloc(size, gfp);
if (!buf) {
usb_free_urb(urb);
return -ENOMEM;
@@ -262,7 +265,7 @@ static int btusb_submit_intr_urb(struct
usb_anchor_urb(urb, &data->intr_anchor);
- err = usb_submit_urb(urb, GFP_ATOMIC);
+ err = usb_submit_urb(urb, gfp);
if (err < 0) {
BT_ERR("%s urb %p submission failed (%d)",
hdev->name, urb, -err);
@@ -311,7 +314,7 @@ static void btusb_bulk_complete(struct u
}
}
-static int btusb_submit_bulk_urb(struct hci_dev *hdev)
+static int btusb_submit_bulk_urb(struct hci_dev *hdev, gfp_t gfp)
{
struct btusb_data *data = hdev->driver_data;
struct urb *urb;
@@ -324,18 +327,19 @@ static int btusb_submit_bulk_urb(struct
if (!data->bulk_rx_ep)
return -ENODEV;
- urb = usb_alloc_urb(0, GFP_KERNEL);
+ urb = usb_alloc_urb(0, gfp);
if (!urb)
return -ENOMEM;
size = le16_to_cpu(data->bulk_rx_ep->wMaxPacketSize);
- buf = kmalloc(size, GFP_KERNEL);
+ buf = kmalloc(size, gfp);
if (!buf) {
usb_free_urb(urb);
return -ENOMEM;
}
+ usb_mark_last_busy(data->udev);
pipe = usb_rcvbulkpipe(data->udev, data->bulk_rx_ep->bEndpointAddress);
usb_fill_bulk_urb(urb, data->udev, pipe,
@@ -345,7 +349,7 @@ static int btusb_submit_bulk_urb(struct
usb_anchor_urb(urb, &data->bulk_anchor);
- err = usb_submit_urb(urb, GFP_KERNEL);
+ err = usb_submit_urb(urb, gfp);
if (err < 0) {
BT_ERR("%s urb %p submission failed (%d)",
hdev->name, urb, -err);
@@ -514,7 +518,7 @@ static int btusb_open(struct hci_dev *hd
if (test_and_set_bit(BTUSB_INTR_RUNNING, &data->flags))
return 0;
- err = btusb_submit_intr_urb(hdev);
+ err = btusb_submit_intr_urb(hdev, GFP_KERNEL);
if (err < 0) {
clear_bit(BTUSB_INTR_RUNNING, &hdev->flags);
clear_bit(HCI_RUNNING, &hdev->flags);
@@ -523,6 +527,13 @@ static int btusb_open(struct hci_dev *hd
return err;
}
+static void btusb_stop_traffic(struct btusb_data *data)
+{
+ usb_kill_anchored_urbs(&data->intr_anchor);
+ usb_kill_anchored_urbs(&data->bulk_anchor);
+ usb_kill_anchored_urbs(&data->isoc_anchor);
+}
+
static int btusb_close(struct hci_dev *hdev)
{
struct btusb_data *data = hdev->driver_data;
@@ -532,14 +543,12 @@ static int btusb_close(struct hci_dev *h
if (!test_and_clear_bit(HCI_RUNNING, &hdev->flags))
return 0;
- clear_bit(BTUSB_ISOC_RUNNING, &data->flags);
- usb_kill_anchored_urbs(&data->intr_anchor);
+ flush_work(&data->work);
+ clear_bit(BTUSB_ISOC_RUNNING, &data->flags);
clear_bit(BTUSB_BULK_RUNNING, &data->flags);
- usb_kill_anchored_urbs(&data->bulk_anchor);
-
clear_bit(BTUSB_INTR_RUNNING, &data->flags);
- usb_kill_anchored_urbs(&data->intr_anchor);
+ btusb_stop_traffic(data);
return 0;
}
@@ -672,8 +681,19 @@ static void btusb_notify(struct hci_dev
BT_DBG("%s evt %d", hdev->name, evt);
- if (evt == HCI_NOTIFY_CONN_ADD || evt == HCI_NOTIFY_CONN_DEL)
- schedule_work(&data->work);
+ if (hdev->conn_hash.acl_num > 0) {
+ if (!test_and_set_bit(BTUSB_BULK_RUNNING, &data->flags)) {
+ if (btusb_submit_bulk_urb(hdev, GFP_ATOMIC) < 0)
+ clear_bit(BTUSB_BULK_RUNNING, &data->flags);
+ else
+ btusb_submit_bulk_urb(hdev, GFP_ATOMIC);
+ }
+ } else {
+ clear_bit(BTUSB_BULK_RUNNING, &data->flags);
+ usb_unlink_anchored_urbs(&data->bulk_anchor);
+ }
+
+ schedule_work(&data->work);
}
static int inline __set_isoc_interface(struct hci_dev *hdev, int altsetting)
@@ -724,18 +744,6 @@ static void btusb_work(struct work_struc
struct btusb_data *data = container_of(work, struct btusb_data, work);
struct hci_dev *hdev = data->hdev;
- if (hdev->conn_hash.acl_num > 0) {
- if (!test_and_set_bit(BTUSB_BULK_RUNNING, &data->flags)) {
- if (btusb_submit_bulk_urb(hdev) < 0)
- clear_bit(BTUSB_BULK_RUNNING, &data->flags);
- else
- btusb_submit_bulk_urb(hdev);
- }
- } else {
- clear_bit(BTUSB_BULK_RUNNING, &data->flags);
- usb_kill_anchored_urbs(&data->bulk_anchor);
- }
-
if (hdev->conn_hash.sco_num > 0) {
if (data->isoc_altsetting != 2) {
clear_bit(BTUSB_ISOC_RUNNING, &data->flags);
@@ -821,6 +829,7 @@ static int btusb_probe(struct usb_interf
}
data->udev = interface_to_usbdev(intf);
+ data->acl = intf;
spin_lock_init(&data->lock);
@@ -889,7 +898,7 @@ static int btusb_probe(struct usb_interf
if (data->isoc) {
err = usb_driver_claim_interface(&btusb_driver,
- data->isoc, NULL);
+ data->isoc, data);
if (err < 0) {
hci_free_dev(hdev);
kfree(data);
@@ -921,20 +930,92 @@ static void btusb_disconnect(struct usb_
hdev = data->hdev;
- if (data->isoc)
- usb_driver_release_interface(&btusb_driver, data->isoc);
+ /* make sure we have a reference */
+ __hci_dev_hold(hdev);
- usb_set_intfdata(intf, NULL);
+ usb_set_intfdata(data->acl, NULL);
+ if (data->isoc)
+ usb_set_intfdata(data->isoc, NULL);
+ /* unregister before releasing any interface */
hci_unregister_dev(hdev);
+ if (intf == data->isoc)
+ usb_driver_release_interface(&btusb_driver, data->acl);
+ else if (data->isoc)
+ usb_driver_release_interface(&btusb_driver, data->isoc);
+
+ /* release the reference */
+ __hci_dev_put(hdev);
hci_free_dev(hdev);
}
+static int btusb_suspend(struct usb_interface *intf, pm_message_t message)
+{
+ struct btusb_data *data = usb_get_intfdata(intf);
+
+ BT_DBG("%s called\n", __func__);
+
+ if (data->susp_count++)
+ return 0;
+
+ cancel_work_sync(&data->work);
+ btusb_stop_traffic(data);
+ usb_kill_anchored_urbs(&data->tx_anchor);
+ return 0;
+}
+
+static int btusb_resume(struct usb_interface *intf)
+{
+ struct btusb_data *data = usb_get_intfdata(intf);
+ struct hci_dev *hdev = data->hdev;
+ int ret;
+
+ if (--data->susp_count)
+ return 0;
+ if (test_bit(HCI_RUNNING, &hdev->flags)) {
+ ret = btusb_submit_intr_urb(hdev, GFP_NOIO);
+ if (ret < 0) {
+ clear_bit(HCI_RUNNING, &hdev->flags);
+ return ret;
+ }
+ }
+
+ if (hdev->conn_hash.acl_num > 0) {
+ ret = btusb_submit_bulk_urb(hdev, GFP_NOIO);
+ if (ret < 0) {
+ clear_bit(BTUSB_BULK_RUNNING, &data->flags);
+ return ret;
+ } else {
+ ret = btusb_submit_bulk_urb(hdev, GFP_NOIO);
+ if (ret < 0) {
+ clear_bit(BTUSB_BULK_RUNNING, &data->flags);
+ usb_kill_anchored_urbs(&data->bulk_anchor);
+ return ret;
+ }
+ }
+ }
+
+ if (data->isoc) {
+ if (test_bit(BTUSB_ISOC_RUNNING, &data->flags)) {
+ ret = btusb_submit_isoc_urb(hdev);
+ if (ret < 0)
+ clear_bit(BTUSB_ISOC_RUNNING, &data->flags);
+ else
+ btusb_submit_isoc_urb(hdev);
+ }
+ }
+
+ schedule_work(&data->work);
+ return 0;
+}
+
static struct usb_driver btusb_driver = {
.name = "btusb",
.probe = btusb_probe,
.disconnect = btusb_disconnect,
+ .suspend = btusb_suspend,
+ .resume = btusb_resume,
.id_table = btusb_table,
};
Hi Oliver,
> > I couldn't reproduce the breakage with suspend/resume support removed from
> > usbhid.
> >
> > Let's just say it's related to BT, since your patch evidently fixes the problem
> > for me and I have just no reason to believe it's anything else than BT.
>
> Very good.
>
> here it is again. You may need to cut the USB bits depending
> on which tree you want to apply it to.
do we have a version that could be applied to 2.6.27-rc5, because the
patch depends on unmerged USB stuff. I tend to leave this for 2.6.28 for
now.
Regards
Marcel