2015-03-17 01:37:28

by Jakub Pawlowski

[permalink] [raw]
Subject: [PATCH v4 1/7] Bluetooth: Introduce HCI_QUIRK_SIMULTANEOUS_DISCOVERY

Some controllers allow both LE scan and BR/EDR inquiry to run at
the same time, while others allow only one, LE SCAN or BR/EDR
inquiry at given time.

Since this is specific to each controller, add a new quirk setting
that allows drivers to tell the core wether given controller can
do both LE scan and BR/EDR inquiry at same time.

Signed-off-by: Jakub Pawlowski <[email protected]>
---
include/net/bluetooth/hci.h | 9 +++++++++
1 file changed, 9 insertions(+)

diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index ce75730..7091601 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -160,6 +160,15 @@ enum {
* during the hdev->setup vendor callback.
*/
HCI_QUIRK_STRICT_DUPLICATE_FILTER,
+
+ /* When this quirk is set, LE scan and BR/EDR inquiry is done
+ * simultaneously, otherwise it's interleaved.
+ * .
+ *
+ * This quirk can be set before hci_register_dev is called or
+ * during the hdev->setup vendor callback.
+ */
+ HCI_QUIRK_SIMULTANEOUS_DISCOVERY,
};

/* HCI device flags */
--
2.2.0.rc0.207.ga3a616c



2015-03-17 02:19:34

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH v4 6/7] Bluetooth: Set HCI_QUIRK_SIMULTANEOUS_DISCOVERY for BTUSB_INTEL

Hi Jakub,

> Intel controllers can do both LE and classic scan at once.

please name it LE scanning and BR/EDR inquiry. The classic term is not really defined.

Regards

Marcel


2015-03-17 02:18:39

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH v4 1/7] Bluetooth: Introduce HCI_QUIRK_SIMULTANEOUS_DISCOVERY

Hi Jakub,

> Some controllers allow both LE scan and BR/EDR inquiry to run at
> the same time, while others allow only one, LE SCAN or BR/EDR
> inquiry at given time.
>
> Since this is specific to each controller, add a new quirk setting
> that allows drivers to tell the core wether given controller can
> do both LE scan and BR/EDR inquiry at same time.
>
> Signed-off-by: Jakub Pawlowski <[email protected]>
> ---
> include/net/bluetooth/hci.h | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> index ce75730..7091601 100644
> --- a/include/net/bluetooth/hci.h
> +++ b/include/net/bluetooth/hci.h
> @@ -160,6 +160,15 @@ enum {
> * during the hdev->setup vendor callback.
> */
> HCI_QUIRK_STRICT_DUPLICATE_FILTER,
> +
> + /* When this quirk is set, LE scan and BR/EDR inquiry is done
> + * simultaneously, otherwise it's interleaved.
> + * .

you might to remove this line with the lonely dot. Looks like a leftover.

> + *
> + * This quirk can be set before hci_register_dev is called or
> + * during the hdev->setup vendor callback.
> + */
> + HCI_QUIRK_SIMULTANEOUS_DISCOVERY,
> };

Regards

Marcel


2015-03-17 02:18:37

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH v4 3/7] Bluetooth: Split triggering le and interleaved scan into separate cases

Hi Jakub,

> This patch splits triggering of le and interleaved scan into
> separate cases.
>
> Signed-off-by: Jakub Pawlowski <[email protected]>
> ---
> net/bluetooth/mgmt.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> index 92fa8e88..9d3c7a1 100644
> --- a/net/bluetooth/mgmt.c
> +++ b/net/bluetooth/mgmt.c
> @@ -4009,9 +4009,13 @@ static bool trigger_discovery(struct hci_request *req, u8 *status)
> break;
>
> case DISCOV_TYPE_LE:
> + if (!trigger_le_scan(req, status,
> + cpu_to_le16(DISCOV_LE_SCAN_INT)))
> + return false;
> + break;
> +
> case DISCOV_TYPE_INTERLEAVED:
> - if (hdev->discovery.type == DISCOV_TYPE_INTERLEAVED &&
> - !hci_dev_test_flag(hdev, HCI_BREDR_ENABLED)) {
> + if (!hci_dev_test_flag(hdev, HCI_BREDR_ENABLED)) {
> *status = MGMT_STATUS_NOT_SUPPORTED;
> return false;
> }

see my re-ordering with a fall through statement from my previous comment. Lets split them out properly first and then the sequence on how things evolve will make more sense. And I think this patch is then no longer needed.

Regards

Marcel


2015-03-17 02:18:35

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH v4 4/7] Bluetooth: Add simultaneous dual mode scan

Hi Jakub,

