Hi,
here's my current version of btusb with SCO support. This is preliminary.
I am still looking at a way to delay using the higher altsettings until SCO
is actually used, but the timeouts seem to be too long to do the obvious.
Furthermore, could somebody point me at the description of a test setup
for bluetooth?
Regards
Oliver
---
--- linux-2.6/drivers/bluetooth/btusb.c 2008-07-31 10:26:38.000000000 +0200
+++ linux-btusb/drivers/bluetooth/btusb.c 2008-07-31 13:57:17.000000000 +0200
@@ -32,6 +32,8 @@
#include <linux/usb.h>
+#include "hci_usb.h"
+
#include <net/bluetooth/bluetooth.h>
#include <net/bluetooth/hci_core.h>
@@ -41,21 +43,127 @@
#define BT_DBG(D...)
#endif
-#define VERSION "0.1"
+#define VERSION "0.3"
static struct usb_device_id btusb_table[] = {
/* Generic Bluetooth USB device */
{ USB_DEVICE_INFO(0xe0, 0x01, 0x01) },
+ /* AVM BlueFRITZ! USB v2.0 */
+ { USB_DEVICE(0x057c, 0x3800) },
+
+ /* Bluetooth Ultraport Module from IBM */
+ { USB_DEVICE(0x04bf, 0x030a) },
+
+ /* ALPS Modules with non-standard id */
+ { USB_DEVICE(0x044e, 0x3001) },
+ { USB_DEVICE(0x044e, 0x3002) },
+
+ /* Ericsson with non-standard id */
+ { USB_DEVICE(0x0bdb, 0x1002) },
+
+ /* Canyon CN-BTU1 with HID interfaces */
+ { USB_DEVICE(0x0c10, 0x0000), .driver_info = HCI_RESET },
+
{ } /* Terminating entry */
};
MODULE_DEVICE_TABLE(usb, btusb_table);
static struct usb_device_id blacklist_table[] = {
+ /* CSR BlueCore devices */
+ { USB_DEVICE(0x0a12, 0x0001), .driver_info = HCI_CSR },
+
+ /* Broadcom BCM2033 without firmware */
+ { USB_DEVICE(0x0a5c, 0x2033), .driver_info = HCI_IGNORE },
+
+ /* Broadcom BCM2035 */
+ { USB_DEVICE(0x0a5c, 0x2035), .driver_info = HCI_RESET | HCI_WRONG_SCO_MTU },
+ { USB_DEVICE(0x0a5c, 0x200a), .driver_info = HCI_RESET | HCI_WRONG_SCO_MTU },
+
+ /* Broadcom BCM2045 */
+ { USB_DEVICE(0x0a5c, 0x2039), .driver_info = HCI_RESET | HCI_WRONG_SCO_MTU },
+ { USB_DEVICE(0x0a5c, 0x2101), .driver_info = HCI_RESET | HCI_WRONG_SCO_MTU },
+
+ /* IBM/Lenovo ThinkPad with Broadcom chip */
+ { USB_DEVICE(0x0a5c, 0x201e), .driver_info = HCI_RESET | HCI_WRONG_SCO_MTU },
+ { USB_DEVICE(0x0a5c, 0x2110), .driver_info = HCI_RESET | HCI_WRONG_SCO_MTU },
+
+ /* Targus ACB10US */
+ { USB_DEVICE(0x0a5c, 0x2100), .driver_info = HCI_RESET },
+
+ /* ANYCOM Bluetooth USB-200 and USB-250 */
+ { USB_DEVICE(0x0a5c, 0x2111), .driver_info = HCI_RESET },
+
+ /* HP laptop with Broadcom chip */
+ { USB_DEVICE(0x03f0, 0x171d), .driver_info = HCI_RESET | HCI_WRONG_SCO_MTU },
+
+ /* Dell laptop with Broadcom chip */
+ { USB_DEVICE(0x413c, 0x8126), .driver_info = HCI_RESET | HCI_WRONG_SCO_MTU },
+
+ /* Microsoft Wireless Transceiver for Bluetooth 2.0 */
+ { USB_DEVICE(0x045e, 0x009c), .driver_info = HCI_RESET },
+
+ /* Kensington Bluetooth USB adapter */
+ { USB_DEVICE(0x047d, 0x105d), .driver_info = HCI_RESET },
+ { USB_DEVICE(0x047d, 0x105e), .driver_info = HCI_RESET | HCI_WRONG_SCO_MTU },
+
+ /* ISSC Bluetooth Adapter v3.1 */
+ { USB_DEVICE(0x1131, 0x1001), .driver_info = HCI_RESET },
+
+ /* RTX Telecom based adapters with buggy SCO support */
+ { USB_DEVICE(0x0400, 0x0807), .driver_info = HCI_BROKEN_ISOC },
+ { USB_DEVICE(0x0400, 0x080a), .driver_info = HCI_BROKEN_ISOC },
+
+ /* CONWISE Technology based adapters with buggy SCO support */
+ { USB_DEVICE(0x0e5e, 0x6622), .driver_info = HCI_BROKEN_ISOC },
+
+ /* Belkin F8T012 and F8T013 devices */
+ { USB_DEVICE(0x050d, 0x0012), .driver_info = HCI_RESET | HCI_WRONG_SCO_MTU },
+ { USB_DEVICE(0x050d, 0x0013), .driver_info = HCI_RESET | HCI_WRONG_SCO_MTU },
+
+ /* Digianswer devices */
+ { USB_DEVICE(0x08fd, 0x0001), .driver_info = HCI_DIGIANSWER },
+ { USB_DEVICE(0x08fd, 0x0002), .driver_info = HCI_IGNORE },
+
+ /* CSR BlueCore Bluetooth Sniffer */
+ { USB_DEVICE(0x0a12, 0x0002), .driver_info = HCI_SNIFFER },
+
+ /* Frontline ComProbe Bluetooth Sniffer */
+ { USB_DEVICE(0x16d3, 0x0002), .driver_info = HCI_SNIFFER },
{ } /* Terminating entry */
};
+static struct usb_driver btusb_driver;
+
+static int reset = 0;
+module_param(reset, bool, 0644);
+MODULE_PARM_DESC(reset, "Send HCI reset command on initialization");
+
+static int force_scofix;
+module_param(force_scofix, bool, 0644);
+MODULE_PARM_DESC(force_scofix, "Force fixup of wrong SCO buffers size");
+
+static int disable_scofix;
+module_param(disable_scofix, bool, 0644);
+MODULE_PARM_DESC(disable_scofix, "Disable fixup of wrong SCO buffer size");
+
+static int ignore_csr;
+module_param(ignore_csr, bool, 0644);
+MODULE_PARM_DESC(ignore_csr, "Ignore devices with id 0a12:0001");
+
+static int ignore_sniffer;
+module_param(ignore_sniffer, bool, 0644);
+MODULE_PARM_DESC(ignore_sniffer, "Ignore devices with id 0a12:0002");
+
+static int ignore_dga;
+module_param(ignore_dga, bool, 0644);
+MODULE_PARM_DESC(ignore_dga, "Ignore devices with id 08fd:0001");
+
+static int override_ignore;
+module_param(override_ignore, bool, 0644);
+MODULE_PARM_DESC(override_ignore, "Drive blacklisted devices");
+
#define BTUSB_INTR_RUNNING 0
#define BTUSB_BULK_RUNNING 1
@@ -67,17 +175,22 @@ struct btusb_data {
unsigned long flags;
- struct work_struct work;
-
struct usb_anchor tx_anchor;
struct usb_anchor intr_anchor;
struct usb_anchor bulk_anchor;
+ struct usb_anchor sco_anchor;
struct usb_endpoint_descriptor *intr_ep;
struct usb_endpoint_descriptor *bulk_tx_ep;
struct usb_endpoint_descriptor *bulk_rx_ep;
+ struct usb_host_endpoint *isoc_out_ep;
+ struct usb_host_endpoint *isoc_in_ep;
+
+ struct usb_interface *iso;
};
+static void btusb_start_sco(struct btusb_data *data);
+
static void btusb_intr_complete(struct urb *urb)
{
struct hci_dev *hdev = urb->context;
@@ -257,26 +370,81 @@ done:
kfree_skb(skb);
}
+static void btusb_sco_tx_complete(struct urb *urb)
+{
+ struct sk_buff *skb = (struct sk_buff *)urb->context;
+
+ kfree_skb(skb);
+}
+
+static void btusb_sco_rx_complete(struct urb *urb)
+{
+ struct btusb_data *data = urb->context;
+ int status = urb->status;
+ int i;
+
+ if (!test_bit(HCI_RUNNING, &data->flags))
+ goto out;
+ if (status < 0)
+ goto out;
+
+ for (i=0; i < urb->number_of_packets; i++) {
+ BT_DBG("desc %d status %d offset %d len %d", i,
+ urb->iso_frame_desc[i].status,
+ urb->iso_frame_desc[i].offset,
+ urb->iso_frame_desc[i].actual_length);
+
+ if (!urb->iso_frame_desc[i].status) {
+ data->hdev->stat.byte_rx += urb->iso_frame_desc[i].actual_length;
+ hci_recv_fragment(data->hdev, HCI_SCODATA_PKT,
+ urb->transfer_buffer + urb->iso_frame_desc[i].offset,
+ urb->iso_frame_desc[i].actual_length);
+ }
+ }
+
+ usb_anchor_urb(urb, &data->sco_anchor);
+ i = usb_submit_urb(urb, GFP_ATOMIC);
+ if (i < 0) {
+ usb_unanchor_urb(urb);
+out:
+ kfree(urb->transfer_buffer);
+ }
+}
+
static int btusb_open(struct hci_dev *hdev)
{
struct btusb_data *data = hdev->driver_data;
+ static DEFINE_MUTEX(open_mut);
int err;
BT_DBG("%s", hdev->name);
+ mutex_lock(&open_mut);
if (test_and_set_bit(HCI_RUNNING, &hdev->flags))
- return 0;
+ goto out;
if (test_and_set_bit(BTUSB_INTR_RUNNING, &data->flags))
- return 0;
+ goto out;;
+ err = usb_set_interface(data->udev, 1, 2);
+ if (err < 0)
+ goto err_out;
err = btusb_submit_intr_urb(hdev);
- if (err < 0) {
- clear_bit(BTUSB_INTR_RUNNING, &hdev->flags);
- clear_bit(HCI_RUNNING, &hdev->flags);
- }
+ if (err < 0)
+ goto err_out_setting;
- return err;
+ btusb_start_sco(data);
+out:
+ mutex_unlock(&open_mut);
+ return 0;
+
+err_out_setting:
+ usb_set_interface(data->udev, 1, 0);
+err_out:
+ clear_bit(BTUSB_INTR_RUNNING, &hdev->flags);
+ clear_bit(HCI_RUNNING, &hdev->flags);
+ mutex_unlock(&open_mut);
+ return -EIO;
}
static int btusb_close(struct hci_dev *hdev)
@@ -294,6 +462,9 @@ static int btusb_close(struct hci_dev *h
clear_bit(BTUSB_INTR_RUNNING, &data->flags);
usb_kill_anchored_urbs(&data->intr_anchor);
+ usb_kill_anchored_urbs(&data->sco_anchor);
+ usb_set_interface(data->udev, 1, 0);
+
return 0;
}
@@ -304,15 +475,36 @@ static int btusb_flush(struct hci_dev *h
BT_DBG("%s", hdev->name);
usb_kill_anchored_urbs(&data->tx_anchor);
-
+ usb_kill_anchored_urbs(&data->sco_anchor);
return 0;
}
+static void btusb_fill_isoc_desc(struct urb *urb, int len, int mtu)
+{
+ int offset = 0, i;
+
+ BT_DBG("len %d mtu %d", len, mtu);
+
+ for (i=0; i < HCI_MAX_ISOC_FRAMES && len >= mtu; i++, offset += mtu, len -= mtu) {
+ urb->iso_frame_desc[i].offset = offset;
+ urb->iso_frame_desc[i].length = mtu;
+ BT_DBG("desc %d offset %d len %d", i, offset, mtu);
+ }
+ if (len && i < HCI_MAX_ISOC_FRAMES) {
+ urb->iso_frame_desc[i].offset = offset;
+ urb->iso_frame_desc[i].length = len;
+ BT_DBG("desc %d offset %d len %d", i, offset, len);
+ i++;
+ }
+ urb->number_of_packets = i;
+}
+
static int btusb_send_frame(struct sk_buff *skb)
{
struct hci_dev *hdev = (struct hci_dev *) skb->dev;
struct btusb_data *data = hdev->driver_data;
struct usb_ctrlrequest *dr;
+ struct usb_anchor *queue = &data->tx_anchor;
struct urb *urb;
unsigned int pipe;
int err;
@@ -363,15 +555,32 @@ static int btusb_send_frame(struct sk_bu
break;
case HCI_SCODATA_PKT:
+ if (!data->iso)
+ return -ENODEV;
+ queue = &data->sco_anchor;
+ urb = usb_alloc_urb(HCI_MAX_ISOC_FRAMES, GFP_ATOMIC);
+ if (!urb)
+ return -ENOMEM;
+
+ urb->context = skb;
+ urb->dev = data->udev;
+ urb->pipe = usb_sndisocpipe(data->udev, data->isoc_out_ep->desc.bEndpointAddress);
+ urb->complete = btusb_sco_tx_complete;
+ urb->transfer_flags = URB_ISO_ASAP;
+
+ urb->interval = data->isoc_out_ep->desc.bInterval;
+
+ urb->transfer_buffer = skb->data;
+ urb->transfer_buffer_length = skb->len;
hdev->stat.sco_tx++;
- kfree_skb(skb);
- return 0;
+ btusb_fill_isoc_desc(urb, skb->len, le16_to_cpu(data->isoc_out_ep->desc.wMaxPacketSize));
+ break;
default:
return -EILSEQ;
}
- usb_anchor_urb(urb, &data->tx_anchor);
+ usb_anchor_urb(urb, queue);
err = usb_submit_urb(urb, GFP_ATOMIC);
if (err < 0) {
@@ -396,43 +605,65 @@ static void btusb_destruct(struct hci_de
static void btusb_notify(struct hci_dev *hdev, unsigned int evt)
{
- struct btusb_data *data = hdev->driver_data;
-
BT_DBG("%s evt %d", hdev->name, evt);
-
- if (evt == HCI_NOTIFY_CONN_ADD || evt == HCI_NOTIFY_CONN_DEL)
- schedule_work(&data->work);
}
-static void btusb_work(struct work_struct *work)
+static void btusb_start_sco(struct btusb_data *data)
{
- struct btusb_data *data = container_of(work, struct btusb_data, work);
- struct hci_dev *hdev = data->hdev;
+ struct urb *urb;
+ int mtu, size, err;
+ u8 *buf;
+
+ mtu = le16_to_cpu(data->isoc_in_ep->desc.wMaxPacketSize);
+ size = mtu * HCI_MAX_ISOC_FRAMES;
- if (hdev->conn_hash.acl_num == 0) {
- clear_bit(BTUSB_BULK_RUNNING, &data->flags);
- usb_kill_anchored_urbs(&data->bulk_anchor);
+ urb = usb_alloc_urb(HCI_MAX_ISOC_FRAMES, GFP_KERNEL);
+ if (!urb)
return;
- }
- 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);
+ buf = kmalloc(size, GFP_KERNEL);
+ if (!buf) {
+ usb_free_urb(urb);
+ return;
}
+
+ urb->context = data;
+ urb->dev = data->udev;
+ urb->pipe = usb_rcvisocpipe(data->udev, data->isoc_in_ep->desc.bEndpointAddress);
+ urb->complete = btusb_sco_rx_complete;
+
+ urb->interval = data->isoc_in_ep->desc.bInterval;
+
+ urb->transfer_buffer_length = size;
+ urb->transfer_buffer = buf;
+ urb->transfer_flags = URB_ISO_ASAP;
+
+ btusb_fill_isoc_desc(urb, size, mtu);
+
+ usb_anchor_urb(urb, &data->sco_anchor);
+
+ err = usb_submit_urb(urb, GFP_KERNEL);
+ if (err < 0)
+ usb_unanchor_urb(urb);
+ usb_free_urb(urb);
}
static int btusb_probe(struct usb_interface *intf,
const struct usb_device_id *id)
{
struct usb_endpoint_descriptor *ep_desc;
+ struct usb_interface *iso_intf;
struct btusb_data *data;
struct hci_dev *hdev;
- int i, err;
+ int i, a, err;
+ struct usb_host_endpoint *isoc_out_ep = NULL;
+ struct usb_host_endpoint *isoc_in_ep = NULL;
+ struct usb_host_endpoint *ep;
+ struct usb_host_interface *uif;
BT_DBG("intf %p id %p", intf, id);
+ /* interface numbers are hardcoded in the spec */
if (intf->cur_altsetting->desc.bInterfaceNumber != 0)
return -ENODEV;
@@ -443,6 +674,15 @@ static int btusb_probe(struct usb_interf
id = match;
}
+ if (id->driver_info == HCI_IGNORE && !override_ignore)
+ return -ENODEV;
+ if (ignore_sniffer && id->driver_info & HCI_SNIFFER)
+ return -ENODEV;
+ if (ignore_csr && id->driver_info & HCI_CSR)
+ return -ENODEV;
+ if (ignore_dga && id->driver_info & HCI_DIGIANSWER)
+ return -ENODEV;
+
data = kzalloc(sizeof(*data), GFP_KERNEL);
if (!data)
return -ENOMEM;
@@ -473,13 +713,44 @@ static int btusb_probe(struct usb_interf
data->udev = interface_to_usbdev(intf);
- spin_lock_init(&data->lock);
+ /* for sound */
+ iso_intf = usb_ifnum_to_if(data->udev, 1);
+ if (iso_intf && !(id->driver_info & (HCI_BROKEN_ISOC | HCI_SNIFFER))) {
+ err = usb_driver_claim_interface(&btusb_driver,
+ iso_intf,
+ data);
+
+ if (!err)
+ data->iso = iso_intf;
+
+ for (a = 0; a < iso_intf->num_altsetting; a++) {
+ uif = &iso_intf->altsetting[a];
+ for (i = 0; i < uif->desc.bNumEndpoints; i++) {
+ ep = &uif->endpoint[i];
+
+ switch (ep->desc.bmAttributes & USB_ENDPOINT_XFERTYPE_MASK) {
+ case USB_ENDPOINT_XFER_ISOC:
+ if (uif->desc.bAlternateSetting != 2)
+ break;
+
+ if (ep->desc.bEndpointAddress & USB_DIR_IN)
+ isoc_in_ep = ep;
+ else
+ isoc_out_ep = ep;
+ break;
+ }
+ }
+ }
+ data->isoc_in_ep = isoc_in_ep;
+ data->isoc_out_ep = isoc_out_ep;
+ }
- INIT_WORK(&data->work, btusb_work);
+ spin_lock_init(&data->lock);
init_usb_anchor(&data->tx_anchor);
init_usb_anchor(&data->intr_anchor);
init_usb_anchor(&data->bulk_anchor);
+ init_usb_anchor(&data->sco_anchor);
hdev = hci_alloc_dev();
if (!hdev) {
@@ -503,10 +774,18 @@ static int btusb_probe(struct usb_interf
hdev->owner = THIS_MODULE;
- set_bit(HCI_QUIRK_RESET_ON_INIT, &hdev->quirks);
+ if (reset || id->driver_info & HCI_RESET)
+ set_bit(HCI_QUIRK_RESET_ON_INIT, &hdev->quirks);
+
+ if (force_scofix || id->driver_info & HCI_WRONG_SCO_MTU) {
+ if (!disable_scofix)
+ set_bit(HCI_QUIRK_FIXUP_BUFFER_SIZE, &hdev->quirks);
+ }
err = hci_register_dev(hdev);
if (err < 0) {
+ if (iso_intf)
+ usb_driver_release_interface(&btusb_driver, iso_intf);
hci_free_dev(hdev);
kfree(data);
return err;
@@ -524,12 +803,16 @@ static void btusb_disconnect(struct usb_
BT_DBG("intf %p", intf);
+ /* ignore second interface */
if (!data)
return;
hdev = data->hdev;
- usb_set_intfdata(intf, NULL);
+ if (intf == data->iso)
+ usb_set_intfdata(intf, NULL);
+ else if (data->iso)
+ usb_set_intfdata(data->iso, NULL);
hci_unregister_dev(hdev);
Am Donnerstag 31 Juli 2008 16:03:13 schrieb Marcel Holtmann:
> yeah, I was afraid of that switching the alternate settings is not as =A0
> easy as the spec. might think. Personally I think it is a broken =A0
> specification anyway. So the best assumption is that we have one SCO =A0
> connection and that is either 8-bit or 16-bit. So we do the switching =A0
> when we get the notify() callback for a changed voice setting.
That's problematic because in the notify() callback you cannot sleep,
can you? In addition, a device is legally allowed to take a lot of time
to switch altsettings.
Regards
Oliver
Hi Oliver,
> here's my current version of btusb with SCO support. This is
> preliminary.
> I am still looking at a way to delay using the higher altsettings
> until SCO
> is actually used, but the timeouts seem to be too long to do the
> obvious.
yeah, I was afraid of that switching the alternate settings is not as
easy as the spec. might think. Personally I think it is a broken
specification anyway. So the best assumption is that we have one SCO
connection and that is either 8-bit or 16-bit. So we do the switching
when we get the notify() callback for a changed voice setting.
> Furthermore, could somebody point me at the description of a test
> setup
> for bluetooth?
So scotest works for basic testing and for the rest get a Bluetooth
headset and then use the audio setup from wiki.bluez.org.
> --- linux-2.6/drivers/bluetooth/btusb.c 2008-07-31
> 10:26:38.000000000 +0200
> +++ linux-btusb/drivers/bluetooth/btusb.c 2008-07-31
> 13:57:17.000000000 +0200
> @@ -32,6 +32,8 @@
>
> #include <linux/usb.h>
>
> +#include "hci_usb.h"
Can we please not do that and just include the parts from hci_usb.h
that we need directly here.
> static struct usb_device_id btusb_table[] = {
> /* Generic Bluetooth USB device */
> { USB_DEVICE_INFO(0xe0, 0x01, 0x01) },
>
> + /* AVM BlueFRITZ! USB v2.0 */
> + { USB_DEVICE(0x057c, 0x3800) },
> +
> + /* Bluetooth Ultraport Module from IBM */
> + { USB_DEVICE(0x04bf, 0x030a) },
> +
> + /* ALPS Modules with non-standard id */
> + { USB_DEVICE(0x044e, 0x3001) },
> + { USB_DEVICE(0x044e, 0x3002) },
> +
> + /* Ericsson with non-standard id */
> + { USB_DEVICE(0x0bdb, 0x1002) },
> +
> + /* Canyon CN-BTU1 with HID interfaces */
> + { USB_DEVICE(0x0c10, 0x0000), .driver_info = HCI_RESET },
> +
> { } /* Terminating entry */
> };
>
> MODULE_DEVICE_TABLE(usb, btusb_table);
>
> static struct usb_device_id blacklist_table[] = {
> + /* CSR BlueCore devices */
> + { USB_DEVICE(0x0a12, 0x0001), .driver_info = HCI_CSR },
> +
> + /* Broadcom BCM2033 without firmware */
> + { USB_DEVICE(0x0a5c, 0x2033), .driver_info = HCI_IGNORE },
> +
> + /* Broadcom BCM2035 */
> + { USB_DEVICE(0x0a5c, 0x2035), .driver_info = HCI_RESET |
> HCI_WRONG_SCO_MTU },
> + { USB_DEVICE(0x0a5c, 0x200a), .driver_info = HCI_RESET |
> HCI_WRONG_SCO_MTU },
> +
> + /* Broadcom BCM2045 */
> + { USB_DEVICE(0x0a5c, 0x2039), .driver_info = HCI_RESET |
> HCI_WRONG_SCO_MTU },
> + { USB_DEVICE(0x0a5c, 0x2101), .driver_info = HCI_RESET |
> HCI_WRONG_SCO_MTU },
> +
> + /* IBM/Lenovo ThinkPad with Broadcom chip */
> + { USB_DEVICE(0x0a5c, 0x201e), .driver_info = HCI_RESET |
> HCI_WRONG_SCO_MTU },
> + { USB_DEVICE(0x0a5c, 0x2110), .driver_info = HCI_RESET |
> HCI_WRONG_SCO_MTU },
> +
> + /* Targus ACB10US */
> + { USB_DEVICE(0x0a5c, 0x2100), .driver_info = HCI_RESET },
> +
> + /* ANYCOM Bluetooth USB-200 and USB-250 */
> + { USB_DEVICE(0x0a5c, 0x2111), .driver_info = HCI_RESET },
> +
> + /* HP laptop with Broadcom chip */
> + { USB_DEVICE(0x03f0, 0x171d), .driver_info = HCI_RESET |
> HCI_WRONG_SCO_MTU },
> +
> + /* Dell laptop with Broadcom chip */
> + { USB_DEVICE(0x413c, 0x8126), .driver_info = HCI_RESET |
> HCI_WRONG_SCO_MTU },
> +
> + /* Microsoft Wireless Transceiver for Bluetooth 2.0 */
> + { USB_DEVICE(0x045e, 0x009c), .driver_info = HCI_RESET },
> +
> + /* Kensington Bluetooth USB adapter */
> + { USB_DEVICE(0x047d, 0x105d), .driver_info = HCI_RESET },
> + { USB_DEVICE(0x047d, 0x105e), .driver_info = HCI_RESET |
> HCI_WRONG_SCO_MTU },
> +
> + /* ISSC Bluetooth Adapter v3.1 */
> + { USB_DEVICE(0x1131, 0x1001), .driver_info = HCI_RESET },
> +
> + /* RTX Telecom based adapters with buggy SCO support */
> + { USB_DEVICE(0x0400, 0x0807), .driver_info = HCI_BROKEN_ISOC },
> + { USB_DEVICE(0x0400, 0x080a), .driver_info = HCI_BROKEN_ISOC },
> +
> + /* CONWISE Technology based adapters with buggy SCO support */
> + { USB_DEVICE(0x0e5e, 0x6622), .driver_info = HCI_BROKEN_ISOC },
> +
> + /* Belkin F8T012 and F8T013 devices */
> + { USB_DEVICE(0x050d, 0x0012), .driver_info = HCI_RESET |
> HCI_WRONG_SCO_MTU },
> + { USB_DEVICE(0x050d, 0x0013), .driver_info = HCI_RESET |
> HCI_WRONG_SCO_MTU },
> +
> + /* Digianswer devices */
> + { USB_DEVICE(0x08fd, 0x0001), .driver_info = HCI_DIGIANSWER },
> + { USB_DEVICE(0x08fd, 0x0002), .driver_info = HCI_IGNORE },
> +
> + /* CSR BlueCore Bluetooth Sniffer */
> + { USB_DEVICE(0x0a12, 0x0002), .driver_info = HCI_SNIFFER },
> +
> + /* Frontline ComProbe Bluetooth Sniffer */
> + { USB_DEVICE(0x16d3, 0x0002), .driver_info = HCI_SNIFFER },
> { } /* Terminating entry */
> };
Both table updates should be a separate patch and we can even submit
that for 2.6.27 inclusion. I was just to lazy to do that, but I have
it on my todo list, but the Simple Pairing support had higher priority.
Also the HCI_RESET thing needs to be turn-around. We always do
HCI_RESET except for some chips where it will break (old CSR ones for
example).
> +static struct usb_driver btusb_driver;
> +
> +static int reset = 0;
> +module_param(reset, bool, 0644);
> +MODULE_PARM_DESC(reset, "Send HCI reset command on initialization");
> +
> +static int force_scofix;
> +module_param(force_scofix, bool, 0644);
> +MODULE_PARM_DESC(force_scofix, "Force fixup of wrong SCO buffers
> size");
> +
> +static int disable_scofix;
> +module_param(disable_scofix, bool, 0644);
> +MODULE_PARM_DESC(disable_scofix, "Disable fixup of wrong SCO buffer
> size");
> +
> +static int ignore_csr;
> +module_param(ignore_csr, bool, 0644);
> +MODULE_PARM_DESC(ignore_csr, "Ignore devices with id 0a12:0001");
> +
> +static int ignore_sniffer;
> +module_param(ignore_sniffer, bool, 0644);
> +MODULE_PARM_DESC(ignore_sniffer, "Ignore devices with id 0a12:0002");
> +
> +static int ignore_dga;
> +module_param(ignore_dga, bool, 0644);
> +MODULE_PARM_DESC(ignore_dga, "Ignore devices with id 08fd:0001");
> +
> +static int override_ignore;
> +module_param(override_ignore, bool, 0644);
> +MODULE_PARM_DESC(override_ignore, "Drive blacklisted devices");
This is my personal taste. I want the variable at the top, but the
module_param() and the description at the bottom with the other
defines for the module.
> - struct work_struct work;
> -
> struct usb_anchor tx_anchor;
> struct usb_anchor intr_anchor;
> struct usb_anchor bulk_anchor;
> + struct usb_anchor sco_anchor;
>
> struct usb_endpoint_descriptor *intr_ep;
> struct usb_endpoint_descriptor *bulk_tx_ep;
> struct usb_endpoint_descriptor *bulk_rx_ep;
> + struct usb_host_endpoint *isoc_out_ep;
> + struct usb_host_endpoint *isoc_in_ep;
> +
> + struct usb_interface *iso;
> };
Call the later one either *isoc or *sco. Actually *sco_intf might make
a lot sense since you have the sco_anchor.
> +static void btusb_sco_rx_complete(struct urb *urb)
> +{
> + struct btusb_data *data = urb->context;
> + int status = urb->status;
> + int i;
> +
> + if (!test_bit(HCI_RUNNING, &data->flags))
> + goto out;
> + if (status < 0)
> + goto out;
> +
> + for (i=0; i < urb->number_of_packets; i++) {
> + BT_DBG("desc %d status %d offset %d len %d", i,
> + urb->iso_frame_desc[i].status,
> + urb->iso_frame_desc[i].offset,
> + urb->iso_frame_desc[i].actual_length);
Small coding style nitpick in the for loop. And yes I know, that is
wrong in hci_usb.c driver, too :)
> if (!urb->iso_frame_desc[i].status) {
> + data->hdev->stat.byte_rx += urb->iso_frame_desc[i].actual_length;
> + hci_recv_fragment(data->hdev, HCI_SCODATA_PKT,
> + urb->transfer_buffer + urb->iso_frame_desc[i].offset,
> + urb->iso_frame_desc[i].actual_length);
> + }
> + }
> +
> + usb_anchor_urb(urb, &data->sco_anchor);
> + i = usb_submit_urb(urb, GFP_ATOMIC);
> + if (i < 0) {
> + usb_unanchor_urb(urb);
> +out:
> + kfree(urb->transfer_buffer);
> + }
> +}
This label is ugly. Please just simple to the kfree in the error case
directly in the test at the top of the function. Also please not re-
use the i from the loop iterator and lets just check directly here
without storing the result. Since we don't do anything usefull with it
anyway, it gives us no benefit and would make the code a little bit
easier to read.
> static int btusb_open(struct hci_dev *hdev)
> {
> struct btusb_data *data = hdev->driver_data;
> + static DEFINE_MUTEX(open_mut);
I dislike static mutex definition inside functions. Just move this to
a global mutex.
> +static void btusb_fill_isoc_desc(struct urb *urb, int len, int mtu)
> +{
> + int offset = 0, i;
> +
> + BT_DBG("len %d mtu %d", len, mtu);
> +
> + for (i=0; i < HCI_MAX_ISOC_FRAMES && len >= mtu; i++, offset +=
> mtu, len -= mtu) {
> + urb->iso_frame_desc[i].offset = offset;
> + urb->iso_frame_desc[i].length = mtu;
> + BT_DBG("desc %d offset %d len %d", i, offset, mtu);
> + }
Same coding style comment as above. We also might wanna break this
complex for-loop up in easier code.
> + if (id->driver_info == HCI_IGNORE && !override_ignore)
> + return -ENODEV;
> + if (ignore_sniffer && id->driver_info & HCI_SNIFFER)
> + return -ENODEV;
> + if (ignore_csr && id->driver_info & HCI_CSR)
> + return -ENODEV;
> + if (ignore_dga && id->driver_info & HCI_DIGIANSWER)
> + return -ENODEV;
This as to be a separate patch with the blacklist stuff.
Regards
Marcel
On Wed, 20 Aug 2008, Dave Higton wrote:
> > Since these are isoc URBs anyway, shouldn't you simply drop them until
> > the new altsetting is ready? That's a lot easier than trying to keep
> > track of them and deferring them (which doesn't make much sense for
> > Isochronous data).
>
> If they were scheduled before an altsetting change, they must be plain
> wrong for the new altsetting - and indeed the new altsetting could be
> 0, in which case they wouldn't be sent anyway. So it seems to me that
> they should be dropped.
That's not a good description of the situation. The URBs were
scheduled _after_ an altsetting change was requested but _before_ the
change took place. Therefore the URBs are designed for the new
altsetting and so are wrong for the altsetting in effect at the time of
submission. (Indeed, the old altsetting might be 0, in which case
submission would fail anyway.) So they should be dropped.
> The only difference it makes, from the POV of the outside world, is an
> ever so slight change in the time the stream is switched off.
No. Since the submission or transmission of those URBs would have
failed anyway (if they were submitted before the altsetting change
occurred), they wouldn't get delivered in either case. The length of
time the stream is off is the time required to switch altsettings --
and that simply is unavoidable.
The other possibility suggested was to delay submitting the URBs until
after the new altsetting was in place. That is a bad idea because it
means delaying the delivery of isoc data, which makes no sense -- the
whole idea of an isochronous stream is that the information should be
delivered as quickly as possible and it doesn't much matter if some of
it gets lost.
Alan Stern
Alan Stern wrote:
> On Tue, 19 Aug 2008, Marcel Holtmann wrote:
>=20
> > Our problem is only that we are using a workqueue and can't make any
> > assumption when we get scheduled. This is obviously not=20
> perfect, but it
> > seems there is nothing much that can be done.
> >=20
> > I think the specification is simply bad and we have to live with it.
>=20
> I guess so.
>=20
> Since these are isoc URBs anyway, shouldn't you simply drop them until
> the new altsetting is ready? That's a lot easier than trying to keep
> track of them and deferring them (which doesn't make much sense for
> Isochronous data).
If they were scheduled before an altsetting change, they must be plain
wrong for the new altsetting - and indeed the new altsetting could be
0, in which case they wouldn't be sent anyway. So it seems to me that
they should be dropped.
The only difference it makes, from the POV of the outside world, is an
ever so slight change in the time the stream is switched off.
Dave
***************************************************************************=
***************************************************************************=
***************************************************************************=
****************
NICE CTI Systems UK Limited ("NICE") is registered in England under company=
number, 3403044. The registered office of NICE is at Tollbar Way, Hedge E=
nd, Southampton, Hampshire SO30 2ZP.
Confidentiality: This communication and any attachments are intended for th=
e above-named persons only and may be confidential and/or legally privilege=
d. Any opinions expressed in this communication are not necessarily those o=
f NICE. If this communication has come to you in error you must take no act=
ion based on it, nor must you copy or show it to anyone; please delete/dest=
roy and inform the sender by e-mail immediately.
Monitoring: NICE may monitor incoming and outgoing e-mails.
Viruses: Although we have taken steps toward ensuring that this e-mail and=
attachments are free from any virus, we advise that in keeping with good c=
omputing practice the recipient should ensure they are actually virus free.
***************************************************************************=
***************************************************************************=
***************************************************************************=
*******************
=20
On Tue, 19 Aug 2008, Marcel Holtmann wrote:
> Our problem is only that we are using a workqueue and can't make any
> assumption when we get scheduled. This is obviously not perfect, but it
> seems there is nothing much that can be done.
>
> I think the specification is simply bad and we have to live with it.
I guess so.
Since these are isoc URBs anyway, shouldn't you simply drop them until
the new altsetting is ready? That's a lot easier than trying to keep
track of them and deferring them (which doesn't make much sense for
Isochronous data).
Alan Stern
Hi Alan,
> > > And presumably, if the number of SCO connections increases from one to
> > > two then you need to switch to yet another altsetting -- while keeping
> > > the existing connection intact, yes?
> >
> > The connection will be kept alive. That is not the problem. We have to
> > cancel all URBs, select the new setting and then re-submit them.
>
> Along with any URBs that were generated while the new altsetting was
> being installed, right?
>
> If you're already keeping track of old URBs that were cancelled, why is
> it hard to keep track of new URBs as well?
>
> > As mentioned above, there is nothing much we can do. Once we get the
> > connection established event, we have to submit URBs. The event comes in
> > via an interrupt URB. In theory we could defer the processing of the HCI
> > events, but that would have bad impact on all other parts of Bluetooth
> > and doing this only for audio makes no sense.
>
> How about deferring only the submission of isoc URBs while doing all
> the others immediately?
the bulk, control and interrupt URBs are on a different interface and so
they are not affected.
> > > Is it possible to change the subsystem design so that, for example, in
> > > addition to getting a notify() callback when the connection settings
> > > change, you also call a ready() function in the subsystem core to tell
> > > it when the new settings are ready for use?
> >
> > I was thinking about that. However it is still the same problem. We do
> > have to submit URBs as soon as the connection is up. For the bulk URBs
> > (for ACL) it is no problem. The only issue is with isoc URB (for SCO),
> > because we have to pick an alternate setting first.
>
> Well, you _can't_ submit isoc URBs before changing the altsetting; it
> just won't work. So you can't start submitting them as soon as the
> connection is up -- the hardware doesn't allow it. One way or another
> they have to be deferred. The only question is how the deferrals
> should be implemented.
Our problem is only that we are using a workqueue and can't make any
assumption when we get scheduled. This is obviously not perfect, but it
seems there is nothing much that can be done.
I think the specification is simply bad and we have to live with it.
> > > If not, and you are forced to rely on queuing URBs for later
> > > submission, then I think it might be more appropriate to do this
> > > queuing in the Bluetooth driver code rather than in usbcore. You could
> > > have an entire anchor devoted to deferred URBs.
> >
> > What happens if we submit the isoc URBs right away and the call
> > usb_set_interface at some later time. Will these be canceled or what
> > happens to them when switching the endpoint.
>
> When you call usb_set_interface(), all pending URBs for that interface
> will be cancelled and will complete with a status of -ESHUTDOWN.
>
> (Hmm, looking at the code I see that the altsetting gets changed
> _before_ the old URBs are cancelled. That probably is a bug...)
Currently we cancel the URBs before calling usb_set_inferface.
Regards
Marcel
On Tue, 19 Aug 2008, Marcel Holtmann wrote:
> > And presumably, if the number of SCO connections increases from one to
> > two then you need to switch to yet another altsetting -- while keeping
> > the existing connection intact, yes?
>
> The connection will be kept alive. That is not the problem. We have to
> cancel all URBs, select the new setting and then re-submit them.
Along with any URBs that were generated while the new altsetting was
being installed, right?
If you're already keeping track of old URBs that were cancelled, why is
it hard to keep track of new URBs as well?
> As mentioned above, there is nothing much we can do. Once we get the
> connection established event, we have to submit URBs. The event comes in
> via an interrupt URB. In theory we could defer the processing of the HCI
> events, but that would have bad impact on all other parts of Bluetooth
> and doing this only for audio makes no sense.
How about deferring only the submission of isoc URBs while doing all
the others immediately?
> > Is it possible to change the subsystem design so that, for example, in
> > addition to getting a notify() callback when the connection settings
> > change, you also call a ready() function in the subsystem core to tell
> > it when the new settings are ready for use?
>
> I was thinking about that. However it is still the same problem. We do
> have to submit URBs as soon as the connection is up. For the bulk URBs
> (for ACL) it is no problem. The only issue is with isoc URB (for SCO),
> because we have to pick an alternate setting first.
Well, you _can't_ submit isoc URBs before changing the altsetting; it
just won't work. So you can't start submitting them as soon as the
connection is up -- the hardware doesn't allow it. One way or another
they have to be deferred. The only question is how the deferrals
should be implemented.
> > If not, and you are forced to rely on queuing URBs for later
> > submission, then I think it might be more appropriate to do this
> > queuing in the Bluetooth driver code rather than in usbcore. You could
> > have an entire anchor devoted to deferred URBs.
>
> What happens if we submit the isoc URBs right away and the call
> usb_set_interface at some later time. Will these be canceled or what
> happens to them when switching the endpoint.
When you call usb_set_interface(), all pending URBs for that interface
will be cancelled and will complete with a status of -ESHUTDOWN.
(Hmm, looking at the code I see that the altsetting gets changed
_before_ the old URBs are cancelled. That probably is a bug...)
Alan Stern
Hi Alan,
> > > > example have the USB core queue submitted URBs until the new alternate
> > > > setting has been selected?
> > >
> > > this one's for you. Is there a deeper reason we enable and disable endpoints
> > > therein?
>
> There is. Endpoint properties can and do differ between altsettings.
> In order to work properly, the host controller hardware needs to be
> aware of changes in the endpoint properties. The only way to
> accomplish this is to disable the endpoints for the altsetting going
> away (thereby flushing their properties from the controller hardware)
> and then to enable the endpoints for the altsetting coming into
> existence.
>
> > just let me give you details on rational behind my question. A Bluetooth
> > USB device uses two interfaces. The first one for control (commands),
> > interrupt (event) and bulk (ACL data) endpoints. The second interface
> > contains the isoc (SCO data) endpoints.
> >
> > T: Bus=02 Lev=01 Prnt=01 Port=00 Cnt=01 Dev#= 2 Spd=12 MxCh= 0
> > D: Ver= 2.00 Cls=e0(unk. ) Sub=01 Prot=01 MxPS=64 #Cfgs= 1
> > P: Vendor=0bf8 ProdID=1003 Rev=53.42
> > C:* #Ifs= 3 Cfg#= 1 Atr=c0 MxPwr= 0mA
> > I:* If#= 0 Alt= 0 #EPs= 3 Cls=e0(unk. ) Sub=01 Prot=01 Driver=btusb
> > E: Ad=81(I) Atr=03(Int.) MxPS= 16 Ivl=1ms
> > E: Ad=02(O) Atr=02(Bulk) MxPS= 64 Ivl=0ms
> > E: Ad=82(I) Atr=02(Bulk) MxPS= 64 Ivl=0ms
> > I:* If#= 1 Alt= 0 #EPs= 2 Cls=e0(unk. ) Sub=01 Prot=01 Driver=btusb
> > E: Ad=03(O) Atr=01(Isoc) MxPS= 0 Ivl=1ms
> > E: Ad=83(I) Atr=01(Isoc) MxPS= 0 Ivl=1ms
> > I: If#= 1 Alt= 1 #EPs= 2 Cls=e0(unk. ) Sub=01 Prot=01 Driver=btusb
> > E: Ad=03(O) Atr=01(Isoc) MxPS= 9 Ivl=1ms
> > E: Ad=83(I) Atr=01(Isoc) MxPS= 9 Ivl=1ms
> > I: If#= 1 Alt= 2 #EPs= 2 Cls=e0(unk. ) Sub=01 Prot=01 Driver=btusb
> > E: Ad=03(O) Atr=01(Isoc) MxPS= 17 Ivl=1ms
> > E: Ad=83(I) Atr=01(Isoc) MxPS= 17 Ivl=1ms
> > I: If#= 1 Alt= 3 #EPs= 2 Cls=e0(unk. ) Sub=01 Prot=01 Driver=btusb
> > E: Ad=03(O) Atr=01(Isoc) MxPS= 25 Ivl=1ms
> > E: Ad=83(I) Atr=01(Isoc) MxPS= 25 Ivl=1ms
> > I: If#= 1 Alt= 4 #EPs= 2 Cls=e0(unk. ) Sub=01 Prot=01 Driver=btusb
> > E: Ad=03(O) Atr=01(Isoc) MxPS= 33 Ivl=1ms
> > E: Ad=83(I) Atr=01(Isoc) MxPS= 33 Ivl=1ms
> > I: If#= 1 Alt= 5 #EPs= 2 Cls=e0(unk. ) Sub=01 Prot=01 Driver=btusb
> > E: Ad=03(O) Atr=01(Isoc) MxPS= 49 Ivl=1ms
> > E: Ad=83(I) Atr=01(Isoc) MxPS= 49 Ivl=1ms
> > I:* If#= 2 Alt= 0 #EPs= 0 Cls=fe(app. ) Sub=01 Prot=00 Driver=(none)
>
> What is that last interface for? It doesn't appear to be very useful,
> with no endpoints (and no driver)...
that is DFU. Don't worry about that one.
> > The second interface contains 6 alternate settings and you have to
> > select them based on your voice setting (8-bit or 16-bit) and the number
> > of SCO connections (0-3).
> >
> > The Bluetooth specification has the details, but when using 16-bit input
> > data and one SCO connection, you have to select alternate setting #2. If
> > no SCO connections are open, we have to go with setting #0.
>
> And presumably, if the number of SCO connections increases from one to
> two then you need to switch to yet another altsetting -- while keeping
> the existing connection intact, yes?
The connection will be kept alive. That is not the problem. We have to
cancel all URBs, select the new setting and then re-submit them.
> > So we can't have isoc URBs running all them time since that would drain
> > power. The old hci_usb did it this way and was an obvious issue showing
> > up on powertop.
> >
> > The Bluetooth subsystem calls our notify() callback when the number of
> > connection or the voice setting changes. However this context can't
> > sleep and thus we can't call usb_set_interface().
>
> That's the real problem. Why doesn't opening or closing a connection
> take place in a sleepable process context?
Because we get the HCI event for an established connection in an
interrupt context and then do the notifications based on that.
> > Right now are using a
> > workqueue, but that is not an optimal solution since as Oliver pointed
> > correctly out, we have no idea when this gets scheduled.
> >
> > My question is if we can have a usb_set_interface() version that we can
> > call from interrupt context and that will queue the URBs that we submit
> > in between until the new alternate setting has been selected.
> >
> > I personally think the Bluetooth USB specification is broken and a bad
> > design, but that is what we have at the moment.
>
> I don't know how much of this comes from the Bluetooth USB spec and how
> much comes from the design of the Bluetooth subsystem. It sounds like
> you're being asked to do too much in too short a time.
As mentioned above, there is nothing much we can do. Once we get the
connection established event, we have to submit URBs. The event comes in
via an interrupt URB. In theory we could defer the processing of the HCI
events, but that would have bad impact on all other parts of Bluetooth
and doing this only for audio makes no sense.
> Is it possible to change the subsystem design so that, for example, in
> addition to getting a notify() callback when the connection settings
> change, you also call a ready() function in the subsystem core to tell
> it when the new settings are ready for use?
I was thinking about that. However it is still the same problem. We do
have to submit URBs as soon as the connection is up. For the bulk URBs
(for ACL) it is no problem. The only issue is with isoc URB (for SCO),
because we have to pick an alternate setting first.
> If not, and you are forced to rely on queuing URBs for later
> submission, then I think it might be more appropriate to do this
> queuing in the Bluetooth driver code rather than in usbcore. You could
> have an entire anchor devoted to deferred URBs.
What happens if we submit the isoc URBs right away and the call
usb_set_interface at some later time. Will these be canceled or what
happens to them when switching the endpoint.
Regards
Marcel
On Tue, 19 Aug 2008, Marcel Holtmann wrote:
> Hi Alan,
>
> > > example have the USB core queue submitted URBs until the new alternate
> > > setting has been selected?
> >
> > this one's for you. Is there a deeper reason we enable and disable endpoints
> > therein?
There is. Endpoint properties can and do differ between altsettings.
In order to work properly, the host controller hardware needs to be
aware of changes in the endpoint properties. The only way to
accomplish this is to disable the endpoints for the altsetting going
away (thereby flushing their properties from the controller hardware)
and then to enable the endpoints for the altsetting coming into
existence.
> just let me give you details on rational behind my question. A Bluetooth
> USB device uses two interfaces. The first one for control (commands),
> interrupt (event) and bulk (ACL data) endpoints. The second interface
> contains the isoc (SCO data) endpoints.
>
> T: Bus=02 Lev=01 Prnt=01 Port=00 Cnt=01 Dev#= 2 Spd=12 MxCh= 0
> D: Ver= 2.00 Cls=e0(unk. ) Sub=01 Prot=01 MxPS=64 #Cfgs= 1
> P: Vendor=0bf8 ProdID=1003 Rev=53.42
> C:* #Ifs= 3 Cfg#= 1 Atr=c0 MxPwr= 0mA
> I:* If#= 0 Alt= 0 #EPs= 3 Cls=e0(unk. ) Sub=01 Prot=01 Driver=btusb
> E: Ad=81(I) Atr=03(Int.) MxPS= 16 Ivl=1ms
> E: Ad=02(O) Atr=02(Bulk) MxPS= 64 Ivl=0ms
> E: Ad=82(I) Atr=02(Bulk) MxPS= 64 Ivl=0ms
> I:* If#= 1 Alt= 0 #EPs= 2 Cls=e0(unk. ) Sub=01 Prot=01 Driver=btusb
> E: Ad=03(O) Atr=01(Isoc) MxPS= 0 Ivl=1ms
> E: Ad=83(I) Atr=01(Isoc) MxPS= 0 Ivl=1ms
> I: If#= 1 Alt= 1 #EPs= 2 Cls=e0(unk. ) Sub=01 Prot=01 Driver=btusb
> E: Ad=03(O) Atr=01(Isoc) MxPS= 9 Ivl=1ms
> E: Ad=83(I) Atr=01(Isoc) MxPS= 9 Ivl=1ms
> I: If#= 1 Alt= 2 #EPs= 2 Cls=e0(unk. ) Sub=01 Prot=01 Driver=btusb
> E: Ad=03(O) Atr=01(Isoc) MxPS= 17 Ivl=1ms
> E: Ad=83(I) Atr=01(Isoc) MxPS= 17 Ivl=1ms
> I: If#= 1 Alt= 3 #EPs= 2 Cls=e0(unk. ) Sub=01 Prot=01 Driver=btusb
> E: Ad=03(O) Atr=01(Isoc) MxPS= 25 Ivl=1ms
> E: Ad=83(I) Atr=01(Isoc) MxPS= 25 Ivl=1ms
> I: If#= 1 Alt= 4 #EPs= 2 Cls=e0(unk. ) Sub=01 Prot=01 Driver=btusb
> E: Ad=03(O) Atr=01(Isoc) MxPS= 33 Ivl=1ms
> E: Ad=83(I) Atr=01(Isoc) MxPS= 33 Ivl=1ms
> I: If#= 1 Alt= 5 #EPs= 2 Cls=e0(unk. ) Sub=01 Prot=01 Driver=btusb
> E: Ad=03(O) Atr=01(Isoc) MxPS= 49 Ivl=1ms
> E: Ad=83(I) Atr=01(Isoc) MxPS= 49 Ivl=1ms
> I:* If#= 2 Alt= 0 #EPs= 0 Cls=fe(app. ) Sub=01 Prot=00 Driver=(none)
What is that last interface for? It doesn't appear to be very useful,
with no endpoints (and no driver)...
> The second interface contains 6 alternate settings and you have to
> select them based on your voice setting (8-bit or 16-bit) and the number
> of SCO connections (0-3).
>
> The Bluetooth specification has the details, but when using 16-bit input
> data and one SCO connection, you have to select alternate setting #2. If
> no SCO connections are open, we have to go with setting #0.
And presumably, if the number of SCO connections increases from one to
two then you need to switch to yet another altsetting -- while keeping
the existing connection intact, yes?
> So we can't have isoc URBs running all them time since that would drain
> power. The old hci_usb did it this way and was an obvious issue showing
> up on powertop.
>
> The Bluetooth subsystem calls our notify() callback when the number of
> connection or the voice setting changes. However this context can't
> sleep and thus we can't call usb_set_interface().
That's the real problem. Why doesn't opening or closing a connection
take place in a sleepable process context?
> Right now are using a
> workqueue, but that is not an optimal solution since as Oliver pointed
> correctly out, we have no idea when this gets scheduled.
>
> My question is if we can have a usb_set_interface() version that we can
> call from interrupt context and that will queue the URBs that we submit
> in between until the new alternate setting has been selected.
>
> I personally think the Bluetooth USB specification is broken and a bad
> design, but that is what we have at the moment.
I don't know how much of this comes from the Bluetooth USB spec and how
much comes from the design of the Bluetooth subsystem. It sounds like
you're being asked to do too much in too short a time.
Is it possible to change the subsystem design so that, for example, in
addition to getting a notify() callback when the connection settings
change, you also call a ready() function in the subsystem core to tell
it when the new settings are ready for use?
If not, and you are forced to rely on queuing URBs for later
submission, then I think it might be more appropriate to do this
queuing in the Bluetooth driver code rather than in usbcore. You could
have an entire anchor devoted to deferred URBs.
Alan Stern
Hi Alan,
> > example have the USB core queue submitted URBs until the new alternate
> > setting has been selected?
>
> this one's for you. Is there a deeper reason we enable and disable endpoints
> therein?
just let me give you details on rational behind my question. A Bluetooth
USB device uses two interfaces. The first one for control (commands),
interrupt (event) and bulk (ACL data) endpoints. The second interface
contains the isoc (SCO data) endpoints.
T: Bus=02 Lev=01 Prnt=01 Port=00 Cnt=01 Dev#= 2 Spd=12 MxCh= 0
D: Ver= 2.00 Cls=e0(unk. ) Sub=01 Prot=01 MxPS=64 #Cfgs= 1
P: Vendor=0bf8 ProdID=1003 Rev=53.42
C:* #Ifs= 3 Cfg#= 1 Atr=c0 MxPwr= 0mA
I:* If#= 0 Alt= 0 #EPs= 3 Cls=e0(unk. ) Sub=01 Prot=01 Driver=btusb
E: Ad=81(I) Atr=03(Int.) MxPS= 16 Ivl=1ms
E: Ad=02(O) Atr=02(Bulk) MxPS= 64 Ivl=0ms
E: Ad=82(I) Atr=02(Bulk) MxPS= 64 Ivl=0ms
I:* If#= 1 Alt= 0 #EPs= 2 Cls=e0(unk. ) Sub=01 Prot=01 Driver=btusb
E: Ad=03(O) Atr=01(Isoc) MxPS= 0 Ivl=1ms
E: Ad=83(I) Atr=01(Isoc) MxPS= 0 Ivl=1ms
I: If#= 1 Alt= 1 #EPs= 2 Cls=e0(unk. ) Sub=01 Prot=01 Driver=btusb
E: Ad=03(O) Atr=01(Isoc) MxPS= 9 Ivl=1ms
E: Ad=83(I) Atr=01(Isoc) MxPS= 9 Ivl=1ms
I: If#= 1 Alt= 2 #EPs= 2 Cls=e0(unk. ) Sub=01 Prot=01 Driver=btusb
E: Ad=03(O) Atr=01(Isoc) MxPS= 17 Ivl=1ms
E: Ad=83(I) Atr=01(Isoc) MxPS= 17 Ivl=1ms
I: If#= 1 Alt= 3 #EPs= 2 Cls=e0(unk. ) Sub=01 Prot=01 Driver=btusb
E: Ad=03(O) Atr=01(Isoc) MxPS= 25 Ivl=1ms
E: Ad=83(I) Atr=01(Isoc) MxPS= 25 Ivl=1ms
I: If#= 1 Alt= 4 #EPs= 2 Cls=e0(unk. ) Sub=01 Prot=01 Driver=btusb
E: Ad=03(O) Atr=01(Isoc) MxPS= 33 Ivl=1ms
E: Ad=83(I) Atr=01(Isoc) MxPS= 33 Ivl=1ms
I: If#= 1 Alt= 5 #EPs= 2 Cls=e0(unk. ) Sub=01 Prot=01 Driver=btusb
E: Ad=03(O) Atr=01(Isoc) MxPS= 49 Ivl=1ms
E: Ad=83(I) Atr=01(Isoc) MxPS= 49 Ivl=1ms
I:* If#= 2 Alt= 0 #EPs= 0 Cls=fe(app. ) Sub=01 Prot=00 Driver=(none)
The second interface contains 6 alternate settings and you have to
select them based on your voice setting (8-bit or 16-bit) and the number
of SCO connections (0-3).
The Bluetooth specification has the details, but when using 16-bit input
data and one SCO connection, you have to select alternate setting #2. If
no SCO connections are open, we have to go with setting #0.
T: Bus=02 Lev=01 Prnt=01 Port=00 Cnt=01 Dev#= 2 Spd=12 MxCh= 0
D: Ver= 2.00 Cls=e0(unk. ) Sub=01 Prot=01 MxPS=64 #Cfgs= 1
P: Vendor=0bf8 ProdID=1003 Rev=53.42
C:* #Ifs= 3 Cfg#= 1 Atr=c0 MxPwr= 0mA
I:* If#= 0 Alt= 0 #EPs= 3 Cls=e0(unk. ) Sub=01 Prot=01 Driver=btusb
E: Ad=81(I) Atr=03(Int.) MxPS= 16 Ivl=1ms
E: Ad=02(O) Atr=02(Bulk) MxPS= 64 Ivl=0ms
E: Ad=82(I) Atr=02(Bulk) MxPS= 64 Ivl=0ms
I: If#= 1 Alt= 0 #EPs= 2 Cls=e0(unk. ) Sub=01 Prot=01 Driver=btusb
E: Ad=03(O) Atr=01(Isoc) MxPS= 0 Ivl=1ms
E: Ad=83(I) Atr=01(Isoc) MxPS= 0 Ivl=1ms
I: If#= 1 Alt= 1 #EPs= 2 Cls=e0(unk. ) Sub=01 Prot=01 Driver=btusb
E: Ad=03(O) Atr=01(Isoc) MxPS= 9 Ivl=1ms
E: Ad=83(I) Atr=01(Isoc) MxPS= 9 Ivl=1ms
I:* If#= 1 Alt= 2 #EPs= 2 Cls=e0(unk. ) Sub=01 Prot=01 Driver=btusb
E: Ad=03(O) Atr=01(Isoc) MxPS= 17 Ivl=1ms
E: Ad=83(I) Atr=01(Isoc) MxPS= 17 Ivl=1ms
I: If#= 1 Alt= 3 #EPs= 2 Cls=e0(unk. ) Sub=01 Prot=01 Driver=btusb
E: Ad=03(O) Atr=01(Isoc) MxPS= 25 Ivl=1ms
E: Ad=83(I) Atr=01(Isoc) MxPS= 25 Ivl=1ms
I: If#= 1 Alt= 4 #EPs= 2 Cls=e0(unk. ) Sub=01 Prot=01 Driver=btusb
E: Ad=03(O) Atr=01(Isoc) MxPS= 33 Ivl=1ms
E: Ad=83(I) Atr=01(Isoc) MxPS= 33 Ivl=1ms
I: If#= 1 Alt= 5 #EPs= 2 Cls=e0(unk. ) Sub=01 Prot=01 Driver=btusb
E: Ad=03(O) Atr=01(Isoc) MxPS= 49 Ivl=1ms
E: Ad=83(I) Atr=01(Isoc) MxPS= 49 Ivl=1ms
I:* If#= 2 Alt= 0 #EPs= 0 Cls=fe(app. ) Sub=01 Prot=00 Driver=(none)
So we can't have isoc URBs running all them time since that would drain
power. The old hci_usb did it this way and was an obvious issue showing
up on powertop.
The Bluetooth subsystem calls our notify() callback when the number of
connection or the voice setting changes. However this context can't
sleep and thus we can't call usb_set_interface(). Right now are using a
workqueue, but that is not an optimal solution since as Oliver pointed
correctly out, we have no idea when this gets scheduled.
My question is if we can have a usb_set_interface() version that we can
call from interrupt context and that will queue the URBs that we submit
in between until the new alternate setting has been selected.
I personally think the Bluetooth USB specification is broken and a bad
design, but that is what we have at the moment.
Regards
Marcel
Am Dienstag 19 August 2008 09:19:44 schrieb Marcel Holtmann:
> So do we have any chance to make usb_set_interface asynchron? For
> example have the USB core queue submitted URBs until the new alternate
> setting has been selected?
Alan,
this one's for you. Is there a deeper reason we enable and disable endpoints
therein?
Regards
Oliver
Hi Oliver,
> > However when using the unlink version, this gives me spinlock lockup
> > (with and without your IRQ disable patch). From the backtrace it seems
> > that usb_unanchor_urb is calling spinlock_irq_save here.
>
> This patch should fix it.
yes, that patch fixes it for me.
Acked-by: Marcel Holtmann <[email protected]>
Together with your IRQ disable patch, this should go into 2.6.27-rc4 and
it should also be submitted for 2.6.26-stable.
So do we have any chance to make usb_set_interface asynchron? For
example have the USB core queue submitted URBs until the new alternate
setting has been selected?
Regards
Marcel
Am Dienstag 19 August 2008 06:05:54 schrieb Marcel Holtmann:
> However when using the unlink version, this gives me spinlock lockup
> (with and without your IRQ disable patch). From the backtrace it seems
> that usb_unanchor_urb is calling spinlock_irq_save here.
This patch should fix it.
Regards
Oliver
----
--- linux-2.6.27-rc3/drivers/usb/core/urb.c.alt2 2008-08-18 16:24:41.000000000 +0200
+++ linux-2.6.27-rc3/drivers/usb/core/urb.c 2008-08-19 08:12:27.000000000 +0200
@@ -607,8 +607,12 @@ void usb_unlink_anchored_urbs(struct usb
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_irqrestore(&anchor->lock, flags);
}
Hi Oliver,
> > > > actually we can't move the URB killing for ACL into notify() since that
> > > > gives me a kernel panic (fatal exception in interrupt).
> > > >
> > > > Your patch for the IRQ disabling doesn't make a difference.
> > >
> > > Please send me the version that triggers it.
> >
> > the attached version makes it oops. Just create a connection with rfcomm
> > or sdptool and you will see it when disconnecting.
>
> This is a context you cannot sleep in:
>
> @@ -504,8 +674,62 @@ static void btusb_notify(struct hci_dev *hdev, unsigned int evt)
>
> 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) < 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);
>
> But you use usb_kill_anchored_urbs(). You should use usb_unlink_anchored_urbs()
good catch. That was starring me at the code and not realizing that all
usb_kill_* version need to sleep.
However when using the unlink version, this gives me spinlock lockup
(with and without your IRQ disable patch). From the backtrace it seems
that usb_unanchor_urb is calling spinlock_irq_save here.
Regards
Marcel
Am Montag 18 August 2008 19:07:53 schrieb Marcel Holtmann:
> Hi Oliver,
>
> > > actually we can't move the URB killing for ACL into notify() since that
> > > gives me a kernel panic (fatal exception in interrupt).
> > >
> > > Your patch for the IRQ disabling doesn't make a difference.
> >
> > Please send me the version that triggers it.
>
> the attached version makes it oops. Just create a connection with rfcomm
> or sdptool and you will see it when disconnecting.
This is a context you cannot sleep in:
@@ -504,8 +674,62 @@ static void btusb_notify(struct hci_dev *hdev, unsigned int evt)
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) < 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);
But you use usb_kill_anchored_urbs(). You should use usb_unlink_anchored_urbs()
Regards
Oliver
Hi Oliver,
> > actually we can't move the URB killing for ACL into notify() since that
> > gives me a kernel panic (fatal exception in interrupt).
> >
> > Your patch for the IRQ disabling doesn't make a difference.
>
> Please send me the version that triggers it.
the attached version makes it oops. Just create a connection with rfcomm
or sdptool and you will see it when disconnecting.
Regards
Marcel
Am Montag 18 August 2008 17:12:40 schrieb Marcel Holtmann:
> actually we can't move the URB killing for ACL into notify() since that
> gives me a kernel panic (fatal exception in interrupt).
>
> Your patch for the IRQ disabling doesn't make a difference.
Please send me the version that triggers it.
Regards
Oliver
Hi Oliver,
> > > > > > On the other hand, this is audio and I don't really care if we loose a
> > > > > > packet or not.
> > > > >
> > > > > It isn't limited to sound. The URBs for acl reception can also be delayed
> > > > > arbitrarily long.
> > > >
> > > > We can move that into the notify() callback, but the killing the URBs
> > > > becomes a problem.
> > >
> > > /**
> > > * usb_unlink_anchored_urbs - asynchronously cancel transfer requests en masse
> > > * @anchor: anchor the requests are bound to
> > > *
> > > * this allows all outstanding URBs to be unlinked starting
> > > * from the back of the queue. This function is asynchronous.
> > > * The unlinking is just tiggered. It may happen after this
> > > * function has returned.
> > > */
> > > void usb_unlink_anchored_urbs(struct usb_anchor *anchor)
> >
> > then we can move the ACL handling into the notify() callback.
> >
> > For the SCO ones, I don't see any chance since we have to do the
> > altsetting first.
>
> You are right.
actually we can't move the URB killing for ACL into notify() since that
gives me a kernel panic (fatal exception in interrupt).
Your patch for the IRQ disabling doesn't make a difference.
Regards
Marcel
Am Montag 18 August 2008 16:36:22 schrieb Marcel Holtmann:
> Hi Oliver,
>
> > > > > On the other hand, this is audio and I don't really care if we loose a
> > > > > packet or not.
> > > >
> > > > It isn't limited to sound. The URBs for acl reception can also be delayed
> > > > arbitrarily long.
> > >
> > > We can move that into the notify() callback, but the killing the URBs
> > > becomes a problem.
> >
> > /**
> > * usb_unlink_anchored_urbs - asynchronously cancel transfer requests en masse
> > * @anchor: anchor the requests are bound to
> > *
> > * this allows all outstanding URBs to be unlinked starting
> > * from the back of the queue. This function is asynchronous.
> > * The unlinking is just tiggered. It may happen after this
> > * function has returned.
> > */
> > void usb_unlink_anchored_urbs(struct usb_anchor *anchor)
>
> then we can move the ACL handling into the notify() callback.
>
> For the SCO ones, I don't see any chance since we have to do the
> altsetting first.
You are right.
Regards
Oliver
Hi Oliver,
> > > > On the other hand, this is audio and I don't really care if we loose a
> > > > packet or not.
> > >
> > > It isn't limited to sound. The URBs for acl reception can also be delayed
> > > arbitrarily long.
> >
> > We can move that into the notify() callback, but the killing the URBs
> > becomes a problem.
>
> /**
> * usb_unlink_anchored_urbs - asynchronously cancel transfer requests en masse
> * @anchor: anchor the requests are bound to
> *
> * this allows all outstanding URBs to be unlinked starting
> * from the back of the queue. This function is asynchronous.
> * The unlinking is just tiggered. It may happen after this
> * function has returned.
> */
> void usb_unlink_anchored_urbs(struct usb_anchor *anchor)
then we can move the ACL handling into the notify() callback.
For the SCO ones, I don't see any chance since we have to do the
altsetting first.
Regards
Marcel
Am Montag 18 August 2008 16:10:20 schrieb Marcel Holtmann:
> run powertop and you will see why we can't do that. Also there is no max
> altsetting. It doesn't work like this. You have to pick the right one.
>
> The Bluetooth USB spec. is messed up when it comes to SCO.
Very well, it can't be helped then.
> > > On the other hand, this is audio and I don't really care if we loose a
> > > packet or not.
> >
> > It isn't limited to sound. The URBs for acl reception can also be delayed
> > arbitrarily long.
>
> We can move that into the notify() callback, but the killing the URBs
> becomes a problem.
/**
* usb_unlink_anchored_urbs - asynchronously cancel transfer requests en masse
* @anchor: anchor the requests are bound to
*
* this allows all outstanding URBs to be unlinked starting
* from the back of the queue. This function is asynchronous.
* The unlinking is just tiggered. It may happen after this
* function has returned.
*/
void usb_unlink_anchored_urbs(struct usb_anchor *anchor)
> On the other hand, ACL shouldn't be any problem since there is a HCI
> connection setup in between and the ACL part of Bluetooth is reliable
> and we have flow control on it.
>
> Also these are bulk URBs. I am under the assumption that the Bluetooth
> controller will queue packets up until we submit the first URB.
>
> > > > Secondly, what happens when this next event comes so quickly that
> > > > the work is still scheduled or running? it seems to me that the work handler
> > > > can read stale conn_hash values.
> > >
> > > I don't see that happening since Bluetooth connection setup is
> > > serialized and we only have to make sure that bulk and isoc URBs are
> > > running when the connection is up.
> >
> > CPU A CPU B
> > HCI_NOTIFY_CONN_ADD
> > schedule_work(&data->work);
> > hdev->conn_hash.acl_num > 0
> > HCI_NOTIFY_CONN_DEL
> > schedule_work(&data->work);
> >
> > will the work handler run again?
>
> As I said, the ACL and SCO connection setup is serialized. While in
> theory this can happen, I don't see it in practice.
>
> What would be your proposal to handle this in a cleaner way?
Difficult. I was hoping to avoid scheduling work. I have to rethink.
Regards
Oliver
Hi Oliver,
> > > > > > > here's my current version of btusb with SCO support. This is preliminary.
> > > > > > > I am still looking at a way to delay using the higher altsettings until SCO
> > > > > > > is actually used, but the timeouts seem to be too long to do the obvious.
> > > > > >
> > > > > > the module parameter and blacklist/quirks stuff has been merged upstream
> > > > > > with Linus now. Feel free to update your SCO support patch and then lets
> > > > > > get this merged.
> > > > >
> > > > > Still testing. I am new to bluetooth so getting a sound testing environment
> > > > > up takes a bit of time. I am getting iso urbs to complete now.
> > > >
> > > > I hacked up a version that does work fine for me and has been tested on
> > > > my Quad G5. The attached applies on top of 2.6.27-rc3.
> > > >
> > > > The alternate settings are still fixed to selecting #2, however the
> > > > change to always select the appropriate one would be simple. We only
> > > > need to calculate the right value. The killing and re-submitting URB
> > > > code is already present.
> > >
> > > This approach has a principal race condition. You have no idea when
> > > the work queue will be run. Thus you can lose the first SCO packages.
> >
> > I am open for suggestions, but I don't see any other way to get support
> > for this. We can't keep the isoc URBs running all the time, because that
> > consumes power.
>
> How much? Or rather why not change the altsetting to the maximum
> on open and defer submitting the URBs to notify() ?
run powertop and you will see why we can't do that. Also there is no max
altsetting. It doesn't work like this. You have to pick the right one.
The Bluetooth USB spec. is messed up when it comes to SCO.
> > On the other hand, this is audio and I don't really care if we loose a
> > packet or not.
>
> It isn't limited to sound. The URBs for acl reception can also be delayed
> arbitrarily long.
We can move that into the notify() callback, but the killing the URBs
becomes a problem.
On the other hand, ACL shouldn't be any problem since there is a HCI
connection setup in between and the ACL part of Bluetooth is reliable
and we have flow control on it.
Also these are bulk URBs. I am under the assumption that the Bluetooth
controller will queue packets up until we submit the first URB.
> > > Secondly, what happens when this next event comes so quickly that
> > > the work is still scheduled or running? it seems to me that the work handler
> > > can read stale conn_hash values.
> >
> > I don't see that happening since Bluetooth connection setup is
> > serialized and we only have to make sure that bulk and isoc URBs are
> > running when the connection is up.
>
> CPU A CPU B
> HCI_NOTIFY_CONN_ADD
> schedule_work(&data->work);
> hdev->conn_hash.acl_num > 0
> HCI_NOTIFY_CONN_DEL
> schedule_work(&data->work);
>
> will the work handler run again?
As I said, the ACL and SCO connection setup is serialized. While in
theory this can happen, I don't see it in practice.
What would be your proposal to handle this in a cleaner way?
Regards
Marcel
Am Montag 18 August 2008 15:38:19 schrieben Sie:
> Hi Oliver,
>
> > > > > > here's my current version of btusb with SCO support. This is preliminary.
> > > > > > I am still looking at a way to delay using the higher altsettings until SCO
> > > > > > is actually used, but the timeouts seem to be too long to do the obvious.
> > > > >
> > > > > the module parameter and blacklist/quirks stuff has been merged upstream
> > > > > with Linus now. Feel free to update your SCO support patch and then lets
> > > > > get this merged.
> > > >
> > > > Still testing. I am new to bluetooth so getting a sound testing environment
> > > > up takes a bit of time. I am getting iso urbs to complete now.
> > >
> > > I hacked up a version that does work fine for me and has been tested on
> > > my Quad G5. The attached applies on top of 2.6.27-rc3.
> > >
> > > The alternate settings are still fixed to selecting #2, however the
> > > change to always select the appropriate one would be simple. We only
> > > need to calculate the right value. The killing and re-submitting URB
> > > code is already present.
> >
> > This approach has a principal race condition. You have no idea when
> > the work queue will be run. Thus you can lose the first SCO packages.
>
> I am open for suggestions, but I don't see any other way to get support
> for this. We can't keep the isoc URBs running all the time, because that
> consumes power.
How much? Or rather why not change the altsetting to the maximum
on open and defer submitting the URBs to notify() ?
> On the other hand, this is audio and I don't really care if we loose a
> packet or not.
It isn't limited to sound. The URBs for acl reception can also be delayed
arbitrarily long.
> > Secondly, what happens when this next event comes so quickly that
> > the work is still scheduled or running? it seems to me that the work handler
> > can read stale conn_hash values.
>
> I don't see that happening since Bluetooth connection setup is
> serialized and we only have to make sure that bulk and isoc URBs are
> running when the connection is up.
CPU A CPU B
HCI_NOTIFY_CONN_ADD
schedule_work(&data->work);
hdev->conn_hash.acl_num > 0
HCI_NOTIFY_CONN_DEL
schedule_work(&data->work);
will the work handler run again?
Regards
Oliver
Hi Oliver,
> > > > > here's my current version of btusb with SCO support. This is preliminary.
> > > > > I am still looking at a way to delay using the higher altsettings until SCO
> > > > > is actually used, but the timeouts seem to be too long to do the obvious.
> > > >
> > > > the module parameter and blacklist/quirks stuff has been merged upstream
> > > > with Linus now. Feel free to update your SCO support patch and then lets
> > > > get this merged.
> > >
> > > Still testing. I am new to bluetooth so getting a sound testing environment
> > > up takes a bit of time. I am getting iso urbs to complete now.
> >
> > I hacked up a version that does work fine for me and has been tested on
> > my Quad G5. The attached applies on top of 2.6.27-rc3.
> >
> > The alternate settings are still fixed to selecting #2, however the
> > change to always select the appropriate one would be simple. We only
> > need to calculate the right value. The killing and re-submitting URB
> > code is already present.
>
> This approach has a principal race condition. You have no idea when
> the work queue will be run. Thus you can lose the first SCO packages.
I am open for suggestions, but I don't see any other way to get support
for this. We can't keep the isoc URBs running all the time, because that
consumes power.
On the other hand, this is audio and I don't really care if we loose a
packet or not.
> Secondly, what happens when this next event comes so quickly that
> the work is still scheduled or running? it seems to me that the work handler
> can read stale conn_hash values.
I don't see that happening since Bluetooth connection setup is
serialized and we only have to make sure that bulk and isoc URBs are
running when the connection is up.
> Thirdly, close() needs to be able to deal with the work still scheduled.
> You need to flush workqueues there.
Good point. I am going to fix that.
Regards
Marcel
Am Montag 18 August 2008 12:13:49 schrieb Marcel Holtmann:
> Hi Oliver,
>
> > > > here's my current version of btusb with SCO support. This is preliminary.
> > > > I am still looking at a way to delay using the higher altsettings until SCO
> > > > is actually used, but the timeouts seem to be too long to do the obvious.
> > >
> > > the module parameter and blacklist/quirks stuff has been merged upstream
> > > with Linus now. Feel free to update your SCO support patch and then lets
> > > get this merged.
> >
> > Still testing. I am new to bluetooth so getting a sound testing environment
> > up takes a bit of time. I am getting iso urbs to complete now.
>
> I hacked up a version that does work fine for me and has been tested on
> my Quad G5. The attached applies on top of 2.6.27-rc3.
>
> The alternate settings are still fixed to selecting #2, however the
> change to always select the appropriate one would be simple. We only
> need to calculate the right value. The killing and re-submitting URB
> code is already present.
This approach has a principal race condition. You have no idea when
the work queue will be run. Thus you can lose the first SCO packages.
Secondly, what happens when this next event comes so quickly that
the work is still scheduled or running? it seems to me that the work handler
can read stale conn_hash values.
Thirdly, close() needs to be able to deal with the work still scheduled.
You need to flush workqueues there.
Regards
Oliver
Hi Oliver,
> > > here's my current version of btusb with SCO support. This is preliminary.
> > > I am still looking at a way to delay using the higher altsettings until SCO
> > > is actually used, but the timeouts seem to be too long to do the obvious.
> >
> > the module parameter and blacklist/quirks stuff has been merged upstream
> > with Linus now. Feel free to update your SCO support patch and then lets
> > get this merged.
>
> Still testing. I am new to bluetooth so getting a sound testing environment
> up takes a bit of time. I am getting iso urbs to complete now.
I hacked up a version that does work fine for me and has been tested on
my Quad G5. The attached applies on top of 2.6.27-rc3.
The alternate settings are still fixed to selecting #2, however the
change to always select the appropriate one would be simple. We only
need to calculate the right value. The killing and re-submitting URB
code is already present.
Regards
Marcel
Hi Oliver,
> > > Still testing. I am new to bluetooth so getting a sound testing environment
> > > up takes a bit of time. I am getting iso urbs to complete now.
> >
> > I think using scotest would be fine in the beginning. It will of course
> > not send proper audio, but you see data going forth and back.
>
> Compiling scotest is the problem:
>
> make[3]: Entering directory `/home/oliver/Documents/bluez-utils-3.36/hcid'
> gcc -DHAVE_CONFIG_H -I. -I.. -I../common -I../sdpd -I/usr/include/dbus-1.0 -I/usr/lib64/dbus-1.0/include -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include -I../gdbus -DPLUGINDIR=\""/usr/local/lib/bluetooth/plugins"\" -Wall -O2 -D_FORTIFY_SOURCE=2 -fPIE -MT security.o -MD -MP -MF .deps/security.Tpo -c -o security.o security.c
> security.c: In function ‘link_key_request’:
> security.c:279: error: storage size of ‘req’ isn’t known
> security.c:291: error: ‘HCIGETAUTHINFO’ undeclared (first use in this function)
> security.c:291: error: (Each undeclared identifier is reported only once
> security.c:291: error: for each function it appears in.)
> security.c:279: warning: unused variable ‘req’
> make[3]: *** [security.o] Fehler 1
> make[3]: Leaving directory `/home/oliver/Documents/bluez-utils-3.36/hcid'
> make[2]: *** [all] Fehler 2
> make[2]: Leaving directory `/home/oliver/Documents/bluez-utils-3.36/hcid'
> make[1]: *** [all-recursive] Fehler 1
> make[1]: Leaving directory `/home/oliver/Documents/bluez-utils-3.36'
> make: *** [all] Fehler 2
check your ChangeLog file. It says you need a matching or newer
bluez-libs to get it compiled.
Regards
Marcel
Am Dienstag 12 August 2008 23:36:58 schrieb Marcel Holtmann:
> > Still testing. I am new to bluetooth so getting a sound testing environ=
ment
> > up takes a bit of time. I am getting iso urbs to complete now.
>=20
> I think using scotest would be fine in the beginning. It will of course
> not send proper audio, but you see data going forth and back.
Compiling scotest is the problem:
make[3]: Entering directory `/home/oliver/Documents/bluez-utils-3.36/hcid'
gcc -DHAVE_CONFIG_H -I. -I.. -I../common -I../sdpd -I/usr/include/dbus-1.=
0 -I/usr/lib64/dbus-1.0/include -I/usr/include/glib-2.0 -I/usr/lib64/glib-2=
=2E0/include -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include -I..=
/gdbus -DPLUGINDIR=3D\""/usr/local/lib/bluetooth/plugins"\" -Wall -O2 -D_FO=
RTIFY_SOURCE=3D2 -fPIE -MT security.o -MD -MP -MF .deps/security.Tpo -c -o =
security.o security.c
security.c: In function =E2=80=98link_key_request=E2=80=99:
security.c:279: error: storage size of =E2=80=98req=E2=80=99 isn=E2=80=99t =
known
security.c:291: error: =E2=80=98HCIGETAUTHINFO=E2=80=99 undeclared (first u=
se in this function)
security.c:291: error: (Each undeclared identifier is reported only once
security.c:291: error: for each function it appears in.)
security.c:279: warning: unused variable =E2=80=98req=E2=80=99
make[3]: *** [security.o] Fehler 1
make[3]: Leaving directory `/home/oliver/Documents/bluez-utils-3.36/hcid'
make[2]: *** [all] Fehler 2
make[2]: Leaving directory `/home/oliver/Documents/bluez-utils-3.36/hcid'
make[1]: *** [all-recursive] Fehler 1
make[1]: Leaving directory `/home/oliver/Documents/bluez-utils-3.36'
make: *** [all] Fehler 2
Regards
Oliver
Hi Oliver,
> > > here's my current version of btusb with SCO support. This is preliminary.
> > > I am still looking at a way to delay using the higher altsettings until SCO
> > > is actually used, but the timeouts seem to be too long to do the obvious.
> >
> > the module parameter and blacklist/quirks stuff has been merged upstream
> > with Linus now. Feel free to update your SCO support patch and then lets
> > get this merged.
>
> Still testing. I am new to bluetooth so getting a sound testing environment
> up takes a bit of time. I am getting iso urbs to complete now.
I think using scotest would be fine in the beginning. It will of course
not send proper audio, but you see data going forth and back.
Also post patches around and I can see if I can test them here on my
side.
Regards
Marcel
Am Freitag 08 August 2008 23:14:30 schrieb Marcel Holtmann:
> Hi Oliver,
>
> > here's my current version of btusb with SCO support. This is preliminary.
> > I am still looking at a way to delay using the higher altsettings until SCO
> > is actually used, but the timeouts seem to be too long to do the obvious.
>
> the module parameter and blacklist/quirks stuff has been merged upstream
> with Linus now. Feel free to update your SCO support patch and then lets
> get this merged.
Still testing. I am new to bluetooth so getting a sound testing environment
up takes a bit of time. I am getting iso urbs to complete now.
Regards
Oliver
Hi Oliver,
> here's my current version of btusb with SCO support. This is preliminary.
> I am still looking at a way to delay using the higher altsettings until SCO
> is actually used, but the timeouts seem to be too long to do the obvious.
the module parameter and blacklist/quirks stuff has been merged upstream
with Linus now. Feel free to update your SCO support patch and then lets
get this merged.
Regards
Marcel
Am Montag 04 August 2008 22:24:08 schrieb Marcel Holtmann:
> Hi Oliver,
>
> > > >>> Then why do you implement this option for hci_usb?
> > > >>> And why can the other IGNORE options be overridden?
> > > >>
> > > >> if I wanna use the generic Bluetooth descriptor for matching, I
> > > >> need a
> > > >> way to mark broken devices as to be ignored. Otherwise I would have a
> > > >> really long list of matching vendor and product ids.
> > > >
> > > > True, but if btusb is to replace hci_usb, the module options should
> > > > match.
> > > > So will you remove that option in hci_usb?
> > >
> > > the override_ignore is your invention. So what do you mean?
> >
> > Hm, I may be smoking strange kernels, but I copied this as far as I can tell
> > from 2.6.25 hci_usb and merely renamed it because "ignore" seemed too
> > generic to me. Could you check we are talking about the same parameter?
>
> the generic ignore parameter can be removed. There is no other way to
> deal with Bluetooth in Linux and we can use other ways to unattach a
> driver from a device.
Exactly. Therefore I wonder whether this in vanilla hci_usb is not a mistake:
if (ignore || id->driver_info & HCI_IGNORE)
return -ENODEV;
if (ignore_dga && id->driver_info & HCI_DIGIANSWER)
return -ENODEV;
if (ignore_csr && id->driver_info & HCI_CSR)
return -ENODEV;
And should be (ignore && id->driver_info & HCI_IGNORE) or
negatively formulated (!ignore && id->driver_info & HCI_IGNORE)
so that the blacklist is used by default.
Regards
Oliver
Hi Oliver,
> > >>> Then why do you implement this option for hci_usb?
> > >>> And why can the other IGNORE options be overridden?
> > >>
> > >> if I wanna use the generic Bluetooth descriptor for matching, I
> > >> need a
> > >> way to mark broken devices as to be ignored. Otherwise I would have a
> > >> really long list of matching vendor and product ids.
> > >
> > > True, but if btusb is to replace hci_usb, the module options should
> > > match.
> > > So will you remove that option in hci_usb?
> >
> > the override_ignore is your invention. So what do you mean?
>
> Hm, I may be smoking strange kernels, but I copied this as far as I can tell
> from 2.6.25 hci_usb and merely renamed it because "ignore" seemed too
> generic to me. Could you check we are talking about the same parameter?
the generic ignore parameter can be removed. There is no other way to
deal with Bluetooth in Linux and we can use other ways to unattach a
driver from a device.
Regards
Marcel
Am Montag 04 August 2008 19:25:17 schrieb Marcel Holtmann:
> Hi Oliver,
>=20
> >>> Then why do you implement this option for hci_usb?
> >>> And why can the other IGNORE options be overridden?
> >>
> >> if I wanna use the generic Bluetooth descriptor for matching, I =20
> >> need a
> >> way to mark broken devices as to be ignored. Otherwise I would have a
> >> really long list of matching vendor and product ids.
> >
> > True, but if btusb is to replace hci_usb, the module options should =20
> > match.
> > So will you remove that option in hci_usb?
>=20
> the override_ignore is your invention. So what do you mean?
Hm, I may be smoking strange kernels, but I copied this as far as I can tell
from 2.6.25 hci_usb and merely renamed it because "ignore" seemed too
generic to me. Could you check we are talking about the same parameter?
Gru=DF
Oliver
Hi Oliver,
>>> Then why do you implement this option for hci_usb?
>>> And why can the other IGNORE options be overridden?
>>
>> if I wanna use the generic Bluetooth descriptor for matching, I
>> need a
>> way to mark broken devices as to be ignored. Otherwise I would have a
>> really long list of matching vendor and product ids.
>
> True, but if btusb is to replace hci_usb, the module options should
> match.
> So will you remove that option in hci_usb?
the override_ignore is your invention. So what do you mean?
Regards
Marcel
Am Montag 04 August 2008 18:33:36 schrieben Sie:
> > Then why do you implement this option for hci_usb?
> > And why can the other IGNORE options be overridden?
>=20
> if I wanna use the generic Bluetooth descriptor for matching, I need a =A0
> way to mark broken devices as to be ignored. Otherwise I would have a =A0
> really long list of matching vendor and product ids.
True, but if btusb is to replace hci_usb, the module options should match.
So will you remove that option in hci_usb?
Regards
Oliver
Hi Oliver,
>>>>> +module_param(override_ignore, bool, 0644);
>>>>> +MODULE_PARM_DESC(override_ignore, "Drive blacklisted devices");
>>>>
>>>> Why do we need this one? What is it good for?
>>>
>>> To override HCI_IGNORE. IF we provide overrides for blacklist
>>> entries,
>>> it should be done systematically.
>>
>> no we should not do that. The HCI_IGNORE is for devices that pretend
>> to be Bluetooth H:2 compatible, but they are not. In these cases we
>> do
>> have other drivers that do this right. See bcm203x and bpa10x
>> drivers.
>
> Then why do you implement this option for hci_usb?
> And why can the other IGNORE options be overridden?
if I wanna use the generic Bluetooth descriptor for matching, I need a
way to mark broken devices as to be ignored. Otherwise I would have a
really long list of matching vendor and product ids.
The other ones are special cases where it in some situations make
sense to either ignore the device or allow it. The ignore_csr is for
the CSR ROM chips that need loading of persistent settings first. The
Digianswer is for an old development hardware that had some issues.
And the sniffer is normally driven by userspace apps. However it might
work with the hci_usb driver, but it will not give you a normal
working Bluetooth device.
Regards
Marcel
Am Montag 04 August 2008 18:05:11 schrieb Marcel Holtmann:
> Hi Oliver,
>
> >>>
> >>> +module_param(override_ignore, bool, 0644);
> >>> +MODULE_PARM_DESC(override_ignore, "Drive blacklisted devices");
> >>
> >> Why do we need this one? What is it good for?
> >
> > To override HCI_IGNORE. IF we provide overrides for blacklist entries,
> > it should be done systematically.
>
> no we should not do that. The HCI_IGNORE is for devices that pretend
> to be Bluetooth H:2 compatible, but they are not. In these cases we do
> have other drivers that do this right. See bcm203x and bpa10x drivers.
Then why do you implement this option for hci_usb?
And why can the other IGNORE options be overridden?
Regards
Oliver
Hi Oliver,
>>>
>>> +module_param(override_ignore, bool, 0644);
>>> +MODULE_PARM_DESC(override_ignore, "Drive blacklisted devices");
>>
>> Why do we need this one? What is it good for?
>
> To override HCI_IGNORE. IF we provide overrides for blacklist entries,
> it should be done systematically.
no we should not do that. The HCI_IGNORE is for devices that pretend
to be Bluetooth H:2 compatible, but they are not. In these cases we do
have other drivers that do this right. See bcm203x and bpa10x drivers.
Regards
Marcel
Am Freitag 01 August 2008 19:45:18 schrieb Marcel Holtmann:
> Hi Oliver,
>
> > > Both table updates should be a separate patch and we can even submit
> > > that for 2.6.27 inclusion. I was just to lazy to do that, but I have
> > > it on my todo list, but the Simple Pairing support had higher priority.
> >
> > This patch implements the quirks from hci_usb in btusb, too.
> >
> > --- linux-2.6.22/drivers/bluetooth/btusb.c 2008-08-01 10:53:38.000000000 +0200
> > +++ linux-2.6.26/drivers/bluetooth/btusb.c 2008-08-01 13:47:54.000000000 +0200
> > @@ -41,21 +41,108 @@
> > #define BT_DBG(D...)
> > #endif
> >
> > -#define VERSION "0.1"
> > +#define VERSION "0.3"
>
> the version number increase to 0.3 is bogus, it should be 0.2.
Roger.
> > +module_param(override_ignore, bool, 0644);
> > +MODULE_PARM_DESC(override_ignore, "Drive blacklisted devices");
>
> Why do we need this one? What is it good for?
To override HCI_IGNORE. IF we provide overrides for blacklist entries,
it should be done systematically.
Regards
Oliver
On Fri, 1 Aug 2008, Marcel Holtmann wrote:
> Question is if selecting the 2nd alternate setting and not submitting
> the URBs for it is a good idea or if that will still consume bandwidth
> and power. And thus it would be better to use the first alternate
> setting that is meant for the case with no SCO connections.
Installing the second altsetting and not submitting URBs will not
consume bandwidth. It may consume a little power, but probably not
much more than just using the first altsetting.
Alan Stern
Hi Oliver,
> > Both table updates should be a separate patch and we can even submit
> > that for 2.6.27 inclusion. I was just to lazy to do that, but I have
> > it on my todo list, but the Simple Pairing support had higher priority.
>
> This patch implements the quirks from hci_usb in btusb, too.
>
> --- linux-2.6.22/drivers/bluetooth/btusb.c 2008-08-01 10:53:38.000000000 +0200
> +++ linux-2.6.26/drivers/bluetooth/btusb.c 2008-08-01 13:47:54.000000000 +0200
> @@ -41,21 +41,108 @@
> #define BT_DBG(D...)
> #endif
>
> -#define VERSION "0.1"
> +#define VERSION "0.3"
the version number increase to 0.3 is bogus, it should be 0.2.
> +module_param(override_ignore, bool, 0644);
> +MODULE_PARM_DESC(override_ignore, "Drive blacklisted devices");
Why do we need this one? What is it good for?
Regards
Marcel
Hi Oliver,
> > Also the HCI_RESET thing needs to be turn-around. We always do
> > HCI_RESET except for some chips where it will break (old CSR ones for
> > example).
>
> OK, I am making a separate patch. I've looked at 2.6.26-mh. It seems
> to do the reset only for quirky devices. Is the reversal just planned or am
> I looking at the wrong trees?
the reversal is just planned, but we should do it soon since we need the
reset quirk for almost every Broadcom chip.
Regards
Marcel
Hi Oliver,
> > yeah, I was afraid of that switching the alternate settings is not as
> > easy as the spec. might think. Personally I think it is a broken
> > specification anyway. So the best assumption is that we have one SCO
> > connection and that is either 8-bit or 16-bit. So we do the switching
> > when we get the notify() callback for a changed voice setting.
>
> That's problematic because in the notify() callback you cannot sleep,
> can you? In addition, a device is legally allowed to take a lot of time
> to switch altsettings.
that was not my point. My point was to switch between first or second
alternate setting when you get the notify() callback for a changed voice
setting. Check with the "hciconfig hci0 voice 0x0040" command.
We have to do it in a work queue and that is fine, but then when we get
a connection add notify(), then we only have to start the SCO URBs. This
leaves the only broken case where we have to switch the alternate
setting, because the second SCO channel got established.
Question is if selecting the 2nd alternate setting and not submitting
the URBs for it is a good idea or if that will still consume bandwidth
and power. And thus it would be better to use the first alternate
setting that is meant for the case with no SCO connections.
Regards
Marcel
Am Donnerstag 31 Juli 2008 16:03:13 schrieb Marcel Holtmann:
> Both table updates should be a separate patch and we can even submit =A0
> that for 2.6.27 inclusion. I was just to lazy to do that, but I have =A0
> it on my todo list, but the Simple Pairing support had higher priority.
This patch implements the quirks from hci_usb in btusb, too.
Signed-off-by: Oliver Neukum <[email protected]>
Regards
Oliver
=2D--
=2D-- linux-2.6.22/drivers/bluetooth/btusb.c 2008-08-01 10:53:38.000000000 =
+0200
+++ linux-2.6.26/drivers/bluetooth/btusb.c 2008-08-01 13:47:54.000000000 +0=
200
@@ -41,21 +41,108 @@
#define BT_DBG(D...)
#endif
=20
=2D#define VERSION "0.1"
+#define VERSION "0.3"
+
+#define HCI_IGNORE 0x01
+#define HCI_RESET 0x02
+#define HCI_DIGIANSWER 0x04
+#define HCI_CSR 0x08
+#define HCI_SNIFFER 0x10
+#define HCI_BCM92035 0x20
+#define HCI_BROKEN_ISOC 0x40
+#define HCI_WRONG_SCO_MTU 0x80
=20
static struct usb_device_id btusb_table[] =3D {
/* Generic Bluetooth USB device */
{ USB_DEVICE_INFO(0xe0, 0x01, 0x01) },
=20
+ /* AVM BlueFRITZ! USB v2.0 */
+ { USB_DEVICE(0x057c, 0x3800) },
+
+ /* Bluetooth Ultraport Module from IBM */
+ { USB_DEVICE(0x04bf, 0x030a) },
+
+ /* ALPS Modules with non-standard id */
+ { USB_DEVICE(0x044e, 0x3001) },
+ { USB_DEVICE(0x044e, 0x3002) },
+
+ /* Ericsson with non-standard id */
+ { USB_DEVICE(0x0bdb, 0x1002) },
+
+ /* Canyon CN-BTU1 with HID interfaces */
+ { USB_DEVICE(0x0c10, 0x0000), .driver_info =3D HCI_RESET },
+
{ } /* Terminating entry */
};
=20
MODULE_DEVICE_TABLE(usb, btusb_table);
=20
static struct usb_device_id blacklist_table[] =3D {
+ /* CSR BlueCore devices */
+ { USB_DEVICE(0x0a12, 0x0001), .driver_info =3D HCI_CSR },
+
+ /* Broadcom BCM2033 without firmware */
+ { USB_DEVICE(0x0a5c, 0x2033), .driver_info =3D HCI_IGNORE },
+
+ /* Broadcom BCM2035 */
+ { USB_DEVICE(0x0a5c, 0x2035), .driver_info =3D HCI_RESET | HCI_WRONG_SCO_=
MTU },
+ { USB_DEVICE(0x0a5c, 0x200a), .driver_info =3D HCI_RESET | HCI_WRONG_SCO_=
MTU },
+
+ /* Broadcom BCM2045 */
+ { USB_DEVICE(0x0a5c, 0x2039), .driver_info =3D HCI_RESET | HCI_WRONG_SCO_=
MTU },
+ { USB_DEVICE(0x0a5c, 0x2101), .driver_info =3D HCI_RESET | HCI_WRONG_SCO_=
MTU },
+
+ /* IBM/Lenovo ThinkPad with Broadcom chip */
+ { USB_DEVICE(0x0a5c, 0x201e), .driver_info =3D HCI_RESET | HCI_WRONG_SCO_=
MTU },
+ { USB_DEVICE(0x0a5c, 0x2110), .driver_info =3D HCI_RESET | HCI_WRONG_SCO_=
MTU },
+
+ /* Targus ACB10US */
+ { USB_DEVICE(0x0a5c, 0x2100), .driver_info =3D HCI_RESET },
+
+ /* ANYCOM Bluetooth USB-200 and USB-250 */
+ { USB_DEVICE(0x0a5c, 0x2111), .driver_info =3D HCI_RESET },
+
+ /* HP laptop with Broadcom chip */
+ { USB_DEVICE(0x03f0, 0x171d), .driver_info =3D HCI_RESET | HCI_WRONG_SCO_=
MTU },
+
+ /* Dell laptop with Broadcom chip */
+ { USB_DEVICE(0x413c, 0x8126), .driver_info =3D HCI_RESET | HCI_WRONG_SCO_=
MTU },
+
+ /* Microsoft Wireless Transceiver for Bluetooth 2.0 */
+ { USB_DEVICE(0x045e, 0x009c), .driver_info =3D HCI_RESET },
+
+ /* Kensington Bluetooth USB adapter */
+ { USB_DEVICE(0x047d, 0x105d), .driver_info =3D HCI_RESET },
+ { USB_DEVICE(0x047d, 0x105e), .driver_info =3D HCI_RESET | HCI_WRONG_SCO_=
MTU },
+
+ /* ISSC Bluetooth Adapter v3.1 */
+ { USB_DEVICE(0x1131, 0x1001), .driver_info =3D HCI_RESET },
+
+ /* RTX Telecom based adapters with buggy SCO support */
+ { USB_DEVICE(0x0400, 0x0807), .driver_info =3D HCI_BROKEN_ISOC },
+ { USB_DEVICE(0x0400, 0x080a), .driver_info =3D HCI_BROKEN_ISOC },
+
+ /* CONWISE Technology based adapters with buggy SCO support */
+ { USB_DEVICE(0x0e5e, 0x6622), .driver_info =3D HCI_BROKEN_ISOC },
+
+ /* Belkin F8T012 and F8T013 devices */
+ { USB_DEVICE(0x050d, 0x0012), .driver_info =3D HCI_RESET | HCI_WRONG_SCO_=
MTU },
+ { USB_DEVICE(0x050d, 0x0013), .driver_info =3D HCI_RESET | HCI_WRONG_SCO_=
MTU },
+
+ /* Digianswer devices */
+ { USB_DEVICE(0x08fd, 0x0001), .driver_info =3D HCI_DIGIANSWER },
+ { USB_DEVICE(0x08fd, 0x0002), .driver_info =3D HCI_IGNORE },
+
+ /* CSR BlueCore Bluetooth Sniffer */
+ { USB_DEVICE(0x0a12, 0x0002), .driver_info =3D HCI_SNIFFER },
+
+ /* Frontline ComProbe Bluetooth Sniffer */
+ { USB_DEVICE(0x16d3, 0x0002), .driver_info =3D HCI_SNIFFER },
{ } /* Terminating entry */
};
=20
+static struct usb_driver btusb_driver;
+
#define BTUSB_INTR_RUNNING 0
#define BTUSB_BULK_RUNNING 1
=20
@@ -78,6 +165,16 @@ struct btusb_data {
struct usb_endpoint_descriptor *bulk_rx_ep;
};
=20
+static int ignore_dga;
+static int ignore_csr;
+static int ignore_sniffer;
+static int disable_scofix;
+#ifdef CONFIG_BT_HCIUSB_SCO
+static int force_scofix;
+#endif
+static int reset;
+static int override_ignore;
+
static void btusb_intr_complete(struct urb *urb)
{
struct hci_dev *hdev =3D urb->context;
@@ -433,6 +530,7 @@ static int btusb_probe(struct usb_interf
=20
BT_DBG("intf %p id %p", intf, id);
=20
+ /* interface numbers are hardcoded in the spec */
if (intf->cur_altsetting->desc.bInterfaceNumber !=3D 0)
return -ENODEV;
=20
@@ -443,6 +541,15 @@ static int btusb_probe(struct usb_interf
id =3D match;
}
=20
+ if (id->driver_info =3D=3D HCI_IGNORE && !override_ignore)
+ return -ENODEV;
+ if (ignore_sniffer && id->driver_info & HCI_SNIFFER)
+ return -ENODEV;
+ if (ignore_csr && id->driver_info & HCI_CSR)
+ return -ENODEV;
+ if (ignore_dga && id->driver_info & HCI_DIGIANSWER)
+ return -ENODEV;
+
data =3D kzalloc(sizeof(*data), GFP_KERNEL);
if (!data)
return -ENOMEM;
@@ -503,7 +610,15 @@ static int btusb_probe(struct usb_interf
=20
hdev->owner =3D THIS_MODULE;
=20
=2D set_bit(HCI_QUIRK_RESET_ON_INIT, &hdev->quirks);
+ if (reset || id->driver_info & HCI_RESET)
+ set_bit(HCI_QUIRK_RESET_ON_INIT, &hdev->quirks);
+
+#ifdef CONFIG_BT_HCIUSB_SCO
+ if (force_scofix || id->driver_info & HCI_WRONG_SCO_MTU) {
+ if (!disable_scofix)
+ set_bit(HCI_QUIRK_FIXUP_BUFFER_SIZE, &hdev->quirks);
+ }
+#endif
=20
err =3D hci_register_dev(hdev);
if (err < 0) {
@@ -558,6 +673,29 @@ static void __exit btusb_exit(void)
module_init(btusb_init);
module_exit(btusb_exit);
=20
+module_param(reset, bool, 0644);
+MODULE_PARM_DESC(reset, "Skip HCI reset command on initialization");
+
+#ifdef CONFIG_BT_HCIUSB_SCO
+module_param(force_scofix, bool, 0644);
+MODULE_PARM_DESC(force_scofix, "Force fixup of wrong SCO buffers size");
+#endif
+
+module_param(disable_scofix, bool, 0644);
+MODULE_PARM_DESC(disable_scofix, "Disable fixup of wrong SCO buffer size");
+
+module_param(ignore_csr, bool, 0644);
+MODULE_PARM_DESC(ignore_csr, "Ignore devices with id 0a12:0001");
+
+module_param(ignore_sniffer, bool, 0644);
+MODULE_PARM_DESC(ignore_sniffer, "Ignore devices with id 0a12:0002");
+
+module_param(ignore_dga, bool, 0644);
+MODULE_PARM_DESC(ignore_dga, "Ignore devices with id 08fd:0001");
+
+module_param(override_ignore, bool, 0644);
+MODULE_PARM_DESC(override_ignore, "Drive blacklisted devices");
+
MODULE_AUTHOR("Marcel Holtmann <[email protected]>");
MODULE_DESCRIPTION("Generic Bluetooth USB driver ver " VERSION);
MODULE_VERSION(VERSION);
Am Donnerstag 31 Juli 2008 16:03:13 schrieb Marcel Holtmann:
> Also the HCI_RESET thing needs to be turn-around. We always do =A0
> HCI_RESET except for some chips where it will break (old CSR ones for =A0
> example).
OK, I am making a separate patch. I've looked at 2.6.26-mh. It seems
to do the reset only for quirky devices. Is the reversal just planned or am
I looking at the wrong trees?
Regards
Oliver