All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 00/16] Full support discovery procedure
@ 2011-07-11 21:11 Andre Guedes
  2011-07-11 21:11 ` [PATCH 01/16] Bluetooth: Periodic Inquiry and mgmt discovering event Andre Guedes
                   ` (15 more replies)
  0 siblings, 16 replies; 26+ messages in thread
From: Andre Guedes @ 2011-07-11 21:11 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Andre Guedes

Hi all,

The main changes from previous RFC series are:
    - 'status' parameter added to *_discovery_failed() functions. This
       way, we provide a more meaningful value to command status events.
    - Some code refactoring in stop_discovery() race condition prevention.
    - Minor code refactoring in BR/EDR/LE discovery support.

Thanks,

Andre Guedes.

Andre Guedes (16):
  Bluetooth: Periodic Inquiry and mgmt discovering event
  Bluetooth: Add failed/complete functions to discovery commands
  Bluetooth: Remove pending discovery commands
  Bluetooth: Check pending command in start_discovery()
  Bluetooth: Check pending commands in stop_discovery()
  Bluetooth: Create do_inquiry()
  Bluetooth: Create cancel_inquiry()
  Bluetooth: Fix stop_discovery()
  Bluetooth: Prepare for full support discovery procedures
  Bluetooth: Check 'dev_class' in mgmt_device_found()
  Bluetooth: Add 'eir_len' param to mgmt_device_found()
  Bluetooth: Report LE devices
  Bluetooth: Add 'le_scan_timer' to struct hci_dev
  Bluetooth: Add LE Scan helper functions
  Bluetooth: Support LE-Only discovery procedure
  Bluetooth: Support BR/EDR/LE discovery procedure

 include/net/bluetooth/hci.h      |   12 ++
 include/net/bluetooth/hci_core.h |   13 ++-
 net/bluetooth/hci_core.c         |   13 ++
 net/bluetooth/hci_event.c        |  134 +++++++++++++------
 net/bluetooth/mgmt.c             |  282 ++++++++++++++++++++++++++++++++++++--
 5 files changed, 402 insertions(+), 52 deletions(-)

-- 
1.7.4.1


^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH 01/16] Bluetooth: Periodic Inquiry and mgmt discovering event
  2011-07-11 21:11 [PATCH 00/16] Full support discovery procedure Andre Guedes
@ 2011-07-11 21:11 ` Andre Guedes
  2011-07-13 20:21   ` Gustavo Padovan
  2011-07-11 21:11 ` [PATCH 02/16] Bluetooth: Add failed/complete functions to discovery commands Andre Guedes
                   ` (14 subsequent siblings)
  15 siblings, 1 reply; 26+ messages in thread
From: Andre Guedes @ 2011-07-11 21:11 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Andre Guedes

By using periodic inquiry command we're not able to detect correctly
when the controller has started inquiry.

Today we have this workaround in inquiry result event handler to set
the HCI_INQUIRY flag when it sees the first inquiry result event.
This workaround isn't enough because the device may be performing
an inquiry but the HCI_INQUIRY flag is not set. For instance, if
there is no device in range, no inquiry result event is generated,
consequently, the HCI_INQUIRY flags isn't set when it should so.

We rely on HCI_INQUIRY flag to implement the discovery procedure
properly. So, as we aren't able to clear/set the HCI_INQUIRY flag in
a reliable manner, periodic inquiry events shouldn't change the
HCI_INQUIRY flag. In future, if needed, we might add a new flag (e.g.
HCI_PINQUIRY) to know if the controller is performing periodic
inquiry.

Thus, due to that issue and in order to keep compatibility with
userspace, periodic inquiry events shouldn't send mgmt discovering
events.

Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
---
 net/bluetooth/hci_event.c |   46 ++++++++++++--------------------------------
 1 files changed, 13 insertions(+), 33 deletions(-)

diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index a40170e..cf0efe5 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -58,13 +58,13 @@ static void hci_cc_inquiry_cancel(struct hci_dev *hdev, struct sk_buff *skb)
 	if (status)
 		return;
 
-	if (test_bit(HCI_MGMT, &hdev->flags) &&
-				test_and_clear_bit(HCI_INQUIRY, &hdev->flags))
-		mgmt_discovering(hdev->id, 0);
+	clear_bit(HCI_INQUIRY, &hdev->flags);
 
 	hci_req_complete(hdev, HCI_OP_INQUIRY_CANCEL, status);
 
 	hci_conn_check_pending(hdev);
+
+	mgmt_discovering(hdev->id, 0);
 }
 
 static void hci_cc_exit_periodic_inq(struct hci_dev *hdev, struct sk_buff *skb)
@@ -76,10 +76,6 @@ static void hci_cc_exit_periodic_inq(struct hci_dev *hdev, struct sk_buff *skb)
 	if (status)
 		return;
 
-	if (test_bit(HCI_MGMT, &hdev->flags) &&
-				test_and_clear_bit(HCI_INQUIRY, &hdev->flags))
-		mgmt_discovering(hdev->id, 0);
-
 	hci_conn_check_pending(hdev);
 }
 
@@ -959,10 +955,9 @@ static inline void hci_cs_inquiry(struct hci_dev *hdev, __u8 status)
 		return;
 	}
 
-	if (test_bit(HCI_MGMT, &hdev->flags) &&
-					!test_and_set_bit(HCI_INQUIRY,
-							&hdev->flags))
-		mgmt_discovering(hdev->id, 1);
+	set_bit(HCI_INQUIRY, &hdev->flags);
+
+	mgmt_discovering(hdev->id, 1);
 }
 
 static inline void hci_cs_create_conn(struct hci_dev *hdev, __u8 status)
@@ -1340,13 +1335,16 @@ static inline void hci_inquiry_complete_evt(struct hci_dev *hdev, struct sk_buff
 
 	BT_DBG("%s status %d", hdev->name, status);
 
-	if (test_bit(HCI_MGMT, &hdev->flags) &&
-				test_and_clear_bit(HCI_INQUIRY, &hdev->flags))
-		mgmt_discovering(hdev->id, 0);
+	hci_conn_check_pending(hdev);
+
+	if (!test_bit(HCI_INQUIRY, &hdev->flags))
+		return;
+
+	clear_bit(HCI_INQUIRY, &hdev->flags);
 
 	hci_req_complete(hdev, HCI_OP_INQUIRY, status);
 
-	hci_conn_check_pending(hdev);
+	mgmt_discovering(hdev->id, 0);
 }
 
 static inline void hci_inquiry_result_evt(struct hci_dev *hdev, struct sk_buff *skb)
