Skip to contentSkip to conversation

Bluetooth: HCI: Add HCI_QUIRK_BROKEN_ENHANCED_SETUP_SYNC_CONN quirk

PATCH Superseded Luiz Augusto von Dentz 4 participants 8 messages Mar 29, 2022 – Mar 30, 2022

Subsystems (2)
Bluetooth drivers Bluetooth

v2 of 4 Superseded Latest is v4 →

lore
Share & tools
lore

Luiz Augusto von Dentz

author maintainer
#0 lore raw

PATCH v2 1/3 Bluetooth: HCI: Add HCI_QUIRK_BROKEN_ENHANCED_SETUP_SYNC_CONN quirk

From: Luiz Augusto von Dentz <[email protected]>

This adds HCI_QUIRK_BROKEN_ENHANCED_SETUP_SYNC_CONN quirk which can be used to mark HCI_Enhanced_Setup_Synchronous_Connection as broken even if its support command bit are set since some controller report it as supported but the command don't work properly with some configurations (e.g. BT_VOICE_TRANSPARENT/mSBC).

Signed-off-by Luiz Augusto von Dentz <[email protected]>

---
v2: Change hci_broken/hci_quirk_broken and HCI_BROKEN/HCI_QUIRK_BROKEN and add patch 3/3 setting HCI_QUIRK_BROKEN_ENHANCED_SETUP_SYNC_CONN for QCA controllers.

 include/net/bluetooth/hci.h      | 9 +++++++++
 include/net/bluetooth/hci_core.h | 8 ++++++--
 net/bluetooth/hci_conn.c         | 2 +-
 net/bluetooth/sco.c              | 2 +-
 4 files changed, 17 insertions(+), 4 deletions(-)
include/net/bluetooth/hci.h +9 -0
index 5cb095b09a94..8bb81ea4d286 100644
@@ -265,6 +265,15 @@ enum {
 	 * runtime suspend, because event filtering takes place there.
 	 */
 	HCI_QUIRK_BROKEN_FILTER_CLEAR_ALL,
+
+	/*
+	 * When this quirk is set, disables the use of
+	 * HCI_OP_ENHANCED_SETUP_SYNC_CONN command to setup SCO connections.
+	 *
+	 * This quirk can be set before hci_register_dev is called or
+	 * during the hdev->setup vendor callback.
+	 */
+	HCI_QUIRK_BROKEN_ENHANCED_SETUP_SYNC_CONN,
 };
 
 /* HCI device flags */
include/net/bluetooth/hci_core.h +6 -2
index d5377740e99c..59815df1272a 100644
@@ -1492,8 +1492,12 @@ void hci_conn_del_sysfs(struct hci_conn *conn);
 #define privacy_mode_capable(dev) (use_ll_privacy(dev) && \
 				   (hdev->commands[39] & 0x04))
 
-/* Use enhanced synchronous connection if command is supported */
-#define enhanced_sco_capable(dev) ((dev)->commands[29] & 0x08)
+/* Use enhanced synchronous connection if command is supported and its quirk
+ * has not been set.
+ */
+#define enhanced_sync_conn_capable(dev) \
+	(((dev)->commands[29] & 0x08) && \
+	 !test_bit(HCI_QUIRK_BROKEN_ENHANCED_SETUP_SYNC_CONN, &(dev)->quirks))
 
 /* Use ext scanning if set ext scan param and ext scan enable is supported */
 #define use_ext_scan(dev) (((dev)->commands[37] & 0x20) && \
