2020-03-04 01:07:21

by Abhishek Pandit-Subedi

[permalink] [raw]
Subject: [RFC PATCH v4 0/5] Bluetooth: Handle system suspend gracefully


Hi linux-bluetooth,

This patch series prepares the Bluetooth controller for system suspend
by disconnecting all devices and preparing the event filter and LE
whitelist with devices that can wake the system from suspend.

The main motivation for doing this is so we can enable Bluetooth as
a wake up source during suspend without it being noisy. Bluetooth should
wake the system when a HID device receives user input but otherwise not
send any events to the host.

This patch series was tested on several Chromebooks with both btusb and
hci_serdev on kernel 4.19. The set of tests was basically the following:
* Reconnects after suspend succeed
* HID devices can wake the system from suspend (needs some related bluez
changes to call the Set Wake Capable management command)
* System properly pauses and unpauses discovery + advertising around
suspend
* System does not wake from any events from non wakeable devices

Series 2 has refactored the change into multiple smaller commits as
requested. I tried to simplify some of the whitelist filtering edge
cases but unfortunately it remains quite complex.

Series 3 has refactored it further and should have resolved the
whitelisting complexity in series 2.

Series 4 adds a fix to check for powered down and powering down adapters.

Please review and provide any feedback.

Thanks
Abhishek


Changes in v4:
* Added check for mgmt_powering_down and hdev_is_powered in notifier

Changes in v3:
* Added wakeable property to le_conn_param
* Use wakeable list for BR/EDR and wakeable property for LE
* Refactored to only handle BR/EDR devices
* Split LE changes into its own commit

Changes in v2:
* Moved pm notifier registration into its own patch and moved params out
of separate suspend_state
* Refactored filters and whitelist settings to its own patch
* Refactored update_white_list to have clearer edge cases
* Add connected devices to whitelist (previously missing corner case)
* Refactored pause discovery + advertising into its own patch

Abhishek Pandit-Subedi (5):
Bluetooth: Add mgmt op set_wake_capable
Bluetooth: Handle PM_SUSPEND_PREPARE and PM_POST_SUSPEND
Bluetooth: Handle BR/EDR devices during suspend
Bluetooth: Handle LE devices during suspend
Bluetooth: Pause discovery and advertising during suspend

include/net/bluetooth/hci.h | 17 +-
include/net/bluetooth/hci_core.h | 43 ++++
include/net/bluetooth/mgmt.h | 7 +
net/bluetooth/hci_core.c | 102 ++++++++++
net/bluetooth/hci_event.c | 24 +++
net/bluetooth/hci_request.c | 327 ++++++++++++++++++++++++++-----
net/bluetooth/hci_request.h | 2 +
net/bluetooth/mgmt.c | 92 +++++++++
8 files changed, 554 insertions(+), 60 deletions(-)

--
2.25.0.265.gbab2e86ba0-goog


2020-03-04 01:07:32

by Abhishek Pandit-Subedi

[permalink] [raw]
Subject: [RFC PATCH v4 4/5] Bluetooth: Handle LE devices during suspend

To handle LE devices, we must first disable passive scanning and
disconnect all connected devices. Once that is complete, we update the
whitelist and re-enable scanning

Signed-off-by: Abhishek Pandit-Subedi <[email protected]>
---

Changes in v4: None
Changes in v3:
* Split LE changes into its own commit

Changes in v2: None

net/bluetooth/hci_request.c | 164 ++++++++++++++++++++++++------------
1 file changed, 110 insertions(+), 54 deletions(-)

diff --git a/net/bluetooth/hci_request.c b/net/bluetooth/hci_request.c
index 4d67b1d08608..88fd95d70f89 100644
--- a/net/bluetooth/hci_request.c
+++ b/net/bluetooth/hci_request.c
@@ -34,6 +34,9 @@
#define HCI_REQ_PEND 1
#define HCI_REQ_CANCELED 2

