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

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

[Andre Guedes]

Hi all,

The changes in this version are based on Marcel's comments about previous
patch series.

Here they are:
	* Add Marcel's acks
	* Minor code refactoring in hci_discovery_active
	* Use hci_request framework in hci_do_le_scan

BR,

Andre

Andre Guedes (6):
  Bluetooth: LE scan should send Discovering events
  Bluetooth: Minor code refactoring
  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 |   15 +++++
 net/bluetooth/hci_core.c         |  125 +++++++++++++++++++++++++++++++++++++-
 net/bluetooth/hci_event.c        |   29 ++++++++-
 net/bluetooth/mgmt.c             |   22 ++++++-
 4 files changed, 183 insertions(+), 8 deletions(-)

-- 
1.7.9

[PATCH v4 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>
Acked-by: Marcel Holtmann <marcel@holtmann.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 896d9e4..eab98d3 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 45e2d2a..1fb6228 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.9

[PATCH v4 2/6] Bluetooth: Minor code refactoring

[Andre Guedes]

This patch does a trivial code refacting in hci_discovery_active.

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

diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 1fb6228..35843f1 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -360,12 +360,15 @@ 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)
+	switch (discov->state) {
+	case DISCOVERY_INQUIRY:
+	case DISCOVERY_LE_SCAN:
+	case DISCOVERY_RESOLVING:
 		return true;
 
-	return false;
+	default:
+		return false;
+	}
 }
 
 void hci_discovery_set_state(struct hci_dev *hdev, int state)
-- 
1.7.9

Re: [PATCH v4 2/6] Bluetooth: Minor code refactoring

[Marcel Holtmann]

Hi Andre,

> This patch does a trivial code refacting in hci_discovery_active.
> 
> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
> ---
>  net/bluetooth/hci_core.c |   11 +++++++----
>  1 files changed, 7 insertions(+), 4 deletions(-)

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

Regards

Marcel

