* [PATCH v2 0/1] Bluetooth: Suspend improvements @ 2021-03-02 18:49 Abhishek Pandit-Subedi 2021-03-02 18:49 ` [PATCH v2 1/1] Bluetooth: Remove unneeded commands for suspend Abhishek Pandit-Subedi 0 siblings, 1 reply; 4+ messages in thread From: Abhishek Pandit-Subedi @ 2021-03-02 18:49 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 Changes in v2: * Removed hci_dev_lock from hci_cc_set_event_filter since flags are set/cleared atomically Abhishek Pandit-Subedi (1): Bluetooth: Remove unneeded commands for suspend include/net/bluetooth/hci.h | 1 + net/bluetooth/hci_event.c | 24 ++++++++++++++++++++ net/bluetooth/hci_request.c | 44 +++++++++++++++++++++++-------------- 3 files changed, 52 insertions(+), 17 deletions(-) -- 2.30.1.766.gb4fecdf3b7-goog ^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH v2 1/1] Bluetooth: Remove unneeded commands for suspend 2021-03-02 18:49 [PATCH v2 0/1] Bluetooth: Suspend improvements Abhishek Pandit-Subedi @ 2021-03-02 18:49 ` Abhishek Pandit-Subedi 2021-03-03 0:11 ` Bluetooth: Suspend improvements bluez.test.bot 2021-03-03 15:47 ` [PATCH v2 1/1] Bluetooth: Remove unneeded commands for suspend Marcel Holtmann 0 siblings, 2 replies; 4+ messages in thread From: Abhishek Pandit-Subedi @ 2021-03-02 18:49 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> --- Changes in v2: * Removed hci_dev_lock from hci_cc_set_event_filter since flags are set/cleared atomically include/net/bluetooth/hci.h | 1 + net/bluetooth/hci_event.c | 24 ++++++++++++++++++++ net/bluetooth/hci_request.c | 44 +++++++++++++++++++++++-------------- 3 files changed, 52 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..6eadc999ea1474 100644 --- a/net/bluetooth/hci_event.c +++ b/net/bluetooth/hci_event.c @@ -395,6 +395,26 @@ 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 || status) + return; + + cp = (struct hci_cp_set_event_filter *)sent; + + 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); +} + 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 +3348,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] 4+ messages in thread
* RE: Bluetooth: Suspend improvements 2021-03-02 18:49 ` [PATCH v2 1/1] Bluetooth: Remove unneeded commands for suspend Abhishek Pandit-Subedi @ 2021-03-03 0:11 ` bluez.test.bot 2021-03-03 15:47 ` [PATCH v2 1/1] Bluetooth: Remove unneeded commands for suspend Marcel Holtmann 1 sibling, 0 replies; 4+ messages in thread From: bluez.test.bot @ 2021-03-03 0:11 UTC (permalink / raw) To: linux-bluetooth, abhishekpandit [-- Attachment #1: Type: text/plain, Size: 1599 bytes --] This is automated email and please do not reply to this email! Dear submitter, Thank you for submitting the patches to the linux bluetooth mailing list. This is a CI test results with your patch series: PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=440759 ---Test result--- ############################## Test: CheckPatch - PASS ############################## Test: CheckGitLint - PASS ############################## Test: CheckBuildK - PASS ############################## Test: CheckTestRunner: Setup - PASS ############################## Test: CheckTestRunner: l2cap-tester - PASS Total: 40, Passed: 34 (85.0%), Failed: 0, Not Run: 6 ############################## Test: CheckTestRunner: bnep-tester - PASS Total: 1, Passed: 1 (100.0%), Failed: 0, Not Run: 0 ############################## Test: CheckTestRunner: mgmt-tester - PASS Total: 416, Passed: 400 (96.2%), Failed: 0, Not Run: 16 ############################## Test: CheckTestRunner: rfcomm-tester - PASS Total: 9, Passed: 9 (100.0%), Failed: 0, Not Run: 0 ############################## Test: CheckTestRunner: sco-tester - PASS Total: 8, Passed: 8 (100.0%), Failed: 0, Not Run: 0 ############################## Test: CheckTestRunner: smp-tester - PASS Total: 8, Passed: 8 (100.0%), Failed: 0, Not Run: 0 ############################## Test: CheckTestRunner: userchan-tester - PASS Total: 3, Passed: 3 (100.0%), Failed: 0, Not Run: 0 --- Regards, Linux Bluetooth [-- Attachment #2: l2cap-tester.log --] [-- Type: application/octet-stream, Size: 43341 bytes --] [-- Attachment #3: bnep-tester.log --] [-- Type: application/octet-stream, Size: 3531 bytes --] [-- Attachment #4: mgmt-tester.log --] [-- Type: application/octet-stream, Size: 546229 bytes --] [-- Attachment #5: rfcomm-tester.log --] [-- Type: application/octet-stream, Size: 11652 bytes --] [-- Attachment #6: sco-tester.log --] [-- Type: application/octet-stream, Size: 9887 bytes --] [-- Attachment #7: smp-tester.log --] [-- Type: application/octet-stream, Size: 11798 bytes --] [-- Attachment #8: userchan-tester.log --] [-- Type: application/octet-stream, Size: 5429 bytes --] ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2 1/1] Bluetooth: Remove unneeded commands for suspend 2021-03-02 18:49 ` [PATCH v2 1/1] Bluetooth: Remove unneeded commands for suspend Abhishek Pandit-Subedi 2021-03-03 0:11 ` Bluetooth: Suspend improvements bluez.test.bot @ 2021-03-03 15:47 ` Marcel Holtmann 1 sibling, 0 replies; 4+ messages in thread From: Marcel Holtmann @ 2021-03-03 15:47 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> > --- > > Changes in v2: > * Removed hci_dev_lock from hci_cc_set_event_filter since flags are > set/cleared atomically > > include/net/bluetooth/hci.h | 1 + > net/bluetooth/hci_event.c | 24 ++++++++++++++++++++ > net/bluetooth/hci_request.c | 44 +++++++++++++++++++++++-------------- > 3 files changed, 52 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..6eadc999ea1474 100644 > --- a/net/bluetooth/hci_event.c > +++ b/net/bluetooth/hci_event.c > @@ -395,6 +395,26 @@ 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 || status) > + return; can we do this: if (status) return; sent = hci_.. if (!sent) return; Regards Marcel ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-03-03 19:07 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-03-02 18:49 [PATCH v2 0/1] Bluetooth: Suspend improvements Abhishek Pandit-Subedi 2021-03-02 18:49 ` [PATCH v2 1/1] Bluetooth: Remove unneeded commands for suspend Abhishek Pandit-Subedi 2021-03-03 0:11 ` Bluetooth: Suspend improvements bluez.test.bot 2021-03-03 15:47 ` [PATCH v2 1/1] Bluetooth: Remove unneeded commands for suspend Marcel Holtmann
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.