> When doing scan through mgmt api, some controllers can do both le and
> classic scan at same time. They can be distinguished by
> HCI_QUIRK_SIMULTANEOUS_DISCOVERY set.
>
> This patch enables them to use this feature when doing dual mode scan.
> Instead of doing le, then classic scan, both scans are run at once.
>
> Signed-off-by: Jakub Pawlowski <[email protected]>
> ---
> net/bluetooth/hci_core.c | 24 +++++++++++++++++++-----
> net/bluetooth/hci_event.c | 18 ++++++++++++++++--
> net/bluetooth/mgmt.c | 36 ++++++++++++++++++++++++++++++------
> 3 files changed, 65 insertions(+), 13 deletions(-)
>
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index 750d344..3d51bf7 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -2902,12 +2902,26 @@ static void le_scan_disable_work_complete(struct hci_dev *hdev, u8 status,
>
> hci_dev_lock(hdev);
>
> - hci_inquiry_cache_flush(hdev);
> + if (test_bit(HCI_QUIRK_SIMULTANEOUS_DISCOVERY,
> + &hdev->quirks)) {
> + /* If we were running le only scan, change discovery

Please name it LE only scan.

> + * state. If we were running both le and classic scans

LE scan and BR/EDR inquiry.

> + * simultaneously, and classic is already finished,

BR/EDR inquiry.

> + * stop discovery, otherwise classic scan will stop

Single spaces are fine. And please also call it BR/EDR inquiry.

> + * discovery when finished.
> + */

/* When only LE scanning is running, then change discovery
* state to indicate that it completed. In case LE scanning
* and BR/EDR inquiry is active and the inquiry completed
* already, then also change the discovery state.
*
* In case only BR/EDR inquiry is running, then it will
* change the discovery state when it finished. Nothing to
* do here in that case.
*/

So I would have written it something like this to make why we are doing it this way. If I understood you correctly.

> + if (!test_bit(HCI_INQUIRY, &hdev->flags))
> + hci_discovery_set_state(hdev,
> + DISCOVERY_STOPPED);
> + } else {
> + hci_inquiry_cache_flush(hdev);
>
> - err = hci_req_run(&req, inquiry_complete);
> - if (err) {
> - BT_ERR("Inquiry request failed: err %d", err);
> - hci_discovery_set_state(hdev, DISCOVERY_STOPPED);
> + err = hci_req_run(&req, inquiry_complete);
> + if (err) {
> + BT_ERR("Inquiry request failed: err %d", err);
> + hci_discovery_set_state(hdev,
> + DISCOVERY_STOPPED);
> + }
> }
>
> hci_dev_unlock(hdev);
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index d800f0c..4a3cc58 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -2126,7 +2126,14 @@ static void hci_inquiry_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
> goto unlock;
>
> if (list_empty(&discov->resolve)) {
> - hci_discovery_set_state(hdev, DISCOVERY_STOPPED);
> + /* if we were running BR/EDR inquiry change discovery state.

> + * If we were running both BR/EDR inquiry and LE scan
> + * simultaneously, and LE is already finished, change state,
> + * otherwise LE scan will stop discovery when finished.
> + */

/* When BR/EDR inquiry is active and not LE scanning is in
* progress, then change discovery state to indicate completion.
*
* When running LE scanning and BR/EDR inquiry simultaneously
* and the LE scan already finished, then change the discovery
* state to indicate completion.
*/

> + if (!hci_dev_test_flag(hdev, HCI_LE_SCAN) ||
> + !test_bit(HCI_QUIRK_SIMULTANEOUS_DISCOVERY, &hdev->quirks))
> + hci_discovery_set_state(hdev, DISCOVERY_STOPPED);
> goto unlock;
> }
>
> @@ -2135,7 +2142,14 @@ static void hci_inquiry_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
> e->name_state = NAME_PENDING;
> hci_discovery_set_state(hdev, DISCOVERY_RESOLVING);
> } else {
> - hci_discovery_set_state(hdev, DISCOVERY_STOPPED);
> + /* if we were running classic discovery change discovery state.
> + * If we were running both le and classic scans simultaneously,
> + * and le is already finished, change state, otherwise le
> + * scan will stop discovery when finished.
> + */

I leave this as an exercise to reword it similar to the two other statements.

> + if (!hci_dev_test_flag(hdev, HCI_LE_SCAN) ||
> + !test_bit(HCI_QUIRK_SIMULTANEOUS_DISCOVERY, &hdev->quirks))
> + hci_discovery_set_state(hdev, DISCOVERY_STOPPED);
> }
>
> unlock:
> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> index 9d3c7a1..ef4db8a 100644
> --- a/net/bluetooth/mgmt.c
> +++ b/net/bluetooth/mgmt.c
> @@ -1402,7 +1402,9 @@ static bool hci_stop_discovery(struct hci_request *req)
> case DISCOVERY_FINDING:
> if (test_bit(HCI_INQUIRY, &hdev->flags)) {
> hci_req_add(req, HCI_OP_INQUIRY_CANCEL, 0, NULL);
> - } else {
> + }
> +

If this is correct, then the { } can be removed now.

> + if (hci_dev_test_flag(hdev, HCI_LE_SCAN)) {
> cancel_delayed_work(&hdev->le_scan_disable);
> hci_req_add_le_scan_disable(req);
> }
> @@ -4015,13 +4017,27 @@ static bool trigger_discovery(struct hci_request *req, u8 *status)
> break;
>
> case DISCOV_TYPE_INTERLEAVED:
> - if (!hci_dev_test_flag(hdev, HCI_BREDR_ENABLED)) {
> - *status = MGMT_STATUS_NOT_SUPPORTED;
> - return false;
> + if (!test_bit(HCI_QUIRK_SIMULTANEOUS_DISCOVERY,
> + &hdev->quirks)) {
> + if (!hci_dev_test_flag(hdev, HCI_BREDR_ENABLED)) {
> + *status = MGMT_STATUS_NOT_SUPPORTED;
> + return false;
> + }
> +
> + if (!trigger_le_scan(req, status,
> + cpu_to_le16(DISCOV_LE_SCAN_INT)))
> + return false;

Extra empty space here.

> + return true;
> }
>
> + /* During simultaneous scan, we double scan interval. We must

Lets call it discovery and not scan. It is also LE scan interval.

> + * leave some time to do BR/EDR scan.

some time for the controller to execute BR/EDR inquiry.

> + */
> if (!trigger_le_scan(req, status,
> - cpu_to_le16(DISCOV_LE_SCAN_INT)))
> + cpu_to_le16(DISCOV_LE_SCAN_INT * 2)))
> + return false;
> +
> + if (!trigger_bredr_inquiry(req, status))
> return false;
> break;
>
> @@ -4067,7 +4083,15 @@ static void start_discovery_complete(struct hci_dev *hdev, u8 status,
> timeout = msecs_to_jiffies(DISCOV_LE_TIMEOUT);
> break;
> case DISCOV_TYPE_INTERLEAVED:
> - timeout = msecs_to_jiffies(hdev->discov_interleaved_timeout);
> + /* if we're doing simultaneous discovery, LE scan should last
> + * whole time. If we do interleaved scan, LE scan last only half
> + * of that (BR/EDR inquiry takes rest of time).
> + */

/* When running simultaneous discovery, the LE scanning time
* should occupy the whole discovery time sine BR/EDR inquiry
* and LE scanning are scheduled by the controller.
*
* For interleaving discovery in comparison, BR/EDR inquiry
* and LE scanning are done sequentially with separate
* timeouts.
*/

> + if (test_bit(HCI_QUIRK_SIMULTANEOUS_DISCOVERY, &hdev->quirks))
> + timeout = msecs_to_jiffies(DISCOV_LE_TIMEOUT);
> + else
> + timeout = msecs_to_jiffies(
> + hdev->discov_interleaved_timeout);

This indentation is odd. Just go over 80 characters here.

> break;
> case DISCOV_TYPE_BREDR:
> timeout = 0;

Regards

Marcel


2015-03-17 02:18:34

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH v4 2/7] Bluetooth: Refactor BREDR inquiry and LE scan triggering.

