[PATCH 0/7] Bluetooth: Hook up connectable setting for LE

[PATCH 0/7] Bluetooth: Hook up connectable setting for LE

[johan.hedberg]

Hi,

This set of patches adds hooks to associsate the connectable setting
with the LE advertising type. This means that we can now do
non-connectable advertising by enabling the advertising setting but
having the connectable setting disabled.

Johan

----------------------------------------------------------------
Johan Hedberg (7):
      Bluetooth: Add connectable argument to enable_advertising()
      Bluetooth: Reorganize set_connectable HCI command sending
      Bluetooth: Move static advertising functions to avoid forward declarations
      Bluetooth: Fix updating advertising data needlessly
      Bluetooth: Update Set Connectable to also impact the advertising type
      Bluetooth: Move set_bredr_scan() to avoid forward declaration
      Bluetooth: Fix updating scan mode in set_bredr()

 net/bluetooth/hci_event.c |   2 +-
 net/bluetooth/mgmt.c      | 169 ++++++++++++++++++++++++++--------------------
 2 files changed, 98 insertions(+), 73 deletions(-)

[PATCH 1/7] Bluetooth: Add connectable argument to enable_advertising()

[johan.hedberg]

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

We want to be able to control whether enable_advertising() enables
connectable or non-connectable advertising based on the connectable
setting of the adapter. We could have the function check directly for
the HCI_CONNECTABLE flag, but since the flag may not be set or unset
before we call the function it's necessary to pass an explicit parameter
value to it.

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

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 020f95b..6f07523 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -1418,7 +1418,7 @@ unlock:
 	return err;
 }
 
-static void enable_advertising(struct hci_request *req)
+static void enable_advertising(struct hci_request *req, bool connectable)
 {
 	struct hci_dev *hdev = req->hdev;
 	struct hci_cp_le_set_adv_param cp;
@@ -1427,7 +1427,10 @@ static void enable_advertising(struct hci_request *req)
 	memset(&cp, 0, sizeof(cp));
 	cp.min_interval = __constant_cpu_to_le16(0x0800);
 	cp.max_interval = __constant_cpu_to_le16(0x0800);
-	cp.type = LE_ADV_IND;
+	if (connectable)
+		cp.type = LE_ADV_IND;
+	else
+		cp.type = LE_ADV_NONCONN_IND;
 	if (bacmp(&hdev->bdaddr, BDADDR_ANY))
 		cp.own_address_type = ADDR_LE_DEV_PUBLIC;
 	else
@@ -3323,7 +3326,8 @@ static int set_advertising(struct sock *sk, struct hci_dev *hdev, void *data,
 	hci_req_init(&req, hdev);
 
 	if (val)
-		enable_advertising(&req);
+		enable_advertising(&req,
+				   test_bit(HCI_CONNECTABLE, &hdev->dev_flags));
 	else
 		disable_advertising(&req);
 
@@ -3948,7 +3952,9 @@ static int powered_update_hci(struct hci_dev *hdev)
 				    &hdev->static_addr);
 
 		if (test_bit(HCI_ADVERTISING, &hdev->dev_flags))
-			enable_advertising(&req);
+			enable_advertising(&req,
+					   test_bit(HCI_CONNECTABLE,
+						    &hdev->dev_flags));
 	}
 
 	link_sec = test_bit(HCI_LINK_SECURITY, &hdev->dev_flags);
@@ -4735,7 +4741,7 @@ void mgmt_reenable_advertising(struct hci_dev *hdev)
 		return;
 
 	hci_req_init(&req, hdev);
