Hi,
this implements very simple power management for btusb.
Comments?
Regards
Oliver
---
--- linux-2.6.27-rc3/drivers/bluetooth/btusb.c.alt2 2008-08-20 09:46:20.000000000 +0200
+++ linux-2.6.27-rc3/drivers/bluetooth/btusb.c 2008-08-20 10:59:08.000000000 +0200
@@ -228,7 +228,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;
@@ -241,13 +241,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;
@@ -263,7 +263,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);
@@ -312,7 +312,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;
@@ -325,13 +325,13 @@ static int btusb_submit_bulk_urb(struct
if (!data->bulk_rx_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->bulk_rx_ep->wMaxPacketSize);
- buf = kmalloc(size, GFP_ATOMIC);
+ buf = kmalloc(size, gfp);
if (!buf) {
usb_free_urb(urb);
return -ENOMEM;
@@ -346,7 +346,7 @@ static int btusb_submit_bulk_urb(struct
usb_anchor_urb(urb, &data->bulk_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);
@@ -509,21 +509,32 @@ static int btusb_open(struct hci_dev *hd
BT_DBG("%s", hdev->name);
+ err = usb_autopm_get_interface(data->acl);
+ if (err < 0)
+ return err;
if (test_and_set_bit(HCI_RUNNING, &hdev->flags))
return 0;
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_ATOMIC);
if (err < 0) {
clear_bit(BTUSB_INTR_RUNNING, &hdev->flags);
clear_bit(HCI_RUNNING, &hdev->flags);
+ usb_autopm_put_interface(data->acl);
}
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;
@@ -536,13 +547,10 @@ static int btusb_close(struct hci_dev *h
flush_work(&data->work);
clear_bit(BTUSB_ISOC_RUNNING, &data->flags);
- usb_kill_anchored_urbs(&data->intr_anchor);
-
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);
+ usb_autopm_put_interface(data->acl);
return 0;
}
@@ -677,10 +685,10 @@ static void btusb_notify(struct hci_dev
if (hdev->conn_hash.acl_num > 0) {
if (!test_and_set_bit(BTUSB_BULK_RUNNING, &data->flags)) {
- if (btusb_submit_bulk_urb(hdev) < 0)
+ if (btusb_submit_bulk_urb(hdev, GFP_ATOMIC) < 0)
clear_bit(BTUSB_BULK_RUNNING, &data->flags);
else
- btusb_submit_bulk_urb(hdev);
+ btusb_submit_bulk_urb(hdev, GFP_ATOMIC);
}
} else {
clear_bit(BTUSB_BULK_RUNNING, &data->flags);
@@ -944,11 +952,67 @@ static void btusb_disconnect(struct usb_
hci_free_dev(hdev);
}
+static int btusb_suspend(struct usb_interface *intf, pm_message_t message)
+{
+ struct btusb_data *data = usb_get_intfdata(intf);
+
+ 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 (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;
+ }
+ }
+ }
+
+ schedule_work(&data->work);
+ if (!data->isoc)
+ return 0;
+
+ 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);
+ }
+ 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,
+ .supports_autosuspend = 1,
};
static int __init btusb_init(void)
Am Mittwoch 20 August 2008 17:55:23 schrieb Alan Stern:
> On Wed, 20 Aug 2008, Marcel Holtmann wrote:
>
> > is there any way to make it default if we know it works all the time?
>
> No, probably not. But in general we don't know that. And we do know
> that a fair number of devices really are broken in this way -- lots of
> printers or scanners have this problem. Too many for us to want to
> keep a blacklist in the kernel, as Oliver mentioned.
>
> The idea is that some desktop management program, like hal, should be
> able to recognize which devices can autosuspend safely and then enable
> them. That way the whole problem is pushed out to userspace. :-)
You could write a udev rule.
Regards
Oliver
On Wed, 20 Aug 2008, Marcel Holtmann wrote:
> Hi Alan,
>
> > > finally I found the device and setting it to auto makes it actually
> > > suspend. Why is this not default? The driver has to enable it anyway.
> >
> > If there's no driver then it would take effect right away. This causes
> > problems for devices during startup -- the period of time from device
> > discovery to driver binding is long enough (because there's so much
> > other activity during bootup) that the device can autosuspend before
> > the driver is there to prevent it.
> >
> > For well-behaved devices this wouldn't matter. But unfortunately there
> > are lots of devices which break when they suspend. That's why
> > level = auto is not the default.
>
> is there any way to make it default if we know it works all the time?
No, probably not. But in general we don't know that. And we do know
that a fair number of devices really are broken in this way -- lots of
printers or scanners have this problem. Too many for us to want to
keep a blacklist in the kernel, as Oliver mentioned.
The idea is that some desktop management program, like hal, should be
able to recognize which devices can autosuspend safely and then enable
them. That way the whole problem is pushed out to userspace. :-)
There is one exception: The kernel enables autosuspend automatically
for hubs.
Alan Stern
Hi Alan,
> > finally I found the device and setting it to auto makes it actually
> > suspend. Why is this not default? The driver has to enable it anyway.
>
> If there's no driver then it would take effect right away. This causes
> problems for devices during startup -- the period of time from device
> discovery to driver binding is long enough (because there's so much
> other activity during bootup) that the device can autosuspend before
> the driver is there to prevent it.
>
> For well-behaved devices this wouldn't matter. But unfortunately there
> are lots of devices which break when they suspend. That's why
> level = auto is not the default.
is there any way to make it default if we know it works all the time?
Regards
Marcel
On Wed, 20 Aug 2008, Marcel Holtmann wrote:
> finally I found the device and setting it to auto makes it actually
> suspend. Why is this not default? The driver has to enable it anyway.
If there's no driver then it would take effect right away. This causes
problems for devices during startup -- the period of time from device
discovery to driver binding is long enough (because there's so much
other activity during bootup) that the device can autosuspend before
the driver is there to prevent it.
For well-behaved devices this wouldn't matter. But unfortunately there
are lots of devices which break when they suspend. That's why
level = auto is not the default.
Alan Stern
Am Mittwoch 20 August 2008 17:25:14 schrieb Marcel Holtmann:
Hi,
> finally I found the device and setting it to auto makes it actually
> suspend. Why is this not default? The driver has to enable it anyway.
for too many class drivers some devices survive suspension and others crash.
We don't want the lists of such devices in kernel space.
Regards
Oliver
Hi Oliver,
> > > > > > > > > > > > I tried to run this on my Quad G5, but I never see suspend() or resume()
> > > > > > > > > > > > called. Do I have to do something to make autosuspend work?
> > > > > > > > > > >
> > > > > > > > > > > echo "auto" > $(directry in sysfs for device)/power/level
> > > > > > > > > > > And you'll see power events in syslog only if you compile with
> > > > > > > > > > > CONFIG_USB_DEBUG
> > > > > > > > > >
> > > > > > > > > > I only seem to see power/wakeup entries. I don't have power/level. Am I
> > > > > > > > > > missing some configuration option?
> > > > > > > > >
> > > > > > > > > Probably CONFIG_USB_SUSPEND
> > > > > > > >
> > > > > > > > that one is on of course. Do you think it makes a difference that it is
> > > > > > > > PowerPC system I am testing this with?
> > > > > > >
> > > > > > > If it does make a difference, that's a bug. Runtime PM should work
> > > > > > > anyway.
> > > > > >
> > > > > > I really don't see any calls to suspend() and I am missing the
> > > > > > power/level option. This is the latest 2.6.27-rc3-git tree from Linus.
> > > > >
> > > > > Odd. Send /proc/config.gz please.
> > > >
> > > > there you go. It is a nice Quad G5 machine.
> > >
> > > That should work. Did you perhaps forget to remake your initrd?
> >
> > no initrd on this system.
>
> It should work. Are you perhaps looking at the interface level as opposed
> to the device level?
finally I found the device and setting it to auto makes it actually
suspend. Why is this not default? The driver has to enable it anyway.
Regards
Marcel
Am Mittwoch 20 August 2008 15:39:35 schrieb Marcel Holtmann:
> Hi Oliver,
>
> > > > > > > > > > > I tried to run this on my Quad G5, but I never see suspend() or resume()
> > > > > > > > > > > called. Do I have to do something to make autosuspend work?
> > > > > > > > > >
> > > > > > > > > > echo "auto" > $(directry in sysfs for device)/power/level
> > > > > > > > > > And you'll see power events in syslog only if you compile with
> > > > > > > > > > CONFIG_USB_DEBUG
> > > > > > > > >
> > > > > > > > > I only seem to see power/wakeup entries. I don't have power/level. Am I
> > > > > > > > > missing some configuration option?
> > > > > > > >
> > > > > > > > Probably CONFIG_USB_SUSPEND
> > > > > > >
> > > > > > > that one is on of course. Do you think it makes a difference that it is
> > > > > > > PowerPC system I am testing this with?
> > > > > >
> > > > > > If it does make a difference, that's a bug. Runtime PM should work
> > > > > > anyway.
> > > > >
> > > > > I really don't see any calls to suspend() and I am missing the
> > > > > power/level option. This is the latest 2.6.27-rc3-git tree from Linus.
> > > >
> > > > Odd. Send /proc/config.gz please.
> > >
> > > there you go. It is a nice Quad G5 machine.
> >
> > That should work. Did you perhaps forget to remake your initrd?
>
> no initrd on this system.
It should work. Are you perhaps looking at the interface level as opposed
to the device level?
Regards
Oliver
Hi Oliver,
> > > > > > > > > > I tried to run this on my Quad G5, but I never see suspend() or resume()
> > > > > > > > > > called. Do I have to do something to make autosuspend work?
> > > > > > > > >
> > > > > > > > > echo "auto" > $(directry in sysfs for device)/power/level
> > > > > > > > > And you'll see power events in syslog only if you compile with
> > > > > > > > > CONFIG_USB_DEBUG
> > > > > > > >
> > > > > > > > I only seem to see power/wakeup entries. I don't have power/level. Am I
> > > > > > > > missing some configuration option?
> > > > > > >
> > > > > > > Probably CONFIG_USB_SUSPEND
> > > > > >
> > > > > > that one is on of course. Do you think it makes a difference that it is
> > > > > > PowerPC system I am testing this with?
> > > > >
> > > > > If it does make a difference, that's a bug. Runtime PM should work
> > > > > anyway.
> > > >
> > > > I really don't see any calls to suspend() and I am missing the
> > > > power/level option. This is the latest 2.6.27-rc3-git tree from Linus.
> > >
> > > Odd. Send /proc/config.gz please.
> >
> > there you go. It is a nice Quad G5 machine.
>
> That should work. Did you perhaps forget to remake your initrd?
no initrd on this system.
Regards
Marcel
Am Mittwoch 20 August 2008 15:24:42 schrieb Marcel Holtmann:
> Hi Oliver,
>
> > > > > > > > > I tried to run this on my Quad G5, but I never see suspend() or resume()
> > > > > > > > > called. Do I have to do something to make autosuspend work?
> > > > > > > >
> > > > > > > > echo "auto" > $(directry in sysfs for device)/power/level
> > > > > > > > And you'll see power events in syslog only if you compile with
> > > > > > > > CONFIG_USB_DEBUG
> > > > > > >
> > > > > > > I only seem to see power/wakeup entries. I don't have power/level. Am I
> > > > > > > missing some configuration option?
> > > > > >
> > > > > > Probably CONFIG_USB_SUSPEND
> > > > >
> > > > > that one is on of course. Do you think it makes a difference that it is
> > > > > PowerPC system I am testing this with?
> > > >
> > > > If it does make a difference, that's a bug. Runtime PM should work
> > > > anyway.
> > >
> > > I really don't see any calls to suspend() and I am missing the
> > > power/level option. This is the latest 2.6.27-rc3-git tree from Linus.
> >
> > Odd. Send /proc/config.gz please.
>
> there you go. It is a nice Quad G5 machine.
That should work. Did you perhaps forget to remake your initrd?
Regards
Oliver
Hi Oliver,
> > > > > > > > I tried to run this on my Quad G5, but I never see suspend() or resume()
> > > > > > > > called. Do I have to do something to make autosuspend work?
> > > > > > >
> > > > > > > echo "auto" > $(directry in sysfs for device)/power/level
> > > > > > > And you'll see power events in syslog only if you compile with
> > > > > > > CONFIG_USB_DEBUG
> > > > > >
> > > > > > I only seem to see power/wakeup entries. I don't have power/level. Am I
> > > > > > missing some configuration option?
> > > > >
> > > > > Probably CONFIG_USB_SUSPEND
> > > >
> > > > that one is on of course. Do you think it makes a difference that it is
> > > > PowerPC system I am testing this with?
> > >
> > > If it does make a difference, that's a bug. Runtime PM should work
> > > anyway.
> >
> > I really don't see any calls to suspend() and I am missing the
> > power/level option. This is the latest 2.6.27-rc3-git tree from Linus.
>
> Odd. Send /proc/config.gz please.
there you go. It is a nice Quad G5 machine.
Regards
Marcel
Am Mittwoch 20 August 2008 15:13:04 schrieb Marcel Holtmann:
> Hi Oliver,
>
> > > > > > > I tried to run this on my Quad G5, but I never see suspend() or resume()
> > > > > > > called. Do I have to do something to make autosuspend work?
> > > > > >
> > > > > > echo "auto" > $(directry in sysfs for device)/power/level
> > > > > > And you'll see power events in syslog only if you compile with
> > > > > > CONFIG_USB_DEBUG
> > > > >
> > > > > I only seem to see power/wakeup entries. I don't have power/level. Am I
> > > > > missing some configuration option?
> > > >
> > > > Probably CONFIG_USB_SUSPEND
> > >
> > > that one is on of course. Do you think it makes a difference that it is
> > > PowerPC system I am testing this with?
> >
> > If it does make a difference, that's a bug. Runtime PM should work
> > anyway.
>
> I really don't see any calls to suspend() and I am missing the
> power/level option. This is the latest 2.6.27-rc3-git tree from Linus.
Odd. Send /proc/config.gz please.
Regards
Oliver
Hi Oliver,
> > > > > > I tried to run this on my Quad G5, but I never see suspend() or resume()
> > > > > > called. Do I have to do something to make autosuspend work?
> > > > >
> > > > > echo "auto" > $(directry in sysfs for device)/power/level
> > > > > And you'll see power events in syslog only if you compile with
> > > > > CONFIG_USB_DEBUG
> > > >
> > > > I only seem to see power/wakeup entries. I don't have power/level. Am I
> > > > missing some configuration option?
> > >
> > > Probably CONFIG_USB_SUSPEND
> >
> > that one is on of course. Do you think it makes a difference that it is
> > PowerPC system I am testing this with?
>
> If it does make a difference, that's a bug. Runtime PM should work
> anyway.
I really don't see any calls to suspend() and I am missing the
power/level option. This is the latest 2.6.27-rc3-git tree from Linus.
Regards
Marcel
Am Mittwoch 20 August 2008 15:05:54 schrieb Marcel Holtmann:
> Hi Oliver,
>
> > > > > I tried to run this on my Quad G5, but I never see suspend() or resume()
> > > > > called. Do I have to do something to make autosuspend work?
> > > >
> > > > echo "auto" > $(directry in sysfs for device)/power/level
> > > > And you'll see power events in syslog only if you compile with
> > > > CONFIG_USB_DEBUG
> > >
> > > I only seem to see power/wakeup entries. I don't have power/level. Am I
> > > missing some configuration option?
> >
> > Probably CONFIG_USB_SUSPEND
>
> that one is on of course. Do you think it makes a difference that it is
> PowerPC system I am testing this with?
If it does make a difference, that's a bug. Runtime PM should work
anyway.
Regards
Oliver
Hi Oliver,
> > > > I tried to run this on my Quad G5, but I never see suspend() or resume()
> > > > called. Do I have to do something to make autosuspend work?
> > >
> > > echo "auto" > $(directry in sysfs for device)/power/level
> > > And you'll see power events in syslog only if you compile with
> > > CONFIG_USB_DEBUG
> >
> > I only seem to see power/wakeup entries. I don't have power/level. Am I
> > missing some configuration option?
>
> Probably CONFIG_USB_SUSPEND
that one is on of course. Do you think it makes a difference that it is
PowerPC system I am testing this with?
Regards
Marcel
Am Mittwoch 20 August 2008 15:00:50 schrieb Marcel Holtmann:
> Hi Oliver,
>
> > > I tried to run this on my Quad G5, but I never see suspend() or resume()
> > > called. Do I have to do something to make autosuspend work?
> >
> > echo "auto" > $(directry in sysfs for device)/power/level
> > And you'll see power events in syslog only if you compile with
> > CONFIG_USB_DEBUG
>
> I only seem to see power/wakeup entries. I don't have power/level. Am I
> missing some configuration option?
Probably CONFIG_USB_SUSPEND
HTH
Oliver
On Wed, Aug 20, 2008 at 03:00:50PM +0200, ext Marcel Holtmann wrote:
> I only seem to see power/wakeup entries. I don't have power/level. Am I
> missing some configuration option?
CONFIG_USB_SUSPEND
--
balbi
Hi Oliver,
> > I tried to run this on my Quad G5, but I never see suspend() or resume()
> > called. Do I have to do something to make autosuspend work?
>
> echo "auto" > $(directry in sysfs for device)/power/level
> And you'll see power events in syslog only if you compile with
> CONFIG_USB_DEBUG
I only seem to see power/wakeup entries. I don't have power/level. Am I
missing some configuration option?
Regards
Marcel
Am Mittwoch 20 August 2008 14:34:43 schrieb Marcel Holtmann:
> Hi Oliver,
>
> > > What are the semantics for usb_autopm_{get,put}_interface? Can we expect
> > > to always get a suspend() and resume() callback?
> >
> > usb_autopm_get_interface() guarantees that after it returns successsfully
> > the interface (and teh device with it) remains unautosuspended until you
> > call usb_autopm_put_interface. The calls stack with a counter.
> >
> > You'll get a resume() callback only if the device was actually suspended.
>
> so a device didn't start out suspended? Too bad, otherwise you could cut
> the logic in the driver a lot.
Devices can have multiple interfaces. Therefore this guarantee is impossible.
> I tried to run this on my Quad G5, but I never see suspend() or resume()
> called. Do I have to do something to make autosuspend work?
echo "auto" > $(directry in sysfs for device)/power/level
And you'll see power events in syslog only if you compile with
CONFIG_USB_DEBUG
Regards
Oliver
Hi Oliver,
> > What are the semantics for usb_autopm_{get,put}_interface? Can we expect
> > to always get a suspend() and resume() callback?
>
> usb_autopm_get_interface() guarantees that after it returns successsfully
> the interface (and teh device with it) remains unautosuspended until you
> call usb_autopm_put_interface. The calls stack with a counter.
>
> You'll get a resume() callback only if the device was actually suspended.
so a device didn't start out suspended? Too bad, otherwise you could cut
the logic in the driver a lot.
I tried to run this on my Quad G5, but I never see suspend() or resume()
called. Do I have to do something to make autosuspend work?
Regards
Marcel
Am Mittwoch 20 August 2008 12:18:04 schrieb Marcel Holtmann:
> Hi Oliver,
>
> > this implements very simple power management for btusb.
> > Comments?
>
> looks a little but too much.
How so?
> What are the semantics for usb_autopm_{get,put}_interface? Can we expect
> to always get a suspend() and resume() callback?
usb_autopm_get_interface() guarantees that after it returns successsfully
the interface (and teh device with it) remains unautosuspended until you
call usb_autopm_put_interface. The calls stack with a counter.
You'll get a resume() callback only if the device was actually suspended.
> So in open(), do we have to actually start the interrupt URB or can we
> just wait for resume() to be happening?
We have to start the URB.
> I don't wanna misuse the *_RUNNING bits. They are only there to make
> sure that we non re-submit the URBs.
Do you suggest making a copy of these bits at suspend() time?
Regards
Oliver
Hi Oliver,
> this implements very simple power management for btusb.
> Comments?
looks a little but too much.
What are the semantics for usb_autopm_{get,put}_interface? Can we expect
to always get a suspend() and resume() callback?
So in open(), do we have to actually start the interrupt URB or can we
just wait for resume() to be happening?
I don't wanna misuse the *_RUNNING bits. They are only there to make
sure that we non re-submit the URBs.
Regards
Marcel