netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] Bluetooth: Suspend improvements
@ 2021-03-01 20:06 Abhishek Pandit-Subedi
  2021-03-01 20:06 ` [PATCH 1/2] Bluetooth: Notify suspend on le conn failed Abhishek Pandit-Subedi
  2021-03-01 20:06 ` [PATCH 2/2] Bluetooth: Remove unneeded commands for suspend Abhishek Pandit-Subedi
  0 siblings, 2 replies; 5+ messages in thread
From: Abhishek Pandit-Subedi @ 2021-03-01 20:06 UTC (permalink / raw)
  To: marcel
  Cc: chromeos-bluetooth-upstreaming, Hans de Goede, linux-bluetooth,
	Abhishek Pandit-Subedi, David S. Miller, Johan Hedberg, netdev,
	linux-kernel, Jakub Kicinski, Luiz Augusto von Dentz


Hi Marcel (and linux bluetooth),

Here are a few suspend improvements based on user reports we saw on
ChromeOS and feedback from Hans de Goede on the mailing list.

I have tested this using our ChromeOS suspend/resume automated tests
(full SRHealth test coverage and some suspend resume stress tests).

Thanks
Abhishek



Abhishek Pandit-Subedi (2):
  Bluetooth: Notify suspend on le conn failed
  Bluetooth: Remove unneeded commands for suspend

 include/net/bluetooth/hci.h |  1 +
 net/bluetooth/hci_conn.c    | 10 +++++++++
 net/bluetooth/hci_event.c   | 31 ++++++++++++++++++++++++++
 net/bluetooth/hci_request.c | 44 +++++++++++++++++++++++--------------
 4 files changed, 69 insertions(+), 17 deletions(-)

-- 
2.30.1.766.gb4fecdf3b7-goog


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 1/2] Bluetooth: Notify suspend on le conn failed
  2021-03-01 20:06 [PATCH 0/2] Bluetooth: Suspend improvements Abhishek Pandit-Subedi
