[PATCH 0/7] Bluetooth: Graceful power off procedure

[PATCH 0/7] Bluetooth: Graceful power off procedure

[johan.hedberg]

Hi,

This set of patches updates the power off procedure (as triggered
through mgmt) to perform a graceful power off by first cleaning up HCI
state such as connections, scan modes and advertising before requesting
the HCI driver to actually power off.

Johan

----------------------------------------------------------------
Johan Hedberg (7):
      Bluetooth: Add convenience function for getting total connection count
      Bluetooth: Move HCI_ADVERTISING handling into mgmt.c
      Bluetooth: Move check for MGMT_CONNECTED flag into mgmt.c
      Bluetooth: Don't clear HCI_DISCOVERABLE when powering off
      Bluetooth: Don't clear HCI_CONNECTABLE when powering off
      Bluetooth: Don't clear HCI_ADVERTISING when powering off
      Bluetooth: Clean up HCI state when doing power off

 include/net/bluetooth/hci_core.h | 11 ++++-
 net/bluetooth/hci_event.c        | 15 +++---
 net/bluetooth/mgmt.c             | 96 ++++++++++++++++++++++++++++++++++--
 3 files changed, 107 insertions(+), 15 deletions(-)

[PATCH 1/7] Bluetooth: Add convenience function for getting total connection count

[johan.hedberg]

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

This patch adds a convenience function to return the number of
connections in the conn_hash list. This will be useful once we update
the power off procedure to disconnect any open connections.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 include/net/bluetooth/hci_core.h | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index fb3b677ff8a6..d2d756753714 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -561,6 +561,13 @@ static inline unsigned int hci_conn_num(struct hci_dev *hdev, __u8 type)
 	}
 }
 
+static inline unsigned int hci_conn_count(struct hci_dev *hdev)
+{
+	struct hci_conn_hash *c = &hdev->conn_hash;
+
+	return c->acl_num + c->amp_num + c->sco_num + c->le_num;
+}
+
 static inline struct hci_conn *hci_conn_hash_lookup_handle(struct hci_dev *hdev,
 								__u16 handle)
 {
-- 
1.8.5.3

[PATCH 2/7] Bluetooth: Move HCI_ADVERTISING handling into mgmt.c

[johan.hedberg]

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

We'll soon need to make decisions on toggling the HCI_ADVERTISING flag
based on pending mgmt_set_powered commands. Therefore, move the handling
from hci_event.c into mgmt.c.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 include/net/bluetooth/hci_core.h | 1 +
 net/bluetooth/hci_event.c        | 8 ++------
 net/bluetooth/mgmt.c             | 8 ++++++++
 3 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index d2d756753714..6ff882e727d4 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -1174,6 +1174,7 @@ int mgmt_powered(struct hci_dev *hdev, u8 powered);
 void mgmt_discoverable_timeout(struct hci_dev *hdev);
 void mgmt_discoverable(struct hci_dev *hdev, u8 discoverable);
 void mgmt_connectable(struct hci_dev *hdev, u8 connectable);
+void mgmt_advertising(struct hci_dev *hdev, u8 advertising);
 void mgmt_write_scan_failed(struct hci_dev *hdev, u8 scan, u8 status);
 void mgmt_new_link_key(struct hci_dev *hdev, struct link_key *key,
 		       bool persistent);
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 064d619344b3..dea465ba276b 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -991,12 +991,8 @@ static void hci_cc_le_set_adv_enable(struct hci_dev *hdev, struct sk_buff *skb)
 
 	hci_dev_lock(hdev);
 
-	if (!status) {
-		if (*sent)
-			set_bit(HCI_ADVERTISING, &hdev->dev_flags);
-		else
-			clear_bit(HCI_ADVERTISING, &hdev->dev_flags);
-	}
+	if (!status)
+		mgmt_advertising(hdev, *sent);
 
 	hci_dev_unlock(hdev);
 }
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 9865e523df20..d39e57e9fed6 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -4832,6 +4832,14 @@ void mgmt_connectable(struct hci_dev *hdev, u8 connectable)
 		new_settings(hdev, NULL);
 }
 
