From: Luiz Augusto von Dentz <[email protected]>
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 <[email protected]>
---
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
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
<[email protected]> wrote:
>
> From: Luiz Augusto von Dentz <[email protected]>
>
> 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 <[email protected]>
> ---
> 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?
--
Luiz Augusto von Dentz