net/bluetooth/hci_conn.c +1 -1
index 84312c836549..cd51bf2a709b 100644
@@ -481,7 +481,7 @@ static bool hci_setup_sync_conn(struct hci_conn *conn, __u16 handle)
 
 bool hci_setup_sync(struct hci_conn *conn, __u16 handle)
 {
-	if (enhanced_sco_capable(conn->hdev))
+	if (enhanced_sync_conn_capable(conn->hdev))
 		return hci_enhanced_setup_sync_conn(conn, handle);
 
 	return hci_setup_sync_conn(conn, handle);
net/bluetooth/sco.c +1 -1
index 8eabf41b2993..2a58c7d88433 100644
@@ -885,7 +885,7 @@ static int sco_sock_setsockopt(struct socket *sock, int level, int optname,
 			err = -EBADFD;
 			break;
 		}
-		if (enhanced_sco_capable(hdev) &&
+		if (enhanced_sync_conn_capable(hdev) &&
 		    voice.setting == BT_VOICE_TRANSPARENT)
 			sco_pi(sk)->codec.id = BT_CODEC_TRANSPARENT;
 		hci_dev_put(hdev);
Signature · 1 line
2.35.1
author maintainer
#1 lore raw

PATCH v2 2/3 Bluetooth: Print broken quirks

From: Luiz Augusto von Dentz <[email protected]>

This prints warnings for controllers setting broken quirks to increase their visibility and warn about broken controllers firmware that probably needs updates to behave properly.

Signed-off-by Luiz Augusto von Dentz <[email protected]>
---
 net/bluetooth/hci_sync.c | 66 +++++++++++++++++++++++++++++++++++++---
 1 file changed, 62 insertions(+), 4 deletions(-)
net/bluetooth/hci_sync.c +62 -4
index 8f4c5698913d..8994ff1f94e6 100644
@@ -3825,6 +3825,67 @@ static int hci_init_sync(struct hci_dev *hdev)
 	return 0;
 }
 
+#define HCI_QUIRK_BROKEN(_quirk, _desc) \
+{ \
+	.quirk = _quirk, \
+	.desc = _desc, \
+}
+
+static const struct hci_quirk_broken {
+	unsigned long quirk;
+	const char *desc;
+} hci_broken_table[] = {
+	HCI_QUIRK_BROKEN(HCI_QUIRK_BROKEN_LOCAL_COMMANDS,
+			 "HCI Read Local Supported Commands not supported"),
+	HCI_QUIRK_BROKEN(HCI_QUIRK_BROKEN_STORED_LINK_KEY,
+			 "HCI Delete Stored Link Key command is advertised, "
+			 "but not supported."),
+	HCI_QUIRK_BROKEN(HCI_QUIRK_BROKEN_ERR_DATA_REPORTING,
+			 "HCI Read Default Erroneous Data Reporting command is "
+			 "advertised, but not supported."),
+	HCI_QUIRK_BROKEN(HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER,
+			 "HCI Read Transmit Power Level command is advertised, "
+			 "but not supported."),
+	HCI_QUIRK_BROKEN(HCI_QUIRK_BROKEN_FILTER_CLEAR_ALL,
+			 "HCI Set Event Filter command not supported."),
+	HCI_QUIRK_BROKEN(HCI_QUIRK_BROKEN_ENHANCED_SETUP_SYNC_CONN,
+			 "HCI Enhanced Setup Synchronous Connection command is "
+			 "advertised, but not supported.")
+};
+
+static void hci_dev_print_broken_quirks(struct hci_dev *hdev)
+{
+	int i;
+
+	bt_dev_dbg(hdev, "");
+
+	for (i = 0; i < ARRAY_SIZE(hci_broken_table); i++) {
+		const struct hci_quirk_broken *broken = &hci_broken_table[i];
+
+		if (test_bit(broken->quirk, &hdev->quirks))
+			bt_dev_warn(hdev, "%s", broken->desc);
+	}
+}
+
+static int hci_dev_setup_sync(struct hci_dev *hdev)
+{
+	bt_dev_dbg(hdev, "");
+
+	hci_sock_dev_event(hdev, HCI_DEV_SETUP);
+
+	if (hdev->setup) {
+		int ret;
+
+		ret = hdev->setup(hdev);
+		if (ret)
+			return ret;
+
+		hci_dev_print_broken_quirks(hdev);
+	}
+
+	return 0;
+}
+
 int hci_dev_open_sync(struct hci_dev *hdev)
 {
 	int ret = 0;

@@ -3887,10 +3948,7 @@ int hci_dev_open_sync(struct hci_dev *hdev)
 	    test_bit(HCI_QUIRK_NON_PERSISTENT_SETUP, &hdev->quirks)) {
 		bool invalid_bdaddr;
 
-		hci_sock_dev_event(hdev, HCI_DEV_SETUP);
-
-		if (hdev->setup)
-			ret = hdev->setup(hdev);
+		ret = hci_dev_setup_sync(hdev);
 
 		/* The transport driver can set the quirk to mark the
 		 * BD_ADDR invalid before creating the HCI device or in
Signature · 1 line
2.35.1
#2 lore raw

Re: [PATCH v2 2/3] Bluetooth: Print broken quirks

Dear Luiz,

Thank you for your patch.

Maybe as commit message summary:

Warn about broken quirks

Am 29.03.22 um 22:16 schrieb Luiz Augusto von Dentz:

Quoted 21 lines

From: Luiz Augusto von Dentz <[email protected]>

This prints warnings for controllers setting broken quirks to increase their visibility and warn about broken controllers firmware that probably needs updates to behave properly.

Signed-off-by Luiz Augusto von Dentz <[email protected]>
---
  net/bluetooth/hci_sync.c | 66 +++++++++++++++++++++++++++++++++++++---
  1 file changed, 62 insertions(+), 4 deletions(-)
net/bluetooth/hci_sync.c +2 -0
index 8f4c5698913d..8994ff1f94e6 100644
@@ -3825,6 +3825,67 @@ static int hci_init_sync(struct hci_dev *hdev)
  	return 0;
  }
 
+#define HCI_QUIRK_BROKEN(_quirk, _desc) \
+{ \

net/bluetooth/hci_sync.c static int hci_init_sync(struct hci_dev *hdev) · line 3839

Quoted 10 lines
net/bluetooth/hci_sync.c +10 -0
+	.quirk = _quirk, \
+	.desc = _desc, \
+}
+
+static const struct hci_quirk_broken {
+	unsigned long quirk;
+	const char *desc;
+} hci_broken_table[] = {
+	HCI_QUIRK_BROKEN(HCI_QUIRK_BROKEN_LOCAL_COMMANDS,
+			 "HCI Read Local Supported Commands not supported"),

The user wouldn’t know, that this is a device firmware problem. Could something be added to the warning, like:

Therefore, device firmware VERSION violates the spec. Please contact the manufacturer.

Quoted 9 lines
net/bluetooth/hci_sync.c +9 -0
+	HCI_QUIRK_BROKEN(HCI_QUIRK_BROKEN_STORED_LINK_KEY,
+			 "HCI Delete Stored Link Key command is advertised, "
+			 "but not supported."),
+	HCI_QUIRK_BROKEN(HCI_QUIRK_BROKEN_ERR_DATA_REPORTING,
+			 "HCI Read Default Erroneous Data Reporting command is "
+			 "advertised, but not supported."),
+	HCI_QUIRK_BROKEN(HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER,
+			 "HCI Read Transmit Power Level command is advertised, "
+			 "but not supported."),

net/bluetooth/hci_sync.c static int hci_init_sync(struct hci_dev *hdev) · line 3858

Quoted 10 lines
net/bluetooth/hci_sync.c +10 -0
+	HCI_QUIRK_BROKEN(HCI_QUIRK_BROKEN_FILTER_CLEAR_ALL,
+			 "HCI Set Event Filter command not supported."),
+	HCI_QUIRK_BROKEN(HCI_QUIRK_BROKEN_ENHANCED_SETUP_SYNC_CONN,
+			 "HCI Enhanced Setup Synchronous Connection command is "
+			 "advertised, but not supported.")
+};
+
+static void hci_dev_print_broken_quirks(struct hci_dev *hdev)
+{
+	int i;

I’d use unsigned int, but no idea, what the subsystem does regarding counting variables.

Kind regards,

Paul

Quoted 45 lines
net/bluetooth/hci_sync.c +31 -4
+
+	bt_dev_dbg(hdev, "");
+
+	for (i = 0; i < ARRAY_SIZE(hci_broken_table); i++) {
+		const struct hci_quirk_broken *broken = &hci_broken_table[i];
+
+		if (test_bit(broken->quirk, &hdev->quirks))
+			bt_dev_warn(hdev, "%s", broken->desc);
+	}
+}
+
+static int hci_dev_setup_sync(struct hci_dev *hdev)
+{
+	bt_dev_dbg(hdev, "");
+
+	hci_sock_dev_event(hdev, HCI_DEV_SETUP);
+
+	if (hdev->setup) {
+		int ret;
+
+		ret = hdev->setup(hdev);
+		if (ret)
+			return ret;
+
+		hci_dev_print_broken_quirks(hdev);
+	}
+
+	return 0;
+}
+
  int hci_dev_open_sync(struct hci_dev *hdev)
  {
  	int ret = 0;

@@ -3887,10 +3948,7 @@ int hci_dev_open_sync(struct hci_dev *hdev)
  	    test_bit(HCI_QUIRK_NON_PERSISTENT_SETUP, &hdev->quirks)) {
  		bool invalid_bdaddr;
 
-		hci_sock_dev_event(hdev, HCI_DEV_SETUP);
-
-		if (hdev->setup)
-			ret = hdev->setup(hdev);
+		ret = hci_dev_setup_sync(hdev);
 
  		/* The transport driver can set the quirk to mark the
  		 * BD_ADDR invalid before creating the HCI device or in
author maintainer
#3 lore raw

Re: [PATCH v2 2/3] Bluetooth: Print broken quirks

Hi Paul,

Quoted 46 lines · Paul Menzel

Dear Luiz,

Thank you for your patch.

Maybe as commit message summary:

Warn about broken quirks

Am 29.03.22 um 22:16 schrieb Luiz Augusto von Dentz:

Quoted 21 lines

From: Luiz Augusto von Dentz <[email protected]>

This prints warnings for controllers setting broken quirks to increase their visibility and warn about broken controllers firmware that probably needs updates to behave properly.

Signed-off-by Luiz Augusto von Dentz <[email protected]>
---
  net/bluetooth/hci_sync.c | 66 +++++++++++++++++++++++++++++++++++++---
  1 file changed, 62 insertions(+), 4 deletions(-)
net/bluetooth/hci_sync.c +2 -0
index 8f4c5698913d..8994ff1f94e6 100644
@@ -3825,6 +3825,67 @@ static int hci_init_sync(struct hci_dev *hdev)
      return 0;
  }
 
+#define HCI_QUIRK_BROKEN(_quirk, _desc) \
+{ \

net/bluetooth/hci_sync.c static int hci_init_sync(struct hci_dev *hdev) · line 3839

Quoted 10 lines
net/bluetooth/hci_sync.c +10 -0
+     .quirk = _quirk, \
+     .desc = _desc, \
+}
+
+static const struct hci_quirk_broken {
+     unsigned long quirk;
+     const char *desc;
+} hci_broken_table[] = {
+     HCI_QUIRK_BROKEN(HCI_QUIRK_BROKEN_LOCAL_COMMANDS,
+                      "HCI Read Local Supported Commands not supported"),

The user wouldn’t know, that this is a device firmware problem. Could something be added to the warning, like:

Quoted 2 lines

Therefore, device firmware VERSION violates the spec. Please contact the manufacturer.

We do print out the firmware filename when loading it so I'm not convinced printing it again would make any difference for the user.

Quoted 14 lines
Quoted 14 lines
net/bluetooth/hci_sync.c +14 -0
+     HCI_QUIRK_BROKEN(HCI_QUIRK_BROKEN_STORED_LINK_KEY,
+                      "HCI Delete Stored Link Key command is advertised, "
+                      "but not supported."),
+     HCI_QUIRK_BROKEN(HCI_QUIRK_BROKEN_ERR_DATA_REPORTING,
+                      "HCI Read Default Erroneous Data Reporting command is "
+                      "advertised, but not supported."),
+     HCI_QUIRK_BROKEN(HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER,
+                      "HCI Read Transmit Power Level command is advertised, "
+                      "but not supported."),
+     HCI_QUIRK_BROKEN(HCI_QUIRK_BROKEN_FILTER_CLEAR_ALL,
+                      "HCI Set Event Filter command not supported."),
+     HCI_QUIRK_BROKEN(HCI_QUIRK_BROKEN_ENHANCED_SETUP_SYNC_CONN,
+                      "HCI Enhanced Setup Synchronous Connection command is "
+                      "advertised, but not supported.")
Quoted 8 lines

net/bluetooth/hci_sync.c static int hci_init_sync(struct hci_dev *hdev) · line 3858

Quoted 5 lines
net/bluetooth/hci_sync.c +5 -0
+};
+
+static void hci_dev_print_broken_quirks(struct hci_dev *hdev)
+{
+     int i;

I’d use unsigned int, but no idea, what the subsystem does regarding counting variables.

I suppose size_t would be better given that sizeof is used.

Quoted 51 lines

Kind regards,

Paul

Quoted 45 lines
net/bluetooth/hci_sync.c +31 -4
+
+     bt_dev_dbg(hdev, "");
+
+     for (i = 0; i < ARRAY_SIZE(hci_broken_table); i++) {
+             const struct hci_quirk_broken *broken = &hci_broken_table[i];
+
+             if (test_bit(broken->quirk, &hdev->quirks))
+                     bt_dev_warn(hdev, "%s", broken->desc);
+     }
+}
+
+static int hci_dev_setup_sync(struct hci_dev *hdev)
+{
+     bt_dev_dbg(hdev, "");
+
+     hci_sock_dev_event(hdev, HCI_DEV_SETUP);
+
+     if (hdev->setup) {
+             int ret;
+
+             ret = hdev->setup(hdev);
+             if (ret)
+                     return ret;
+
+             hci_dev_print_broken_quirks(hdev);
+     }
+
+     return 0;
+}
+
  int hci_dev_open_sync(struct hci_dev *hdev)
  {
      int ret = 0;

@@ -3887,10 +3948,7 @@ int hci_dev_open_sync(struct hci_dev *hdev)
          test_bit(HCI_QUIRK_NON_PERSISTENT_SETUP, &hdev->quirks)) {
              bool invalid_bdaddr;
 
-             hci_sock_dev_event(hdev, HCI_DEV_SETUP);
-
-             if (hdev->setup)
-                     ret = hdev->setup(hdev);
+             ret = hci_dev_setup_sync(hdev);
 
              /* The transport driver can set the quirk to mark the
               * BD_ADDR invalid before creating the HCI device or in
Signature · 1 line
Luiz Augusto von Dentz
maintainer
#4 lore raw

Re: [PATCH v2 2/3] Bluetooth: Print broken quirks

In reply to Luiz Augusto von Dentz

Hi Luiz,

Quoted 14 lines

This prints warnings for controllers setting broken quirks to increase their visibility and warn about broken controllers firmware that probably needs updates to behave properly.

Signed-off-by Luiz Augusto von Dentz <[email protected]>

---
net/bluetooth/hci_sync.c | 66 +++++++++++++++++++++++++++++++++++++---

1 file changed, 62 insertions(+), 4 deletions(-)
net/bluetooth/hci_sync.c +0 -0
index 8f4c5698913d..8994ff1f94e6 100644

net/bluetooth/hci_sync.c static int hci_init_sync(struct hci_dev *hdev) · line 3833

Quoted 9 lines
net/bluetooth/hci_sync.c +6 -0
@@ -3825,6 +3825,67 @@ static int hci_init_sync(struct hci_dev *hdev)
 	return 0;
 }
 
+#define HCI_QUIRK_BROKEN(_quirk, _desc) \
+{ \
+	.quirk = _quirk, \
+	.desc = _desc, \
+}
+

#define HCI_QUIRK_BROKEN(_quirk, _desc) { .quirk = _quirk, .desc = _desc }

Doesn’t this fit into a single line?

net/bluetooth/hci_sync.c static int hci_init_sync(struct hci_dev *hdev) · line 3836

Quoted 3 lines
net/bluetooth/hci_sync.c +3 -0
+static const struct hci_quirk_broken {
+	unsigned long quirk;
+	const char *desc;

I am not sure if we better use an anonymous table here.

static const struct {
	..
} hci_broken_table[] = {
	..
};

net/bluetooth/hci_sync.c static int hci_init_sync(struct hci_dev *hdev) · line 3839

Quoted 3 lines
net/bluetooth/hci_sync.c +3 -0
+} hci_broken_table[] = {
+	HCI_QUIRK_BROKEN(HCI_QUIRK_BROKEN_LOCAL_COMMANDS,
+			 "HCI Read Local Supported Commands not supported"),

I am after this:

HCI_QUIRK_BROKEN(LOCAL_COMMANDS, “HCI Read Local Supported ..”),

Quoted 18 lines
net/bluetooth/hci_sync.c +18 -0
+	HCI_QUIRK_BROKEN(HCI_QUIRK_BROKEN_STORED_LINK_KEY,
+			 "HCI Delete Stored Link Key command is advertised, "
+			 "but not supported."),
+	HCI_QUIRK_BROKEN(HCI_QUIRK_BROKEN_ERR_DATA_REPORTING,
+			 "HCI Read Default Erroneous Data Reporting command is "
+			 "advertised, but not supported."),
+	HCI_QUIRK_BROKEN(HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER,
+			 "HCI Read Transmit Power Level command is advertised, "
+			 "but not supported."),
+	HCI_QUIRK_BROKEN(HCI_QUIRK_BROKEN_FILTER_CLEAR_ALL,
+			 "HCI Set Event Filter command not supported."),
+	HCI_QUIRK_BROKEN(HCI_QUIRK_BROKEN_ENHANCED_SETUP_SYNC_CONN,
+			 "HCI Enhanced Setup Synchronous Connection command is "
+			 "advertised, but not supported.")
+};
+
+static void hci_dev_print_broken_quirks(struct hci_dev *hdev)
+{

net/bluetooth/hci_sync.c static int hci_init_sync(struct hci_dev *hdev) · line 3867

Quoted 10 lines
net/bluetooth/hci_sync.c +10 -0
+	int i;
+
+	bt_dev_dbg(hdev, "");
+
+	for (i = 0; i < ARRAY_SIZE(hci_broken_table); i++) {
+		const struct hci_quirk_broken *broken = &hci_broken_table[i];
+
+		if (test_bit(broken->quirk, &hdev->quirks))
+			bt_dev_warn(hdev, "%s", broken->desc);
+	}

	for (i = 0; .. ) }
		if (!test_bit(hci_broken_table[i], &hdev->quirks))
			continue;

		bt_dev_warn(hdev, “%s”, hci_broken_table[i].desc);
	}
Quoted 25 lines
net/bluetooth/hci_sync.c +21 -0
+}
+
+static int hci_dev_setup_sync(struct hci_dev *hdev)
+{
+	bt_dev_dbg(hdev, "");
+
+	hci_sock_dev_event(hdev, HCI_DEV_SETUP);
+
+	if (hdev->setup) {
+		int ret;
+
+		ret = hdev->setup(hdev);
+		if (ret)
+			return ret;
+
+		hci_dev_print_broken_quirks(hdev);
+	}
+
+	return 0;
+}
+
 int hci_dev_open_sync(struct hci_dev *hdev)
 {
 	int ret = 0;

net/bluetooth/hci_sync.c int hci_dev_open_sync(struct hci_dev *hdev) · line 3951

Quoted 8 lines
net/bluetooth/hci_sync.c +1 -4
@@ -3887,10 +3948,7 @@ int hci_dev_open_sync(struct hci_dev *hdev)
 	    test_bit(HCI_QUIRK_NON_PERSISTENT_SETUP, &hdev->quirks)) {
 		bool invalid_bdaddr;
 
-		hci_sock_dev_event(hdev, HCI_DEV_SETUP);
-
-		if (hdev->setup)
-			ret = hdev->setup(hdev);
+		ret = hci_dev_setup_sync(hdev);

Please don’t create spaghetti code. Just add the print of the quirks here.

In addition you also need to print the quirks even if no hdev->setup is defined. Quirks can also be set on probe() without the need of a vendor specific setup function.

net/bluetooth/hci_sync.c int hci_dev_open_sync(struct hci_dev *hdev) · line 3954

Quoted 3 lines
net/bluetooth/hci_sync.c +0 -0
 		/* The transport driver can set the quirk to mark the
 		 * BD_ADDR invalid before creating the HCI device or in

Regards

Marcel

author maintainer
#5 lore raw

PATCH v2 3/3 Bluetooth: btusb: Set HCI_QUIRK_BROKEN_ENHANCED_SETUP_SYNC_CONN for QCA

In reply to Luiz Augusto von Dentz

From: Luiz Augusto von Dentz <[email protected]>

This sets HCI_QUIRK_BROKEN_ENHANCED_SETUP_SYNC_CONN for QCA controllers since SCO appear to not work when using HCI_OP_ENHANCED_SETUP_SYNC_CONN.

Link https://bugzilla.kernel.org/show_bug.cgi?id=215576
Signed-off-by Luiz Augusto von Dentz <[email protected]>
---
 drivers/bluetooth/btusb.c | 3 +++
 1 file changed, 3 insertions(+)
drivers/bluetooth/btusb.c +3 -0
index 50df417207af..2470c3d4ef0f 100644
@@ -3335,6 +3335,9 @@ static int btusb_setup_qca(struct hci_dev *hdev)
 			msleep(QCA_BT_RESET_WAIT_MS);
 	}
 
+	/* https://bugzilla.kernel.org/show_bug.cgi?id=215576 */
+	set_bit(HCI_QUIRK_BROKEN_ENHANCED_SETUP_SYNC_CONN, &hdev->quirks);
+
 	return 0;
 }
Signature · 1 line
2.35.1
author maintainer
#6 lore raw

Re: [PATCH v2 3/3] Bluetooth: btusb: Set HCI_QUIRK_BROKEN_ENHANCED_SETUP_SYNC_CONN for QCA

Hi,

Quoted 27 lines · Luiz Augusto von Dentz

From: Luiz Augusto von Dentz <[email protected]>

This sets HCI_QUIRK_BROKEN_ENHANCED_SETUP_SYNC_CONN for QCA controllers since SCO appear to not work when using HCI_OP_ENHANCED_SETUP_SYNC_CONN.

Link https://bugzilla.kernel.org/show_bug.cgi?id=215576
Signed-off-by Luiz Augusto von Dentz <[email protected]>
---
 drivers/bluetooth/btusb.c | 3 +++
 1 file changed, 3 insertions(+)
drivers/bluetooth/btusb.c +3 -1
index 50df417207af..2470c3d4ef0f 100644
@@ -3335,6 +3335,9 @@ static int btusb_setup_qca(struct hci_dev *hdev)
                        msleep(QCA_BT_RESET_WAIT_MS);
        }
 