@ 2021-03-01 20:06 ` Abhishek Pandit-Subedi
  2021-03-02 14:03   ` Marcel Holtmann
  2021-03-01 20:06 ` [PATCH 2/2] Bluetooth: Remove unneeded commands for suspend Abhishek Pandit-Subedi
  1 sibling, 1 reply; 5+ messages in thread
From: Abhishek Pandit-Subedi @ 2021-03-01 20:06 UTC (permalink / raw)
  To: marcel
  Cc: chromeos-bluetooth-upstreaming, Hans de Goede, linux-bluetooth,
	Abhishek Pandit-Subedi, Archie Pusaka, David S. Miller,
	Johan Hedberg, netdev, linux-kernel, Jakub Kicinski,
	Luiz Augusto von Dentz

When suspending, Bluetooth disconnects all connected peers devices. If
an LE connection is started but isn't completed, we will see an LE
Create Connection Cancel instead of an HCI disconnect. This just adds
a check to see if an LE cancel was the last disconnected device and wake
the suspend thread when that is the case.

Signed-off-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
Reviewed-by: Archie Pusaka <apusaka@chromium.org>
---
Here is an HCI trace when the issue occurred.

< HCI Command: LE Create Connection (0x08|0x000d) plen 25                                           #187777 [hci0] 2021-02-03 21:42:35.130208
        Scan interval: 60.000 msec (0x0060)
        Scan window: 60.000 msec (0x0060)
        Filter policy: White list is not used (0x00)
        Peer address type: Random (0x01)
        Peer address: D9:DC:6B:61:EB:3A (Static)
        Own address type: Public (0x00)
        Min connection interval: 15.00 msec (0x000c)
        Max connection interval: 30.00 msec (0x0018)
        Connection latency: 20 (0x0014)
        Supervision timeout: 3000 msec (0x012c)
        Min connection length: 0.000 msec (0x0000)
        Max connection length: 0.000 msec (0x0000)
> HCI Event: Command Status (0x0f) plen 4                                                           #187778 [hci0] 2021-02-03 21:42:35.131184
      LE Create Connection (0x08|0x000d) ncmd 1
        Status: Success (0x00)
< HCI Command: LE Create Connection Cancel (0x08|0x000e) plen 0                                     #187805 [hci0] 2021-02-03 21:42:37.183336
> HCI Event: Command Complete (0x0e) plen 4                                                         #187806 [hci0] 2021-02-03 21:42:37.192394
      LE Create Connection Cancel (0x08|0x000e) ncmd 1
        Status: Success (0x00)
> HCI Event: LE Meta Event (0x3e) plen 19                                                           #187807 [hci0] 2021-02-03 21:42:37.193400
      LE Connection Complete (0x01)
        Status: Unknown Connection Identifier (0x02)
        Handle: 0
        Role: Master (0x00)
        Peer address type: Random (0x01)
        Peer address: D9:DC:6B:61:EB:3A (Static)
        Connection interval: 0.00 msec (0x0000)
        Connection latency: 0 (0x0000)
        Supervision timeout: 0 msec (0x0000)
        Master clock accuracy: 0x00
... <skip a few unrelated events>
@ MGMT Event: Controller Suspended (0x002d) plen 1                                                 {0x0002} [hci0] 2021-02-03 21:42:39.178780
        Suspend state: Controller running (failed to suspend) (0)
@ MGMT Event: Controller Suspended (0x002d) plen 1                                                 {0x0001} [hci0] 2021-02-03 21:42:39.178780
        Suspend state: Controller running (failed to suspend) (0)
... <actual suspended time>
< HCI Command: Set Event Filter (0x03|0x0005) plen 1                                                #187808 [hci0] 2021-02-04 09:23:07.313591
        Type: Clear All Filters (0x00)

 net/bluetooth/hci_conn.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index 6ffa89e3ba0a85..468d31f3303d7a 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -772,6 +772,16 @@ void hci_le_conn_failed(struct hci_conn *conn, u8 status)
 
 	hci_conn_del(conn);
 
+	/* The suspend notifier is waiting for all devices to disconnect and an
+	 * LE connect cancel will result in an hci_le_conn_failed. Once the last
+	 * connection is deleted, we should also wake the suspend queue to
+	 * complete suspend operations.
+	 */
+	if (list_empty(&hdev->conn_hash.list) &&
+	    test_and_clear_bit(SUSPEND_DISCONNECTING, hdev->suspend_tasks)) {
+		wake_up(&hdev->suspend_wait_q);
+	}
+
 	/* Since we may have temporarily stopped the background scanning in
 	 * favor of connection establishment, we should restart it.
 	 */
-- 
2.30.1.766.gb4fecdf3b7-goog


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 2/2] Bluetooth: Remove unneeded commands for suspend
  2021-03-01 20:06 [PATCH 0/2] Bluetooth: Suspend improvements Abhishek Pandit-Subedi
  2021-03-01 20:06 ` [PATCH 1/2] Bluetooth: Notify suspend on le conn failed Abhishek Pandit-Subedi
@ 2021-03-01 20:06 ` Abhishek Pandit-Subedi
  2021-03-02 13:56   ` Marcel Holtmann
  1 sibling, 1 reply; 5+ messages in thread
From: Abhishek Pandit-Subedi @ 2021-03-01 20:06 UTC (permalink / raw)
  To: marcel
  Cc: chromeos-bluetooth-upstreaming, Hans de Goede, linux-bluetooth,
	Abhishek Pandit-Subedi, Archie Pusaka, Alain Michaud,
	David S. Miller, Johan Hedberg, netdev, linux-kernel,
	Jakub Kicinski, Luiz Augusto von Dentz

During suspend, there are a few scan enable and set event filter
commands that don't need to be sent unless there are actual BR/EDR
devices capable of waking the system. Check the HCI_PSCAN bit before
writing scan enable and use a new dev flag, HCI_EVENT_FILTER_CONFIGURED
to control whether to clear the event filter.

Signed-off-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
Reviewed-by: Archie Pusaka <apusaka@chromium.org>
Reviewed-by: Alain Michaud <alainm@chromium.org>
---

 include/net/bluetooth/hci.h |  1 +
 net/bluetooth/hci_event.c   | 31 ++++++++++++++++++++++++++
 net/bluetooth/hci_request.c | 44 +++++++++++++++++++++++--------------
 3 files changed, 59 insertions(+), 17 deletions(-)

diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index ba2f439bc04d34..ea4ae551c42687 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -320,6 +320,7 @@ enum {
 	HCI_BREDR_ENABLED,
 	HCI_LE_SCAN_INTERRUPTED,
 	HCI_WIDEBAND_SPEECH_ENABLED,
+	HCI_EVENT_FILTER_CONFIGURED,
 
 	HCI_DUT_MODE,
 	HCI_VENDOR_DIAG,
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 67668be3461e93..17847e672b98cf 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -395,6 +395,33 @@ static void hci_cc_write_scan_enable(struct hci_dev *hdev, struct sk_buff *skb)
 	hci_dev_unlock(hdev);
 }
 
+static void hci_cc_set_event_filter(struct hci_dev *hdev, struct sk_buff *skb)
+{
+	__u8 status = *((__u8 *)skb->data);
+	struct hci_cp_set_event_filter *cp;
+	void *sent;
+
+	BT_DBG("%s status 0x%2.2x", hdev->name, status);
+
+	sent = hci_sent_cmd_data(hdev, HCI_OP_SET_EVENT_FLT);
+	if (!sent)
+		return;
+
+	cp = (struct hci_cp_set_event_filter *)sent;
+
+	hci_dev_lock(hdev);
+
+	if (status)
+		goto done;
+
+	if (cp->flt_type == HCI_FLT_CLEAR_ALL)
+		hci_dev_clear_flag(hdev, HCI_EVENT_FILTER_CONFIGURED);
+	else
+		hci_dev_set_flag(hdev, HCI_EVENT_FILTER_CONFIGURED);
+done:
+	hci_dev_unlock(hdev);
+}
+
 static void hci_cc_read_class_of_dev(struct hci_dev *hdev, struct sk_buff *skb)
 {
 	struct hci_rp_read_class_of_dev *rp = (void *) skb->data;
@@ -3328,6 +3355,10 @@ static void hci_cmd_complete_evt(struct hci_dev *hdev, struct sk_buff *skb,
 		hci_cc_write_scan_enable(hdev, skb);
 		break;
 
+	case HCI_OP_SET_EVENT_FLT:
+		hci_cc_set_event_filter(hdev, skb);
+		break;
+
 	case HCI_OP_READ_CLASS_OF_DEV:
 		hci_cc_read_class_of_dev(hdev, skb);
 		break;
diff --git a/net/bluetooth/hci_request.c b/net/bluetooth/hci_request.c
index e55976db4403e7..75a42178c82d9b 100644
--- a/net/bluetooth/hci_request.c
+++ b/net/bluetooth/hci_request.c
@@ -1131,14 +1131,14 @@ static void hci_req_clear_event_filter(struct hci_request *req)
 {
 	struct hci_cp_set_event_filter f;
 
-	memset(&f, 0, sizeof(f));
-	f.flt_type = HCI_FLT_CLEAR_ALL;
-	hci_req_add(req, HCI_OP_SET_EVENT_FLT, 1, &f);
+	if (!hci_dev_test_flag(req->hdev, HCI_BREDR_ENABLED))
+		return;
 
-	/* Update page scan state (since we may have modified it when setting
-	 * the event filter).
-	 */
-	__hci_req_update_scan(req);
+	if (hci_dev_test_flag(req->hdev, HCI_EVENT_FILTER_CONFIGURED)) {
+		memset(&f, 0, sizeof(f));
+		f.flt_type = HCI_FLT_CLEAR_ALL;
+		hci_req_add(req, HCI_OP_SET_EVENT_FLT, 1, &f);
+	}
 }
 
 static void hci_req_set_event_filter(struct hci_request *req)
@@ -1147,6 +1147,10 @@ static void hci_req_set_event_filter(struct hci_request *req)
 	struct hci_cp_set_event_filter f;
 	struct hci_dev *hdev = req->hdev;
 	u8 scan = SCAN_DISABLED;
+	bool scanning = test_bit(HCI_PSCAN, &hdev->flags);
+
+	if (!hci_dev_test_flag(hdev, HCI_BREDR_ENABLED))
+		return;
 
 	/* Always clear event filter when starting */
 	hci_req_clear_event_filter(req);
@@ -1167,12 +1171,13 @@ static void hci_req_set_event_filter(struct hci_request *req)
 		scan = SCAN_PAGE;
 	}
 
-	if (scan)
+	if (scan && !scanning) {
 		set_bit(SUSPEND_SCAN_ENABLE, hdev->suspend_tasks);
-	else
+		hci_req_add(req, HCI_OP_WRITE_SCAN_ENABLE, 1, &scan);
+	} else if (!scan && scanning) {
 		set_bit(SUSPEND_SCAN_DISABLE, hdev->suspend_tasks);
-
-	hci_req_add(req, HCI_OP_WRITE_SCAN_ENABLE, 1, &scan);
+		hci_req_add(req, HCI_OP_WRITE_SCAN_ENABLE, 1, &scan);
+	}
 }
 
 static void cancel_adv_timeout(struct hci_dev *hdev)