@@ -1362,12 +1360,6 @@ static inline void hci_inquiry_result_evt(struct hci_dev *hdev, struct sk_buff *
 
 	hci_dev_lock(hdev);
 
-	if (!test_and_set_bit(HCI_INQUIRY, &hdev->flags)) {
-
-		if (test_bit(HCI_MGMT, &hdev->flags))
-			mgmt_discovering(hdev->id, 1);
-	}
-
 	for (; num_rsp; num_rsp--, info++) {
 		bacpy(&data.bdaddr, &info->bdaddr);
 		data.pscan_rep_mode	= info->pscan_rep_mode;
@@ -2360,12 +2352,6 @@ static inline void hci_inquiry_result_with_rssi_evt(struct hci_dev *hdev, struct
 
 	hci_dev_lock(hdev);
 
-	if (!test_and_set_bit(HCI_INQUIRY, &hdev->flags)) {
-
-		if (test_bit(HCI_MGMT, &hdev->flags))
-			mgmt_discovering(hdev->id, 1);
-	}
-
 	if ((skb->len - 1) / num_rsp != sizeof(struct inquiry_info_with_rssi)) {
 		struct inquiry_info_with_rssi_and_pscan_mode *info;
 		info = (void *) (skb->data + 1);
@@ -2528,12 +2514,6 @@ static inline void hci_extended_inquiry_result_evt(struct hci_dev *hdev, struct
 	if (!num_rsp)
 		return;
 
-	if (!test_and_set_bit(HCI_INQUIRY, &hdev->flags)) {
-
-		if (test_bit(HCI_MGMT, &hdev->flags))
-			mgmt_discovering(hdev->id, 1);
-	}
-
 	hci_dev_lock(hdev);
 
 	for (; num_rsp; num_rsp--, info++) {
-- 
1.7.4.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 02/16] Bluetooth: Add failed/complete functions to discovery commands
  2011-07-11 21:11 [PATCH 00/16] Full support discovery procedure Andre Guedes
  2011-07-11 21:11 ` [PATCH 01/16] Bluetooth: Periodic Inquiry and mgmt discovering event Andre Guedes
@ 2011-07-11 21:11 ` Andre Guedes
  2011-07-11 21:11 ` [PATCH 03/16] Bluetooth: Remove pending " Andre Guedes
                   ` (13 subsequent siblings)
  15 siblings, 0 replies; 26+ messages in thread
From: Andre Guedes @ 2011-07-11 21:11 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Andre Guedes

These functions remove pending commands and send command complete/
status events.

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

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index c41e275..0ccd724 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -857,6 +857,10 @@ int mgmt_device_found(u16 index, bdaddr_t *bdaddr, u8 *dev_class, s8 rssi,
 								u8 *eir);
 int mgmt_remote_name(u16 index, bdaddr_t *bdaddr, u8 *name);
 int mgmt_discovering(u16 index, u8 discovering);
+int mgmt_start_discovery_complete(u16 index);
+int mgmt_start_discovery_failed(u16 index, u8 status);
+int mgmt_stop_discovery_complete(u16 index);
+int mgmt_stop_discovery_failed(u16 index, u8 status);
 
 /* HCI info for socket */
 #define hci_pi(sk) ((struct hci_pinfo *) sk)
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 53e109e..6102648 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -2286,3 +2286,83 @@ int mgmt_discovering(u16 index, u8 discovering)
 	return mgmt_event(MGMT_EV_DISCOVERING, index, &discovering,
 						sizeof(discovering), NULL);
 }
+
+int mgmt_start_discovery_complete(u16 index)
+{
+	struct pending_cmd *cmd;
+	int err;
+
+	cmd = mgmt_pending_find(MGMT_OP_START_DISCOVERY, index);
+	if (!cmd)
+		return -ENOENT;
+
+	err = cmd_complete(cmd->sk, index, MGMT_OP_START_DISCOVERY, NULL, 0);
+
+	mgmt_pending_remove(cmd);
+
+	cmd = mgmt_pending_find(MGMT_OP_STOP_DISCOVERY, index);
+	if (cmd) {
+		cmd_status(cmd->sk, index, MGMT_OP_STOP_DISCOVERY, EPERM);
+		mgmt_pending_remove(cmd);
+	}
+
+	return err;
+}
+
+int mgmt_start_discovery_failed(u16 index, u8 status)
+{
+	struct pending_cmd *cmd;
+	int err;
+
+	cmd = mgmt_pending_find(MGMT_OP_START_DISCOVERY, index);
+	if (!cmd)
+		return -ENOENT;
+
+	err = cmd_status(cmd->sk, index, MGMT_OP_START_DISCOVERY, status);
+
+	mgmt_pending_remove(cmd);
+
+	cmd = mgmt_pending_find(MGMT_OP_STOP_DISCOVERY, index);
+	if (cmd) {
+		cmd_status(cmd->sk, index, MGMT_OP_STOP_DISCOVERY, EPERM);
+		mgmt_pending_remove(cmd);
+	}
+
+	return err;
+}
+
+int mgmt_stop_discovery_complete(u16 index)
+{
+	struct pending_cmd *cmd;
+	int err;
+
+	cmd = mgmt_pending_find(MGMT_OP_STOP_DISCOVERY, index);
+	if (!cmd)
+		return -ENOENT;
+
+	err = cmd_complete(cmd->sk, index, MGMT_OP_STOP_DISCOVERY, NULL, 0);
+
+	mgmt_pending_remove(cmd);
+
+	cmd = mgmt_pending_find(MGMT_OP_START_DISCOVERY, index);
+	if (cmd)
+		mgmt_pending_remove(cmd);
+
+	return err;
+}
+
+int mgmt_stop_discovery_failed(u16 index, u8 status)
+{
+	struct pending_cmd *cmd;
+	int err;
+
+	cmd = mgmt_pending_find(MGMT_OP_STOP_DISCOVERY, index);
+	if (!cmd)
+		return -ENOENT;
+
+	err = cmd_status(cmd->sk, index, MGMT_OP_STOP_DISCOVERY, status);
+
+	mgmt_pending_remove(cmd);
+
+	return err;
+}
-- 
1.7.4.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 03/16] Bluetooth: Remove pending discovery commands
  2011-07-11 21:11 [PATCH 00/16] Full support discovery procedure Andre Guedes
  2011-07-11 21:11 ` [PATCH 01/16] Bluetooth: Periodic Inquiry and mgmt discovering event Andre Guedes
  2011-07-11 21:11 ` [PATCH 02/16] Bluetooth: Add failed/complete functions to discovery commands Andre Guedes
@ 2011-07-11 21:11 ` Andre Guedes
  2011-07-11 21:11 ` [PATCH 04/16] Bluetooth: Check pending command in start_discovery() Andre Guedes
                   ` (12 subsequent siblings)
  15 siblings, 0 replies; 26+ messages in thread
From: Andre Guedes @ 2011-07-11 21:11 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Andre Guedes

Discovery commands are added to the pending list but they aren't
removed.

This patch removes MGMT_OP_START_DISCOVERY and MGMT_OP_STOP_DISCOVERY
commands from pending command list when they terminate.

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

diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index cf0efe5..ea9e105 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -55,8 +55,10 @@ static void hci_cc_inquiry_cancel(struct hci_dev *hdev, struct sk_buff *skb)
 
 	BT_DBG("%s status 0x%x", hdev->name, status);
 
-	if (status)
+	if (status) {
+		mgmt_stop_discovery_failed(hdev->id, bt_to_errno(status));
 		return;
+	}
 
 	clear_bit(HCI_INQUIRY, &hdev->flags);
 
@@ -65,6 +67,7 @@ static void hci_cc_inquiry_cancel(struct hci_dev *hdev, struct sk_buff *skb)
 	hci_conn_check_pending(hdev);
 
 	mgmt_discovering(hdev->id, 0);
+	mgmt_stop_discovery_complete(hdev->id);
 }
 
 static void hci_cc_exit_periodic_inq(struct hci_dev *hdev, struct sk_buff *skb)
@@ -952,6 +955,9 @@ static inline void hci_cs_inquiry(struct hci_dev *hdev, __u8 status)
 	if (status) {
 		hci_req_complete(hdev, HCI_OP_INQUIRY, status);
 		hci_conn_check_pending(hdev);
+
+		mgmt_start_discovery_failed(hdev->id, bt_to_errno(status));
+
 		return;
 	}
 
@@ -1340,11 +1346,17 @@ static inline void hci_inquiry_complete_evt(struct hci_dev *hdev, struct sk_buff
 	if (!test_bit(HCI_INQUIRY, &hdev->flags))
 		return;
 
+	if (status) {
+		mgmt_start_discovery_failed(hdev->id, bt_to_errno(status));
+		return;
+	}
+
 	clear_bit(HCI_INQUIRY, &hdev->flags);
 
 	hci_req_complete(hdev, HCI_OP_INQUIRY, status);
 
 	mgmt_discovering(hdev->id, 0);
+	mgmt_start_discovery_complete(hdev->id);
 }
 
 static inline void hci_inquiry_result_evt(struct hci_dev *hdev, struct sk_buff *skb)
-- 
1.7.4.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 04/16] Bluetooth: Check pending command in start_discovery()
  2011-07-11 21:11 [PATCH 00/16] Full support discovery procedure Andre Guedes
                   ` (2 preceding siblings ...)
  2011-07-11 21:11 ` [PATCH 03/16] Bluetooth: Remove pending " Andre Guedes
@ 2011-07-11 21:11 ` Andre Guedes
  2011-07-11 21:11 ` [PATCH 05/16] Bluetooth: Check pending commands in stop_discovery() Andre Guedes
                   ` (11 subsequent siblings)
  15 siblings, 0 replies; 26+ messages in thread
From: Andre Guedes @ 2011-07-11 21:11 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Andre Guedes

If discovery procedure is already running then EINPROGRESS command
status should be returned.

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

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 6102648..83693ac 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -1632,6 +1632,12 @@ static int start_discovery(struct sock *sk, u16 index)
 
 	hci_dev_lock_bh(hdev);
 
+	if (mgmt_pending_find(MGMT_OP_START_DISCOVERY, index)) {
+		err = cmd_status(sk, index, MGMT_OP_START_DISCOVERY,
+								EINPROGRESS);
+		goto failed;
+	}
+
 	cmd = mgmt_pending_add(sk, MGMT_OP_START_DISCOVERY, index, NULL, 0);
 	if (!cmd) {
 		err = -ENOMEM;
-- 
1.7.4.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 05/16] Bluetooth: Check pending commands in stop_discovery()
  2011-07-11 21:11 [PATCH 00/16] Full support discovery procedure Andre Guedes
                   ` (3 preceding siblings ...)
  2011-07-11 21:11 ` [PATCH 04/16] Bluetooth: Check pending command in start_discovery() Andre Guedes
@ 2011-07-11 21:11 ` Andre Guedes
  2011-07-13 20:20   ` Gustavo Padovan
  2011-07-11 21:11 ` [PATCH 06/16] Bluetooth: Create do_inquiry() Andre Guedes
                   ` (10 subsequent siblings)
  15 siblings, 1 reply; 26+ messages in thread
From: Andre Guedes @ 2011-07-11 21:11 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Andre Guedes

This patch adds extra checks in stop_discovery().

The MGMT_OP_STOP_DISCOVERY command should be executed if the device
is running the discovery procedure. So, if there is no discovery
procedure running then EINVAL command status should be returned.

Also, if a MGMT_OP_STOP_DISCOVERY command has been already issued
then EINPROGRESS command status should returned.

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

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 83693ac..d673ea3 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -1674,6 +1674,17 @@ static int stop_discovery(struct sock *sk, u16 index)
 
 	hci_dev_lock_bh(hdev);
 
+	if (!mgmt_pending_find(MGMT_OP_START_DISCOVERY, index)) {
+		err = cmd_status(sk, index, MGMT_OP_STOP_DISCOVERY, EINVAL);
+		goto failed;
+	}
+
+	if (mgmt_pending_find(MGMT_OP_STOP_DISCOVERY, index)) {
+		err = cmd_status(sk, index, MGMT_OP_STOP_DISCOVERY,
+								EINPROGRESS);
+		goto failed;
+	}
+
 	cmd = mgmt_pending_add(sk, MGMT_OP_STOP_DISCOVERY, index, NULL, 0);
 	if (!cmd) {
 		err = -ENOMEM;
-- 
1.7.4.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 06/16] Bluetooth: Create do_inquiry()
  2011-07-11 21:11 [PATCH 00/16] Full support discovery procedure Andre Guedes
                   ` (4 preceding siblings ...)
  2011-07-11 21:11 ` [PATCH 05/16] Bluetooth: Check pending commands in stop_discovery() Andre Guedes
@ 2011-07-11 21:11 ` Andre Guedes
  2011-07-11 21:11 ` [PATCH 07/16] Bluetooth: Create cancel_inquiry() Andre Guedes
                   ` (9 subsequent siblings)
  15 siblings, 0 replies; 26+ messages in thread
From: Andre Guedes @ 2011-07-11 21:11 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Andre Guedes

This patch moves all inquiry code from start_discovery() to a new
function called do_inquiry().

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

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index d673ea3..7ea8f95 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -1616,10 +1616,20 @@ static int remove_remote_oob_data(struct sock *sk, u16 index,
 	return err;
 }
 
-static int start_discovery(struct sock *sk, u16 index)
+static int do_inquiry(struct hci_dev *hdev, __u8 inq_length)
 {
 	u8 lap[3] = { 0x33, 0x8b, 0x9e };
 	struct hci_cp_inquiry cp;
+
+	memset(&cp, 0, sizeof(cp));
+	memcpy(&cp.lap, lap, 3);
+	cp.length  = inq_length;
+
+	return hci_send_cmd(hdev, HCI_OP_INQUIRY, sizeof(cp), &cp);
+}
+
+static int start_discovery(struct sock *sk, u16 index)
+{
 	struct pending_cmd *cmd;
 	struct hci_dev *hdev;
 	int err;
@@ -1644,12 +1654,7 @@ static int start_discovery(struct sock *sk, u16 index)
 		goto failed;
 	}
 
-	memset(&cp, 0, sizeof(cp));
-	memcpy(&cp.lap, lap, 3);
-	cp.length  = 0x08;
-	cp.num_rsp = 0x00;
-
-	err = hci_send_cmd(hdev, HCI_OP_INQUIRY, sizeof(cp), &cp);
+	err = do_inquiry(hdev, 0x08);
 	if (err < 0)
 		mgmt_pending_remove(cmd);
 
-- 
1.7.4.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 07/16] Bluetooth: Create cancel_inquiry()
  2011-07-11 21:11 [PATCH 00/16] Full support discovery procedure Andre Guedes
                   ` (5 preceding siblings ...)
  2011-07-11 21:11 ` [PATCH 06/16] Bluetooth: Create do_inquiry() Andre Guedes
@ 2011-07-11 21:11 ` Andre Guedes
  2011-07-11 21:11 ` [PATCH 08/16] Bluetooth: Fix stop_discovery() Andre Guedes
                   ` (8 subsequent siblings)
  15 siblings, 0 replies; 26+ messages in thread
From: Andre Guedes @ 2011-07-11 21:11 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Andre Guedes

This patch moves all cancel inquiry code from stop_discovery() to
a new function called cancel_inquiry().

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

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 7ea8f95..e43940e 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -1665,6 +1665,11 @@ failed:
 	return err;
 }
 
+static int cancel_inquiry(struct hci_dev *hdev)
+{
+	return hci_send_cmd(hdev, HCI_OP_INQUIRY_CANCEL, 0, NULL);
+}
+
 static int stop_discovery(struct sock *sk, u16 index)
 {
 	struct hci_dev *hdev;
@@ -1696,7 +1701,7 @@ static int stop_discovery(struct sock *sk, u16 index)
 		goto failed;
 	}
 
-	err = hci_send_cmd(hdev, HCI_OP_INQUIRY_CANCEL, 0, NULL);
+	err = cancel_inquiry(hdev);
 	if (err < 0)
 		mgmt_pending_remove(cmd);
 
-- 
1.7.4.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 08/16] Bluetooth: Fix stop_discovery()
  2011-07-11 21:11 [PATCH 00/16] Full support discovery procedure Andre Guedes
                   ` (6 preceding siblings ...)
  2011-07-11 21:11 ` [PATCH 07/16] Bluetooth: Create cancel_inquiry() Andre Guedes
@ 2011-07-11 21:11 ` Andre Guedes
  2011-07-13 20:15   ` Gustavo Padovan
  2011-07-11 21:11 ` [PATCH 09/16] Bluetooth: Prepare for full support discovery procedures Andre Guedes
                   ` (7 subsequent siblings)
  15 siblings, 1 reply; 26+ messages in thread
From: Andre Guedes @ 2011-07-11 21:11 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Andre Guedes

According to the Bluetooth spec, the inquiry cancel command should
only be issued after the inquiry command has been issued, a command
status event has been received for the inquiry command, and before
the inquiry complete event occurs.

As HCI_INQUIRY flag is only set just after an inquiry command status
event occurs and it is cleared just after an inquiry complete event
occurs, the inquiry cancel command should be issued only if HCI_INQUIRY
flag is set.

This spec constraint raises two possible race condition we must handle.

1. A MGMT_OP_STOP_DISCOVERY command is issued just after a
   MGMT_OP_START_DISCOVERY. The controller didn't send the inquiry
   command status yet therefore the HCI_INQUIRY flag is not set. Thus,
   stop_discovery() will send no inquiry cancel command and the
   discovery procedure won't be stopped. This race is addressed by
   checking for pending MGMT_OP_STOP_DISCOVERY command in inquiry
   command status event handler.

2. While the MGMT_OP_STOP_DISCOVERY is being processed the controller
   sends the inquiry complete event and the HCI_INQUIRY flag is
   cleared. stop_discovery() will add a pending MGMT_OP_STOP_DISCOVERY
   command which won't be removed since there is no ongoing discovery.
   This race is addressed by synchronizing stop_discovery and inquiry
   complete event handler threads.

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

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 0ccd724..1ff59f2 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -861,6 +861,8 @@ int mgmt_start_discovery_complete(u16 index);
 int mgmt_start_discovery_failed(u16 index, u8 status);
 int mgmt_stop_discovery_complete(u16 index);
 int mgmt_stop_discovery_failed(u16 index, u8 status);
+int mgmt_has_pending_stop_discov(u16 index);
+int mgmt_cancel_discovery(u16 index);
 
 /* HCI info for socket */
 #define hci_pi(sk) ((struct hci_pinfo *) sk)
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index ea9e105..c75211c 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -963,6 +963,11 @@ static inline void hci_cs_inquiry(struct hci_dev *hdev, __u8 status)
 
 	set_bit(HCI_INQUIRY, &hdev->flags);
 
+	if (mgmt_has_pending_stop_discov(hdev->id)) {
+		mgmt_cancel_discovery(hdev->id);
+		return;
+	}
+
 	mgmt_discovering(hdev->id, 1);
 }
 