+void mgmt_advertising(struct hci_dev *hdev, u8 advertising)
+{
+	if (advertising)
+		set_bit(HCI_ADVERTISING, &hdev->dev_flags);
+	else
+		clear_bit(HCI_ADVERTISING, &hdev->dev_flags);
+}
+
 void mgmt_write_scan_failed(struct hci_dev *hdev, u8 scan, u8 status)
 {
 	u8 mgmt_err = mgmt_status(status);
-- 
1.8.5.3

[PATCH 3/7] Bluetooth: Move check for MGMT_CONNECTED flag into mgmt.c

[johan.hedberg]

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

Once mgmt_set_powered(off) starts doing disconnections we'll need to
care about any disconnections in mgmt.c and not just those with the
MGMT_CONNECTED flag set. Therefore, move the check into mgmt.c from
hci_event.c.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 include/net/bluetooth/hci_core.h | 3 ++-
 net/bluetooth/hci_event.c        | 7 ++++---
 net/bluetooth/mgmt.c             | 6 +++++-
 3 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 6ff882e727d4..269c8201a362 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -1182,7 +1182,8 @@ void mgmt_device_connected(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 link_type,
 			   u8 addr_type, u32 flags, u8 *name, u8 name_len,
 			   u8 *dev_class);
 void mgmt_device_disconnected(struct hci_dev *hdev, bdaddr_t *bdaddr,
-			      u8 link_type, u8 addr_type, u8 reason);
+			      u8 link_type, u8 addr_type, u8 reason,
+			      bool mgmt_connected);
 void mgmt_disconnect_failed(struct hci_dev *hdev, bdaddr_t *bdaddr,
 			    u8 link_type, u8 addr_type, u8 status);
 void mgmt_connect_failed(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 link_type,
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index dea465ba276b..877cee844b9e 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -1842,6 +1842,7 @@ static void hci_disconn_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
 	struct hci_ev_disconn_complete *ev = (void *) skb->data;
 	u8 reason = hci_to_mgmt_reason(ev->reason);
 	struct hci_conn *conn;
+	bool mgmt_connected;
 	u8 type;
 
 	BT_DBG("%s status 0x%2.2x", hdev->name, ev->status);
@@ -1860,9 +1861,9 @@ static void hci_disconn_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
 
 	conn->state = BT_CLOSED;
 
-	if (test_and_clear_bit(HCI_CONN_MGMT_CONNECTED, &conn->flags))
-		mgmt_device_disconnected(hdev, &conn->dst, conn->type,
-					 conn->dst_type, reason);
+	mgmt_connected = test_and_clear_bit(HCI_CONN_MGMT_CONNECTED, &conn->flags);
+	mgmt_device_disconnected(hdev, &conn->dst, conn->type, conn->dst_type,
+				reason, mgmt_connected);
 
 	if (conn->type == ACL_LINK && conn->flush_key)
 		hci_remove_link_key(hdev, &conn->dst);
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index d39e57e9fed6..bdc831b3bb97 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -5012,11 +5012,15 @@ static void unpair_device_rsp(struct pending_cmd *cmd, void *data)
 }
 
 void mgmt_device_disconnected(struct hci_dev *hdev, bdaddr_t *bdaddr,
-			      u8 link_type, u8 addr_type, u8 reason)
+			      u8 link_type, u8 addr_type, u8 reason,
+			      bool mgmt_connected)
 {
 	struct mgmt_ev_device_disconnected ev;
 	struct sock *sk = NULL;
 
+	if (!mgmt_connected)
+		return;
+
 	if (link_type != ACL_LINK && link_type != LE_LINK)
 		return;
 
-- 
1.8.5.3

[PATCH 4/7] Bluetooth: Don't clear HCI_DISCOVERABLE when powering off

[johan.hedberg]

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

Once mgmt_set_powered(off) is updated to clear the scan mode we should
not just blindly clear the HCI_DISCOVERABLE flag in mgmt_discoverable()
but first check if there is a pending set_powered operation.

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

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index bdc831b3bb97..769b5dc0270d 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -4790,6 +4790,10 @@ void mgmt_discoverable(struct hci_dev *hdev, u8 discoverable)
 	if (mgmt_pending_find(MGMT_OP_SET_DISCOVERABLE, hdev))
 		return;
 
+	/* Powering off may clear the scan mode - don't let that interfere */
+	if (!discoverable && mgmt_pending_find(MGMT_OP_SET_POWERED, hdev))
+		return;
+
 	if (discoverable) {
 		changed = !test_and_set_bit(HCI_DISCOVERABLE, &hdev->dev_flags);
 	} else {
-- 
1.8.5.3

[PATCH 5/7] Bluetooth: Don't clear HCI_CONNECTABLE when powering off

[johan.hedberg]

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

Once mgmt_set_powered(off) is updated to clear the scan mode we should
not just blindly clear the HCI_CONNECTABLE flag in mgmt_connectable()
but first check if there is a pending set_powered operation.

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

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 769b5dc0270d..5899ac7264ff 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -4827,6 +4827,10 @@ void mgmt_connectable(struct hci_dev *hdev, u8 connectable)
 	if (mgmt_pending_find(MGMT_OP_SET_CONNECTABLE, hdev))
 		return;
 
+	/* Powering off may clear the scan mode - don't let that interfere */
+	if (!connectable && mgmt_pending_find(MGMT_OP_SET_POWERED, hdev))
+		return;
+
 	if (connectable)
 		changed = !test_and_set_bit(HCI_CONNECTABLE, &hdev->dev_flags);
 	else
-- 
1.8.5.3

[PATCH 6/7] Bluetooth: Don't clear HCI_ADVERTISING when powering off

[johan.hedberg]

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

Once mgmt_set_powered(off) is updated to clear the scan mode we should
not just blindly clear the HCI_ADVERTISING flag in mgmt_advertising()
but first check if there is a pending set_powered operation.

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

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 5899ac7264ff..610ac32e797b 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -4842,6 +4842,10 @@ void mgmt_connectable(struct hci_dev *hdev, u8 connectable)
 
 void mgmt_advertising(struct hci_dev *hdev, u8 advertising)
 {
+	/* Powering off may stop advertising - don't let that interfere */
+	if (!advertising && mgmt_pending_find(MGMT_OP_SET_POWERED, hdev))
+		return;
+
 	if (advertising)
 		set_bit(HCI_ADVERTISING, &hdev->dev_flags);
 	else
-- 
1.8.5.3

[PATCH 7/7] Bluetooth: Clean up HCI state when doing power off

[johan.hedberg]

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

To be friendly to user space and to behave well with controllers that
lack a proper internal power off procedure we should try to clean up as
much state as possible before requesting the HCI driver to power off.

This patch updates the power off procedure that's triggered by
mgmt_set_powered to clean any scan modes, stop LE scanning and
advertising and to disconnect any open connections.

The asynchronous cleanup procedure uses the HCI request framework,
however since HCI_Disconnect is only covered until its Command Status
event we need some extra tracking/waiting of disconnections. This is
done by monitoring when hci_conn_count() indicates that there are no
more connections.

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

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 610ac32e797b..25b8b278debd 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -1026,6 +1026,49 @@ static int send_settings_rsp(struct sock *sk, u16 opcode, struct hci_dev *hdev)
 			    sizeof(settings));
 }
 
+static void clean_up_hci_complete(struct hci_dev *hdev, u8 status)
+{
+	BT_DBG("%s status 0x%02x", hdev->name, status);
+
+	if (hci_conn_count(hdev) == 0)
+		queue_work(hdev->req_workqueue, &hdev->power_off.work);
+}
+
+static int clean_up_hci_state(struct hci_dev *hdev)
+{
+	struct hci_request req;
+	struct hci_conn *conn;
+
+	hci_req_init(&req, hdev);
+
+	if (test_bit(HCI_ISCAN, &hdev->flags) ||
+	    test_bit(HCI_PSCAN, &hdev->flags)) {
+		u8 scan = 0x00;
+		hci_req_add(&req, HCI_OP_WRITE_SCAN_ENABLE, 1, &scan);
+	}
+
+	if (test_bit(HCI_ADVERTISING, &hdev->dev_flags))
+		disable_advertising(&req);
+
+	if (test_bit(HCI_LE_SCAN, &hdev->dev_flags)) {
+		struct hci_cp_le_set_scan_enable cp;
+
+		memset(&cp, 0, sizeof(cp));
+		cp.enable = LE_SCAN_DISABLE;
+		hci_req_add(&req, HCI_OP_LE_SET_SCAN_ENABLE, sizeof(cp), &cp);
+	}
+
+	list_for_each_entry(conn, &hdev->conn_hash.list, list) {
+		struct hci_cp_disconnect dc;
+
+		dc.handle = cpu_to_le16(conn->handle);
+		dc.reason = 0x15; /* Terminated due to Power Off */
+		hci_req_add(&req, HCI_OP_DISCONNECT, sizeof(dc), &dc);
+	}
+
+	return hci_req_run(&req, clean_up_hci_complete);
+}
+
 static int set_powered(struct sock *sk, struct hci_dev *hdev, void *data,
 		       u16 len)
 {
@@ -1069,12 +1112,19 @@ static int set_powered(struct sock *sk, struct hci_dev *hdev, void *data,
 		goto failed;
 	}
 
-	if (cp->val)
+	if (cp->val) {
 		queue_work(hdev->req_workqueue, &hdev->power_on);
-	else
-		queue_work(hdev->req_workqueue, &hdev->power_off.work);
+		err = 0;
+	} else {
+		/* Disconnect connections, stop scans, etc */
+		err = clean_up_hci_state(hdev);
 
-	err = 0;
+		/* ENODATA means there were no HCI commands queued */
+		if (err == -ENODATA) {
+			queue_work(hdev->req_workqueue, &hdev->power_off.work);
+			err = 0;
+		}
+	}
 
 failed:
 	hci_dev_unlock(hdev);
@@ -5028,8 +5078,20 @@ void mgmt_device_disconnected(struct hci_dev *hdev, bdaddr_t *bdaddr,
 			      bool mgmt_connected)
 {
 	struct mgmt_ev_device_disconnected ev;
+	struct pending_cmd *power_off;
 	struct sock *sk = NULL;
 
+	power_off = mgmt_pending_find(MGMT_OP_SET_POWERED, hdev);
+	if (power_off) {
+		struct mgmt_mode *cp = power_off->param;
+
+		/* The connection is still in hci_conn_hash so test for 1
+		 * instead of 0 to know if this is the last one.
+		 */
+		if (!cp->val && hci_conn_count(hdev) == 1)
+			queue_work(hdev->req_workqueue, &hdev->power_off.work);
+	}
+
 	if (!mgmt_connected)
 		return;
 
-- 
1.8.5.3

Re: [PATCH 0/7] Bluetooth: Graceful power off procedure

[Marcel Holtmann]

Hi Johan,

> This set of patches updates the power off procedure (as triggered
> through mgmt) to perform a graceful power off by first cleaning up HCI
> state such as connections, scan modes and advertising before requesting
> the HCI driver to actually power off.
> 
> Johan
> 
> ----------------------------------------------------------------
> Johan Hedberg (7):
>      Bluetooth: Add convenience function for getting total connection count
>      Bluetooth: Move HCI_ADVERTISING handling into mgmt.c
>      Bluetooth: Move check for MGMT_CONNECTED flag into mgmt.c
>      Bluetooth: Don't clear HCI_DISCOVERABLE when powering off
>      Bluetooth: Don't clear HCI_CONNECTABLE when powering off
>      Bluetooth: Don't clear HCI_ADVERTISING when powering off
>      Bluetooth: Clean up HCI state when doing power off
> 
> include/net/bluetooth/hci_core.h | 11 ++++-
> net/bluetooth/hci_event.c        | 15 +++---
> net/bluetooth/mgmt.c             | 96 ++++++++++++++++++++++++++++++++++--
> 3 files changed, 107 insertions(+), 15 deletions(-)

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

Regards

Marcel