diff mbox series

[v4,1/4] Bluetooth: HCI: Add HCI_QUIRK_BROKEN_ENHANCED_SETUP_SYNC_CONN quirk

Message ID 20220401233826.122544-1-luiz.dentz@gmail.com (mailing list archive)
State Accepted
Commit c625b467f47cb5c88c61741c43d89d842d9246a4
Headers show
Series [v4,1/4] Bluetooth: HCI: Add HCI_QUIRK_BROKEN_ENHANCED_SETUP_SYNC_CONN quirk | expand

Checks

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

Commit Message

Luiz Augusto von Dentz April 1, 2022, 11:38 p.m. UTC 
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

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 <luiz.von.dentz@intel.com>
---
v4: Address latest comments, add patch 4/4 spliting hci_dev_open_sync so each
stage can be handled by its own function.
v3: Make HCI_QUIRK_BROKEN macro expand quirk name and address other comments
from v2.
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(-)

Comments

bluez.test.bot@gmail.com April 2, 2022, 12:01 a.m. UTC | #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=628357

---Test result---

Test Summary:
CheckPatch                    FAIL      3.82 seconds
GitLint                       PASS      4.00 seconds
SubjectPrefix                 PASS      3.54 seconds
BuildKernel                   PASS      30.56 seconds
BuildKernel32                 PASS      27.22 seconds
Incremental Build with patchesPASS      57.55 seconds
TestRunner: Setup             PASS      453.98 seconds
TestRunner: l2cap-tester      PASS      14.95 seconds
TestRunner: bnep-tester       PASS      5.76 seconds
TestRunner: mgmt-tester       PASS      95.91 seconds
TestRunner: rfcomm-tester     PASS      7.36 seconds
TestRunner: sco-tester        PASS      7.14 seconds
TestRunner: smp-tester        PASS      7.14 seconds
TestRunner: userchan-tester   PASS      5.90 seconds

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

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

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

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

total: 0 errors, 4 warnings, 0 checks, 49 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/12798857.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
patchwork-bot+bluetooth@kernel.org April 5, 2022, 5:30 p.m. UTC | #2 
Hello:

This series was applied to bluetooth/bluetooth-next.git (master)
by Marcel Holtmann <marcel@holtmann.org>:

On Fri,  1 Apr 2022 16:38:23 -0700 you wrote:
> From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> 
> 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).
> 
> [...]

Here is the summary with links:
  - [v4,1/4] Bluetooth: HCI: Add HCI_QUIRK_BROKEN_ENHANCED_SETUP_SYNC_CONN quirk
    https://git.kernel.org/bluetooth/bluetooth-next/c/c625b467f47c
  - [v4,2/4] Bluetooth: Print broken quirks
    https://git.kernel.org/bluetooth/bluetooth-next/c/fa5cd0fd5bdf
  - [v4,3/4] Bluetooth: btusb: Set HCI_QUIRK_BROKEN_ENHANCED_SETUP_SYNC_CONN for QCA
    https://git.kernel.org/bluetooth/bluetooth-next/c/38a1944deda4
  - [v4,4/4] Bluetooth: hci_sync: Split hci_dev_open_sync
    (no matching commit)

You are awesome, thank you!
Thorsten Leemhuis April 10, 2022, 9:05 a.m. UTC | #3 
Hi, this is your Linux kernel regression tracker.

On 05.04.22 19:30, patchwork-bot+bluetooth@kernel.org wrote:
> 
> This series was applied to bluetooth/bluetooth-next.git (master)
> by Marcel Holtmann <marcel@holtmann.org>:
> 
> On Fri,  1 Apr 2022 16:38:23 -0700 you wrote:
>> From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

BTW: Thx for taking care of this, Luiz!

>> 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).
>>
>> [...]
> 
> Here is the summary with links:
>   - [v4,1/4] Bluetooth: HCI: Add HCI_QUIRK_BROKEN_ENHANCED_SETUP_SYNC_CONN quirk
>     https://git.kernel.org/bluetooth/bluetooth-next/c/c625b467f47c
>   - [v4,2/4] Bluetooth: Print broken quirks
>     https://git.kernel.org/bluetooth/bluetooth-next/c/fa5cd0fd5bdf
>   - [v4,3/4] Bluetooth: btusb: Set HCI_QUIRK_BROKEN_ENHANCED_SETUP_SYNC_CONN for QCA
>     https://git.kernel.org/bluetooth/bluetooth-next/c/38a1944deda4
>   - [v4,4/4] Bluetooth: hci_sync: Split hci_dev_open_sync
>     (no matching commit)
> 
> You are awesome, thank you!

Hey, this patchset is fixing a regression from 5.16 (see patch 3/4), but
apparently is only scheduled to go upstream in the next cycle, as it was
applied to bluetooth-next.git. The patches also seem to miss tags to get
them backported to stable. Is that intentional? At least patches 1, 2,
and 3 don't look that complex, why can't they get in this cycle and get
backported to stable soon to finally provide a solution for users that
are plagued by this for weeks already?

Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)

P.S.: As the Linux kernel's regression tracker I'm getting a lot of
reports on my table. I can only look briefly into most of them and lack
knowledge about most of the areas they concern. I thus unfortunately
will sometimes get things wrong or miss something important. I hope
that's not the case here; if you think it is, don't hesitate to tell me
in a public reply, it's in everyone's interest to set the public record
straight.
diff mbox series

Patch

diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index 5cb095b09a94..8bb81ea4d286 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -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 */
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index d5377740e99c..59815df1272a 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -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) && \
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index 84312c836549..cd51bf2a709b 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -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);
diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
index 8eabf41b2993..2a58c7d88433 100644
--- a/net/bluetooth/sco.c
+++ b/net/bluetooth/sco.c
@@ -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);