-	enable_advertising(&req);
+	enable_advertising(&req, test_bit(HCI_CONNECTABLE, &hdev->dev_flags));
 
 	/* If this fails we have no option but to let user space know
 	 * that we've disabled advertising.
-- 
1.8.3.1

Re: [PATCH 1/7] Bluetooth: Add connectable argument to enable_advertising()

[Marcel Holtmann]

Hi Johan,

> We want to be able to control whether enable_advertising() enables
> connectable or non-connectable advertising based on the connectable
> setting of the adapter. We could have the function check directly for
> the HCI_CONNECTABLE flag, but since the flag may not be set or unset
> before we call the function it's necessary to pass an explicit parameter
> value to it.
> 
> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> ---
> net/bluetooth/mgmt.c | 16 +++++++++++-----
> 1 file changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> index 020f95b..6f07523 100644
> --- a/net/bluetooth/mgmt.c
> +++ b/net/bluetooth/mgmt.c
> @@ -1418,7 +1418,7 @@ unlock:
> 	return err;
> }
> 
> -static void enable_advertising(struct hci_request *req)
> +static void enable_advertising(struct hci_request *req, bool connectable)
> {
> 	struct hci_dev *hdev = req->hdev;
> 	struct hci_cp_le_set_adv_param cp;
> @@ -1427,7 +1427,10 @@ static void enable_advertising(struct hci_request *req)
> 	memset(&cp, 0, sizeof(cp));
> 	cp.min_interval = __constant_cpu_to_le16(0x0800);
> 	cp.max_interval = __constant_cpu_to_le16(0x0800);
> -	cp.type = LE_ADV_IND;
> +	if (connectable)
> +		cp.type = LE_ADV_IND;
> +	else
> +		cp.type = LE_ADV_NONCONN_IND;

this is the part that I do not like. Having the extra parameter seems rather pointless. We could check for the HCI_CONNECTABLE bit in dev_flags right here.

> 	if (bacmp(&hdev->bdaddr, BDADDR_ANY))
> 		cp.own_address_type = ADDR_LE_DEV_PUBLIC;
> 	else
> @@ -3323,7 +3326,8 @@ static int set_advertising(struct sock *sk, struct hci_dev *hdev, void *data,
> 	hci_req_init(&req, hdev);
> 
> 	if (val)
> -		enable_advertising(&req);
> +		enable_advertising(&req,
> +				   test_bit(HCI_CONNECTABLE, &hdev->dev_flags));
> 	else
> 		disable_advertising(&req);
> 
> @@ -3948,7 +3952,9 @@ static int powered_update_hci(struct hci_dev *hdev)
> 				    &hdev->static_addr);
> 
> 		if (test_bit(HCI_ADVERTISING, &hdev->dev_flags))
> -			enable_advertising(&req);
> +			enable_advertising(&req,
> +					   test_bit(HCI_CONNECTABLE,
> +						    &hdev->dev_flags));
> 	}
> 
> 	link_sec = test_bit(HCI_LINK_SECURITY, &hdev->dev_flags);
> @@ -4735,7 +4741,7 @@ void mgmt_reenable_advertising(struct hci_dev *hdev)
> 		return;
> 
> 	hci_req_init(&req, hdev);
> -	enable_advertising(&req);
> +	enable_advertising(&req, test_bit(HCI_CONNECTABLE, &hdev->dev_flags));

Regards

Marcel

Re: [PATCH 1/7] Bluetooth: Add connectable argument to enable_advertising()

[Johan Hedberg]

Hi Marcel,

On Mon, Oct 14, 2013, Marcel Holtmann wrote:
> > We want to be able to control whether enable_advertising() enables
> > connectable or non-connectable advertising based on the connectable
> > setting of the adapter. We could have the function check directly for
> > the HCI_CONNECTABLE flag, but since the flag may not be set or unset
> > before we call the function it's necessary to pass an explicit parameter
> > value to it.
> > 
> > Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> > ---
> > net/bluetooth/mgmt.c | 16 +++++++++++-----
> > 1 file changed, 11 insertions(+), 5 deletions(-)
> > 
> > diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> > index 020f95b..6f07523 100644
> > --- a/net/bluetooth/mgmt.c
> > +++ b/net/bluetooth/mgmt.c
> > @@ -1418,7 +1418,7 @@ unlock:
> > 	return err;
> > }
> > 
> > -static void enable_advertising(struct hci_request *req)
> > +static void enable_advertising(struct hci_request *req, bool connectable)
> > {
> > 	struct hci_dev *hdev = req->hdev;
> > 	struct hci_cp_le_set_adv_param cp;
> > @@ -1427,7 +1427,10 @@ static void enable_advertising(struct hci_request *req)
> > 	memset(&cp, 0, sizeof(cp));
> > 	cp.min_interval = __constant_cpu_to_le16(0x0800);
> > 	cp.max_interval = __constant_cpu_to_le16(0x0800);
> > -	cp.type = LE_ADV_IND;
> > +	if (connectable)
> > +		cp.type = LE_ADV_IND;
> > +	else
> > +		cp.type = LE_ADV_NONCONN_IND;
> 
> this is the part that I do not like. Having the extra parameter seems
> rather pointless. We could check for the HCI_CONNECTABLE bit in
> dev_flags right here.

This is what I tried to explain in the commit message. When we do
set_connectable() we want to call enable_advertising() with an explicit
value since the HCI_CONNECTABLE flag gets toggled only when the HCI
commands have been successful.

Johan

Re: [PATCH 1/7] Bluetooth: Add connectable argument to enable_advertising()

[Marcel Holtmann]

Hi Johan,

>>> We want to be able to control whether enable_advertising() enables
>>> connectable or non-connectable advertising based on the connectable
>>> setting of the adapter. We could have the function check directly for
>>> the HCI_CONNECTABLE flag, but since the flag may not be set or unset
>>> before we call the function it's necessary to pass an explicit parameter
>>> value to it.
>>> 
>>> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
>>> ---
>>> net/bluetooth/mgmt.c | 16 +++++++++++-----
>>> 1 file changed, 11 insertions(+), 5 deletions(-)
>>> 
>>> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
>>> index 020f95b..6f07523 100644
>>> --- a/net/bluetooth/mgmt.c
>>> +++ b/net/bluetooth/mgmt.c
>>> @@ -1418,7 +1418,7 @@ unlock:
>>> 	return err;
>>> }
>>> 
>>> -static void enable_advertising(struct hci_request *req)
>>> +static void enable_advertising(struct hci_request *req, bool connectable)
>>> {
>>> 	struct hci_dev *hdev = req->hdev;
>>> 	struct hci_cp_le_set_adv_param cp;
>>> @@ -1427,7 +1427,10 @@ static void enable_advertising(struct hci_request *req)
>>> 	memset(&cp, 0, sizeof(cp));
>>> 	cp.min_interval = __constant_cpu_to_le16(0x0800);
>>> 	cp.max_interval = __constant_cpu_to_le16(0x0800);
>>> -	cp.type = LE_ADV_IND;
>>> +	if (connectable)
>>> +		cp.type = LE_ADV_IND;
>>> +	else
>>> +		cp.type = LE_ADV_NONCONN_IND;
>> 
>> this is the part that I do not like. Having the extra parameter seems
>> rather pointless. We could check for the HCI_CONNECTABLE bit in
>> dev_flags right here.
> 
> This is what I tried to explain in the commit message. When we do
> set_connectable() we want to call enable_advertising() with an explicit
> value since the HCI_CONNECTABLE flag gets toggled only when the HCI
> commands have been successful.

I have a bad feeling with doing it like this. So I rather try to avoid this. We might need an internal difference between BR/EDR connectable and LE connectable. So that at least we can track this internally independent.

The whole feeding test_bit() as parameter is getting out of control quickly. So I would propose to have one extra flag that gets set.

Regards

Marcel

[PATCH 2/7] Bluetooth: Reorganize set_connectable HCI command sending

[johan.hedberg]

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

This patch moves all the decisions of which HCI commands to send (or not
to send) to the code between hci_req_init() and hci_req_run() this
allows us to further extend the request with further commands but still
keep the same logic of handling whether to return a direct mgmt response
in the case that no HCI commands were sent.

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

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 6f07523..982b397 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -1144,30 +1144,29 @@ static int set_connectable(struct sock *sk, struct hci_dev *hdev, void *data,
 		goto failed;
 	}
 
-	if (!!cp->val == test_bit(HCI_PSCAN, &hdev->flags)) {
-		err = send_settings_rsp(sk, MGMT_OP_SET_CONNECTABLE, hdev);
-		goto failed;
-	}
-
 	cmd = mgmt_pending_add(sk, MGMT_OP_SET_CONNECTABLE, hdev, data, len);
 	if (!cmd) {
 		err = -ENOMEM;
 		goto failed;
 	}
 
-	if (cp->val) {
-		scan = SCAN_PAGE;
-	} else {
-		scan = 0;
+	hci_req_init(&req, hdev);
 
-		if (test_bit(HCI_ISCAN, &hdev->flags) &&
-		    hdev->discov_timeout > 0)
-			cancel_delayed_work(&hdev->discov_off);
-	}
+	if (test_bit(HCI_BREDR_ENABLED, &hdev->dev_flags) &&
+	    cp->val != test_bit(HCI_PSCAN, &hdev->flags)) {
 
-	hci_req_init(&req, hdev);
+		if (cp->val) {
+			scan = SCAN_PAGE;
+		} else {
+			scan = 0;
 
-	hci_req_add(&req, HCI_OP_WRITE_SCAN_ENABLE, 1, &scan);
+			if (test_bit(HCI_ISCAN, &hdev->flags) &&
+					hdev->discov_timeout > 0)
+				cancel_delayed_work(&hdev->discov_off);
+		}
+
+		hci_req_add(&req, HCI_OP_WRITE_SCAN_ENABLE, 1, &scan);
+	}
 
 	/* If we're going from non-connectable to connectable or
 	 * vice-versa when fast connectable is enabled ensure that fast
@@ -1179,8 +1178,13 @@ static int set_connectable(struct sock *sk, struct hci_dev *hdev, void *data,
 		write_fast_connectable(&req, false);
 
 	err = hci_req_run(&req, set_connectable_complete);
-	if (err < 0)
+	if (err < 0) {
 		mgmt_pending_remove(cmd);
+		if (err == -ENODATA)
+			err = send_settings_rsp(sk, MGMT_OP_SET_CONNECTABLE,
+						hdev);
+		goto failed;
+	}
 
 failed:
 	hci_dev_unlock(hdev);
-- 
1.8.3.1

[PATCH 3/7] Bluetooth: Move static advertising functions to avoid forward declarations

[johan.hedberg]

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

These functions will soon be used by set_connectable() so move them to a
location in mgmt.c that doesn't require forward declarations.

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

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 982b397..0200c7e 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -1072,6 +1072,37 @@ static void write_fast_connectable(struct hci_request *req, bool enable)
 		hci_req_add(req, HCI_OP_WRITE_PAGE_SCAN_TYPE, 1, &type);
 }
 
+static void enable_advertising(struct hci_request *req, bool connectable)
+{
+	struct hci_dev *hdev = req->hdev;
+	struct hci_cp_le_set_adv_param cp;
+	u8 enable = 0x01;
+
+	memset(&cp, 0, sizeof(cp));
+	cp.min_interval = __constant_cpu_to_le16(0x0800);
+	cp.max_interval = __constant_cpu_to_le16(0x0800);
+	if (connectable)
+		cp.type = LE_ADV_IND;
+	else
+		cp.type = LE_ADV_NONCONN_IND;
+	if (bacmp(&hdev->bdaddr, BDADDR_ANY))
+		cp.own_address_type = ADDR_LE_DEV_PUBLIC;
+	else
+		cp.own_address_type = ADDR_LE_DEV_RANDOM;
+	cp.channel_map = 0x07;
+
+	hci_req_add(req, HCI_OP_LE_SET_ADV_PARAM, sizeof(cp), &cp);
+
+	hci_req_add(req, HCI_OP_LE_SET_ADV_ENABLE, sizeof(enable), &enable);
+}
+
+static void disable_advertising(struct hci_request *req)
+{
+	u8 enable = 0x00;
+
+	hci_req_add(req, HCI_OP_LE_SET_ADV_ENABLE, sizeof(enable), &enable);
+}
+
 static void set_connectable_complete(struct hci_dev *hdev, u8 status)
 {
 	struct pending_cmd *cmd;
@@ -1422,37 +1453,6 @@ unlock:
 	return err;
 }
 
-static void enable_advertising(struct hci_request *req, bool connectable)
-{
-	struct hci_dev *hdev = req->hdev;
-	struct hci_cp_le_set_adv_param cp;
-	u8 enable = 0x01;
-
-	memset(&cp, 0, sizeof(cp));
-	cp.min_interval = __constant_cpu_to_le16(0x0800);
-	cp.max_interval = __constant_cpu_to_le16(0x0800);
-	if (connectable)
-		cp.type = LE_ADV_IND;
-	else
-		cp.type = LE_ADV_NONCONN_IND;
-	if (bacmp(&hdev->bdaddr, BDADDR_ANY))
-		cp.own_address_type = ADDR_LE_DEV_PUBLIC;
-	else
-		cp.own_address_type = ADDR_LE_DEV_RANDOM;
-	cp.channel_map = 0x07;
-
-	hci_req_add(req, HCI_OP_LE_SET_ADV_PARAM, sizeof(cp), &cp);
-
-	hci_req_add(req, HCI_OP_LE_SET_ADV_ENABLE, sizeof(enable), &enable);
-}
-
-static void disable_advertising(struct hci_request *req)
-{
-	u8 enable = 0x00;
-
-	hci_req_add(req, HCI_OP_LE_SET_ADV_ENABLE, sizeof(enable), &enable);
-}
-
 static void le_enable_complete(struct hci_dev *hdev, u8 status)
 {
 	struct cmd_lookup match = { NULL, hdev };
-- 
1.8.3.1

[PATCH 4/7] Bluetooth: Fix updating advertising data needlessly

[johan.hedberg]

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

We need to ensure that the advertising data is up-to-date whenever
advertising is enabled, but when disabling advertising we do not need to
worry about it (since it will eventually get fixed as soon as
advertising is enabled again). This patch fixes this in the command
complete callback for set_adv_enable.

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

diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index aa6fed3..da2bc3d 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -924,7 +924,7 @@ static void hci_cc_le_set_adv_enable(struct hci_dev *hdev, struct sk_buff *skb)
 			clear_bit(HCI_ADVERTISING, &hdev->dev_flags);
 	}
 
-	if (!test_bit(HCI_INIT, &hdev->flags)) {
+	if (*sent && !test_bit(HCI_INIT, &hdev->flags)) {
 		struct hci_request req;
 
 		hci_req_init(&req, hdev);
-- 
1.8.3.1

[PATCH 5/7] Bluetooth: Update Set Connectable to also impact the advertising type

[johan.hedberg]

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

This patch updates the Set Connectable Management command to also update
the advertising type to either connectable or non-connectable
advertising. Extra care needs to be taken when BR/EDR is disabled since
then we need to have an extra flip of the HCI_CONNECTABLE flag in the
set_bredr() handler as well as an extra new_settings() call in the
request completion callback.

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

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 0200c7e..b8feb8a 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -1115,6 +1115,9 @@ static void set_connectable_complete(struct hci_dev *hdev, u8 status)
 	if (!cmd)
 		goto unlock;
 
+	if (!test_bit(HCI_BREDR_ENABLED, &hdev->dev_flags))
+		new_settings(hdev, cmd->sk);
+
 	send_settings_rsp(cmd->sk, MGMT_OP_SET_CONNECTABLE, hdev);
 
 	mgmt_pending_remove(cmd);
@@ -1129,15 +1132,15 @@ static int set_connectable(struct sock *sk, struct hci_dev *hdev, void *data,
 	struct mgmt_mode *cp = data;
 	struct pending_cmd *cmd;
 	struct hci_request req;
-	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_CONNECTABLE,
-				  status);
+				  MGMT_STATUS_REJECTED);
 
 	if (cp->val != 0x00 && cp->val != 0x01)
 		return cmd_status(sk, hdev->id, MGMT_OP_SET_CONNECTABLE,
@@ -1208,6 +1211,14 @@ static int set_connectable(struct sock *sk, struct hci_dev *hdev, void *data,
 	if (cp->val || test_bit(HCI_FAST_CONNECTABLE, &hdev->dev_flags))
 		write_fast_connectable(&req, false);
 
+	if (test_bit(HCI_ADVERTISING, &hdev->dev_flags) &&
+	    hci_conn_num(hdev, LE_LINK) == 0) {
+		if (!test_bit(HCI_BREDR_ENABLED, &hdev->dev_flags))
+			change_bit(HCI_CONNECTABLE, &hdev->dev_flags);
+		disable_advertising(&req);
+		enable_advertising(&req, cp->val);
+	}
+
 	err = hci_req_run(&req, set_connectable_complete);
 	if (err < 0) {
 		mgmt_pending_remove(cmd);
-- 
1.8.3.1

[PATCH 6/7] Bluetooth: Move set_bredr_scan() to avoid forward declaration

[johan.hedberg]

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

The set_bredr_scan() function will soon be needed by the set_bredr()
function, so move it to a new location to avoid having to add a forward
declaration.

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

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index b8feb8a..4f3fd50 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -3530,6 +3530,26 @@ unlock:
 	return err;
 }
 
+static void set_bredr_scan(struct hci_request *req)
+{
+	struct hci_dev *hdev = req->hdev;
+	u8 scan = 0;
+
+	/* Ensure that fast connectable is disabled. This function will
+	 * not do anything if the page scan parameters are already what
+	 * they should be.
+	 */
+	write_fast_connectable(req, false);
+
+	if (test_bit(HCI_CONNECTABLE, &hdev->dev_flags))
+		scan |= SCAN_PAGE;
+	if (test_bit(HCI_DISCOVERABLE, &hdev->dev_flags))
+		scan |= SCAN_INQUIRY;
+
+	if (scan)
+		hci_req_add(req, HCI_OP_WRITE_SCAN_ENABLE, 1, &scan);
+}
+
 static void set_bredr_complete(struct hci_dev *hdev, u8 status)
 {
 	struct pending_cmd *cmd;
@@ -3889,26 +3909,6 @@ void mgmt_index_removed(struct hci_dev *hdev)
 	mgmt_event(MGMT_EV_INDEX_REMOVED, hdev, NULL, 0, NULL);
 }
 