+       /* https://bugzilla.kernel.org/show_bug.cgi?id=215576 */
+       set_bit(HCI_QUIRK_BROKEN_ENHANCED_SETUP_SYNC_CONN, &hdev->quirks);
+
        return 0;
 }
 
--
Quoted 1 line

2.35.1

Im CCs you guys since you have been involved with Bluetooth QCA drivers, the patch above marks all QCA controller as broken with respect to use of HCI Enhanced Setup Sync Connection, if you guys have know that it only affect specific models it would probably be a good idea to introduce a table with the affected models or better yet have their firmware fixed.

Signature · 1 line
Luiz Augusto von Dentz
#7 lore raw
In reply to Luiz Augusto von Dentz

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=627309

---Test result---

Test Summary:

CheckPatch                    FAIL      2.56 seconds
GitLint                       PASS      1.51 seconds
SubjectPrefix                 PASS      1.00 seconds
BuildKernel                   PASS      42.17 seconds
BuildKernel32                 PASS      37.86 seconds
Incremental Build with patchesPASS      66.78 seconds
TestRunner: Setup             PASS      637.22 seconds
TestRunner: l2cap-tester      PASS      19.72 seconds
TestRunner: bnep-tester       PASS      8.15 seconds
TestRunner: mgmt-tester       PASS      133.25 seconds
TestRunner: rfcomm-tester     PASS      10.46 seconds
TestRunner: sco-tester        PASS      10.34 seconds
TestRunner: smp-tester        PASS      10.30 seconds
TestRunner: userchan-tester   PASS      8.39 seconds