@@ -1315,9 +1320,14 @@ void hci_req_prepare_suspend(struct hci_dev *hdev, enum suspended_state next)
 
 		hdev->advertising_paused = true;
 		hdev->advertising_old_state = old_state;
-		/* Disable page scan */
-		page_scan = SCAN_DISABLED;
-		hci_req_add(&req, HCI_OP_WRITE_SCAN_ENABLE, 1, &page_scan);
+
+		/* Disable page scan if enabled */
+		if (test_bit(HCI_PSCAN, &hdev->flags)) {
+			page_scan = SCAN_DISABLED;
+			hci_req_add(&req, HCI_OP_WRITE_SCAN_ENABLE, 1,
+				    &page_scan);
+			set_bit(SUSPEND_SCAN_DISABLE, hdev->suspend_tasks);
+		}
 
 		/* Disable LE passive scan if enabled */
 		if (hci_dev_test_flag(hdev, HCI_LE_SCAN)) {
@@ -1328,9 +1338,6 @@ void hci_req_prepare_suspend(struct hci_dev *hdev, enum suspended_state next)
 		/* Disable advertisement filters */
 		hci_req_add_set_adv_filter_enable(&req, false);
 
-		/* Mark task needing completion */
-		set_bit(SUSPEND_SCAN_DISABLE, hdev->suspend_tasks);
-
 		/* Prevent disconnects from causing scanning to be re-enabled */
 		hdev->scanning_paused = true;
 
@@ -1364,7 +1371,10 @@ void hci_req_prepare_suspend(struct hci_dev *hdev, enum suspended_state next)
 		hdev->suspended = false;
 		hdev->scanning_paused = false;
 
+		/* Clear any event filters and restore scan state */
 		hci_req_clear_event_filter(&req);
+		__hci_req_update_scan(&req);
+
 		/* Reset passive/background scanning to normal */
 		__hci_update_background_scan(&req);
 		/* Enable all of the advertisement filters */
-- 
2.30.1.766.gb4fecdf3b7-goog


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH 2/2] Bluetooth: Remove unneeded commands for suspend
  2021-03-01 20:06 ` [PATCH 2/2] Bluetooth: Remove unneeded commands for suspend Abhishek Pandit-Subedi
@ 2021-03-02 13:56   ` Marcel Holtmann
  0 siblings, 0 replies; 5+ messages in thread
From: Marcel Holtmann @ 2021-03-02 13:56 UTC (permalink / raw)
  To: Abhishek Pandit-Subedi
  Cc: CrosBT Upstreaming, Hans de Goede, Bluetooth Kernel Mailing List,
	Archie Pusaka, Alain Michaud, David S. Miller, Johan Hedberg,
	netdev, LKML, Jakub Kicinski, Luiz Augusto von Dentz

Hi Abhishek,