-static void set_bredr_scan(struct hci_request *req)
-{
-	struct hci_dev *hdev = req->hdev;
-	u8 scan = 0;
-
-	/* Ensure that fast connectable is disabled. This function will
-	 * not do anything if the page scan parameters are already what
-	 * they should be.
-	 */
-	write_fast_connectable(req, false);
-
-	if (test_bit(HCI_CONNECTABLE, &hdev->dev_flags))
-		scan |= SCAN_PAGE;
-	if (test_bit(HCI_DISCOVERABLE, &hdev->dev_flags))
-		scan |= SCAN_INQUIRY;
-
-	if (scan)
-		hci_req_add(req, HCI_OP_WRITE_SCAN_ENABLE, 1, &scan);
-}
-
 static void powered_complete(struct hci_dev *hdev, u8 status)
 {
 	struct cmd_lookup match = { NULL, hdev };
-- 
1.8.3.1

[PATCH 7/7] Bluetooth: Fix updating scan mode in set_bredr()

[johan.hedberg]

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

Now that the connectable setting is also applicable for the LE side it's
possible that the HCI_CONNECTABLE flag is already set when changing the
BR/EDR setting from false to true while the controller is powered. In
this situation we need to update the BR/EDR scan mode to reflect the
setting. Additionally, since HCI_CONNECTABLE also applies to LE we must
not clear the HCI_CONNECTABLE flag when disabling bredr.

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

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 4f3fd50..abaa58a 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -3612,7 +3612,6 @@ static int set_bredr(struct sock *sk, struct hci_dev *hdev, void *data, u16 len)
 
 	if (!hdev_is_powered(hdev)) {
 		if (!cp->val) {
-			clear_bit(HCI_CONNECTABLE, &hdev->dev_flags);
 			clear_bit(HCI_DISCOVERABLE, &hdev->dev_flags);
 			clear_bit(HCI_SSP_ENABLED, &hdev->dev_flags);
 			clear_bit(HCI_LINK_SECURITY, &hdev->dev_flags);
@@ -3655,7 +3654,12 @@ static int set_bredr(struct sock *sk, struct hci_dev *hdev, void *data, u16 len)
 	set_bit(HCI_BREDR_ENABLED, &hdev->dev_flags);
 
 	hci_req_init(&req, hdev);
+
+	if (test_bit(HCI_CONNECTABLE, &hdev->dev_flags))
+		set_bredr_scan(&req);
+
 	hci_update_ad(&req);
+
 	err = hci_req_run(&req, set_bredr_complete);
 	if (err < 0)
 		mgmt_pending_remove(cmd);
-- 
1.8.3.1