[PATCH v2 00/13] Update Discovery to use HCI request framework

[PATCH v2 00/13] Update Discovery to use HCI request framework

[Andre Guedes]

Hi all,

This v2 basically implements Johan's comments from the previous patch set. The
main changes are:
* A new patch was added to fix start_discovery for LE-only controllers.
* The hci_ prefix was added to inquiry_cache_flush.
* Interleaved discovery implementation was changed. Instead of building a
  request with HCI_OP_LE_SET_SCAN_ENABLE and HCI_OP_INQUIRY commands, we send
  the inquiry command in disable LE scanning complete callback.

Below follows the summary of the patches (from the previous cover letter with
some adaptations):

* Patch 1 fixes start_discovery for LE-only controllers. This patch can be
  applied even if the rest of this patch set is not. Otherwise, start_discovery
  always fails for LE-only controllers.

* Patch 2 makes inquiry_cache_flush function non-static since it will be used
  in mgmt.c by the upcoming patch.

* Patch 3 updates start_discovery function to use HCI request to send HCI
  inquiry/LE scanning commands to the controller. It also defines the start_
  discovery_complete function where all command complete events are handled.

* Patch 4 simply removes Start Discovery command complete handling from hci_
  event.c since it is now handled by start_discovery_complete function in
  mgmt.c.

* Patch 5 moves discovery macros from mgmt.c to a common place (hci_core.h)
  because some of these macros will be used in hci_core.c in the upcoming
  patch.

* Patch 6 updates the interleaved discovery to use HCI requests. First we send
  a request to disable the LE scanning. Then, if it completes successfully,
  we send another request to trigger the inquiry procedure.

* Patch 7 updates stop_discovery function to use HCI request. This patch does
  a similar job as Patch 3, but for Stop Discovery command instead.

* Patch 8 simply removes Stop Discovery command complete handling from hci_
  event.c. This patch does a similar job as Patch 4, but for Stop Discovery
  command instead.

* Patch 9 does a very trivial refactoring in hci_cc_le_set_scan_enable
  function.

* Patches 10 and 11 removes LE scanning helpers and inquiry helpers since they
  are no longer used.

* Patch 12 removes the hci_cc_le_set_scan_param function since it became an
  empty event handler.

* Patch 13 is not really related to this patch set, but it is related to the
  whole Device Discovery functionality. As discussed on IRC with Marcel, we
  don't want the kernel sending Mgmt Device Found events to user-space when we
  are not running the Device Discovery. Therefore, Inquiry or LE scanning
  triggered by HCI raw interface (e.g. hcitool) won't generate Mgmt Device
  Found events anymore.

BR,

Andre

Andre Guedes (13):
  Bluetooth: Fix start_discovery for LE-only controllers
  Bluetooth: Make inquiry_cache_flush non-static
  Bluetooth: Update start_discovery to use HCI request
  Bluetooth: Remove start discovery handling from hci_event.c
  Bluetooth: Move discovery macros to hci_core.h
  Bluetooth: Use HCI request in interleaved discovery
  Bluetooth: Update stop_discovery to use HCI request
  Bluetooth: Remove stop discovery handling from hci_event.c
  Bluetooth: Refactor hci_cc_le_set_scan_enable
  Bluetooth: Remove LE scan helpers
  Bluetooth: Remove inquiry helpers
  Bluetooth: Remove empty event handler
  Bluetooth: Mgmt Device Found Event

 include/net/bluetooth/hci_core.h |  27 +++---
 net/bluetooth/hci_core.c         | 186 +++++++++++----------------------------
 net/bluetooth/hci_event.c        |  67 +-------------
 net/bluetooth/mgmt.c             | 157 +++++++++++++++++++++++++--------
 4 files changed, 188 insertions(+), 249 deletions(-)

-- 
1.8.2.1

[PATCH v2 01/13] Bluetooth: Fix start_discovery for LE-only controllers

[Andre Guedes]

LE-only controllers don't have local extended features (see commit
c73eee9172bf061202c75ad4d16506a7efd04d1c). Thus, lmp_host_le_capable
helper is not applicable and we should use test_bit(HCI_LE_ENABLED)
to check if the host supports LE. Otherwise, start_discovery will
always fail for LE-only controllers.