> During suspend, there are a few scan enable and set event filter
> commands that don't need to be sent unless there are actual BR/EDR
> devices capable of waking the system. Check the HCI_PSCAN bit before
> writing scan enable and use a new dev flag, HCI_EVENT_FILTER_CONFIGURED
> to control whether to clear the event filter.
> 
> Signed-off-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
> Reviewed-by: Archie Pusaka <apusaka@chromium.org>
> Reviewed-by: Alain Michaud <alainm@chromium.org>
> ---
> 
> include/net/bluetooth/hci.h |  1 +
> net/bluetooth/hci_event.c   | 31 ++++++++++++++++++++++++++
> net/bluetooth/hci_request.c | 44 +++++++++++++++++++++++--------------
> 3 files changed, 59 insertions(+), 17 deletions(-)
> 
> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> index ba2f439bc04d34..ea4ae551c42687 100644
> --- a/include/net/bluetooth/hci.h
> +++ b/include/net/bluetooth/hci.h
> @@ -320,6 +320,7 @@ enum {
> 	HCI_BREDR_ENABLED,
> 	HCI_LE_SCAN_INTERRUPTED,
> 	HCI_WIDEBAND_SPEECH_ENABLED,
> +	HCI_EVENT_FILTER_CONFIGURED,
> 
> 	HCI_DUT_MODE,
> 	HCI_VENDOR_DIAG,
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index 67668be3461e93..17847e672b98cf 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -395,6 +395,33 @@ static void hci_cc_write_scan_enable(struct hci_dev *hdev, struct sk_buff *skb)
> 	hci_dev_unlock(hdev);
> }
> 
> +static void hci_cc_set_event_filter(struct hci_dev *hdev, struct sk_buff *skb)
> +{
> +	__u8 status = *((__u8 *)skb->data);
> +	struct hci_cp_set_event_filter *cp;
> +	void *sent;
> +
> +	BT_DBG("%s status 0x%2.2x", hdev->name, status);
> +
> +	sent = hci_sent_cmd_data(hdev, HCI_OP_SET_EVENT_FLT);
> +	if (!sent)
> +		return;
> +
> +	cp = (struct hci_cp_set_event_filter *)sent;
> +
> +	hci_dev_lock(hdev);
> +
> +	if (status)
> +		goto done;

So I have no idea why this is inside the hdev_lock scope. Just leave the function right before sent assignment if you don’t care about handling the failure.

> +
> +	if (cp->flt_type == HCI_FLT_CLEAR_ALL)
> +		hci_dev_clear_flag(hdev, HCI_EVENT_FILTER_CONFIGURED);
> +	else
> +		hci_dev_set_flag(hdev, HCI_EVENT_FILTER_CONFIGURED);
> +done:
> +	hci_dev_unlock(hdev);
> +}
> +
> static void hci_cc_read_class_of_dev(struct hci_dev *hdev, struct sk_buff *skb)
> {
> 	struct hci_rp_read_class_of_dev *rp = (void *) skb->data;
> @@ -3328,6 +3355,10 @@ static void hci_cmd_complete_evt(struct hci_dev *hdev, struct sk_buff *skb,
> 		hci_cc_write_scan_enable(hdev, skb);
> 		break;
> 
> +	case HCI_OP_SET_EVENT_FLT:
> +		hci_cc_set_event_filter(hdev, skb);
> +		break;
> +
> 	case HCI_OP_READ_CLASS_OF_DEV:
> 		hci_cc_read_class_of_dev(hdev, skb);
> 		break;
> diff --git a/net/bluetooth/hci_request.c b/net/bluetooth/hci_request.c
> index e55976db4403e7..75a42178c82d9b 100644
> --- a/net/bluetooth/hci_request.c
> +++ b/net/bluetooth/hci_request.c
> @@ -1131,14 +1131,14 @@ static void hci_req_clear_event_filter(struct hci_request *req)
> {
> 	struct hci_cp_set_event_filter f;
> 
> -	memset(&f, 0, sizeof(f));
> -	f.flt_type = HCI_FLT_CLEAR_ALL;
> -	hci_req_add(req, HCI_OP_SET_EVENT_FLT, 1, &f);
> +	if (!hci_dev_test_flag(req->hdev, HCI_BREDR_ENABLED))
> +		return;
> 
> -	/* Update page scan state (since we may have modified it when setting
> -	 * the event filter).
> -	 */
> -	__hci_req_update_scan(req);
> +	if (hci_dev_test_flag(req->hdev, HCI_EVENT_FILTER_CONFIGURED)) {
> +		memset(&f, 0, sizeof(f));
> +		f.flt_type = HCI_FLT_CLEAR_ALL;
> +		hci_req_add(req, HCI_OP_SET_EVENT_FLT, 1, &f);
> +	}
> }
> 
> static void hci_req_set_event_filter(struct hci_request *req)
> @@ -1147,6 +1147,10 @@ static void hci_req_set_event_filter(struct hci_request *req)
> 	struct hci_cp_set_event_filter f;
> 	struct hci_dev *hdev = req->hdev;
> 	u8 scan = SCAN_DISABLED;
> +	bool scanning = test_bit(HCI_PSCAN, &hdev->flags);
> +
> +	if (!hci_dev_test_flag(hdev, HCI_BREDR_ENABLED))
> +		return;
> 
> 	/* Always clear event filter when starting */
> 	hci_req_clear_event_filter(req);
> @@ -1167,12 +1171,13 @@ static void hci_req_set_event_filter(struct hci_request *req)
> 		scan = SCAN_PAGE;
> 	}
> 
> -	if (scan)
> +	if (scan && !scanning) {
> 		set_bit(SUSPEND_SCAN_ENABLE, hdev->suspend_tasks);
> -	else
> +		hci_req_add(req, HCI_OP_WRITE_SCAN_ENABLE, 1, &scan);
> +	} else if (!scan && scanning) {
> 		set_bit(SUSPEND_SCAN_DISABLE, hdev->suspend_tasks);
> -
> -	hci_req_add(req, HCI_OP_WRITE_SCAN_ENABLE, 1, &scan);
> +		hci_req_add(req, HCI_OP_WRITE_SCAN_ENABLE, 1, &scan);
> +	}
> }
> 
> static void cancel_adv_timeout(struct hci_dev *hdev)
> @@ -1315,9 +1320,14 @@ void hci_req_prepare_suspend(struct hci_dev *hdev, enum suspended_state next)
> 
> 		hdev->advertising_paused = true;
> 		hdev->advertising_old_state = old_state;
> -		/* Disable page scan */
> -		page_scan = SCAN_DISABLED;
> -		hci_req_add(&req, HCI_OP_WRITE_SCAN_ENABLE, 1, &page_scan);
> +
> +		/* Disable page scan if enabled */
> +		if (test_bit(HCI_PSCAN, &hdev->flags)) {
> +			page_scan = SCAN_DISABLED;
> +			hci_req_add(&req, HCI_OP_WRITE_SCAN_ENABLE, 1,
> +				    &page_scan);
> +			set_bit(SUSPEND_SCAN_DISABLE, hdev->suspend_tasks);
> +		}
> 
> 		/* Disable LE passive scan if enabled */
> 		if (hci_dev_test_flag(hdev, HCI_LE_SCAN)) {
> @@ -1328,9 +1338,6 @@ void hci_req_prepare_suspend(struct hci_dev *hdev, enum suspended_state next)
> 		/* Disable advertisement filters */
> 		hci_req_add_set_adv_filter_enable(&req, false);
> 
> -		/* Mark task needing completion */
> -		set_bit(SUSPEND_SCAN_DISABLE, hdev->suspend_tasks);
> -
> 		/* Prevent disconnects from causing scanning to be re-enabled */
> 		hdev->scanning_paused = true;
> 
> @@ -1364,7 +1371,10 @@ void hci_req_prepare_suspend(struct hci_dev *hdev, enum suspended_state next)
> 		hdev->suspended = false;
> 		hdev->scanning_paused = false;
> 
> +		/* Clear any event filters and restore scan state */
> 		hci_req_clear_event_filter(&req);
> +		__hci_req_update_scan(&req);
> +
> 		/* Reset passive/background scanning to normal */
> 		__hci_update_background_scan(&req);
> 		/* Enable all of the advertisement filters */

Rest looks good.

Regards

Marcel


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/2] Bluetooth: Notify suspend on le conn failed
  2021-03-01 20:06 ` [PATCH 1/2] Bluetooth: Notify suspend on le conn failed Abhishek Pandit-Subedi
@ 2021-03-02 14:03   ` Marcel Holtmann
  0 siblings, 0 replies; 5+ messages in thread