Hi Jakub,

> This patch refactor BREDR inquiry and LE scan triggering logic into
> separate methods.

please write it as BR/EDR which makes it generally more consistent.

>
> Signed-off-by: Jakub Pawlowski <[email protected]>
> ---
> net/bluetooth/mgmt.c | 145 +++++++++++++++++++++++++++++----------------------
> 1 file changed, 82 insertions(+), 63 deletions(-)
>
> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> index 32c2c75..92fa8e88 100644
> --- a/net/bluetooth/mgmt.c
> +++ b/net/bluetooth/mgmt.c
> @@ -3913,93 +3913,112 @@ done:
> return err;
> }
>
> -static bool trigger_discovery(struct hci_request *req, u8 *status)
> +static bool trigger_bredr_inquiry(struct hci_request *req, u8 *status)
> {
> struct hci_dev *hdev = req->hdev;
> - struct hci_cp_le_set_scan_param param_cp;
> - struct hci_cp_le_set_scan_enable enable_cp;
> struct hci_cp_inquiry inq_cp;

I think you can rename this into just cp now. There is only now.

> /* General inquiry access code (GIAC) */
> u8 lap[3] = { 0x33, 0x8b, 0x9e };
> +
> + *status = mgmt_bredr_support(hdev);
> + if (*status)
> + return false;
> +
> + if (hci_dev_test_flag(hdev, HCI_INQUIRY)) {
> + *status = MGMT_STATUS_BUSY;
> + return false;
> + }
> +
> + hci_inquiry_cache_flush(hdev);
> +
> + memset(&inq_cp, 0, sizeof(inq_cp));
> + memcpy(&inq_cp.lap, lap, sizeof(inq_cp.lap));
> + inq_cp.length = DISCOV_BREDR_INQUIRY_LEN;

I think we normally have an empty line between filling the structs and then calling hci_req_add.

> + hci_req_add(req, HCI_OP_INQUIRY, sizeof(inq_cp), &inq_cp);

Extra empty line here.

> + return true;
> +}
> +
> +static bool trigger_le_scan(struct hci_request *req, u8 *status,
> + __le16 interval)
> +{

Lets make the status the last parameter. Having a return value in the middle is odd.

> + struct hci_dev *hdev = req->hdev;
> + struct hci_cp_le_set_scan_param param_cp;
> + struct hci_cp_le_set_scan_enable enable_cp;
> u8 own_addr_type;
> int err;
>
> - switch (hdev->discovery.type) {
> - case DISCOV_TYPE_BREDR:
> - *status = mgmt_bredr_support(hdev);
> - if (*status)
> - return false;
> + *status = mgmt_le_support(hdev);
> + if (*status)
> + return false;
>
> - if (test_bit(HCI_INQUIRY, &hdev->flags)) {
> - *status = MGMT_STATUS_BUSY;
> + if (hci_dev_test_flag(hdev, HCI_LE_ADV)) {
> + /* Don't let discovery abort an outgoing connection attempt
> + * that's using directed advertising.
> + */
> + if (hci_conn_hash_lookup_state(hdev, LE_LINK, BT_CONNECT)) {
> + *status = MGMT_STATUS_REJECTED;
> return false;
> }
>
> - hci_inquiry_cache_flush(hdev);
> + disable_advertising(req);
> + }
> +
> + /* 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 (hci_dev_test_flag(hdev, HCI_LE_SCAN))
> + hci_req_add_le_scan_disable(req);
>
> - memset(&inq_cp, 0, sizeof(inq_cp));
> - memcpy(&inq_cp.lap, lap, sizeof(inq_cp.lap));
> - inq_cp.length = DISCOV_BREDR_INQUIRY_LEN;
> - hci_req_add(req, HCI_OP_INQUIRY, sizeof(inq_cp), &inq_cp);
> + memset(&param_cp, 0, sizeof(param_cp));

This memset should go above the code that fills the param_cp.

> +
> + /* All active scans will be done with either a resolvable private
> + * address (when privacy feature has been enabled) or non-resolvable
> + * private address.
> + */
> + err = hci_update_random_address(req, true, &own_addr_type);
> + if (err < 0) {
> + *status = MGMT_STATUS_FAILED;
> + return false;
> + }
> +

Put the memset from above right here.

> + param_cp.type = LE_SCAN_ACTIVE;
> + param_cp.interval = interval;
> + param_cp.window = cpu_to_le16(DISCOV_LE_SCAN_WIN);
> + param_cp.own_address_type = own_addr_type;
> + hci_req_add(req, HCI_OP_LE_SET_SCAN_PARAM, sizeof(param_cp),
> + &param_cp);
> +
> + memset(&enable_cp, 0, sizeof(enable_cp));
> + enable_cp.enable = LE_SCAN_ENABLE;
> + enable_cp.filter_dup = LE_SCAN_FILTER_DUP_ENABLE;
> + hci_req_add(req, HCI_OP_LE_SET_SCAN_ENABLE, sizeof(enable_cp),
> + &enable_cp);
> +
> + return true;
> +}
> +
> +static bool trigger_discovery(struct hci_request *req, u8 *status)
> +{
> + struct hci_dev *hdev = req->hdev;
> +
> + switch (hdev->discovery.type) {
> + case DISCOV_TYPE_BREDR:
> + if (!trigger_bredr_inquiry(req, status))
> + return false;
> break;
>
> case DISCOV_TYPE_LE:
> case DISCOV_TYPE_INTERLEAVED:
> - *status = mgmt_le_support(hdev);
> - if (*status)
> - return false;
> -
> if (hdev->discovery.type == DISCOV_TYPE_INTERLEAVED &&
> !hci_dev_test_flag(hdev, HCI_BREDR_ENABLED)) {
> *status = MGMT_STATUS_NOT_SUPPORTED;
> return false;
> }
>
> - if (hci_dev_test_flag(hdev, HCI_LE_ADV)) {
> - /* Don't let discovery abort an outgoing
> - * connection attempt that's using directed
> - * advertising.
> - */
> - if (hci_conn_hash_lookup_state(hdev, LE_LINK,
> - BT_CONNECT)) {
> - *status = MGMT_STATUS_REJECTED;
> - return false;
> - }
> -
> - disable_advertising(req);
> - }
> -
> - /* 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 (hci_dev_test_flag(hdev, HCI_LE_SCAN))
> - hci_req_add_le_scan_disable(req);
> -
> - memset(&param_cp, 0, sizeof(param_cp));
> -
> - /* All active scans will be done with either a resolvable
> - * private address (when privacy feature has been enabled)
> - * or non-resolvable private address.
> - */
> - err = hci_update_random_address(req, true, &own_addr_type);
> - if (err < 0) {
> - *status = MGMT_STATUS_FAILED;
> + if (!trigger_le_scan(req, status,
> + cpu_to_le16(DISCOV_LE_SCAN_INT)))
> return false;

I had to apply the patch see the resulting code, but I prefer if we turn this actually into nice code while we are at it. I pretty much dislike entering a case block and then checking the same value again.

switch (hdev->discovery.type) {
case DISCOV_TYPE_BREDR:
if (!trigger_bredr_inquiry(req, status))
return false;
break;

case DISCOV_TYPE_INTERLEAVED:
if (!hci_dev_test_flag(hdev, HCI_BREDR_ENABLED)) {
*status = MGMT_STATUS_NOT_SUPPORTED;
return false;
}
/* fall through */

case DISCOV_TYPE_LE:
if (!trigger_le_scan(req, status,
cpu_to_le16(DISCOV_LE_SCAN_INT)))
return false;
break;

default:
*status = MGMT_STATUS_INVALID_PARAMS;
return false;
}

Regards

Marcel


2015-03-17 01:37:34

by Jakub Pawlowski

[permalink] [raw]
Subject: [PATCH v4 7/7] Bluetooth: Set HCI_QUIRK_SIMULTANEOUS_DISCOVERY for BTUSB_CSR

CSR controllers can do both LE and classic scan at once.

Signed-off-by: Jakub Pawlowski <[email protected]>
---
drivers/bluetooth/btusb.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index 879b55e..a2a7310 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -3087,6 +3087,8 @@ static int btusb_probe(struct usb_interface *intf,
/* Fake CSR devices with broken commands */
if (bcdDevice <= 0x100)
hdev->setup = btusb_setup_csr;
+
+ set_bit(HCI_QUIRK_SIMULTANEOUS_DISCOVERY, &hdev->quirks);
}

if (id->driver_info & BTUSB_SNIFFER) {
--
2.2.0.rc0.207.ga3a616c


2015-03-17 01:37:32

by Jakub Pawlowski

[permalink] [raw]
Subject: [PATCH v4 5/7] Bluetooth: Set HCI_QUIRK_SIMULTANEOUS_DISCOVERY for BTUSB_ATH3012

Atheros controllers can do both LE and classic scan at once.

Signed-off-by: Jakub Pawlowski <[email protected]>
---
drivers/bluetooth/btusb.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index 6fa9338..7c3c2a1 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -3042,6 +3042,7 @@ static int btusb_probe(struct usb_interface *intf,

if (id->driver_info & BTUSB_ATH3012) {
hdev->set_bdaddr = btusb_set_bdaddr_ath3012;
+ set_bit(HCI_QUIRK_SIMULTANEOUS_DISCOVERY, &hdev->quirks);
set_bit(HCI_QUIRK_STRICT_DUPLICATE_FILTER, &hdev->quirks);
}

--
2.2.0.rc0.207.ga3a616c


2015-03-17 01:37:33

by Jakub Pawlowski

[permalink] [raw]
Subject: [PATCH v4 6/7] Bluetooth: Set HCI_QUIRK_SIMULTANEOUS_DISCOVERY for BTUSB_INTEL

Intel controllers can do both LE and classic scan at once.

Signed-off-by: Jakub Pawlowski <[email protected]>
---
drivers/bluetooth/btusb.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index 7c3c2a1..879b55e 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -3019,6 +3019,7 @@ static int btusb_probe(struct usb_interface *intf,
hdev->shutdown = btusb_shutdown_intel;
hdev->set_bdaddr = btusb_set_bdaddr_intel;
set_bit(HCI_QUIRK_STRICT_DUPLICATE_FILTER, &hdev->quirks);
+ set_bit(HCI_QUIRK_SIMULTANEOUS_DISCOVERY, &hdev->quirks);
}

if (id->driver_info & BTUSB_INTEL_NEW) {
--
2.2.0.rc0.207.ga3a616c


2015-03-17 01:37:31

by Jakub Pawlowski

[permalink] [raw]
Subject: [PATCH v4 4/7] Bluetooth: Add simultaneous dual mode scan

When doing scan through mgmt api, some controllers can do both le and
classic scan at same time. They can be distinguished by
HCI_QUIRK_SIMULTANEOUS_DISCOVERY set.

This patch enables them to use this feature when doing dual mode scan.
Instead of doing le, then classic scan, both scans are run at once.

Signed-off-by: Jakub Pawlowski <[email protected]>
---
net/bluetooth/hci_core.c | 24 +++++++++++++++++++-----
net/bluetooth/hci_event.c | 18 ++++++++++++++++--
net/bluetooth/mgmt.c | 36 ++++++++++++++++++++++++++++++------
3 files changed, 65 insertions(+), 13 deletions(-)

diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 750d344..3d51bf7 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -2902,12 +2902,26 @@ static void le_scan_disable_work_complete(struct hci_dev *hdev, u8 status,

hci_dev_lock(hdev);

- hci_inquiry_cache_flush(hdev);
+ if (test_bit(HCI_QUIRK_SIMULTANEOUS_DISCOVERY,
+ &hdev->quirks)) {
+ /* If we were running le only scan, change discovery
+ * state. If we were running both le and classic scans
+ * simultaneously, and classic is already finished,
+ * stop discovery, otherwise classic scan will stop
+ * discovery when finished.
+ */
+ if (!test_bit(HCI_INQUIRY, &hdev->flags))
+ hci_discovery_set_state(hdev,
+ DISCOVERY_STOPPED);
+ } else {
+ hci_inquiry_cache_flush(hdev);

- err = hci_req_run(&req, inquiry_complete);
- if (err) {
- BT_ERR("Inquiry request failed: err %d", err);
- hci_discovery_set_state(hdev, DISCOVERY_STOPPED);
+ err = hci_req_run(&req, inquiry_complete);
+ if (err) {
+ BT_ERR("Inquiry request failed: err %d", err);
+ hci_discovery_set_state(hdev,
+ DISCOVERY_STOPPED);
+ }
}

hci_dev_unlock(hdev);
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index d800f0c..4a3cc58 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -2126,7 +2126,14 @@ static void hci_inquiry_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
goto unlock;

if (list_empty(&discov->resolve)) {
- hci_discovery_set_state(hdev, DISCOVERY_STOPPED);
+ /* if we were running BR/EDR inquiry change discovery state.
+ * If we were running both BR/EDR inquiry and LE scan
+ * simultaneously, and LE is already finished, change state,
+ * otherwise LE scan will stop discovery when finished.
+ */
+ if (!hci_dev_test_flag(hdev, HCI_LE_SCAN) ||
+ !test_bit(HCI_QUIRK_SIMULTANEOUS_DISCOVERY, &hdev->quirks))
+ hci_discovery_set_state(hdev, DISCOVERY_STOPPED);
goto unlock;
}

@@ -2135,7 +2142,14 @@ static void hci_inquiry_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
e->name_state = NAME_PENDING;
hci_discovery_set_state(hdev, DISCOVERY_RESOLVING);
} else {
- hci_discovery_set_state(hdev, DISCOVERY_STOPPED);
+ /* if we were running classic discovery change discovery state.
+ * If we were running both le and classic scans simultaneously,
+ * and le is already finished, change state, otherwise le
+ * scan will stop discovery when finished.
+ */
+ if (!hci_dev_test_flag(hdev, HCI_LE_SCAN) ||
+ !test_bit(HCI_QUIRK_SIMULTANEOUS_DISCOVERY, &hdev->quirks))
+ hci_discovery_set_state(hdev, DISCOVERY_STOPPED);
}

unlock:
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 9d3c7a1..ef4db8a 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -1402,7 +1402,9 @@ static bool hci_stop_discovery(struct hci_request *req)
case DISCOVERY_FINDING:
if (test_bit(HCI_INQUIRY, &hdev->flags)) {
hci_req_add(req, HCI_OP_INQUIRY_CANCEL, 0, NULL);
- } else {
+ }
+
+ if (hci_dev_test_flag(hdev, HCI_LE_SCAN)) {
cancel_delayed_work(&hdev->le_scan_disable);
hci_req_add_le_scan_disable(req);
}
@@ -4015,13 +4017,27 @@ static bool trigger_discovery(struct hci_request *req, u8 *status)
break;

case DISCOV_TYPE_INTERLEAVED:
- if (!hci_dev_test_flag(hdev, HCI_BREDR_ENABLED)) {
- *status = MGMT_STATUS_NOT_SUPPORTED;
- return false;
+ if (!test_bit(HCI_QUIRK_SIMULTANEOUS_DISCOVERY,
+ &hdev->quirks)) {
+ if (!hci_dev_test_flag(hdev, HCI_BREDR_ENABLED)) {
+ *status = MGMT_STATUS_NOT_SUPPORTED;
+ return false;
+ }
+
+ if (!trigger_le_scan(req, status,
+ cpu_to_le16(DISCOV_LE_SCAN_INT)))
+ return false;
+ return true;
}

+ /* During simultaneous scan, we double scan interval. We must
+ * leave some time to do BR/EDR scan.
+ */
if (!trigger_le_scan(req, status,
- cpu_to_le16(DISCOV_LE_SCAN_INT)))
+ cpu_to_le16(DISCOV_LE_SCAN_INT * 2)))
+ return false;
+
+ if (!trigger_bredr_inquiry(req, status))
return false;
break;

@@ -4067,7 +4083,15 @@ static void start_discovery_complete(struct hci_dev *hdev, u8 status,
timeout = msecs_to_jiffies(DISCOV_LE_TIMEOUT);
break;
case DISCOV_TYPE_INTERLEAVED:
- timeout = msecs_to_jiffies(hdev->discov_interleaved_timeout);
+ /* if we're doing simultaneous discovery, LE scan should last
+ * whole time. If we do interleaved scan, LE scan last only half
+ * of that (BR/EDR inquiry takes rest of time).
+ */
+ if (test_bit(HCI_QUIRK_SIMULTANEOUS_DISCOVERY, &hdev->quirks))
+ timeout = msecs_to_jiffies(DISCOV_LE_TIMEOUT);
+ else
+ timeout = msecs_to_jiffies(
+ hdev->discov_interleaved_timeout);
break;
case DISCOV_TYPE_BREDR:
timeout = 0;
--
2.2.0.rc0.207.ga3a616c


2015-03-17 01:37:30

by Jakub Pawlowski

[permalink] [raw]
Subject: [PATCH v4 3/7] Bluetooth: Split triggering le and interleaved scan into separate cases

This patch splits triggering of le and interleaved scan into
separate cases.

Signed-off-by: Jakub Pawlowski <[email protected]>
---
net/bluetooth/mgmt.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 92fa8e88..9d3c7a1 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -4009,9 +4009,13 @@ static bool trigger_discovery(struct hci_request *req, u8 *status)
break;

case DISCOV_TYPE_LE:
+ if (!trigger_le_scan(req, status,
+ cpu_to_le16(DISCOV_LE_SCAN_INT)))
+ return false;
+ break;
+
case DISCOV_TYPE_INTERLEAVED:
- if (hdev->discovery.type == DISCOV_TYPE_INTERLEAVED &&
- !hci_dev_test_flag(hdev, HCI_BREDR_ENABLED)) {
+ if (!hci_dev_test_flag(hdev, HCI_BREDR_ENABLED)) {
*status = MGMT_STATUS_NOT_SUPPORTED;
return false;
}
--
2.2.0.rc0.207.ga3a616c


2015-03-17 01:37:29

by Jakub Pawlowski

[permalink] [raw]
Subject: [PATCH v4 2/7] Bluetooth: Refactor BREDR inquiry and LE scan triggering.

This patch refactor BREDR inquiry and LE scan triggering logic into
separate methods.

Signed-off-by: Jakub Pawlowski <[email protected]>
---
net/bluetooth/mgmt.c | 145 +++++++++++++++++++++++++++++----------------------
1 file changed, 82 insertions(+), 63 deletions(-)

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 32c2c75..92fa8e88 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -3913,93 +3913,112 @@ done:
return err;
}

-static bool trigger_discovery(struct hci_request *req, u8 *status)
+static bool trigger_bredr_inquiry(struct hci_request *req, u8 *status)
{
struct hci_dev *hdev = req->hdev;
- struct hci_cp_le_set_scan_param param_cp;
- struct hci_cp_le_set_scan_enable enable_cp;
struct hci_cp_inquiry inq_cp;
/* General inquiry access code (GIAC) */
u8 lap[3] = { 0x33, 0x8b, 0x9e };
+
+ *status = mgmt_bredr_support(hdev);
+ if (*status)
+ return false;
+
+ if (hci_dev_test_flag(hdev, HCI_INQUIRY)) {
+ *status = MGMT_STATUS_BUSY;
+ return false;
+ }
+
+ hci_inquiry_cache_flush(hdev);
+
+ memset(&inq_cp, 0, sizeof(inq_cp));
+ memcpy(&inq_cp.lap, lap, sizeof(inq_cp.lap));
+ inq_cp.length = DISCOV_BREDR_INQUIRY_LEN;
+ hci_req_add(req, HCI_OP_INQUIRY, sizeof(inq_cp), &inq_cp);
+ return true;
+}
+
+static bool trigger_le_scan(struct hci_request *req, u8 *status,
+ __le16 interval)
+{
+ struct hci_dev *hdev = req->hdev;
+ struct hci_cp_le_set_scan_param param_cp;
+ struct hci_cp_le_set_scan_enable enable_cp;
u8 own_addr_type;
int err;

- switch (hdev->discovery.type) {
- case DISCOV_TYPE_BREDR:
- *status = mgmt_bredr_support(hdev);
- if (*status)
- return false;
+ *status = mgmt_le_support(hdev);
+ if (*status)
+ return false;

- if (test_bit(HCI_INQUIRY, &hdev->flags)) {
- *status = MGMT_STATUS_BUSY;
+ if (hci_dev_test_flag(hdev, HCI_LE_ADV)) {
+ /* Don't let discovery abort an outgoing connection attempt
+ * that's using directed advertising.
+ */
+ if (hci_conn_hash_lookup_state(hdev, LE_LINK, BT_CONNECT)) {
+ *status = MGMT_STATUS_REJECTED;
return false;
}

- hci_inquiry_cache_flush(hdev);
+ disable_advertising(req);
+ }
+
+ /* 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 (hci_dev_test_flag(hdev, HCI_LE_SCAN))
+ hci_req_add_le_scan_disable(req);

- memset(&inq_cp, 0, sizeof(inq_cp));
- memcpy(&inq_cp.lap, lap, sizeof(inq_cp.lap));
- inq_cp.length = DISCOV_BREDR_INQUIRY_LEN;
- hci_req_add(req, HCI_OP_INQUIRY, sizeof(inq_cp), &inq_cp);
+ memset(&param_cp, 0, sizeof(param_cp));
+
+ /* All active scans will be done with either a resolvable private
+ * address (when privacy feature has been enabled) or non-resolvable
+ * private address.
+ */
+ err = hci_update_random_address(req, true, &own_addr_type);
+ if (err < 0) {
+ *status = MGMT_STATUS_FAILED;
+ return false;
+ }
+
+ param_cp.type = LE_SCAN_ACTIVE;
+ param_cp.interval = interval;
+ param_cp.window = cpu_to_le16(DISCOV_LE_SCAN_WIN);
+ param_cp.own_address_type = own_addr_type;
+ hci_req_add(req, HCI_OP_LE_SET_SCAN_PARAM, sizeof(param_cp),
+ &param_cp);
+
+ memset(&enable_cp, 0, sizeof(enable_cp));
+ enable_cp.enable = LE_SCAN_ENABLE;
+ enable_cp.filter_dup = LE_SCAN_FILTER_DUP_ENABLE;
+ hci_req_add(req, HCI_OP_LE_SET_SCAN_ENABLE, sizeof(enable_cp),
+ &enable_cp);
+
+ return true;
+}
+
+static bool trigger_discovery(struct hci_request *req, u8 *status)
+{
+ struct hci_dev *hdev = req->hdev;
+
+ switch (hdev->discovery.type) {
+ case DISCOV_TYPE_BREDR:
+ if (!trigger_bredr_inquiry(req, status))
+ return false;
break;

case DISCOV_TYPE_LE:
case DISCOV_TYPE_INTERLEAVED:
- *status = mgmt_le_support(hdev);
- if (*status)
- return false;
-
if (hdev->discovery.type == DISCOV_TYPE_INTERLEAVED &&
!hci_dev_test_flag(hdev, HCI_BREDR_ENABLED)) {
*status = MGMT_STATUS_NOT_SUPPORTED;
return false;
}

- if (hci_dev_test_flag(hdev, HCI_LE_ADV)) {
- /* Don't let discovery abort an outgoing
- * connection attempt that's using directed
- * advertising.
- */
- if (hci_conn_hash_lookup_state(hdev, LE_LINK,
- BT_CONNECT)) {
- *status = MGMT_STATUS_REJECTED;
- return false;
- }
-
- disable_advertising(req);
- }
-
- /* 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 (hci_dev_test_flag(hdev, HCI_LE_SCAN))
- hci_req_add_le_scan_disable(req);
-
- memset(&param_cp, 0, sizeof(param_cp));
-
- /* All active scans will be done with either a resolvable
- * private address (when privacy feature has been enabled)
- * or non-resolvable private address.
- */
- err = hci_update_random_address(req, true, &own_addr_type);
- if (err < 0) {
- *status = MGMT_STATUS_FAILED;
+ if (!trigger_le_scan(req, status,
+ cpu_to_le16(DISCOV_LE_SCAN_INT)))
return false;
- }
-
- param_cp.type = LE_SCAN_ACTIVE;
- param_cp.interval = cpu_to_le16(DISCOV_LE_SCAN_INT);
- param_cp.window = cpu_to_le16(DISCOV_LE_SCAN_WIN);
- param_cp.own_address_type = own_addr_type;
- hci_req_add(req, HCI_OP_LE_SET_SCAN_PARAM, sizeof(param_cp),
- &param_cp);
-
- memset(&enable_cp, 0, sizeof(enable_cp));
- enable_cp.enable = LE_SCAN_ENABLE;
- enable_cp.filter_dup = LE_SCAN_FILTER_DUP_ENABLE;
- hci_req_add(req, HCI_OP_LE_SET_SCAN_ENABLE, sizeof(enable_cp),
- &enable_cp);
break;

default:
--
2.2.0.rc0.207.ga3a616c