| Message ID | 20220405215207.1415731-1-luiz.dentz@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Bluetooth: hci_sync: Split hci_dev_open_sync | expand |
| Context | Check | Description |
|---|---|---|
| tedd_an/pre-ci_am | success | Success |
| tedd_an/checkpatch | success | Checkpatch PASS |
| tedd_an/gitlint | success | Gitlint PASS |
| tedd_an/subjectprefix | success | PASS |
| tedd_an/buildkernel | success | Build Kernel PASS |
| tedd_an/buildkernel32 | success | Build Kernel32 PASS |
| tedd_an/incremental_build | success | Pass |
| tedd_an/testrunnersetup | success | Test Runner Setup PASS |
| tedd_an/testrunnerl2cap-tester | success | Total: 40, Passed: 40 (100.0%), Failed: 0, Not Run: 0 |
| tedd_an/testrunnerbnep-tester | success | Total: 1, Passed: 1 (100.0%), Failed: 0, Not Run: 0 |
| tedd_an/testrunnermgmt-tester | success | Total: 493, Passed: 493 (100.0%), Failed: 0, Not Run: 0 |
| tedd_an/testrunnerrfcomm-tester | success | Total: 10, Passed: 10 (100.0%), Failed: 0, Not Run: 0 |
| tedd_an/testrunnersco-tester | success | Total: 12, Passed: 12 (100.0%), Failed: 0, Not Run: 0 |
| tedd_an/testrunnersmp-tester | success | Total: 8, Passed: 8 (100.0%), Failed: 0, Not Run: 0 |
| tedd_an/testrunneruserchan-tester | success | Total: 4, Passed: 4 (100.0%), Failed: 0, Not Run: 0 |
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=629301 ---Test result--- Test Summary: CheckPatch PASS 1.78 seconds GitLint PASS 1.12 seconds SubjectPrefix PASS 0.94 seconds BuildKernel PASS 31.71 seconds BuildKernel32 PASS 27.35 seconds Incremental Build with patchesPASS 36.12 seconds TestRunner: Setup PASS 441.79 seconds TestRunner: l2cap-tester PASS 15.41 seconds TestRunner: bnep-tester PASS 5.96 seconds TestRunner: mgmt-tester PASS 102.57 seconds TestRunner: rfcomm-tester PASS 7.97 seconds TestRunner: sco-tester PASS 7.87 seconds TestRunner: smp-tester PASS 8.00 seconds TestRunner: userchan-tester PASS 6.63 seconds --- Regards, Linux Bluetooth
Hi Marcel, On Tue, Apr 5, 2022 at 2:52 PM Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote: > > From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> > > This splits hci_dev_open_sync so each stage is handle by its own > function so it is easier to identify each stage. > > Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> > --- > net/bluetooth/hci_sync.c | 255 ++++++++++++++++++++++----------------- > 1 file changed, 141 insertions(+), 114 deletions(-) > > diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c > index 5610ec1242d6..2d3b9adbd215 100644 > --- a/net/bluetooth/hci_sync.c > +++ b/net/bluetooth/hci_sync.c > @@ -3849,9 +3849,148 @@ static const struct { > "advertised, but not supported.") > }; > > -int hci_dev_open_sync(struct hci_dev *hdev) > +/* This function handles hdev setup stage: > + * > + * Calls hdev->setup > + * Setup address if HCI_QUIRK_USE_BDADDR_PROPERTY is set. > + */ > +static int hci_dev_setup_sync(struct hci_dev *hdev) > { > int ret = 0; > + bool invalid_bdaddr; > + size_t i; > + > + if (!hci_dev_test_flag(hdev, HCI_SETUP) && > + !test_bit(HCI_QUIRK_NON_PERSISTENT_SETUP, &hdev->quirks)) > + return 0; > + > + bt_dev_dbg(hdev, ""); > + > + hci_sock_dev_event(hdev, HCI_DEV_SETUP); > + > + if (hdev->setup) > + ret = hdev->setup(hdev); > + > + for (i = 0; i < ARRAY_SIZE(hci_broken_table); i++) { > + if (test_bit(hci_broken_table[i].quirk, &hdev->quirks)) > + bt_dev_warn(hdev, "%s", hci_broken_table[i].desc); > + } > + > + /* The transport driver can set the quirk to mark the > + * BD_ADDR invalid before creating the HCI device or in > + * its setup callback. > + */ > + invalid_bdaddr = test_bit(HCI_QUIRK_INVALID_BDADDR, &hdev->quirks); > + > + if (!ret) { > + if (test_bit(HCI_QUIRK_USE_BDADDR_PROPERTY, &hdev->quirks)) { > + if (!bacmp(&hdev->public_addr, BDADDR_ANY)) > + hci_dev_get_bd_addr_from_property(hdev); > + > + if (bacmp(&hdev->public_addr, BDADDR_ANY) && > + hdev->set_bdaddr) { > + ret = hdev->set_bdaddr(hdev, > + &hdev->public_addr); > + > + /* If setting of the BD_ADDR from the device > + * property succeeds, then treat the address > + * as valid even if the invalid BD_ADDR > + * quirk indicates otherwise. > + */ > + if (!ret) > + invalid_bdaddr = false; > + } > + } > + } > + > + /* The transport driver can set these quirks before > + * creating the HCI device or in its setup callback. > + * > + * For the invalid BD_ADDR quirk it is possible that > + * it becomes a valid address if the bootloader does > + * provide it (see above). > + * > + * In case any of them is set, the controller has to > + * start up as unconfigured. > + */ > + if (test_bit(HCI_QUIRK_EXTERNAL_CONFIG, &hdev->quirks) || > + invalid_bdaddr) > + hci_dev_set_flag(hdev, HCI_UNCONFIGURED); > + > + /* For an unconfigured controller it is required to > + * read at least the version information provided by > + * the Read Local Version Information command. > + * > + * If the set_bdaddr driver callback is provided, then > + * also the original Bluetooth public device address > + * will be read using the Read BD Address command. > + */ > + if (hci_dev_test_flag(hdev, HCI_UNCONFIGURED)) > + return hci_unconf_init_sync(hdev); > + > + return ret; > +} > + > +/* This function handles hdev init stage: > + * > + * Calls hci_dev_setup_sync to perform setup stage > + * Calls hci_init_sync to perform HCI command init sequence > + */ > +static int hci_dev_init_sync(struct hci_dev *hdev) > +{ > + int ret; > + > + bt_dev_dbg(hdev, ""); > + > + atomic_set(&hdev->cmd_cnt, 1); > + set_bit(HCI_INIT, &hdev->flags); > + > + ret = hci_dev_setup_sync(hdev); > + > + if (hci_dev_test_flag(hdev, HCI_CONFIG)) { > + /* If public address change is configured, ensure that > + * the address gets programmed. If the driver does not > + * support changing the public address, fail the power > + * on procedure. > + */ > + if (bacmp(&hdev->public_addr, BDADDR_ANY) && > + hdev->set_bdaddr) > + ret = hdev->set_bdaddr(hdev, &hdev->public_addr); > + else > + ret = -EADDRNOTAVAIL; > + } > + > + if (!ret) { > + if (!hci_dev_test_flag(hdev, HCI_UNCONFIGURED) && > + !hci_dev_test_flag(hdev, HCI_USER_CHANNEL)) { > + ret = hci_init_sync(hdev); > + if (!ret && hdev->post_init) > + ret = hdev->post_init(hdev); > + } > + } > + > + /* If the HCI Reset command is clearing all diagnostic settings, > + * then they need to be reprogrammed after the init procedure > + * completed. > + */ > + if (test_bit(HCI_QUIRK_NON_PERSISTENT_DIAG, &hdev->quirks) && > + !hci_dev_test_flag(hdev, HCI_USER_CHANNEL) && > + hci_dev_test_flag(hdev, HCI_VENDOR_DIAG) && hdev->set_diag) > + ret = hdev->set_diag(hdev, true); > + > + if (!hci_dev_test_flag(hdev, HCI_USER_CHANNEL)) { > + msft_do_open(hdev); > + aosp_do_open(hdev); > + } > + > + clear_bit(HCI_INIT, &hdev->flags); > + > + return ret; > +} > + > +int hci_dev_open_sync(struct hci_dev *hdev) > +{ > + int ret; > > bt_dev_dbg(hdev, ""); > > @@ -3904,119 +4043,7 @@ int hci_dev_open_sync(struct hci_dev *hdev) > set_bit(HCI_RUNNING, &hdev->flags); > hci_sock_dev_event(hdev, HCI_DEV_OPEN); > > - atomic_set(&hdev->cmd_cnt, 1); > - set_bit(HCI_INIT, &hdev->flags); > - > - if (hci_dev_test_flag(hdev, HCI_SETUP) || > - test_bit(HCI_QUIRK_NON_PERSISTENT_SETUP, &hdev->quirks)) { > - bool invalid_bdaddr; > - size_t i; > - > - hci_sock_dev_event(hdev, HCI_DEV_SETUP); > - > - if (hdev->setup) > - ret = hdev->setup(hdev); > - > - for (i = 0; i < ARRAY_SIZE(hci_broken_table); i++) { > - if (test_bit(hci_broken_table[i].quirk, &hdev->quirks)) > - bt_dev_warn(hdev, "%s", > - hci_broken_table[i].desc); > - } > - > - /* The transport driver can set the quirk to mark the > - * BD_ADDR invalid before creating the HCI device or in > - * its setup callback. > - */ > - invalid_bdaddr = test_bit(HCI_QUIRK_INVALID_BDADDR, > - &hdev->quirks); > - > - if (ret) > - goto setup_failed; > - > - if (test_bit(HCI_QUIRK_USE_BDADDR_PROPERTY, &hdev->quirks)) { > - if (!bacmp(&hdev->public_addr, BDADDR_ANY)) > - hci_dev_get_bd_addr_from_property(hdev); > - > - if (bacmp(&hdev->public_addr, BDADDR_ANY) && > - hdev->set_bdaddr) { > - ret = hdev->set_bdaddr(hdev, > - &hdev->public_addr); > - > - /* If setting of the BD_ADDR from the device > - * property succeeds, then treat the address > - * as valid even if the invalid BD_ADDR > - * quirk indicates otherwise. > - */ > - if (!ret) > - invalid_bdaddr = false; > - } > - } > - > -setup_failed: > - /* The transport driver can set these quirks before > - * creating the HCI device or in its setup callback. > - * > - * For the invalid BD_ADDR quirk it is possible that > - * it becomes a valid address if the bootloader does > - * provide it (see above). > - * > - * In case any of them is set, the controller has to > - * start up as unconfigured. > - */ > - if (test_bit(HCI_QUIRK_EXTERNAL_CONFIG, &hdev->quirks) || > - invalid_bdaddr) > - hci_dev_set_flag(hdev, HCI_UNCONFIGURED); > - > - /* For an unconfigured controller it is required to > - * read at least the version information provided by > - * the Read Local Version Information command. > - * > - * If the set_bdaddr driver callback is provided, then > - * also the original Bluetooth public device address > - * will be read using the Read BD Address command. > - */ > - if (hci_dev_test_flag(hdev, HCI_UNCONFIGURED)) > - ret = hci_unconf_init_sync(hdev); > - } > - > - if (hci_dev_test_flag(hdev, HCI_CONFIG)) { > - /* If public address change is configured, ensure that > - * the address gets programmed. If the driver does not > - * support changing the public address, fail the power > - * on procedure. > - */ > - if (bacmp(&hdev->public_addr, BDADDR_ANY) && > - hdev->set_bdaddr) > - ret = hdev->set_bdaddr(hdev, &hdev->public_addr); > - else > - ret = -EADDRNOTAVAIL; > - } > - > - if (!ret) { > - if (!hci_dev_test_flag(hdev, HCI_UNCONFIGURED) && > - !hci_dev_test_flag(hdev, HCI_USER_CHANNEL)) { > - ret = hci_init_sync(hdev); > - if (!ret && hdev->post_init) > - ret = hdev->post_init(hdev); > - } > - } > - > - /* If the HCI Reset command is clearing all diagnostic settings, > - * then they need to be reprogrammed after the init procedure > - * completed. > - */ > - if (test_bit(HCI_QUIRK_NON_PERSISTENT_DIAG, &hdev->quirks) && > - !hci_dev_test_flag(hdev, HCI_USER_CHANNEL) && > - hci_dev_test_flag(hdev, HCI_VENDOR_DIAG) && hdev->set_diag) > - ret = hdev->set_diag(hdev, true); > - > - if (!hci_dev_test_flag(hdev, HCI_USER_CHANNEL)) { > - msft_do_open(hdev); > - aosp_do_open(hdev); > - } > - > - clear_bit(HCI_INIT, &hdev->flags); > - > + ret = hci_dev_init_sync(hdev); > if (!ret) { > hci_dev_hold(hdev); > hci_dev_set_flag(hdev, HCI_RPA_EXPIRED); > -- > 2.35.1 Any feedback on these changes?
diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c index 5610ec1242d6..2d3b9adbd215 100644 --- a/net/bluetooth/hci_sync.c +++ b/net/bluetooth/hci_sync.c @@ -3849,9 +3849,148 @@ static const struct { "advertised, but not supported.") }; -int hci_dev_open_sync(struct hci_dev *hdev) +/* This function handles hdev setup stage: + * + * Calls hdev->setup + * Setup address if HCI_QUIRK_USE_BDADDR_PROPERTY is set. + */ +static int hci_dev_setup_sync(struct hci_dev *hdev) { int ret = 0; + bool invalid_bdaddr; + size_t i; + + if (!hci_dev_test_flag(hdev, HCI_SETUP) && + !test_bit(HCI_QUIRK_NON_PERSISTENT_SETUP, &hdev->quirks)) + return 0; + + bt_dev_dbg(hdev, ""); + + hci_sock_dev_event(hdev, HCI_DEV_SETUP); + + if (hdev->setup) + ret = hdev->setup(hdev); + + for (i = 0; i < ARRAY_SIZE(hci_broken_table); i++) { + if (test_bit(hci_broken_table[i].quirk, &hdev->quirks)) + bt_dev_warn(hdev, "%s", hci_broken_table[i].desc); + } + + /* The transport driver can set the quirk to mark the + * BD_ADDR invalid before creating the HCI device or in + * its setup callback. + */ + invalid_bdaddr = test_bit(HCI_QUIRK_INVALID_BDADDR, &hdev->quirks); + + if (!ret) { + if (test_bit(HCI_QUIRK_USE_BDADDR_PROPERTY, &hdev->quirks)) { + if (!bacmp(&hdev->public_addr, BDADDR_ANY)) + hci_dev_get_bd_addr_from_property(hdev); + + if (bacmp(&hdev->public_addr, BDADDR_ANY) && + hdev->set_bdaddr) { + ret = hdev->set_bdaddr(hdev, + &hdev->public_addr); + + /* If setting of the BD_ADDR from the device + * property succeeds, then treat the address + * as valid even if the invalid BD_ADDR + * quirk indicates otherwise. + */ + if (!ret) + invalid_bdaddr = false; + } + } + } + + /* The transport driver can set these quirks before + * creating the HCI device or in its setup callback. + * + * For the invalid BD_ADDR quirk it is possible that + * it becomes a valid address if the bootloader does + * provide it (see above). + * + * In case any of them is set, the controller has to + * start up as unconfigured. + */ + if (test_bit(HCI_QUIRK_EXTERNAL_CONFIG, &hdev->quirks) || + invalid_bdaddr) + hci_dev_set_flag(hdev, HCI_UNCONFIGURED); + + /* For an unconfigured controller it is required to + * read at least the version information provided by + * the Read Local Version Information command. + * + * If the set_bdaddr driver callback is provided, then + * also the original Bluetooth public device address + * will be read using the Read BD Address command. + */ + if (hci_dev_test_flag(hdev, HCI_UNCONFIGURED)) + return hci_unconf_init_sync(hdev); + + return ret; +} + +/* This function handles hdev init stage: + * + * Calls hci_dev_setup_sync to perform setup stage + * Calls hci_init_sync to perform HCI command init sequence + */ +static int hci_dev_init_sync(struct hci_dev *hdev) +{ + int ret; + + bt_dev_dbg(hdev, ""); + + atomic_set(&hdev->cmd_cnt, 1); + set_bit(HCI_INIT, &hdev->flags); + + ret = hci_dev_setup_sync(hdev); + + if (hci_dev_test_flag(hdev, HCI_CONFIG)) { + /* If public address change is configured, ensure that + * the address gets programmed. If the driver does not + * support changing the public address, fail the power + * on procedure. + */ + if (bacmp(&hdev->public_addr, BDADDR_ANY) && + hdev->set_bdaddr) + ret = hdev->set_bdaddr(hdev, &hdev->public_addr); + else + ret = -EADDRNOTAVAIL; + } + + if (!ret) { + if (!hci_dev_test_flag(hdev, HCI_UNCONFIGURED) && + !hci_dev_test_flag(hdev, HCI_USER_CHANNEL)) { + ret = hci_init_sync(hdev); + if (!ret && hdev->post_init) + ret = hdev->post_init(hdev); + } + } + + /* If the HCI Reset command is clearing all diagnostic settings, + * then they need to be reprogrammed after the init procedure + * completed. + */ + if (test_bit(HCI_QUIRK_NON_PERSISTENT_DIAG, &hdev->quirks) && + !hci_dev_test_flag(hdev, HCI_USER_CHANNEL) && + hci_dev_test_flag(hdev, HCI_VENDOR_DIAG) && hdev->set_diag) + ret = hdev->set_diag(hdev, true); + + if (!hci_dev_test_flag(hdev, HCI_USER_CHANNEL)) { + msft_do_open(hdev); + aosp_do_open(hdev); + } + + clear_bit(HCI_INIT, &hdev->flags); + + return ret; +} + +int hci_dev_open_sync(struct hci_dev *hdev) +{ + int ret; bt_dev_dbg(hdev, ""); @@ -3904,119 +4043,7 @@ int hci_dev_open_sync(struct hci_dev *hdev) set_bit(HCI_RUNNING, &hdev->flags); hci_sock_dev_event(hdev, HCI_DEV_OPEN); - atomic_set(&hdev->cmd_cnt, 1); - set_bit(HCI_INIT, &hdev->flags); - - if (hci_dev_test_flag(hdev, HCI_SETUP) || - test_bit(HCI_QUIRK_NON_PERSISTENT_SETUP, &hdev->quirks)) { - bool invalid_bdaddr; - size_t i; - - hci_sock_dev_event(hdev, HCI_DEV_SETUP); - - if (hdev->setup) - ret = hdev->setup(hdev); - - for (i = 0; i < ARRAY_SIZE(hci_broken_table); i++) { - if (test_bit(hci_broken_table[i].quirk, &hdev->quirks)) - bt_dev_warn(hdev, "%s", - hci_broken_table[i].desc); - } - - /* The transport driver can set the quirk to mark the - * BD_ADDR invalid before creating the HCI device or in - * its setup callback. - */ - invalid_bdaddr = test_bit(HCI_QUIRK_INVALID_BDADDR, - &hdev->quirks); - - if (ret) - goto setup_failed; - - if (test_bit(HCI_QUIRK_USE_BDADDR_PROPERTY, &hdev->quirks)) { - if (!bacmp(&hdev->public_addr, BDADDR_ANY)) - hci_dev_get_bd_addr_from_property(hdev); - - if (bacmp(&hdev->public_addr, BDADDR_ANY) && - hdev->set_bdaddr) { - ret = hdev->set_bdaddr(hdev, - &hdev->public_addr); - - /* If setting of the BD_ADDR from the device - * property succeeds, then treat the address - * as valid even if the invalid BD_ADDR - * quirk indicates otherwise. - */ - if (!ret) - invalid_bdaddr = false; - } - } - -setup_failed: - /* The transport driver can set these quirks before - * creating the HCI device or in its setup callback. - * - * For the invalid BD_ADDR quirk it is possible that - * it becomes a valid address if the bootloader does - * provide it (see above). - * - * In case any of them is set, the controller has to - * start up as unconfigured. - */ - if (test_bit(HCI_QUIRK_EXTERNAL_CONFIG, &hdev->quirks) || - invalid_bdaddr) - hci_dev_set_flag(hdev, HCI_UNCONFIGURED); - - /* For an unconfigured controller it is required to - * read at least the version information provided by - * the Read Local Version Information command. - * - * If the set_bdaddr driver callback is provided, then - * also the original Bluetooth public device address - * will be read using the Read BD Address command. - */ - if (hci_dev_test_flag(hdev, HCI_UNCONFIGURED)) - ret = hci_unconf_init_sync(hdev); - } - - if (hci_dev_test_flag(hdev, HCI_CONFIG)) { - /* If public address change is configured, ensure that - * the address gets programmed. If the driver does not - * support changing the public address, fail the power - * on procedure. - */ - if (bacmp(&hdev->public_addr, BDADDR_ANY) && - hdev->set_bdaddr) - ret = hdev->set_bdaddr(hdev, &hdev->public_addr); - else - ret = -EADDRNOTAVAIL; - } - - if (!ret) { - if (!hci_dev_test_flag(hdev, HCI_UNCONFIGURED) && - !hci_dev_test_flag(hdev, HCI_USER_CHANNEL)) { - ret = hci_init_sync(hdev); - if (!ret && hdev->post_init) - ret = hdev->post_init(hdev); - } - } - - /* If the HCI Reset command is clearing all diagnostic settings, - * then they need to be reprogrammed after the init procedure - * completed. - */ - if (test_bit(HCI_QUIRK_NON_PERSISTENT_DIAG, &hdev->quirks) && - !hci_dev_test_flag(hdev, HCI_USER_CHANNEL) && - hci_dev_test_flag(hdev, HCI_VENDOR_DIAG) && hdev->set_diag) - ret = hdev->set_diag(hdev, true); - - if (!hci_dev_test_flag(hdev, HCI_USER_CHANNEL)) { - msft_do_open(hdev); - aosp_do_open(hdev); - } - - clear_bit(HCI_INIT, &hdev->flags); - + ret = hci_dev_init_sync(hdev); if (!ret) { hci_dev_hold(hdev); hci_dev_set_flag(hdev, HCI_RPA_EXPIRED);