This misbehavior can be easily checked by running mgmt-tester tool
(test "Start Discovery - Success 2" will fail).

Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
---
 net/bluetooth/mgmt.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 35fef22..9ea3783 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -2700,7 +2700,7 @@ static int start_discovery(struct sock *sk, struct hci_dev *hdev,
 		break;
 
 	case DISCOV_TYPE_LE:
-		if (!lmp_host_le_capable(hdev)) {
+		if (!test_bit(HCI_LE_ENABLED, &hdev->dev_flags)) {
 			err = cmd_status(sk, hdev->id, MGMT_OP_START_DISCOVERY,
 					 MGMT_STATUS_NOT_SUPPORTED);
 			mgmt_pending_remove(cmd);
@@ -2712,7 +2712,8 @@ static int start_discovery(struct sock *sk, struct hci_dev *hdev,
 		break;
 
 	case DISCOV_TYPE_INTERLEAVED:
-		if (!lmp_host_le_capable(hdev) || !lmp_bredr_capable(hdev)) {
+		if (!test_bit(HCI_LE_ENABLED, &hdev->dev_flags) ||
+		    !lmp_bredr_capable(hdev)) {
 			err = cmd_status(sk, hdev->id, MGMT_OP_START_DISCOVERY,
 					 MGMT_STATUS_NOT_SUPPORTED);
 			mgmt_pending_remove(cmd);
-- 
1.8.2.1

[PATCH v2 02/13] Bluetooth: Make inquiry_cache_flush non-static

[Andre Guedes]

In order to use HCI request framework in start_discovery, we'll need
to call inquiry_cache_flush in mgmt.c. Therefore, this patch adds the
hci_ prefix to inquiry_cache_flush and makes it non-static.

Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
---
 include/net/bluetooth/hci_core.h |  1 +
 net/bluetooth/hci_core.c         | 10 +++++-----
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 35a57cd..1ff7f67 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -432,6 +432,7 @@ void hci_inquiry_cache_update_resolve(struct hci_dev *hdev,
 				      struct inquiry_entry *ie);
 bool hci_inquiry_cache_update(struct hci_dev *hdev, struct inquiry_data *data,
 			      bool name_known, bool *ssp);
+void hci_inquiry_cache_flush(struct hci_dev *hdev);
 
 /* ----- HCI Connections ----- */
 enum {
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 33843c5..8aefd17 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -746,7 +746,7 @@ void hci_discovery_set_state(struct hci_dev *hdev, int state)
 	hdev->discovery.state = state;
 }
 
-static void inquiry_cache_flush(struct hci_dev *hdev)
+void hci_inquiry_cache_flush(struct hci_dev *hdev)
 {
 	struct discovery_state *cache = &hdev->discovery;
 	struct inquiry_entry *p, *n;
@@ -959,7 +959,7 @@ int hci_inquiry(void __user *arg)
 	hci_dev_lock(hdev);
 	if (inquiry_cache_age(hdev) > INQUIRY_CACHE_AGE_MAX ||
 	    inquiry_cache_empty(hdev) || ir.flags & IREQ_CACHE_FLUSH) {
-		inquiry_cache_flush(hdev);
+		hci_inquiry_cache_flush(hdev);
 		do_inquiry = 1;
 	}
 	hci_dev_unlock(hdev);
@@ -1225,7 +1225,7 @@ static int hci_dev_do_close(struct hci_dev *hdev)
 	cancel_delayed_work_sync(&hdev->le_scan_disable);
 
 	hci_dev_lock(hdev);
-	inquiry_cache_flush(hdev);
+	hci_inquiry_cache_flush(hdev);
 	hci_conn_hash_flush(hdev);
 	hci_dev_unlock(hdev);
 
@@ -1326,7 +1326,7 @@ int hci_dev_reset(__u16 dev)
 	skb_queue_purge(&hdev->cmd_q);
 
 	hci_dev_lock(hdev);
-	inquiry_cache_flush(hdev);
+	hci_inquiry_cache_flush(hdev);
 	hci_conn_hash_flush(hdev);
 	hci_dev_unlock(hdev);
 
@@ -3553,7 +3553,7 @@ int hci_do_inquiry(struct hci_dev *hdev, u8 length)
 	if (test_bit(HCI_INQUIRY, &hdev->flags))
 		return -EINPROGRESS;
 
-	inquiry_cache_flush(hdev);
+	hci_inquiry_cache_flush(hdev);
 
 	memset(&cp, 0, sizeof(cp));
 	memcpy(&cp.lap, lap, sizeof(cp.lap));
-- 
1.8.2.1

[PATCH v2 03/13] Bluetooth: Update start_discovery to use HCI request

[Andre Guedes]

This patch modifies the start_discovery function so it uses the HCI
request framework.

We build the HCI request according to the discovery type (add inquiry
or LE scan HCI commands) and run the HCI request. We also register
the start_discovery_complete callback which handles mgmt command
complete events for this command. This way, we move all start_
discovery mgmt handling code spread in hci_event.c to a single place
in mgmt.c.

This patch also merges the LE-only and interleaved discovery type
cases since these cases are pretty much the same now.

Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
---
 net/bluetooth/mgmt.c | 93 +++++++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 84 insertions(+), 9 deletions(-)

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 9ea3783..c701eac 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -2650,11 +2650,51 @@ int mgmt_interleaved_discovery(struct hci_dev *hdev)
 	return err;
 }
 
+static void start_discovery_complete(struct hci_dev *hdev, u8 status)
+{
+	BT_DBG("status %d", status);
+
+	if (status) {
+		hci_dev_lock(hdev);
+		mgmt_start_discovery_failed(hdev, status);
+		hci_dev_unlock(hdev);
+		return;
+	}
+
+	hci_dev_lock(hdev);
+	hci_discovery_set_state(hdev, DISCOVERY_FINDING);
+	hci_dev_unlock(hdev);
+
+	switch (hdev->discovery.type) {
+	case DISCOV_TYPE_LE:
+		queue_delayed_work(hdev->workqueue, &hdev->le_scan_disable,
+				   LE_SCAN_TIMEOUT_LE_ONLY);
+		break;
+
+	case DISCOV_TYPE_INTERLEAVED:
+		queue_delayed_work(hdev->workqueue, &hdev->le_scan_disable,
+				   LE_SCAN_TIMEOUT_BREDR_LE);
+		break;
+
+	case DISCOV_TYPE_BREDR:
+		break;
+
+	default:
+		BT_ERR("Invalid discovery type %d", hdev->discovery.type);
+	}
+}
+
 static int start_discovery(struct sock *sk, struct hci_dev *hdev,
 			   void *data, u16 len)
 {
 	struct mgmt_cp_start_discovery *cp = data;
 	struct pending_cmd *cmd;
+	struct hci_cp_le_set_scan_param param_cp;
+	struct hci_cp_le_set_scan_enable enable_cp;
+	struct hci_cp_inquiry inq_cp;
+	struct hci_request req;
+	/* General inquiry access code (GIAC) */
+	u8 lap[3] = { 0x33, 0x8b, 0x9e };
 	int err;
 
 	BT_DBG("%s", hdev->name);
@@ -2687,6 +2727,8 @@ static int start_discovery(struct sock *sk, struct hci_dev *hdev,
 
 	hdev->discovery.type = cp->type;
 
+	hci_req_init(&req, hdev);
+
 	switch (hdev->discovery.type) {
 	case DISCOV_TYPE_BREDR:
 		if (!lmp_bredr_capable(hdev)) {
@@ -2696,10 +2738,23 @@ static int start_discovery(struct sock *sk, struct hci_dev *hdev,
 			goto failed;
 		}
 
-		err = hci_do_inquiry(hdev, INQUIRY_LEN_BREDR);
+		if (test_bit(HCI_INQUIRY, &hdev->flags)) {
+			err = cmd_status(sk, hdev->id, MGMT_OP_START_DISCOVERY,
+					 MGMT_STATUS_BUSY);
+			mgmt_pending_remove(cmd);
+			goto failed;
+		}
+
+		hci_inquiry_cache_flush(hdev);
+
+		memset(&inq_cp, 0, sizeof(inq_cp));
+		memcpy(&inq_cp.lap, lap, sizeof(inq_cp.lap));
+		inq_cp.length = INQUIRY_LEN_BREDR;
+		hci_req_add(&req, HCI_OP_INQUIRY, sizeof(inq_cp), &inq_cp);
 		break;
 
 	case DISCOV_TYPE_LE:
+	case DISCOV_TYPE_INTERLEAVED:
 		if (!test_bit(HCI_LE_ENABLED, &hdev->dev_flags)) {
 			err = cmd_status(sk, hdev->id, MGMT_OP_START_DISCOVERY,
 					 MGMT_STATUS_NOT_SUPPORTED);
@@ -2707,12 +2762,7 @@ static int start_discovery(struct sock *sk, struct hci_dev *hdev,
 			goto failed;
 		}
 
-		err = hci_le_scan(hdev, LE_SCAN_ACTIVE, LE_SCAN_INT,
-				  LE_SCAN_WIN, LE_SCAN_TIMEOUT_LE_ONLY);
-		break;
-
-	case DISCOV_TYPE_INTERLEAVED:
-		if (!test_bit(HCI_LE_ENABLED, &hdev->dev_flags) ||
+		if (hdev->discovery.type == DISCOV_TYPE_INTERLEAVED &&
 		    !lmp_bredr_capable(hdev)) {
 			err = cmd_status(sk, hdev->id, MGMT_OP_START_DISCOVERY,
 					 MGMT_STATUS_NOT_SUPPORTED);
@@ -2720,8 +2770,32 @@ static int start_discovery(struct sock *sk, struct hci_dev *hdev,
 			goto failed;
 		}
 
-		err = hci_le_scan(hdev, LE_SCAN_ACTIVE, LE_SCAN_INT,
-				  LE_SCAN_WIN, LE_SCAN_TIMEOUT_BREDR_LE);
+		if (test_bit(HCI_LE_PERIPHERAL, &hdev->dev_flags)) {
+			err = cmd_status(sk, hdev->id, MGMT_OP_START_DISCOVERY,
+					 MGMT_STATUS_REJECTED);
+			mgmt_pending_remove(cmd);
+			goto failed;
+		}
+
+		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(&param_cp, 0, sizeof(param_cp));
+		param_cp.type = LE_SCAN_ACTIVE;
+		param_cp.interval = cpu_to_le16(LE_SCAN_INT);
+		param_cp.window = cpu_to_le16(LE_SCAN_WIN);
+		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:
@@ -2731,6 +2805,7 @@ static int start_discovery(struct sock *sk, struct hci_dev *hdev,
 		goto failed;
 	}
 
+	err = hci_req_run(&req, start_discovery_complete);
 	if (err < 0)
 		mgmt_pending_remove(cmd);
 	else
-- 
1.8.2.1

[PATCH v2 04/13] Bluetooth: Remove start discovery handling from hci_event.c

[Andre Guedes]

Since all mgmt start discovery command complete events are now handled
in start_discovery_complete callback in mgmt.c, we can remove this
handling from hci_event.c.

Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
---
 net/bluetooth/hci_event.c | 24 +-----------------------
 1 file changed, 1 insertion(+), 23 deletions(-)

diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index b93cd2e..0e71e6c 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -943,12 +943,6 @@ static void hci_cc_le_set_scan_param(struct hci_dev *hdev, struct sk_buff *skb)
 
 	BT_DBG("%s status 0x%2.2x", hdev->name, status);
 
-	if (status) {
-		hci_dev_lock(hdev);
-		mgmt_start_discovery_failed(hdev, status);
-		hci_dev_unlock(hdev);
-		return;
-	}
 }
 
 static void hci_cc_le_set_scan_enable(struct hci_dev *hdev,
@@ -965,18 +959,10 @@ static void hci_cc_le_set_scan_enable(struct hci_dev *hdev,
 
 	switch (cp->enable) {
 	case LE_SCAN_ENABLE:
-		if (status) {
-			hci_dev_lock(hdev);
-			mgmt_start_discovery_failed(hdev, status);
-			hci_dev_unlock(hdev);
+		if (status)
 			return;
-		}
 
 		set_bit(HCI_LE_SCAN, &hdev->dev_flags);
-
-		hci_dev_lock(hdev);
-		hci_discovery_set_state(hdev, DISCOVERY_FINDING);
-		hci_dev_unlock(hdev);
 		break;
 
 	case LE_SCAN_DISABLE:
@@ -1077,18 +1063,10 @@ static void hci_cs_inquiry(struct hci_dev *hdev, __u8 status)
 
 	if (status) {
 		hci_conn_check_pending(hdev);
-		hci_dev_lock(hdev);
-		if (test_bit(HCI_MGMT, &hdev->dev_flags))
-			mgmt_start_discovery_failed(hdev, status);
-		hci_dev_unlock(hdev);
 		return;
 	}
 
 	set_bit(HCI_INQUIRY, &hdev->flags);
-
-	hci_dev_lock(hdev);
-	hci_discovery_set_state(hdev, DISCOVERY_FINDING);
-	hci_dev_unlock(hdev);
 }
 
 static void hci_cs_create_conn(struct hci_dev *hdev, __u8 status)
-- 
1.8.2.1

[PATCH v2 05/13] Bluetooth: Move discovery macros to hci_core.h

[Andre Guedes]

Some of discovery macros will be used in hci_core so we need to
define them in common place such as hci_core.h. Thus, this patch
moves discovery macros to hci_core.h and also adds the DISCOV_
prefix to them.

Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
---
 include/net/bluetooth/hci_core.h | 10 ++++++++++
 net/bluetooth/mgmt.c             | 24 ++++++------------------
 2 files changed, 16 insertions(+), 18 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 1ff7f67..fbfa5cd 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -1115,6 +1115,16 @@ void hci_sock_dev_event(struct hci_dev *hdev, int event);
 					 BIT(BDADDR_LE_PUBLIC) | \
 					 BIT(BDADDR_LE_RANDOM))
 
+/* These LE scan and inquiry parameters were chosen according to LE General
+ * Discovery Procedure specification.
+ */
+#define DISCOV_LE_SCAN_WIN		0x12
+#define DISCOV_LE_SCAN_INT		0x12
+#define DISCOV_LE_TIMEOUT		msecs_to_jiffies(10240)
+#define DISCOV_INTERLEAVED_TIMEOUT	msecs_to_jiffies(5120)
+#define DISCOV_INTERLEAVED_INQUIRY_LEN	0x04
+#define DISCOV_BREDR_INQUIRY_LEN	0x08
+
 int mgmt_control(struct sock *sk, struct msghdr *msg, size_t len);
 int mgmt_index_added(struct hci_dev *hdev);
 int mgmt_index_removed(struct hci_dev *hdev);
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index c701eac..7673398 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -102,18 +102,6 @@ static const u16 mgmt_events[] = {
 	MGMT_EV_PASSKEY_NOTIFY,
 };
 
-/*
- * These LE scan and inquiry parameters were chosen according to LE General
- * Discovery Procedure specification.
- */
-#define LE_SCAN_WIN			0x12
-#define LE_SCAN_INT			0x12
-#define LE_SCAN_TIMEOUT_LE_ONLY		msecs_to_jiffies(10240)
-#define LE_SCAN_TIMEOUT_BREDR_LE	msecs_to_jiffies(5120)
-
-#define INQUIRY_LEN_BREDR		0x08	/* TGAP(100) */
-#define INQUIRY_LEN_BREDR_LE		0x04	/* TGAP(100)/2 */
-
 #define CACHE_TIMEOUT	msecs_to_jiffies(2 * 1000)
 
 #define hdev_is_powered(hdev) (test_bit(HCI_UP, &hdev->flags) && \
@@ -2641,7 +2629,7 @@ int mgmt_interleaved_discovery(struct hci_dev *hdev)
 
 	hci_dev_lock(hdev);
 
-	err = hci_do_inquiry(hdev, INQUIRY_LEN_BREDR_LE);
+	err = hci_do_inquiry(hdev, DISCOV_INTERLEAVED_INQUIRY_LEN);
 	if (err < 0)
 		hci_discovery_set_state(hdev, DISCOVERY_STOPPED);
 
@@ -2668,12 +2656,12 @@ static void start_discovery_complete(struct hci_dev *hdev, u8 status)
 	switch (hdev->discovery.type) {
 	case DISCOV_TYPE_LE:
 		queue_delayed_work(hdev->workqueue, &hdev->le_scan_disable,
-				   LE_SCAN_TIMEOUT_LE_ONLY);
+				   DISCOV_LE_TIMEOUT);
 		break;
 
 	case DISCOV_TYPE_INTERLEAVED:
 		queue_delayed_work(hdev->workqueue, &hdev->le_scan_disable,
-				   LE_SCAN_TIMEOUT_BREDR_LE);
+				   DISCOV_INTERLEAVED_TIMEOUT);
 		break;
 
 	case DISCOV_TYPE_BREDR:
@@ -2749,7 +2737,7 @@ static int start_discovery(struct sock *sk, struct hci_dev *hdev,
 
 		memset(&inq_cp, 0, sizeof(inq_cp));
 		memcpy(&inq_cp.lap, lap, sizeof(inq_cp.lap));
-		inq_cp.length = INQUIRY_LEN_BREDR;
+		inq_cp.length = DISCOV_BREDR_INQUIRY_LEN;
 		hci_req_add(&req, HCI_OP_INQUIRY, sizeof(inq_cp), &inq_cp);
 		break;
 
@@ -2786,8 +2774,8 @@ static int start_discovery(struct sock *sk, struct hci_dev *hdev,
 
 		memset(&param_cp, 0, sizeof(param_cp));
 		param_cp.type = LE_SCAN_ACTIVE;
-		param_cp.interval = cpu_to_le16(LE_SCAN_INT);
-		param_cp.window = cpu_to_le16(LE_SCAN_WIN);
+		param_cp.interval = cpu_to_le16(DISCOV_LE_SCAN_INT);
+		param_cp.window = cpu_to_le16(DISCOV_LE_SCAN_WIN);
 		hci_req_add(&req, HCI_OP_LE_SET_SCAN_PARAM, sizeof(param_cp),
 			    &param_cp);
 
-- 
1.8.2.1

[PATCH v2 06/13] Bluetooth: Use HCI request in interleaved discovery

[Andre Guedes]

In order to have a better HCI error handling in interleaved discovery
functionality, we should use the HCI request framework.

This patch updates le_scan_disable_work function so it uses the
HCI request framework instead of the hci_send_cmd helper. A complete
callback is registered (le_scan_disable_work_complete function) so we
are able to trigger the inquiry procedure (if we are running the
interleaved discovery) or to stop the discovery procedure (if we are
running LE-only discovery).

This patch also removes the extra logic in hci_cc_le_set_scan_enable
to trigger the inquiry procedure and the mgmt_interleaved_discovery
function since they become useless.

Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
---
 include/net/bluetooth/hci_core.h |  1 -
 net/bluetooth/hci_core.c         | 65 +++++++++++++++++++++++++++++++++++++++-
 net/bluetooth/hci_event.c        | 10 -------
 net/bluetooth/mgmt.c             | 17 -----------
 4 files changed, 64 insertions(+), 29 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index fbfa5cd..99dded9 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -1182,7 +1182,6 @@ int mgmt_remote_name(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 link_type,
 int mgmt_start_discovery_failed(struct hci_dev *hdev, u8 status);
 int mgmt_stop_discovery_failed(struct hci_dev *hdev, u8 status);
 int mgmt_discovering(struct hci_dev *hdev, u8 discovering);
-int mgmt_interleaved_discovery(struct hci_dev *hdev);
 int mgmt_device_blocked(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 type);
 int mgmt_device_unblocked(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 type);
 bool mgmt_valid_hdev(struct hci_dev *hdev);
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 8aefd17..d2b96bd 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -2058,17 +2058,80 @@ int hci_cancel_le_scan(struct hci_dev *hdev)
 	return 0;
 }
 
+static void inquiry_complete(struct hci_dev *hdev, u8 status)
+{
+	if (status) {
+		BT_ERR("Failed to start inquiry: status %d", status);
+
+		hci_dev_lock(hdev);
+		hci_discovery_set_state(hdev, DISCOVERY_STOPPED);
+		hci_dev_unlock(hdev);
+		return;
+	}
+}
+
+static void le_scan_disable_work_complete(struct hci_dev *hdev, u8 status)
+{
+	/* General inquiry access code (GIAC) */
+	u8 lap[3] = { 0x33, 0x8b, 0x9e };
+	struct hci_request req;
+	struct hci_cp_inquiry cp;
+	int err;
+
+	if (status) {
+		BT_ERR("Failed to disable LE scanning: status %d", status);
+		return;
+	}
+
+	switch (hdev->discovery.type) {
+	case DISCOV_TYPE_LE:
+		hci_dev_lock(hdev);
+		hci_discovery_set_state(hdev, DISCOVERY_STOPPED);
+		hci_dev_unlock(hdev);
+		break;
+
+	case DISCOV_TYPE_INTERLEAVED:
+		hci_dev_lock(hdev);
+		hci_inquiry_cache_flush(hdev);
+		hci_dev_unlock(hdev);
+
+		hci_req_init(&req, hdev);
+
+		memset(&cp, 0, sizeof(cp));
+		memcpy(&cp.lap, lap, sizeof(cp.lap));
+		cp.length = DISCOV_INTERLEAVED_INQUIRY_LEN;
+		hci_req_add(&req, HCI_OP_INQUIRY, sizeof(cp), &cp);
+
+		err = hci_req_run(&req, inquiry_complete);
+		if (err) {
+			BT_ERR("Inquiry request failed: err %d", err);
+			hci_dev_lock(hdev);
+			hci_discovery_set_state(hdev, DISCOVERY_STOPPED);
+			hci_dev_unlock(hdev);
+		}
+		break;
+	}
+}
+
 static void le_scan_disable_work(struct work_struct *work)
 {
 	struct hci_dev *hdev = container_of(work, struct hci_dev,
 					    le_scan_disable.work);
 	struct hci_cp_le_set_scan_enable cp;
+	struct hci_request req;
+	int err;
 
 	BT_DBG("%s", hdev->name);
 
+	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);
 
-	hci_send_cmd(hdev, HCI_OP_LE_SET_SCAN_ENABLE, sizeof(cp), &cp);
+	err = hci_req_run(&req, le_scan_disable_work_complete);
+	if (err)
+		BT_ERR("Disable LE scanning request failed: err %d", err);
 }
 
 static void le_scan_work(struct work_struct *work)
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 0e71e6c..faaf1f3 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -974,16 +974,6 @@ static void hci_cc_le_set_scan_enable(struct hci_dev *hdev,
 		}
 
 		clear_bit(HCI_LE_SCAN, &hdev->dev_flags);
-
-		if (hdev->discovery.type == DISCOV_TYPE_INTERLEAVED &&
-		    hdev->discovery.state == DISCOVERY_FINDING) {
-			mgmt_interleaved_discovery(hdev);
-		} else {
-			hci_dev_lock(hdev);
-			hci_discovery_set_state(hdev, DISCOVERY_STOPPED);
-			hci_dev_unlock(hdev);
-		}
-
 		break;
 
 	default:
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 7673398..7201c05 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -2621,23 +2621,6 @@ static int remove_remote_oob_data(struct sock *sk, struct hci_dev *hdev,
 	return err;
 }
 
-int mgmt_interleaved_discovery(struct hci_dev *hdev)
-{
-	int err;
-
-	BT_DBG("%s", hdev->name);
-
-	hci_dev_lock(hdev);
-
-	err = hci_do_inquiry(hdev, DISCOV_INTERLEAVED_INQUIRY_LEN);
-	if (err < 0)
-		hci_discovery_set_state(hdev, DISCOVERY_STOPPED);
-
-	hci_dev_unlock(hdev);
-
-	return err;
-}
-
 static void start_discovery_complete(struct hci_dev *hdev, u8 status)
 {
 	BT_DBG("status %d", status);
-- 
1.8.2.1

[PATCH v2 07/13] Bluetooth: Update stop_discovery to use HCI request

[Andre Guedes]

This patch modifies the stop_discovery function so it uses the HCI
request framework.

The HCI request is built according to the current discovery state
(inquiry, LE scanning or name resolving) and a complete callback is
register to handle the command complete event for the stop discovery
command. This way, we move all stop_discovery mgmt handling code
spread in hci_event.c to a single place in mgmt.c.

Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
---
 net/bluetooth/mgmt.c | 47 ++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 40 insertions(+), 7 deletions(-)

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 7201c05..d2c9881 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -2787,6 +2787,23 @@ failed:
 	return err;
 }
 
+static void stop_discovery_complete(struct hci_dev *hdev, u8 status)
+{
+	BT_DBG("status %d", status);
+
+	hci_dev_lock(hdev);
+
+	if (status) {
+		mgmt_stop_discovery_failed(hdev, status);
+		goto unlock;
+	}
+
+	hci_discovery_set_state(hdev, DISCOVERY_STOPPED);
+
+unlock:
+	hci_dev_unlock(hdev);
+}
+
 static int stop_discovery(struct sock *sk, struct hci_dev *hdev, void *data,
 			  u16 len)
 {
@@ -2794,6 +2811,8 @@ static int stop_discovery(struct sock *sk, struct hci_dev *hdev, void *data,
 	struct pending_cmd *cmd;
 	struct hci_cp_remote_name_req_cancel cp;
 	struct inquiry_entry *e;
+	struct hci_request req;
+	struct hci_cp_le_set_scan_enable enable_cp;
 	int err;
 
 	BT_DBG("%s", hdev->name);
@@ -2820,12 +2839,20 @@ static int stop_discovery(struct sock *sk, struct hci_dev *hdev, void *data,
 		goto unlock;
 	}
 
+	hci_req_init(&req, hdev);
+
 	switch (hdev->discovery.state) {
 	case DISCOVERY_FINDING:
-		if (test_bit(HCI_INQUIRY, &hdev->flags))
-			err = hci_cancel_inquiry(hdev);
-		else
-			err = hci_cancel_le_scan(hdev);
+		if (test_bit(HCI_INQUIRY, &hdev->flags)) {
+			hci_req_add(&req, HCI_OP_INQUIRY_CANCEL, 0, NULL);
+		} else {
+			cancel_delayed_work(&hdev->le_scan_disable);
+
+			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);
+		}
 
 		break;
 
@@ -2843,16 +2870,22 @@ static int stop_discovery(struct sock *sk, struct hci_dev *hdev, void *data,
 		}
 
 		bacpy(&cp.bdaddr, &e->data.bdaddr);
-		err = hci_send_cmd(hdev, HCI_OP_REMOTE_NAME_REQ_CANCEL,
-				   sizeof(cp), &cp);
+		hci_req_add(&req, HCI_OP_REMOTE_NAME_REQ_CANCEL, sizeof(cp),
+			    &cp);
 
 		break;
 
 	default:
 		BT_DBG("unknown discovery state %u", hdev->discovery.state);
-		err = -EFAULT;
+
+		mgmt_pending_remove(cmd);
+		err = cmd_complete(sk, hdev->id, MGMT_OP_STOP_DISCOVERY,
+				   MGMT_STATUS_FAILED, &mgmt_cp->type,
+				   sizeof(mgmt_cp->type));
+		goto unlock;
 	}
 
+	err = hci_req_run(&req, stop_discovery_complete);
 	if (err < 0)
 		mgmt_pending_remove(cmd);
 	else
-- 
1.8.2.1

[PATCH v2 08/13] Bluetooth: Remove stop discovery handling from hci_event.c

[Andre Guedes]

Since all mgmt stop discovery command complete events are now handled
in stop_discovery_complete callback in mgmt.c, we can remove this
handling from hci_event.c.

Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
---
 net/bluetooth/hci_event.c | 16 ++--------------
 1 file changed, 2 insertions(+), 14 deletions(-)

diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index faaf1f3..27f66dc 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -40,21 +40,13 @@ static void hci_cc_inquiry_cancel(struct hci_dev *hdev, struct sk_buff *skb)
 
 	BT_DBG("%s status 0x%2.2x", hdev->name, status);
 
-	if (status) {
-		hci_dev_lock(hdev);
-		mgmt_stop_discovery_failed(hdev, status);
-		hci_dev_unlock(hdev);
+	if (status)
 		return;
-	}
 
 	clear_bit(HCI_INQUIRY, &hdev->flags);
 	smp_mb__after_clear_bit(); /* wake_up_bit advises about this barrier */
 	wake_up_bit(&hdev->flags, HCI_INQUIRY);
 
-	hci_dev_lock(hdev);
-	hci_discovery_set_state(hdev, DISCOVERY_STOPPED);
-	hci_dev_unlock(hdev);
-
 	hci_conn_check_pending(hdev);
 }
 
@@ -966,12 +958,8 @@ static void hci_cc_le_set_scan_enable(struct hci_dev *hdev,
 		break;
 
 	case LE_SCAN_DISABLE:
-		if (status) {
-			hci_dev_lock(hdev);
-			mgmt_stop_discovery_failed(hdev, status);
-			hci_dev_unlock(hdev);
+		if (status)
 			return;
-		}
 
 		clear_bit(HCI_LE_SCAN, &hdev->dev_flags);
 		break;
-- 
1.8.2.1

[PATCH v2 09/13] Bluetooth: Refactor hci_cc_le_set_scan_enable

[Andre Guedes]

This patch does a trivial refactoring in hci_cc_le_set_scan_enable.
Since start and stop discovery command failures are now handled in
mgmt layer, the status check became empty. So, we can move it to
outside the switch statement.

Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
---
 net/bluetooth/hci_event.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 27f66dc..76ff1af 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -949,18 +949,15 @@ static void hci_cc_le_set_scan_enable(struct hci_dev *hdev,
 	if (!cp)
 		return;
 
+	if (status)
+		return;
+
 	switch (cp->enable) {
 	case LE_SCAN_ENABLE:
-		if (status)
-			return;
-
 		set_bit(HCI_LE_SCAN, &hdev->dev_flags);
 		break;
 
 	case LE_SCAN_DISABLE:
-		if (status)
-			return;
-
 		clear_bit(HCI_LE_SCAN, &hdev->dev_flags);
 		break;
 
-- 
1.8.2.1

[PATCH v2 10/13] Bluetooth: Remove LE scan helpers

[Andre Guedes]

This patch removes the LE scan helpers hci_le_scan and hci_cancel_
le_scan and all code related to it. We now use the HCI request
framework in device discovery functionality and these helpers are
no longer needed.

Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
---
 include/net/bluetooth/hci_core.h |  13 -----
 net/bluetooth/hci_core.c         | 113 ---------------------------------------
 2 files changed, 126 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 99dded9..7cc8857 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -117,13 +117,6 @@ struct oob_data {
 	u8 randomizer[16];
 };
 
-struct le_scan_params {
-	u8 type;
-	u16 interval;
-	u16 window;
-	int timeout;
-};
-
 #define HCI_MAX_SHORT_NAME_LENGTH	10
 
 struct amp_assoc {
@@ -283,9 +276,6 @@ struct hci_dev {
 
 	struct delayed_work	le_scan_disable;
 
-	struct work_struct	le_scan;
-	struct le_scan_params	le_scan_params;
-
 	__s8			adv_tx_power;
 	__u8			adv_data[HCI_MAX_AD_LENGTH];
 	__u8			adv_data_len;
@@ -1223,9 +1213,6 @@ void hci_le_start_enc(struct hci_conn *conn, __le16 ediv, __u8 rand[8],
 							__u8 ltk[16]);
 int hci_do_inquiry(struct hci_dev *hdev, u8 length);
 int hci_cancel_inquiry(struct hci_dev *hdev);
-int hci_le_scan(struct hci_dev *hdev, u8 type, u16 interval, u16 window,
-		int timeout);
-int hci_cancel_le_scan(struct hci_dev *hdev);
 
 u8 bdaddr_to_le(u8 bdaddr_type);
 
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index d2b96bd..dcfe639 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -1196,8 +1196,6 @@ static int hci_dev_do_close(struct hci_dev *hdev)
 {
 	BT_DBG("%s %p", hdev->name, hdev);
 
-	cancel_work_sync(&hdev->le_scan);
-
 	cancel_delayed_work(&hdev->power_off);
 
 	hci_req_cancel(hdev, ENODEV);
@@ -1982,82 +1980,6 @@ int hci_blacklist_del(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 type)
 	return mgmt_device_unblocked(hdev, bdaddr, type);
 }
 
-static void le_scan_param_req(struct hci_request *req, unsigned long opt)
-{
-	struct le_scan_params *param =  (struct le_scan_params *) opt;
-	struct hci_cp_le_set_scan_param cp;
-
-	memset(&cp, 0, sizeof(cp));
-	cp.type = param->type;
-	cp.interval = cpu_to_le16(param->interval);
-	cp.window = cpu_to_le16(param->window);
-
-	hci_req_add(req, HCI_OP_LE_SET_SCAN_PARAM, sizeof(cp), &cp);
-}
-
-static void le_scan_enable_req(struct hci_request *req, unsigned long opt)
-{
-	struct hci_cp_le_set_scan_enable cp;
-
-	memset(&cp, 0, sizeof(cp));
-	cp.enable = LE_SCAN_ENABLE;
-	cp.filter_dup = LE_SCAN_FILTER_DUP_ENABLE;
-
-	hci_req_add(req, HCI_OP_LE_SET_SCAN_ENABLE, sizeof(cp), &cp);
-}
-
-static int hci_do_le_scan(struct hci_dev *hdev, u8 type, u16 interval,
-			  u16 window, int timeout)
-{
-	long timeo = msecs_to_jiffies(3000);
-	struct le_scan_params param;
-	int err;
-
-	BT_DBG("%s", hdev->name);
-
-	if (test_bit(HCI_LE_SCAN, &hdev->dev_flags))
-		return -EINPROGRESS;
-
-	param.type = type;
-	param.interval = interval;
-	param.window = window;
-
-	hci_req_lock(hdev);
-
-	err = __hci_req_sync(hdev, le_scan_param_req, (unsigned long) &param,
-			     timeo);
-	if (!err)
-		err = __hci_req_sync(hdev, le_scan_enable_req, 0, timeo);
-
-	hci_req_unlock(hdev);
-
-	if (err < 0)
-		return err;
-
-	queue_delayed_work(hdev->workqueue, &hdev->le_scan_disable,
-			   timeout);
-
-	return 0;
-}
-
-int hci_cancel_le_scan(struct hci_dev *hdev)
-{
-	BT_DBG("%s", hdev->name);
-
-	if (!test_bit(HCI_LE_SCAN, &hdev->dev_flags))
-		return -EALREADY;
-
-	if (cancel_delayed_work(&hdev->le_scan_disable)) {
-		struct hci_cp_le_set_scan_enable cp;
-
-		/* Send HCI command to disable LE Scan */
-		memset(&cp, 0, sizeof(cp));
-		hci_send_cmd(hdev, HCI_OP_LE_SET_SCAN_ENABLE, sizeof(cp), &cp);
-	}
-
-	return 0;
-}
-
 static void inquiry_complete(struct hci_dev *hdev, u8 status)
 {
 	if (status) {
@@ -2134,40 +2056,6 @@ static void le_scan_disable_work(struct work_struct *work)
 		BT_ERR("Disable LE scanning request failed: err %d", err);
 }
 
-static void le_scan_work(struct work_struct *work)
-{
-	struct hci_dev *hdev = container_of(work, struct hci_dev, le_scan);
-	struct le_scan_params *param = &hdev->le_scan_params;
-
-	BT_DBG("%s", hdev->name);
-
-	hci_do_le_scan(hdev, param->type, param->interval, param->window,
-		       param->timeout);
-}
-
-int hci_le_scan(struct hci_dev *hdev, u8 type, u16 interval, u16 window,
-		int timeout)
-{
-	struct le_scan_params *param = &hdev->le_scan_params;
-
-	BT_DBG("%s", hdev->name);
-
-	if (test_bit(HCI_LE_PERIPHERAL, &hdev->dev_flags))
-		return -ENOTSUPP;
-
-	if (work_busy(&hdev->le_scan))
-		return -EINPROGRESS;
-
-	param->type = type;
-	param->interval = interval;
-	param->window = window;
-	param->timeout = timeout;
-
-	queue_work(system_long_wq, &hdev->le_scan);
-
-	return 0;
-}
-
 /* Alloc HCI device */
 struct hci_dev *hci_alloc_dev(void)
 {
@@ -2202,7 +2090,6 @@ struct hci_dev *hci_alloc_dev(void)
 	INIT_WORK(&hdev->cmd_work, hci_cmd_work);
 	INIT_WORK(&hdev->tx_work, hci_tx_work);
 	INIT_WORK(&hdev->power_on, hci_power_on);
-	INIT_WORK(&hdev->le_scan, le_scan_work);
 
 	INIT_DELAYED_WORK(&hdev->power_off, hci_power_off);
 	INIT_DELAYED_WORK(&hdev->discov_off, hci_discov_off);
-- 
1.8.2.1

[PATCH v2 11/13] Bluetooth: Remove inquiry helpers

[Andre Guedes]

This patch removes hci_do_inquiry and hci_cancel_inquiry helpers. We
now use the HCI request framework in device discovery functionality
and these helpers are no longer needed.

Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
---
 include/net/bluetooth/hci_core.h |  2 --
 net/bluetooth/hci_core.c         | 30 ------------------------------
 2 files changed, 32 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 7cc8857..632d7d08a 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -1211,8 +1211,6 @@ void hci_le_conn_update(struct hci_conn *conn, u16 min, u16 max,
 					u16 latency, u16 to_multiplier);
 void hci_le_start_enc(struct hci_conn *conn, __le16 ediv, __u8 rand[8],
 							__u8 ltk[16]);
-int hci_do_inquiry(struct hci_dev *hdev, u8 length);
-int hci_cancel_inquiry(struct hci_dev *hdev);
 
 u8 bdaddr_to_le(u8 bdaddr_type);
 
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index dcfe639..ff032e3 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -3492,36 +3492,6 @@ static void hci_cmd_work(struct work_struct *work)
 	}
 }
 
-int hci_do_inquiry(struct hci_dev *hdev, u8 length)
-{
-	/* General inquiry access code (GIAC) */
-	u8 lap[3] = { 0x33, 0x8b, 0x9e };
-	struct hci_cp_inquiry cp;
-
-	BT_DBG("%s", hdev->name);
-
-	if (test_bit(HCI_INQUIRY, &hdev->flags))
-		return -EINPROGRESS;
-
-	hci_inquiry_cache_flush(hdev);
-
-	memset(&cp, 0, sizeof(cp));
-	memcpy(&cp.lap, lap, sizeof(cp.lap));
-	cp.length  = length;
-
-	return hci_send_cmd(hdev, HCI_OP_INQUIRY, sizeof(cp), &cp);
-}
-
-int hci_cancel_inquiry(struct hci_dev *hdev)
-{
-	BT_DBG("%s", hdev->name);
-
-	if (!test_bit(HCI_INQUIRY, &hdev->flags))
-		return -EALREADY;
-
-	return hci_send_cmd(hdev, HCI_OP_INQUIRY_CANCEL, 0, NULL);
-}
-
 u8 bdaddr_to_le(u8 bdaddr_type)
 {
 	switch (bdaddr_type) {
-- 
1.8.2.1

[PATCH v2 12/13] Bluetooth: Remove empty event handler

[Andre Guedes]

This patch removes the hci_cc_le_set_scan_param event handler. This
handler became empty because failures of this event are now handled
by start_discovery_complete function in mgmt.c.

Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
---
 net/bluetooth/hci_event.c | 12 ------------
 1 file changed, 12 deletions(-)

diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 76ff1af..db58e72 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -929,14 +929,6 @@ static void hci_cc_le_set_adv_enable(struct hci_dev *hdev, struct sk_buff *skb)
 	hci_dev_unlock(hdev);
 }
 
-static void hci_cc_le_set_scan_param(struct hci_dev *hdev, struct sk_buff *skb)
-{
-	__u8 status = *((__u8 *) skb->data);
-
-	BT_DBG("%s status 0x%2.2x", hdev->name, status);
-
-}
-
 static void hci_cc_le_set_scan_enable(struct hci_dev *hdev,
 				      struct sk_buff *skb)
 {
@@ -2251,10 +2243,6 @@ static void hci_cmd_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
 		hci_cc_user_passkey_neg_reply(hdev, skb);
 		break;
 
-	case HCI_OP_LE_SET_SCAN_PARAM:
-		hci_cc_le_set_scan_param(hdev, skb);
-		break;
-
 	case HCI_OP_LE_SET_ADV_ENABLE:
 		hci_cc_le_set_adv_enable(hdev, skb);
 		break;
-- 
1.8.2.1

[PATCH v2 13/13] Bluetooth: Mgmt Device Found Event

[Andre Guedes]

We only want to send Mgmt Device Found Events if we are running the
Device Discovery procedure (started by the MGMT Start Discovery
Command). Inquiry or LE scanning triggered by HCI raw interface (e.g.
hcitool) or kernel internals should not send Mgmt Device Found Events.

Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
---
 net/bluetooth/mgmt.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index d2c9881..9239535 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -4122,6 +4122,9 @@ int mgmt_device_found(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 link_type,
 	struct mgmt_ev_device_found *ev = (void *) buf;
 	size_t ev_size;
 
+	if (!hci_discovery_active(hdev))
+		return -EPERM;
+
 	/* Leave 5 bytes for a potential CoD field */
 	if (sizeof(*ev) + eir_len + 5 > sizeof(buf))
 		return -EINVAL;
-- 
1.8.2.1

Re: [PATCH v2 01/13] Bluetooth: Fix start_discovery for LE-only controllers

[Johan Hedberg]

Hi Andre,

On Tue, Apr 30, 2013, Andre Guedes wrote:
> LE-only controllers don't have local extended features (see commit
> c73eee9172bf061202c75ad4d16506a7efd04d1c). Thus, lmp_host_le_capable
> helper is not applicable and we should use test_bit(HCI_LE_ENABLED)
> to check if the host supports LE. Otherwise, start_discovery will
> always fail for LE-only controllers.
> 
> This misbehavior can be easily checked by running mgmt-tester tool
> (test "Start Discovery - Success 2" will fail).
> 
> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
> ---
>  net/bluetooth/mgmt.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)

I already sent a patch for this last Wednesday:

  Bluetooth: Fix checks for LE support on LE-only controllers

So this patch can be ignored.

Johan

Re: [PATCH v2 02/13] Bluetooth: Make inquiry_cache_flush non-static

[Johan Hedberg]

Hi Andre,

On Tue, Apr 30, 2013, Andre Guedes wrote:
> In order to use HCI request framework in start_discovery, we'll need
> to call inquiry_cache_flush in mgmt.c. Therefore, this patch adds the
> hci_ prefix to inquiry_cache_flush and makes it non-static.
> 
> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
> ---
>  include/net/bluetooth/hci_core.h |  1 +
>  net/bluetooth/hci_core.c         | 10 +++++-----
>  2 files changed, 6 insertions(+), 5 deletions(-)

Acked-by: Johan Hedberg <johan.hedberg@intel.com>

Johan

Re: [PATCH v2 03/13] Bluetooth: Update start_discovery to use HCI request

[Johan Hedberg]

Hi Andre,

On Tue, Apr 30, 2013, Andre Guedes wrote:
> This patch modifies the start_discovery function so it uses the HCI
> request framework.
> 
> We build the HCI request according to the discovery type (add inquiry
> or LE scan HCI commands) and run the HCI request. We also register
> the start_discovery_complete callback which handles mgmt command
> complete events for this command. This way, we move all start_
> discovery mgmt handling code spread in hci_event.c to a single place
> in mgmt.c.
> 
> This patch also merges the LE-only and interleaved discovery type
> cases since these cases are pretty much the same now.
> 
> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
> ---
>  net/bluetooth/mgmt.c | 93 +++++++++++++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 84 insertions(+), 9 deletions(-)

Acked-by: Johan Hedberg <johan.hedberg@intel.com>

Johan

Re: [PATCH v2 04/13] Bluetooth: Remove start discovery handling from hci_event.c

[Johan Hedberg]

Hi Andre,

On Tue, Apr 30, 2013, Andre Guedes wrote:
> Since all mgmt start discovery command complete events are now handled
> in start_discovery_complete callback in mgmt.c, we can remove this
> handling from hci_event.c.
> 
> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
> ---
>  net/bluetooth/hci_event.c | 24 +-----------------------
>  1 file changed, 1 insertion(+), 23 deletions(-)

Acked-by: Johan Hedberg <johan.hedberg@intel.com>

However, it looks like after this patch you could have another one to
remove mgmt_start_discovery_failed from hci_core.h since it's only
needed inside mgmt.c. Or did I miss something?

Johan

Re: [PATCH v2 05/13] Bluetooth: Move discovery macros to hci_core.h

[Johan Hedberg]

Hi Andre,

On Tue, Apr 30, 2013, Andre Guedes wrote:
> Some of discovery macros will be used in hci_core so we need to
> define them in common place such as hci_core.h. Thus, this patch
> moves discovery macros to hci_core.h and also adds the DISCOV_
> prefix to them.
> 
> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
> ---
>  include/net/bluetooth/hci_core.h | 10 ++++++++++
>  net/bluetooth/mgmt.c             | 24 ++++++------------------
>  2 files changed, 16 insertions(+), 18 deletions(-)

Acked-by: Johan Hedberg <johan.hedberg@intel.com>

Johan

Re: [PATCH v2 06/13] Bluetooth: Use HCI request in interleaved discovery

[Johan Hedberg]

Hi Andre,

On Tue, Apr 30, 2013, Andre Guedes wrote:
> +	case DISCOV_TYPE_INTERLEAVED:
> +		hci_dev_lock(hdev);
> +		hci_inquiry_cache_flush(hdev);
> +		hci_dev_unlock(hdev);
> +
> +		hci_req_init(&req, hdev);
> +
> +		memset(&cp, 0, sizeof(cp));
> +		memcpy(&cp.lap, lap, sizeof(cp.lap));
> +		cp.length = DISCOV_INTERLEAVED_INQUIRY_LEN;
> +		hci_req_add(&req, HCI_OP_INQUIRY, sizeof(cp), &cp);
> +
> +		err = hci_req_run(&req, inquiry_complete);
> +		if (err) {
> +			BT_ERR("Inquiry request failed: err %d", err);
> +			hci_dev_lock(hdev);
> +			hci_discovery_set_state(hdev, DISCOVERY_STOPPED);
> +			hci_dev_unlock(hdev);
> +		}
> +		break;

The locking here seems a bit needlessly fine grained. I know hci_req_run
doesn't strictly speaking need the hdev lock but you could simplify this
a bit by moving the inquiry cache flush right before hci_req_run and
doing

	hci_dev_lock(hdev);

	hci_inquiry_cache_flush(hdev);

	err = hci_req_run(..);
	if (err) {
		...
	}

	hci_dev_unlock(hdev);

Johan

Re: [PATCH v2 07/13] Bluetooth: Update stop_discovery to use HCI request

[Johan Hedberg]

Hi Andre,

On Tue, Apr 30, 2013, Andre Guedes wrote:
> This patch modifies the stop_discovery function so it uses the HCI
> request framework.
> 
> The HCI request is built according to the current discovery state
> (inquiry, LE scanning or name resolving) and a complete callback is
> register to handle the command complete event for the stop discovery
> command. This way, we move all stop_discovery mgmt handling code
> spread in hci_event.c to a single place in mgmt.c.
> 
> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
> ---
>  net/bluetooth/mgmt.c | 47 ++++++++++++++++++++++++++++++++++++++++-------
>  1 file changed, 40 insertions(+), 7 deletions(-)

Acked-by: Johan Hedberg <johan.hedberg@intel.com>

Johan

Re: [PATCH v2 08/13] Bluetooth: Remove stop discovery handling from hci_event.c

[Johan Hedberg]

Hi Andre,

On Tue, Apr 30, 2013, Andre Guedes wrote:
> Since all mgmt stop discovery command complete events are now handled
> in stop_discovery_complete callback in mgmt.c, we can remove this
> handling from hci_event.c.
> 
> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
> ---
>  net/bluetooth/hci_event.c | 16 ++--------------
>  1 file changed, 2 insertions(+), 14 deletions(-)

Acked-by: Johan Hedberg <johan.hedberg@intel.com>

However, looks like you're missing a patch to remove
mgmt_stop_discovery_failed() from hci_core.h (since it seems it's now
only needed inside mgmt.c).

Johan

Re: [PATCH v2 09/13] Bluetooth: Refactor hci_cc_le_set_scan_enable

[Johan Hedberg]

Hi Andre,

On Tue, Apr 30, 2013, Andre Guedes wrote:
> This patch does a trivial refactoring in hci_cc_le_set_scan_enable.
> Since start and stop discovery command failures are now handled in
> mgmt layer, the status check became empty. So, we can move it to
> outside the switch statement.
> 
> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
> ---
>  net/bluetooth/hci_event.c | 9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)

Acked-by: Johan Hedberg <johan.hedberg@intel.com>

Johan

Re: [PATCH v2 10/13] Bluetooth: Remove LE scan helpers

[Johan Hedberg]

Hi Andre,

On Tue, Apr 30, 2013, Andre Guedes wrote:
> This patch removes the LE scan helpers hci_le_scan and hci_cancel_
> le_scan and all code related to it. We now use the HCI request
> framework in device discovery functionality and these helpers are
> no longer needed.
> 
> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
> ---
>  include/net/bluetooth/hci_core.h |  13 -----
>  net/bluetooth/hci_core.c         | 113 ---------------------------------------
>  2 files changed, 126 deletions(-)

Acked-by: Johan Hedberg <johan.hedberg@intel.com>

Johan

Re: [PATCH v2 11/13] Bluetooth: Remove inquiry helpers

[Johan Hedberg]

Hi Andre,

On Tue, Apr 30, 2013, Andre Guedes wrote:
> This patch removes hci_do_inquiry and hci_cancel_inquiry helpers. We
> now use the HCI request framework in device discovery functionality
> and these helpers are no longer needed.
> 
> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
> ---
>  include/net/bluetooth/hci_core.h |  2 --
>  net/bluetooth/hci_core.c         | 30 ------------------------------
>  2 files changed, 32 deletions(-)

Acked-by: Johan Hedberg <johan.hedberg@intel.com>

Johan

Re: [PATCH v2 12/13] Bluetooth: Remove empty event handler

[Johan Hedberg]

Hi Andre,

On Tue, Apr 30, 2013, Andre Guedes wrote:
> This patch removes the hci_cc_le_set_scan_param event handler. This
> handler became empty because failures of this event are now handled
> by start_discovery_complete function in mgmt.c.
> 
> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
> ---
>  net/bluetooth/hci_event.c | 12 ------------
>  1 file changed, 12 deletions(-)

Acked-by: Johan Hedberg <johan.hedberg@intel.com>

Johan

Re: [PATCH v2 13/13] Bluetooth: Mgmt Device Found Event

[Johan Hedberg]

Hi Andre,

On Tue, Apr 30, 2013, Andre Guedes wrote:
> We only want to send Mgmt Device Found Events if we are running the
> Device Discovery procedure (started by the MGMT Start Discovery
> Command). Inquiry or LE scanning triggered by HCI raw interface (e.g.
> hcitool) or kernel internals should not send Mgmt Device Found Events.
> 
> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
> ---
>  net/bluetooth/mgmt.c | 3 +++
>  1 file changed, 3 insertions(+)

Acked-by: Johan Hedberg <johan.hedberg@intel.com>

Johan

Re: [PATCH v2 01/13] Bluetooth: Fix start_discovery for LE-only controllers

[Andre Guedes]

Hi Johan,

On Tue, Apr 30, 2013 at 3:43 AM, Johan Hedberg <johan.hedberg@gmail.com> wrote:
> Hi Andre,
>
> On Tue, Apr 30, 2013, Andre Guedes wrote:
>> LE-only controllers don't have local extended features (see commit
>> c73eee9172bf061202c75ad4d16506a7efd04d1c). Thus, lmp_host_le_capable
>> helper is not applicable and we should use test_bit(HCI_LE_ENABLED)
>> to check if the host supports LE. Otherwise, start_discovery will
>> always fail for LE-only controllers.
>>
>> This misbehavior can be easily checked by running mgmt-tester tool
>> (test "Start Discovery - Success 2" will fail).
>>
>> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
>> ---
>>  net/bluetooth/mgmt.c | 5 +++--
>>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> I already sent a patch for this last Wednesday:
>
>   Bluetooth: Fix checks for LE support on LE-only controllers
>
> So this patch can be ignored.

Great, I'll remove it from the v3 patch set.

Thanks,

Andre

Re: [PATCH v2 04/13] Bluetooth: Remove start discovery handling from hci_event.c

[Andre Guedes]

Hi Johan,

On Tue, Apr 30, 2013 at 5:25 AM, Johan Hedberg <johan.hedberg@gmail.com> wrote:
> Hi Andre,
>
> On Tue, Apr 30, 2013, Andre Guedes wrote:
>> Since all mgmt start discovery command complete events are now handled
>> in start_discovery_complete callback in mgmt.c, we can remove this
>> handling from hci_event.c.
>>
>> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
>> ---
>>  net/bluetooth/hci_event.c | 24 +-----------------------
>>  1 file changed, 1 insertion(+), 23 deletions(-)
>
> Acked-by: Johan Hedberg <johan.hedberg@intel.com>
>
> However, it looks like after this patch you could have another one to
> remove mgmt_start_discovery_failed from hci_core.h since it's only
> needed inside mgmt.c. Or did I miss something?

Yes, you're right. I'll make mgmt_start_disovery_failed static and
remove it from hci_core.h.

Thanks,

Andre

Re: [PATCH v2 06/13] Bluetooth: Use HCI request in interleaved discovery

[Andre Guedes]

Hi Johan,

On Tue, Apr 30, 2013 at 5:34 AM, Johan Hedberg <johan.hedberg@gmail.com> wrote:
> Hi Andre,
>
> On Tue, Apr 30, 2013, Andre Guedes wrote:
>> +     case DISCOV_TYPE_INTERLEAVED:
>> +             hci_dev_lock(hdev);
>> +             hci_inquiry_cache_flush(hdev);
>> +             hci_dev_unlock(hdev);
>> +
>> +             hci_req_init(&req, hdev);
>> +
>> +             memset(&cp, 0, sizeof(cp));
>> +             memcpy(&cp.lap, lap, sizeof(cp.lap));
>> +             cp.length = DISCOV_INTERLEAVED_INQUIRY_LEN;
>> +             hci_req_add(&req, HCI_OP_INQUIRY, sizeof(cp), &cp);
>> +
>> +             err = hci_req_run(&req, inquiry_complete);
>> +             if (err) {
>> +                     BT_ERR("Inquiry request failed: err %d", err);
>> +                     hci_dev_lock(hdev);
>> +                     hci_discovery_set_state(hdev, DISCOVERY_STOPPED);
>> +                     hci_dev_unlock(hdev);
>> +             }
>> +             break;
>
> The locking here seems a bit needlessly fine grained. I know hci_req_run
> doesn't strictly speaking need the hdev lock but you could simplify this
> a bit by moving the inquiry cache flush right before hci_req_run and
> doing
>
>         hci_dev_lock(hdev);
>
>         hci_inquiry_cache_flush(hdev);
>
>         err = hci_req_run(..);
>         if (err) {
>                 ...
>         }
>
>         hci_dev_unlock(hdev);

Ok, I'll change this.

Regards,

Andre

Re: [PATCH v2 08/13] Bluetooth: Remove stop discovery handling from hci_event.c

[Andre Guedes]

Hi Johan,

On Tue, Apr 30, 2013 at 5:36 AM, Johan Hedberg <johan.hedberg@gmail.com> wrote:
> Hi Andre,
>
> On Tue, Apr 30, 2013, Andre Guedes wrote:
>> Since all mgmt stop discovery command complete events are now handled
>> in stop_discovery_complete callback in mgmt.c, we can remove this
>> handling from hci_event.c.
>>
>> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
>> ---
>>  net/bluetooth/hci_event.c | 16 ++--------------
>>  1 file changed, 2 insertions(+), 14 deletions(-)
>
> Acked-by: Johan Hedberg <johan.hedberg@intel.com>
>
> However, looks like you're missing a patch to remove
> mgmt_stop_discovery_failed() from hci_core.h (since it seems it's now
> only needed inside mgmt.c).

Yes, you're right. I'll make mgmt_stop_disovery_failed static and
remove it from hci_core.h.

I'm going to do all these fixes and send a v3 patch set.

Thanks,

Andre