[PATCH 0/9] Bluetooth: Add LE support for mgmt_set_discoverable

[PATCH 0/9] Bluetooth: Add LE support for mgmt_set_discoverable

[johan.hedberg]

Hi,

In addition to a couple if fixes in the existing mgmt code (first 5
patches), this patch set adds support for hooking up the
set_discoverable command to LE, in particular the advertising flags.

Johan

----------------------------------------------------------------
Johan Hedberg (9):
      Bluetooth: Check for flag instead of features in update_scan_rsp_data()
      Bluetooth: Check for flag instead of features in update_adv_data()
      Bluetooth: Add missing check for BREDR_ENABLED flag in update_class()
      Bluetooth: Refactor set_connectable settings update to separate function
      Bluetooth: Fix updating settings when there are no HCI commands to send
      Bluetooth: Move mgmt_pending_find to avoid forward declarations
      Bluetooth: Fix sending write_scan_enable when BR/EDR is disabled
      Bluetooth: Move HCI_LIMITED_DISCOVERABLE changes to a general place
      Bluetooth: Update Set Discoverable to support LE

 net/bluetooth/mgmt.c | 147 +++++++++++++++++++++++++++++++++----------------
 1 file changed, 100 insertions(+), 47 deletions(-)

[PATCH 1/9] Bluetooth: Check for flag instead of features in update_scan_rsp_data()

[johan.hedberg]

From: Johan Hedberg <johan.hedberg@intel.com>

It's better to check for the device flag instead of device features so
that we avoid unnecessary HCI commands when the feature is supported but
disabled (i.e. the flag is unset).

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 net/bluetooth/mgmt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index fb7fc9f..f39bab0 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -568,7 +568,7 @@ static void update_scan_rsp_data(struct hci_request *req)
 	struct hci_cp_le_set_scan_rsp_data cp;
 	u8 len;
 
-	if (!lmp_le_capable(hdev))
+	if (!test_bit(HCI_LE_ENABLED, &hdev->dev_flags))
 		return;
 
 	memset(&cp, 0, sizeof(cp));
-- 
1.8.3.1

[PATCH 2/9] Bluetooth: Check for flag instead of features in update_adv_data()

[johan.hedberg]

From: Johan Hedberg <johan.hedberg@intel.com>

It's better to check for the device flag instead of device features so
that we avoid unnecessary HCI commands when the feature is supported but
disabled (i.e. the flag is unset).

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 net/bluetooth/mgmt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index f39bab0..dd0bd4a 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -632,7 +632,7 @@ static void update_adv_data(struct hci_request *req)
 	struct hci_cp_le_set_adv_data cp;
 	u8 len;
 
-	if (!lmp_le_capable(hdev))
+	if (!test_bit(HCI_LE_ENABLED, &hdev->dev_flags))
 		return;
 
 	memset(&cp, 0, sizeof(cp));
-- 
1.8.3.1

[PATCH 3/9] Bluetooth: Add missing check for BREDR_ENABLED flag in update_class()

[johan.hedberg]

From: Johan Hedberg <johan.hedberg@intel.com>

We shouldn't be sending the HCI_Write_Class_Of_Device command when
BR/EDR is disabled since this is a BR/EDR-only command.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 net/bluetooth/mgmt.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index dd0bd4a..c363285 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -749,6 +749,9 @@ static void update_class(struct hci_request *req)
 	if (!hdev_is_powered(hdev))
 		return;
 
+	if (!test_bit(HCI_BREDR_ENABLED, &hdev->dev_flags))
+		return;
+
 	if (test_bit(HCI_SERVICE_CACHE, &hdev->dev_flags))
 		return;
 
-- 
1.8.3.1

[PATCH 4/9] Bluetooth: Refactor set_connectable settings update to separate function

[johan.hedberg]

From: Johan Hedberg <johan.hedberg@intel.com>

We will need to directly update the device flags and notify user space
of the new settings not just when we're powered off but also if it turns
out that there are no HCI commands to send (which can happen in
particular when BR/EDR is disabled). Since this is a considerable amount
of code, refactor it to a separate function so it can be reused for the
"no HCI commands to send" case.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 net/bluetooth/mgmt.c | 46 +++++++++++++++++++++++++++-------------------
 1 file changed, 27 insertions(+), 19 deletions(-)

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index c363285..b74a157 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -1381,6 +1381,32 @@ unlock:
 	hci_dev_unlock(hdev);
 }
 
