EDR support

EDR support

[Manuel Naranjo]

Hi guys,

I was checking the latest BlueZ source code and the kernel source code, 
and I noticed that by default EDR is disabled, when a dongle is detected 
net/bluetooth/hci_core.c initializes the packet types with DM1, DH1 and 
HV1, and then net/bluetooth/hci_event.c does it for DM3-5 DH3-5 and 
HV3-5, but it never initializes the 2DHx or 3DHx. Was it made on purpose 
or is it a bug in the code?

I just figured out how to help and try fixing it, but kernel hacking 
isn't an easy to do task, and would prefer to get professional advice first.

Manuel

Re: EDR support

[Marcel Holtmann]

Hi Manuel,

> I was checking the latest BlueZ source code and the kernel source code, 
> and I noticed that by default EDR is disabled, when a dongle is detected 
> net/bluetooth/hci_core.c initializes the packet types with DM1, DH1 and 
> HV1, and then net/bluetooth/hci_event.c does it for DM3-5 DH3-5 and 
> HV3-5, but it never initializes the 2DHx or 3DHx. Was it made on purpose 
> or is it a bug in the code?

you have read the specification and realized that EDR uses inverse
values?

Regards

Marcel

[RFC][PATCH] Re: EDR support

[Manuel Naranjo]

