| Message ID | d43dffdc43a40782ec6d5d6c24b1638005992a8f.camel@iki.fi (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Bluetooth: btusb: check SCO MTU before enabling USB ALT 3 for WBS | expand |
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=516813
---Test result---
Test Summary:
CheckPatch FAIL 0.57 seconds
GitLint PASS 0.12 seconds
BuildKernel PASS 613.39 seconds
TestRunner: Setup PASS 431.82 seconds
TestRunner: l2cap-tester PASS 3.11 seconds
TestRunner: bnep-tester PASS 2.17 seconds
TestRunner: mgmt-tester PASS 34.16 seconds
TestRunner: rfcomm-tester PASS 2.43 seconds
TestRunner: sco-tester PASS 2.34 seconds
TestRunner: smp-tester FAIL 2.48 seconds
TestRunner: userchan-tester PASS 2.28 seconds
Details
##############################
Test: CheckPatch - FAIL - 0.57 seconds
Run checkpatch.pl script with rule in .checkpatch.conf
Bluetooth: btusb: check SCO MTU before enabling USB ALT 3 for WBS
WARNING: Unknown commit id 'e848dbd364ac', maybe rebased or not pulled?
#8:
commit e848dbd364ac ("Bluetooth: btusb: Add support USB ALT 3 for WBS")
total: 0 errors, 1 warnings, 13 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 SCO MTU before enabling USB ALT 3 for" 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.12 seconds
Run gitlint with rule in .gitlint
##############################
Test: BuildKernel - PASS - 613.39 seconds
Build Kernel with minimal configuration supports Bluetooth
##############################
Test: TestRunner: Setup - PASS - 431.82 seconds
Setup environment for running Test Runner
##############################
Test: TestRunner: l2cap-tester - PASS - 3.11 seconds
Run test-runner with l2cap-tester
Total: 40, Passed: 40 (100.0%), Failed: 0, Not Run: 0
##############################
Test: TestRunner: bnep-tester - PASS - 2.17 seconds
Run test-runner with bnep-tester
Total: 1, Passed: 1 (100.0%), Failed: 0, Not Run: 0
##############################
Test: TestRunner: mgmt-tester - PASS - 34.16 seconds
Run test-runner with mgmt-tester
Total: 446, Passed: 443 (99.3%), Failed: 0, Not Run: 3
##############################
Test: TestRunner: rfcomm-tester - PASS - 2.43 seconds
Run test-runner with rfcomm-tester
Total: 9, Passed: 9 (100.0%), Failed: 0, Not Run: 0
##############################
Test: TestRunner: sco-tester - PASS - 2.34 seconds
Run test-runner with sco-tester
Total: 8, Passed: 8 (100.0%), Failed: 0, Not Run: 0
##############################
Test: TestRunner: smp-tester - FAIL - 2.48 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.028 seconds
##############################
Test: TestRunner: userchan-tester - PASS - 2.28 seconds
Run test-runner with userchan-tester
Total: 3, Passed: 3 (100.0%), Failed: 0, Not Run: 0
---
Regards,
Linux Bluetooth
It looks reasonable to check sco_mtu. My suggestion is to use hci_packet_size_usb_alt[3] instead of the magic number 72. Thanks, Joseph On Sat, Jul 17, 2021 at 1:58 AM Pauli Virtanen <pav@iki.fi> 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. Check that the MTU > condition is satisfied, and fall back to ALT 1 if not. > > Tested with USB adapters (mtu<72, produce sound only with ALT1) > BCM20702A1 0b05:17cb, CSR8510A10 0a12:0001, and (mtu>=72, ALT3) > RTL8761BU 0bda:8771, Intel AX200 8087:0029 (after disabling ALT6). > > Signed-off-by: Pauli Virtanen <pav@iki.fi> > --- > drivers/bluetooth/btusb.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c > index a9855a2dd561..3ee66e415c4d 100644 > --- a/drivers/bluetooth/btusb.c > +++ b/drivers/bluetooth/btusb.c > @@ -1763,9 +1763,11 @@ static void btusb_work(struct work_struct *work) > /* 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. > + * data satisfy 60 bytes. USB Alt 3 support also needs > + * HFP transparent MTU >= 72 Bytes. > */ > - if (new_alts == 1 && btusb_find_altsetting(data, 3)) > + if (new_alts == 1 && hdev->sco_mtu >= 72 && > + btusb_find_altsetting(data, 3)) > new_alts = 3; > } > > -- > 2.31.1 > >
Hi Pauli, On Fri, Jul 16, 2021 at 08:58:17PM +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. Check that the MTU > condition is satisfied, and fall back to ALT 1 if not. > > Tested with USB adapters (mtu<72, produce sound only with ALT1) > BCM20702A1 0b05:17cb, CSR8510A10 0a12:0001, and (mtu>=72, ALT3) > RTL8761BU 0bda:8771, Intel AX200 8087:0029 (after disabling ALT6). > > Signed-off-by: Pauli Virtanen <pav@iki.fi> > --- > Maybe add a Fixes: tag ? Thanks, -- Pasi > drivers/bluetooth/btusb.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c > index a9855a2dd561..3ee66e415c4d 100644 > --- a/drivers/bluetooth/btusb.c > +++ b/drivers/bluetooth/btusb.c > @@ -1763,9 +1763,11 @@ static void btusb_work(struct work_struct *work) > /* 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. > + * data satisfy 60 bytes. USB Alt 3 support also needs > + * HFP transparent MTU >= 72 Bytes. > */ > - if (new_alts == 1 && btusb_find_altsetting(data, 3)) > + if (new_alts == 1 && hdev->sco_mtu >= 72 && > + btusb_find_altsetting(data, 3)) > new_alts = 3; > } > > -- > 2.31.1 > >
Hi, la, 2021-07-17 kello 21:34 +0800, Joseph Hwang kirjoitti: > It looks reasonable to check sco_mtu. My suggestion is to use > hci_packet_size_usb_alt[3] instead of the magic number 72. Thanks for looking at it. IIUC, hci_packet_size_usb_alt only appears in previous discussions on this list, which did not seem to reach a conclusion if this should be computed from wMaxPacketSize. I can do less magical v2 and add the table. Different solution would be to make the altsetting choice manufacturer/device specific flag, as I'm not sure on what hardware e848dbd364ac was tested on, so maybe it is intended Realtek specific. Best, Pauli > On Sat, Jul 17, 2021 at 1:58 AM Pauli Virtanen <pav@iki.fi> 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. Check that the MTU > > condition is satisfied, and fall back to ALT 1 if not. > > > > Tested with USB adapters (mtu<72, produce sound only with ALT1) > > BCM20702A1 0b05:17cb, CSR8510A10 0a12:0001, and (mtu>=72, ALT3) > > RTL8761BU 0bda:8771, Intel AX200 8087:0029 (after disabling ALT6). > > > > Signed-off-by: Pauli Virtanen <pav@iki.fi> > > --- > > drivers/bluetooth/btusb.c | 6 ++++-- > > 1 file changed, 4 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c > > index a9855a2dd561..3ee66e415c4d 100644 > > --- a/drivers/bluetooth/btusb.c > > +++ b/drivers/bluetooth/btusb.c > > @@ -1763,9 +1763,11 @@ static void btusb_work(struct work_struct *work) > > /* 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. > > + * data satisfy 60 bytes. USB Alt 3 support also needs > > + * HFP transparent MTU >= 72 Bytes. > > */ > > - if (new_alts == 1 && btusb_find_altsetting(data, 3)) > > + if (new_alts == 1 && hdev->sco_mtu >= 72 && > > + btusb_find_altsetting(data, 3)) > > new_alts = 3; > > } > > > > -- > > 2.31.1 > > > > > >
Hi, Thanks for your supplement. Add the sco_mtu condition is better. When USB BT adapters support the alternate 3, it will announce alternate 3. However, USB Alt 3 supported also need HFP support transparent MTU in 72 Bytes. Add this sco_mtu condition could avoid HFP profile transparent MTU not match or not suitable ALT 3. Regards, Hilda -----Original Message----- From: Pauli Virtanen <pav@iki.fi> Sent: Wednesday, July 21, 2021 11:40 PM To: Joseph Hwang <josephsih@google.com> Cc: linux-bluetooth <linux-bluetooth@vger.kernel.org>; Hilda Wu <hildawu@realtek.com> Subject: Re: [PATCH] Bluetooth: btusb: check SCO MTU before enabling USB ALT 3 for WBS Hi, la, 2021-07-17 kello 21:34 +0800, Joseph Hwang kirjoitti: > It looks reasonable to check sco_mtu. My suggestion is to use > hci_packet_size_usb_alt[3] instead of the magic number 72. Thanks for looking at it. IIUC, hci_packet_size_usb_alt only appears in previous discussions on this list, which did not seem to reach a conclusion if this should be computed from wMaxPacketSize. I can do less magical v2 and add the table. Different solution would be to make the altsetting choice manufacturer/device specific flag, as I'm not sure on what hardware e848dbd364ac was tested on, so maybe it is intended Realtek specific. Best, Pauli > On Sat, Jul 17, 2021 at 1:58 AM Pauli Virtanen <pav@iki.fi> 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. Check > > that the MTU condition is satisfied, and fall back to ALT 1 if not. > > > > Tested with USB adapters (mtu<72, produce sound only with ALT1) > > BCM20702A1 0b05:17cb, CSR8510A10 0a12:0001, and (mtu>=72, ALT3) > > RTL8761BU 0bda:8771, Intel AX200 8087:0029 (after disabling ALT6). > > > > Signed-off-by: Pauli Virtanen <pav@iki.fi> > > --- > > drivers/bluetooth/btusb.c | 6 ++++-- > > 1 file changed, 4 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c > > index a9855a2dd561..3ee66e415c4d 100644 > > --- a/drivers/bluetooth/btusb.c > > +++ b/drivers/bluetooth/btusb.c > > @@ -1763,9 +1763,11 @@ static void btusb_work(struct work_struct *work) > > /* 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. > > + * data satisfy 60 bytes. USB Alt 3 support also needs > > + * HFP transparent MTU >= 72 Bytes. > > */ > > - if (new_alts == 1 && btusb_find_altsetting(data, 3)) > > + if (new_alts == 1 && hdev->sco_mtu >= 72 && > > + btusb_find_altsetting(data, 3)) > > new_alts = 3; > > } > > > > -- > > 2.31.1 > > > > > > ------Please consider the environment before printing this e-mail.
Hi Pauli, Thank you for your work on this patch! > 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. Check that the MTU > condition is satisfied, and fall back to ALT 1 if not. > > Tested with USB adapters (mtu<72, produce sound only with ALT1) > BCM20702A1 0b05:17cb, CSR8510A10 0a12:0001, and (mtu>=72, ALT3) > RTL8761BU 0bda:8771, Intel AX200 8087:0029 (after disabling ALT6). Commit e848dbd364aca44c9d23c04bef964fab79e2b34f causes garbled mSBC audio on my Bluetooth adapter (8087:0a2b), so I gave your patch a try. Unfortunately, the patch in its current shape does not seem to be enough to fix things for this adapter, which reports an SCO MTU of 96 bytes and produces garbled mSBC audio with ALT 3. For the record, reverting e848dbd364aca44c9d23c04bef964fab79e2b34f on top of v5.14-rc2 makes mSBC work again with my adapter. I am no Bluetooth expert, but I will be happy to perform any additional debugging and/or testing.
Hi Michał, to, 2021-07-22 kello 15:29 +0200, Michał Kępień kirjoitti: > Thank you for your work on this patch! > > > 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. Check that the MTU > > condition is satisfied, and fall back to ALT 1 if not. > > > > Tested with USB adapters (mtu<72, produce sound only with ALT1) > > BCM20702A1 0b05:17cb, CSR8510A10 0a12:0001, and (mtu>=72, ALT3) > > RTL8761BU 0bda:8771, Intel AX200 8087:0029 (after disabling ALT6). > > Commit e848dbd364aca44c9d23c04bef964fab79e2b34f causes garbled mSBC > audio on my Bluetooth adapter (8087:0a2b), so I gave your patch a try. > > Unfortunately, the patch in its current shape does not seem to be enough > to fix things for this adapter, which reports an SCO MTU of 96 bytes and > produces garbled mSBC audio with ALT 3. > > For the record, reverting e848dbd364aca44c9d23c04bef964fab79e2b34f on > top of v5.14-rc2 makes mSBC work again with my adapter. > > I am no Bluetooth expert, but I will be happy to perform any additional > debugging and/or testing. Thanks for testing! What userspace software did you use? Pulseaudio at least I think had assumption that mtu <= 60, which breaks here (Pipewire works, don't know about bluez-alsa). If otherwise, maybe it then needs also the manufacturer flag. Best, Pauli
Pauli, > Thanks for testing! What userspace software did you use? Pulseaudio at > least I think had assumption that mtu <= 60, which breaks here > (Pipewire works, don't know about bluez-alsa). If otherwise, maybe it > then needs also the manufacturer flag. I am using Pipewire 0.3.32. When I wrote "this adapter (...) reports an SCO MTU of 96 bytes" in my previous message, I meant that this was the value present in hdev->sco_mtu around the problematic code location. If this was the wrong thing to take a look at, please let me know.
diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c index a9855a2dd561..3ee66e415c4d 100644 --- a/drivers/bluetooth/btusb.c +++ b/drivers/bluetooth/btusb.c @@ -1763,9 +1763,11 @@ static void btusb_work(struct work_struct *work) /* 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. + * data satisfy 60 bytes. USB Alt 3 support also needs + * HFP transparent MTU >= 72 Bytes. */ - if (new_alts == 1 && btusb_find_altsetting(data, 3)) + if (new_alts == 1 && hdev->sco_mtu >= 72 && + btusb_find_altsetting(data, 3)) new_alts = 3; }
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. Check that the MTU condition is satisfied, and fall back to ALT 1 if not. Tested with USB adapters (mtu<72, produce sound only with ALT1) BCM20702A1 0b05:17cb, CSR8510A10 0a12:0001, and (mtu>=72, ALT3) RTL8761BU 0bda:8771, Intel AX200 8087:0029 (after disabling ALT6). Signed-off-by: Pauli Virtanen <pav@iki.fi> --- drivers/bluetooth/btusb.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)