@@ -1356,7 +1361,12 @@ static inline void hci_inquiry_complete_evt(struct hci_dev *hdev, struct sk_buff
 	hci_req_complete(hdev, HCI_OP_INQUIRY, status);
 
 	mgmt_discovering(hdev->id, 0);
+
+	hci_dev_lock(hdev);
+
 	mgmt_start_discovery_complete(hdev->id);
+
+	hci_dev_unlock(hdev);
 }
 
 static inline void hci_inquiry_result_evt(struct hci_dev *hdev, struct sk_buff *skb)
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index e43940e..bbb0daa 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -1670,6 +1670,21 @@ static int cancel_inquiry(struct hci_dev *hdev)
 	return hci_send_cmd(hdev, HCI_OP_INQUIRY_CANCEL, 0, NULL);
 }
 
+int mgmt_cancel_discovery(u16 index)
+{
+	struct hci_dev *hdev;
+	int res = 0;
+
+	hdev = hci_dev_get(index);
+
+	if (test_bit(HCI_INQUIRY, &hdev->flags))
+		res = cancel_inquiry(hdev);
+
+	hci_dev_put(hdev);
+
+	return res;
+}
+
 static int stop_discovery(struct sock *sk, u16 index)
 {
 	struct hci_dev *hdev;
@@ -1701,7 +1716,7 @@ static int stop_discovery(struct sock *sk, u16 index)
 		goto failed;
 	}
 
-	err = cancel_inquiry(hdev);
+	err = mgmt_cancel_discovery(index);
 	if (err < 0)
 		mgmt_pending_remove(cmd);
 
@@ -2393,3 +2408,11 @@ int mgmt_stop_discovery_failed(u16 index, u8 status)
 
 	return err;
 }
+
+int mgmt_has_pending_stop_discov(u16 index)
+{
+	if (mgmt_pending_find(MGMT_OP_STOP_DISCOVERY, index))
+		return 1;
+
+	return 0;
+}
-- 
1.7.4.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 09/16] Bluetooth: Prepare for full support discovery procedures
  2011-07-11 21:11 [PATCH 00/16] Full support discovery procedure Andre Guedes
                   ` (7 preceding siblings ...)
  2011-07-11 21:11 ` [PATCH 08/16] Bluetooth: Fix stop_discovery() Andre Guedes
@ 2011-07-11 21:11 ` Andre Guedes
  2011-07-13 20:26   ` Gustavo Padovan
  2011-07-11 21:11 ` [PATCH 10/16] Bluetooth: Check 'dev_class' in mgmt_device_found() Andre Guedes
                   ` (6 subsequent siblings)
  15 siblings, 1 reply; 26+ messages in thread
From: Andre Guedes @ 2011-07-11 21:11 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Andre Guedes

This patch prepares start_discovery() to support LE-Only and BR/EDR/LE
devices discovery procedures (BR/EDR devices are already supported).

Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
---
 include/net/bluetooth/hci.h      |    1 +
 include/net/bluetooth/hci_core.h |    1 +
 net/bluetooth/mgmt.c             |   37 ++++++++++++++++++++++++++++++++++++-
 3 files changed, 38 insertions(+), 1 deletions(-)

diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index be30aab..653daec 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -202,6 +202,7 @@ enum {
 
 #define LMP_EV4		0x01
 #define LMP_EV5		0x02
+#define LMP_NO_BREDR	0x20
 #define LMP_LE		0x40
 
 #define LMP_SNIFF_SUBR	0x02
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 1ff59f2..fc75c61 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -597,6 +597,7 @@ void hci_conn_del_sysfs(struct hci_conn *conn);
 #define lmp_esco_capable(dev)      ((dev)->features[3] & LMP_ESCO)
 #define lmp_ssp_capable(dev)       ((dev)->features[6] & LMP_SIMPLE_PAIR)
 #define lmp_no_flush_capable(dev)  ((dev)->features[6] & LMP_NO_FLUSH)
+#define lmp_no_bredr_capable(dev)  ((dev)->features[4] & LMP_NO_BREDR)
 #define lmp_le_capable(dev)        ((dev)->features[4] & LMP_LE)
 
 /* ----- Extended LMP capabilities ----- */
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index bbb0daa..af3beca 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -32,6 +32,15 @@
 #define MGMT_VERSION	0
 #define MGMT_REVISION	1
 
+enum bt_device_type {
+	BREDR_ONLY,
+	LE_ONLY,
+	BREDR_LE,
+	UNKNOWN,
+};
+
+#define BREDR_ONLY_INQ_LENGTH 0x08 /* TGAP(100) */
+
 struct pending_cmd {
 	struct list_head list;
 	__u16 opcode;
@@ -1628,10 +1637,23 @@ static int do_inquiry(struct hci_dev *hdev, __u8 inq_length)
 	return hci_send_cmd(hdev, HCI_OP_INQUIRY, sizeof(cp), &cp);
 }
 
+static int get_device_type(struct hci_dev *hdev)
+{
+	if (!lmp_no_bredr_capable(hdev) && lmp_host_le_capable(hdev))
+		return BREDR_LE;
+	else if (lmp_host_le_capable(hdev))
+		return LE_ONLY;
+	else if (!lmp_no_bredr_capable(hdev))
+		return BREDR_ONLY;
+	else
+		return UNKNOWN;
+}
+
 static int start_discovery(struct sock *sk, u16 index)
 {
 	struct pending_cmd *cmd;
 	struct hci_dev *hdev;
+	int dev_type;
 	int err;
 
 	BT_DBG("hci%u", index);
@@ -1654,7 +1676,20 @@ static int start_discovery(struct sock *sk, u16 index)
 		goto failed;
 	}
 
-	err = do_inquiry(hdev, 0x08);
+	dev_type = get_device_type(hdev);
+
+	switch (dev_type) {
+	case BREDR_ONLY:
+		err = do_inquiry(hdev, BREDR_ONLY_INQ_LENGTH);
+		break;
+	case LE_ONLY:
+	case BREDR_LE:
+		err = -ENOSYS;
+		break;
+	default:
+		err = -EINVAL;
+	}
+
 	if (err < 0)
 		mgmt_pending_remove(cmd);
 
-- 
1.7.4.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 10/16] Bluetooth: Check 'dev_class' in mgmt_device_found()
  2011-07-11 21:11 [PATCH 00/16] Full support discovery procedure Andre Guedes
                   ` (8 preceding siblings ...)
  2011-07-11 21:11 ` [PATCH 09/16] Bluetooth: Prepare for full support discovery procedures Andre Guedes
@ 2011-07-11 21:11 ` Andre Guedes
  2011-07-11 21:11 ` [PATCH 11/16] Bluetooth: Add 'eir_len' param to mgmt_device_found() Andre Guedes
                   ` (5 subsequent siblings)
  15 siblings, 0 replies; 26+ messages in thread
From: Andre Guedes @ 2011-07-11 21:11 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Andre Guedes

The mgmt_device_found event will be used to report LE devices found
during discovery procedure. Since LE advertising reports events
doesn't have class of device information, we need to check if
'dev_class' is not NULL before copying it.

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

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index af3beca..e27477c 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -2337,12 +2337,14 @@ int mgmt_device_found(u16 index, bdaddr_t *bdaddr, u8 *dev_class, s8 rssi,
 	memset(&ev, 0, sizeof(ev));
 
 	bacpy(&ev.bdaddr, bdaddr);
-	memcpy(ev.dev_class, dev_class, sizeof(ev.dev_class));
 	ev.rssi = rssi;
 
 	if (eir)
 		memcpy(ev.eir, eir, sizeof(ev.eir));
 
+	if (dev_class)
+		memcpy(ev.dev_class, dev_class, sizeof(ev.dev_class));
+
 	return mgmt_event(MGMT_EV_DEVICE_FOUND, index, &ev, sizeof(ev), NULL);
 }
 
-- 
1.7.4.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 11/16] Bluetooth: Add 'eir_len' param to mgmt_device_found()
  2011-07-11 21:11 [PATCH 00/16] Full support discovery procedure Andre Guedes
                   ` (9 preceding siblings ...)
  2011-07-11 21:11 ` [PATCH 10/16] Bluetooth: Check 'dev_class' in mgmt_device_found() Andre Guedes
@ 2011-07-11 21:11 ` Andre Guedes
  2011-07-11 21:11 ` [PATCH 12/16] Bluetooth: Report LE devices Andre Guedes
                   ` (4 subsequent siblings)
  15 siblings, 0 replies; 26+ messages in thread
From: Andre Guedes @ 2011-07-11 21:11 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Andre Guedes

This patch adds a new parameter to mgmt_device_found() to inform
the length of 'eir' pointer.

EIR data from LE advertising report event doesn't have a fixed length
as EIR data from extended inquiry result event does. We needed to
change mgmt_device_found() so it copies 'eir_len' bytes instead of
HCI_MAX_EIR_LENGTH.

Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
---
 include/net/bluetooth/hci_core.h |    2 +-
 net/bluetooth/hci_event.c        |    9 +++++----
 net/bluetooth/mgmt.c             |    7 +++++--
 3 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index fc75c61..fc9e8c4 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -855,7 +855,7 @@ int mgmt_set_local_name_complete(u16 index, u8 *name, u8 status);
 int mgmt_read_local_oob_data_reply_complete(u16 index, u8 *hash, u8 *randomizer,
 								u8 status);
 int mgmt_device_found(u16 index, bdaddr_t *bdaddr, u8 *dev_class, s8 rssi,
-								u8 *eir);
+							u8 *eir, u8 eir_len);
 int mgmt_remote_name(u16 index, bdaddr_t *bdaddr, u8 *name);
 int mgmt_discovering(u16 index, u8 discovering);
 int mgmt_start_discovery_complete(u16 index);
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index c75211c..4f85669 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -1393,7 +1393,7 @@ static inline void hci_inquiry_result_evt(struct hci_dev *hdev, struct sk_buff *
 		data.ssp_mode		= 0x00;
 		hci_inquiry_cache_update(hdev, &data);
 		mgmt_device_found(hdev->id, &info->bdaddr, info->dev_class, 0,
-									NULL);
+								NULL, 0);
 	}
 
 	hci_dev_unlock(hdev);
@@ -2390,7 +2390,7 @@ static inline void hci_inquiry_result_with_rssi_evt(struct hci_dev *hdev, struct
 			hci_inquiry_cache_update(hdev, &data);
 			mgmt_device_found(hdev->id, &info->bdaddr,
 						info->dev_class, info->rssi,
-						NULL);
+						NULL, 0);
 		}
 	} else {
 		struct inquiry_info_with_rssi *info = (void *) (skb->data + 1);
@@ -2407,7 +2407,7 @@ static inline void hci_inquiry_result_with_rssi_evt(struct hci_dev *hdev, struct
 			hci_inquiry_cache_update(hdev, &data);
 			mgmt_device_found(hdev->id, &info->bdaddr,
 						info->dev_class, info->rssi,
-						NULL);
+						NULL, 0);
 		}
 	}
 
@@ -2549,7 +2549,8 @@ static inline void hci_extended_inquiry_result_evt(struct hci_dev *hdev, struct
 		data.ssp_mode		= 0x01;
 		hci_inquiry_cache_update(hdev, &data);
 		mgmt_device_found(hdev->id, &info->bdaddr, info->dev_class,
-						info->rssi, info->data);
+						info->rssi, info->data,
+						sizeof(info->data));
 	}
 
 	hci_dev_unlock(hdev);
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index e27477c..83cd8be 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -2330,17 +2330,20 @@ int mgmt_read_local_oob_data_reply_complete(u16 index, u8 *hash, u8 *randomizer,
 }
 
 int mgmt_device_found(u16 index, bdaddr_t *bdaddr, u8 *dev_class, s8 rssi,
-								u8 *eir)
+							u8 *eir, u8 eir_len)
 {
 	struct mgmt_ev_device_found ev;
 
+	if (eir_len > sizeof(ev.eir))
+		return -EINVAL;
+
 	memset(&ev, 0, sizeof(ev));
 
 	bacpy(&ev.bdaddr, bdaddr);
 	ev.rssi = rssi;
 
 	if (eir)
-		memcpy(ev.eir, eir, sizeof(ev.eir));
+		memcpy(ev.eir, eir, eir_len);
 
 	if (dev_class)
 		memcpy(ev.dev_class, dev_class, sizeof(ev.dev_class));
-- 
1.7.4.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 12/16] Bluetooth: Report LE devices
  2011-07-11 21:11 [PATCH 00/16] Full support discovery procedure Andre Guedes
                   ` (10 preceding siblings ...)
  2011-07-11 21:11 ` [PATCH 11/16] Bluetooth: Add 'eir_len' param to mgmt_device_found() Andre Guedes
@ 2011-07-11 21:11 ` Andre Guedes
  2011-07-11 21:11 ` [PATCH 13/16] Bluetooth: Add 'le_scan_timer' to struct hci_dev Andre Guedes
                   ` (3 subsequent siblings)
  15 siblings, 0 replies; 26+ messages in thread
From: Andre Guedes @ 2011-07-11 21:11 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Andre Guedes

Devices found during LE scan should be reported to userspace through
mgmt_device_found events.

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

diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 4f85669..a4f0929 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -2840,6 +2840,7 @@ static inline void hci_le_adv_report_evt(struct hci_dev *hdev,
 {
 	struct hci_ev_le_advertising_info *ev;
 	u8 num_reports;
+	s8 rssi;
 
 	num_reports = skb->data[0];
 	ev = (void *) &skb->data[1];
@@ -2848,9 +2849,18 @@ static inline void hci_le_adv_report_evt(struct hci_dev *hdev,
 
 	hci_add_adv_entry(hdev, ev);
 
+	rssi = ev->data[ev->length];
+	mgmt_device_found(hdev->id, &ev->bdaddr, NULL, rssi, ev->data,
+								ev->length);
+
 	while (--num_reports) {
 		ev = (void *) (ev->data + ev->length + 1);
+
 		hci_add_adv_entry(hdev, ev);
+
+		rssi = ev->data[ev->length];
+		mgmt_device_found(hdev->id, &ev->bdaddr, NULL, rssi, ev->data,
+								ev->length);
 	}
 
 	hci_dev_unlock(hdev);
-- 
1.7.4.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 13/16] Bluetooth: Add 'le_scan_timer' to struct hci_dev
  2011-07-11 21:11 [PATCH 00/16] Full support discovery procedure Andre Guedes
                   ` (11 preceding siblings ...)
  2011-07-11 21:11 ` [PATCH 12/16] Bluetooth: Report LE devices Andre Guedes
@ 2011-07-11 21:11 ` Andre Guedes
  2011-07-11 21:11 ` [PATCH 14/16] Bluetooth: Add LE Scan helper functions Andre Guedes
                   ` (2 subsequent siblings)
  15 siblings, 0 replies; 26+ messages in thread
From: Andre Guedes @ 2011-07-11 21:11 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Andre Guedes

This patch adds a timer to disable LE Scan after some amount of
time. The timer will be controlled by the management interface and
it will be used to carry out discovery procedure in LE capable
devices.

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

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index fc9e8c4..5e78c45 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -209,6 +209,8 @@ struct hci_dev {
 	struct list_head	adv_entries;
 	struct timer_list	adv_timer;
 
+	struct timer_list	le_scan_timer;
+
 	struct hci_dev_stats	stat;
 
 	struct sk_buff_head	driver_init;
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 908fcd3..c0c46bf 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -1440,6 +1440,15 @@ int hci_add_adv_entry(struct hci_dev *hdev,
 	return 0;
 }
 
+static void hci_disable_le_scan(unsigned long arg)
+{
+	struct hci_cp_le_set_scan_enable cp;
+	struct hci_dev *hdev = (void *) arg;
+
+	memset(&cp, 0, sizeof(cp));
+	hci_send_cmd(hdev, HCI_OP_LE_SET_SCAN_ENABLE, sizeof(cp), &cp);
+}
+
 /* Register HCI device */
 int hci_register_dev(struct hci_dev *hdev)
 {
@@ -1510,6 +1519,9 @@ int hci_register_dev(struct hci_dev *hdev)
 	setup_timer(&hdev->adv_timer, hci_clear_adv_cache,
 						(unsigned long) hdev);
 
+	setup_timer(&hdev->le_scan_timer, hci_disable_le_scan,
+						(unsigned long) hdev);
+
 	INIT_WORK(&hdev->power_on, hci_power_on);
 	INIT_WORK(&hdev->power_off, hci_power_off);
 	setup_timer(&hdev->off_timer, hci_auto_off, (unsigned long) hdev);
@@ -1591,6 +1603,7 @@ int hci_unregister_dev(struct hci_dev *hdev)
 
 	hci_del_off_timer(hdev);
 	del_timer(&hdev->adv_timer);
+	del_timer(&hdev->le_scan_timer);
 
 	destroy_workqueue(hdev->workqueue);
 
-- 
1.7.4.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 14/16] Bluetooth: Add LE Scan helper functions
  2011-07-11 21:11 [PATCH 00/16] Full support discovery procedure Andre Guedes
                   ` (12 preceding siblings ...)
  2011-07-11 21:11 ` [PATCH 13/16] Bluetooth: Add 'le_scan_timer' to struct hci_dev Andre Guedes
@ 2011-07-11 21:11 ` Andre Guedes
  2011-07-11 21:11 ` [PATCH 15/16] Bluetooth: Support LE-Only discovery procedure Andre Guedes
  2011-07-11 21:11 ` [PATCH 16/16] Bluetooth: Support BR/EDR/LE " Andre Guedes
  15 siblings, 0 replies; 26+ messages in thread
From: Andre Guedes @ 2011-07-11 21:11 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Andre Guedes

This patch adds some helper functions to perform LE scan according
to the general discovery procedure recommendations.

Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
---
 include/net/bluetooth/hci.h |    9 +++++++
 net/bluetooth/mgmt.c        |   51 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 60 insertions(+), 0 deletions(-)

diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index 653daec..fb40388 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -729,6 +729,15 @@ struct hci_rp_le_read_buffer_size {
 	__u8     le_max_pkt;
 } __packed;
 
+#define HCI_OP_LE_SET_SCAN_PARAM	0x200b
+struct hci_cp_le_set_scan_param {
+	__u8    type;
+	__le16  interval;
+	__le16  window;
+	__u8    own_address_type;
+	__u8    filter_policy;
+} __packed;
+
 #define HCI_OP_LE_SET_SCAN_ENABLE	0x200c
 struct hci_cp_le_set_scan_enable {
 	__u8     enable;
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 83cd8be..01c63db8 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -1637,6 +1637,50 @@ static int do_inquiry(struct hci_dev *hdev, __u8 inq_length)
 	return hci_send_cmd(hdev, HCI_OP_INQUIRY, sizeof(cp), &cp);
 }
 
+static int set_le_scan_param(struct hci_dev *hdev)
+{
+	struct hci_cp_le_set_scan_param cp;
+
+	/*
+	 * These values are set according to LE General Discovery Procedure
+	 * specification.
+	 */
+	cp.type = 0x01; /* Active scanning */
+	cp.interval = cpu_to_le16(0x0012); /* TGAP(gen_disc_scan_int) */
+	cp.window = cpu_to_le16(0x0012); /* TGAP(gen_disc_scan_wind) */
+	cp.own_address_type = 0x00; /* Public */
+	cp.filter_policy = 0x00; /* Accept all advertisement packets */
+
+	return hci_send_cmd(hdev, HCI_OP_LE_SET_SCAN_PARAM, sizeof(cp), &cp);
+}
+
+static int enable_le_scan(struct hci_dev *hdev, __u8 enable)
+{
+	struct hci_cp_le_set_scan_enable cp;
+
+	memset(&cp, 0, sizeof(cp));
+	cp.enable = enable;
+
+	return hci_send_cmd(hdev, HCI_OP_LE_SET_SCAN_ENABLE, sizeof(cp), &cp);
+}
+
+static int do_le_scan(struct hci_dev *hdev, int timeout)
+{
+	int err;
+
+	err = set_le_scan_param(hdev);
+	if (err < 0)
+		return err;
+
+	err = enable_le_scan(hdev, 1);
+	if (err < 0)
+		return err;
+
+	mod_timer(&hdev->le_scan_timer,	jiffies + msecs_to_jiffies(timeout));
+
+	return 0;
+}
+
 static int get_device_type(struct hci_dev *hdev)
 {
 	if (!lmp_no_bredr_capable(hdev) && lmp_host_le_capable(hdev))
@@ -1705,6 +1749,13 @@ static int cancel_inquiry(struct hci_dev *hdev)
 	return hci_send_cmd(hdev, HCI_OP_INQUIRY_CANCEL, 0, NULL);
 }
 
+static int cancel_le_scan(struct hci_dev *hdev)
+{
+	del_timer(&hdev->le_scan_timer);
+
+	return enable_le_scan(hdev, 0);
+}
+
 int mgmt_cancel_discovery(u16 index)
 {
 	struct hci_dev *hdev;
-- 
1.7.4.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 15/16] Bluetooth: Support LE-Only discovery procedure
  2011-07-11 21:11 [PATCH 00/16] Full support discovery procedure Andre Guedes
                   ` (13 preceding siblings ...)
  2011-07-11 21:11 ` [PATCH 14/16] Bluetooth: Add LE Scan helper functions Andre Guedes
@ 2011-07-11 21:11 ` Andre Guedes
  2011-07-11 21:11 ` [PATCH 16/16] Bluetooth: Support BR/EDR/LE " Andre Guedes
  15 siblings, 0 replies; 26+ messages in thread
From: Andre Guedes @ 2011-07-11 21:11 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Andre Guedes

This patch adds support for LE-Only discovery procedure through
management interface.

A new flag (HCI_LE_SCAN) was created to inform if the controller is
performing LE scan. The HCI_LE_SCAN flag is set/cleared when the
controller starts/stops scanning.

Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
---
 include/net/bluetooth/hci.h |    2 ++
 net/bluetooth/hci_event.c   |   39 ++++++++++++++++++++++++++++++++++++---
 net/bluetooth/mgmt.c        |    5 +++++
 3 files changed, 43 insertions(+), 3 deletions(-)

diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index fb40388..c4fdeeb 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -86,6 +86,8 @@ enum {
 	HCI_DEBUG_KEYS,
 
 	HCI_RESET,
+
+	HCI_LE_SCAN,
 };
 
 /* HCI ioctl defines */
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index a4f0929..08774c2 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -890,9 +890,6 @@ static void hci_cc_le_set_scan_enable(struct hci_dev *hdev,
 
 	BT_DBG("%s status 0x%x", hdev->name, status);
 
-	if (status)
-		return;
-
 	cp = hci_sent_cmd_data(hdev, HCI_OP_LE_SET_SCAN_ENABLE);
 	if (!cp)
 		return;
@@ -900,12 +897,48 @@ static void hci_cc_le_set_scan_enable(struct hci_dev *hdev,
 	hci_dev_lock(hdev);
 
 	if (cp->enable == 0x01) {
+		if (status) {
+			mgmt_start_discovery_failed(hdev->id,
+							bt_to_errno(status));
+			goto unlock;
+		}
+
+		set_bit(HCI_LE_SCAN, &hdev->flags);
+
 		del_timer(&hdev->adv_timer);
 		hci_adv_entries_clear(hdev);
+
+		if (mgmt_has_pending_stop_discov(hdev->id)) {
+			mgmt_cancel_discovery(hdev->id);
+			goto unlock;
+		}
+
+		mgmt_discovering(hdev->id, 1);
 	} else if (cp->enable == 0x00) {
+		if (status) {
+			if (mgmt_has_pending_stop_discov(hdev->id))
+				mgmt_stop_discovery_failed(hdev->id,
+							bt_to_errno(status));
+			else
+				mgmt_start_discovery_failed(hdev->id,
+							bt_to_errno(status));
+
+			goto unlock;
+		}
+
+		clear_bit(HCI_LE_SCAN, &hdev->flags);
+
 		mod_timer(&hdev->adv_timer, jiffies + ADV_CLEAR_TIMEOUT);
+
+		mgmt_discovering(hdev->id, 0);
+
+		if (mgmt_has_pending_stop_discov(hdev->id))
+			mgmt_stop_discovery_complete(hdev->id);
+		else
+			mgmt_start_discovery_complete(hdev->id);
 	}
 
+unlock:
 	hci_dev_unlock(hdev);
 }
 
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 01c63db8..dcfc66f 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -40,6 +40,7 @@ enum bt_device_type {
 };
 
 #define BREDR_ONLY_INQ_LENGTH 0x08 /* TGAP(100) */
+#define LE_ONLY_SCAN_TIMEOUT 10240 /* TGAP(gen_disc_scan_min) */
 
 struct pending_cmd {
 	struct list_head list;
@@ -1727,6 +1728,8 @@ static int start_discovery(struct sock *sk, u16 index)
 		err = do_inquiry(hdev, BREDR_ONLY_INQ_LENGTH);
 		break;
 	case LE_ONLY:
+		err = do_le_scan(hdev, LE_ONLY_SCAN_TIMEOUT);
+		break;
 	case BREDR_LE:
 		err = -ENOSYS;
 		break;
@@ -1765,6 +1768,8 @@ int mgmt_cancel_discovery(u16 index)
 
 	if (test_bit(HCI_INQUIRY, &hdev->flags))
 		res = cancel_inquiry(hdev);
+	else if (test_bit(HCI_LE_SCAN, &hdev->flags))
+		res = cancel_le_scan(hdev);
 
 	hci_dev_put(hdev);
 
-- 
1.7.4.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 16/16] Bluetooth: Support BR/EDR/LE discovery procedure
  2011-07-11 21:11 [PATCH 00/16] Full support discovery procedure Andre Guedes
                   ` (14 preceding siblings ...)
  2011-07-11 21:11 ` [PATCH 15/16] Bluetooth: Support LE-Only discovery procedure Andre Guedes
@ 2011-07-11 21:11 ` Andre Guedes
  15 siblings, 0 replies; 26+ messages in thread
From: Andre Guedes @ 2011-07-11 21:11 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Andre Guedes

This patch adds support for BR/EDR/LE discovery procedure through
management interface.

Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
---
 include/net/bluetooth/hci_core.h |    2 ++
 net/bluetooth/hci_event.c        |    8 +++++++-
 net/bluetooth/mgmt.c             |   38 +++++++++++++++++++++++++++++++++++++-
 3 files changed, 46 insertions(+), 2 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 5e78c45..9e5736b 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -866,6 +866,8 @@ int mgmt_stop_discovery_complete(u16 index);
 int mgmt_stop_discovery_failed(u16 index, u8 status);
 int mgmt_has_pending_stop_discov(u16 index);
 int mgmt_cancel_discovery(u16 index);
+int mgmt_is_interleaved_discovery(u16 index);
+int mgmt_do_interleaved_discovery(u16 index);
 
 /* HCI info for socket */
 #define hci_pi(sk) ((struct hci_pinfo *) sk)
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 08774c2..55872ff 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -913,7 +913,8 @@ static void hci_cc_le_set_scan_enable(struct hci_dev *hdev,
 			goto unlock;
 		}
 
-		mgmt_discovering(hdev->id, 1);
+		if (!mgmt_is_interleaved_discovery(hdev->id))
+			mgmt_discovering(hdev->id, 1);
 	} else if (cp->enable == 0x00) {
 		if (status) {
 			if (mgmt_has_pending_stop_discov(hdev->id))
@@ -1393,6 +1394,11 @@ static inline void hci_inquiry_complete_evt(struct hci_dev *hdev, struct sk_buff
 
 	hci_req_complete(hdev, HCI_OP_INQUIRY, status);
 
+	if (mgmt_is_interleaved_discovery(hdev->id)) {
+		mgmt_do_interleaved_discovery(hdev->id);
+		return;
+	}
+
 	mgmt_discovering(hdev->id, 0);
 
 	hci_dev_lock(hdev);
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index dcfc66f..f2d9078 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -40,6 +40,8 @@ enum bt_device_type {
 };
 
 #define BREDR_ONLY_INQ_LENGTH 0x08 /* TGAP(100) */
+#define BREDR_LE_INQ_LENGTH 0x04 /* TGAP(100)/2 */
+#define BREDR_LE_SCAN_TIMEOUT 5120 /* TGAP(100)/2 */
 #define LE_ONLY_SCAN_TIMEOUT 10240 /* TGAP(gen_disc_scan_min) */
 
 struct pending_cmd {
@@ -1731,7 +1733,7 @@ static int start_discovery(struct sock *sk, u16 index)
 		err = do_le_scan(hdev, LE_ONLY_SCAN_TIMEOUT);
 		break;
 	case BREDR_LE:
-		err = -ENOSYS;
+		err = do_inquiry(hdev, BREDR_LE_INQ_LENGTH);
 		break;
 	default:
 		err = -EINVAL;
@@ -2512,3 +2514,37 @@ int mgmt_has_pending_stop_discov(u16 index)
 
 	return 0;
 }
+
+int mgmt_is_interleaved_discovery(u16 index)
+{
+	struct hci_dev *hdev;
+	int res = 0;
+
+	hdev = hci_dev_get(index);
+
+	if (get_device_type(hdev) == BREDR_LE &&
+			mgmt_pending_find(MGMT_OP_START_DISCOVERY, hdev->id))
+		res = 1;
+
+	hci_dev_put(hdev);
+
+	return res;
+}
+
+int mgmt_do_interleaved_discovery(u16 index)
+{
+	struct hci_dev *hdev;
+	int err;
+
+	hdev = hci_dev_get(index);
+
+	err = do_le_scan(hdev, BREDR_LE_SCAN_TIMEOUT);
+	if (err < 0) {
+		mgmt_discovering(hdev->id, 0);
+		mgmt_start_discovery_failed(hdev->id, err);
+	}
+
+	hci_dev_put(hdev);
+
+	return err;
+}
-- 
1.7.4.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* Re: [PATCH 08/16] Bluetooth: Fix stop_discovery()
  2011-07-11 21:11 ` [PATCH 08/16] Bluetooth: Fix stop_discovery() Andre Guedes
@ 2011-07-13 20:15   ` Gustavo Padovan
  2011-07-14 14:31     ` Andre Guedes
  0 siblings, 1 reply; 26+ messages in thread
From: Gustavo Padovan @ 2011-07-13 20:15 UTC (permalink / raw)
  To: Andre Guedes; +Cc: linux-bluetooth

Hi Andre,

* Andre Guedes <andre.guedes@openbossa.org> [2011-07-11 18:11:51 -0300]:

> According to the Bluetooth spec, the inquiry cancel command should
> only be issued after the inquiry command has been issued, a command
> status event has been received for the inquiry command, and before
> the inquiry complete event occurs.
> 
> As HCI_INQUIRY flag is only set just after an inquiry command status
> event occurs and it is cleared just after an inquiry complete event
> occurs, the inquiry cancel command should be issued only if HCI_INQUIRY
> flag is set.
> 
> This spec constraint raises two possible race condition we must handle.
> 
> 1. A MGMT_OP_STOP_DISCOVERY command is issued just after a
>    MGMT_OP_START_DISCOVERY. The controller didn't send the inquiry
>    command status yet therefore the HCI_INQUIRY flag is not set. Thus,
>    stop_discovery() will send no inquiry cancel command and the
>    discovery procedure won't be stopped. This race is addressed by
>    checking for pending MGMT_OP_STOP_DISCOVERY command in inquiry
>    command status event handler.
> 
> 2. While the MGMT_OP_STOP_DISCOVERY is being processed the controller
>    sends the inquiry complete event and the HCI_INQUIRY flag is
>    cleared. stop_discovery() will add a pending MGMT_OP_STOP_DISCOVERY
>    command which won't be removed since there is no ongoing discovery.
>    This race is addressed by synchronizing stop_discovery and inquiry
>    complete event handler threads.
> 
> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
> ---
>  include/net/bluetooth/hci_core.h |    2 ++
>  net/bluetooth/hci_event.c        |   10 ++++++++++
>  net/bluetooth/mgmt.c             |   25 ++++++++++++++++++++++++-
>  3 files changed, 36 insertions(+), 1 deletions(-)
> 
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index 0ccd724..1ff59f2 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -861,6 +861,8 @@ int mgmt_start_discovery_complete(u16 index);
>  int mgmt_start_discovery_failed(u16 index, u8 status);
>  int mgmt_stop_discovery_complete(u16 index);
>  int mgmt_stop_discovery_failed(u16 index, u8 status);
> +int mgmt_has_pending_stop_discov(u16 index);
> +int mgmt_cancel_discovery(u16 index);
>  
>  /* HCI info for socket */
>  #define hci_pi(sk) ((struct hci_pinfo *) sk)
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index ea9e105..c75211c 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -963,6 +963,11 @@ static inline void hci_cs_inquiry(struct hci_dev *hdev, __u8 status)
>  
>  	set_bit(HCI_INQUIRY, &hdev->flags);
>  
> +	if (mgmt_has_pending_stop_discov(hdev->id)) {

Isn't a new bit flag log HCI_INQUIRY_CANCEL better that this? First this has
read a list, second it is a really ugly  function name.

> +		mgmt_cancel_discovery(hdev->id);

Just call hci_send_cmd(HCI_OP_INQUIRY_CANCEL)

> +		return;
> +	}
> +
>  	mgmt_discovering(hdev->id, 1);
>  }
>  
> @@ -1356,7 +1361,12 @@ static inline void hci_inquiry_complete_evt(struct hci_dev *hdev, struct sk_buff
>  	hci_req_complete(hdev, HCI_OP_INQUIRY, status);
>  
>  	mgmt_discovering(hdev->id, 0);
> +
> +	hci_dev_lock(hdev);
> +
>  	mgmt_start_discovery_complete(hdev->id);
> +
> +	hci_dev_unlock(hdev);
>  }
>  
>  static inline void hci_inquiry_result_evt(struct hci_dev *hdev, struct sk_buff *skb)
> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> index e43940e..bbb0daa 100644
> --- a/net/bluetooth/mgmt.c
> +++ b/net/bluetooth/mgmt.c
> @@ -1670,6 +1670,21 @@ static int cancel_inquiry(struct hci_dev *hdev)
>  	return hci_send_cmd(hdev, HCI_OP_INQUIRY_CANCEL, 0, NULL);
>  }
>  
> +int mgmt_cancel_discovery(u16 index)
> +{
> +	struct hci_dev *hdev;
> +	int res = 0;
> +
> +	hdev = hci_dev_get(index);
> +
> +	if (test_bit(HCI_INQUIRY, &hdev->flags))
> +		res = cancel_inquiry(hdev);
> +
> +	hci_dev_put(hdev);
> +
> +	return res;
> +}
> +
>  static int stop_discovery(struct sock *sk, u16 index)
>  {
>  	struct hci_dev *hdev;
> @@ -1701,7 +1716,7 @@ static int stop_discovery(struct sock *sk, u16 index)
>  		goto failed;
>  	}

You can check here with mgmt_pending_find() if the HCI_OP_INQUIRY_CANCEL was
already issued or not. This simplifies code a bit.

	Gustavo

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 05/16] Bluetooth: Check pending commands in stop_discovery()
  2011-07-11 21:11 ` [PATCH 05/16] Bluetooth: Check pending commands in stop_discovery() Andre Guedes
@ 2011-07-13 20:20   ` Gustavo Padovan
  2011-07-14 14:29     ` Andre Guedes
  0 siblings, 1 reply; 26+ messages in thread
From: Gustavo Padovan @ 2011-07-13 20:20 UTC (permalink / raw)
  To: Andre Guedes; +Cc: linux-bluetooth

* Andre Guedes <andre.guedes@openbossa.org> [2011-07-11 18:11:48 -0300]:

> This patch adds extra checks in stop_discovery().
> 
> The MGMT_OP_STOP_DISCOVERY command should be executed if the device
> is running the discovery procedure. So, if there is no discovery
> procedure running then EINVAL command status should be returned.
> 
> Also, if a MGMT_OP_STOP_DISCOVERY command has been already issued
> then EINPROGRESS command status should returned.
> 
> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
> ---
>  net/bluetooth/mgmt.c |   11 +++++++++++
>  1 files changed, 11 insertions(+), 0 deletions(-)
> 
> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> index 83693ac..d673ea3 100644
> --- a/net/bluetooth/mgmt.c
> +++ b/net/bluetooth/mgmt.c
> @@ -1674,6 +1674,17 @@ static int stop_discovery(struct sock *sk, u16 index)
>  
>  	hci_dev_lock_bh(hdev);
>  
> +	if (!mgmt_pending_find(MGMT_OP_START_DISCOVERY, index)) {
> +		err = cmd_status(sk, index, MGMT_OP_STOP_DISCOVERY, EINVAL);
> +		goto failed;
> +	}
> +
> +	if (mgmt_pending_find(MGMT_OP_STOP_DISCOVERY, index)) {
> +		err = cmd_status(sk, index, MGMT_OP_STOP_DISCOVERY,
> +								EINPROGRESS);
> +		goto failed;
> +	}

So here is what I said. Doesn't it fix that sync issue?

	Gustavo

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 01/16] Bluetooth: Periodic Inquiry and mgmt discovering event
  2011-07-11 21:11 ` [PATCH 01/16] Bluetooth: Periodic Inquiry and mgmt discovering event Andre Guedes
@ 2011-07-13 20:21   ` Gustavo Padovan
  2011-07-14 14:28     ` Andre Guedes
  0 siblings, 1 reply; 26+ messages in thread
From: Gustavo Padovan @ 2011-07-13 20:21 UTC (permalink / raw)
  To: Andre Guedes; +Cc: linux-bluetooth

* Andre Guedes <andre.guedes@openbossa.org> [2011-07-11 18:11:44 -0300]:

> By using periodic inquiry command we're not able to detect correctly
> when the controller has started inquiry.
> 
> Today we have this workaround in inquiry result event handler to set
> the HCI_INQUIRY flag when it sees the first inquiry result event.
> This workaround isn't enough because the device may be performing
> an inquiry but the HCI_INQUIRY flag is not set. For instance, if
> there is no device in range, no inquiry result event is generated,
> consequently, the HCI_INQUIRY flags isn't set when it should so.
> 
> We rely on HCI_INQUIRY flag to implement the discovery procedure
> properly. So, as we aren't able to clear/set the HCI_INQUIRY flag in
> a reliable manner, periodic inquiry events shouldn't change the
> HCI_INQUIRY flag. In future, if needed, we might add a new flag (e.g.
> HCI_PINQUIRY) to know if the controller is performing periodic
> inquiry.
> 
> Thus, due to that issue and in order to keep compatibility with
> userspace, periodic inquiry events shouldn't send mgmt discovering
> events.
> 
> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
> ---
>  net/bluetooth/hci_event.c |   46 ++++++++++++--------------------------------
>  1 files changed, 13 insertions(+), 33 deletions(-)
> 
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index a40170e..cf0efe5 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -58,13 +58,13 @@ static void hci_cc_inquiry_cancel(struct hci_dev *hdev, struct sk_buff *skb)
>  	if (status)
>  		return;
>  
> -	if (test_bit(HCI_MGMT, &hdev->flags) &&
> -				test_and_clear_bit(HCI_INQUIRY, &hdev->flags))
> -		mgmt_discovering(hdev->id, 0);
> +	clear_bit(HCI_INQUIRY, &hdev->flags);
>  
>  	hci_req_complete(hdev, HCI_OP_INQUIRY_CANCEL, status);
>  
>  	hci_conn_check_pending(hdev);
> +
> +	mgmt_discovering(hdev->id, 0);
>  }
>  
>  static void hci_cc_exit_periodic_inq(struct hci_dev *hdev, struct sk_buff *skb)
> @@ -76,10 +76,6 @@ static void hci_cc_exit_periodic_inq(struct hci_dev *hdev, struct sk_buff *skb)
>  	if (status)
>  		return;
>  
> -	if (test_bit(HCI_MGMT, &hdev->flags) &&
> -				test_and_clear_bit(HCI_INQUIRY, &hdev->flags))
> -		mgmt_discovering(hdev->id, 0);
> -
>  	hci_conn_check_pending(hdev);
>  }
>  
> @@ -959,10 +955,9 @@ static inline void hci_cs_inquiry(struct hci_dev *hdev, __u8 status)
>  		return;
>  	}
>  
> -	if (test_bit(HCI_MGMT, &hdev->flags) &&
> -					!test_and_set_bit(HCI_INQUIRY,
> -							&hdev->flags))
> -		mgmt_discovering(hdev->id, 1);
> +	set_bit(HCI_INQUIRY, &hdev->flags);
> +
> +	mgmt_discovering(hdev->id, 1);
>  }
>  
>  static inline void hci_cs_create_conn(struct hci_dev *hdev, __u8 status)
> @@ -1340,13 +1335,16 @@ static inline void hci_inquiry_complete_evt(struct hci_dev *hdev, struct sk_buff
>  
>  	BT_DBG("%s status %d", hdev->name, status);
>  
> -	if (test_bit(HCI_MGMT, &hdev->flags) &&
> -				test_and_clear_bit(HCI_INQUIRY, &hdev->flags))
> -		mgmt_discovering(hdev->id, 0);
> +	hci_conn_check_pending(hdev);
> +
> +	if (!test_bit(HCI_INQUIRY, &hdev->flags))
> +		return;
> +
> +	clear_bit(HCI_INQUIRY, &hdev->flags);

this can be test_and_clear_bit

	Gustavo

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 09/16] Bluetooth: Prepare for full support discovery procedures
  2011-07-11 21:11 ` [PATCH 09/16] Bluetooth: Prepare for full support discovery procedures Andre Guedes
@ 2011-07-13 20:26   ` Gustavo Padovan
  2011-07-14 14:31     ` Andre Guedes
  0 siblings, 1 reply; 26+ messages in thread
From: Gustavo Padovan @ 2011-07-13 20:26 UTC (permalink / raw)
  To: Andre Guedes; +Cc: linux-bluetooth

* Andre Guedes <andre.guedes@openbossa.org> [2011-07-11 18:11:52 -0300]:

> This patch prepares start_discovery() to support LE-Only and BR/EDR/LE
> devices discovery procedures (BR/EDR devices are already supported).
> 
> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
> ---
>  include/net/bluetooth/hci.h      |    1 +
>  include/net/bluetooth/hci_core.h |    1 +
>  net/bluetooth/mgmt.c             |   37 ++++++++++++++++++++++++++++++++++++-
>  3 files changed, 38 insertions(+), 1 deletions(-)
> 
> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> index be30aab..653daec 100644
> --- a/include/net/bluetooth/hci.h
> +++ b/include/net/bluetooth/hci.h
> @@ -202,6 +202,7 @@ enum {
>  
>  #define LMP_EV4		0x01
>  #define LMP_EV5		0x02
> +#define LMP_NO_BREDR	0x20
>  #define LMP_LE		0x40
>  
>  #define LMP_SNIFF_SUBR	0x02
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index 1ff59f2..fc75c61 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -597,6 +597,7 @@ void hci_conn_del_sysfs(struct hci_conn *conn);
>  #define lmp_esco_capable(dev)      ((dev)->features[3] & LMP_ESCO)
>  #define lmp_ssp_capable(dev)       ((dev)->features[6] & LMP_SIMPLE_PAIR)
>  #define lmp_no_flush_capable(dev)  ((dev)->features[6] & LMP_NO_FLUSH)
> +#define lmp_no_bredr_capable(dev)  ((dev)->features[4] & LMP_NO_BREDR)

It's more logic call this lmp_bredr_capable() regardless what the bit really
means.

	Gustavo

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 01/16] Bluetooth: Periodic Inquiry and mgmt discovering event
  2011-07-13 20:21   ` Gustavo Padovan
@ 2011-07-14 14:28     ` Andre Guedes
  0 siblings, 0 replies; 26+ messages in thread
From: Andre Guedes @ 2011-07-14 14:28 UTC (permalink / raw)
  To: Andre Guedes, linux-bluetooth

Hi Gustavo,

On Wed, Jul 13, 2011 at 5:21 PM, Gustavo Padovan <padovan@profusion.mobi> wrote:
>> @@ -1340,13 +1335,16 @@ static inline void hci_inquiry_complete_evt(struct hci_dev *hdev, struct sk_buff
>>
>>       BT_DBG("%s status %d", hdev->name, status);
>>
>> -     if (test_bit(HCI_MGMT, &hdev->flags) &&
>> -                             test_and_clear_bit(HCI_INQUIRY, &hdev->flags))
>> -             mgmt_discovering(hdev->id, 0);
>> +     hci_conn_check_pending(hdev);
>> +
>> +     if (!test_bit(HCI_INQUIRY, &hdev->flags))
>> +             return;
>> +
>> +     clear_bit(HCI_INQUIRY, &hdev->flags);
>
> this can be test_and_clear_bit

Yes, I could have done like this. However it would be replaced by
test_bit and clear_bit anyway later on patch 03/16.

Andre

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 05/16] Bluetooth: Check pending commands in stop_discovery()
  2011-07-13 20:20   ` Gustavo Padovan
@ 2011-07-14 14:29     ` Andre Guedes
  0 siblings, 0 replies; 26+ messages in thread
From: Andre Guedes @ 2011-07-14 14:29 UTC (permalink / raw)
  To: Andre Guedes, linux-bluetooth

Hi Gustavo,

On Wed, Jul 13, 2011 at 5:20 PM, Gustavo Padovan <padovan@profusion.mobi> wrote:
>> @@ -1674,6 +1674,17 @@ static int stop_discovery(struct sock *sk, u16 index)
>>
>>       hci_dev_lock_bh(hdev);
>>
>> +     if (!mgmt_pending_find(MGMT_OP_START_DISCOVERY, index)) {
>> +             err = cmd_status(sk, index, MGMT_OP_STOP_DISCOVERY, EINVAL);
>> +             goto failed;
>> +     }
>> +
>> +     if (mgmt_pending_find(MGMT_OP_STOP_DISCOVERY, index)) {
>> +             err = cmd_status(sk, index, MGMT_OP_STOP_DISCOVERY,
>> +                                                             EINPROGRESS);
>> +             goto failed;
>> +     }
>
> So here is what I said. Doesn't it fix that sync issue?

No, it doesn't. This peice of code only prevents you from carrying out
a MGMT_OP_STOP_DISCOVERY when there is no ongoing discovery.
IOW, if there is no ongoing discovery then MGMT_OP_STOP_DISCOVERY
command should failed.

The race condition scenario described in 08/16 you have a ongoing
discovery and a MGMT_OP_STOP_DISCOVERY command is issued.
During stop_discovery() execution, after checking for pending
MGMT_OP_START_DISCOVERY command, an inquiry complete event is
raised.

Andre

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 08/16] Bluetooth: Fix stop_discovery()
  2011-07-13 20:15   ` Gustavo Padovan
@ 2011-07-14 14:31     ` Andre Guedes
  2011-07-22 15:24       ` Gustavo Padovan
  0 siblings, 1 reply; 26+ messages in thread
From: Andre Guedes @ 2011-07-14 14:31 UTC (permalink / raw)
  To: Andre Guedes, linux-bluetooth

Hi Gustavo,

On Wed, Jul 13, 2011 at 5:15 PM, Gustavo Padovan <padovan@profusion.mobi> wrote:
>> @@ -963,6 +963,11 @@ static inline void hci_cs_inquiry(struct hci_dev *hdev, __u8 status)
>>
>>       set_bit(HCI_INQUIRY, &hdev->flags);
>>
>> +     if (mgmt_has_pending_stop_discov(hdev->id)) {
>
> Isn't a new bit flag log HCI_INQUIRY_CANCEL better that this? First this has
> read a list, second it is a really ugly  function name.

A flag called HCI_CANCEL_DISCOV would be more appropriated since it
would be used to cancel the discovery procedure (inquiry/le scan).

My point is: I don't need a new flag to tell me the same information I already
have by checking for pending stop discovery command. IMO, traversing the
pending command list is not a issue today since it isn't that large.
If that list
becomes too large in future, then we'll have a bigger issue because lots of
mgmt commands do traversing the command pending list. In that case we
would need to come up with some optimization like pending list per hdev,
for instance.

Besides, I don't think mixing up mgmt layer and hci layer logic is a good
idea. The whole discovery procedure logic should go in mgmt layer, so no
discovery logic should be implemented in hci layer.

>> +             mgmt_cancel_discovery(hdev->id);
>
> Just call hci_send_cmd(HCI_OP_INQUIRY_CANCEL)

It would mix up mgmt and hci layer logic.

>
>> +             return;
>> +     }
>> +
>>       mgmt_discovering(hdev->id, 1);
>>  }
>>
>> @@ -1356,7 +1361,12 @@ static inline void hci_inquiry_complete_evt(struct hci_dev *hdev, struct sk_buff
>>       hci_req_complete(hdev, HCI_OP_INQUIRY, status);
>>
>>       mgmt_discovering(hdev->id, 0);
>> +
>> +     hci_dev_lock(hdev);
>> +
>>       mgmt_start_discovery_complete(hdev->id);
>> +
>> +     hci_dev_unlock(hdev);
>>  }
>>
>>  static inline void hci_inquiry_result_evt(struct hci_dev *hdev, struct sk_buff *skb)
>> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
>> index e43940e..bbb0daa 100644
>> --- a/net/bluetooth/mgmt.c
>> +++ b/net/bluetooth/mgmt.c
>> @@ -1670,6 +1670,21 @@ static int cancel_inquiry(struct hci_dev *hdev)
>>       return hci_send_cmd(hdev, HCI_OP_INQUIRY_CANCEL, 0, NULL);
>>  }
>>
>> +int mgmt_cancel_discovery(u16 index)
>> +{
>> +     struct hci_dev *hdev;
>> +     int res = 0;
>> +
>> +     hdev = hci_dev_get(index);
>> +
>> +     if (test_bit(HCI_INQUIRY, &hdev->flags))
>> +             res = cancel_inquiry(hdev);
>> +
>> +     hci_dev_put(hdev);
>> +
>> +     return res;
>> +}
>> +
>>  static int stop_discovery(struct sock *sk, u16 index)
>>  {
>>       struct hci_dev *hdev;
>> @@ -1701,7 +1716,7 @@ static int stop_discovery(struct sock *sk, u16 index)
>>               goto failed;
>>       }
>
> You can check here with mgmt_pending_find() if the HCI_OP_INQUIRY_CANCEL was
> already issued or not. This simplifies code a bit.

Sorry, I didn't see what you mean here.

Regards,

Andre.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 09/16] Bluetooth: Prepare for full support discovery procedures
  2011-07-13 20:26   ` Gustavo Padovan
@ 2011-07-14 14:31     ` Andre Guedes
  0 siblings, 0 replies; 26+ messages in thread
From: Andre Guedes @ 2011-07-14 14:31 UTC (permalink / raw)
  To: Andre Guedes, linux-bluetooth

Hi Gustavo,

On Wed, Jul 13, 2011 at 5:26 PM, Gustavo Padovan <padovan@profusion.mobi> wrote:
>> @@ -202,6 +202,7 @@ enum {
>>
>>  #define LMP_EV4              0x01
>>  #define LMP_EV5              0x02
>> +#define LMP_NO_BREDR 0x20
>>  #define LMP_LE               0x40
>>
>>  #define LMP_SNIFF_SUBR       0x02
>> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
>> index 1ff59f2..fc75c61 100644
>> --- a/include/net/bluetooth/hci_core.h
>> +++ b/include/net/bluetooth/hci_core.h
>> @@ -597,6 +597,7 @@ void hci_conn_del_sysfs(struct hci_conn *conn);
>>  #define lmp_esco_capable(dev)      ((dev)->features[3] & LMP_ESCO)
>>  #define lmp_ssp_capable(dev)       ((dev)->features[6] & LMP_SIMPLE_PAIR)
>>  #define lmp_no_flush_capable(dev)  ((dev)->features[6] & LMP_NO_FLUSH)
>> +#define lmp_no_bredr_capable(dev)  ((dev)->features[4] & LMP_NO_BREDR)
>
> It's more logic call this lmp_bredr_capable() regardless what the bit really
> means.

I agree. I was concerned about having a *_capable macro with different
meaning from the LMP bit. Since you agree too, I'll change this. Thanks.

Andre

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 08/16] Bluetooth: Fix stop_discovery()
  2011-07-14 14:31     ` Andre Guedes
@ 2011-07-22 15:24       ` Gustavo Padovan
  0 siblings, 0 replies; 26+ messages in thread
From: Gustavo Padovan @ 2011-07-22 15:24 UTC (permalink / raw)
  To: Andre Guedes; +Cc: linux-bluetooth

* Andre Guedes <andre.guedes@openbossa.org> [2011-07-14 11:31:01 -0300]:

> Hi Gustavo,
> 
> On Wed, Jul 13, 2011 at 5:15 PM, Gustavo Padovan <padovan@profusion.mobi> wrote:
> >> @@ -963,6 +963,11 @@ static inline void hci_cs_inquiry(struct hci_dev *hdev, __u8 status)
> >>
> >>       set_bit(HCI_INQUIRY, &hdev->flags);
> >>
> >> +     if (mgmt_has_pending_stop_discov(hdev->id)) {
> >
> > Isn't a new bit flag log HCI_INQUIRY_CANCEL better that this? First this has
> > read a list, second it is a really ugly  function name.
> 
> A flag called HCI_CANCEL_DISCOV would be more appropriated since it
> would be used to cancel the discovery procedure (inquiry/le scan).

Let it be HCI_CANCEL_DISCOV then.

> 
> My point is: I don't need a new flag to tell me the same information I already
> have by checking for pending stop discovery command. IMO, traversing the
> pending command list is not a issue today since it isn't that large.
> If that list
> becomes too large in future, then we'll have a bigger issue because lots of
> mgmt commands do traversing the command pending list. In that case we
> would need to come up with some optimization like pending list per hdev,
> for instance.
> 
> Besides, I don't think mixing up mgmt layer and hci layer logic is a good
> idea. The whole discovery procedure logic should go in mgmt layer, so no
> discovery logic should be implemented in hci layer.
> 
> >> +             mgmt_cancel_discovery(hdev->id);
> >
> > Just call hci_send_cmd(HCI_OP_INQUIRY_CANCEL)
> 
> It would mix up mgmt and hci layer logic.

No, the only thing mgmt_cancel_discovery does is call hci_send_cmd. So what
you want is do the same thing but with some more functions in your call chain.

	Gustavo

^ permalink raw reply	[flat|nested] 26+ messages in thread

end of thread, other threads:[~2011-07-22 15:24 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-07-11 21:11 [PATCH 00/16] Full support discovery procedure Andre Guedes
2011-07-11 21:11 ` [PATCH 01/16] Bluetooth: Periodic Inquiry and mgmt discovering event Andre Guedes
2011-07-13 20:21   ` Gustavo Padovan
2011-07-14 14:28     ` Andre Guedes
2011-07-11 21:11 ` [PATCH 02/16] Bluetooth: Add failed/complete functions to discovery commands Andre Guedes
2011-07-11 21:11 ` [PATCH 03/16] Bluetooth: Remove pending " Andre Guedes
2011-07-11 21:11 ` [PATCH 04/16] Bluetooth: Check pending command in start_discovery() Andre Guedes
2011-07-11 21:11 ` [PATCH 05/16] Bluetooth: Check pending commands in stop_discovery() Andre Guedes
2011-07-13 20:20   ` Gustavo Padovan
2011-07-14 14:29     ` Andre Guedes
2011-07-11 21:11 ` [PATCH 06/16] Bluetooth: Create do_inquiry() Andre Guedes
2011-07-11 21:11 ` [PATCH 07/16] Bluetooth: Create cancel_inquiry() Andre Guedes
2011-07-11 21:11 ` [PATCH 08/16] Bluetooth: Fix stop_discovery() Andre Guedes
2011-07-13 20:15   ` Gustavo Padovan
2011-07-14 14:31     ` Andre Guedes
2011-07-22 15:24       ` Gustavo Padovan
2011-07-11 21:11 ` [PATCH 09/16] Bluetooth: Prepare for full support discovery procedures Andre Guedes
2011-07-13 20:26   ` Gustavo Padovan
2011-07-14 14:31     ` Andre Guedes
2011-07-11 21:11 ` [PATCH 10/16] Bluetooth: Check 'dev_class' in mgmt_device_found() Andre Guedes
2011-07-11 21:11 ` [PATCH 11/16] Bluetooth: Add 'eir_len' param to mgmt_device_found() Andre Guedes
2011-07-11 21:11 ` [PATCH 12/16] Bluetooth: Report LE devices Andre Guedes
2011-07-11 21:11 ` [PATCH 13/16] Bluetooth: Add 'le_scan_timer' to struct hci_dev Andre Guedes
2011-07-11 21:11 ` [PATCH 14/16] Bluetooth: Add LE Scan helper functions Andre Guedes
2011-07-11 21:11 ` [PATCH 15/16] Bluetooth: Support LE-Only discovery procedure Andre Guedes
2011-07-11 21:11 ` [PATCH 16/16] Bluetooth: Support BR/EDR/LE " Andre Guedes

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.