| Message ID | 20210909140945.v6.1.Id9bc5434114de07512661f002cdc0ada8b3d6d02@changeid (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [v6] Bluetooth: Keep MSFT ext info throughout a hci_dev's life cycle | expand |
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=544617
---Test result---
Test Summary:
CheckPatch FAIL 0.95 seconds
GitLint PASS 0.13 seconds
BuildKernel PASS 591.02 seconds
TestRunner: Setup PASS 388.03 seconds
TestRunner: l2cap-tester PASS 2.74 seconds
TestRunner: bnep-tester PASS 2.02 seconds
TestRunner: mgmt-tester FAIL 32.52 seconds
TestRunner: rfcomm-tester PASS 2.30 seconds
TestRunner: sco-tester PASS 2.26 seconds
TestRunner: smp-tester PASS 2.32 seconds
TestRunner: userchan-tester PASS 2.05 seconds
Details
##############################
Test: CheckPatch - FAIL - 0.95 seconds
Run checkpatch.pl script with rule in .checkpatch.conf
Bluetooth: Keep MSFT ext info throughout a hci_dev's life cycle
WARNING: Possible unnecessary 'out of memory' message
#121: FILE: net/bluetooth/msft.c:255:
+ if (!msft) {
+ bt_dev_err(hdev, "Failed to register MSFT extension");
total: 0 errors, 1 warnings, 0 checks, 119 lines checked
NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.
"[PATCH] Bluetooth: Keep MSFT ext info throughout a hci_dev's life" has style problems, please review.
NOTE: If any of the errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.
##############################
Test: GitLint - PASS - 0.13 seconds
Run gitlint with rule in .gitlint
##############################
Test: BuildKernel - PASS - 591.02 seconds
Build Kernel with minimal configuration supports Bluetooth
##############################
Test: TestRunner: Setup - PASS - 388.03 seconds
Setup environment for running Test Runner
##############################
Test: TestRunner: l2cap-tester - PASS - 2.74 seconds
Run test-runner with l2cap-tester
Total: 40, Passed: 40 (100.0%), Failed: 0, Not Run: 0
##############################
Test: TestRunner: bnep-tester - PASS - 2.02 seconds
Run test-runner with bnep-tester
Total: 1, Passed: 1 (100.0%), Failed: 0, Not Run: 0
##############################
Test: TestRunner: mgmt-tester - FAIL - 32.52 seconds
Run test-runner with mgmt-tester
Total: 452, Passed: 451 (99.8%), Failed: 1, Not Run: 0
Failed Test Cases
Read Exp Feature - Success Failed 0.016 seconds
##############################
Test: TestRunner: rfcomm-tester - PASS - 2.30 seconds
Run test-runner with rfcomm-tester
Total: 9, Passed: 9 (100.0%), Failed: 0, Not Run: 0
##############################
Test: TestRunner: sco-tester - PASS - 2.26 seconds
Run test-runner with sco-tester
Total: 11, Passed: 11 (100.0%), Failed: 0, Not Run: 0
##############################
Test: TestRunner: smp-tester - PASS - 2.32 seconds
Run test-runner with smp-tester
Total: 8, Passed: 8 (100.0%), Failed: 0, Not Run: 0
##############################
Test: TestRunner: userchan-tester - PASS - 2.05 seconds
Run test-runner with userchan-tester
Total: 3, Passed: 3 (100.0%), Failed: 0, Not Run: 0
---
Regards,
Linux Bluetooth
Hi Manish, > This splits the msft_do_{open/close} to msft_do_{open/close} and > msft_{register/unregister}. With this change it is possible to retain > the MSFT extension info irrespective of controller power on/off state. > This helps bluetoothd to report correct 'supported features' of the > controller to the D-Bus clients event if the controller is off. It also > re-reads the MSFT info upon every msft_do_open(). > > The following test steps were performed. > 1. Boot the test device and verify the MSFT support debug log in syslog. > 2. Power off the controller and read the 'supported features', power on > and read again. > 3. Restart the bluetoothd and verify the 'supported features' value. > > Signed-off-by: Miao-chen Chou <mcchou@chromium.org> > Reviewed-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org> > Reviewed-by: Archie Pusaka <apusaka@chromium.org> > Reviewed-by: Alain Michaud <alainm@chromium.org> > Signed-off-by: Manish Mandlik <mmandlik@google.com> > --- > > Changes in v6: > - Split msft_do_{open/close} into msft_do_{open/close} and > msft_{register/unregister} > > Changes in v5: > - Rebase on ToT and remove extra blank line > > Changes in v4: > - Re-read the MSFT data instead of skipping if it's initiated already > > Changes in v3: > - Remove the accepted commits from the series > > net/bluetooth/hci_core.c | 3 +++ > net/bluetooth/msft.c | 55 +++++++++++++++++++++++++++++++++------- > net/bluetooth/msft.h | 4 +++ > 3 files changed, 53 insertions(+), 9 deletions(-) patch has been applied to bluetooth-next tree. Regards Marcel
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c index fb296478b86e..8af0ea0934fa 100644 --- a/net/bluetooth/hci_core.c +++ b/net/bluetooth/hci_core.c @@ -3994,6 +3994,7 @@ int hci_register_dev(struct hci_dev *hdev) queue_work(hdev->req_workqueue, &hdev->power_on); idr_init(&hdev->adv_monitors_idr); + msft_register(hdev); return id; @@ -4026,6 +4027,8 @@ void hci_unregister_dev(struct hci_dev *hdev) cancel_work_sync(&hdev->suspend_prepare); } + msft_unregister(hdev); + hci_dev_do_close(hdev); if (!test_bit(HCI_INIT, &hdev->flags) && diff --git a/net/bluetooth/msft.c b/net/bluetooth/msft.c index b4bfae41e8a5..21b1787e7893 100644 --- a/net/bluetooth/msft.c +++ b/net/bluetooth/msft.c @@ -184,28 +184,36 @@ static void reregister_monitor_on_restart(struct hci_dev *hdev, int handle) void msft_do_open(struct hci_dev *hdev) { - struct msft_data *msft; + struct msft_data *msft = hdev->msft_data; if (hdev->msft_opcode == HCI_OP_NOP) return; + if (!msft) { + bt_dev_err(hdev, "MSFT extension not registered"); + return; + } + bt_dev_dbg(hdev, "Initialize MSFT extension"); - msft = kzalloc(sizeof(*msft), GFP_KERNEL); - if (!msft) - return; + /* Reset existing MSFT data before re-reading */ + kfree(msft->evt_prefix); + msft->evt_prefix = NULL; + msft->evt_prefix_len = 0; + msft->features = 0; if (!read_supported_features(hdev, msft)) { + hdev->msft_data = NULL; kfree(msft); return; } - INIT_LIST_HEAD(&msft->handle_map); - hdev->msft_data = msft; - if (msft_monitor_supported(hdev)) { msft->reregistering = true; msft_set_filter_enable(hdev, true); + /* Monitors get removed on power off, so we need to explicitly + * tell the controller to re-monitor. + */ reregister_monitor_on_restart(hdev, 0); } } @@ -221,8 +229,9 @@ void msft_do_close(struct hci_dev *hdev) bt_dev_dbg(hdev, "Cleanup of MSFT extension"); - hdev->msft_data = NULL; - + /* The controller will silently remove all monitors on power off. + * Therefore, remove handle_data mapping and reset monitor state. + */ list_for_each_entry_safe(handle_data, tmp, &msft->handle_map, list) { monitor = idr_find(&hdev->adv_monitors_idr, handle_data->mgmt_handle); @@ -233,6 +242,34 @@ void msft_do_close(struct hci_dev *hdev) list_del(&handle_data->list); kfree(handle_data); } +} + +void msft_register(struct hci_dev *hdev) +{ + struct msft_data *msft = NULL; + + bt_dev_dbg(hdev, "Register MSFT extension"); + + msft = kzalloc(sizeof(*msft), GFP_KERNEL); + if (!msft) { + bt_dev_err(hdev, "Failed to register MSFT extension"); + return; + } + + INIT_LIST_HEAD(&msft->handle_map); + hdev->msft_data = msft; +} + +void msft_unregister(struct hci_dev *hdev) +{ + struct msft_data *msft = hdev->msft_data; + + if (!msft) + return; + + bt_dev_dbg(hdev, "Unregister MSFT extension"); + + hdev->msft_data = NULL; kfree(msft->evt_prefix); kfree(msft); diff --git a/net/bluetooth/msft.h b/net/bluetooth/msft.h index 6e56d94b88d8..8018948c5975 100644 --- a/net/bluetooth/msft.h +++ b/net/bluetooth/msft.h @@ -13,6 +13,8 @@ #if IS_ENABLED(CONFIG_BT_MSFTEXT) bool msft_monitor_supported(struct hci_dev *hdev); +void msft_register(struct hci_dev *hdev); +void msft_unregister(struct hci_dev *hdev); void msft_do_open(struct hci_dev *hdev); void msft_do_close(struct hci_dev *hdev); void msft_vendor_evt(struct hci_dev *hdev, struct sk_buff *skb); @@ -31,6 +33,8 @@ static inline bool msft_monitor_supported(struct hci_dev *hdev) return false; } +static inline void msft_register(struct hci_dev *hdev) {} +static inline void msft_unregister(struct hci_dev *hdev) {} static inline void msft_do_open(struct hci_dev *hdev) {} static inline void msft_do_close(struct hci_dev *hdev) {} static inline void msft_vendor_evt(struct hci_dev *hdev, struct sk_buff *skb) {}