+#define LE_SUSPEND_SCAN_WINDOW 0x0012
+#define LE_SUSPEND_SCAN_INTERVAL 0x0060
+
void hci_req_init(struct hci_request *req, struct hci_dev *hdev)
{
skb_queue_head_init(&req->cmd_q);
@@ -654,6 +657,11 @@ void hci_req_add_le_scan_disable(struct hci_request *req)
{
struct hci_dev *hdev = req->hdev;

+ if (hdev->scanning_paused) {
+ BT_DBG("Scanning is paused for suspend");
+ return;
+ }
+
if (use_ext_scan(hdev)) {
struct hci_cp_le_set_ext_scan_enable cp;

@@ -670,15 +678,53 @@ void hci_req_add_le_scan_disable(struct hci_request *req)
}
}

-static void add_to_white_list(struct hci_request *req,
- struct hci_conn_params *params)
+static void del_from_white_list(struct hci_request *req, bdaddr_t *bdaddr,
+ u8 bdaddr_type)
+{
+ struct hci_cp_le_del_from_white_list cp;
+
+ cp.bdaddr_type = bdaddr_type;
+ bacpy(&cp.bdaddr, bdaddr);
+
+ BT_DBG("Remove %pMR (0x%x) from whitelist", &cp.bdaddr, cp.bdaddr_type);
+ hci_req_add(req, HCI_OP_LE_DEL_FROM_WHITE_LIST, sizeof(cp), &cp);
+}
+
+/* Adds connection to white list if needed. On error, returns -1. */
+static int add_to_white_list(struct hci_request *req,
+ struct hci_conn_params *params, u8 *num_entries,
+ bool allow_rpa)
{
struct hci_cp_le_add_to_white_list cp;
+ struct hci_dev *hdev = req->hdev;
+
+ /* Already in white list */
+ if (hci_bdaddr_list_lookup(&hdev->le_white_list, &params->addr,
+ params->addr_type))
+ return 0;
+
+ /* Select filter policy to accept all advertising */
+ if (*num_entries >= hdev->le_white_list_size)
+ return -1;

+ /* White list can not be used with RPAs */
+ if (!allow_rpa &&
+ hci_find_irk_by_addr(hdev, &params->addr, params->addr_type)) {
+ return -1;
+ }
+
+ /* During suspend, only wakeable devices can be in whitelist */
+ if (hdev->suspended && !params->wakeable)
+ return 0;
+
+ *num_entries += 1;
cp.bdaddr_type = params->addr_type;
bacpy(&cp.bdaddr, &params->addr);

+ BT_DBG("Add %pMR (0x%x) to whitelist", &cp.bdaddr, cp.bdaddr_type);
hci_req_add(req, HCI_OP_LE_ADD_TO_WHITE_LIST, sizeof(cp), &cp);
+
+ return 0;
}

static u8 update_white_list(struct hci_request *req)
@@ -686,7 +732,14 @@ static u8 update_white_list(struct hci_request *req)
struct hci_dev *hdev = req->hdev;
struct hci_conn_params *params;
struct bdaddr_list *b;
- uint8_t white_list_entries = 0;
+ u8 num_entries = 0;
+ bool pend_conn, pend_report;
+ /* We allow whitelisting even with RPAs in suspend. In the worst case,
+ * we won't be able to wake from devices that use the privacy1.2
+ * features. Additionally, once we support privacy1.2 and IRK
+ * offloading, we can update this to also check for those conditions.
+ */
+ bool allow_rpa = hdev->suspended;

/* Go through the current white list programmed into the
* controller one by one and check if that address is still
@@ -695,29 +748,28 @@ static u8 update_white_list(struct hci_request *req)
* command to remove it from the controller.
*/
list_for_each_entry(b, &hdev->le_white_list, list) {
- /* If the device is neither in pend_le_conns nor
- * pend_le_reports then remove it from the whitelist.
+ pend_conn = hci_pend_le_action_lookup(&hdev->pend_le_conns,
+ &b->bdaddr,
+ b->bdaddr_type);
+ pend_report = hci_pend_le_action_lookup(&hdev->pend_le_reports,
+ &b->bdaddr,
+ b->bdaddr_type);
+
+ /* If the device is not likely to connect or report,
+ * remove it from the whitelist.
*/
- if (!hci_pend_le_action_lookup(&hdev->pend_le_conns,
- &b->bdaddr, b->bdaddr_type) &&
- !hci_pend_le_action_lookup(&hdev->pend_le_reports,
- &b->bdaddr, b->bdaddr_type)) {
- struct hci_cp_le_del_from_white_list cp;
-
- cp.bdaddr_type = b->bdaddr_type;
- bacpy(&cp.bdaddr, &b->bdaddr);
-
- hci_req_add(req, HCI_OP_LE_DEL_FROM_WHITE_LIST,
- sizeof(cp), &cp);
+ if (!pend_conn && !pend_report) {
+ del_from_white_list(req, &b->bdaddr, b->bdaddr_type);
continue;
}

- if (hci_find_irk_by_addr(hdev, &b->bdaddr, b->bdaddr_type)) {
- /* White list can not be used with RPAs */
+ /* White list can not be used with RPAs */
+ if (!allow_rpa &&
+ hci_find_irk_by_addr(hdev, &b->bdaddr, b->bdaddr_type)) {
return 0x00;
}

- white_list_entries++;
+ num_entries++;
}

/* Since all no longer valid white list entries have been
@@ -731,47 +783,17 @@ static u8 update_white_list(struct hci_request *req)
* white list.
*/
list_for_each_entry(params, &hdev->pend_le_conns, action) {
- if (hci_bdaddr_list_lookup(&hdev->le_white_list,
- &params->addr, params->addr_type))
- continue;
-
- if (white_list_entries >= hdev->le_white_list_size) {
- /* Select filter policy to accept all advertising */
+ if (add_to_white_list(req, params, &num_entries, allow_rpa))
return 0x00;
- }
-
- if (hci_find_irk_by_addr(hdev, &params->addr,
- params->addr_type)) {
- /* White list can not be used with RPAs */
- return 0x00;
- }
-
- white_list_entries++;
- add_to_white_list(req, params);
}

/* After adding all new pending connections, walk through
* the list of pending reports and also add these to the
- * white list if there is still space.
+ * white list if there is still space. Abort if space runs out.
*/
list_for_each_entry(params, &hdev->pend_le_reports, action) {
- if (hci_bdaddr_list_lookup(&hdev->le_white_list,
- &params->addr, params->addr_type))
- continue;
-
- if (white_list_entries >= hdev->le_white_list_size) {
- /* Select filter policy to accept all advertising */
+ if (add_to_white_list(req, params, &num_entries, allow_rpa))
return 0x00;
- }
-
- if (hci_find_irk_by_addr(hdev, &params->addr,
- params->addr_type)) {
- /* White list can not be used with RPAs */
- return 0x00;
- }
-
- white_list_entries++;
- add_to_white_list(req, params);
}

/* Select filter policy to use white list */
@@ -866,6 +888,12 @@ void hci_req_add_le_passive_scan(struct hci_request *req)
struct hci_dev *hdev = req->hdev;
u8 own_addr_type;
u8 filter_policy;
+ u8 window, interval;
+
+ if (hdev->scanning_paused) {
+ BT_DBG("Scanning is paused for suspend");
+ return;
+ }

/* Set require_privacy to false since no SCAN_REQ are send
* during passive scanning. Not using an non-resolvable address
@@ -896,8 +924,17 @@ void hci_req_add_le_passive_scan(struct hci_request *req)
(hdev->le_features[0] & HCI_LE_EXT_SCAN_POLICY))
filter_policy |= 0x02;

- hci_req_start_scan(req, LE_SCAN_PASSIVE, hdev->le_scan_interval,
- hdev->le_scan_window, own_addr_type, filter_policy);
+ if (hdev->suspended) {
+ window = LE_SUSPEND_SCAN_WINDOW;
+ interval = LE_SUSPEND_SCAN_INTERVAL;
+ } else {
+ window = hdev->le_scan_window;
+ interval = hdev->le_scan_interval;
+ }
+
+ BT_DBG("LE passive scan with whitelist = %d", filter_policy);
+ hci_req_start_scan(req, LE_SCAN_PASSIVE, interval, window,
+ own_addr_type, filter_policy);
}

static u8 get_adv_instance_scan_rsp_len(struct hci_dev *hdev, u8 instance)
@@ -957,6 +994,18 @@ static void hci_req_set_event_filter(struct hci_request *req)
hci_req_add(req, HCI_OP_WRITE_SCAN_ENABLE, 1, &scan);
}

+static void hci_req_config_le_suspend_scan(struct hci_request *req)
+{
+ /* Can't change params without disabling first */
+ hci_req_add_le_scan_disable(req);
+
+ /* Configure params and enable scanning */
+ hci_req_add_le_passive_scan(req);
+
+ /* Block suspend notifier on response */
+ set_bit(SUSPEND_SCAN_ENABLE, req->hdev->suspend_tasks);
+}
+
static void suspend_req_complete(struct hci_dev *hdev, u8 status, u16 opcode)
{
BT_DBG("Request complete opcode=0x%x, status=0x%x", opcode, status);
@@ -991,6 +1040,9 @@ void hci_req_prepare_suspend(struct hci_dev *hdev, enum suspended_state next)
page_scan = SCAN_DISABLED;
hci_req_add(&req, HCI_OP_WRITE_SCAN_ENABLE, 1, &page_scan);

+ /* Disable LE passive scan */
+ hci_req_add_le_scan_disable(&req);
+
/* Mark task needing completion */
set_bit(SUSPEND_SCAN_DISABLE, hdev->suspend_tasks);

@@ -1017,6 +1069,8 @@ void hci_req_prepare_suspend(struct hci_dev *hdev, enum suspended_state next)
hdev->scanning_paused = false;
/* Enable event filter for paired devices */
hci_req_set_event_filter(&req);
+ /* Enable passive scan at lower duty cycle */
+ hci_req_config_le_suspend_scan(&req);
/* Pause scan changes again. */
hdev->scanning_paused = true;
hci_req_run(&req, suspend_req_complete);
@@ -1025,6 +1079,8 @@ void hci_req_prepare_suspend(struct hci_dev *hdev, enum suspended_state next)
hdev->scanning_paused = false;

hci_req_clear_event_filter(&req);
+ /* Reset passive/background scanning to normal */
+ hci_req_config_le_suspend_scan(&req);
hci_req_run(&req, suspend_req_complete);
}

--
2.25.0.265.gbab2e86ba0-goog

2020-03-04 01:07:35

by Abhishek Pandit-Subedi

[permalink] [raw]
Subject: [RFC PATCH v4 5/5] Bluetooth: Pause discovery and advertising during suspend

To prevent spurious wake ups, we disable any discovery or advertising
when we enter suspend and restore it when we exit suspend. While paused,
we disable any management requests to modify discovery or advertising.

Signed-off-by: Abhishek Pandit-Subedi <[email protected]>
---

Changes in v4: None
Changes in v3: None
Changes in v2:
* Refactored pause discovery + advertising into its own patch

include/net/bluetooth/hci_core.h | 11 ++++++++
net/bluetooth/hci_request.c | 43 ++++++++++++++++++++++++++++++++
net/bluetooth/mgmt.c | 41 ++++++++++++++++++++++++++++++
3 files changed, 95 insertions(+)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 4eb5b2786048..af264a247636 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -91,6 +91,12 @@ struct discovery_state {
#define SUSPEND_NOTIFIER_TIMEOUT msecs_to_jiffies(2000) /* 2 seconds */

enum suspend_tasks {
+ SUSPEND_PAUSE_DISCOVERY,
+ SUSPEND_UNPAUSE_DISCOVERY,
+
+ SUSPEND_PAUSE_ADVERTISING,
+ SUSPEND_UNPAUSE_ADVERTISING,
+
SUSPEND_SCAN_DISABLE,
SUSPEND_SCAN_ENABLE,
SUSPEND_DISCONNECTING,
@@ -409,6 +415,11 @@ struct hci_dev {

struct discovery_state discovery;

+ int discovery_old_state;
+ bool discovery_paused;
+ int advertising_old_state;
+ bool advertising_paused;
+
struct notifier_block suspend_notifier;
struct work_struct suspend_prepare;
enum suspended_state suspend_state_next;
diff --git a/net/bluetooth/hci_request.c b/net/bluetooth/hci_request.c
index 88fd95d70f89..e25cfb6fd9aa 100644
--- a/net/bluetooth/hci_request.c
+++ b/net/bluetooth/hci_request.c
@@ -1036,6 +1036,28 @@ void hci_req_prepare_suspend(struct hci_dev *hdev, enum suspended_state next)
/* Mark device as suspended */
hdev->suspended = true;

+ /* Pause discovery if not already stopped */
+ old_state = hdev->discovery.state;
+ if (old_state != DISCOVERY_STOPPED) {
+ set_bit(SUSPEND_PAUSE_DISCOVERY, hdev->suspend_tasks);
+ hci_discovery_set_state(hdev, DISCOVERY_STOPPING);
+ queue_work(hdev->req_workqueue, &hdev->discov_update);
+ }
+
+ hdev->discovery_paused = true;
+ hdev->discovery_old_state = old_state;
+
+ /* Stop advertising */
+ old_state = hci_dev_test_flag(hdev, HCI_ADVERTISING);
+ if (old_state) {
+ set_bit(SUSPEND_PAUSE_ADVERTISING, hdev->suspend_tasks);
+ cancel_delayed_work(&hdev->discov_off);
+ queue_delayed_work(hdev->req_workqueue,
+ &hdev->discov_off, 0);
+ }
+
+ 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);
@@ -1081,6 +1103,27 @@ void hci_req_prepare_suspend(struct hci_dev *hdev, enum suspended_state next)
hci_req_clear_event_filter(&req);
/* Reset passive/background scanning to normal */
hci_req_config_le_suspend_scan(&req);
+
+ /* Unpause advertising */
+ hdev->advertising_paused = false;
+ if (hdev->advertising_old_state) {
+ set_bit(SUSPEND_UNPAUSE_ADVERTISING,
+ hdev->suspend_tasks);
+ hci_dev_set_flag(hdev, HCI_ADVERTISING);
+ queue_work(hdev->req_workqueue,
+ &hdev->discoverable_update);
+ hdev->advertising_old_state = 0;
+ }
+
+ /* Unpause discovery */
+ hdev->discovery_paused = false;
+ if (hdev->discovery_old_state != DISCOVERY_STOPPED &&
+ hdev->discovery_old_state != DISCOVERY_STOPPING) {
+ set_bit(SUSPEND_UNPAUSE_DISCOVERY, hdev->suspend_tasks);
+ hci_discovery_set_state(hdev, DISCOVERY_STARTING);
+ queue_work(hdev->req_workqueue, &hdev->discov_update);
+ }
+
hci_req_run(&req, suspend_req_complete);
}

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index f6751ce0d561..28572579c06d 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -1387,6 +1387,12 @@ static int set_discoverable(struct sock *sk, struct hci_dev *hdev, void *data,
goto failed;
}

+ if (hdev->advertising_paused) {
+ err = mgmt_cmd_status(sk, hdev->id, MGMT_OP_SET_DISCOVERABLE,
+ MGMT_STATUS_BUSY);
+ goto failed;
+ }
+
if (!hdev_is_powered(hdev)) {
bool changed = false;

@@ -3870,6 +3876,13 @@ void mgmt_start_discovery_complete(struct hci_dev *hdev, u8 status)
}

hci_dev_unlock(hdev);
+
+ /* Handle suspend notifier */
+ if (test_and_clear_bit(SUSPEND_UNPAUSE_DISCOVERY,
+ hdev->suspend_tasks)) {
+ BT_DBG("Unpaused discovery");
+ wake_up(&hdev->suspend_wait_q);
+ }
}

static bool discovery_type_is_valid(struct hci_dev *hdev, uint8_t type,
@@ -3931,6 +3944,13 @@ static int start_discovery_internal(struct sock *sk, struct hci_dev *hdev,
goto failed;
}

+ /* Can't start discovery when it is paused */
+ if (hdev->discovery_paused) {
+ err = mgmt_cmd_complete(sk, hdev->id, op, MGMT_STATUS_BUSY,
+ &cp->type, sizeof(cp->type));
+ goto failed;
+ }
+
/* Clear the discovery filter first to free any previously
* allocated memory for the UUID list.
*/
@@ -4098,6 +4118,12 @@ void mgmt_stop_discovery_complete(struct hci_dev *hdev, u8 status)
}

hci_dev_unlock(hdev);
+
+ /* Handle suspend notifier */
+ if (test_and_clear_bit(SUSPEND_PAUSE_DISCOVERY, hdev->suspend_tasks)) {
+ BT_DBG("Paused discovery");
+ wake_up(&hdev->suspend_wait_q);
+ }
}

static int stop_discovery(struct sock *sk, struct hci_dev *hdev, void *data,
@@ -4329,6 +4355,17 @@ static void set_advertising_complete(struct hci_dev *hdev, u8 status,
if (match.sk)
sock_put(match.sk);

+ /* Handle suspend notifier */
+ if (test_and_clear_bit(SUSPEND_PAUSE_ADVERTISING,
+ hdev->suspend_tasks)) {
+ BT_DBG("Paused advertising");
+ wake_up(&hdev->suspend_wait_q);
+ } else if (test_and_clear_bit(SUSPEND_UNPAUSE_ADVERTISING,
+ hdev->suspend_tasks)) {
+ BT_DBG("Unpaused advertising");
+ wake_up(&hdev->suspend_wait_q);
+ }
+
/* If "Set Advertising" was just disabled and instance advertising was
* set up earlier, then re-enable multi-instance advertising.
*/
@@ -4380,6 +4417,10 @@ static int set_advertising(struct sock *sk, struct hci_dev *hdev, void *data,
return mgmt_cmd_status(sk, hdev->id, MGMT_OP_SET_ADVERTISING,
MGMT_STATUS_INVALID_PARAMS);

+ if (hdev->advertising_paused)
+ return mgmt_cmd_status(sk, hdev->id, MGMT_OP_SET_ADVERTISING,
+ MGMT_STATUS_BUSY);
+
hci_dev_lock(hdev);

val = !!cp->val;
--
2.25.0.265.gbab2e86ba0-goog

2020-03-04 01:07:42

by Abhishek Pandit-Subedi

[permalink] [raw]
Subject: [RFC PATCH v4 3/5] Bluetooth: Handle BR/EDR devices during suspend

To handle BR/EDR devices, we first disable page scan and disconnect all
connected devices. Once that is complete, we add event filters (for
devices that can wake the system) and re-enable page scan.

Signed-off-by: Abhishek Pandit-Subedi <[email protected]>
---

Changes in v4: None
Changes in v3:
* Refactored to only handle BR/EDR devices

Changes in v2:
* Refactored filters and whitelist settings to its own patch
* Refactored update_white_list to have clearer edge cases
* Add connected devices to whitelist (previously missing corner case)

include/net/bluetooth/hci.h | 17 ++++--
include/net/bluetooth/hci_core.h | 9 ++-
net/bluetooth/hci_core.c | 21 ++++++-
net/bluetooth/hci_event.c | 24 ++++++++
net/bluetooth/hci_request.c | 101 +++++++++++++++++++++++++++++++
5 files changed, 162 insertions(+), 10 deletions(-)

diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index 0b3ebd35681d..414f91dec221 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -941,10 +941,14 @@ struct hci_cp_sniff_subrate {
#define HCI_OP_RESET 0x0c03

#define HCI_OP_SET_EVENT_FLT 0x0c05
-struct hci_cp_set_event_flt {
- __u8 flt_type;
- __u8 cond_type;
- __u8 condition[];
+#define HCI_SET_EVENT_FLT_SIZE 9
+struct hci_cp_set_event_filter {
+ __u8 flt_type;
+ __u8 cond_type;
+ struct {
+ bdaddr_t bdaddr;
+ __u8 auto_accept;
+ } __packed addr_conn_flt;
} __packed;

/* Filter types */
@@ -958,8 +962,9 @@ struct hci_cp_set_event_flt {
#define HCI_CONN_SETUP_ALLOW_BDADDR 0x02

/* CONN_SETUP Conditions */
-#define HCI_CONN_SETUP_AUTO_OFF 0x01
-#define HCI_CONN_SETUP_AUTO_ON 0x02
+#define HCI_CONN_SETUP_AUTO_OFF 0x01
+#define HCI_CONN_SETUP_AUTO_ON 0x02
+#define HCI_CONN_SETUP_AUTO_ON_WITH_RS 0x03

#define HCI_OP_READ_STORED_LINK_KEY 0x0c0d
struct hci_cp_read_stored_link_key {
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index b82a89b88d1b..4eb5b2786048 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -91,6 +91,10 @@ struct discovery_state {
#define SUSPEND_NOTIFIER_TIMEOUT msecs_to_jiffies(2000) /* 2 seconds */

enum suspend_tasks {
+ SUSPEND_SCAN_DISABLE,
+ SUSPEND_SCAN_ENABLE,
+ SUSPEND_DISCONNECTING,
+
SUSPEND_POWERING_DOWN,

SUSPEND_PREPARE_NOTIFIER,
@@ -99,7 +103,8 @@ enum suspend_tasks {

enum suspended_state {
BT_RUNNING = 0,
- BT_SUSPENDED,
+ BT_SUSPEND_DISCONNECT,
+ BT_SUSPEND_COMPLETE,
};

struct hci_conn_hash {
@@ -408,6 +413,8 @@ struct hci_dev {
struct work_struct suspend_prepare;
enum suspended_state suspend_state_next;
enum suspended_state suspend_state;
+ bool scanning_paused;
+ bool suspended;

wait_queue_head_t suspend_wait_q;
DECLARE_BITMAP(suspend_tasks, __SUSPEND_NUM_TASKS);
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index ad89ff3f8f57..72e53fde2a74 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -3302,16 +3302,31 @@ static int hci_suspend_notifier(struct notifier_block *nb, unsigned long action,
goto done;

if (action == PM_SUSPEND_PREPARE) {
- hdev->suspend_state_next = BT_SUSPENDED;
+ /* Suspend consists of two actions:
+ * - First, disconnect everything and make the controller not
+ * connectable (disabling scanning)
+ * - Second, program event filter/whitelist and enable scan
+ */
+ hdev->suspend_state_next = BT_SUSPEND_DISCONNECT;
set_bit(SUSPEND_PREPARE_NOTIFIER, hdev->suspend_tasks);
queue_work(hdev->req_workqueue, &hdev->suspend_prepare);
-
ret = hci_suspend_wait_event(hdev);
+
+ /* If the disconnect portion failed, don't attempt to complete
+ * by configuring the whitelist. The suspend notifier will
+ * follow a cancelled suspend with a PM_POST_SUSPEND
+ * notification.
+ */
+ if (!ret) {
+ hdev->suspend_state_next = BT_SUSPEND_COMPLETE;
+ set_bit(SUSPEND_PREPARE_NOTIFIER, hdev->suspend_tasks);
+ queue_work(hdev->req_workqueue, &hdev->suspend_prepare);
+ ret = hci_suspend_wait_event(hdev);
+ }
} else if (action == PM_POST_SUSPEND) {
hdev->suspend_state_next = BT_RUNNING;
set_bit(SUSPEND_PREPARE_NOTIFIER, hdev->suspend_tasks);
queue_work(hdev->req_workqueue, &hdev->suspend_prepare);
-
ret = hci_suspend_wait_event(hdev);
}

diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 591e7477e925..e3c36230f705 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -2474,6 +2474,7 @@ static void hci_inquiry_result_evt(struct hci_dev *hdev, struct sk_buff *skb)
static void hci_conn_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
{
struct hci_ev_conn_complete *ev = (void *) skb->data;
+ struct inquiry_entry *ie;
struct hci_conn *conn;

BT_DBG("%s", hdev->name);
@@ -2482,6 +2483,21 @@ static void hci_conn_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)

conn = hci_conn_hash_lookup_ba(hdev, ev->link_type, &ev->bdaddr);
if (!conn) {
+ /* Connection may not exist if auto-connected. Check the inquiry
+ * cache to see if we've already discovered this bdaddr before.
+ * If found and link is an ACL type, create a connection class
+ * automatically.
+ */
+ ie = hci_inquiry_cache_lookup(hdev, &ev->bdaddr);
+ if (ie && ev->link_type == ACL_LINK) {
+ conn = hci_conn_add(hdev, ev->link_type, &ev->bdaddr,
+ HCI_ROLE_SLAVE);
+ if (!conn) {
+ bt_dev_err(hdev, "no memory for new conn");
+ goto unlock;
+ }
+ }
+
if (ev->link_type != SCO_LINK)
goto unlock;

@@ -2743,6 +2759,14 @@ static void hci_disconn_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
hci_disconn_cfm(conn, ev->reason);
hci_conn_del(conn);

+ /* The suspend notifier is waiting for all devices to disconnect so
+ * clear the bit from pending tasks and inform the wait queue.
+ */
+ if (list_empty(&hdev->conn_hash.list) &&
+ test_and_clear_bit(SUSPEND_DISCONNECTING, hdev->suspend_tasks)) {
+ wake_up(&hdev->suspend_wait_q);
+ }
+
/* Re-enable advertising if necessary, since it might
* have been disabled by the connection. From the
* HCI_LE_Set_Advertise_Enable command description in
diff --git a/net/bluetooth/hci_request.c b/net/bluetooth/hci_request.c
index 08908469c043..4d67b1d08608 100644
--- a/net/bluetooth/hci_request.c
+++ b/net/bluetooth/hci_request.c
@@ -918,12 +918,62 @@ static u8 get_adv_instance_scan_rsp_len(struct hci_dev *hdev, u8 instance)
return adv_instance->scan_rsp_len;
}

+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);
+
+ /* Update page scan state (since we may have modified it when setting
+ * the event filter).
+ */
+ __hci_req_update_scan(req);
+}
+
+static void hci_req_set_event_filter(struct hci_request *req)
+{
+ struct bdaddr_list *b;
+ struct hci_cp_set_event_filter f;
+ struct hci_dev *hdev = req->hdev;
+ u8 scan;
+
+ /* Always clear event filter when starting */
+ hci_req_clear_event_filter(req);
+
+ list_for_each_entry(b, &hdev->wakeable, list) {
+ memset(&f, 0, sizeof(f));
+ bacpy(&f.addr_conn_flt.bdaddr, &b->bdaddr);
+ f.flt_type = HCI_FLT_CONN_SETUP;
+ f.cond_type = HCI_CONN_SETUP_ALLOW_BDADDR;
+ f.addr_conn_flt.auto_accept = HCI_CONN_SETUP_AUTO_ON;
+
+ BT_DBG("Adding event filters for %pMR", &b->bdaddr);
+ hci_req_add(req, HCI_OP_SET_EVENT_FLT, sizeof(f), &f);
+ }
+
+ scan = !list_empty(&hdev->wakeable) ? SCAN_PAGE : SCAN_DISABLED;
+ hci_req_add(req, HCI_OP_WRITE_SCAN_ENABLE, 1, &scan);
+}
+
+static void suspend_req_complete(struct hci_dev *hdev, u8 status, u16 opcode)
+{
+ BT_DBG("Request complete opcode=0x%x, status=0x%x", opcode, status);
+ if (test_and_clear_bit(SUSPEND_SCAN_ENABLE, hdev->suspend_tasks) ||
+ test_and_clear_bit(SUSPEND_SCAN_DISABLE, hdev->suspend_tasks)) {
+ wake_up(&hdev->suspend_wait_q);
+ }
+}
+
/* Call with hci_dev_lock */
void hci_req_prepare_suspend(struct hci_dev *hdev, enum suspended_state next)
{
int old_state;
struct hci_conn *conn;
struct hci_request req;
+ u8 page_scan;
+ int disconnect_counter;

if (next == hdev->suspend_state) {
BT_DBG("Same state before and after: %d", next);
@@ -931,6 +981,54 @@ void hci_req_prepare_suspend(struct hci_dev *hdev, enum suspended_state next)
}

hdev->suspend_state = next;
+ hci_req_init(&req, hdev);
+
+ if (next == BT_SUSPEND_DISCONNECT) {
+ /* Mark device as suspended */
+ hdev->suspended = true;
+
+ /* Disable page scan */
+ page_scan = SCAN_DISABLED;
+ hci_req_add(&req, HCI_OP_WRITE_SCAN_ENABLE, 1, &page_scan);
+
+ /* 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;
+
+ /* Run commands before disconnecting */
+ hci_req_run(&req, suspend_req_complete);
+
+ disconnect_counter = 0;
+ /* Soft disconnect everything (power off) */
+ list_for_each_entry(conn, &hdev->conn_hash.list, list) {
+ hci_disconnect(conn, HCI_ERROR_REMOTE_POWER_OFF);
+ disconnect_counter++;
+ }
+
+ if (disconnect_counter > 0) {
+ BT_DBG("Had %d disconnects. Will wait on them",
+ disconnect_counter);
+ set_bit(SUSPEND_DISCONNECTING, hdev->suspend_tasks);
+ }
+ } else if (next == BT_SUSPEND_COMPLETE) {
+ /* Unpause to take care of updating scanning params */
+ hdev->scanning_paused = false;
+ /* Enable event filter for paired devices */
+ hci_req_set_event_filter(&req);
+ /* Pause scan changes again. */
+ hdev->scanning_paused = true;
+ hci_req_run(&req, suspend_req_complete);
+ } else {
+ hdev->suspended = false;
+ hdev->scanning_paused = false;
+
+ hci_req_clear_event_filter(&req);
+ hci_req_run(&req, suspend_req_complete);
+ }
+
+ hdev->suspend_state = next;

done:
clear_bit(SUSPEND_PREPARE_NOTIFIER, hdev->suspend_tasks);
@@ -2034,6 +2132,9 @@ void __hci_req_update_scan(struct hci_request *req)
if (mgmt_powering_down(hdev))
return;

+ if (hdev->scanning_paused)
+ return;
+
if (hci_dev_test_flag(hdev, HCI_CONNECTABLE) ||
disconnected_whitelist_entries(hdev))
scan = SCAN_PAGE;
--
2.25.0.265.gbab2e86ba0-goog

2020-03-08 08:31:08

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [RFC PATCH v4 3/5] Bluetooth: Handle BR/EDR devices during suspend

Hi Abhishek,

> To handle BR/EDR devices, we first disable page scan and disconnect all
> connected devices. Once that is complete, we add event filters (for
> devices that can wake the system) and re-enable page scan.
>
> Signed-off-by: Abhishek Pandit-Subedi <[email protected]>
> ---
>
> Changes in v4: None
> Changes in v3:
> * Refactored to only handle BR/EDR devices
>
> Changes in v2:
> * Refactored filters and whitelist settings to its own patch
> * Refactored update_white_list to have clearer edge cases
> * Add connected devices to whitelist (previously missing corner case)
>
> include/net/bluetooth/hci.h | 17 ++++--
> include/net/bluetooth/hci_core.h | 9 ++-
> net/bluetooth/hci_core.c | 21 ++++++-
> net/bluetooth/hci_event.c | 24 ++++++++
> net/bluetooth/hci_request.c | 101 +++++++++++++++++++++++++++++++
> 5 files changed, 162 insertions(+), 10 deletions(-)
>
> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> index 0b3ebd35681d..414f91dec221 100644
> --- a/include/net/bluetooth/hci.h
> +++ b/include/net/bluetooth/hci.h
> @@ -941,10 +941,14 @@ struct hci_cp_sniff_subrate {
> #define HCI_OP_RESET 0x0c03
>
> #define HCI_OP_SET_EVENT_FLT 0x0c05
> -struct hci_cp_set_event_flt {
> - __u8 flt_type;
> - __u8 cond_type;
> - __u8 condition[];
> +#define HCI_SET_EVENT_FLT_SIZE 9
> +struct hci_cp_set_event_filter {
> + __u8 flt_type;
> + __u8 cond_type;
> + struct {
> + bdaddr_t bdaddr;
> + __u8 auto_accept;
> + } __packed addr_conn_flt;
> } __packed;
>
> /* Filter types */
> @@ -958,8 +962,9 @@ struct hci_cp_set_event_flt {
> #define HCI_CONN_SETUP_ALLOW_BDADDR 0x02
>
> /* CONN_SETUP Conditions */
> -#define HCI_CONN_SETUP_AUTO_OFF 0x01
> -#define HCI_CONN_SETUP_AUTO_ON 0x02
> +#define HCI_CONN_SETUP_AUTO_OFF 0x01
> +#define HCI_CONN_SETUP_AUTO_ON 0x02
> +#define HCI_CONN_SETUP_AUTO_ON_WITH_RS 0x03
>
> #define HCI_OP_READ_STORED_LINK_KEY 0x0c0d
> struct hci_cp_read_stored_link_key {
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index b82a89b88d1b..4eb5b2786048 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -91,6 +91,10 @@ struct discovery_state {
> #define SUSPEND_NOTIFIER_TIMEOUT msecs_to_jiffies(2000) /* 2 seconds */
>
> enum suspend_tasks {
> + SUSPEND_SCAN_DISABLE,
> + SUSPEND_SCAN_ENABLE,
> + SUSPEND_DISCONNECTING,
> +
> SUSPEND_POWERING_DOWN,
>
> SUSPEND_PREPARE_NOTIFIER,
> @@ -99,7 +103,8 @@ enum suspend_tasks {
>
> enum suspended_state {
> BT_RUNNING = 0,
> - BT_SUSPENDED,
> + BT_SUSPEND_DISCONNECT,
> + BT_SUSPEND_COMPLETE,
> };
>
> struct hci_conn_hash {
> @@ -408,6 +413,8 @@ struct hci_dev {
> struct work_struct suspend_prepare;
> enum suspended_state suspend_state_next;
> enum suspended_state suspend_state;
> + bool scanning_paused;
> + bool suspended;
>
> wait_queue_head_t suspend_wait_q;
> DECLARE_BITMAP(suspend_tasks, __SUSPEND_NUM_TASKS);
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index ad89ff3f8f57..72e53fde2a74 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -3302,16 +3302,31 @@ static int hci_suspend_notifier(struct notifier_block *nb, unsigned long action,
> goto done;
>
> if (action == PM_SUSPEND_PREPARE) {
> - hdev->suspend_state_next = BT_SUSPENDED;
> + /* Suspend consists of two actions:
> + * - First, disconnect everything and make the controller not
> + * connectable (disabling scanning)
> + * - Second, program event filter/whitelist and enable scan
> + */
> + hdev->suspend_state_next = BT_SUSPEND_DISCONNECT;
> set_bit(SUSPEND_PREPARE_NOTIFIER, hdev->suspend_tasks);
> queue_work(hdev->req_workqueue, &hdev->suspend_prepare);
> -
> ret = hci_suspend_wait_event(hdev);
> +
> + /* If the disconnect portion failed, don't attempt to complete
> + * by configuring the whitelist. The suspend notifier will
> + * follow a cancelled suspend with a PM_POST_SUSPEND
> + * notification.
> + */
> + if (!ret) {
> + hdev->suspend_state_next = BT_SUSPEND_COMPLETE;
> + set_bit(SUSPEND_PREPARE_NOTIFIER, hdev->suspend_tasks);
> + queue_work(hdev->req_workqueue, &hdev->suspend_prepare);
> + ret = hci_suspend_wait_event(hdev);
> + }
> } else if (action == PM_POST_SUSPEND) {
> hdev->suspend_state_next = BT_RUNNING;
> set_bit(SUSPEND_PREPARE_NOTIFIER, hdev->suspend_tasks);
> queue_work(hdev->req_workqueue, &hdev->suspend_prepare);
> -
> ret = hci_suspend_wait_event(hdev);
> }
>
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index 591e7477e925..e3c36230f705 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -2474,6 +2474,7 @@ static void hci_inquiry_result_evt(struct hci_dev *hdev, struct sk_buff *skb)
> static void hci_conn_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
> {
> struct hci_ev_conn_complete *ev = (void *) skb->data;
> + struct inquiry_entry *ie;
> struct hci_conn *conn;
>
> BT_DBG("%s", hdev->name);
> @@ -2482,6 +2483,21 @@ static void hci_conn_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
>
> conn = hci_conn_hash_lookup_ba(hdev, ev->link_type, &ev->bdaddr);
> if (!conn) {
> + /* Connection may not exist if auto-connected. Check the inquiry
> + * cache to see if we've already discovered this bdaddr before.
> + * If found and link is an ACL type, create a connection class
> + * automatically.
> + */
> + ie = hci_inquiry_cache_lookup(hdev, &ev->bdaddr);
> + if (ie && ev->link_type == ACL_LINK) {
> + conn = hci_conn_add(hdev, ev->link_type, &ev->bdaddr,
> + HCI_ROLE_SLAVE);
> + if (!conn) {
> + bt_dev_err(hdev, "no memory for new conn");
> + goto unlock;
> + }
> + }
> +
> if (ev->link_type != SCO_LINK)
> goto unlock;
>
> @@ -2743,6 +2759,14 @@ static void hci_disconn_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
> hci_disconn_cfm(conn, ev->reason);
> hci_conn_del(conn);
>
> + /* The suspend notifier is waiting for all devices to disconnect so
> + * clear the bit from pending tasks and inform the wait queue.
> + */
> + if (list_empty(&hdev->conn_hash.list) &&
> + test_and_clear_bit(SUSPEND_DISCONNECTING, hdev->suspend_tasks)) {
> + wake_up(&hdev->suspend_wait_q);
> + }
> +
> /* Re-enable advertising if necessary, since it might
> * have been disabled by the connection. From the
> * HCI_LE_Set_Advertise_Enable command description in
> diff --git a/net/bluetooth/hci_request.c b/net/bluetooth/hci_request.c
> index 08908469c043..4d67b1d08608 100644
> --- a/net/bluetooth/hci_request.c
> +++ b/net/bluetooth/hci_request.c
> @@ -918,12 +918,62 @@ static u8 get_adv_instance_scan_rsp_len(struct hci_dev *hdev, u8 instance)
> return adv_instance->scan_rsp_len;
> }
>
> +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);
> +
> + /* Update page scan state (since we may have modified it when setting
> + * the event filter).
> + */
> + __hci_req_update_scan(req);
> +}
> +
> +static void hci_req_set_event_filter(struct hci_request *req)
> +{
> + struct bdaddr_list *b;
> + struct hci_cp_set_event_filter f;
> + struct hci_dev *hdev = req->hdev;
> + u8 scan;
> +
> + /* Always clear event filter when starting */
> + hci_req_clear_event_filter(req);
> +
> + list_for_each_entry(b, &hdev->wakeable, list) {
> + memset(&f, 0, sizeof(f));
> + bacpy(&f.addr_conn_flt.bdaddr, &b->bdaddr);
> + f.flt_type = HCI_FLT_CONN_SETUP;
> + f.cond_type = HCI_CONN_SETUP_ALLOW_BDADDR;
> + f.addr_conn_flt.auto_accept = HCI_CONN_SETUP_AUTO_ON;
> +
> + BT_DBG("Adding event filters for %pMR", &b->bdaddr);
> + hci_req_add(req, HCI_OP_SET_EVENT_FLT, sizeof(f), &f);
> + }
> +
> + scan = !list_empty(&hdev->wakeable) ? SCAN_PAGE : SCAN_DISABLED;
> + hci_req_add(req, HCI_OP_WRITE_SCAN_ENABLE, 1, &scan);
> +}
> +
> +static void suspend_req_complete(struct hci_dev *hdev, u8 status, u16 opcode)
> +{
> + BT_DBG("Request complete opcode=0x%x, status=0x%x", opcode, status);
> + if (test_and_clear_bit(SUSPEND_SCAN_ENABLE, hdev->suspend_tasks) ||
> + test_and_clear_bit(SUSPEND_SCAN_DISABLE, hdev->suspend_tasks)) {
> + wake_up(&hdev->suspend_wait_q);
> + }
> +}
> +
> /* Call with hci_dev_lock */
> void hci_req_prepare_suspend(struct hci_dev *hdev, enum suspended_state next)
> {
> int old_state;
> struct hci_conn *conn;
> struct hci_request req;
> + u8 page_scan;
> + int disconnect_counter;
>
> if (next == hdev->suspend_state) {
> BT_DBG("Same state before and after: %d", next);
> @@ -931,6 +981,54 @@ void hci_req_prepare_suspend(struct hci_dev *hdev, enum suspended_state next)
> }
>
> hdev->suspend_state = next;
> + hci_req_init(&req, hdev);
> +
> + if (next == BT_SUSPEND_DISCONNECT) {
> + /* Mark device as suspended */
> + hdev->suspended = true;
> +
> + /* Disable page scan */
> + page_scan = SCAN_DISABLED;
> + hci_req_add(&req, HCI_OP_WRITE_SCAN_ENABLE, 1, &page_scan);
> +
> + /* 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;
> +
> + /* Run commands before disconnecting */
> + hci_req_run(&req, suspend_req_complete);
> +
> + disconnect_counter = 0;
> + /* Soft disconnect everything (power off) */
> + list_for_each_entry(conn, &hdev->conn_hash.list, list) {
> + hci_disconnect(conn, HCI_ERROR_REMOTE_POWER_OFF);
> + disconnect_counter++;
> + }
> +
> + if (disconnect_counter > 0) {
> + BT_DBG("Had %d disconnects. Will wait on them",
> + disconnect_counter);
> + set_bit(SUSPEND_DISCONNECTING, hdev->suspend_tasks);
> + }
> + } else if (next == BT_SUSPEND_COMPLETE) {
> + /* Unpause to take care of updating scanning params */
> + hdev->scanning_paused = false;
> + /* Enable event filter for paired devices */
> + hci_req_set_event_filter(&req);
> + /* Pause scan changes again. */
> + hdev->scanning_paused = true;
> + hci_req_run(&req, suspend_req_complete);
> + } else {
> + hdev->suspended = false;
> + hdev->scanning_paused = false;
> +
> + hci_req_clear_event_filter(&req);
> + hci_req_run(&req, suspend_req_complete);
> + }
> +
> + hdev->suspend_state = next;
>
> done:
> clear_bit(SUSPEND_PREPARE_NOTIFIER, hdev->suspend_tasks);
> @@ -2034,6 +2132,9 @@ void __hci_req_update_scan(struct hci_request *req)
> if (mgmt_powering_down(hdev))
> return;
>
> + if (hdev->scanning_paused)
> + return;
> +
> if (hci_dev_test_flag(hdev, HCI_CONNECTABLE) ||
> disconnected_whitelist_entries(hdev))
> scan = SCAN_PAGE;

looks good. Just fold the struct list_head wakeable; change from 1/5 into this and use bt_dev_dbg.

Regards

Marcel

2020-04-09 17:33:18

by Abhishek Pandit-Subedi

[permalink] [raw]
Subject: Re: [RFC PATCH v4 0/5] Bluetooth: Handle system suspend gracefully

Hi,

Could you collect more logs w/ dynamic debug and post on the bug?

Dyndebug instructions:
https://www.kernel.org/doc/html/latest/admin-guide/dynamic-debug-howto.html

Please enable debug for files hci_core.c, hci_request.c and hci_event.c:
(for example: echo 'file hci_core.c +p' > <debugfs>/dynamic_debug/control)

Thanks
Abhishek

On Thu, Apr 9, 2020 at 10:08 AM youling257 <[email protected]> wrote:
>
> This patch series cause a suspend problem on BYT/CHT device, https://bugzilla.kernel.org/show_bug.cgi?id=207169