From: Marcel Holtmann @ 2021-03-02 14:03 UTC (permalink / raw)
  To: Abhishek Pandit-Subedi
  Cc: CrosBT Upstreaming, Hans de Goede, linux-bluetooth,
	Archie Pusaka, David S. Miller, Johan Hedberg, netdev,
	linux-kernel, Jakub Kicinski, Luiz Augusto von Dentz

Hi Abhishek,

> When suspending, Bluetooth disconnects all connected peers devices. If
> an LE connection is started but isn't completed, we will see an LE
> Create Connection Cancel instead of an HCI disconnect. This just adds
> a check to see if an LE cancel was the last disconnected device and wake
> the suspend thread when that is the case.
> 
> Signed-off-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
> Reviewed-by: Archie Pusaka <apusaka@chromium.org>
> ---
> Here is an HCI trace when the issue occurred.
> 
> < HCI Command: LE Create Connection (0x08|0x000d) plen 25                                           #187777 [hci0] 2021-02-03 21:42:35.130208
>        Scan interval: 60.000 msec (0x0060)
>        Scan window: 60.000 msec (0x0060)
>        Filter policy: White list is not used (0x00)
>        Peer address type: Random (0x01)
>        Peer address: D9:DC:6B:61:EB:3A (Static)
>        Own address type: Public (0x00)
>        Min connection interval: 15.00 msec (0x000c)
>        Max connection interval: 30.00 msec (0x0018)
>        Connection latency: 20 (0x0014)
>        Supervision timeout: 3000 msec (0x012c)
>        Min connection length: 0.000 msec (0x0000)
>        Max connection length: 0.000 msec (0x0000)
>> HCI Event: Command Status (0x0f) plen 4                                                           #187778 [hci0] 2021-02-03 21:42:35.131184
>      LE Create Connection (0x08|0x000d) ncmd 1
>        Status: Success (0x00)
> < HCI Command: LE Create Connection Cancel (0x08|0x000e) plen 0                                     #187805 [hci0] 2021-02-03 21:42:37.183336
>> HCI Event: Command Complete (0x0e) plen 4                                                         #187806 [hci0] 2021-02-03 21:42:37.192394
>      LE Create Connection Cancel (0x08|0x000e) ncmd 1
>        Status: Success (0x00)
>> HCI Event: LE Meta Event (0x3e) plen 19                                                           #187807 [hci0] 2021-02-03 21:42:37.193400
>      LE Connection Complete (0x01)
>        Status: Unknown Connection Identifier (0x02)
>        Handle: 0
>        Role: Master (0x00)
>        Peer address type: Random (0x01)
>        Peer address: D9:DC:6B:61:EB:3A (Static)
>        Connection interval: 0.00 msec (0x0000)
>        Connection latency: 0 (0x0000)
>        Supervision timeout: 0 msec (0x0000)
>        Master clock accuracy: 0x00
> ... <skip a few unrelated events>
> @ MGMT Event: Controller Suspended (0x002d) plen 1                                                 {0x0002} [hci0] 2021-02-03 21:42:39.178780
>        Suspend state: Controller running (failed to suspend) (0)
> @ MGMT Event: Controller Suspended (0x002d) plen 1                                                 {0x0001} [hci0] 2021-02-03 21:42:39.178780
>        Suspend state: Controller running (failed to suspend) (0)
> ... <actual suspended time>
> < HCI Command: Set Event Filter (0x03|0x0005) plen 1                                                #187808 [hci0] 2021-02-04 09:23:07.313591
>        Type: Clear All Filters (0x00)
> 
> net/bluetooth/hci_conn.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)

patch has been applied to bluetooth-next tree.

Regards

Marcel


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2021-03-03  4:10 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-01 20:06 [PATCH 0/2] Bluetooth: Suspend improvements Abhishek Pandit-Subedi
2021-03-01 20:06 ` [PATCH 1/2] Bluetooth: Notify suspend on le conn failed Abhishek Pandit-Subedi
2021-03-02 14:03   ` Marcel Holtmann
2021-03-01 20:06 ` [PATCH 2/2] Bluetooth: Remove unneeded commands for suspend Abhishek Pandit-Subedi
2021-03-02 13:56   ` Marcel Holtmann

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).