diff mbox series

[v3] Bluetooth: btusb: check conditions before enabling USB ALT 3 for WBS

Message ID 20210726180206.49703-1-pav@iki.fi (mailing list archive)
State Accepted
Delegated to: Luiz Von Dentz
Headers show
Series [v3] Bluetooth: btusb: check conditions before enabling USB ALT 3 for WBS | expand

Commit Message

Pauli Virtanen July 26, 2021, 6:02 p.m. UTC
Some USB BT adapters don't satisfy the MTU requirement mentioned in
commit e848dbd364ac ("Bluetooth: btusb: Add support USB ALT 3 for WBS")
and have ALT 3 setting that produces no/garbled audio. Some adapters
with larger MTU were also reported to have problems with ALT 3.

Add a flag and check it and MTU before selecting ALT 3, falling back to
ALT 1. Enable the flag for Realtek, restoring the previous behavior for
non-Realtek devices.

Tested with USB adapters (mtu<72, no/garbled sound with ALT3, ALT1
works) BCM20702A1 0b05:17cb, CSR8510A10 0a12:0001, and (mtu>=72, ALT3
works) RTL8761BU 0bda:8771, Intel AX200 8087:0029 (after disabling
ALT6). Also got reports for (mtu>=72, ALT 3 reported to produce bad
audio) Intel 8087:0a2b.

Signed-off-by: Pauli Virtanen <pav@iki.fi>
Fixes: e848dbd364ac ("Bluetooth: btusb: Add support USB ALT 3 for WBS")
Tested-by: Michał Kępień <kernel@kempniu.pl>
---

Changes in v3:
- Rename flag to BTUSB_USE_ALT3_FOR_WBS.
- No spaces in indent.
- Added Tested-by: Michał Kępień

Changes in v2:
- Explain magic number 72 in a comment; didn't add the table for them,
  because it's not used elsewhere and we need just one number from it.
- Add flag for ALT3 support, restoring the behavior
  for non-Realtek devices the same as before e848dbd364ac, due to
  the problems reported on an Intel adapter. Don't have the device
  myself.

 drivers/bluetooth/btusb.c | 22 ++++++++++++++--------
 1 file changed, 14 insertions(+), 8 deletions(-)

Comments

bluez.test.bot@gmail.com July 27, 2021, 2:11 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=521451

---Test result---

Test Summary:
CheckPatch                    FAIL      0.51 seconds
GitLint                       PASS      0.11 seconds
BuildKernel                   PASS      553.49 seconds
TestRunner: Setup             PASS      361.85 seconds
TestRunner: l2cap-tester      PASS      2.61 seconds
TestRunner: bnep-tester       PASS      2.01 seconds
TestRunner: mgmt-tester       PASS      31.71 seconds
TestRunner: rfcomm-tester     PASS      2.19 seconds
TestRunner: sco-tester        PASS      2.16 seconds
TestRunner: smp-tester        FAIL      2.18 seconds
TestRunner: userchan-tester   PASS      2.01 seconds

Details
##############################
Test: CheckPatch - FAIL - 0.51 seconds
Run checkpatch.pl script with rule in .checkpatch.conf
Bluetooth: btusb: check conditions before enabling USB ALT 3 for WBS
WARNING: Unknown commit id 'e848dbd364ac', maybe rebased or not pulled?
#11: 
commit e848dbd364ac ("Bluetooth: btusb: Add support USB ALT 3 for WBS")

total: 0 errors, 1 warnings, 42 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: btusb: check conditions before enabling USB ALT 3" 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.11 seconds
Run gitlint with rule in .gitlint


##############################
Test: BuildKernel - PASS - 553.49 seconds
Build Kernel with minimal configuration supports Bluetooth


##############################
Test: TestRunner: Setup - PASS - 361.85 seconds
Setup environment for running Test Runner


##############################
Test: TestRunner: l2cap-tester - PASS - 2.61 seconds
Run test-runner with l2cap-tester
Total: 40, Passed: 40 (100.0%), Failed: 0, Not Run: 0

##############################
Test: TestRunner: bnep-tester - PASS - 2.01 seconds
Run test-runner with bnep-tester
Total: 1, Passed: 1 (100.0%), Failed: 0, Not Run: 0

##############################
Test: TestRunner: mgmt-tester - PASS - 31.71 seconds
Run test-runner with mgmt-tester
Total: 448, Passed: 445 (99.3%), Failed: 0, Not Run: 3

##############################
Test: TestRunner: rfcomm-tester - PASS - 2.19 seconds
Run test-runner with rfcomm-tester
Total: 9, Passed: 9 (100.0%), Failed: 0, Not Run: 0

##############################
Test: TestRunner: sco-tester - PASS - 2.16 seconds
Run test-runner with sco-tester
Total: 8, Passed: 8 (100.0%), Failed: 0, Not Run: 0

##############################
Test: TestRunner: smp-tester - FAIL - 2.18 seconds
Run test-runner with smp-tester
Total: 8, Passed: 7 (87.5%), Failed: 1, Not Run: 0

Failed Test Cases
SMP Client - SC Request 2                            Failed       0.023 seconds

##############################
Test: TestRunner: userchan-tester - PASS - 2.01 seconds
Run test-runner with userchan-tester
Total: 3, Passed: 3 (100.0%), Failed: 0, Not Run: 0



---
Regards,
Linux Bluetooth
Pasi Kärkkäinen July 30, 2021, 12:35 p.m. UTC | #2
Hi,

On Mon, Jul 26, 2021 at 09:02:06PM +0300, Pauli Virtanen wrote:
> Some USB BT adapters don't satisfy the MTU requirement mentioned in
> commit e848dbd364ac ("Bluetooth: btusb: Add support USB ALT 3 for WBS")
> and have ALT 3 setting that produces no/garbled audio. Some adapters
> with larger MTU were also reported to have problems with ALT 3.
> 
> Add a flag and check it and MTU before selecting ALT 3, falling back to
> ALT 1. Enable the flag for Realtek, restoring the previous behavior for
> non-Realtek devices.
> 
> Tested with USB adapters (mtu<72, no/garbled sound with ALT3, ALT1
> works) BCM20702A1 0b05:17cb, CSR8510A10 0a12:0001, and (mtu>=72, ALT3
> works) RTL8761BU 0bda:8771, Intel AX200 8087:0029 (after disabling
> ALT6). Also got reports for (mtu>=72, ALT 3 reported to produce bad
> audio) Intel 8087:0a2b.
> 
> Signed-off-by: Pauli Virtanen <pav@iki.fi>
> Fixes: e848dbd364ac ("Bluetooth: btusb: Add support USB ALT 3 for WBS")
> Tested-by: Michał Kępień <kernel@kempniu.pl>
> ---
>

This probably also should have CC stable@kernel.org, as users have
started reporting this bug as distros have started shipping kernels with
the faulty patch in it.. so it'd be nice to have the fix backported to
stable kernel trees.


Thanks,

-- Pasi

> 
> Changes in v3:
> - Rename flag to BTUSB_USE_ALT3_FOR_WBS.
> - No spaces in indent.
> - Added Tested-by: Michał Kępień
> 
> Changes in v2:
> - Explain magic number 72 in a comment; didn't add the table for them,
>   because it's not used elsewhere and we need just one number from it.
> - Add flag for ALT3 support, restoring the behavior
>   for non-Realtek devices the same as before e848dbd364ac, due to
>   the problems reported on an Intel adapter. Don't have the device
>   myself.
>
diff mbox series

Patch

diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index 6d23308119d1..e8062d0b7d4d 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -516,6 +516,7 @@  static const struct dmi_system_id btusb_needs_reset_resume_table[] = {
 #define BTUSB_HW_RESET_ACTIVE	12
 #define BTUSB_TX_WAIT_VND_EVT	13
 #define BTUSB_WAKEUP_DISABLE	14
+#define BTUSB_USE_ALT3_FOR_WBS	15
 
 struct btusb_data {
 	struct hci_dev       *hdev;
@@ -1748,16 +1749,20 @@  static void btusb_work(struct work_struct *work)
 			/* Bluetooth USB spec recommends alt 6 (63 bytes), but
 			 * many adapters do not support it.  Alt 1 appears to
 			 * work for all adapters that do not have alt 6, and
-			 * which work with WBS at all.
+			 * which work with WBS at all.  Some devices prefer
+			 * alt 3 (HCI payload >= 60 Bytes let air packet
+			 * data satisfy 60 bytes), requiring
+			 * MTU >= 3 (packets) * 25 (size) - 3 (headers) = 72
+			 * see also Core spec 5, vol 4, B 2.1.1 & Table 2.1.
 			 */
-			new_alts = btusb_find_altsetting(data, 6) ? 6 : 1;
-			/* Because mSBC frames do not need to be aligned to the
-			 * SCO packet boundary. If support the Alt 3, use the
-			 * Alt 3 for HCI payload >= 60 Bytes let air packet
-			 * data satisfy 60 bytes.
-			 */
-			if (new_alts == 1 && btusb_find_altsetting(data, 3))
+			if (btusb_find_altsetting(data, 6))
+				new_alts = 6;
+			else if (test_bit(BTUSB_USE_ALT3_FOR_WBS, &data->flags) &&
+					hdev->sco_mtu >= 72 &&
+					btusb_find_altsetting(data, 3))
 				new_alts = 3;
+			else
+				new_alts = 1;
 		}
 
 		if (btusb_switch_alt_setting(hdev, new_alts) < 0)
@@ -4733,6 +4738,7 @@  static int btusb_probe(struct usb_interface *intf,
 		 * (DEVICE_REMOTE_WAKEUP)
 		 */
 		set_bit(BTUSB_WAKEUP_DISABLE, &data->flags);
+		set_bit(BTUSB_USE_ALT3_FOR_WBS, &data->flags);
 	}
 
 	if (!reset)