[PATCH v4 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 |    8 ++++
 net/bluetooth/hci_core.c         |   79 ++++++++++++++++++++++++++++++++++++++
 net/bluetooth/hci_event.c        |   13 +++++-
 3 files changed, 97 insertions(+), 3 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index eab98d3..47111df 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -126,6 +126,12 @@ struct adv_entry {
 	u8 bdaddr_type;
 };
 
+struct le_scan_params {
+	u8 type;
+	u16 interval;
+	u16 window;
+};
+
 #define NUM_REASSEMBLY 4
 struct hci_dev {
 	struct list_head list;
@@ -263,6 +269,8 @@ struct hci_dev {
 
 	unsigned long		dev_flags;
 
+	struct delayed_work	le_scan_disable;
+
 	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 35843f1..2f5d1ae 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -759,6 +759,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);
@@ -1573,6 +1575,81 @@ int hci_add_adv_entry(struct hci_dev *hdev,
 	return 0;
 }
 
+static void le_scan_param_req(struct hci_dev *hdev, unsigned long opt)
+{
+	struct le_scan_params *param =  (struct le_scan_params *) opt;
+	struct hci_cp_le_set_scan_param cp;
+
+	memset(&cp, 0, sizeof(cp));
+	cp.type = param->type;
+	cp.interval = cpu_to_le16(param->interval);
+	cp.window = cpu_to_le16(param->window);
+
+	hci_send_cmd(hdev, HCI_OP_LE_SET_SCAN_PARAM, sizeof(cp), &cp);
+}
+
+static void le_scan_enable_req(struct hci_dev *hdev, unsigned long opt)
+{
+	struct hci_cp_le_set_scan_enable cp;
+
+	memset(&cp, 0, sizeof(cp));
+	cp.enable = 1;
+
+	hci_send_cmd(hdev, HCI_OP_LE_SET_SCAN_ENABLE, sizeof(cp), &cp);
+}
+
+static int hci_do_le_scan(struct hci_dev *hdev, u8 type, u16 interval,
+						u16 window, int timeout)
+{
+	long timeo = msecs_to_jiffies(3000);
+	struct le_scan_params param;
+	int err;
+
+	BT_DBG("%s", hdev->name);
+
+	if (test_bit(HCI_LE_SCAN, &hdev->dev_flags))
+		return -EINPROGRESS;
+
+	param.type = type;
+	param.interval = interval;
+	param.window = window;
+
+	hci_req_lock(hdev);
+
+	err = __hci_request(hdev, le_scan_param_req, (unsigned long) &param,
+									timeo);
+	if (err < 0)
+		goto failed;
+
+	err = __hci_request(hdev, le_scan_enable_req, 0, timeo);
+	if (err < 0)
+		goto failed;
+
+	hci_req_unlock(hdev);
+
+	schedule_delayed_work(&hdev->le_scan_disable,
+						msecs_to_jiffies(timeout));
+
+	return 0;
+
+failed:
+	hci_req_unlock(hdev);
+	return err;
+}
+
+static void le_scan_disable_work(struct work_struct *work)
+{
+	struct hci_dev *hdev = container_of(work, struct hci_dev,
+						le_scan_disable.work);
+	struct hci_cp_le_set_scan_enable cp;
+
+	BT_DBG("%s", hdev->name);
+
+	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)
 {
@@ -1658,6 +1735,8 @@ int hci_register_dev(struct hci_dev *hdev)
 
 	atomic_set(&hdev->promisc, 0);
 
+	INIT_DELAYED_WORK(&hdev->le_scan_disable, le_scan_disable_work);
+
 	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..0bcfeca 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -1030,6 +1030,8 @@ 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);
+
+	hci_req_complete(hdev, HCI_OP_LE_SET_SCAN_PARAM, status);
 }
 
 static void hci_cc_le_set_scan_enable(struct hci_dev *hdev,
@@ -1040,15 +1042,17 @@ 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:
+		hci_req_complete(hdev, HCI_OP_LE_SET_SCAN_ENABLE, status);
+
+		if (status)
+			return;
+
 		set_bit(HCI_LE_SCAN, &hdev->dev_flags);
 
 		cancel_delayed_work_sync(&hdev->adv_work);
@@ -1060,6 +1064,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.9

Re: [PATCH v4 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 |    8 ++++
>  net/bluetooth/hci_core.c         |   79 ++++++++++++++++++++++++++++++++++++++
>  net/bluetooth/hci_event.c        |   13 +++++-
>  3 files changed, 97 insertions(+), 3 deletions(-)
> 
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index eab98d3..47111df 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -126,6 +126,12 @@ struct adv_entry {
>  	u8 bdaddr_type;
>  };
>  
> +struct le_scan_params {
> +	u8 type;
> +	u16 interval;
> +	u16 window;
> +};
> +
>  #define NUM_REASSEMBLY 4
>  struct hci_dev {
>  	struct list_head list;
> @@ -263,6 +269,8 @@ struct hci_dev {
>  
>  	unsigned long		dev_flags;
>  
> +	struct delayed_work	le_scan_disable;
> +
>  	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 35843f1..2f5d1ae 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -759,6 +759,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);
> @@ -1573,6 +1575,81 @@ int hci_add_adv_entry(struct hci_dev *hdev,
>  	return 0;
>  }
>  
> +static void le_scan_param_req(struct hci_dev *hdev, unsigned long opt)
> +{
> +	struct le_scan_params *param =  (struct le_scan_params *) opt;
> +	struct hci_cp_le_set_scan_param cp;
> +
> +	memset(&cp, 0, sizeof(cp));
> +	cp.type = param->type;
> +	cp.interval = cpu_to_le16(param->interval);
> +	cp.window = cpu_to_le16(param->window);
> +
> +	hci_send_cmd(hdev, HCI_OP_LE_SET_SCAN_PARAM, sizeof(cp), &cp);
> +}
> +
> +static void le_scan_enable_req(struct hci_dev *hdev, unsigned long opt)
> +{
> +	struct hci_cp_le_set_scan_enable cp;
> +
> +	memset(&cp, 0, sizeof(cp));
> +	cp.enable = 1;
> +
> +	hci_send_cmd(hdev, HCI_OP_LE_SET_SCAN_ENABLE, sizeof(cp), &cp);
> +}
> +
> +static int hci_do_le_scan(struct hci_dev *hdev, u8 type, u16 interval,
> +						u16 window, int timeout)
> +{
> +	long timeo = msecs_to_jiffies(3000);
> +	struct le_scan_params param;
> +	int err;
> +
> +	BT_DBG("%s", hdev->name);
> +
> +	if (test_bit(HCI_LE_SCAN, &hdev->dev_flags))
> +		return -EINPROGRESS;
> +
> +	param.type = type;
> +	param.interval = interval;
> +	param.window = window;
> +
> +	hci_req_lock(hdev);
> +
> +	err = __hci_request(hdev, le_scan_param_req, (unsigned long) &param,
> +									timeo);
> +	if (err < 0)
> +		goto failed;
> +
> +	err = __hci_request(hdev, le_scan_enable_req, 0, timeo);
> +	if (err < 0)
> +		goto failed;
> +
> +	hci_req_unlock(hdev);

in this specific case, it might be better to do it like this:

	hci_req_lock();

	err = __hci_request(hdev, ...)
	if (!err)
		err = __hci_request(hdev, ...)

	hci_req_unlock()

	if (err < 0)
		return err;

> +
> +	schedule_delayed_work(&hdev->le_scan_disable,
> +						msecs_to_jiffies(timeout));
> +
> +	return 0;
> +
> +failed:
> +	hci_req_unlock(hdev);
> +	return err;
> +}
> +
> +static void le_scan_disable_work(struct work_struct *work)
> +{
> +	struct hci_dev *hdev = container_of(work, struct hci_dev,
> +						le_scan_disable.work);
> +	struct hci_cp_le_set_scan_enable cp;
> +
> +	BT_DBG("%s", hdev->name);
> +
> +	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)
>  {
> @@ -1658,6 +1735,8 @@ int hci_register_dev(struct hci_dev *hdev)
>  
>  	atomic_set(&hdev->promisc, 0);
>  
> +	INIT_DELAYED_WORK(&hdev->le_scan_disable, le_scan_disable_work);
> +
>  	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..0bcfeca 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -1030,6 +1030,8 @@ 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);
> +
> +	hci_req_complete(hdev, HCI_OP_LE_SET_SCAN_PARAM, status);
>  }
>  
>  static void hci_cc_le_set_scan_enable(struct hci_dev *hdev,
> @@ -1040,15 +1042,17 @@ 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:
> +		hci_req_complete(hdev, HCI_OP_LE_SET_SCAN_ENABLE, status);
> +
> +		if (status)
> +			return;
> +
>  		set_bit(HCI_LE_SCAN, &hdev->dev_flags);
>  
>  		cancel_delayed_work_sync(&hdev->adv_work);
> @@ -1060,6 +1064,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);

Long term we can not spread hci_req_complete around. We need a more
general solution. Please start working on that one and send a proposal.

Regards

Marcel

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

[Andre Guedes]

Hi Marcel,

On Fri, Feb 3, 2012 at 2:39 PM, Marcel Holtmann <marcel@holtmann.org> wrote=
:
> 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 =A08 ++++
>> =A0net/bluetooth/hci_core.c =A0 =A0 =A0 =A0 | =A0 79 +++++++++++++++++++=
+++++++++++++++++++
>> =A0net/bluetooth/hci_event.c =A0 =A0 =A0 =A0| =A0 13 +++++-
>> =A03 files changed, 97 insertions(+), 3 deletions(-)
>>
>> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hc=
i_core.h
>> index eab98d3..47111df 100644
>> --- a/include/net/bluetooth/hci_core.h
>> +++ b/include/net/bluetooth/hci_core.h
>> @@ -126,6 +126,12 @@ struct adv_entry {
>> =A0 =A0 =A0 u8 bdaddr_type;
>> =A0};
>>
>> +struct le_scan_params {
>> + =A0 =A0 u8 type;
>> + =A0 =A0 u16 interval;
>> + =A0 =A0 u16 window;
>> +};
>> +
>> =A0#define NUM_REASSEMBLY 4
>> =A0struct hci_dev {
>> =A0 =A0 =A0 struct list_head list;
>> @@ -263,6 +269,8 @@ 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 =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 35843f1..2f5d1ae 100644
>> --- a/net/bluetooth/hci_core.c
>> +++ b/net/bluetooth/hci_core.c
>> @@ -759,6 +759,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);
>> @@ -1573,6 +1575,81 @@ int hci_add_adv_entry(struct hci_dev *hdev,
>> =A0 =A0 =A0 return 0;
>> =A0}
>>
>> +static void le_scan_param_req(struct hci_dev *hdev, unsigned long opt)
>> +{
>> + =A0 =A0 struct le_scan_params *param =3D =A0(struct le_scan_params *) =
opt;
>> + =A0 =A0 struct hci_cp_le_set_scan_param cp;
>> +
>> + =A0 =A0 memset(&cp, 0, sizeof(cp));
>> + =A0 =A0 cp.type =3D param->type;
>> + =A0 =A0 cp.interval =3D cpu_to_le16(param->interval);
>> + =A0 =A0 cp.window =3D cpu_to_le16(param->window);
>> +
>> + =A0 =A0 hci_send_cmd(hdev, HCI_OP_LE_SET_SCAN_PARAM, sizeof(cp), &cp);
>> +}
>> +
>> +static void le_scan_enable_req(struct hci_dev *hdev, unsigned long opt)
>> +{
>> + =A0 =A0 struct hci_cp_le_set_scan_enable cp;
>> +
>> + =A0 =A0 memset(&cp, 0, sizeof(cp));
>> + =A0 =A0 cp.enable =3D 1;
>> +
>> + =A0 =A0 hci_send_cmd(hdev, HCI_OP_LE_SET_SCAN_ENABLE, sizeof(cp), &cp)=
;
>> +}
>> +
>> +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 struct le_scan_params param;
>> + =A0 =A0 int err;
>> +
>> + =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 param.type =3D type;
>> + =A0 =A0 param.interval =3D interval;
>> + =A0 =A0 param.window =3D window;
>> +
>> + =A0 =A0 hci_req_lock(hdev);
>> +
>> + =A0 =A0 err =3D __hci_request(hdev, le_scan_param_req, (unsigned long)=
 &param,
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 timeo);
>> + =A0 =A0 if (err < 0)
>> + =A0 =A0 =A0 =A0 =A0 =A0 goto failed;
>> +
>> + =A0 =A0 err =3D __hci_request(hdev, le_scan_enable_req, 0, timeo);
>> + =A0 =A0 if (err < 0)
>> + =A0 =A0 =A0 =A0 =A0 =A0 goto failed;
>> +
>> + =A0 =A0 hci_req_unlock(hdev);
>
> in this specific case, it might be better to do it like this:
>
> =A0 =A0 =A0 =A0hci_req_lock();
>
> =A0 =A0 =A0 =A0err =3D __hci_request(hdev, ...)
> =A0 =A0 =A0 =A0if (!err)
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0err =3D __hci_request(hdev, ...)
>
> =A0 =A0 =A0 =A0hci_req_unlock()
>
> =A0 =A0 =A0 =A0if (err < 0)
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0return err;

Ok, I'm gonna change this and send a new version of this series.

>> +
>> + =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));
>> +
>> + =A0 =A0 return 0;
>> +
>> +failed:
>> + =A0 =A0 hci_req_unlock(hdev);
>> + =A0 =A0 return err;
>> +}
>> +
>> +static void le_scan_disable_work(struct work_struct *work)
>> +{
>> + =A0 =A0 struct hci_dev *hdev =3D container_of(work, struct hci_dev,
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0 =A0 =A0 =A0 le_scan_disable.work);
>> + =A0 =A0 struct hci_cp_le_set_scan_enable cp;
>> +
>> + =A0 =A0 BT_DBG("%s", hdev->name);
>> +
>> + =A0 =A0 memset(&cp, 0, sizeof(cp));
>> +
>> + =A0 =A0 hci_send_cmd(hdev, HCI_OP_LE_SET_SCAN_ENABLE, sizeof(cp), &cp)=
;
>> +}
>> +
>> =A0/* Register HCI device */
>> =A0int hci_register_dev(struct hci_dev *hdev)
>> =A0{
>> @@ -1658,6 +1735,8 @@ int hci_register_dev(struct hci_dev *hdev)
>>
>> =A0 =A0 =A0 atomic_set(&hdev->promisc, 0);
>>
>> + =A0 =A0 INIT_DELAYED_WORK(&hdev->le_scan_disable, le_scan_disable_work=
);
>> +
>> =A0 =A0 =A0 write_unlock(&hci_dev_list_lock);
>>
>> =A0 =A0 =A0 hdev->workqueue =3D alloc_workqueue(hdev->name, WQ_HIGHPRI |=
 WQ_UNBOUND |
>> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
>> index dd8056c..0bcfeca 100644
>> --- a/net/bluetooth/hci_event.c
>> +++ b/net/bluetooth/hci_event.c
>> @@ -1030,6 +1030,8 @@ static void hci_cc_le_set_scan_param(struct hci_de=
v *hdev, struct sk_buff *skb)
>> =A0 =A0 =A0 __u8 status =3D *((__u8 *) skb->data);
>>
>> =A0 =A0 =A0 BT_DBG("%s status 0x%x", hdev->name, status);
>> +
>> + =A0 =A0 hci_req_complete(hdev, HCI_OP_LE_SET_SCAN_PARAM, status);
>> =A0}
>>
>> =A0static void hci_cc_le_set_scan_enable(struct hci_dev *hdev,
>> @@ -1040,15 +1042,17 @@ static void hci_cc_le_set_scan_enable(struct hci=
_dev *hdev,
>>
>> =A0 =A0 =A0 BT_DBG("%s status 0x%x", hdev->name, status);
>>
>> - =A0 =A0 if (status)
>> - =A0 =A0 =A0 =A0 =A0 =A0 return;
>> -
>> =A0 =A0 =A0 cp =3D hci_sent_cmd_data(hdev, HCI_OP_LE_SET_SCAN_ENABLE);
>> =A0 =A0 =A0 if (!cp)
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 return;
>>
>> =A0 =A0 =A0 switch (cp->enable) {
>> =A0 =A0 =A0 case LE_SCANNING_ENABLED:
>> + =A0 =A0 =A0 =A0 =A0 =A0 hci_req_complete(hdev, HCI_OP_LE_SET_SCAN_ENAB=
LE, status);
>> +
>> + =A0 =A0 =A0 =A0 =A0 =A0 if (status)
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 return;
>> +
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 set_bit(HCI_LE_SCAN, &hdev->dev_flags);
>>
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 cancel_delayed_work_sync(&hdev->adv_work);
>> @@ -1060,6 +1064,9 @@ static void hci_cc_le_set_scan_enable(struct hci_d=
ev *hdev,
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 break;
>>
>> =A0 =A0 =A0 case LE_SCANNING_DISABLED:
>> + =A0 =A0 =A0 =A0 =A0 =A0 if (status)
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 return;
>> +
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 clear_bit(HCI_LE_SCAN, &hdev->dev_flags);
>>
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 hci_dev_lock(hdev);
>
> Long term we can not spread hci_req_complete around. We need a more
> general solution. Please start working on that one and send a proposal.

Sure, I talked with Claudio and as soon as we finish pushing discovery
patches upstream I can work on a general solution for that.

BR,

Andre

[PATCH v4 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>
Acked-by: Marcel Holtmann <marcel@holtmann.org>
---
 include/net/bluetooth/hci_core.h |    6 ++++++
 net/bluetooth/hci_core.c         |   35 +++++++++++++++++++++++++++++++++++
 2 files changed, 41 insertions(+), 0 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 47111df..0cffb25 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -130,6 +130,7 @@ struct le_scan_params {
 	u8 type;
 	u16 interval;
 	u16 window;
+	int timeout;
 };
 
 #define NUM_REASSEMBLY 4
@@ -271,6 +272,9 @@ struct hci_dev {
 
 	struct delayed_work	le_scan_disable;
 
+	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);
@@ -1030,5 +1034,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 2f5d1ae..046add5 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -735,6 +735,8 @@ static int hci_dev_do_close(struct hci_dev *hdev)
 {
 	BT_DBG("%s %p", hdev->name, hdev);
 
+	cancel_work_sync(&hdev->le_scan);
+
 	hci_req_cancel(hdev, ENODEV);
 	hci_req_lock(hdev);
 
@@ -1650,6 +1652,37 @@ static void le_scan_disable_work(struct work_struct *work)
 	hci_send_cmd(hdev, HCI_OP_LE_SET_SCAN_ENABLE, sizeof(cp), &cp);
 }
 
+static void le_scan_work(struct work_struct *work)
+{
+	struct hci_dev *hdev = container_of(work, struct hci_dev, le_scan);
+	struct le_scan_params *param = &hdev->le_scan_params;
+
+	BT_DBG("%s", hdev->name);
+
+	hci_do_le_scan(hdev, param->type, param->interval,
+					param->window, param->timeout);
+}
+
+int hci_le_scan(struct hci_dev *hdev, u8 type, u16 interval, u16 window,
+								int timeout)
+{
+	struct le_scan_params *param = &hdev->le_scan_params;
+
+	BT_DBG("%s", hdev->name);
+
+	if (work_busy(&hdev->le_scan))
+		return -EINPROGRESS;
+
+	param->type = type;
+	param->interval = interval;
+	param->window = window;
+	param->timeout = timeout;
+
+	queue_work(system_long_wq, &hdev->le_scan);
+
+	return 0;
+}
+
 /* Register HCI device */
 int hci_register_dev(struct hci_dev *hdev)
 {
@@ -1735,6 +1768,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);
 
 	write_unlock(&hci_dev_list_lock);
-- 
1.7.9

[PATCH v4 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 0bcfeca..448c7af 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -1032,6 +1032,13 @@ static void hci_cc_le_set_scan_param(struct hci_dev *hdev, struct sk_buff *skb)
 	BT_DBG("%s status 0x%x", hdev->name, status);
 
 	hci_req_complete(hdev, HCI_OP_LE_SET_SCAN_PARAM, status);
+
+	if (status) {
+		hci_dev_lock(hdev);
+		mgmt_start_discovery_failed(hdev, status);
+		hci_dev_unlock(hdev);
+		return;
+	}
 }
 
 static void hci_cc_le_set_scan_enable(struct hci_dev *hdev,
@@ -1050,8 +1057,12 @@ static void hci_cc_le_set_scan_enable(struct hci_dev *hdev,
 	case LE_SCANNING_ENABLED:
 		hci_req_complete(hdev, HCI_OP_LE_SET_SCAN_ENABLE, status);
 
-		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.9

Re: [PATCH v4 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(-)

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

Regards

Marcel

[PATCH v4 6/6] Bluetooth: Fix indentation

[Andre Guedes]

This patch fixes a #define indentation in mgmt.c.

Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
Acked-by: Marcel Holtmann <marcel@holtmann.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.9