Details
##############################
Test: CheckPatch - FAIL - 2.56 seconds
Run checkpatch.pl script with rule in .checkpatch.conf
[v2,2/3] Bluetooth: Print broken quirks\WARNING:SPLIT_STRING: quoted string split across lines #113: FILE: net/bluetooth/hci_sync.c:3842:
+ "HCI Delete Stored Link Key command is advertised, "
+ "but not supported."),

WARNING:SPLIT_STRING: quoted string split across lines #116: FILE: net/bluetooth/hci_sync.c:3845:
+ "HCI Read Default Erroneous Data Reporting command is "
+ "advertised, but not supported."),

WARNING:SPLIT_STRING: quoted string split across lines #119: FILE: net/bluetooth/hci_sync.c:3848:
+ "HCI Read Transmit Power Level command is advertised, "
+ "but not supported."),

WARNING:SPLIT_STRING: quoted string split across lines #124: FILE: net/bluetooth/hci_sync.c:3853:
+ "HCI Enhanced Setup Synchronous Connection command is "
+ "advertised, but not supported.")

total: 0 errors, 4 warnings, 0 checks, 78 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.

/github/workspace/src/12795219.patch has style problems, please review.

NOTE: Ignored message types: UNKNOWN_COMMIT_ID

NOTE: If any of the errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.

---
Regards,
Linux Bluetooth