[-- Attachment #1: Type: text/plain, Size: 2544 bytes --]

Hi Marcel,
> Hi Manuel,
>
>> I was checking the latest BlueZ source code and the kernel source code,
>> and I noticed that by default EDR is disabled, when a dongle is detected
>> net/bluetooth/hci_core.c initializes the packet types with DM1, DH1 and
>> HV1, and then net/bluetooth/hci_event.c does it for DM3-5 DH3-5 and
>> HV3-5, but it never initializes the 2DHx or 3DHx. Was it made on purpose
>> or is it a bug in the code?
> you have read the specification and realized that EDR uses inverse
> values?
I can't find that, but still I could find that according to the 2.1 
specs + EDR when you want to enable EDR is up to the master or slave to 
ask for the package type switch, that means as far as I understand ask 
the other side to enable 2-DH1,2-DH3,2-DH5,3-DH1,3-DH3,3-DH5.

I get from this that from the BlueZ point of view is either the kernel 
that should do that, or the BlueZ daemon or just rely that to the user 
application.

But I think there are a few of non covered issues here, first when you 
attach an EDR dongle which has the EDR (this means is capable of doing 
2mbps and 3mbps acl connections the kernel side doesn't show this to the 
userland (hciconfig), I made a patch that does exactly that, during the 
device recognition of the kernel just read the features provided and or 
that to the pkt_type flag. This helps a lot, now my dongle shows it can 
do EDR:
EDR compatible:
         Features: 0xff 0xff 0x8f 0xfe 0x9b 0xff 0x59 0x83
         Packet type: DM1 DM3 DM5 DH1 DH3 DH5 HV1 HV2 HV3 2-DH1 2-DH3 
2-DH5 3-DH1 3-DH3 3-DH5

EDR not compatible:
         Features: 0xff 0xff 0x8f 0xf8 0x1b 0xf8 0x00 0x80
         Packet type: DM1 DM3 DM5 DH1 DH3 DH5 HV1 HV2 HV3

If you think the patch is sane to be included then I format it properly 
so far I made it against compat-wireless-2.6.36-4 but it wouldn't be an 
issue to make it against the current kernel source.

Next thing was playing with hcitool cpt and that did work, I get speed 
improvements, from ~ 60KBs I get up to 100KBs to the same device, just 
by forcing it to only do 3-DH5 (which I know is not a good idea, because 
it's best for both radios to adapt, based on the environment).

I think it's a good idea to rely EDR "enabling" to the user space, 
otherwise connection complexity on the kernel well be way harder to 
keep. I think the best would be to add some stuff to the DBUS so first 
let the user know which packet types are available, second to allow it 
to change on a new connection.

Please let me know what you think about it,

Manuel

[-- Attachment #2: enableEDR.patch --]
[-- Type: text/plain, Size: 2926 bytes --]

diff -uprN compat-wireless-2.6.36-4/include/net/bluetooth/hci.h compat-wireless-2.6.36-4.mine//include/net/bluetooth/hci.h
--- compat-wireless-2.6.36-4/include/net/bluetooth/hci.h	2010-11-08 22:00:51.000000000 -0300
+++ compat-wireless-2.6.36-4.mine//include/net/bluetooth/hci.h	2010-12-11 19:13:14.000000000 -0300
@@ -1,6 +1,7 @@
 /* 
    BlueZ - Bluetooth protocol stack for Linux
    Copyright (C) 2000-2001 Qualcomm Incorporated
+   Copyright (C) 2010 Naranjo Manuel Francisco <manuel@aircable.net>
 
    Written 2000,2001 by Maxim Krasnyansky <maxk@qualcomm.com>
 
@@ -120,17 +121,31 @@ enum {
 #define HCI_VENDOR_PKT		0xff
 
 /* HCI packet types */
+#define HCI_2DH1	0x0002
+#define HCI_3DH1	0x0004
 #define HCI_DM1		0x0008
-#define HCI_DM3		0x0400
-#define HCI_DM5		0x4000
 #define HCI_DH1		0x0010
+#define HCI_2DH3	0x0100
+#define HCI_3DH3	0x0200
+#define HCI_DM3		0x0400
 #define HCI_DH3		0x0800
+#define HCI_2DH5	0x1000
+#define HCI_3DH5	0x2000
+#define HCI_DM5		0x4000
 #define HCI_DH5		0x8000
 
 #define HCI_HV1		0x0020
 #define HCI_HV2		0x0040
 #define HCI_HV3		0x0080
 
+#define HCI_EV3		0x0008
+#define HCI_EV4		0x0010
+#define HCI_EV5		0x0020
+#define HCI_2EV3	0x0040
+#define HCI_3EV3	0x0080
+#define HCI_2EV5	0x0100
+#define HCI_3EV5	0x0200
+
 #define SCO_PTYPE_MASK	(HCI_HV1 | HCI_HV2 | HCI_HV3)
 #define ACL_PTYPE_MASK	(~SCO_PTYPE_MASK)
 
@@ -183,6 +198,12 @@ enum {
 #define LMP_PSCHEME	0x02
 #define LMP_PCONTROL	0x04
 
+#define LMP_EDR_ACL_2M	0x02
+#define LMP_EDR_ACL_3M	0x04
+#define LMP_ENH_ISCAN	0x08
+#define LMP_ILACE_ISCAN	0x10
+#define LMP_ILACE_PSCAN	0x20
+#define LMP_RSSI_INQ	0x40
 #define LMP_ESCO	0x80
 
 #define LMP_EV4		0x01
diff -uprN compat-wireless-2.6.36-4/net/bluetooth/hci_event.c compat-wireless-2.6.36-4.mine//net/bluetooth/hci_event.c
--- compat-wireless-2.6.36-4/net/bluetooth/hci_event.c	2010-11-08 22:00:51.000000000 -0300
+++ compat-wireless-2.6.36-4.mine//net/bluetooth/hci_event.c	2010-12-11 19:13:46.000000000 -0300
@@ -1,6 +1,7 @@
 /*
    BlueZ - Bluetooth protocol stack for Linux
    Copyright (c) 2000-2001, 2010, Code Aurora Forum. All rights reserved.
+   Copyright (c) 2010 Naranjo Manuel Francisco <manuel@aircable.net>
 
    Written 2000,2001 by Maxim Krasnyansky <maxk@qualcomm.com>
 
@@ -478,6 +479,26 @@ static void hci_cc_read_local_features(s
 	if (hdev->features[3] & LMP_ESCO)
 		hdev->esco_type |= (ESCO_EV3);
 
+	if (hdev->features[3] & LMP_EDR_ACL_2M){
+		hdev->pkt_type |= (HCI_2DH1);
+
+		if (hdev->features[0] & LMP_3SLOT)
+		    hdev->pkt_type |= (HCI_2DH3);
+
+		if (hdev->features[0] & LMP_5SLOT)
+		    hdev->pkt_type |= (HCI_2DH5);
+	}
+
+	if (hdev->features[3] & LMP_EDR_ACL_3M){
+		hdev->pkt_type |= (HCI_3DH1);
+
+		if (hdev->features[0] & LMP_3SLOT)
+		    hdev->pkt_type |= (HCI_3DH3);
+
+		if (hdev->features[0] & LMP_5SLOT)
+		    hdev->pkt_type |= (HCI_3DH5);
+	}
+
 	if (hdev->features[4] & LMP_EV4)
 		hdev->esco_type |= (ESCO_EV4);