+static int set_connectable_update_settings(struct hci_dev *hdev,
+					   struct sock *sk, u8 val)
+{
+	bool changed = false;
+	int err;
+
+	if (!!val != test_bit(HCI_CONNECTABLE, &hdev->dev_flags))
+		changed = true;
+
+	if (val) {
+		set_bit(HCI_CONNECTABLE, &hdev->dev_flags);
+	} else {
+		clear_bit(HCI_CONNECTABLE, &hdev->dev_flags);
+		clear_bit(HCI_DISCOVERABLE, &hdev->dev_flags);
+	}
+
+	err = send_settings_rsp(sk, MGMT_OP_SET_CONNECTABLE, hdev);
+	if (err < 0)
+		return err;
+
+	if (changed)
+		return new_settings(hdev, sk);
+
+	return 0;
+}
+
 static int set_connectable(struct sock *sk, struct hci_dev *hdev, void *data,
 			   u16 len)
 {
@@ -1404,25 +1430,7 @@ static int set_connectable(struct sock *sk, struct hci_dev *hdev, void *data,
 	hci_dev_lock(hdev);
 
 	if (!hdev_is_powered(hdev)) {
-		bool changed = false;
-
-		if (!!cp->val != test_bit(HCI_CONNECTABLE, &hdev->dev_flags))
-			changed = true;
-
-		if (cp->val) {
-			set_bit(HCI_CONNECTABLE, &hdev->dev_flags);
-		} else {
-			clear_bit(HCI_CONNECTABLE, &hdev->dev_flags);
-			clear_bit(HCI_DISCOVERABLE, &hdev->dev_flags);
-		}
-
-		err = send_settings_rsp(sk, MGMT_OP_SET_CONNECTABLE, hdev);
-		if (err < 0)
-			goto failed;
-
-		if (changed)
-			err = new_settings(hdev, sk);
-
+		err = set_connectable_update_settings(hdev, sk, cp->val);
 		goto failed;
 	}
 
-- 
1.8.3.1

[PATCH 5/9] Bluetooth: Fix updating settings when there are no HCI commands to send

[johan.hedberg]

From: Johan Hedberg <johan.hedberg@intel.com>

It is possible that the Set Connectable management command doesn't cause
any HCI commands to send (such as when BR/EDR is disabled). We can't
just send a response to user space in this case but must also update the
necessary device flags and settings. This patch fixes the issue by using
the recently introduced set_connectable_update_settings function.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 net/bluetooth/mgmt.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index b74a157..51d0446 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -1483,8 +1483,8 @@ static int set_connectable(struct sock *sk, struct hci_dev *hdev, void *data,
 	if (err < 0) {
 		mgmt_pending_remove(cmd);
 		if (err == -ENODATA)
-			err = send_settings_rsp(sk, MGMT_OP_SET_CONNECTABLE,
-						hdev);
+			err = set_connectable_update_settings(hdev, sk,
+							      cp->val);
 		goto failed;
 	}
 
-- 
1.8.3.1

[PATCH 6/9] Bluetooth: Move mgmt_pending_find to avoid forward declarations

[johan.hedberg]

From: Johan Hedberg <johan.hedberg@intel.com>

We will soon need this function for updating the advertising data, so
move it higher up in mgmt.c to avoid a forward declaration.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 net/bluetooth/mgmt.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 51d0446..20f6309 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -536,6 +536,18 @@ static u8 *create_uuid128_list(struct hci_dev *hdev, u8 *data, ptrdiff_t len)
 	return ptr;
 }
 
+static struct pending_cmd *mgmt_pending_find(u16 opcode, struct hci_dev *hdev)
+{
+	struct pending_cmd *cmd;
+
+	list_for_each_entry(cmd, &hdev->mgmt_pending, list) {
+		if (cmd->opcode == opcode)
+			return cmd;
+	}
+
+	return NULL;
+}
+
 static u8 create_scan_rsp_data(struct hci_dev *hdev, u8 *ptr)
 {
 	u8 ad_len = 0;
@@ -886,18 +898,6 @@ static void mgmt_pending_foreach(u16 opcode, struct hci_dev *hdev,
 	}
 }
 
-static struct pending_cmd *mgmt_pending_find(u16 opcode, struct hci_dev *hdev)
-{
-	struct pending_cmd *cmd;
-
-	list_for_each_entry(cmd, &hdev->mgmt_pending, list) {
-		if (cmd->opcode == opcode)
-			return cmd;
-	}
-
-	return NULL;
-}
-
 static void mgmt_pending_remove(struct pending_cmd *cmd)
 {
 	list_del(&cmd->list);
-- 
1.8.3.1

[PATCH 7/9] Bluetooth: Fix sending write_scan_enable when BR/EDR is disabled

[johan.hedberg]

From: Johan Hedberg <johan.hedberg@intel.com>

We should only send the HCI_Write_Scan_Enable command from
mgmt_set_powered_failed() when BR/EDR support is enabled. This is
particularly important when the discoverable setting is also tied to LE.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 net/bluetooth/mgmt.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 20f6309..67b95a8 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -4337,7 +4337,6 @@ void mgmt_set_powered_failed(struct hci_dev *hdev, int err)
 void mgmt_discoverable_timeout(struct hci_dev *hdev)
 {
 	struct hci_request req;
-	u8 scan = SCAN_PAGE;
 
 	hci_dev_lock(hdev);
 
@@ -4349,7 +4348,11 @@ void mgmt_discoverable_timeout(struct hci_dev *hdev)
 	clear_bit(HCI_LIMITED_DISCOVERABLE, &hdev->dev_flags);
 
 	hci_req_init(&req, hdev);
-	hci_req_add(&req, HCI_OP_WRITE_SCAN_ENABLE, sizeof(scan), &scan);
+	if (test_bit(HCI_BREDR_ENABLED, &hdev->dev_flags)) {
+		u8 scan = SCAN_PAGE;
+		hci_req_add(&req, HCI_OP_WRITE_SCAN_ENABLE,
+			    sizeof(scan), &scan);
+	}
 	update_class(&req);
 	hci_req_run(&req, NULL);
 
-- 
1.8.3.1

[PATCH 8/9] Bluetooth: Move HCI_LIMITED_DISCOVERABLE changes to a general place

[johan.hedberg]

From: Johan Hedberg <johan.hedberg@intel.com>

We'll soon be introducing also LE support for the Set Discoverable
management command, so move the HCI_LIMITED_DISCOVERABLE flag clearing
and setting out from the if-branch that is only used for a BR/EDR
specific HCI command.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 net/bluetooth/mgmt.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 67b95a8..796db58 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -1220,6 +1220,12 @@ static int set_discoverable(struct sock *sk, struct hci_dev *hdev, void *data,
 	cancel_delayed_work(&hdev->discov_off);
 	hdev->discov_timeout = timeout;
 
+	/* Limited discoverable mode */
+	if (cp->val == 0x02)
+		set_bit(HCI_LIMITED_DISCOVERABLE, &hdev->dev_flags);
+	else
+		clear_bit(HCI_LIMITED_DISCOVERABLE, &hdev->dev_flags);
+
 	hci_req_init(&req, hdev);
 
 	scan = SCAN_PAGE;
@@ -1229,8 +1235,6 @@ static int set_discoverable(struct sock *sk, struct hci_dev *hdev, void *data,
 
 		if (cp->val == 0x02) {
 			/* Limited discoverable mode */
-			set_bit(HCI_LIMITED_DISCOVERABLE, &hdev->dev_flags);
-
 			hci_cp.num_iac = 2;
 			hci_cp.iac_lap[0] = 0x00;	/* LIAC */
 			hci_cp.iac_lap[1] = 0x8b;
@@ -1240,8 +1244,6 @@ static int set_discoverable(struct sock *sk, struct hci_dev *hdev, void *data,
 			hci_cp.iac_lap[5] = 0x9e;
 		} else {
 			/* General discoverable mode */
-			clear_bit(HCI_LIMITED_DISCOVERABLE, &hdev->dev_flags);
-
 			hci_cp.num_iac = 1;
 			hci_cp.iac_lap[0] = 0x33;	/* GIAC */
 			hci_cp.iac_lap[1] = 0x8b;
-- 
1.8.3.1

[PATCH 9/9] Bluetooth: Update Set Discoverable to support LE

[johan.hedberg]

From: Johan Hedberg <johan.hedberg@intel.com>

This patch updates the Set Discoverable management command to also be
applicable for LE. In particular this affects the advertising flags
where we can say "general discoverable" or "limited discoverable".

Since the device flags may not be up-to-date when the advertising data
is written this patch introduces a get_adv_discov_flags() helper
function which also looks at any pending mgmt commands (a pending
set_discoverable would be the exception when the flags are not yet
correct).

The patch also adds HCI_DISCOVERABLE flag clearing to the
mgmt_discoverable_timeout function, since the code was previously
relying on the mgmt_discoverable callback to handle this, which is only
called for the BR/EDR-only HCI_Write_Scan_Enable command.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 net/bluetooth/mgmt.c | 49 +++++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 43 insertions(+), 6 deletions(-)

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 796db58..73ed132 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -599,12 +599,35 @@ static void update_scan_rsp_data(struct hci_request *req)
 	hci_req_add(req, HCI_OP_LE_SET_SCAN_RSP_DATA, sizeof(cp), &cp);
 }
 
+static u8 get_adv_discov_flags(struct hci_dev *hdev)
+{
+	struct pending_cmd *cmd;
+
+	/* If there's a pending mgmt command the flags will not yet have
+	 * their final values, so check for this first.
+	 */
+	cmd = mgmt_pending_find(MGMT_OP_SET_DISCOVERABLE, hdev);
+	if (cmd) {
+		struct mgmt_mode *cp = cmd->param;
+		if (cp->val == 0x01)
+			return LE_AD_GENERAL;
+		else if (cp->val == 0x02)
+			return LE_AD_LIMITED;
+	} else {
+		if (test_bit(HCI_LIMITED_DISCOVERABLE, &hdev->dev_flags))
+			return LE_AD_LIMITED;
+		else if (test_bit(HCI_DISCOVERABLE, &hdev->dev_flags))
+			return LE_AD_GENERAL;
+	}
+
+	return 0;
+}
+
 static u8 create_adv_data(struct hci_dev *hdev, u8 *ptr)
 {
 	u8 ad_len = 0, flags = 0;
 
-	if (test_bit(HCI_ADVERTISING, &hdev->dev_flags))
-		flags |= LE_AD_GENERAL;
+	flags |= get_adv_discov_flags(hdev);
 
 	if (test_bit(HCI_BREDR_ENABLED, &hdev->dev_flags)) {
 		if (lmp_le_br_capable(hdev))
@@ -1120,15 +1143,15 @@ static int set_discoverable(struct sock *sk, struct hci_dev *hdev, void *data,
 	struct pending_cmd *cmd;
 	struct hci_request req;
 	u16 timeout;
-	u8 scan, status;
+	u8 scan;
 	int err;
 
 	BT_DBG("request for %s", hdev->name);
 
-	status = mgmt_bredr_support(hdev);
-	if (status)
+	if (!test_bit(HCI_LE_ENABLED, &hdev->dev_flags) &&
+	    !test_bit(HCI_BREDR_ENABLED, &hdev->dev_flags))
 		return cmd_status(sk, hdev->id, MGMT_OP_SET_DISCOVERABLE,
-				  status);
+				  MGMT_STATUS_REJECTED);
 
 	if (cp->val != 0x00 && cp->val != 0x01 && cp->val != 0x02)
 		return cmd_status(sk, hdev->id, MGMT_OP_SET_DISCOVERABLE,
@@ -1228,6 +1251,12 @@ static int set_discoverable(struct sock *sk, struct hci_dev *hdev, void *data,
 
 	hci_req_init(&req, hdev);
 
+	/* The procedure for LE-only controllers is much simpler - just
+	 * update the advertising data.
+	 */
+	if (!test_bit(HCI_BREDR_ENABLED, &hdev->dev_flags))
+		goto update_ad;
+
 	scan = SCAN_PAGE;
 
 	if (cp->val) {
@@ -1260,6 +1289,10 @@ static int set_discoverable(struct sock *sk, struct hci_dev *hdev, void *data,
 
 	hci_req_add(&req, HCI_OP_WRITE_SCAN_ENABLE, sizeof(scan), &scan);
 
+update_ad:
+	if (test_bit(HCI_LE_ENABLED, &hdev->dev_flags))
+		update_adv_data(&req);
+
 	err = hci_req_run(&req, set_discoverable_complete);
 	if (err < 0)
 		mgmt_pending_remove(cmd);
@@ -4348,6 +4381,7 @@ void mgmt_discoverable_timeout(struct hci_dev *hdev)
 	 * safe to unconditionally clear the flag.
 	 */
 	clear_bit(HCI_LIMITED_DISCOVERABLE, &hdev->dev_flags);
+	clear_bit(HCI_DISCOVERABLE, &hdev->dev_flags);
 
 	hci_req_init(&req, hdev);
 	if (test_bit(HCI_BREDR_ENABLED, &hdev->dev_flags)) {
@@ -4356,10 +4390,13 @@ void mgmt_discoverable_timeout(struct hci_dev *hdev)
 			    sizeof(scan), &scan);
 	}
 	update_class(&req);
+	update_adv_data(&req);
 	hci_req_run(&req, NULL);
 
 	hdev->discov_timeout = 0;
 
+	new_settings(hdev, NULL);
+
 	hci_dev_unlock(hdev);
 }
 
-- 
1.8.3.1

Re: [PATCH 9/9] Bluetooth: Update Set Discoverable to support LE

[Marcel Holtmann]

Hi Johan,

> This patch updates the Set Discoverable management command to also be
> applicable for LE. In particular this affects the advertising flags
> where we can say "general discoverable" or "limited discoverable".
> 
> Since the device flags may not be up-to-date when the advertising data
> is written this patch introduces a get_adv_discov_flags() helper
> function which also looks at any pending mgmt commands (a pending
> set_discoverable would be the exception when the flags are not yet
> correct).
> 
> The patch also adds HCI_DISCOVERABLE flag clearing to the
> mgmt_discoverable_timeout function, since the code was previously
> relying on the mgmt_discoverable callback to handle this, which is only
> called for the BR/EDR-only HCI_Write_Scan_Enable command.
> 
> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> ---
> net/bluetooth/mgmt.c | 49 +++++++++++++++++++++++++++++++++++++++++++------
> 1 file changed, 43 insertions(+), 6 deletions(-)
> 
> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> index 796db58..73ed132 100644
> --- a/net/bluetooth/mgmt.c
> +++ b/net/bluetooth/mgmt.c
> @@ -599,12 +599,35 @@ static void update_scan_rsp_data(struct hci_request *req)
> 	hci_req_add(req, HCI_OP_LE_SET_SCAN_RSP_DATA, sizeof(cp), &cp);
> }
> 
> +static u8 get_adv_discov_flags(struct hci_dev *hdev)
> +{
> +	struct pending_cmd *cmd;
> +
> +	/* If there's a pending mgmt command the flags will not yet have
> +	 * their final values, so check for this first.
> +	 */
> +	cmd = mgmt_pending_find(MGMT_OP_SET_DISCOVERABLE, hdev);
> +	if (cmd) {
> +		struct mgmt_mode *cp = cmd->param;
> +		if (cp->val == 0x01)
> +			return LE_AD_GENERAL;
> +		else if (cp->val == 0x02)
> +			return LE_AD_LIMITED;
> +	} else {
> +		if (test_bit(HCI_LIMITED_DISCOVERABLE, &hdev->dev_flags))
> +			return LE_AD_LIMITED;
> +		else if (test_bit(HCI_DISCOVERABLE, &hdev->dev_flags))
> +			return LE_AD_GENERAL;
> +	}
> +
> +	return 0;
> +}
> +
> static u8 create_adv_data(struct hci_dev *hdev, u8 *ptr)
> {
> 	u8 ad_len = 0, flags = 0;
> 
> -	if (test_bit(HCI_ADVERTISING, &hdev->dev_flags))
> -		flags |= LE_AD_GENERAL;
> +	flags |= get_adv_discov_flags(hdev);
> 
> 	if (test_bit(HCI_BREDR_ENABLED, &hdev->dev_flags)) {
> 		if (lmp_le_br_capable(hdev))
> @@ -1120,15 +1143,15 @@ static int set_discoverable(struct sock *sk, struct hci_dev *hdev, void *data,
> 	struct pending_cmd *cmd;
> 	struct hci_request req;
> 	u16 timeout;
> -	u8 scan, status;
> +	u8 scan;
> 	int err;
> 
> 	BT_DBG("request for %s", hdev->name);
> 
> -	status = mgmt_bredr_support(hdev);
> -	if (status)
> +	if (!test_bit(HCI_LE_ENABLED, &hdev->dev_flags) &&
> +	    !test_bit(HCI_BREDR_ENABLED, &hdev->dev_flags))
> 		return cmd_status(sk, hdev->id, MGMT_OP_SET_DISCOVERABLE,
> -				  status);
> +				  MGMT_STATUS_REJECTED);
> 
> 	if (cp->val != 0x00 && cp->val != 0x01 && cp->val != 0x02)
> 		return cmd_status(sk, hdev->id, MGMT_OP_SET_DISCOVERABLE,
> @@ -1228,6 +1251,12 @@ static int set_discoverable(struct sock *sk, struct hci_dev *hdev, void *data,
> 
> 	hci_req_init(&req, hdev);
> 
> +	/* The procedure for LE-only controllers is much simpler - just
> +	 * update the advertising data.
> +	 */
> +	if (!test_bit(HCI_BREDR_ENABLED, &hdev->dev_flags))
> +		goto update_ad;
> +
> 	scan = SCAN_PAGE;
> 
> 	if (cp->val) {
> @@ -1260,6 +1289,10 @@ static int set_discoverable(struct sock *sk, struct hci_dev *hdev, void *data,
> 
> 	hci_req_add(&req, HCI_OP_WRITE_SCAN_ENABLE, sizeof(scan), &scan);
> 
> +update_ad:
> +	if (test_bit(HCI_LE_ENABLED, &hdev->dev_flags))
> +		update_adv_data(&req);
> +

actually update_adv_data() check for HCI_LE_ENABLED all by itself. So this check is just duplicated and not needed here.

Regards

Marcel

[PATCH v2] Bluetooth: Update Set Discoverable to support LE

[johan.hedberg]

From: Johan Hedberg <johan.hedberg@intel.com>

This patch updates the Set Discoverable management command to also be
applicable for LE. In particular this affects the advertising flags
where we can say "general discoverable" or "limited discoverable".

Since the device flags may not be up-to-date when the advertising data
is written this patch introduces a get_adv_discov_flags() helper
function which also looks at any pending mgmt commands (a pending
set_discoverable would be the exception when the flags are not yet
correct).

The patch also adds HCI_DISCOVERABLE flag clearing to the
mgmt_discoverable_timeout function, since the code was previously
relying on the mgmt_discoverable callback to handle this, which is only
called for the BR/EDR-only HCI_Write_Scan_Enable command.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
v2:
* Fix advertising flags update when disabling connectable while
  discoverable is enabled.
* Fix redundant check for HCI_LE_ENABLED

 net/bluetooth/mgmt.c | 72 +++++++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 63 insertions(+), 9 deletions(-)

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 796db58..5ac0b9a 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -599,12 +599,35 @@ static void update_scan_rsp_data(struct hci_request *req)
 	hci_req_add(req, HCI_OP_LE_SET_SCAN_RSP_DATA, sizeof(cp), &cp);
 }
 
+static u8 get_adv_discov_flags(struct hci_dev *hdev)
+{
+	struct pending_cmd *cmd;
+
+	/* If there's a pending mgmt command the flags will not yet have
+	 * their final values, so check for this first.
+	 */
+	cmd = mgmt_pending_find(MGMT_OP_SET_DISCOVERABLE, hdev);
+	if (cmd) {
+		struct mgmt_mode *cp = cmd->param;
+		if (cp->val == 0x01)
+			return LE_AD_GENERAL;
+		else if (cp->val == 0x02)
+			return LE_AD_LIMITED;
+	} else {
+		if (test_bit(HCI_LIMITED_DISCOVERABLE, &hdev->dev_flags))
+			return LE_AD_LIMITED;
+		else if (test_bit(HCI_DISCOVERABLE, &hdev->dev_flags))
+			return LE_AD_GENERAL;
+	}
+
+	return 0;
+}
+
 static u8 create_adv_data(struct hci_dev *hdev, u8 *ptr)
 {
 	u8 ad_len = 0, flags = 0;
 
-	if (test_bit(HCI_ADVERTISING, &hdev->dev_flags))
-		flags |= LE_AD_GENERAL;
+	flags |= get_adv_discov_flags(hdev);
 
 	if (test_bit(HCI_BREDR_ENABLED, &hdev->dev_flags)) {
 		if (lmp_le_br_capable(hdev))
@@ -1120,15 +1143,15 @@ static int set_discoverable(struct sock *sk, struct hci_dev *hdev, void *data,
 	struct pending_cmd *cmd;
 	struct hci_request req;
 	u16 timeout;
-	u8 scan, status;
+	u8 scan;
 	int err;
 
 	BT_DBG("request for %s", hdev->name);
 
-	status = mgmt_bredr_support(hdev);
-	if (status)
+	if (!test_bit(HCI_LE_ENABLED, &hdev->dev_flags) &&
+	    !test_bit(HCI_BREDR_ENABLED, &hdev->dev_flags))
 		return cmd_status(sk, hdev->id, MGMT_OP_SET_DISCOVERABLE,
-				  status);
+				  MGMT_STATUS_REJECTED);
 
 	if (cp->val != 0x00 && cp->val != 0x01 && cp->val != 0x02)
 		return cmd_status(sk, hdev->id, MGMT_OP_SET_DISCOVERABLE,
@@ -1228,6 +1251,12 @@ static int set_discoverable(struct sock *sk, struct hci_dev *hdev, void *data,
 
 	hci_req_init(&req, hdev);
 
+	/* The procedure for LE-only controllers is much simpler - just
+	 * update the advertising data.
+	 */
+	if (!test_bit(HCI_BREDR_ENABLED, &hdev->dev_flags))
+		goto update_ad;
+
 	scan = SCAN_PAGE;
 
 	if (cp->val) {
@@ -1260,6 +1289,9 @@ static int set_discoverable(struct sock *sk, struct hci_dev *hdev, void *data,
 
 	hci_req_add(&req, HCI_OP_WRITE_SCAN_ENABLE, sizeof(scan), &scan);
 
+update_ad:
+	update_adv_data(&req);
+
 	err = hci_req_run(&req, set_discoverable_complete);
 	if (err < 0)
 		mgmt_pending_remove(cmd);
@@ -1451,8 +1483,15 @@ static int set_connectable(struct sock *sk, struct hci_dev *hdev, void *data,
 
 	hci_req_init(&req, hdev);
 
-	if (test_bit(HCI_BREDR_ENABLED, &hdev->dev_flags) &&
-	    cp->val != test_bit(HCI_PSCAN, &hdev->flags)) {
+	/* If BR/EDR is not enabled and we disable advertising as a
+	 * by-product of disabling connectable, we need to update the
+	 * advertising flags.
+	 */
+	if (!test_bit(HCI_BREDR_ENABLED, &hdev->dev_flags)) {
+		if (!cp->val)
+			clear_bit(HCI_DISCOVERABLE, &hdev->dev_flags);
+		update_adv_data(&req);
+	} else if (cp->val != test_bit(HCI_PSCAN, &hdev->flags)) {
 		if (cp->val) {
 			scan = SCAN_PAGE;
 		} else {
@@ -4348,6 +4387,7 @@ void mgmt_discoverable_timeout(struct hci_dev *hdev)
 	 * safe to unconditionally clear the flag.
 	 */
 	clear_bit(HCI_LIMITED_DISCOVERABLE, &hdev->dev_flags);
+	clear_bit(HCI_DISCOVERABLE, &hdev->dev_flags);
 
 	hci_req_init(&req, hdev);
 	if (test_bit(HCI_BREDR_ENABLED, &hdev->dev_flags)) {
@@ -4356,10 +4396,13 @@ void mgmt_discoverable_timeout(struct hci_dev *hdev)
 			    sizeof(scan), &scan);
 	}
 	update_class(&req);
+	update_adv_data(&req);
 	hci_req_run(&req, NULL);
 
 	hdev->discov_timeout = 0;
 
+	new_settings(hdev, NULL);
+
 	hci_dev_unlock(hdev);
 }
 
@@ -4379,8 +4422,19 @@ void mgmt_discoverable(struct hci_dev *hdev, u8 discoverable)
 	else
 		changed = test_and_clear_bit(HCI_DISCOVERABLE, &hdev->dev_flags);
 
-	if (changed)
+	if (changed) {
+		struct hci_request req;
+
+		/* In case this change in discoverable was triggered by
+		 * a disabling of connectable there could be a need to
+		 * update the advertising flags.
+		 */
+		hci_req_init(&req, hdev);
+		update_adv_data(&req);
+		hci_req_run(&req, NULL);
+
 		new_settings(hdev, NULL);
+	}
 }
 
 void mgmt_connectable(struct hci_dev *hdev, u8 connectable)
-- 
1.8.3.1

[PATCH v3] Bluetooth: Update Set Discoverable to support LE

[johan.hedberg]

From: Johan Hedberg <johan.hedberg@intel.com>

This patch updates the Set Discoverable management command to also be
applicable for LE. In particular this affects the advertising flags
where we can say "general discoverable" or "limited discoverable".

Since the device flags may not be up-to-date when the advertising data
is written this patch introduces a get_adv_discov_flags() helper
function which also looks at any pending mgmt commands (a pending
set_discoverable would be the exception when the flags are not yet
correct).

The patch also adds HCI_DISCOVERABLE flag clearing to the
mgmt_discoverable_timeout function, since the code was previously
relying on the mgmt_discoverable callback to handle this, which is only
called for the BR/EDR-only HCI_Write_Scan_Enable command.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
v3:
 * Fix updating advertising flags when switching connectable off while
   limited discoverable is enabled.
v2:
 * Fix advertising flags update when disabling connectable while
  discoverable is enabled.
 * Fix redundant check for HCI_LE_ENABLED

 net/bluetooth/mgmt.c | 80 ++++++++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 69 insertions(+), 11 deletions(-)

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 796db58..bd91ee5 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -599,12 +599,35 @@ static void update_scan_rsp_data(struct hci_request *req)
 	hci_req_add(req, HCI_OP_LE_SET_SCAN_RSP_DATA, sizeof(cp), &cp);
 }
 
+static u8 get_adv_discov_flags(struct hci_dev *hdev)
+{
+	struct pending_cmd *cmd;
+
+	/* If there's a pending mgmt command the flags will not yet have
+	 * their final values, so check for this first.
+	 */
+	cmd = mgmt_pending_find(MGMT_OP_SET_DISCOVERABLE, hdev);
+	if (cmd) {
+		struct mgmt_mode *cp = cmd->param;
+		if (cp->val == 0x01)
+			return LE_AD_GENERAL;
+		else if (cp->val == 0x02)
+			return LE_AD_LIMITED;
+	} else {
+		if (test_bit(HCI_LIMITED_DISCOVERABLE, &hdev->dev_flags))
+			return LE_AD_LIMITED;
+		else if (test_bit(HCI_DISCOVERABLE, &hdev->dev_flags))
+			return LE_AD_GENERAL;
+	}
+
+	return 0;
+}
+
 static u8 create_adv_data(struct hci_dev *hdev, u8 *ptr)
 {
 	u8 ad_len = 0, flags = 0;
 
-	if (test_bit(HCI_ADVERTISING, &hdev->dev_flags))
-		flags |= LE_AD_GENERAL;
+	flags |= get_adv_discov_flags(hdev);
 
 	if (test_bit(HCI_BREDR_ENABLED, &hdev->dev_flags)) {
 		if (lmp_le_br_capable(hdev))
@@ -1120,15 +1143,15 @@ static int set_discoverable(struct sock *sk, struct hci_dev *hdev, void *data,
 	struct pending_cmd *cmd;
 	struct hci_request req;
 	u16 timeout;
-	u8 scan, status;
+	u8 scan;
 	int err;
 
 	BT_DBG("request for %s", hdev->name);
 
-	status = mgmt_bredr_support(hdev);
-	if (status)
+	if (!test_bit(HCI_LE_ENABLED, &hdev->dev_flags) &&
+	    !test_bit(HCI_BREDR_ENABLED, &hdev->dev_flags))
 		return cmd_status(sk, hdev->id, MGMT_OP_SET_DISCOVERABLE,
-				  status);
+				  MGMT_STATUS_REJECTED);
 
 	if (cp->val != 0x00 && cp->val != 0x01 && cp->val != 0x02)
 		return cmd_status(sk, hdev->id, MGMT_OP_SET_DISCOVERABLE,
@@ -1228,6 +1251,12 @@ static int set_discoverable(struct sock *sk, struct hci_dev *hdev, void *data,
 
 	hci_req_init(&req, hdev);
 
+	/* The procedure for LE-only controllers is much simpler - just
+	 * update the advertising data.
+	 */
+	if (!test_bit(HCI_BREDR_ENABLED, &hdev->dev_flags))
+		goto update_ad;
+
 	scan = SCAN_PAGE;
 
 	if (cp->val) {
@@ -1260,6 +1289,9 @@ static int set_discoverable(struct sock *sk, struct hci_dev *hdev, void *data,
 
 	hci_req_add(&req, HCI_OP_WRITE_SCAN_ENABLE, sizeof(scan), &scan);
 
+update_ad:
+	update_adv_data(&req);
+
 	err = hci_req_run(&req, set_discoverable_complete);
 	if (err < 0)
 		mgmt_pending_remove(cmd);
@@ -1451,8 +1483,17 @@ static int set_connectable(struct sock *sk, struct hci_dev *hdev, void *data,
 
 	hci_req_init(&req, hdev);
 
-	if (test_bit(HCI_BREDR_ENABLED, &hdev->dev_flags) &&
-	    cp->val != test_bit(HCI_PSCAN, &hdev->flags)) {
+	/* If BR/EDR is not enabled and we disable advertising as a
+	 * by-product of disabling connectable, we need to update the
+	 * advertising flags.
+	 */
+	if (!test_bit(HCI_BREDR_ENABLED, &hdev->dev_flags)) {
+		if (!cp->val) {
+			clear_bit(HCI_LIMITED_DISCOVERABLE, &hdev->dev_flags);
+			clear_bit(HCI_DISCOVERABLE, &hdev->dev_flags);
+		}
+		update_adv_data(&req);
+	} else if (cp->val != test_bit(HCI_PSCAN, &hdev->flags)) {
 		if (cp->val) {
 			scan = SCAN_PAGE;
 		} else {
@@ -4348,6 +4389,7 @@ void mgmt_discoverable_timeout(struct hci_dev *hdev)
 	 * safe to unconditionally clear the flag.
 	 */
 	clear_bit(HCI_LIMITED_DISCOVERABLE, &hdev->dev_flags);
+	clear_bit(HCI_DISCOVERABLE, &hdev->dev_flags);
 
 	hci_req_init(&req, hdev);
 	if (test_bit(HCI_BREDR_ENABLED, &hdev->dev_flags)) {
@@ -4356,10 +4398,13 @@ void mgmt_discoverable_timeout(struct hci_dev *hdev)
 			    sizeof(scan), &scan);
 	}
 	update_class(&req);
+	update_adv_data(&req);
 	hci_req_run(&req, NULL);
 
 	hdev->discov_timeout = 0;
 
+	new_settings(hdev, NULL);
+
 	hci_dev_unlock(hdev);
 }
 
@@ -4374,13 +4419,26 @@ void mgmt_discoverable(struct hci_dev *hdev, u8 discoverable)
 	if (mgmt_pending_find(MGMT_OP_SET_DISCOVERABLE, hdev))
 		return;
 
-	if (discoverable)
+	if (discoverable) {
 		changed = !test_and_set_bit(HCI_DISCOVERABLE, &hdev->dev_flags);
-	else
+	} else {
+		clear_bit(HCI_LIMITED_DISCOVERABLE, &hdev->dev_flags);
 		changed = test_and_clear_bit(HCI_DISCOVERABLE, &hdev->dev_flags);
+	}
+
+	if (changed) {
+		struct hci_request req;
+
+		/* In case this change in discoverable was triggered by
+		 * a disabling of connectable there could be a need to
+		 * update the advertising flags.
+		 */
+		hci_req_init(&req, hdev);
+		update_adv_data(&req);
+		hci_req_run(&req, NULL);
 
-	if (changed)
 		new_settings(hdev, NULL);
+	}
 }
 
 void mgmt_connectable(struct hci_dev *hdev, u8 connectable)
-- 
1.8.3.1

Re: [PATCH v3] Bluetooth: Update Set Discoverable to support LE

[Marcel Holtmann]

Hi Johan,

> This patch updates the Set Discoverable management command to also be
> applicable for LE. In particular this affects the advertising flags
> where we can say "general discoverable" or "limited discoverable".
> 
> Since the device flags may not be up-to-date when the advertising data
> is written this patch introduces a get_adv_discov_flags() helper
> function which also looks at any pending mgmt commands (a pending
> set_discoverable would be the exception when the flags are not yet
> correct).
> 
> The patch also adds HCI_DISCOVERABLE flag clearing to the
> mgmt_discoverable_timeout function, since the code was previously
> relying on the mgmt_discoverable callback to handle this, which is only
> called for the BR/EDR-only HCI_Write_Scan_Enable command.
> 
> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> ---
> v3:
> * Fix updating advertising flags when switching connectable off while
>   limited discoverable is enabled.
> v2:
> * Fix advertising flags update when disabling connectable while
>  discoverable is enabled.
> * Fix redundant check for HCI_LE_ENABLED
> 
> net/bluetooth/mgmt.c | 80 ++++++++++++++++++++++++++++++++++++++++++++--------
> 1 file changed, 69 insertions(+), 11 deletions(-)

all 9 patches have been applied to bluetooth-next tree.

Regards

Marcel