* [RFC v2 2/7] ipv6: Add checks for RAWIP ARP type
From: Jukka Rissanen @ 2013-10-30 9:00 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1383123661-15087-1-git-send-email-jukka.rissanen@linux.intel.com>
Signed-off-by: Jukka Rissanen <jukka.rissanen@linux.intel.com>
---
net/ipv6/addrconf.c | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index d6ff126..60bf947 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -1783,6 +1783,15 @@ static int addrconf_ifid_ip6tnl(u8 *eui, struct net_device *dev)
return 0;
}
+static int addrconf_ifid_rawip(u8 *eui, struct net_device *dev)
+{
+ if (dev->addr_len != 8)
+ return -1;
+ memcpy(eui, dev->dev_addr, 8);
+ eui[0] ^= 2;
+ return 0;
+}
+
static int ipv6_generate_eui64(u8 *eui, struct net_device *dev)
{
switch (dev->type) {
@@ -1803,6 +1812,8 @@ static int ipv6_generate_eui64(u8 *eui, struct net_device *dev)
return addrconf_ifid_ieee1394(eui, dev);
case ARPHRD_TUNNEL6:
return addrconf_ifid_ip6tnl(eui, dev);
+ case ARPHRD_RAWIP:
+ return addrconf_ifid_rawip(eui, dev);
}
return -1;
}
@@ -2681,7 +2692,8 @@ static void addrconf_dev_config(struct net_device *dev)
(dev->type != ARPHRD_INFINIBAND) &&
(dev->type != ARPHRD_IEEE802154) &&
(dev->type != ARPHRD_IEEE1394) &&
- (dev->type != ARPHRD_TUNNEL6)) {
+ (dev->type != ARPHRD_TUNNEL6) &&
+ (dev->type != ARPHRD_RAWIP)) {
/* Alas, we support only Ethernet autoconfiguration. */
return;
}
--
1.7.11.7
^ permalink raw reply related
* [RFC v2 1/7] net: if_arp: add ARPHRD_RAWIP type
From: Jukka Rissanen @ 2013-10-30 9:00 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1383123661-15087-1-git-send-email-jukka.rissanen@linux.intel.com>
This is used when there is no L2 header before IP header.
Example of this is Bluetooth 6LoWPAN network.
The RAWIP header type value is already used in some Android kernels
so same value is used here in order not to break userspace.
Signed-off-by: Jukka Rissanen <jukka.rissanen@linux.intel.com>
---
include/uapi/linux/if_arp.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/include/uapi/linux/if_arp.h b/include/uapi/linux/if_arp.h
index d7fea34..06fc69f 100644
--- a/include/uapi/linux/if_arp.h
+++ b/include/uapi/linux/if_arp.h
@@ -59,6 +59,7 @@
#define ARPHRD_LAPB 516 /* LAPB */
#define ARPHRD_DDCMP 517 /* Digital's DDCMP protocol */
#define ARPHRD_RAWHDLC 518 /* Raw HDLC */
+#define ARPHRD_RAWIP 530 /* Raw IP */
#define ARPHRD_TUNNEL 768 /* IPIP tunnel */
#define ARPHRD_TUNNEL6 769 /* IP6IP6 tunnel */
--
1.7.11.7
^ permalink raw reply related
* [RFC v2 0/7] Bluetooth LE 6LoWPAN
From: Jukka Rissanen @ 2013-10-30 9:00 UTC (permalink / raw)
To: linux-bluetooth
Hi,
this is 6LoWPAN code for BT LE as described in
http://tools.ietf.org/html/draft-ietf-6lowpan-btle-12
v2:
- Change ARPHRD_IEEE802154 to ARPHRD_RAWIP. The generic code
in patches 1 and 2 is also sent to netdev mailing list.
- Sending route exporting patch 5 to netdev ml
- Check private/public BT address and toggle universal/local bit
accordingly in patch 3.
- The virtual interface template name is now shorter (bt%d)
- Various function name renames
- devtype of the interface set to "bluetooth"
v1:
- initial release
TODO:
- Discovery of 6LoWPAN service needs be automatic.
- Refactor compression and fragmentation code after these are fixed
in net/ieee802154/6lowpan.c. Bluetooth 6LoWPAN should be able to share
that part of the code.
Cheers,
Jukka
Jukka Rissanen (7):
net: if_arp: add ARPHRD_RAWIP type
ipv6: Add checks for RAWIP ARP type
Bluetooth: Initial skeleton code for BT 6LoWPAN
Bluetooth: Enable 6LoWPAN support for BT LE devices
route: Exporting ip6_route_add() so that Bluetooth 6LoWPAN can use it
Bluetooth: Set route to peer for 6LoWPAN
Bluetooth: Manually enable or disable 6LoWPAN between devices
include/net/bluetooth/l2cap.h | 1 +
include/uapi/linux/if_arp.h | 1 +
net/bluetooth/6lowpan.c | 1722 +++++++++++++++++++++++++++++++++++++++++
net/bluetooth/6lowpan.h | 28 +
net/bluetooth/Makefile | 2 +-
net/bluetooth/hci_core.c | 4 +
net/bluetooth/l2cap_core.c | 20 +-
net/ipv6/addrconf.c | 14 +-
net/ipv6/route.c | 1 +
9 files changed, 1790 insertions(+), 3 deletions(-)
create mode 100644 net/bluetooth/6lowpan.c
create mode 100644 net/bluetooth/6lowpan.h
--
1.7.11.7
^ permalink raw reply
* Re: [PATCH 2/2] android: Suppress compiler warnings not used in autotools build
From: Andrei Emeltchenko @ 2013-10-30 8:37 UTC (permalink / raw)
To: Szymon Janc; +Cc: linux-bluetooth
In-Reply-To: <1664552.rzmdhg2vR5@uw000953>
Hi Szymon,
On Wed, Oct 30, 2013 at 09:25:03AM +0100, Szymon Janc wrote:
> Hi Andrei,
>
> On Wednesday 30 of October 2013 10:19:15 Andrei Emeltchenko wrote:
> > Hi Szymon,
> >
> > On Tue, Oct 29, 2013 at 04:23:38PM +0100, Szymon Janc wrote:
> > > Android build system is enabling some additional warnings that are not
> > > enabled when building with autotools. This avoids spurious warnings
> > > while building on Android.
> > > ---
> > > android/Android.mk | 3 +++
> > > 1 file changed, 3 insertions(+)
> > >
> > > diff --git a/android/Android.mk b/android/Android.mk
> > > index c88b27e..fc1b276 100644
> > > --- a/android/Android.mk
> > > +++ b/android/Android.mk
> > > @@ -9,6 +9,9 @@ pathmap_INCL += glib:external/bluetooth/glib
> > > # Specify common compiler flags
> > > BLUEZ_COMMON_CFLAGS := -DVERSION=\"$(BLUEZ_VERSION)\"
> > >
> > > +# Disable warnings enabled by Android but not enabled in autotools build
> > > +BLUEZ_COMMON_CFLAGS += -Wno-pointer-arith
> >
> > I wouldn't put this to BLUEZ_COMMON_CFLAGS, maybe only bluetoothd target
> > CFLAGS, the issue withing HAL might be easily fixed. We use void pointer
> > in math operations.
>
> This is done on purpose as using void* to operation is simply convenient as
> there is no need for char* casting (and sizeof(void *) == 1 is documented GCC
> feature). Further more I think it will be easier for developers if we can have
> same code guidelines and warnings while building on host and android for all
> components.
I just sent patch with some fix proposal. I know there are discussions
about this issue, let's maintainers decide on this.
PS: I personally think that casting when assigning 2 pointers is good in a
way it shows you that you cast to a different structure.
Best regards
Andrei Emeltchenko
^ permalink raw reply
* [PATCH] android/hal: Fix using void pointer in math operations
From: Andrei Emeltchenko @ 2013-10-30 8:27 UTC (permalink / raw)
To: linux-bluetooth
From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
Fixes following compiler warnings:
...
hal-bluetooth.c: In function 'handle_adapter_props_changed':
hal-bluetooth.c:57:9: warning: pointer of type 'void *' used in arithmetic [-Wpointer-arith]
hal-bluetooth.c:57:29: warning: pointer of type 'void *' used in arithmetic [-Wpointer-arith]
hal-bluetooth.c:57:51: warning: pointer of type 'void *' used in arithmetic [-Wpointer-arith]
hal-bluetooth.c:66:5: warning: pointer of type 'void *' used in arithmetic [-Wpointer-arith]
...
---
android/hal-bluetooth.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/android/hal-bluetooth.c b/android/hal-bluetooth.c
index fc60d72..0c083de 100644
--- a/android/hal-bluetooth.c
+++ b/android/hal-bluetooth.c
@@ -44,17 +44,18 @@ static void handle_adapter_props_changed(void *buf, uint16_t len)
struct hal_ev_adapter_props_changed *ev = buf;
bt_property_t props[ev->num_props];
struct hal_property *hal_prop;
- void *p;
+ uint8_t *p;
int i;
if (!bt_hal_cbacks->adapter_properties_cb)
return;
hal_prop = ev->props;
- p = ev->props;
+ p = (uint8_t *) ev->props;
for (i = 0; i < ev->num_props; i++) {
- if (p + sizeof(*hal_prop) + hal_prop->len > buf + len) {
+ if (p + sizeof(*hal_prop) + hal_prop->len >
+ (uint8_t *) buf + len) {
error("invalid adapter properties event, aborting");
exit(EXIT_FAILURE);
}
@@ -64,7 +65,7 @@ static void handle_adapter_props_changed(void *buf, uint16_t len)
props[i].val = hal_prop->val;
p += sizeof(*hal_prop) + hal_prop->len;
- hal_prop = p;
+ hal_prop = (struct hal_property *) p;
DBG("prop[%d]: %s", i, btproperty2str(&props[i]));
}
--
1.7.10.4
^ permalink raw reply related
* Re: [PATCH 2/2] android: Suppress compiler warnings not used in autotools build
From: Szymon Janc @ 2013-10-30 8:25 UTC (permalink / raw)
To: Andrei Emeltchenko; +Cc: linux-bluetooth
In-Reply-To: <20131030081914.GH27517@aemeltch-MOBL1>
Hi Andrei,
On Wednesday 30 of October 2013 10:19:15 Andrei Emeltchenko wrote:
> Hi Szymon,
>
> On Tue, Oct 29, 2013 at 04:23:38PM +0100, Szymon Janc wrote:
> > Android build system is enabling some additional warnings that are not
> > enabled when building with autotools. This avoids spurious warnings
> > while building on Android.
> > ---
> > android/Android.mk | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/android/Android.mk b/android/Android.mk
> > index c88b27e..fc1b276 100644
> > --- a/android/Android.mk
> > +++ b/android/Android.mk
> > @@ -9,6 +9,9 @@ pathmap_INCL += glib:external/bluetooth/glib
> > # Specify common compiler flags
> > BLUEZ_COMMON_CFLAGS := -DVERSION=\"$(BLUEZ_VERSION)\"
> >
> > +# Disable warnings enabled by Android but not enabled in autotools build
> > +BLUEZ_COMMON_CFLAGS += -Wno-pointer-arith
>
> I wouldn't put this to BLUEZ_COMMON_CFLAGS, maybe only bluetoothd target
> CFLAGS, the issue withing HAL might be easily fixed. We use void pointer
> in math operations.
This is done on purpose as using void* to operation is simply convenient as
there is no need for char* casting (and sizeof(void *) == 1 is documented GCC
feature). Further more I think it will be easier for developers if we can have
same code guidelines and warnings while building on host and android for all
components.
--
BR
Szymon Janc
^ permalink raw reply
* Re: [PATCH 2/2] android: Suppress compiler warnings not used in autotools build
From: Andrei Emeltchenko @ 2013-10-30 8:19 UTC (permalink / raw)
To: Szymon Janc; +Cc: linux-bluetooth
In-Reply-To: <1383060218-25693-2-git-send-email-szymon.janc@tieto.com>
Hi Szymon,
On Tue, Oct 29, 2013 at 04:23:38PM +0100, Szymon Janc wrote:
> Android build system is enabling some additional warnings that are not
> enabled when building with autotools. This avoids spurious warnings
> while building on Android.
> ---
> android/Android.mk | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/android/Android.mk b/android/Android.mk
> index c88b27e..fc1b276 100644
> --- a/android/Android.mk
> +++ b/android/Android.mk
> @@ -9,6 +9,9 @@ pathmap_INCL += glib:external/bluetooth/glib
> # Specify common compiler flags
> BLUEZ_COMMON_CFLAGS := -DVERSION=\"$(BLUEZ_VERSION)\"
>
> +# Disable warnings enabled by Android but not enabled in autotools build
> +BLUEZ_COMMON_CFLAGS += -Wno-pointer-arith
I wouldn't put this to BLUEZ_COMMON_CFLAGS, maybe only bluetoothd target
CFLAGS, the issue withing HAL might be easily fixed. We use void pointer
in math operations.
Best regards
Andrei Emeltchenko
^ permalink raw reply
* Re: [PATCH BlueZ] audio/AVRCP: Always assign a player to TG role
From: Johan Hedberg @ 2013-10-30 7:39 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: linux-bluetooth
In-Reply-To: <1383055634-11953-1-git-send-email-luiz.dentz@gmail.com>
Hi Luiz,
On Tue, Oct 29, 2013, Luiz Augusto von Dentz wrote:
> Currently the code does not assign a player if version is bellow 1.3,
> but sometimes the version may not be initialized correctly e.g: record
> could not be fetch.
>
> Futhermore by assigning a player for 1.0 CT it won't break backward
> compatibility as no commands will be sent but adds the possibility to
> respond properly in case the CT is sending commands above its own
> version which is a violation of the spec but unfortunately there exists
> such stacks on the market.
> ---
> profiles/audio/avrcp.c | 3 ---
> 1 file changed, 3 deletions(-)
Applied. Thanks.
Johan
^ permalink raw reply
* Re: [PATCH] Bluetooth: Set default own address type only during controller setup
From: Johan Hedberg @ 2013-10-30 7:34 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: linux-bluetooth
In-Reply-To: <1383074811-6623-1-git-send-email-marcel@holtmann.org>
Hi Marcel,
On Tue, Oct 29, 2013, Marcel Holtmann wrote:
> The default own address type is currently set at every power on of
> a controller. This overwrites the value set via debugfs. To avoid
> this issue, set the default own address type only during controller
> setup.
>
> Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
> ---
> net/bluetooth/hci_core.c | 20 +++++++++++---------
> 1 file changed, 11 insertions(+), 9 deletions(-)
Applied to bluetooth-next. Thanks.
Johan
^ permalink raw reply
* Re: [PATCH] btusb: add realtek 8723 in the blacklist
From: Marcel Holtmann @ 2013-10-30 7:21 UTC (permalink / raw)
To: Bruce.Ma; +Cc: linux-bluetooth@vger.kernel.org development
In-Reply-To: <527090CF.9080606@canonical.com>
Hi Bruce,
> From: Bruce Ma <bruce.ma@canonical.com>
> Date: Tue, 22 Oct 2013 11:21:20 +0800
> Subject: [PATCH] btusb: add realtek 8723 in the blacklist
>
> The vendor realtek provides an off-tree driver for realtek 8723, so we
> add realtek 8723 in the in-tree driver's blacklist.
we do not care about out-of-tree drivers. Get the Realtek driver merged upstream.
> Signed-off-by: Bruce Ma <bruce.ma@canonical.com>
> ---
> drivers/bluetooth/btusb.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> index eae58a1..3097ac8 100644
> --- a/drivers/bluetooth/btusb.c
> +++ b/drivers/bluetooth/btusb.c
> @@ -138,6 +138,9 @@ static struct usb_device_id blacklist_table[] = {
> { USB_DEVICE(0x0489, 0xe03d), .driver_info = BTUSB_IGNORE },
> { USB_DEVICE(0x0489, 0xe027), .driver_info = BTUSB_IGNORE },
>
> + /* Realtek 8723 Bluetooth */
> + { USB_DEVICE(0x0bda, 0xb728), .driver_info = BTUSB_IGNORE },
> +
> /* Atheros AR9285 Malbec with sflash firmware */
> { USB_DEVICE(0x03f0, 0x311d), .driver_info = BTUSB_IGNORE },
And as an extra advice, using proper tabs vs spaces is important.
Regards
Marcel
^ permalink raw reply
* [PATCH] btusb: add realtek 8723 in the blacklist
From: Bruce.Ma @ 2013-10-30 4:53 UTC (permalink / raw)
To: linux-bluetooth
From: Bruce Ma <bruce.ma@canonical.com>
Date: Tue, 22 Oct 2013 11:21:20 +0800
Subject: [PATCH] btusb: add realtek 8723 in the blacklist
The vendor realtek provides an off-tree driver for realtek 8723, so we
add realtek 8723 in the in-tree driver's blacklist.
Signed-off-by: Bruce Ma <bruce.ma@canonical.com>
---
drivers/bluetooth/btusb.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index eae58a1..3097ac8 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -138,6 +138,9 @@ static struct usb_device_id blacklist_table[] = {
{ USB_DEVICE(0x0489, 0xe03d), .driver_info = BTUSB_IGNORE },
{ USB_DEVICE(0x0489, 0xe027), .driver_info = BTUSB_IGNORE },
+ /* Realtek 8723 Bluetooth */
+ { USB_DEVICE(0x0bda, 0xb728), .driver_info = BTUSB_IGNORE },
+
/* Atheros AR9285 Malbec with sflash firmware */
{ USB_DEVICE(0x03f0, 0x311d), .driver_info = BTUSB_IGNORE },
--
1.8.1.2
^ permalink raw reply related
* Re: Intel 7260 bluetooth malfunction when it is connected to EHCI bus
From: Hui Wang @ 2013-10-30 2:51 UTC (permalink / raw)
To: Marcel Holtmann
Cc: Tedd An, Johan Hedberg, xiong.y.zhang, Gustavo F. Padovan,
linux-bluetooth@vger.kernel.org development
In-Reply-To: <A07820C9-C272-4AD4-AE16-49F81956C2D7@holtmann.org>
On 10/29/2013 07:16 PM, Marcel Holtmann wrote:
> Hi Hui,
>
>> The problem is:
>> On the machine which has Intel 7260 BT module, i use it to connect a bluetooth headset,
>> it can successfully scan and connect to the headset, when i play sound to the bt headset,
>> the problem comes, if the bt module is connected to the XHCI, it can work very well.
>>
>> u@u-Lenovo-B4400:~$ lsusb -t
>> 1-7:1.0: No such file or directory
>> /: Bus 02.Port 1: Dev 1, Class=root_hub, Driver=xhci_hcd/4p, 5000M
>> /: Bus 01.Port 1: Dev 1, Class=root_hub, Driver=xhci_hcd/14p, 480M
>> |__ Port 6: Dev 8, If 0, Class=HID, Driver=usbhid, 1.5M
>> |__ Port 7: Dev 2, If 0, Class=vend., Driver=, 12M
>> |__ Port 11: Dev 3, If 0, Class='bInterfaceClass 0xe0 not yet handled', Driver=btusb, 12M
>> |__ Port 11: Dev 3, If 1, Class='bInterfaceClass 0xe0 not yet handled', Driver=btusb, 12M
>>
>> But if the bt module is connected to the EHCI, it always fails to play sound.
>>
>> u@u-Lenovo-B4400:~$ lsusb -t
>> /: Bus 04.Port 1: Dev 1, Class=root_hub, Driver=xhci_hcd/4p, 5000M
>> /: Bus 03.Port 1: Dev 1, Class=root_hub, Driver=xhci_hcd/14p, 480M
>> /: Bus 02.Port 1: Dev 1, Class=root_hub, Driver=ehci_hcd/3p, 480M
>> |__ Port 1: Dev 2, If 0, Class=hub, Driver=hub/8p, 480M
>> /: Bus 01.Port 1: Dev 1, Class=root_hub, Driver=ehci_hcd/3p, 480M
>> |__ Port 1: Dev 2, If 0, Class=hub, Driver=hub/6p, 480M
>> |__ Port 5: Dev 3, If 0, Class='bInterfaceClass 0xe0 not yet handled', Driver=btusb, 12M
>> |__ Port 5: Dev 3, If 1, Class='bInterfaceClass 0xe0 not yet handled', Driver=btusb, 12M
>> |__ Port 6: Dev 4, If 0, Class='bInterfaceClass 0x0e not yet handled', Driver=uvcvideo, 480M
>> |__ Port 6: Dev 4, If 1, Class='bInterfaceClass 0x0e not yet handled', Driver=uvcvideo, 480M
> can you paste /sys/kernel/debug/usb/devices here.
>
> If the uvcvideo driver is also using ISOC URBs, we might have just exhausted the bandwidth of the USB bus here or these two devices just do not play nice with each other.
>
> Try to unload the uvcvideo driver and try it again.
Thanks for your reply, follow your instruction, i totally disabled the
camera from the BIOS, then the usb tree like this,
u@u-Lenovo-B4400:~$ lsusb -t
1-1.3:1.0: No such file or directory
/: Bus 04.Port 1: Dev 1, Class=root_hub, Driver=xhci_hcd/4p, 5000M
/: Bus 03.Port 1: Dev 1, Class=root_hub, Driver=xhci_hcd/14p, 480M
/: Bus 02.Port 1: Dev 1, Class=root_hub, Driver=ehci-pci/3p, 480M
|__ Port 1: Dev 2, If 0, Class=hub, Driver=hub/8p, 480M
/: Bus 01.Port 1: Dev 1, Class=root_hub, Driver=ehci-pci/3p, 480M
|__ Port 1: Dev 2, If 0, Class=hub, Driver=hub/6p, 480M
|__ Port 2: Dev 3, If 0, Class=HID, Driver=usbhid, 1.5M
|__ Port 3: Dev 4, If 0, Class=vend., Driver=, 12M
|__ Port 5: Dev 5, If 0, Class='bInterfaceClass 0xe0 not yet
handled', Driver=btusb, 12M
|__ Port 5: Dev 5, If 1, Class='bInterfaceClass 0xe0 not yet
handled', Driver=btusb, 12M
u@u-Lenovo-B4400:~$ uname -a
Linux u-Lenovo-B4400 3.12.0-031200rc6-generic #201310191635 SMP Sat Oct
19 20:36:43 UTC 2013 x86_64 x86_64 x86_64 GNU/Linux
I re-did the test, the result is same as before, after the headset is
connected and i play audio, no sound is heard from the headset.
And hciconfig output shows still no SCO packets received by BT driver.
u@u-Lenovo-B4400:~$ hciconfig
hci0: Type: BR/EDR Bus: USB
BD Address: 00:15:00:CC:2D:D2 ACL MTU: 1021:5 SCO MTU: 96:5
UP RUNNING PSCAN
RX bytes:5853 acl:61 sco:0 events:298 errors:0
TX bytes:19212 acl:68 sco:1 commands:179 errors:0
This is the debug output of "sudo cat /sys/kernel/debug/usb/devices"
after those operations,
u@u-Lenovo-B4400:~$ sudo cat /sys/kernel/debug/usb/devices
T: Bus=04 Lev=00 Prnt=00 Port=00 Cnt=00 Dev#= 1 Spd=5000 MxCh= 4
B: Alloc= 0/800 us ( 0%), #Int= 0, #Iso= 0
D: Ver= 3.00 Cls=09(hub ) Sub=00 Prot=03 MxPS= 9 #Cfgs= 1
P: Vendor=1d6b ProdID=0003 Rev= 3.05
S: Manufacturer=Linux 3.12.0-rc6-WR5.0.1.4_standard+ xhci_hcd
S: Product=xHCI Host Controller
S: SerialNumber=0000:00:14.0
C:* #Ifs= 1 Cfg#= 1 Atr=e0 MxPwr= 0mA
I:* If#= 0 Alt= 0 #EPs= 1 Cls=09(hub ) Sub=00 Prot=00 Driver=hub
E: Ad=81(I) Atr=03(Int.) MxPS= 4 Ivl=256ms
T: Bus=03 Lev=00 Prnt=00 Port=00 Cnt=00 Dev#= 1 Spd=480 MxCh=14
B: Alloc= 0/800 us ( 0%), #Int= 0, #Iso= 0
D: Ver= 2.00 Cls=09(hub ) Sub=00 Prot=01 MxPS=64 #Cfgs= 1
P: Vendor=1d6b ProdID=0002 Rev= 3.05
S: Manufacturer=Linux 3.12.0-rc6-WR5.0.1.4_standard+ xhci_hcd
S: Product=xHCI Host Controller
S: SerialNumber=0000:00:14.0
C:* #Ifs= 1 Cfg#= 1 Atr=e0 MxPwr= 0mA
I:* If#= 0 Alt= 0 #EPs= 1 Cls=09(hub ) Sub=00 Prot=00 Driver=hub
E: Ad=81(I) Atr=03(Int.) MxPS= 4 Ivl=256ms
T: Bus=02 Lev=00 Prnt=00 Port=00 Cnt=00 Dev#= 1 Spd=480 MxCh= 3
B: Alloc= 0/800 us ( 0%), #Int= 0, #Iso= 0
D: Ver= 2.00 Cls=09(hub ) Sub=00 Prot=00 MxPS=64 #Cfgs= 1
P: Vendor=1d6b ProdID=0002 Rev= 3.05
S: Manufacturer=Linux 3.12.0-rc6-WR5.0.1.4_standard+ ehci_hcd
S: Product=EHCI Host Controller
S: SerialNumber=0000:00:1d.0
C:* #Ifs= 1 Cfg#= 1 Atr=e0 MxPwr= 0mA
I:* If#= 0 Alt= 0 #EPs= 1 Cls=09(hub ) Sub=00 Prot=00 Driver=hub
E: Ad=81(I) Atr=03(Int.) MxPS= 4 Ivl=256ms
T: Bus=02 Lev=01 Prnt=01 Port=00 Cnt=01 Dev#= 2 Spd=480 MxCh= 8
D: Ver= 2.00 Cls=09(hub ) Sub=00 Prot=01 MxPS=64 #Cfgs= 1
P: Vendor=8087 ProdID=8000 Rev= 0.05
C:* #Ifs= 1 Cfg#= 1 Atr=e0 MxPwr= 0mA
I:* If#= 0 Alt= 0 #EPs= 1 Cls=09(hub ) Sub=00 Prot=00 Driver=hub
E: Ad=81(I) Atr=03(Int.) MxPS= 2 Ivl=256ms
T: Bus=01 Lev=00 Prnt=00 Port=00 Cnt=00 Dev#= 1 Spd=480 MxCh= 3
B: Alloc= 4/800 us ( 1%), #Int= 3, #Iso= 1
D: Ver= 2.00 Cls=09(hub ) Sub=00 Prot=00 MxPS=64 #Cfgs= 1
P: Vendor=1d6b ProdID=0002 Rev= 3.05
S: Manufacturer=Linux 3.12.0-rc6-WR5.0.1.4_standard+ ehci_hcd
S: Product=EHCI Host Controller
S: SerialNumber=0000:00:1a.0
C:* #Ifs= 1 Cfg#= 1 Atr=e0 MxPwr= 0mA
I:* If#= 0 Alt= 0 #EPs= 1 Cls=09(hub ) Sub=00 Prot=00 Driver=hub
E: Ad=81(I) Atr=03(Int.) MxPS= 4 Ivl=256ms
T: Bus=01 Lev=01 Prnt=01 Port=00 Cnt=01 Dev#= 2 Spd=480 MxCh= 6
D: Ver= 2.00 Cls=09(hub ) Sub=00 Prot=01 MxPS=64 #Cfgs= 1
P: Vendor=8087 ProdID=8008 Rev= 0.05
C:* #Ifs= 1 Cfg#= 1 Atr=e0 MxPwr= 0mA
I:* If#= 0 Alt= 0 #EPs= 1 Cls=09(hub ) Sub=00 Prot=00 Driver=hub
E: Ad=81(I) Atr=03(Int.) MxPS= 1 Ivl=256ms
T: Bus=01 Lev=02 Prnt=02 Port=01 Cnt=01 Dev#= 3 Spd=1.5 MxCh= 0
D: Ver= 2.00 Cls=00(>ifc ) Sub=00 Prot=00 MxPS= 8 #Cfgs= 1
P: Vendor=0461 ProdID=4d22 Rev= 2.00
S: Product=USB Optical Mouse
C:* #Ifs= 1 Cfg#= 1 Atr=a0 MxPwr=100mA
I:* If#= 0 Alt= 0 #EPs= 1 Cls=03(HID ) Sub=01 Prot=02 Driver=usbhid
E: Ad=81(I) Atr=03(Int.) MxPS= 4 Ivl=10ms
T: Bus=01 Lev=02 Prnt=02 Port=02 Cnt=02 Dev#= 4 Spd=12 MxCh= 0
D: Ver= 1.10 Cls=ff(vend.) Sub=11 Prot=ff MxPS= 8 #Cfgs= 1
P: Vendor=138a ProdID=0011 Rev= 0.78
S: SerialNumber=7ffcdfda8afe
C:* #Ifs= 1 Cfg#= 1 Atr=a0 MxPwr=100mA
I:* If#= 0 Alt= 0 #EPs= 4 Cls=ff(vend.) Sub=00 Prot=00 Driver=(none)
E: Ad=01(O) Atr=02(Bulk) MxPS= 64 Ivl=0ms
E: Ad=81(I) Atr=02(Bulk) MxPS= 64 Ivl=0ms
E: Ad=82(I) Atr=02(Bulk) MxPS= 64 Ivl=0ms
E: Ad=83(I) Atr=03(Int.) MxPS= 8 Ivl=4ms
T: Bus=01 Lev=02 Prnt=02 Port=04 Cnt=03 Dev#= 5 Spd=12 MxCh= 0
D: Ver= 2.00 Cls=e0(wlcon) Sub=01 Prot=01 MxPS=64 #Cfgs= 1
P: Vendor=8087 ProdID=07dc Rev= 0.01
C:* #Ifs= 2 Cfg#= 1 Atr=e0 MxPwr=100mA
I:* If#= 0 Alt= 0 #EPs= 3 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
E: Ad=81(I) Atr=03(Int.) MxPS= 64 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(wlcon) 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(wlcon) 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(wlcon) 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(wlcon) 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(wlcon) 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(wlcon) 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
Thanks,
Hui.
>
> Regards
>
> Marcel
>
>
>
^ permalink raw reply
* Re: [RFC v2 05/15] Bluetooth: Make find_conn_param() helper non-local
From: Marcel Holtmann @ 2013-10-29 23:33 UTC (permalink / raw)
To: Andre Guedes; +Cc: linux-bluetooth@vger.kernel.org development
In-Reply-To: <1383053160-10175-6-git-send-email-andre.guedes@openbossa.org>
Hi Andre,
> This patch makes the find_conn_param() helper non-local by adding the
> hci_ prefix and declaring it in hci_core.h. This helper will be used
> in hci_conn.c to get the connection parameters when establishing
> connections.
>
> Since hci_find_conn_param() returns a reference to the hci_conn_param
> object, it was added a refcount to hci_conn_param to control its
> lifetime. This way, we avoid bugs such as use-after-free.
>
> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
> ---
> include/net/bluetooth/hci_core.h | 5 +++++
> net/bluetooth/hci_core.c | 45 ++++++++++++++++++++++++++++++++++------
> 2 files changed, 44 insertions(+), 6 deletions(-)
>
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index 22d16d9..64911aa 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -378,6 +378,8 @@ struct hci_chan {
> };
>
> struct hci_conn_params {
> + struct kref refcount;
> +
> struct list_head list;
>
> bdaddr_t addr;
> @@ -767,6 +769,9 @@ int hci_add_conn_params(struct hci_dev *hdev, bdaddr_t *addr, u8 addr_type,
> u16 conn_interval_max);
> void hci_remove_conn_params(struct hci_dev *hdev, bdaddr_t *addr,
> u8 addr_type);
> +struct hci_conn_params *hci_find_conn_params(struct hci_dev *hdev,
> + bdaddr_t *addr, u8 addr_type);
> +void hci_conn_params_put(struct hci_conn_params *params);
>
> int hci_uuids_clear(struct hci_dev *hdev);
>
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index fa41a58..0a278da 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -2771,8 +2771,33 @@ int hci_blacklist_del(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 type)
> return mgmt_device_unblocked(hdev, bdaddr, type);
> }
>
> -static struct hci_conn_params *find_conn_params(struct hci_dev *hdev,
> - bdaddr_t *addr, u8 addr_type)
> +static void hci_conn_params_get(struct hci_conn_params *params)
> +{
> + kref_get(¶ms->refcount);
> +}
Lets have return hci_conn_params_get the structure itself and give it the parameters we are looking for. So it is essentially the lookup function. That is how it is used anyway.
> +
> +static void release_hci_conn_params(struct kref *kref)
> +{
> + struct hci_conn_params *params = container_of(kref,
> + struct hci_conn_params,
> + refcount);
> +
> + kfree(params);
> +}
> +
> +void hci_conn_params_put(struct hci_conn_params *params)
> +{
> + kref_put(¶ms->refcount, release_hci_conn_params);
> +}
> +
> +/* Lookup hci_conn_params in hdev->conn_params list.
> + *
> + * Return a reference to hci_conn_params object with refcount incremented.
> + * The caller should drop its reference by using hci_conn_params_put(). If
> + * hci_conn_params is not found, NULL is returned.
> + */
> +struct hci_conn_params *hci_find_conn_params(struct hci_dev *hdev,
> + bdaddr_t *addr, u8 addr_type)
> {
> struct hci_conn_params *params;
>
> @@ -2784,6 +2809,8 @@ static struct hci_conn_params *find_conn_params(struct hci_dev *hdev,
> if (params->addr_type != addr_type)
> continue;
>
> + hci_conn_params_get(params);
> +
> rcu_read_unlock();
> return params;
> }
> @@ -2798,14 +2825,18 @@ int hci_add_conn_params(struct hci_dev *hdev, bdaddr_t *addr, u8 addr_type,
> {
> struct hci_conn_params *params;
>
> - params = find_conn_params(hdev, addr, addr_type);
> - if (params)
> + params = hci_find_conn_params(hdev, addr, addr_type);
> + if (params) {
> + hci_conn_params_put(params);
> return -EEXIST;
> + }
>
> params = kmalloc(sizeof(*params), GFP_KERNEL);
> if (!params)
> return -ENOMEM;
>
> + kref_init(¶ms->refcount);
> +
> bacpy(¶ms->addr, addr);
> params->addr_type = addr_type;
> params->auto_connect = auto_connect;
> @@ -2827,20 +2858,22 @@ static void __remove_conn_params(struct hci_conn_params *params)
> list_del_rcu(¶ms->list);
> synchronize_rcu();
>
> - kfree(params);
> + hci_conn_params_put(params);
> }
>
> void hci_remove_conn_params(struct hci_dev *hdev, bdaddr_t *addr, u8 addr_type)
> {
> struct hci_conn_params *params;
>
> - params = find_conn_params(hdev, addr, addr_type);
> + params = hci_find_conn_params(hdev, addr, addr_type);
> if (!params)
> return;
>
> hci_dev_lock(hdev);
> __remove_conn_params(params);
> hci_dev_unlock(hdev);
> +
> + hci_conn_params_put(params);
> }
Regards
Marcel
^ permalink raw reply
* Re: [RFC v2 09/15] Bluetooth: Introduce LE auto connection infrastructure
From: Marcel Holtmann @ 2013-10-29 23:30 UTC (permalink / raw)
To: Andre Guedes; +Cc: linux-bluetooth@vger.kernel.org development
In-Reply-To: <1383053160-10175-10-git-send-email-andre.guedes@openbossa.org>
Hi Andre,
> This patch introduces the LE auto connection infrastructure.
> This infrastructure will be used to implement the auto_connect
> options from hci_conn_params.
>
> The auto connection mechanism works as follows: When the first
> device address is inserted into hdev->pending_auto_con list, we
> start the background scanning. Once the target device is found
> in range, the kernel creates the connection. If connection is
> established successfully, the device address is removed from
> hdev->pending_auto_conn list. Finally, when the last device
> address is removed, the background scanning is stopped.
>
> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
> ---
> include/net/bluetooth/hci_core.h | 2 +
> net/bluetooth/hci_conn.c | 6 ++
> net/bluetooth/hci_core.c | 115 +++++++++++++++++++++++++++++++++++++++
> net/bluetooth/hci_event.c | 43 +++++++++++++++
> 4 files changed, 166 insertions(+)
>
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index 0049036..322918f 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -784,6 +784,8 @@ void __hci_remove_pending_auto_conn(struct hci_dev *hdev, bdaddr_t *addr,
>
> bool hci_is_scan_and_conn_supported(struct hci_dev *hdev);
>
> +void hci_check_background_scan(struct hci_dev *hdev);
> +
> int hci_uuids_clear(struct hci_dev *hdev);
>
> int hci_link_keys_clear(struct hci_dev *hdev);
> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> index b52bcb2..bbe3cab 100644
> --- a/net/bluetooth/hci_conn.c
> +++ b/net/bluetooth/hci_conn.c
> @@ -544,6 +544,12 @@ static void create_le_conn_complete(struct hci_dev *hdev, u8 status)
>
> done:
> hci_dev_unlock(hdev);
> +
> + /* Check the background scanning since it may have been temporarily
> + * stopped if the controller doesn't support scanning and creating
> + * connection at the same time.
> + */
> + hci_check_background_scan(hdev);
> }
>
> static int hci_create_le_conn(struct hci_conn *conn)
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index 19624d1..79debc3 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -2923,6 +2923,72 @@ bool hci_has_pending_auto_conn(struct hci_dev *hdev, bdaddr_t *addr,
> return res;
> }
>
> +static void start_background_scan_complete(struct hci_dev *hdev, u8 status)
> +{
> + if (status)
> + BT_DBG("HCI request failed to start background scanning: "
> + "status 0x%2.2x", status);
> +}
> +
> +static int start_background_scan(struct hci_dev *hdev)
> +{
> + struct hci_cp_le_set_scan_param param_cp;
> + struct hci_cp_le_set_scan_enable enable_cp;
> + struct hci_request req;
> +
> + if (test_bit(HCI_LE_SCAN, &hdev->dev_flags))
> + return 0;
> +
> + BT_DBG("%s", hdev->name);
> +
> + hci_req_init(&req, hdev);
> +
> + memset(¶m_cp, 0, sizeof(param_cp));
> + param_cp.type = LE_SCAN_PASSIVE;
> + param_cp.interval = cpu_to_le16(hdev->le_scan_interval);
> + param_cp.window = cpu_to_le16(hdev->le_scan_window);
> + hci_req_add(&req, HCI_OP_LE_SET_SCAN_PARAM, sizeof(param_cp),
> + ¶m_cp);
> +
> + memset(&enable_cp, 0, sizeof(enable_cp));
> + enable_cp.enable = LE_SCAN_ENABLE;
> + enable_cp.filter_dup = LE_SCAN_FILTER_DUP_DISABLE;
> + hci_req_add(&req, HCI_OP_LE_SET_SCAN_ENABLE, sizeof(enable_cp),
> + &enable_cp);
> +
> + return hci_req_run(&req, start_background_scan_complete);
> +}
> +
> +static void stop_background_scan_complete(struct hci_dev *hdev, u8 status)
> +{
> + if (status)
> + BT_DBG("HCI request failed to stop background scanning: "
> + "status 0x%2.2x", status);
> +}
> +
> +static int stop_background_scan(struct hci_dev *hdev)
> +{
> + struct hci_cp_le_set_scan_enable cp;
> + struct hci_request req;
> +
> + if (!test_bit(HCI_LE_SCAN, &hdev->dev_flags))
> + return 0;
The background scanning needs its own HCI_LE_BG_SCAN flag.
> +
> + /* If device discovery is running, background scanning is stopped so
> + * we return sucess.
> + */
> + if (hdev->discovery.state == DISCOVERY_FINDING)
> + return 0;
> +
> + hci_req_init(&req, hdev);
> +
> + memset(&cp, 0, sizeof(cp));
> + cp.enable = LE_SCAN_DISABLE;
> + hci_req_add(&req, HCI_OP_LE_SET_SCAN_ENABLE, sizeof(cp), &cp);
> +
> + return hci_req_run(&req, stop_background_scan_complete);
> +}
> +
> /* This function requires the caller holds hdev->lock */
> int __hci_add_pending_auto_conn(struct hci_dev *hdev, bdaddr_t *addr,
> u8 addr_type)
> @@ -2940,6 +3006,19 @@ int __hci_add_pending_auto_conn(struct hci_dev *hdev, bdaddr_t *addr,
> bacpy(&entry->bdaddr, addr);
> entry->bdaddr_type = addr_type;
>
> + /* If there is no pending auto connection, the background scanning
> + * is not runnning. So we should start it.
> + */
> + if (list_empty(&hdev->pending_auto_conn)) {
> + int err;
> +
> + err = start_background_scan(hdev);
> + if (err) {
> + kfree(entry);
> + return err;
> + }
> + }
> +
> list_add(&entry->list, &hdev->pending_auto_conn);
>
> return 0;
> @@ -2957,6 +3036,14 @@ void __hci_remove_pending_auto_conn(struct hci_dev *hdev, bdaddr_t *addr,
>
> list_del(&entry->list);
> kfree(entry);
> +
> + if (list_empty(&hdev->pending_auto_conn)) {
> + int err;
> +
> + err = stop_background_scan(hdev);
> + if (err)
> + BT_ERR("Failed to run HCI request: err %d", err);
I am getting sick of these errors. Return an error from a function if you plan to do something with it. Otherwise just don’t. Not to mention that the only one possible here is ENOMEM. That I declared tons and tons of functions void in bluetooth-next might have been an indication that we are not doing this anymore.
> + }
> }
And this should be all one function. Move the list_empty check into the function that updates the background scanning status. This all feels cluttered.
I mean at the end of the day it is pretty simple logic. If the list is empty stop scanning and if the list has at least one item, start scanning. Then deal with the cases where we have discovery, connection attempts etc. And done. I do not want to check 20 different locations that we call the right function with the right if around it.
>
> /* This function requires the caller holds hdev->lock */
> @@ -2970,6 +3057,34 @@ static void __clear_pending_auto_conn(struct hci_dev *hdev)
> }
> }
>
> +/* This function starts the background scanning in case there is still
> + * pending auto connections.
> + */
> +void hci_check_background_scan(struct hci_dev *hdev)
> +{
> + struct hci_conn *conn;
> + int err;
> +
> + if (list_empty(&hdev->pending_auto_conn))
> + return;
> +
> + if (test_bit(HCI_LE_SCAN, &hdev->dev_flags))
> + return;
> +
> + /* If the controller is connecting but it doesn't support scanning
> + * scanning and connecting at the same time we don't start the
> + * background scanning now. It will be started once the connection
> + * is established.
> + */
> + conn = hci_conn_hash_lookup_state(hdev, LE_LINK, BT_CONNECT);
> + if (conn && !hci_is_scan_and_conn_supported(hdev))
> + return;
> +
> + err = start_background_scan(hdev);
> + if (err)
> + BT_ERR("Failed to start background scanning: err %d", err);
The err crap has to go away. It is pretty much annoying me now.
> +}
> +
> static void inquiry_complete(struct hci_dev *hdev, u8 status)
> {
> if (status) {
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index 8b7cd37..e48601d 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -3547,8 +3547,49 @@ static void hci_le_conn_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
>
> hci_proto_connect_cfm(conn, ev->status);
>
> + __hci_remove_pending_auto_conn(hdev, &ev->bdaddr, ev->bdaddr_type);
> +
> unlock:
> hci_dev_unlock(hdev);
> +
> + /* Check the background scanning since it may have been temporarily
> + * stopped if the controller doesn't support scanning and creating
> + * connection at the same time.
> + */
> + hci_check_background_scan(hdev);
I have no idea why hci_remove_pending_auto_conn can not also call the code for updating the scanning status. Make it really simple.
> +}
> +
> +static void check_pending_auto_conn(struct hci_dev *hdev, bdaddr_t *addr,
> + u8 addr_type)
> +{
> + struct hci_conn *conn;
> + u8 bdaddr_type;
> +
> + if (!hci_has_pending_auto_conn(hdev, addr, addr_type))
> + return;
> +
> + if (addr_type == ADDR_LE_DEV_PUBLIC)
> + bdaddr_type = BDADDR_LE_PUBLIC;
> + else
> + bdaddr_type = BDADDR_LE_RANDOM;
> +
> + conn = hci_connect(hdev, LE_LINK, addr, bdaddr_type, BT_SECURITY_LOW,
> + HCI_AT_NO_BONDING);
> + if (IS_ERR(conn)) {
Just returning is !IS_ERR(conn) is better to read.
> + switch (PTR_ERR(conn)) {
> + case -EBUSY:
> + /* When hci_connect() returns EBUSY it means there is
> + * already an LE connection attempt going on. Since the
> + * controller supports only one connection attempt at
> + * the time, we simply return.
> + */
> + return;
And this can be a break then.
> + default:
> + BT_ERR("Failed to auto connect: err %ld",
> + PTR_ERR(conn));
> + return;
Same here.
> + }
> + }
> }
Also do we really care with printing an error here.
>
> static void hci_le_adv_report_evt(struct hci_dev *hdev, struct sk_buff *skb)
> @@ -3560,6 +3601,8 @@ static void hci_le_adv_report_evt(struct hci_dev *hdev, struct sk_buff *skb)
> while (num_reports--) {
> struct hci_ev_le_advertising_info *ev = ptr;
>
> + check_pending_auto_conn(hdev, &ev->bdaddr, ev->bdaddr_type);
> +
> rssi = ev->data[ev->length];
> mgmt_device_found(hdev, &ev->bdaddr, LE_LINK, ev->bdaddr_type,
> NULL, rssi, 0, 1, ev->data, ev->length);
Regards
Marcel
^ permalink raw reply
* Re: [RFC v2 10/15] Bluetooth: Temporarily stop background scanning on discovery
From: Marcel Holtmann @ 2013-10-29 23:19 UTC (permalink / raw)
To: Andre Guedes; +Cc: linux-bluetooth@vger.kernel.org development
In-Reply-To: <1383053160-10175-11-git-send-email-andre.guedes@openbossa.org>
Hi Andre,
> If the user send a mgmt start discovery command while the background
> scanning is running, we should temporarily stop it. Once the discovery
> finishes, we start the background scanning again.
>
> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
> ---
> net/bluetooth/hci_core.c | 5 +++++
> net/bluetooth/mgmt.c | 12 ++++++++----
> 2 files changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index 79debc3..44d3f99 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -1496,6 +1496,11 @@ void hci_discovery_set_state(struct hci_dev *hdev, int state)
>
> switch (state) {
> case DISCOVERY_STOPPED:
> + /* Check the background scanning since it may have been
> + * temporarily stopped by the start discovery command.
> + */
> + hci_check_background_scan(hdev);
> +
I do not know what check here means. For me this is reenable_background_scan or trigger_background_scan or start_background_scan, but not some magic check.
> if (hdev->discovery.state != DISCOVERY_STARTING)
> mgmt_discovering(hdev, 0);
> break;
> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> index 22cf547..0e329e5 100644
> --- a/net/bluetooth/mgmt.c
> +++ b/net/bluetooth/mgmt.c
> @@ -3280,11 +3280,15 @@ static int start_discovery(struct sock *sk, struct hci_dev *hdev,
> goto failed;
> }
>
> + /* If controller is scanning, it means the background scanning
> + * is running. Thus, we should temporarily stop it in order to
> + * set the discovery scanning parameters.
> + */
> if (test_bit(HCI_LE_SCAN, &hdev->dev_flags)) {
> - err = cmd_status(sk, hdev->id, MGMT_OP_START_DISCOVERY,
> - MGMT_STATUS_BUSY);
> - mgmt_pending_remove(cmd);
> - goto failed;
> + memset(&enable_cp, 0, sizeof(enable_cp));
> + enable_cp.enable = LE_SCAN_DISABLE;
> + hci_req_add(&req, HCI_OP_LE_SET_SCAN_ENABLE,
> + sizeof(enable_cp), &enable_cp);
> }
I think we need a new hdev->dev_flags for HCI_LE_BG_SCAN. Magically assuming that it is a background scan is dangerous.
Regards
Marcel
^ permalink raw reply
* Re: [RFC v2 13/15] Bluetooth: Add thread-safe version of helpers
From: Marcel Holtmann @ 2013-10-29 23:16 UTC (permalink / raw)
To: Andre Guedes; +Cc: linux-bluetooth@vger.kernel.org development
In-Reply-To: <1383053160-10175-14-git-send-email-andre.guedes@openbossa.org>
Hi Andre,
> This patch adds the thread-safe version of helper functions to add
> and remove pending auto connections. These helpers will be used in
> next patches to implementing the Mgmt add/remove connection
> parameters commands.
>
> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
> ---
> include/net/bluetooth/hci_core.h | 4 ++++
> net/bluetooth/hci_core.c | 20 ++++++++++++++++++++
> 2 files changed, 24 insertions(+)
and here is where I would stop and start with a debugfs interface. So we can start testing this. Going straight for mgmt without seeing how it all works would be a little bit too early for me.
Regards
Marcel
^ permalink raw reply
* Re: [RFC v2 12/15] Bleutooth: Add support for auto connect options
From: Marcel Holtmann @ 2013-10-29 23:15 UTC (permalink / raw)
To: Andre Guedes; +Cc: linux-bluetooth@vger.kernel.org development
In-Reply-To: <1383053160-10175-13-git-send-email-andre.guedes@openbossa.org>
Hi Andre,
> This patch adds support for the HCI_AUTO_CONN_ALWAYS and HCI_AUTO_
> CONN_LINK_LOSS options from struct hci_conn_params.
>
> The HCI_AUTO_CONN_ALWAYS option configures the kernel to always re-
> establish the connection, no matter the reason the connection was
> terminated. This feature is required by some LE profiles such as
> HID over GATT, Health Thermometer and Blood Pressure. These profiles
> require the host autonomously connect to the device as soon as it
> enters in connectable mode (start advertising) so the device is able
> to delivery a notification or indication.
>
> The BT_AUTO_CONN_LINK_LOSS option configures the kernel to re-
> establish the connection in case the connection was terminated due
> to a link loss. This feature is required by the majority of LE
> profiles such as Proximity, Find Me, Cycling Speed and Cadence and
> Time.
>
> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
> ---
> net/bluetooth/hci_event.c | 26 ++++++++++++++++++++++++++
> 1 file changed, 26 insertions(+)
>
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index e48601d..b2d8aee 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -1785,6 +1785,7 @@ static void hci_disconn_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
> struct hci_conn *conn;
> u8 type;
> bool send_mgmt_event = false;
> + struct hci_conn_params *params;
>
> BT_DBG("%s status 0x%2.2x", hdev->name, ev->status);
>
> @@ -1817,6 +1818,31 @@ static void hci_disconn_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
> if (conn->type == ACL_LINK && conn->flush_key)
> hci_remove_link_key(hdev, &conn->dst);
>
> + params = hci_find_conn_params(hdev, &conn->dst, conn->dst_type);
> + if (params) {
> + int err;
> +
> + switch (params->auto_connect) {
> + case HCI_AUTO_CONN_LINK_LOSS:
> + if (ev->reason != HCI_ERROR_CONNECTION_TIMEOUT)
> + break;
> + /* Fall through */
> +
> + case HCI_AUTO_CONN_ALWAYS:
> + err = __hci_add_pending_auto_conn(hdev, &conn->dst,
> + conn->dst_type);
> + if (err)
> + BT_ERR("Failed to add pending auto connection "
> + " %d", err);
I really wonder what we are getting from pushing the errors out here if the only thing we do is print an error message anyway. I do not see the point. Print the error where it happens and make the rest void.
> + break;
> +
> + default:
> + break;
> + }
> +
> + hci_conn_params_put(params);
> + }
> +
> type = conn->type;
> hci_proto_disconn_cfm(conn, ev->reason);
> hci_conn_del(conn);
Regards
Marcel
^ permalink raw reply
* Re: [RFC v2 11/15] Bluetooth: Auto connection and power on
From: Marcel Holtmann @ 2013-10-29 23:13 UTC (permalink / raw)
To: Andre Guedes; +Cc: linux-bluetooth@vger.kernel.org development
In-Reply-To: <1383053160-10175-12-git-send-email-andre.guedes@openbossa.org>
Hi Andre,
> This patch implements a fixup required by auto connection mechanism
> in order to work properly after a power on.
>
> When hdev is closed (e.g. Mgmt power off command, RFKILL or controller
> is reset), the ongoing active connections are silently dropped by the
> controller (no Disconnection Complete Event is sent to host). For that
> reason, the devices that require HCI_AUTO_CONN_ALWAYS are not added to
> hdev->pending_auto_conn list and they won't auto connect. So to fix
> this issue, after adapter is powered on, we should add all HCI_AUTO_
> CONN_ALWAYS address to hdev->pending_auto_conn list. Besides that, we
> always have to check if there are pending auto connections and start
> the background scanning if it is the case.
>
> This way, the auto connection mechanism works propely after a power
> off and power on sequence as well as RFKILL block/unblock.
>
> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
> ---
> include/net/bluetooth/hci_core.h | 2 ++
> net/bluetooth/hci_core.c | 49 ++++++++++++++++++++++++++++++++++++++++
> net/bluetooth/mgmt.c | 2 ++
> 3 files changed, 53 insertions(+)
>
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index 322918f..379bb36 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -786,6 +786,8 @@ bool hci_is_scan_and_conn_supported(struct hci_dev *hdev);
>
> void hci_check_background_scan(struct hci_dev *hdev);
>
> +void __hci_fixup_auto_conn(struct hci_dev *hdev);
> +
> int hci_uuids_clear(struct hci_dev *hdev);
>
> int hci_link_keys_clear(struct hci_dev *hdev);
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index 44d3f99..63a56f5 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -3090,6 +3090,55 @@ void hci_check_background_scan(struct hci_dev *hdev)
> BT_ERR("Failed to start background scanning: err %d", err);
> }
>
> +/* This functions implements a fixup required by auto connection mechanism
> + * in order to work properly after a power on.
> + *
> + * When hdev is closed (e.g. Mgmt power off command, RFKILL or controller is
> + * reset), the ongoing active connections are silently dropped by the
> + * by the controller (no Disconnection Complete Event is sent to host). For
> + * that reason, the devices that require HCI_AUTO_CONN_ALWAYS are not add to
> + * hdev->pending_auto_conn list and they won't auto connect. So to fix this
> + * issue, after adapter is powered on, we should add all HCI_AUTO_CONN_ALWAYS
> + * address to hdev->pending_auto_conn list. Besides that, we always have to
> + * check if there are pending auto connections and start the background
> + * scanning if it is the case.
> + *
> + * This function requires the caller holds hdev->lock.
> + */
> +void __hci_fixup_auto_conn(struct hci_dev *hdev)
I have no idea why you call this a fixup function. For me this is normal procedure. On power down we should clear the list of pending connections and on power up we re-establish it.
> +{
> + struct hci_conn_params *p;
> + struct bdaddr_list *entry;
> +
> + list_for_each_entry(p, &hdev->conn_params, list) {
> + if (p->auto_connect != HCI_AUTO_CONN_ALWAYS)
> + continue;
> +
> + entry = __find_pending_auto_conn(hdev, &p->addr, p->addr_type);
> + if (entry)
> + continue;
Why are we bothering here with checking if it exists. The list should be empty when we start.
> +
> + entry = kzalloc(sizeof(*entry), GFP_KERNEL);
> + if (!entry) {
> + BT_ERR("Out of memory of auto connection");
> + return;
> + }
> +
> + bacpy(&entry->bdaddr, &p->addr);
> + entry->bdaddr_type = p->addr_type;
> + list_add(&entry->list, &hdev->pending_auto_conn);
> + }
> +
> + if (!list_empty(&hdev->pending_auto_conn)) {
> + int err;
> +
> + err = start_background_scan(hdev);
> + if (err)
> + BT_ERR("Failed to start background scanning: err %d",
> + err);
Is it really worth to have the error here and not directly in start_background_scan.
> + }
> +}
> +
> static void inquiry_complete(struct hci_dev *hdev, u8 status)
> {
> if (status) {
> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> index 0e329e5..d396e47 100644
> --- a/net/bluetooth/mgmt.c
> +++ b/net/bluetooth/mgmt.c
> @@ -4255,6 +4255,8 @@ static void powered_complete(struct hci_dev *hdev, u8 status)
>
> hci_dev_lock(hdev);
>
> + __hci_fixup_auto_conn(hdev);
> +
> mgmt_pending_foreach(MGMT_OP_SET_POWERED, hdev, settings_rsp, &match);
>
> new_settings(hdev, match.sk);
Regards
Marcel
^ permalink raw reply
* Re: [RFC v2 08/15] Bluetooth: Move is_scan_and_conn_supported() to hci_core
From: Marcel Holtmann @ 2013-10-29 23:09 UTC (permalink / raw)
To: Andre Guedes; +Cc: linux-bluetooth@vger.kernel.org development
In-Reply-To: <1383053160-10175-9-git-send-email-andre.guedes@openbossa.org>
Hi Andre,
> This patch adds the "hci_" prefix and moves the is_scan_and_conn_
> supported() helper to hci_core so it can be reused in hci_core by
> the next patch.
>
> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
> ---
> include/net/bluetooth/hci_core.h | 2 ++
> net/bluetooth/hci_conn.c | 13 +------------
> net/bluetooth/hci_core.c | 11 +++++++++++
> 3 files changed, 14 insertions(+), 12 deletions(-)
>
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index e85b37e..0049036 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -782,6 +782,8 @@ int __hci_add_pending_auto_conn(struct hci_dev *hdev, bdaddr_t *addr,
> void __hci_remove_pending_auto_conn(struct hci_dev *hdev, bdaddr_t *addr,
> u8 addr_type);
>
> +bool hci_is_scan_and_conn_supported(struct hci_dev *hdev);
> +
as I said before, leave this all out until we figured out on how we handle the LE supported states. This patch now makes it clear that it is not fully thought through. And I am not moving this function around any time we need to change something.
Regards
Marcel
^ permalink raw reply
* Re: [RFC v2 07/15] Bluetooth: Introduce hdev->pending_auto_conn list
From: Marcel Holtmann @ 2013-10-29 23:08 UTC (permalink / raw)
To: Andre Guedes; +Cc: linux-bluetooth@vger.kernel.org development
In-Reply-To: <1383053160-10175-8-git-send-email-andre.guedes@openbossa.org>
Hi Andre,
> This patch introduces the hdev->pending_auto_conn list which holds
> the device addresses the kernel should autonomously connect. It also
> introduces some helper functions to manipulate the list.
>
> The list and helper functions will be used by the next patch which
> implements the LE auto connection infrastructure.
>
> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
> ---
> include/net/bluetooth/hci_core.h | 9 +++++
> net/bluetooth/hci_core.c | 84 ++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 93 insertions(+)
>
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index 64911aa..e85b37e 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -273,6 +273,8 @@ struct hci_dev {
>
> struct list_head conn_params;
>
> + struct list_head pending_auto_conn;
> +
and more empty lines. Yeah. Seriously, this needs to stop.
Also this should not be named auto_conn since as I mentioned it before, this also makes sense to be used for one-shot connections triggered by connect().
> struct hci_dev_stats stat;
>
> atomic_t promisc;
> @@ -773,6 +775,13 @@ struct hci_conn_params *hci_find_conn_params(struct hci_dev *hdev,
> bdaddr_t *addr, u8 addr_type);
> void hci_conn_params_put(struct hci_conn_params *params);
>
> +bool hci_has_pending_auto_conn(struct hci_dev *hdev, bdaddr_t *addr,
> + u8 addr_type);
> +int __hci_add_pending_auto_conn(struct hci_dev *hdev, bdaddr_t *addr,
> + u8 addr_type);
> +void __hci_remove_pending_auto_conn(struct hci_dev *hdev, bdaddr_t *addr,
> + u8 addr_type);
> +
> int hci_uuids_clear(struct hci_dev *hdev);
>
> int hci_link_keys_clear(struct hci_dev *hdev);
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index 0a278da..94c390b 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -2888,6 +2888,88 @@ static void __clear_conn_params(struct hci_dev *hdev)
> __remove_conn_params(params);
> }
>
> +static struct bdaddr_list *__find_pending_auto_conn(struct hci_dev *hdev,
> + bdaddr_t *addr,
> + u8 addr_type)
> +{
> + struct bdaddr_list *entry;
> +
> + list_for_each_entry(entry, &hdev->pending_auto_conn, list) {
> + if (bacmp(&entry->bdaddr, addr))
> + continue;
> + if (entry->bdaddr_type != addr_type)
> + continue;
Why are we playing the continue trick here. Use positive matching and just return.
> +
> + return entry;
> + }
> +
> + return NULL;
> +}
> +
> +bool hci_has_pending_auto_conn(struct hci_dev *hdev, bdaddr_t *addr,
> + u8 addr_type)
> +{
> + bool res;
> +
> + hci_dev_lock(hdev);
> +
> + if (__find_pending_auto_conn(hdev, addr, addr_type))
> + res = true;
> + else
> + res = false;
> +
> + hci_dev_unlock(hdev);
> +
> + return res;
> +}
> +
> +/* This function requires the caller holds hdev->lock */
> +int __hci_add_pending_auto_conn(struct hci_dev *hdev, bdaddr_t *addr,
> + u8 addr_type)
> +{
> + struct bdaddr_list *entry;
> +
> + entry = __find_pending_auto_conn(hdev, addr, addr_type);
> + if (entry)
> + return 0;
> +
> + entry = kzalloc(sizeof(*entry), GFP_KERNEL);
> + if (!entry)
> + return -ENOMEM;
> +
> + bacpy(&entry->bdaddr, addr);
> + entry->bdaddr_type = addr_type;
> +
> + list_add(&entry->list, &hdev->pending_auto_conn);
> +
> + return 0;
> +}
> +
> +/* This function requires the caller holds hdev->lock */
> +void __hci_remove_pending_auto_conn(struct hci_dev *hdev, bdaddr_t *addr,
> + u8 addr_type)
> +{
> + struct bdaddr_list *entry;
> +
> + entry = __find_pending_auto_conn(hdev, addr, addr_type);
> + if (!entry)
> + return;
> +
> + list_del(&entry->list);
> + kfree(entry);
> +}
> +
> +/* This function requires the caller holds hdev->lock */
> +static void __clear_pending_auto_conn(struct hci_dev *hdev)
> +{
> + struct bdaddr_list *entry, *tmp;
> +
> + list_for_each_entry_safe(entry, tmp, &hdev->pending_auto_conn, list) {
> + list_del(&entry->list);
> + kfree(entry);
> + }
> +}
> +
> static void inquiry_complete(struct hci_dev *hdev, u8 status)
> {
> if (status) {
> @@ -2999,6 +3081,7 @@ struct hci_dev *hci_alloc_dev(void)
> INIT_LIST_HEAD(&hdev->long_term_keys);
> INIT_LIST_HEAD(&hdev->remote_oob_data);
> INIT_LIST_HEAD(&hdev->conn_params);
> + INIT_LIST_HEAD(&hdev->pending_auto_conn);
> INIT_LIST_HEAD(&hdev->conn_hash.list);
>
> INIT_WORK(&hdev->rx_work, hci_rx_work);
> @@ -3185,6 +3268,7 @@ void hci_unregister_dev(struct hci_dev *hdev)
> hci_smp_ltks_clear(hdev);
> hci_remote_oob_data_clear(hdev);
> __clear_conn_params(hdev);
> + __clear_pending_auto_conn(hdev);
All this naming does not make me happier.
Regards
Marcel
^ permalink raw reply
* Re: [RFC v2 06/15] Bluetooth: Use connection parameters if any
From: Marcel Holtmann @ 2013-10-29 23:04 UTC (permalink / raw)
To: Andre Guedes; +Cc: linux-bluetooth@vger.kernel.org development
In-Reply-To: <1383053160-10175-7-git-send-email-andre.guedes@openbossa.org>
Hi Andre,
> This patch changes hci_connect_le() so it uses the connection
> parameters specified for the certain device. If no parameters
> were configured, we use the default values.
>
> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
> ---
> net/bluetooth/hci_conn.c | 13 +++++++++++--
> 1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> index 195b78f..075d070 100644
> --- a/net/bluetooth/hci_conn.c
> +++ b/net/bluetooth/hci_conn.c
> @@ -608,6 +608,7 @@ static struct hci_conn *hci_connect_le(struct hci_dev *hdev, bdaddr_t *dst,
> {
> struct hci_conn *conn;
> int err;
> + struct hci_conn_params *params;
Put it above err variable.
>
> if (test_bit(HCI_ADVERTISING, &hdev->flags))
> return ERR_PTR(-ENOTSUPP);
> @@ -652,8 +653,16 @@ static struct hci_conn *hci_connect_le(struct hci_dev *hdev, bdaddr_t *dst,
> conn->sec_level = BT_SECURITY_LOW;
> conn->pending_sec_level = sec_level;
> conn->auth_type = auth_type;
> - conn->conn_interval_min = hdev->le_conn_min_interval;
> - conn->conn_interval_max = hdev->le_conn_max_interval;
> +
> + params = hci_find_conn_params(hdev, &conn->dst, conn->dst_type);
Here is it now clear why the naming is horrible.
If you want to match with a put, then this needs to be hci_conn_params_get().
> + if (params) {
> + conn->conn_interval_min = params->conn_interval_min;
> + conn->conn_interval_max = params->conn_interval_max;
> + hci_conn_params_put(params);
> + } else {
> + conn->conn_interval_min = hdev->le_conn_min_interval;
> + conn->conn_interval_max = hdev->le_conn_max_interval;
> + }
>
> err = hci_create_le_conn(conn);
> if (err)
Regards
Marcel
^ permalink raw reply
* Re: [RFC v2 04/15] Bluetooth: Introduce connection parameters list
From: Marcel Holtmann @ 2013-10-29 23:03 UTC (permalink / raw)
To: Andre Guedes; +Cc: linux-bluetooth@vger.kernel.org development
In-Reply-To: <1383053160-10175-5-git-send-email-andre.guedes@openbossa.org>
Hi Andre,
> This patch adds to hdev the connection parameters list (hdev->
> conn_params). The elements from this list (struct hci_conn_params)
> contains the connection parameters (for now, minimum and maximum
> connection interval) that should be used during the connection
> establishment.
>
> The struct hci_conn_params also defines the 'auto_connect' field
> which will be used to implement the auto connection mechanism.
>
> Moreover, this patch adds helper functions to manipulate hdev->conn_
> params list. Some of these functions are also declared in hci_core.h
> since they will be used outside hci_core.c in upcoming patches.
>
> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
> ---
> include/net/bluetooth/hci_core.h | 24 +++++++++++
> net/bluetooth/hci_core.c | 86 ++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 110 insertions(+)
>
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index 037a7b5..22d16d9 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -271,6 +271,8 @@ struct hci_dev {
>
> struct list_head remote_oob_data;
>
> + struct list_head conn_params;
> +
these bunch of empty lines between the list_head structs is actually bugging me. They are pointless and give no visual impact. We might want to remove them as well.
I would call it le_conn_params to make sure that it is for LE.
> struct hci_dev_stats stat;
>
> atomic_t promisc;
> @@ -375,6 +377,22 @@ struct hci_chan {
> __u8 state;
> };
>
> +struct hci_conn_params {
> + struct list_head list;
> +
> + bdaddr_t addr;
> + u8 addr_type;
> +
> + enum {
> + HCI_AUTO_CONN_DISABLED,
> + HCI_AUTO_CONN_ALWAYS,
> + HCI_AUTO_CONN_LINK_LOSS,
> + } auto_connect;
> +
> + u16 conn_interval_min;
> + u16 conn_interval_max;
> +};
> +
> extern struct list_head hci_dev_list;
> extern struct list_head hci_cb_list;
> extern rwlock_t hci_dev_list_lock;
> @@ -744,6 +762,12 @@ int hci_blacklist_clear(struct hci_dev *hdev);
> int hci_blacklist_add(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 type);
> int hci_blacklist_del(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 type);
>
> +int hci_add_conn_params(struct hci_dev *hdev, bdaddr_t *addr, u8 addr_type,
> + u8 auto_connect, u16 conn_interval_min,
> + u16 conn_interval_max);
> +void hci_remove_conn_params(struct hci_dev *hdev, bdaddr_t *addr,
> + u8 addr_type);
> +
This is in violation with our other names. hci_conn_params_add might be better.
> int hci_uuids_clear(struct hci_dev *hdev);
>
> int hci_link_keys_clear(struct hci_dev *hdev);
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index 6ccc4eb..fa41a58 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -2771,6 +2771,90 @@ int hci_blacklist_del(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 type)
> return mgmt_device_unblocked(hdev, bdaddr, type);
> }
>
> +static struct hci_conn_params *find_conn_params(struct hci_dev *hdev,
> + bdaddr_t *addr, u8 addr_type)
> +{
Don’t like this find naming. It is either a lookup operation or a get.
> + struct hci_conn_params *params;
> +
> + rcu_read_lock();
> +
> + list_for_each_entry(params, &hdev->conn_params, list) {
> + if (bacmp(¶ms->addr, addr))
> + continue;
> + if (params->addr_type != addr_type)
> + continue;
> +
> + rcu_read_unlock();
> + return params;
> + }
> +
> + rcu_read_unlock();
> + return NULL;
> +}
> +
> +int hci_add_conn_params(struct hci_dev *hdev, bdaddr_t *addr, u8 addr_type,
> + u8 auto_connect, u16 conn_interval_min,
> + u16 conn_interval_max)
> +{
> + struct hci_conn_params *params;
> +
> + params = find_conn_params(hdev, addr, addr_type);
> + if (params)
> + return -EEXIST;
> +
> + params = kmalloc(sizeof(*params), GFP_KERNEL);
> + if (!params)
> + return -ENOMEM;
> +
> + bacpy(¶ms->addr, addr);
> + params->addr_type = addr_type;
> + params->auto_connect = auto_connect;
> + params->conn_interval_min = conn_interval_min;
> + params->conn_interval_max = conn_interval_max;
> +
> + hci_dev_lock(hdev);
> + list_add_rcu(¶ms->list, &hdev->conn_params);
> + hci_dev_unlock(hdev);
> + return 0;
> +}
> +
> +/* Remove from hdev->conn_params and free hci_conn_params.
> + *
> + * This function requires the caller holds hdev->lock.
> + */
> +static void __remove_conn_params(struct hci_conn_params *params)
> +{
> + list_del_rcu(¶ms->list);
> + synchronize_rcu();
> +
> + kfree(params);
> +}
> +
> +void hci_remove_conn_params(struct hci_dev *hdev, bdaddr_t *addr, u8 addr_type)
> +{
> + struct hci_conn_params *params;
> +
> + params = find_conn_params(hdev, addr, addr_type);
> + if (!params)
> + return;
> +
> + hci_dev_lock(hdev);
> + __remove_conn_params(params);
> + hci_dev_unlock(hdev);
> +}
> +
> +/* Remove all elements from hdev->conn_params list.
> + *
> + * This function requires the caller holds hdev->lock.
> + */
> +static void __clear_conn_params(struct hci_dev *hdev)
> +{
> + struct hci_conn_params *params, *tmp;
> +
> + list_for_each_entry_safe(params, tmp, &hdev->conn_params, list)
> + __remove_conn_params(params);
> +}
> +
> static void inquiry_complete(struct hci_dev *hdev, u8 status)
> {
> if (status) {
> @@ -2881,6 +2965,7 @@ struct hci_dev *hci_alloc_dev(void)
> INIT_LIST_HEAD(&hdev->link_keys);
> INIT_LIST_HEAD(&hdev->long_term_keys);
> INIT_LIST_HEAD(&hdev->remote_oob_data);
> + INIT_LIST_HEAD(&hdev->conn_params);
> INIT_LIST_HEAD(&hdev->conn_hash.list);
>
> INIT_WORK(&hdev->rx_work, hci_rx_work);
> @@ -3066,6 +3151,7 @@ void hci_unregister_dev(struct hci_dev *hdev)
> hci_link_keys_clear(hdev);
> hci_smp_ltks_clear(hdev);
> hci_remote_oob_data_clear(hdev);
> + __clear_conn_params(hdev);
Why is this one more special than the others.
> hci_dev_unlock(hdev);
>
> hci_dev_put(hdev);
Regards
Marcel
^ permalink raw reply
* Re: [RFC v2 03/15] Bluetooth: Stop scanning on connection
From: Marcel Holtmann @ 2013-10-29 22:58 UTC (permalink / raw)
To: Andre Guedes; +Cc: linux-bluetooth@vger.kernel.org development
In-Reply-To: <1383053160-10175-4-git-send-email-andre.guedes@openbossa.org>
Hi Andre,
> Some LE controllers don't support scanning and creating a connection
> at the same time. So, for those controllers, we should stop scanning
> in order to establish the connection.
>
> Since we may prematurely stop the discovery procedure in favor of
> the connection establishment, we should also cancel hdev->le_scan_
> disable delayed work and set the discovery state to DISCOVERY_STOPPED.
>
> This change does a small improvement since it is not mandatory user
> stops scanning before connecting anymore. Moreover, this change is
> required by upcoming LE auto connection mechanism in order to work
> properly with controllers that don't support background scanning and
> connection establishment at the same time.
>
> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
> ---
> net/bluetooth/hci_conn.c | 30 +++++++++++++++++++++++++++++-
> 1 file changed, 29 insertions(+), 1 deletion(-)
>
> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> index 9fb7b44..195b78f 100644
> --- a/net/bluetooth/hci_conn.c
> +++ b/net/bluetooth/hci_conn.c
> @@ -518,8 +518,11 @@ static void create_le_conn_complete(struct hci_dev *hdev, u8 status)
> {
> struct hci_conn *conn;
>
> - if (status == 0)
> + if (status == 0) {
> + cancel_delayed_work(&hdev->le_scan_disable);
> + hci_discovery_set_state(hdev, DISCOVERY_STOPPED);
> return;
> + }
>
> BT_ERR("HCI request failed to create LE connection: status 0x%2.2x",
> status);
> @@ -543,6 +546,17 @@ done:
> hci_dev_unlock(hdev);
> }
>
> +/* Check if controller supports creating a connection while scanning is
> + * runnning.
> + */
> +static bool is_scan_and_conn_supported(struct hci_dev *hdev)
> +{
> + u8 mask = BIT(6) | BIT(7);
> +
> + /* Return true if both bits are set */
> + return (hdev->le_states[2] & mask) == mask;
> +}
> +
> static int hci_create_le_conn(struct hci_conn *conn)
> {
> struct hci_dev *hdev = conn->hdev;
> @@ -552,6 +566,20 @@ static int hci_create_le_conn(struct hci_conn *conn)
>
> hci_req_init(&req, hdev);
>
> + /* If controller is scanning but it doesn't support scanning and
> + * creating a connection at the same time, we stop scanning.
> + * Otherwise, LE Create Connection command fails.
> + */
> + if (test_bit(HCI_LE_SCAN, &hdev->dev_flags) &&
> + !is_scan_and_conn_supported(hdev)) {
> + struct hci_cp_le_set_scan_enable enable_cp;
> +
> + memset(&enable_cp, 0, sizeof(enable_cp));
> + enable_cp.enable = LE_SCAN_DISABLE;
> + hci_req_add(&req, HCI_OP_LE_SET_SCAN_ENABLE, sizeof(enable_cp),
> + &enable_cp);
> + }
> +
can we just start with that we always stop scanning before we try to connect something. I would like to get the really simple case working first.
We can figure out on how to deal with the LE supported states later on. Since that has more impact on all kinds of things anyway.
Regards
Marcel
^ permalink raw reply
* Re: [RFC v2 02/15] Bluetooth: Save connection interval parameters in hci_conn
From: Marcel Holtmann @ 2013-10-29 22:55 UTC (permalink / raw)
To: Andre Guedes; +Cc: linux-bluetooth@vger.kernel.org development
In-Reply-To: <1383053160-10175-3-git-send-email-andre.guedes@openbossa.org>
Hi Andre,
> This patch creates two new fields in struct hci_conn to save the
> minimum and maximum connection interval values used to establish
> the connection this object represents.
>
> This change is required in order to know what parameters the
> connection is currently using.
>
> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
> ---
> include/net/bluetooth/hci_core.h | 3 +++
> net/bluetooth/hci_conn.c | 6 ++++--
> 2 files changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index 8c0ab3d..037a7b5 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -341,6 +341,9 @@ struct hci_conn {
>
> unsigned int sent;
>
> + __u16 conn_interval_min;
> + __u16 conn_interval_max;
> +
please move these somewhere between setting and flags. I hate the clutter in these structs.
And maybe prefix it with le_ and use le_conn_{min,max}_interval as we do in hdev.
> struct sk_buff_head data_q;
> struct list_head chan_list;
>
> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> index ba5366c..9fb7b44 100644
> --- a/net/bluetooth/hci_conn.c
> +++ b/net/bluetooth/hci_conn.c
> @@ -558,8 +558,8 @@ static int hci_create_le_conn(struct hci_conn *conn)
> bacpy(&cp.peer_addr, &conn->dst);
> cp.peer_addr_type = conn->dst_type;
> cp.own_address_type = conn->src_type;
> - cp.conn_interval_min = cpu_to_le16(hdev->le_conn_min_interval);
> - cp.conn_interval_max = cpu_to_le16(hdev->le_conn_max_interval);
> + cp.conn_interval_min = cpu_to_le16(conn->conn_interval_min);
> + cp.conn_interval_max = cpu_to_le16(conn->conn_interval_max);
> cp.supervision_timeout = __constant_cpu_to_le16(0x002a);
> cp.min_ce_len = __constant_cpu_to_le16(0x0000);
> cp.max_ce_len = __constant_cpu_to_le16(0x0000);
> @@ -624,6 +624,8 @@ static struct hci_conn *hci_connect_le(struct hci_dev *hdev, bdaddr_t *dst,
> conn->sec_level = BT_SECURITY_LOW;
> conn->pending_sec_level = sec_level;
> conn->auth_type = auth_type;
> + conn->conn_interval_min = hdev->le_conn_min_interval;
> + conn->conn_interval_max = hdev->le_conn_max_interval;
>
Regards
Marcel
^ permalink raw reply
* Re: [RFC v2 01/15] Bluetooth: Refactor hci_disconn_complete_evt
From: Marcel Holtmann @ 2013-10-29 22:52 UTC (permalink / raw)
To: Andre Guedes; +Cc: linux-bluetooth@vger.kernel.org development
In-Reply-To: <1383053160-10175-2-git-send-email-andre.guedes@openbossa.org>
Hi Andre,
> hci_disconn_complete_evt() logic is more complicated than what it
> should be, making it hard to follow and add new features.
>
> So this patch does some code refactoring by handling the error cases
> in the beginning of the function and by moving the main flow into the
> first level of function scope. No change is done in the event handling
> logic itself.
>
> Besides organizing this messy code, this patch makes easier to add
> code for handling LE auto connection (which will be added in a further
> patch).
>
> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
> ---
> net/bluetooth/hci_event.c | 62 +++++++++++++++++++++++++----------------------
> 1 file changed, 33 insertions(+), 29 deletions(-)
>
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index 5935f74..8b7cd37 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -1783,6 +1783,8 @@ static void hci_disconn_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
> {
> struct hci_ev_disconn_complete *ev = (void *) skb->data;
> struct hci_conn *conn;
> + u8 type;
> + bool send_mgmt_event = false;
>
> BT_DBG("%s status 0x%2.2x", hdev->name, ev->status);
>
> @@ -1792,44 +1794,46 @@ static void hci_disconn_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
> if (!conn)
> goto unlock;
>
> - if (ev->status == 0)
> - conn->state = BT_CLOSED;
> -
> if (test_and_clear_bit(HCI_CONN_MGMT_CONNECTED, &conn->flags) &&
> - (conn->type == ACL_LINK || conn->type == LE_LINK)) {
> - if (ev->status) {
> + (conn->type == ACL_LINK || conn->type == LE_LINK))
> + send_mgmt_event = true;
> +
> + if (ev->status) {
> + if (send_mgmt_event)
> mgmt_disconnect_failed(hdev, &conn->dst, conn->type,
> conn->dst_type, ev->status);
> - } else {
> - u8 reason = hci_to_mgmt_reason(ev->reason);
> -
> - mgmt_device_disconnected(hdev, &conn->dst, conn->type,
> - conn->dst_type, reason);
> - }
> + return;
> }
so I get the feeling that mgmt_disconnect_failed should check the link type all by itself. Since we are handing it over to the function anyway. And then this send_mgmt_event could be just removed.
> - if (ev->status == 0) {
> - u8 type = conn->type;
> + conn->state = BT_CLOSED;
>
> - if (type == ACL_LINK && conn->flush_key)
> - hci_remove_link_key(hdev, &conn->dst);
> - hci_proto_disconn_cfm(conn, ev->reason);
> - hci_conn_del(conn);
> + if (send_mgmt_event) {
> + u8 reason = hci_to_mgmt_reason(ev->reason);
>
> - /* Re-enable advertising if necessary, since it might
> - * have been disabled by the connection. From the
> - * HCI_LE_Set_Advertise_Enable command description in
> - * the core specification (v4.0):
> - * "The Controller shall continue advertising until the Host
> - * issues an LE_Set_Advertise_Enable command with
> - * Advertising_Enable set to 0x00 (Advertising is disabled)
> - * or until a connection is created or until the Advertising
> - * is timed out due to Directed Advertising."
> - */
> - if (type == LE_LINK)
> - mgmt_reenable_advertising(hdev);
> + mgmt_device_disconnected(hdev, &conn->dst, conn->type,
> + conn->dst_type, reason);
> }
Same here. Just make sure that mgmt_device_disconneted checks the link type by itself and call it unconditional.
>
> + if (conn->type == ACL_LINK && conn->flush_key)
> + hci_remove_link_key(hdev, &conn->dst);
> +
> + type = conn->type;
> + hci_proto_disconn_cfm(conn, ev->reason);
> + hci_conn_del(conn);
> +
> + /* Re-enable advertising if necessary, since it might
> + * have been disabled by the connection. From the
> + * HCI_LE_Set_Advertise_Enable command description in
> + * the core specification (v4.0):
> + * "The Controller shall continue advertising until the Host
> + * issues an LE_Set_Advertise_Enable command with
> + * Advertising_Enable set to 0x00 (Advertising is disabled)
> + * or until a connection is created or until the Advertising
> + * is timed out due to Directed Advertising."
> + */
> + if (type == LE_LINK)
> + mgmt_reenable_advertising(hdev);
> +
> unlock:
> hci_dev_unlock(hdev);
> }
Regards
Marcel
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox