[PATCH v3 0/6] MGMT Start Discovery command LE-Only Support

[PATCH v3 0/6] MGMT Start Discovery command LE-Only Support

[Andre Guedes]

Hi all,

This patch series is basically a rebased version of the previous
series.

Patches 2 and 3 define functions used in patch 4 so these patches
should be applied together to avoid compiler warnings.

BR,

Andre

Andre Guedes (6):
  Bluetooth: LE scan should send Discovering events
  Bluetooth: Add helper functions to send LE scan commands
  Bluetooth: Add hci_do_le_scan()
  Bluetooth: Add hci_le_scan()
  Bluetooth: MGMT start discovery LE-Only support
  Bluetooth: Fix indentation

 include/net/bluetooth/hci_core.h |   18 ++++++
 net/bluetooth/hci_core.c         |  116 ++++++++++++++++++++++++++++++++++++++
 net/bluetooth/hci_event.c        |   31 +++++++++-
 net/bluetooth/mgmt.c             |   22 +++++++-
 4 files changed, 182 insertions(+), 5 deletions(-)

-- 
1.7.8.4

[PATCH v3 1/6] Bluetooth: LE scan should send Discovering events

[Andre Guedes]

Send MGMT Discovering events once LE scan starts/stops so the
userspace can track when local adapters are discovering LE devices.

This way, we also keep the same behavior of inquiry which sends MGMT
Discovering events once inquiry starts/stops even if it is triggered
by an external tool (e.g. hcitool).

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

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 25f449f..4e569d8 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -61,6 +61,7 @@ struct discovery_state {
 		DISCOVERY_STOPPED,
 		DISCOVERY_STARTING,
 		DISCOVERY_INQUIRY,
+		DISCOVERY_LE_SCAN,
 		DISCOVERY_RESOLVING,
 		DISCOVERY_STOPPING,
 	} state;
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 91166db..fd22035 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -361,6 +361,7 @@ bool hci_discovery_active(struct hci_dev *hdev)
 	struct discovery_state *discov = &hdev->discovery;
 
 	if (discov->state == DISCOVERY_INQUIRY ||
+					discov->state == DISCOVERY_LE_SCAN ||
 					discov->state == DISCOVERY_RESOLVING)
 		return true;
 
@@ -381,6 +382,7 @@ void hci_discovery_set_state(struct hci_dev *hdev, int state)
 	case DISCOVERY_STARTING:
 		break;
 	case DISCOVERY_INQUIRY:
+	case DISCOVERY_LE_SCAN:
 		mgmt_discovering(hdev, 1);
 		break;
 	case DISCOVERY_RESOLVING:
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 6fb9016..dd8056c 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -1055,12 +1055,17 @@ static void hci_cc_le_set_scan_enable(struct hci_dev *hdev,
 
 		hci_dev_lock(hdev);
 		hci_adv_entries_clear(hdev);
+		hci_discovery_set_state(hdev, DISCOVERY_LE_SCAN);
 		hci_dev_unlock(hdev);
 		break;
 
 	case LE_SCANNING_DISABLED:
 		clear_bit(HCI_LE_SCAN, &hdev->dev_flags);
 
+		hci_dev_lock(hdev);
+		hci_discovery_set_state(hdev, DISCOVERY_STOPPED);
+		hci_dev_unlock(hdev);
+
 		schedule_delayed_work(&hdev->adv_work, ADV_CLEAR_TIMEOUT);
 		break;
 
-- 
1.7.8.4

[PATCH v3 2/6] Bluetooth: Add helper functions to send LE scan commands

[Andre Guedes]

This patch creates two helper functions to send LE Set Scan
Parameters and LE Set Scan Enable commands.

Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
Acked-by: Marcel Holtmann <marcel@holtmann.org>
---
 net/bluetooth/hci_core.c |   23 +++++++++++++++++++++++
 1 files changed, 23 insertions(+), 0 deletions(-)

diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index fd22035..4830995 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -89,6 +89,29 @@ static void hci_notify(struct hci_dev *hdev, int event)
 	atomic_notifier_call_chain(&hci_notifier, event, hdev);
 }
 
+static int send_le_scan_param_cmd(struct hci_dev *hdev, u8 type, u16 interval,
+								u16 window)
+{
+	struct hci_cp_le_set_scan_param cp;
+
+	memset(&cp, 0, sizeof(cp));
+	cp.type = type;
+	cp.interval = cpu_to_le16(interval);
+	cp.window = cpu_to_le16(window);
+
+	return hci_send_cmd(hdev, HCI_OP_LE_SET_SCAN_PARAM, sizeof(cp), &cp);
+}
+
+static int send_le_scan_enable_cmd(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);
+}
+
 /* ---- HCI requests ---- */
 
 void hci_req_complete(struct hci_dev *hdev, __u16 cmd, int result)
-- 
1.7.8.4

[PATCH v3 3/6] Bluetooth: Add hci_do_le_scan()

[Andre Guedes]

This patch adds to hci_core the hci_do_le_scan function which
should be used to scan LE devices.

In order to enable LE scan, hci_do_le_scan() sends commands (Set
LE Scan Parameters and Set LE Scan Enable) to the controller and
waits for its results. If commands were executed successfully a
delayed work is scheduled to disable the ongoing scanning after
some amount of time. This function blocks.

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

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 4e569d8..d157762 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -263,6 +263,11 @@ struct hci_dev {
 
 	unsigned long		dev_flags;
 
+	struct delayed_work	le_scan_disable;
+
+	wait_queue_head_t	le_scan_wait_q;
+	u8			le_scan_result;
+
 	int (*open)(struct hci_dev *hdev);
 	int (*close)(struct hci_dev *hdev);
 	int (*flush)(struct hci_dev *hdev);
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 4830995..22abc37 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -779,6 +779,8 @@ static int hci_dev_do_close(struct hci_dev *hdev)
 	if (test_and_clear_bit(HCI_SERVICE_CACHE, &hdev->dev_flags))
 		cancel_delayed_work(&hdev->service_cache);
 
+	cancel_delayed_work_sync(&hdev->le_scan_disable);
+
 	hci_dev_lock(hdev);
 	inquiry_cache_flush(hdev);
 	hci_conn_hash_flush(hdev);
@@ -1593,6 +1595,57 @@ int hci_add_adv_entry(struct hci_dev *hdev,
 	return 0;
 }
 
+static int hci_do_le_scan(struct hci_dev *hdev, u8 type, u16 interval,
+						u16 window, int timeout)
+{
+	long timeo = msecs_to_jiffies(3000);
+	DECLARE_WAITQUEUE(wait, current);
+
+	BT_DBG("%s", hdev->name);
+
+	if (test_bit(HCI_LE_SCAN, &hdev->dev_flags))
+		return -EINPROGRESS;
+
+	add_wait_queue(&hdev->le_scan_wait_q, &wait);
+
+	/* Send LE Set Scan Parameter command and wait for the result */
+	hdev->le_scan_result = -ETIMEDOUT;
+	send_le_scan_param_cmd(hdev, type, interval, window);
+
+	schedule_timeout_uninterruptible(timeo);
+	if (hdev->le_scan_result)
+		goto failed;
+
+	/* Send LE Set Scan Enable command and wait for the result */
+	hdev->le_scan_result = -ETIMEDOUT;
+	send_le_scan_enable_cmd(hdev, 1);
+
+	schedule_timeout_uninterruptible(timeo);
+	if (hdev->le_scan_result)
+		goto failed;
+
+	remove_wait_queue(&hdev->le_scan_wait_q, &wait);
+
+	schedule_delayed_work(&hdev->le_scan_disable,
+						msecs_to_jiffies(timeout));
+
+	return 0;
+
+failed:
+	remove_wait_queue(&hdev->le_scan_wait_q, &wait);
+	return hdev->le_scan_result;
+}
+
+static void le_scan_disable_work(struct work_struct *work)
+{
+	struct hci_dev *hdev = container_of(work, struct hci_dev,
+						le_scan_disable.work);
+
+	BT_DBG("%s", hdev->name);
+
+	send_le_scan_enable_cmd(hdev, 0);
+}
+
 /* Register HCI device */
 int hci_register_dev(struct hci_dev *hdev)
 {
@@ -1678,6 +1731,10 @@ int hci_register_dev(struct hci_dev *hdev)
 
 	atomic_set(&hdev->promisc, 0);
 
+	INIT_DELAYED_WORK(&hdev->le_scan_disable, le_scan_disable_work);
+
+	init_waitqueue_head(&hdev->le_scan_wait_q);
+
 	write_unlock(&hci_dev_list_lock);
 
 	hdev->workqueue = alloc_workqueue(hdev->name, WQ_HIGHPRI | WQ_UNBOUND |
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index dd8056c..856ab35 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -1030,6 +1030,9 @@ 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%x", hdev->name, status);
+
+	hdev->le_scan_result = -bt_to_errno(status);
+	wake_up(&hdev->le_scan_wait_q);
 }
 
 static void hci_cc_le_set_scan_enable(struct hci_dev *hdev,
@@ -1040,15 +1043,18 @@ 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;
 
 	switch (cp->enable) {
 	case LE_SCANNING_ENABLED:
+		hdev->le_scan_result = -bt_to_errno(status);
+		wake_up(&hdev->le_scan_wait_q);
+
+		if (status)
+			return;
+
 		set_bit(HCI_LE_SCAN, &hdev->dev_flags);
 
 		cancel_delayed_work_sync(&hdev->adv_work);
@@ -1060,6 +1066,9 @@ static void hci_cc_le_set_scan_enable(struct hci_dev *hdev,
 		break;
 
 	case LE_SCANNING_DISABLED:
+		if (status)
+			return;
+
 		clear_bit(HCI_LE_SCAN, &hdev->dev_flags);
 
 		hci_dev_lock(hdev);
-- 
1.7.8.4

[PATCH v3 4/6] Bluetooth: Add hci_le_scan()

[Andre Guedes]

We are not supposed to block in start_discovery() because
start_discovery code is running in write() syscall context
and this would block the write operation on the mgmt socket.
This way, we cannot directly call hci_do_le_scan() to scan
LE devices in start_discovery(). To overcome this issue a
derefered work (hdev->le_scan) was created so we can properly
call hci_do_le_scan().

The helper function hci_le_scan() simply set LE scan parameters
and queue hdev->le_scan work. The work is queued on system_long_wq
since it can sleep for a few seconds in the worst case (timeout).

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

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index d157762..8da5eac 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -126,6 +126,13 @@ struct adv_entry {
 	u8 bdaddr_type;
 };
 
+struct le_scan_params {
+	u8 type;
+	u16 interval;
+	u16 window;
+	int timeout;
+};
+
 #define NUM_REASSEMBLY 4
 struct hci_dev {
 	struct list_head list;
@@ -268,6 +275,9 @@ struct hci_dev {
 	wait_queue_head_t	le_scan_wait_q;
 	u8			le_scan_result;
 
+	struct work_struct	le_scan;
+	struct le_scan_params	le_scan_params;
+
 	int (*open)(struct hci_dev *hdev);
 	int (*close)(struct hci_dev *hdev);
 	int (*flush)(struct hci_dev *hdev);
@@ -1027,5 +1037,7 @@ void hci_le_ltk_neg_reply(struct hci_conn *conn);
 
 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);
 
 #endif /* __HCI_CORE_H */
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 22abc37..d77e586 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -779,6 +779,7 @@ static int hci_dev_do_close(struct hci_dev *hdev)
 	if (test_and_clear_bit(HCI_SERVICE_CACHE, &hdev->dev_flags))
 		cancel_delayed_work(&hdev->service_cache);
 
+	cancel_work_sync(&hdev->le_scan);
 	cancel_delayed_work_sync(&hdev->le_scan_disable);
 
 	hci_dev_lock(hdev);
@@ -1646,6 +1647,37 @@ static void le_scan_disable_work(struct work_struct *work)
 	send_le_scan_enable_cmd(hdev, 0);
 }
 
+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 *params = &hdev->le_scan_params;
+
+	BT_DBG("%s", hdev->name);
+
+	hci_do_le_scan(hdev, params->type, params->interval,
+					params->window, params->timeout);
+}
+
+int hci_le_scan(struct hci_dev *hdev, u8 type, u16 interval, u16 window,
+								int timeout)
+{
+	struct le_scan_params *params = &hdev->le_scan_params;
+
+	BT_DBG("%s", hdev->name);
+
+	if (work_busy(&hdev->le_scan))
+		return -EINPROGRESS;
+
+	params->type = type;
+	params->interval = interval;
+	params->window = window;
+	params->timeout = timeout;
+
+	queue_work(system_long_wq, &hdev->le_scan);
+
+	return 0;
+}
+
 /* Register HCI device */
 int hci_register_dev(struct hci_dev *hdev)
 {
@@ -1731,6 +1763,8 @@ int hci_register_dev(struct hci_dev *hdev)
 
 	atomic_set(&hdev->promisc, 0);
 
+	INIT_WORK(&hdev->le_scan, le_scan_work);
+
 	INIT_DELAYED_WORK(&hdev->le_scan_disable, le_scan_disable_work);
 
 	init_waitqueue_head(&hdev->le_scan_wait_q);
-- 
1.7.8.4

[PATCH v3 5/6] Bluetooth: MGMT start discovery LE-Only support

[Andre Guedes]

This patch adds LE-Only discovery procedure support to MGMT Start
Discovery command.

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

diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 856ab35..fb2497b 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -1033,6 +1033,13 @@ static void hci_cc_le_set_scan_param(struct hci_dev *hdev, struct sk_buff *skb)
 
 	hdev->le_scan_result = -bt_to_errno(status);
 	wake_up(&hdev->le_scan_wait_q);
+
+	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,
@@ -1052,8 +1059,12 @@ static void hci_cc_le_set_scan_enable(struct hci_dev *hdev,
 		hdev->le_scan_result = -bt_to_errno(status);
 		wake_up(&hdev->le_scan_wait_q);
 
-		if (status)
+		if (status) {
+			hci_dev_lock(hdev);
+			mgmt_start_discovery_failed(hdev, status);
+			hci_dev_unlock(hdev);
 			return;
+		}
 
 		set_bit(HCI_LE_SCAN, &hdev->dev_flags);
 
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 8970799..0a1fa4c 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -35,6 +35,15 @@
 #define MGMT_VERSION	0
 #define MGMT_REVISION	1
 
+/*
+ * These LE scan and inquiry parameters were chosen according to LE General
+ * Discovery Procedure specification.
+ */
+#define LE_SCAN_TYPE			0x01
+#define LE_SCAN_WIN			0x12
+#define LE_SCAN_INT			0x12
+#define LE_SCAN_TIMEOUT_LE_ONLY		10240	/* TGAP(gen_disc_scan_min) */
+
 #define INQUIRY_LEN_BREDR 0x08 /* TGAP(100) */
 
 #define SERVICE_CACHE_TIMEOUT (5 * 1000)
@@ -1897,6 +1906,7 @@ static int start_discovery(struct sock *sk, u16 index,
 						unsigned char *data, u16 len)
 {
 	struct mgmt_cp_start_discovery *cp = (void *) data;
+	unsigned long discov_type = cp->type;
 	struct pending_cmd *cmd;
 	struct hci_dev *hdev;
 	int err;
@@ -1932,7 +1942,15 @@ static int start_discovery(struct sock *sk, u16 index,
 		goto failed;
 	}
 
-	err = hci_do_inquiry(hdev, INQUIRY_LEN_BREDR);
+	if (test_bit(MGMT_ADDR_BREDR, &discov_type))
+		err = hci_do_inquiry(hdev, INQUIRY_LEN_BREDR);
+	else if (test_bit(MGMT_ADDR_LE_PUBLIC, &discov_type) &&
+				test_bit(MGMT_ADDR_LE_RANDOM, &discov_type))
+		err = hci_le_scan(hdev, LE_SCAN_TYPE, LE_SCAN_INT,
+					LE_SCAN_WIN, LE_SCAN_TIMEOUT_LE_ONLY);
+	else
+		err = -EINVAL;
+
 	if (err < 0)
 		mgmt_pending_remove(cmd);
 	else
-- 
1.7.8.4

[PATCH v3 6/6] Bluetooth: Fix indentation

[Andre Guedes]

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

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 0a1fa4c..e7033c1 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -44,7 +44,7 @@
 #define LE_SCAN_INT			0x12
 #define LE_SCAN_TIMEOUT_LE_ONLY		10240	/* TGAP(gen_disc_scan_min) */
 
-#define INQUIRY_LEN_BREDR 0x08 /* TGAP(100) */
+#define INQUIRY_LEN_BREDR		0x08	/* TGAP(100) */
 
 #define SERVICE_CACHE_TIMEOUT (5 * 1000)
 
-- 
1.7.8.4

Re: [PATCH v3 5/6] Bluetooth: MGMT start discovery LE-Only support

[Anderson Lizardo]

Hi Andre,

On Wed, Jan 25, 2012 at 6:52 PM, Andre Guedes
<andre.guedes@openbossa.org> wrote:
> - =A0 =A0 =A0 err =3D hci_do_inquiry(hdev, INQUIRY_LEN_BREDR);
> + =A0 =A0 =A0 if (test_bit(MGMT_ADDR_BREDR, &discov_type))
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 err =3D hci_do_inquiry(hdev, INQUIRY_LEN_BR=
EDR);
> + =A0 =A0 =A0 else if (test_bit(MGMT_ADDR_LE_PUBLIC, &discov_type) &&
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 test_bit(MG=
MT_ADDR_LE_RANDOM, &discov_type))

Should it be "||" in the line above?

> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 err =3D hci_le_scan(hdev, LE_SCAN_TYPE, LE_=
SCAN_INT,
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=
 =A0 LE_SCAN_WIN, LE_SCAN_TIMEOUT_LE_ONLY);
> + =A0 =A0 =A0 else
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 err =3D -EINVAL;
> +
> =A0 =A0 =A0 =A0if (err < 0)
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0mgmt_pending_remove(cmd);
> =A0 =A0 =A0 =A0else
> --
> 1.7.8.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth=
" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at =A0http://vger.kernel.org/majordomo-info.html


Regards,
--=20
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil

Re: [PATCH v3 5/6] Bluetooth: MGMT start discovery LE-Only support

[Andre Guedes]

Hi Lizardo,

On Thu, Jan 26, 2012 at 9:55 AM, Anderson Lizardo
<anderson.lizardo@openbossa.org> wrote:
> Hi Andre,
>
> On Wed, Jan 25, 2012 at 6:52 PM, Andre Guedes
> <andre.guedes@openbossa.org> wrote:
>> - =A0 =A0 =A0 err =3D hci_do_inquiry(hdev, INQUIRY_LEN_BREDR);
>> + =A0 =A0 =A0 if (test_bit(MGMT_ADDR_BREDR, &discov_type))
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 err =3D hci_do_inquiry(hdev, INQUIRY_LEN_B=
REDR);
>> + =A0 =A0 =A0 else if (test_bit(MGMT_ADDR_LE_PUBLIC, &discov_type) &&
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 test_bit(M=
GMT_ADDR_LE_RANDOM, &discov_type))
>
> Should it be "||" in the line above?

mgmt-api.txt says both bits should be set.

>
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 err =3D hci_le_scan(hdev, LE_SCAN_TYPE, LE=
_SCAN_INT,
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0 LE_SCAN_WIN, LE_SCAN_TIMEOUT_LE_ONLY);
>> + =A0 =A0 =A0 else
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 err =3D -EINVAL;
>> +
>> =A0 =A0 =A0 =A0if (err < 0)
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0mgmt_pending_remove(cmd);
>> =A0 =A0 =A0 =A0else
>> --
>> 1.7.8.4
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-bluetoot=
h" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at =A0http://vger.kernel.org/majordomo-info.html
>
>
> Regards,
> --
> Anderson Lizardo
> Instituto Nokia de Tecnologia - INdT
> Manaus - Brazil

Re: [PATCH v3 1/6] Bluetooth: LE scan should send Discovering events

[Marcel Holtmann]

Hi Andre,

> Send MGMT Discovering events once LE scan starts/stops so the
> userspace can track when local adapters are discovering LE devices.
> 
> This way, we also keep the same behavior of inquiry which sends MGMT
> Discovering events once inquiry starts/stops even if it is triggered
> by an external tool (e.g. hcitool).
> 
> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
> ---
>  include/net/bluetooth/hci_core.h |    1 +
>  net/bluetooth/hci_core.c         |    2 ++
>  net/bluetooth/hci_event.c        |    5 +++++
>  3 files changed, 8 insertions(+), 0 deletions(-)
> 
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index 25f449f..4e569d8 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -61,6 +61,7 @@ struct discovery_state {
>  		DISCOVERY_STOPPED,
>  		DISCOVERY_STARTING,
>  		DISCOVERY_INQUIRY,
> +		DISCOVERY_LE_SCAN,
>  		DISCOVERY_RESOLVING,
>  		DISCOVERY_STOPPING,
>  	} state;
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index 91166db..fd22035 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -361,6 +361,7 @@ bool hci_discovery_active(struct hci_dev *hdev)
>  	struct discovery_state *discov = &hdev->discovery;
>  
>  	if (discov->state == DISCOVERY_INQUIRY ||
> +					discov->state == DISCOVERY_LE_SCAN ||
>  					discov->state == DISCOVERY_RESOLVING)
>  		return true;

I think we need to start using a switch statement here.

> @@ -381,6 +382,7 @@ void hci_discovery_set_state(struct hci_dev *hdev, int state)
>  	case DISCOVERY_STARTING:
>  		break;
>  	case DISCOVERY_INQUIRY:
> +	case DISCOVERY_LE_SCAN:
>  		mgmt_discovering(hdev, 1);
>  		break;
>  	case DISCOVERY_RESOLVING:
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index 6fb9016..dd8056c 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -1055,12 +1055,17 @@ static void hci_cc_le_set_scan_enable(struct hci_dev *hdev,
>  
>  		hci_dev_lock(hdev);
>  		hci_adv_entries_clear(hdev);
> +		hci_discovery_set_state(hdev, DISCOVERY_LE_SCAN);
>  		hci_dev_unlock(hdev);
>  		break;
>  
>  	case LE_SCANNING_DISABLED:
>  		clear_bit(HCI_LE_SCAN, &hdev->dev_flags);
>  
> +		hci_dev_lock(hdev);
> +		hci_discovery_set_state(hdev, DISCOVERY_STOPPED);
> +		hci_dev_unlock(hdev);
> +
>  		schedule_delayed_work(&hdev->adv_work, ADV_CLEAR_TIMEOUT);
>  		break;
>  

Otherwise this looks fine.

Acked-by: Marcel Holtmann <marcel@holtmann.org>

Regards

Marcel

Re: [PATCH v3 3/6] Bluetooth: Add hci_do_le_scan()

[Marcel Holtmann]

Hi Andre,

> This patch adds to hci_core the hci_do_le_scan function which
> should be used to scan LE devices.
> 
> In order to enable LE scan, hci_do_le_scan() sends commands (Set
> LE Scan Parameters and Set LE Scan Enable) to the controller and
> waits for its results. If commands were executed successfully a
> delayed work is scheduled to disable the ongoing scanning after
> some amount of time. This function blocks.
> 
> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
> ---
>  include/net/bluetooth/hci_core.h |    5 +++
>  net/bluetooth/hci_core.c         |   57 ++++++++++++++++++++++++++++++++++++++
>  net/bluetooth/hci_event.c        |   15 ++++++++--
>  3 files changed, 74 insertions(+), 3 deletions(-)
> 
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index 4e569d8..d157762 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -263,6 +263,11 @@ struct hci_dev {
>  
>  	unsigned long		dev_flags;
>  
> +	struct delayed_work	le_scan_disable;
> +
> +	wait_queue_head_t	le_scan_wait_q;
> +	u8			le_scan_result;
> +
>  	int (*open)(struct hci_dev *hdev);
>  	int (*close)(struct hci_dev *hdev);
>  	int (*flush)(struct hci_dev *hdev);
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index 4830995..22abc37 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -779,6 +779,8 @@ static int hci_dev_do_close(struct hci_dev *hdev)
>  	if (test_and_clear_bit(HCI_SERVICE_CACHE, &hdev->dev_flags))
>  		cancel_delayed_work(&hdev->service_cache);
>  
> +	cancel_delayed_work_sync(&hdev->le_scan_disable);
> +
>  	hci_dev_lock(hdev);
>  	inquiry_cache_flush(hdev);
>  	hci_conn_hash_flush(hdev);
> @@ -1593,6 +1595,57 @@ int hci_add_adv_entry(struct hci_dev *hdev,
>  	return 0;
>  }
>  
> +static int hci_do_le_scan(struct hci_dev *hdev, u8 type, u16 interval,
> +						u16 window, int timeout)
> +{
> +	long timeo = msecs_to_jiffies(3000);
> +	DECLARE_WAITQUEUE(wait, current);
> +
> +	BT_DBG("%s", hdev->name);
> +
> +	if (test_bit(HCI_LE_SCAN, &hdev->dev_flags))
> +		return -EINPROGRESS;
> +
> +	add_wait_queue(&hdev->le_scan_wait_q, &wait);
> +
> +	/* Send LE Set Scan Parameter command and wait for the result */
> +	hdev->le_scan_result = -ETIMEDOUT;
> +	send_le_scan_param_cmd(hdev, type, interval, window);
> +
> +	schedule_timeout_uninterruptible(timeo);
> +	if (hdev->le_scan_result)
> +		goto failed;
> +
> +	/* Send LE Set Scan Enable command and wait for the result */
> +	hdev->le_scan_result = -ETIMEDOUT;
> +	send_le_scan_enable_cmd(hdev, 1);
> +
> +	schedule_timeout_uninterruptible(timeo);
> +	if (hdev->le_scan_result)
> +		goto failed;
> +
> +	remove_wait_queue(&hdev->le_scan_wait_q, &wait);
> +
> +	schedule_delayed_work(&hdev->le_scan_disable,
> +						msecs_to_jiffies(timeout));

we need to make this generic. I know that Andrei will need the same for
A2MP and we already do a similar thing with hci_request. If we start
spreading the hand-coded versions, this gets messy really quick.

So essentially we need a generic HCI command execution framework that
will actually sleep. And it needs to be versatile enough to handle all
our cases.

Regards

Marcel

Re: [PATCH v3 4/6] Bluetooth: Add hci_le_scan()

[Marcel Holtmann]

Hi Andre,

> We are not supposed to block in start_discovery() because
> start_discovery code is running in write() syscall context
> and this would block the write operation on the mgmt socket.
> This way, we cannot directly call hci_do_le_scan() to scan
> LE devices in start_discovery(). To overcome this issue a
> derefered work (hdev->le_scan) was created so we can properly
> call hci_do_le_scan().
> 
> The helper function hci_le_scan() simply set LE scan parameters
> and queue hdev->le_scan work. The work is queued on system_long_wq
> since it can sleep for a few seconds in the worst case (timeout).
> 
> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
> ---
>  include/net/bluetooth/hci_core.h |   12 ++++++++++++
>  net/bluetooth/hci_core.c         |   34 ++++++++++++++++++++++++++++++++++
>  2 files changed, 46 insertions(+), 0 deletions(-)

so I am fine with this one, but we might need to make this one generic
as well (if possible).

Acked-by: Marcel Holtmann <marcel@holtmann.org>

Regards

Marcel

Re: [PATCH v3 5/6] Bluetooth: MGMT start discovery LE-Only support

[Marcel Holtmann]

Hi Andre,

> This patch adds LE-Only discovery procedure support to MGMT Start
> Discovery command.
> 
> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
> ---
>  net/bluetooth/hci_event.c |   13 ++++++++++++-
>  net/bluetooth/mgmt.c      |   20 +++++++++++++++++++-
>  2 files changed, 31 insertions(+), 2 deletions(-)
> 
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index 856ab35..fb2497b 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -1033,6 +1033,13 @@ static void hci_cc_le_set_scan_param(struct hci_dev *hdev, struct sk_buff *skb)
>  
>  	hdev->le_scan_result = -bt_to_errno(status);
>  	wake_up(&hdev->le_scan_wait_q);
> +
> +	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,
> @@ -1052,8 +1059,12 @@ static void hci_cc_le_set_scan_enable(struct hci_dev *hdev,
>  		hdev->le_scan_result = -bt_to_errno(status);
>  		wake_up(&hdev->le_scan_wait_q);
>  
> -		if (status)
> +		if (status) {
> +			hci_dev_lock(hdev);
> +			mgmt_start_discovery_failed(hdev, status);
> +			hci_dev_unlock(hdev);
>  			return;
> +		}

are these two related to the other changes or just actual bug fixes?

>  
>  		set_bit(HCI_LE_SCAN, &hdev->dev_flags);
>  
> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> index 8970799..0a1fa4c 100644
> --- a/net/bluetooth/mgmt.c
> +++ b/net/bluetooth/mgmt.c
> @@ -35,6 +35,15 @@
>  #define MGMT_VERSION	0
>  #define MGMT_REVISION	1
>  
> +/*
> + * These LE scan and inquiry parameters were chosen according to LE General
> + * Discovery Procedure specification.
> + */
> +#define LE_SCAN_TYPE			0x01
> +#define LE_SCAN_WIN			0x12
> +#define LE_SCAN_INT			0x12
> +#define LE_SCAN_TIMEOUT_LE_ONLY		10240	/* TGAP(gen_disc_scan_min) */
> +
>  #define INQUIRY_LEN_BREDR 0x08 /* TGAP(100) */
>  
>  #define SERVICE_CACHE_TIMEOUT (5 * 1000)
> @@ -1897,6 +1906,7 @@ static int start_discovery(struct sock *sk, u16 index,
>  						unsigned char *data, u16 len)
>  {
>  	struct mgmt_cp_start_discovery *cp = (void *) data;
> +	unsigned long discov_type = cp->type;
>  	struct pending_cmd *cmd;
>  	struct hci_dev *hdev;
>  	int err;
> @@ -1932,7 +1942,15 @@ static int start_discovery(struct sock *sk, u16 index,
>  		goto failed;
>  	}
>  
> -	err = hci_do_inquiry(hdev, INQUIRY_LEN_BREDR);
> +	if (test_bit(MGMT_ADDR_BREDR, &discov_type))
> +		err = hci_do_inquiry(hdev, INQUIRY_LEN_BREDR);
> +	else if (test_bit(MGMT_ADDR_LE_PUBLIC, &discov_type) &&
> +				test_bit(MGMT_ADDR_LE_RANDOM, &discov_type))

so test_bit can not check for two bits at the same time?

> +		err = hci_le_scan(hdev, LE_SCAN_TYPE, LE_SCAN_INT,
> +					LE_SCAN_WIN, LE_SCAN_TIMEOUT_LE_ONLY);
> +	else
> +		err = -EINVAL;
> +
>  	if (err < 0)
>  		mgmt_pending_remove(cmd);
>  	else

Regards

Marcel

Re: [PATCH v3 6/6] Bluetooth: Fix indentation

[Marcel Holtmann]

Hi Andre,

please fix the commit message to at least mention where.

> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
> ---
>  net/bluetooth/mgmt.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> index 0a1fa4c..e7033c1 100644
> --- a/net/bluetooth/mgmt.c
> +++ b/net/bluetooth/mgmt.c
> @@ -44,7 +44,7 @@
>  #define LE_SCAN_INT			0x12
>  #define LE_SCAN_TIMEOUT_LE_ONLY		10240	/* TGAP(gen_disc_scan_min) */
>  
> -#define INQUIRY_LEN_BREDR 0x08 /* TGAP(100) */
> +#define INQUIRY_LEN_BREDR		0x08	/* TGAP(100) */
>  
>  #define SERVICE_CACHE_TIMEOUT (5 * 1000)
>  

Otherwise this is fine.

Acked-by: Marcel Holtmann <marcel@holtmann.org>

Regards

Marcel

Re: [PATCH v3 3/6] Bluetooth: Add hci_do_le_scan()

[Andre Guedes]

Hi Marcel,

On Mon, Jan 30, 2012 at 6:38 PM, Marcel Holtmann <marcel@holtmann.org> wrot=
e:
> Hi Andre,
>
>> This patch adds to hci_core the hci_do_le_scan function which
>> should be used to scan LE devices.
>>
>> In order to enable LE scan, hci_do_le_scan() sends commands (Set
>> LE Scan Parameters and Set LE Scan Enable) to the controller and
>> waits for its results. If commands were executed successfully a
>> delayed work is scheduled to disable the ongoing scanning after
>> some amount of time. This function blocks.
>>
>> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
>> ---
>> =A0include/net/bluetooth/hci_core.h | =A0 =A05 +++
>> =A0net/bluetooth/hci_core.c =A0 =A0 =A0 =A0 | =A0 57 +++++++++++++++++++=
+++++++++++++++++++
>> =A0net/bluetooth/hci_event.c =A0 =A0 =A0 =A0| =A0 15 ++++++++--
>> =A03 files changed, 74 insertions(+), 3 deletions(-)
>>
>> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hc=
i_core.h
>> index 4e569d8..d157762 100644
>> --- a/include/net/bluetooth/hci_core.h
>> +++ b/include/net/bluetooth/hci_core.h
>> @@ -263,6 +263,11 @@ struct hci_dev {
>>
>> =A0 =A0 =A0 unsigned long =A0 =A0 =A0 =A0 =A0 dev_flags;
>>
>> + =A0 =A0 struct delayed_work =A0 =A0 le_scan_disable;
>> +
>> + =A0 =A0 wait_queue_head_t =A0 =A0 =A0 le_scan_wait_q;
>> + =A0 =A0 u8 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0le_scan_result;
>> +
>> =A0 =A0 =A0 int (*open)(struct hci_dev *hdev);
>> =A0 =A0 =A0 int (*close)(struct hci_dev *hdev);
>> =A0 =A0 =A0 int (*flush)(struct hci_dev *hdev);
>> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
>> index 4830995..22abc37 100644
>> --- a/net/bluetooth/hci_core.c
>> +++ b/net/bluetooth/hci_core.c
>> @@ -779,6 +779,8 @@ static int hci_dev_do_close(struct hci_dev *hdev)
>> =A0 =A0 =A0 if (test_and_clear_bit(HCI_SERVICE_CACHE, &hdev->dev_flags))
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 cancel_delayed_work(&hdev->service_cache);
>>
>> + =A0 =A0 cancel_delayed_work_sync(&hdev->le_scan_disable);
>> +
>> =A0 =A0 =A0 hci_dev_lock(hdev);
>> =A0 =A0 =A0 inquiry_cache_flush(hdev);
>> =A0 =A0 =A0 hci_conn_hash_flush(hdev);
>> @@ -1593,6 +1595,57 @@ int hci_add_adv_entry(struct hci_dev *hdev,
>> =A0 =A0 =A0 return 0;
>> =A0}
>>
>> +static int hci_do_le_scan(struct hci_dev *hdev, u8 type, u16 interval,
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0 =A0 =A0 =A0 u16 window, int timeout)
>> +{
>> + =A0 =A0 long timeo =3D msecs_to_jiffies(3000);
>> + =A0 =A0 DECLARE_WAITQUEUE(wait, current);
>> +
>> + =A0 =A0 BT_DBG("%s", hdev->name);
>> +
>> + =A0 =A0 if (test_bit(HCI_LE_SCAN, &hdev->dev_flags))
>> + =A0 =A0 =A0 =A0 =A0 =A0 return -EINPROGRESS;
>> +
>> + =A0 =A0 add_wait_queue(&hdev->le_scan_wait_q, &wait);
>> +
>> + =A0 =A0 /* Send LE Set Scan Parameter command and wait for the result =
*/
>> + =A0 =A0 hdev->le_scan_result =3D -ETIMEDOUT;
>> + =A0 =A0 send_le_scan_param_cmd(hdev, type, interval, window);
>> +
>> + =A0 =A0 schedule_timeout_uninterruptible(timeo);
>> + =A0 =A0 if (hdev->le_scan_result)
>> + =A0 =A0 =A0 =A0 =A0 =A0 goto failed;
>> +
>> + =A0 =A0 /* Send LE Set Scan Enable command and wait for the result */
>> + =A0 =A0 hdev->le_scan_result =3D -ETIMEDOUT;
>> + =A0 =A0 send_le_scan_enable_cmd(hdev, 1);
>> +
>> + =A0 =A0 schedule_timeout_uninterruptible(timeo);
>> + =A0 =A0 if (hdev->le_scan_result)
>> + =A0 =A0 =A0 =A0 =A0 =A0 goto failed;
>> +
>> + =A0 =A0 remove_wait_queue(&hdev->le_scan_wait_q, &wait);
>> +
>> + =A0 =A0 schedule_delayed_work(&hdev->le_scan_disable,
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0 =A0 =A0 =A0 msecs_to_jiffies(timeout));
>
> we need to make this generic. I know that Andrei will need the same for
> A2MP and we already do a similar thing with hci_request. If we start
> spreading the hand-coded versions, this gets messy really quick.

Yes, I do agree with you about getting messy. So, are you fine with
we use hci_request here too?

Andre

Re: [PATCH v3 3/6] Bluetooth: Add hci_do_le_scan()

[Marcel Holtmann]

Hi Andre,

> >> This patch adds to hci_core the hci_do_le_scan function which
> >> should be used to scan LE devices.
> >>
> >> In order to enable LE scan, hci_do_le_scan() sends commands (Set
> >> LE Scan Parameters and Set LE Scan Enable) to the controller and
> >> waits for its results. If commands were executed successfully a
> >> delayed work is scheduled to disable the ongoing scanning after
> >> some amount of time. This function blocks.
> >>
> >> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
> >> ---
> >>  include/net/bluetooth/hci_core.h |    5 +++
> >>  net/bluetooth/hci_core.c         |   57 ++++++++++++++++++++++++++++++++++++++
> >>  net/bluetooth/hci_event.c        |   15 ++++++++--
> >>  3 files changed, 74 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> >> index 4e569d8..d157762 100644
> >> --- a/include/net/bluetooth/hci_core.h
> >> +++ b/include/net/bluetooth/hci_core.h
> >> @@ -263,6 +263,11 @@ struct hci_dev {
> >>
> >>       unsigned long           dev_flags;
> >>
> >> +     struct delayed_work     le_scan_disable;
> >> +
> >> +     wait_queue_head_t       le_scan_wait_q;
> >> +     u8                      le_scan_result;
> >> +
> >>       int (*open)(struct hci_dev *hdev);
> >>       int (*close)(struct hci_dev *hdev);
> >>       int (*flush)(struct hci_dev *hdev);
> >> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> >> index 4830995..22abc37 100644
> >> --- a/net/bluetooth/hci_core.c
> >> +++ b/net/bluetooth/hci_core.c
> >> @@ -779,6 +779,8 @@ static int hci_dev_do_close(struct hci_dev *hdev)
> >>       if (test_and_clear_bit(HCI_SERVICE_CACHE, &hdev->dev_flags))
> >>               cancel_delayed_work(&hdev->service_cache);
> >>
> >> +     cancel_delayed_work_sync(&hdev->le_scan_disable);
> >> +
> >>       hci_dev_lock(hdev);
> >>       inquiry_cache_flush(hdev);
> >>       hci_conn_hash_flush(hdev);
> >> @@ -1593,6 +1595,57 @@ int hci_add_adv_entry(struct hci_dev *hdev,
> >>       return 0;
> >>  }
> >>
> >> +static int hci_do_le_scan(struct hci_dev *hdev, u8 type, u16 interval,
> >> +                                             u16 window, int timeout)
> >> +{
> >> +     long timeo = msecs_to_jiffies(3000);
> >> +     DECLARE_WAITQUEUE(wait, current);
> >> +
> >> +     BT_DBG("%s", hdev->name);
> >> +
> >> +     if (test_bit(HCI_LE_SCAN, &hdev->dev_flags))
> >> +             return -EINPROGRESS;
> >> +
> >> +     add_wait_queue(&hdev->le_scan_wait_q, &wait);
> >> +
> >> +     /* Send LE Set Scan Parameter command and wait for the result */
> >> +     hdev->le_scan_result = -ETIMEDOUT;
> >> +     send_le_scan_param_cmd(hdev, type, interval, window);
> >> +
> >> +     schedule_timeout_uninterruptible(timeo);
> >> +     if (hdev->le_scan_result)
> >> +             goto failed;
> >> +
> >> +     /* Send LE Set Scan Enable command and wait for the result */
> >> +     hdev->le_scan_result = -ETIMEDOUT;
> >> +     send_le_scan_enable_cmd(hdev, 1);
> >> +
> >> +     schedule_timeout_uninterruptible(timeo);
> >> +     if (hdev->le_scan_result)
> >> +             goto failed;
> >> +
> >> +     remove_wait_queue(&hdev->le_scan_wait_q, &wait);
> >> +
> >> +     schedule_delayed_work(&hdev->le_scan_disable,
> >> +                                             msecs_to_jiffies(timeout));
> >
> > we need to make this generic. I know that Andrei will need the same for
> > A2MP and we already do a similar thing with hci_request. If we start
> > spreading the hand-coded versions, this gets messy really quick.
> 
> Yes, I do agree with you about getting messy. So, are you fine with
> we use hci_request here too?

so hci_request is one command at a time. We have a lock for it that
ensures this. Is this good enough?

Also the core idea of hci_request is fine, but we might wanna make this
a bit better. Currently hci_request is spreading hci_req_complete around
like crazy. I think we need to figure out something smarter if we start
using it more and more.

Regards

Marcel

Re: [PATCH v3 1/6] Bluetooth: LE scan should send Discovering events

[Andre Guedes]

Hi Marcel,

On Mon, Jan 30, 2012 at 6:35 PM, Marcel Holtmann <marcel@holtmann.org> wrote:
> Hi Andre,
>
>> Send MGMT Discovering events once LE scan starts/stops so the
>> userspace can track when local adapters are discovering LE devices.
>>
>> This way, we also keep the same behavior of inquiry which sends MGMT
>> Discovering events once inquiry starts/stops even if it is triggered
>> by an external tool (e.g. hcitool).
>>
>> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
>> ---
>>  include/net/bluetooth/hci_core.h |    1 +
>>  net/bluetooth/hci_core.c         |    2 ++
>>  net/bluetooth/hci_event.c        |    5 +++++
>>  3 files changed, 8 insertions(+), 0 deletions(-)
>>
>> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
>> index 25f449f..4e569d8 100644
>> --- a/include/net/bluetooth/hci_core.h
>> +++ b/include/net/bluetooth/hci_core.h
>> @@ -61,6 +61,7 @@ struct discovery_state {
>>               DISCOVERY_STOPPED,
>>               DISCOVERY_STARTING,
>>               DISCOVERY_INQUIRY,
>> +             DISCOVERY_LE_SCAN,
>>               DISCOVERY_RESOLVING,
>>               DISCOVERY_STOPPING,
>>       } state;
>> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
>> index 91166db..fd22035 100644
>> --- a/net/bluetooth/hci_core.c
>> +++ b/net/bluetooth/hci_core.c
>> @@ -361,6 +361,7 @@ bool hci_discovery_active(struct hci_dev *hdev)
>>       struct discovery_state *discov = &hdev->discovery;
>>
>>       if (discov->state == DISCOVERY_INQUIRY ||
>> +                                     discov->state == DISCOVERY_LE_SCAN ||
>>                                       discov->state == DISCOVERY_RESOLVING)
>>               return true;
>
> I think we need to start using a switch statement here.

Ok, then I'll have a code refactoring patch to handle this separately.

BR,

Andre

Re: [PATCH v3 3/6] Bluetooth: Add hci_do_le_scan()

[Andre Guedes]

Hi Marcel,

On Mon, Jan 30, 2012 at 9:51 PM, Marcel Holtmann <marcel@holtmann.org> wrot=
e:
> Hi Andre,
>
>> >> This patch adds to hci_core the hci_do_le_scan function which
>> >> should be used to scan LE devices.
>> >>
>> >> In order to enable LE scan, hci_do_le_scan() sends commands (Set
>> >> LE Scan Parameters and Set LE Scan Enable) to the controller and
>> >> waits for its results. If commands were executed successfully a
>> >> delayed work is scheduled to disable the ongoing scanning after
>> >> some amount of time. This function blocks.
>> >>
>> >> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
>> >> ---
>> >> =A0include/net/bluetooth/hci_core.h | =A0 =A05 +++
>> >> =A0net/bluetooth/hci_core.c =A0 =A0 =A0 =A0 | =A0 57 ++++++++++++++++=
++++++++++++++++++++++
>> >> =A0net/bluetooth/hci_event.c =A0 =A0 =A0 =A0| =A0 15 ++++++++--
>> >> =A03 files changed, 74 insertions(+), 3 deletions(-)
>> >>
>> >> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth=
/hci_core.h
>> >> index 4e569d8..d157762 100644
>> >> --- a/include/net/bluetooth/hci_core.h
>> >> +++ b/include/net/bluetooth/hci_core.h
>> >> @@ -263,6 +263,11 @@ struct hci_dev {
>> >>
>> >> =A0 =A0 =A0 unsigned long =A0 =A0 =A0 =A0 =A0 dev_flags;
>> >>
>> >> + =A0 =A0 struct delayed_work =A0 =A0 le_scan_disable;
>> >> +
>> >> + =A0 =A0 wait_queue_head_t =A0 =A0 =A0 le_scan_wait_q;
>> >> + =A0 =A0 u8 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0le_scan_resul=
t;
>> >> +
>> >> =A0 =A0 =A0 int (*open)(struct hci_dev *hdev);
>> >> =A0 =A0 =A0 int (*close)(struct hci_dev *hdev);
>> >> =A0 =A0 =A0 int (*flush)(struct hci_dev *hdev);
>> >> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
>> >> index 4830995..22abc37 100644
>> >> --- a/net/bluetooth/hci_core.c
>> >> +++ b/net/bluetooth/hci_core.c
>> >> @@ -779,6 +779,8 @@ static int hci_dev_do_close(struct hci_dev *hdev)
>> >> =A0 =A0 =A0 if (test_and_clear_bit(HCI_SERVICE_CACHE, &hdev->dev_flag=
s))
>> >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 cancel_delayed_work(&hdev->service_cache)=
;
>> >>
>> >> + =A0 =A0 cancel_delayed_work_sync(&hdev->le_scan_disable);
>> >> +
>> >> =A0 =A0 =A0 hci_dev_lock(hdev);
>> >> =A0 =A0 =A0 inquiry_cache_flush(hdev);
>> >> =A0 =A0 =A0 hci_conn_hash_flush(hdev);
>> >> @@ -1593,6 +1595,57 @@ int hci_add_adv_entry(struct hci_dev *hdev,
>> >> =A0 =A0 =A0 return 0;
>> >> =A0}
>> >>
>> >> +static int hci_do_le_scan(struct hci_dev *hdev, u8 type, u16 interva=
l,
>> >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=
 =A0 =A0 =A0 =A0 =A0 u16 window, int timeout)
>> >> +{
>> >> + =A0 =A0 long timeo =3D msecs_to_jiffies(3000);
>> >> + =A0 =A0 DECLARE_WAITQUEUE(wait, current);
>> >> +
>> >> + =A0 =A0 BT_DBG("%s", hdev->name);
>> >> +
>> >> + =A0 =A0 if (test_bit(HCI_LE_SCAN, &hdev->dev_flags))
>> >> + =A0 =A0 =A0 =A0 =A0 =A0 return -EINPROGRESS;
>> >> +
>> >> + =A0 =A0 add_wait_queue(&hdev->le_scan_wait_q, &wait);
>> >> +
>> >> + =A0 =A0 /* Send LE Set Scan Parameter command and wait for the resu=
lt */
>> >> + =A0 =A0 hdev->le_scan_result =3D -ETIMEDOUT;
>> >> + =A0 =A0 send_le_scan_param_cmd(hdev, type, interval, window);
>> >> +
>> >> + =A0 =A0 schedule_timeout_uninterruptible(timeo);
>> >> + =A0 =A0 if (hdev->le_scan_result)
>> >> + =A0 =A0 =A0 =A0 =A0 =A0 goto failed;
>> >> +
>> >> + =A0 =A0 /* Send LE Set Scan Enable command and wait for the result =
*/
>> >> + =A0 =A0 hdev->le_scan_result =3D -ETIMEDOUT;
>> >> + =A0 =A0 send_le_scan_enable_cmd(hdev, 1);
>> >> +
>> >> + =A0 =A0 schedule_timeout_uninterruptible(timeo);
>> >> + =A0 =A0 if (hdev->le_scan_result)
>> >> + =A0 =A0 =A0 =A0 =A0 =A0 goto failed;
>> >> +
>> >> + =A0 =A0 remove_wait_queue(&hdev->le_scan_wait_q, &wait);
>> >> +
>> >> + =A0 =A0 schedule_delayed_work(&hdev->le_scan_disable,
>> >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=
 =A0 =A0 =A0 =A0 =A0 msecs_to_jiffies(timeout));
>> >
>> > we need to make this generic. I know that Andrei will need the same fo=
r
>> > A2MP and we already do a similar thing with hci_request. If we start
>> > spreading the hand-coded versions, this gets messy really quick.
>>
>> Yes, I do agree with you about getting messy. So, are you fine with
>> we use hci_request here too?
>
> so hci_request is one command at a time. We have a lock for it that
> ensures this. Is this good enough?

Yes, for this case, hci_request is just fine.

BR,

Andre

Re: [PATCH v3 5/6] Bluetooth: MGMT start discovery LE-Only support

[Andre Guedes]

Hi Marcel,

On Mon, Jan 30, 2012 at 6:44 PM, Marcel Holtmann <marcel@holtmann.org> wrot=
e:
> Hi Andre,
>
>> This patch adds LE-Only discovery procedure support to MGMT Start
>> Discovery command.
>>
>> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
>> ---
>> =A0net/bluetooth/hci_event.c | =A0 13 ++++++++++++-
>> =A0net/bluetooth/mgmt.c =A0 =A0 =A0| =A0 20 +++++++++++++++++++-
>> =A02 files changed, 31 insertions(+), 2 deletions(-)
>>
>> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
>> index 856ab35..fb2497b 100644
>> --- a/net/bluetooth/hci_event.c
>> +++ b/net/bluetooth/hci_event.c
>> @@ -1033,6 +1033,13 @@ static void hci_cc_le_set_scan_param(struct hci_d=
ev *hdev, struct sk_buff *skb)
>>
>> =A0 =A0 =A0 hdev->le_scan_result =3D -bt_to_errno(status);
>> =A0 =A0 =A0 wake_up(&hdev->le_scan_wait_q);
>> +
>> + =A0 =A0 if (status) {
>> + =A0 =A0 =A0 =A0 =A0 =A0 hci_dev_lock(hdev);
>> + =A0 =A0 =A0 =A0 =A0 =A0 mgmt_start_discovery_failed(hdev, status);
>> + =A0 =A0 =A0 =A0 =A0 =A0 hci_dev_unlock(hdev);
>> + =A0 =A0 =A0 =A0 =A0 =A0 return;
>> + =A0 =A0 }
>> =A0}
>>
>> =A0static void hci_cc_le_set_scan_enable(struct hci_dev *hdev,
>> @@ -1052,8 +1059,12 @@ static void hci_cc_le_set_scan_enable(struct hci_=
dev *hdev,
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 hdev->le_scan_result =3D -bt_to_errno(status=
);
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 wake_up(&hdev->le_scan_wait_q);
>>
>> - =A0 =A0 =A0 =A0 =A0 =A0 if (status)
>> + =A0 =A0 =A0 =A0 =A0 =A0 if (status) {
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 hci_dev_lock(hdev);
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 mgmt_start_discovery_failed(hd=
ev, status);
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 hci_dev_unlock(hdev);
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 return;
>> + =A0 =A0 =A0 =A0 =A0 =A0 }
>
> are these two related to the other changes or just actual bug fixes?

These two are related to MGMT Start Discovery command itself. If LE
scan HCI commands fail we have to send mgmt_start_discovery_failed
event.

>>
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 set_bit(HCI_LE_SCAN, &hdev->dev_flags);
>>
>> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
>> index 8970799..0a1fa4c 100644
>> --- a/net/bluetooth/mgmt.c
>> +++ b/net/bluetooth/mgmt.c
>> @@ -35,6 +35,15 @@
>> =A0#define MGMT_VERSION 0
>> =A0#define MGMT_REVISION =A0 =A0 =A0 =A01
>>
>> +/*
>> + * These LE scan and inquiry parameters were chosen according to LE Gen=
eral
>> + * Discovery Procedure specification.
>> + */
>> +#define LE_SCAN_TYPE =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 0x01
>> +#define LE_SCAN_WIN =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A00x12
>> +#define LE_SCAN_INT =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A00x12
>> +#define LE_SCAN_TIMEOUT_LE_ONLY =A0 =A0 =A0 =A0 =A0 =A0 =A010240 =A0 /*=
 TGAP(gen_disc_scan_min) */
>> +
>> =A0#define INQUIRY_LEN_BREDR 0x08 /* TGAP(100) */
>>
>> =A0#define SERVICE_CACHE_TIMEOUT (5 * 1000)
>> @@ -1897,6 +1906,7 @@ static int start_discovery(struct sock *sk, u16 in=
dex,
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0 =A0 =A0 =A0 unsigned char *data, u16 len)
>> =A0{
>> =A0 =A0 =A0 struct mgmt_cp_start_discovery *cp =3D (void *) data;
>> + =A0 =A0 unsigned long discov_type =3D cp->type;
>> =A0 =A0 =A0 struct pending_cmd *cmd;
>> =A0 =A0 =A0 struct hci_dev *hdev;
>> =A0 =A0 =A0 int err;
>> @@ -1932,7 +1942,15 @@ static int start_discovery(struct sock *sk, u16 i=
ndex,
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 goto failed;
>> =A0 =A0 =A0 }
>>
>> - =A0 =A0 err =3D hci_do_inquiry(hdev, INQUIRY_LEN_BREDR);
>> + =A0 =A0 if (test_bit(MGMT_ADDR_BREDR, &discov_type))
>> + =A0 =A0 =A0 =A0 =A0 =A0 err =3D hci_do_inquiry(hdev, INQUIRY_LEN_BREDR=
);
>> + =A0 =A0 else if (test_bit(MGMT_ADDR_LE_PUBLIC, &discov_type) &&
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 test_bit(MGMT_=
ADDR_LE_RANDOM, &discov_type))
>
> so test_bit can not check for two bits at the same time?

AFAIK, no. If anyone knows a helper like that, please let me
know.

BR,

Andre

Re: [PATCH v3 6/6] Bluetooth: Fix indentation

[Andre Guedes]

Hi Marcel,

On Mon, Jan 30, 2012 at 6:45 PM, Marcel Holtmann <marcel@holtmann.org> wrot=
e:
> Hi Andre,
>
> please fix the commit message to at least mention where.

Ok, I'll fix it.

>> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
>> ---
>> =A0net/bluetooth/mgmt.c | =A0 =A02 +-
>> =A01 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
>> index 0a1fa4c..e7033c1 100644
>> --- a/net/bluetooth/mgmt.c
>> +++ b/net/bluetooth/mgmt.c
>> @@ -44,7 +44,7 @@
>> =A0#define LE_SCAN_INT =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A00x12
>> =A0#define LE_SCAN_TIMEOUT_LE_ONLY =A0 =A0 =A0 =A0 =A0 =A0 =A010240 =A0 =
/* TGAP(gen_disc_scan_min) */
>>
>> -#define INQUIRY_LEN_BREDR 0x08 /* TGAP(100) */
>> +#define INQUIRY_LEN_BREDR =A0 =A0 =A0 =A0 =A0 =A00x08 =A0 =A0/* TGAP(10=
0) */
>>
>> =A0#define SERVICE_CACHE_TIMEOUT (5 * 1000)
>>
>
> Otherwise this is fine.
>
> Acked-by: Marcel Holtmann <marcel@holtmann.org>
>
> Regards
>
> Marcel
>
>

Andre

Re: [PATCH v3 1/6] Bluetooth: LE scan should send Discovering events

[Marcel Holtmann]

Hi Andre,

> >> Send MGMT Discovering events once LE scan starts/stops so the
> >> userspace can track when local adapters are discovering LE devices.
> >>
> >> This way, we also keep the same behavior of inquiry which sends MGMT
> >> Discovering events once inquiry starts/stops even if it is triggered
> >> by an external tool (e.g. hcitool).
> >>
> >> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
> >> ---
> >>  include/net/bluetooth/hci_core.h |    1 +
> >>  net/bluetooth/hci_core.c         |    2 ++
> >>  net/bluetooth/hci_event.c        |    5 +++++
> >>  3 files changed, 8 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> >> index 25f449f..4e569d8 100644
> >> --- a/include/net/bluetooth/hci_core.h
> >> +++ b/include/net/bluetooth/hci_core.h
> >> @@ -61,6 +61,7 @@ struct discovery_state {
> >>               DISCOVERY_STOPPED,
> >>               DISCOVERY_STARTING,
> >>               DISCOVERY_INQUIRY,
> >> +             DISCOVERY_LE_SCAN,
> >>               DISCOVERY_RESOLVING,
> >>               DISCOVERY_STOPPING,
> >>       } state;
> >> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> >> index 91166db..fd22035 100644
> >> --- a/net/bluetooth/hci_core.c
> >> +++ b/net/bluetooth/hci_core.c
> >> @@ -361,6 +361,7 @@ bool hci_discovery_active(struct hci_dev *hdev)
> >>       struct discovery_state *discov = &hdev->discovery;
> >>
> >>       if (discov->state == DISCOVERY_INQUIRY ||
> >> +                                     discov->state == DISCOVERY_LE_SCAN ||
> >>                                       discov->state == DISCOVERY_RESOLVING)
> >>               return true;
> >
> > I think we need to start using a switch statement here.
> 
> Ok, then I'll have a code refactoring patch to handle this separately.

that is fine with me.

Regards

Marcel