| From 0b166c3014ed9c1d3e68256c9b96c3451c30e26e Mon Sep 17 00:00:00 2001 |
| From: Manish Mandlik <mmandlik@google.com> |
| Date: Tue, 21 Sep 2021 14:47:10 -0700 |
| Subject: [PATCH] BACKPORT: FROMGIT: Bluetooth: Fix Advertisement Monitor |
| Suspend/Resume |
| MIME-Version: 1.0 |
| Content-Type: text/plain; charset=UTF-8 |
| Content-Transfer-Encoding: 8bit |
| |
| During system suspend, advertisement monitoring is disabled by setting |
| the HCI_VS_MSFT_LE_Set_Advertisement_Filter_Enable to False. This |
| disables the monitoring during suspend, however, if the controller is |
| monitoring a device, it sends HCI_VS_MSFT_LE_Monitor_Device_Event to |
| indicate that the monitoring has been stopped for that particular |
| device. This event may occur after suspend depending on the |
| low_threshold_timeout and peer device advertisement frequency, which |
| causes early wake up. |
| |
| Right way to disable the monitoring for suspend is by removing all the |
| monitors before suspend and re-monitor after resume to ensure no events |
| are received during suspend. This patch fixes this suspend/resume issue. |
| |
| Following tests are performed: |
| - Add monitors before suspend and make sure DeviceFound gets triggered |
| - Suspend the system and verify that all monitors are removed by kernel |
| but not Released by bluetoothd |
| - Wake up and verify that all monitors are added again and DeviceFound |
| gets triggered |
| |
| Signed-off-by: Manish Mandlik <mmandlik@google.com> |
| Reviewed-by: Archie Pusaka <apusaka@google.com> |
| Reviewed-by: Miao-chen Chou <mcchou@google.com> |
| Signed-off-by: Marcel Holtmann <marcel@holtmann.org> |
| (cherry picked from commit ce81843be24e9d5deb0db0784815efe84c9e3f22 |
| git://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git |
| master) |
| |
| BUG=b:192392727 |
| TEST=Above tests steps are performed |
| |
| Signed-off-by: Manish Mandlik <mmandlik@google.com> |
| Change-Id: I19b781b4dbe549da8b75d27e616b50c91927eb2c |
| Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/3192641 |
| Tested-by: Manish Mandlik <mmandlik@chromium.org> |
| Auto-Submit: Manish Mandlik <mmandlik@chromium.org> |
| Commit-Queue: Manish Mandlik <mmandlik@chromium.org> |
| Commit-Queue: Archie Pusaka <apusaka@chromium.org> |
| Reviewed-by: Archie Pusaka <apusaka@chromium.org> |
| --- |
| net/bluetooth/hci_request.c | 15 +++-- |
| net/bluetooth/msft.c | 117 +++++++++++++++++++++++++++++++----- |
| net/bluetooth/msft.h | 5 ++ |
| 3 files changed, 116 insertions(+), 21 deletions(-) |
| |
| diff --git a/net/bluetooth/hci_request.c b/net/bluetooth/hci_request.c |
| --- a/net/bluetooth/hci_request.c |
| +++ b/net/bluetooth/hci_request.c |
| @@ -1277,21 +1277,24 @@ static void suspend_req_complete(struct hci_dev *hdev, u8 status, u16 opcode) |
| } |
| } |
| |
| -static void hci_req_add_set_adv_filter_enable(struct hci_request *req, |
| - bool enable) |
| +static void hci_req_prepare_adv_monitor_suspend(struct hci_request *req, |
| + bool suspending) |
| { |
| struct hci_dev *hdev = req->hdev; |
| |
| switch (hci_get_adv_monitor_offload_ext(hdev)) { |
| case HCI_ADV_MONITOR_EXT_MSFT: |
| - msft_req_add_set_filter_enable(req, enable); |
| + if (suspending) |
| + msft_suspend(hdev); |
| + else |
| + msft_resume(hdev); |
| break; |
| default: |
| return; |
| } |
| |
| /* No need to block when enabling since it's on resume path */ |
| - if (hdev->suspended && !enable) |
| + if (hdev->suspended && suspending) |
| set_bit(SUSPEND_SET_ADV_FILTER, hdev->suspend_tasks); |
| } |
| |
| @@ -1358,7 +1361,7 @@ 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); |
| + hci_req_prepare_adv_monitor_suspend(&req, true); |
| |
| /* Prevent disconnects from causing scanning to be re-enabled */ |
| hdev->scanning_paused = true; |
| @@ -1400,7 +1403,7 @@ void hci_req_prepare_suspend(struct hci_dev *hdev, enum suspended_state next) |
| /* Reset passive/background scanning to normal */ |
| __hci_update_background_scan(&req); |
| /* Enable all of the advertisement filters */ |
| - hci_req_add_set_adv_filter_enable(&req, true); |
| + hci_req_prepare_adv_monitor_suspend(&req, false); |
| |
| /* Unpause directed advertising */ |
| hdev->advertising_paused = false; |
| diff --git a/net/bluetooth/msft.c b/net/bluetooth/msft.c |
| --- a/net/bluetooth/msft.c |
| +++ b/net/bluetooth/msft.c |
| @@ -94,11 +94,14 @@ struct msft_data { |
| __u16 pending_add_handle; |
| __u16 pending_remove_handle; |
| __u8 reregistering; |
| + __u8 suspending; |
| __u8 filter_enabled; |
| }; |
| |
| static int __msft_add_monitor_pattern(struct hci_dev *hdev, |
| struct adv_monitor *monitor); |
| +static int __msft_remove_monitor(struct hci_dev *hdev, |
| + struct adv_monitor *monitor, u16 handle); |
| |
| bool msft_monitor_supported(struct hci_dev *hdev) |
| { |
| @@ -154,7 +157,7 @@ static bool read_supported_features(struct hci_dev *hdev, |
| } |
| |
| /* This function requires the caller holds hdev->lock */ |
| -static void reregister_monitor_on_restart(struct hci_dev *hdev, int handle) |
| +static void reregister_monitor(struct hci_dev *hdev, int handle) |
| { |
| struct adv_monitor *monitor; |
| struct msft_data *msft = hdev->msft_data; |
| @@ -182,6 +185,69 @@ static void reregister_monitor_on_restart(struct hci_dev *hdev, int handle) |
| } |
| } |
| |
| +/* This function requires the caller holds hdev->lock */ |
| +static void remove_monitor_on_suspend(struct hci_dev *hdev, int handle) |
| +{ |
| + struct adv_monitor *monitor; |
| + struct msft_data *msft = hdev->msft_data; |
| + int err; |
| + |
| + while (1) { |
| + monitor = idr_get_next(&hdev->adv_monitors_idr, &handle); |
| + if (!monitor) { |
| + /* All monitors have been removed */ |
| + msft->suspending = false; |
| + hci_update_background_scan(hdev); |
| + return; |
| + } |
| + |
| + msft->pending_remove_handle = (u16)handle; |
| + err = __msft_remove_monitor(hdev, monitor, handle); |
| + |
| + /* If success, return and wait for monitor removed callback */ |
| + if (!err) |
| + return; |
| + |
| + /* Otherwise free the monitor and keep removing */ |
| + hci_free_adv_monitor(hdev, monitor); |
| + handle++; |
| + } |
| +} |
| + |
| +/* This function requires the caller holds hdev->lock */ |
| +void msft_suspend(struct hci_dev *hdev) |
| +{ |
| + struct msft_data *msft = hdev->msft_data; |
| + |
| + if (!msft) |
| + return; |
| + |
| + if (msft_monitor_supported(hdev)) { |
| + msft->suspending = true; |
| + /* Quitely remove all monitors on suspend to avoid waking up |
| + * the system. |
| + */ |
| + remove_monitor_on_suspend(hdev, 0); |
| + } |
| +} |
| + |
| +/* This function requires the caller holds hdev->lock */ |
| +void msft_resume(struct hci_dev *hdev) |
| +{ |
| + struct msft_data *msft = hdev->msft_data; |
| + |
| + if (!msft) |
| + return; |
| + |
| + if (msft_monitor_supported(hdev)) { |
| + msft->reregistering = true; |
| + /* Monitors are removed on suspend, so we need to add all |
| + * monitors on resume. |
| + */ |
| + reregister_monitor(hdev, 0); |
| + } |
| +} |
| + |
| void msft_do_open(struct hci_dev *hdev) |
| { |
| struct msft_data *msft = hdev->msft_data; |
| @@ -214,7 +280,7 @@ void msft_do_open(struct hci_dev *hdev) |
| /* Monitors get removed on power off, so we need to explicitly |
| * tell the controller to re-monitor. |
| */ |
| - reregister_monitor_on_restart(hdev, 0); |
| + reregister_monitor(hdev, 0); |
| } |
| } |
| |
| @@ -382,8 +448,7 @@ static void msft_le_monitor_advertisement_cb(struct hci_dev *hdev, |
| |
| /* If in restart/reregister sequence, keep registering. */ |
| if (msft->reregistering) |
| - reregister_monitor_on_restart(hdev, |
| - msft->pending_add_handle + 1); |
| + reregister_monitor(hdev, msft->pending_add_handle + 1); |
| |
| hci_dev_unlock(hdev); |
| |
| @@ -420,13 +485,25 @@ static void msft_le_cancel_monitor_advertisement_cb(struct hci_dev *hdev, |
| if (handle_data) { |
| monitor = idr_find(&hdev->adv_monitors_idr, |
| handle_data->mgmt_handle); |
| - if (monitor) |
| + |
| + if (monitor && monitor->state == ADV_MONITOR_STATE_OFFLOADED) |
| + monitor->state = ADV_MONITOR_STATE_REGISTERED; |
| + |
| + /* Do not free the monitor if it is being removed due to |
| + * suspend. It will be re-monitored on resume. |
| + */ |
| + if (monitor && !msft->suspending) |
| hci_free_adv_monitor(hdev, monitor); |
| |
| list_del(&handle_data->list); |
| kfree(handle_data); |
| } |
| |
| + /* If in suspend/remove sequence, keep removing. */ |
| + if (msft->suspending) |
| + remove_monitor_on_suspend(hdev, |
| + msft->pending_remove_handle + 1); |
| + |
| /* If remove all monitors is required, we need to continue the process |
| * here because the earlier it was paused when waiting for the |
| * response from controller. |
| @@ -445,7 +522,8 @@ static void msft_le_cancel_monitor_advertisement_cb(struct hci_dev *hdev, |
| hci_dev_unlock(hdev); |
| |
| done: |
| - hci_remove_adv_monitor_complete(hdev, status); |
| + if (!msft->suspending) |
| + hci_remove_adv_monitor_complete(hdev, status); |
| } |
| |
| static void msft_le_set_advertisement_filter_enable_cb(struct hci_dev *hdev, |
| @@ -578,15 +656,15 @@ int msft_add_monitor_pattern(struct hci_dev *hdev, struct adv_monitor *monitor) |
| if (!msft) |
| return -EOPNOTSUPP; |
| |
| - if (msft->reregistering) |
| + if (msft->reregistering || msft->suspending) |
| return -EBUSY; |
| |
| return __msft_add_monitor_pattern(hdev, monitor); |
| } |
| |
| /* This function requires the caller holds hdev->lock */ |
| -int msft_remove_monitor(struct hci_dev *hdev, struct adv_monitor *monitor, |
| - u16 handle) |
| +static int __msft_remove_monitor(struct hci_dev *hdev, |
| + struct adv_monitor *monitor, u16 handle) |
| { |
| struct msft_cp_le_cancel_monitor_advertisement cp; |
| struct msft_monitor_advertisement_handle_data *handle_data; |
| @@ -594,12 +672,6 @@ int msft_remove_monitor(struct hci_dev *hdev, struct adv_monitor *monitor, |
| struct msft_data *msft = hdev->msft_data; |
| int err = 0; |
| |
| - if (!msft) |
| - return -EOPNOTSUPP; |
| - |
| - if (msft->reregistering) |
| - return -EBUSY; |
| - |
| handle_data = msft_find_handle_data(hdev, monitor->handle, true); |
| |
| /* If no matched handle, just remove without telling controller */ |
| @@ -619,6 +691,21 @@ int msft_remove_monitor(struct hci_dev *hdev, struct adv_monitor *monitor, |
| return err; |
| } |
| |
| +/* This function requires the caller holds hdev->lock */ |
| +int msft_remove_monitor(struct hci_dev *hdev, struct adv_monitor *monitor, |
| + u16 handle) |
| +{ |
| + struct msft_data *msft = hdev->msft_data; |
| + |
| + if (!msft) |
| + return -EOPNOTSUPP; |
| + |
| + if (msft->reregistering || msft->suspending) |
| + return -EBUSY; |
| + |
| + return __msft_remove_monitor(hdev, monitor, handle); |
| +} |
| + |
| void msft_req_add_set_filter_enable(struct hci_request *req, bool enable) |
| { |
| struct hci_dev *hdev = req->hdev; |
| diff --git a/net/bluetooth/msft.h b/net/bluetooth/msft.h |
| --- a/net/bluetooth/msft.h |
| +++ b/net/bluetooth/msft.h |
| @@ -24,6 +24,8 @@ int msft_remove_monitor(struct hci_dev *hdev, struct adv_monitor *monitor, |
| u16 handle); |
| void msft_req_add_set_filter_enable(struct hci_request *req, bool enable); |
| int msft_set_filter_enable(struct hci_dev *hdev, bool enable); |
| +void msft_suspend(struct hci_dev *hdev); |
| +void msft_resume(struct hci_dev *hdev); |
| bool msft_curve_validity(struct hci_dev *hdev); |
| |
| #else |
| @@ -59,6 +61,9 @@ static inline int msft_set_filter_enable(struct hci_dev *hdev, bool enable) |
| return -EOPNOTSUPP; |
| } |
| |
| +static inline void msft_suspend(struct hci_dev *hdev) {} |
| +static inline void msft_resume(struct hci_dev *hdev) {} |
| + |
| static inline bool msft_curve_validity(struct hci_dev *hdev) |
| { |
| return false; |
| -- |
| 2.33.0.800.g4c38ced690-goog |
| |