* RE: Bluetooth: btusb: Make the CSR clone chip force-suspend workaround more generic
2021-07-16 23:21 [PATCH] Bluetooth: btusb: Make the CSR clone chip force-suspend workaround more generic Ismael Ferreras Morezuelas
@ 2021-07-17 0:05 ` bluez.test.bot
2021-07-17 9:31 ` [PATCH] " Hans de Goede
2021-07-29 19:55 ` Marcel Holtmann
2 siblings, 0 replies; 5+ messages in thread
From: bluez.test.bot @ 2021-07-17 0:05 UTC (permalink / raw)
To: linux-bluetooth, swyterzone
[-- Attachment #1: Type: text/plain, Size: 4768 bytes --]
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=516901
---Test result---
Test Summary:
CheckPatch FAIL 0.57 seconds
GitLint FAIL 0.13 seconds
BuildKernel PASS 606.18 seconds
TestRunner: Setup PASS 400.30 seconds
TestRunner: l2cap-tester PASS 2.80 seconds
TestRunner: bnep-tester PASS 2.11 seconds
TestRunner: mgmt-tester PASS 30.91 seconds
TestRunner: rfcomm-tester PASS 2.34 seconds
TestRunner: sco-tester PASS 2.35 seconds
TestRunner: smp-tester FAIL 2.32 seconds
TestRunner: userchan-tester PASS 2.15 seconds
Details
##############################
Test: CheckPatch - FAIL - 0.57 seconds
Run checkpatch.pl script with rule in .checkpatch.conf
Bluetooth: btusb: Make the CSR clone chip force-suspend workaround more generic
WARNING: Unknown commit id '81cac64ba258a', maybe rebased or not pulled?
#21:
Fixes: 81cac64ba258a ("Bluetooth: Deal with USB devices that are faking CSR vendor")
WARNING: Unknown commit id 'cde1a8a992875', maybe rebased or not pulled?
#22:
Fixes: cde1a8a992875 ("Bluetooth: btusb: Fix and detect most of the Chinese Bluetooth controllers")
WARNING: Unknown commit id 'd74e0ae7e0303', maybe rebased or not pulled?
#23:
Fixes: d74e0ae7e0303 ("Bluetooth: btusb: Fix detection of some fake CSR controllers with a bcdDevice val of 0x0134")
WARNING: Unknown commit id '0671c0662383e', maybe rebased or not pulled?
#24:
Fixes: 0671c0662383e ("Bluetooth: btusb: Add workaround for remote-wakeup issues with Barrot 8041a02 fake CSR controllers")
total: 0 errors, 4 warnings, 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.
"[PATCH] Bluetooth: btusb: Make the CSR clone chip force-suspend" 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 - FAIL - 0.13 seconds
Run gitlint with rule in .gitlint
Bluetooth: btusb: Make the CSR clone chip force-suspend workaround more generic
1: T1 Title exceeds max length (79>72): "Bluetooth: btusb: Make the CSR clone chip force-suspend workaround more generic"
17: B1 Line exceeds max length (84>80): "Fixes: 81cac64ba258a ("Bluetooth: Deal with USB devices that are faking CSR vendor")"
18: B1 Line exceeds max length (99>80): "Fixes: cde1a8a992875 ("Bluetooth: btusb: Fix and detect most of the Chinese Bluetooth controllers")"
19: B1 Line exceeds max length (116>80): "Fixes: d74e0ae7e0303 ("Bluetooth: btusb: Fix detection of some fake CSR controllers with a bcdDevice val of 0x0134")"
20: B1 Line exceeds max length (123>80): "Fixes: 0671c0662383e ("Bluetooth: btusb: Add workaround for remote-wakeup issues with Barrot 8041a02 fake CSR controllers")"
##############################
Test: BuildKernel - PASS - 606.18 seconds
Build Kernel with minimal configuration supports Bluetooth
##############################
Test: TestRunner: Setup - PASS - 400.30 seconds
Setup environment for running Test Runner
##############################
Test: TestRunner: l2cap-tester - PASS - 2.80 seconds
Run test-runner with l2cap-tester
Total: 40, Passed: 40 (100.0%), Failed: 0, Not Run: 0
##############################
Test: TestRunner: bnep-tester - PASS - 2.11 seconds
Run test-runner with bnep-tester
Total: 1, Passed: 1 (100.0%), Failed: 0, Not Run: 0
##############################
Test: TestRunner: mgmt-tester - PASS - 30.91 seconds
Run test-runner with mgmt-tester
Total: 446, Passed: 443 (99.3%), Failed: 0, Not Run: 3
##############################
Test: TestRunner: rfcomm-tester - PASS - 2.34 seconds
Run test-runner with rfcomm-tester
Total: 9, Passed: 9 (100.0%), Failed: 0, Not Run: 0
##############################
Test: TestRunner: sco-tester - PASS - 2.35 seconds
Run test-runner with sco-tester
Total: 8, Passed: 8 (100.0%), Failed: 0, Not Run: 0
##############################
Test: TestRunner: smp-tester - FAIL - 2.32 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.024 seconds
##############################
Test: TestRunner: userchan-tester - PASS - 2.15 seconds
Run test-runner with userchan-tester
Total: 3, Passed: 3 (100.0%), Failed: 0, Not Run: 0
---
Regards,
Linux Bluetooth
[-- Attachment #2: l2cap-tester.log --]
[-- Type: application/octet-stream, Size: 44350 bytes --]
[-- Attachment #3: bnep-tester.log --]
[-- Type: application/octet-stream, Size: 3557 bytes --]
[-- Attachment #4: mgmt-tester.log --]
[-- Type: application/octet-stream, Size: 614404 bytes --]
[-- Attachment #5: rfcomm-tester.log --]
[-- Type: application/octet-stream, Size: 11677 bytes --]
[-- Attachment #6: sco-tester.log --]
[-- Type: application/octet-stream, Size: 9912 bytes --]
[-- Attachment #7: smp-tester.log --]
[-- Type: application/octet-stream, Size: 11705 bytes --]
[-- Attachment #8: userchan-tester.log --]
[-- Type: application/octet-stream, Size: 5454 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Bluetooth: btusb: Make the CSR clone chip force-suspend workaround more generic
2021-07-16 23:21 [PATCH] Bluetooth: btusb: Make the CSR clone chip force-suspend workaround more generic Ismael Ferreras Morezuelas
2021-07-17 0:05 ` bluez.test.bot
@ 2021-07-17 9:31 ` Hans de Goede
2021-07-29 19:55 ` Marcel Holtmann
2 siblings, 0 replies; 5+ messages in thread
From: Hans de Goede @ 2021-07-17 9:31 UTC (permalink / raw)
To: Ismael Ferreras Morezuelas, Marcel Holtmann
Cc: Johan Hedberg, Luiz Augusto von Dentz, BlueZ, linux-kernel
Hi,
On 7/17/21 1:21 AM, Ismael Ferreras Morezuelas wrote:
> Turns out Hans de Goede completed the work I started last year trying to
> improve Chinese-clone detection of CSR controller chips. Quirk after quirk
> these Bluetooth dongles are more usable now.
>
> Even after a few BlueZ regressions; these clones are so fickle that some
> days they stop working altogether. Except on Windows, they work fine.
>
>
> But this force-suspend initialization quirk seems to mostly do the trick,
> after a lot of testing Bluetooth now seems to work *all* the time.
>
> The only problem is that the solution ended up being masked under a very
> stringent check; when there are probably hundreds of fake dongle
> models out there that benefit from a good reset. Make it so.
>
>
> Fixes: 81cac64ba258a ("Bluetooth: Deal with USB devices that are faking CSR vendor")
> Fixes: cde1a8a992875 ("Bluetooth: btusb: Fix and detect most of the Chinese Bluetooth controllers")
> Fixes: d74e0ae7e0303 ("Bluetooth: btusb: Fix detection of some fake CSR controllers with a bcdDevice val of 0x0134")
> Fixes: 0671c0662383e ("Bluetooth: btusb: Add workaround for remote-wakeup issues with Barrot 8041a02 fake CSR controllers")
>
> Cc: stable@vger.kernel.org
> Cc: Hans de Goede <hdegoede@redhat.com>
> Tested-by: Ismael Ferreras Morezuelas <swyterzone@gmail.com>
> Signed-off-by: Ismael Ferreras Morezuelas <swyterzone@gmail.com>
FWIW I'm fine with making the force-suspend once quirk which
I added generic to all clones.
The new code looks good to me:
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Regards,
Hans
> ---
>
> I've changed the warning line to make it easy to grep and detect if this updated
> workaround is part of the driver. Should make it much more obvious to users in
> case their dongle doesn't work for other reasons. There's a clear then-now.
>
> Easy to narrow other future issues down. Let me know what you think.
>
> drivers/bluetooth/btusb.c | 61 +++++++++++++++++++++------------------
> 1 file changed, 33 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> index a9855a2dd..197cafe75 100644
> --- a/drivers/bluetooth/btusb.c
> +++ b/drivers/bluetooth/btusb.c
> @@ -1890,7 +1890,7 @@ static int btusb_setup_csr(struct hci_dev *hdev)
> is_fake = true;
>
> if (is_fake) {
> - bt_dev_warn(hdev, "CSR: Unbranded CSR clone detected; adding workarounds...");
> + bt_dev_warn(hdev, "CSR: Unbranded CSR clone detected; adding workarounds and force-suspending once...");
>
> /* Generally these clones have big discrepancies between
> * advertised features and what's actually supported.
> @@ -1907,41 +1907,46 @@ static int btusb_setup_csr(struct hci_dev *hdev)
> clear_bit(HCI_QUIRK_SIMULTANEOUS_DISCOVERY, &hdev->quirks);
>
> /*
> - * Special workaround for clones with a Barrot 8041a02 chip,
> - * these clones are really messed-up:
> - * 1. Their bulk rx endpoint will never report any data unless
> - * the device was suspended at least once (yes really).
> + * Special workaround for these BT 4.0 chip clones, and potentially more:
> + *
> + * - 0x0134: a Barrot 8041a02 (HCI rev: 0x1012 sub: 0x0810)
> + * - 0x7558: IC markings FR3191AHAL 749H15143 (HCI rev/sub-version: 0x0709)
> + *
> + * These controllers are really messed-up.
> + *
> + * 1. Their bulk RX endpoint will never report any data unless
> + * the device was suspended at least once (yes, really).
> * 2. They will not wakeup when autosuspended and receiving data
> - * on their bulk rx endpoint from e.g. a keyboard or mouse
> + * on their bulk RX endpoint from e.g. a keyboard or mouse
> * (IOW remote-wakeup support is broken for the bulk endpoint).
> *
> * To fix 1. enable runtime-suspend, force-suspend the
> - * hci and then wake-it up by disabling runtime-suspend.
> + * HCI and then wake-it up by disabling runtime-suspend.
> *
> - * To fix 2. clear the hci's can_wake flag, this way the hci
> + * To fix 2. clear the HCI's can_wake flag, this way the HCI
> * will still be autosuspended when it is not open.
> + *
> + * --
> + *
> + * Because these are widespread problems we prefer generic solutions; so
> + * apply this initialization quirk to every controller that gets here,
> + * it should be harmless. The alternative is to not work at all.
> */
> - if (bcdDevice == 0x8891 &&
> - le16_to_cpu(rp->lmp_subver) == 0x1012 &&
> - le16_to_cpu(rp->hci_rev) == 0x0810 &&
> - le16_to_cpu(rp->hci_ver) == BLUETOOTH_VER_4_0) {
> - bt_dev_warn(hdev, "CSR: detected a fake CSR dongle using a Barrot 8041a02 chip, this chip is very buggy and may have issues");
> -
> - pm_runtime_allow(&data->udev->dev);
> + pm_runtime_allow(&data->udev->dev);
>
> - ret = pm_runtime_suspend(&data->udev->dev);
> - if (ret >= 0)
> - msleep(200);
> - else
> - bt_dev_err(hdev, "Failed to suspend the device for Barrot 8041a02 receive-issue workaround");
> -
> - pm_runtime_forbid(&data->udev->dev);
> -
> - device_set_wakeup_capable(&data->udev->dev, false);
> - /* Re-enable autosuspend if this was requested */
> - if (enable_autosuspend)
> - usb_enable_autosuspend(data->udev);
> - }
> + ret = pm_runtime_suspend(&data->udev->dev);
> + if (ret >= 0)
> + msleep(200);
> + else
> + bt_dev_err(hdev, "CSR: Failed to suspend the device for our Barrot 8041a02 receive-issue workaround");
> +
> + pm_runtime_forbid(&data->udev->dev);
> +
> + device_set_wakeup_capable(&data->udev->dev, false);
> +
> + /* Re-enable autosuspend if this was requested */
> + if (enable_autosuspend)
> + usb_enable_autosuspend(data->udev);
> }
>
> kfree_skb(skb);
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Bluetooth: btusb: Make the CSR clone chip force-suspend workaround more generic
2021-07-16 23:21 [PATCH] Bluetooth: btusb: Make the CSR clone chip force-suspend workaround more generic Ismael Ferreras Morezuelas
2021-07-17 0:05 ` bluez.test.bot
2021-07-17 9:31 ` [PATCH] " Hans de Goede
@ 2021-07-29 19:55 ` Marcel Holtmann
2021-07-29 22:24 ` Ismael Ferreras Morezuelas
2 siblings, 1 reply; 5+ messages in thread
From: Marcel Holtmann @ 2021-07-29 19:55 UTC (permalink / raw)
To: Ismael Ferreras Morezuelas
Cc: Johan Hedberg, Luiz Augusto von Dentz, BlueZ, linux-kernel,
Hans de Goede
Hi Ismael,
> Turns out Hans de Goede completed the work I started last year trying to
> improve Chinese-clone detection of CSR controller chips. Quirk after quirk
> these Bluetooth dongles are more usable now.
>
> Even after a few BlueZ regressions; these clones are so fickle that some
> days they stop working altogether. Except on Windows, they work fine.
>
>
> But this force-suspend initialization quirk seems to mostly do the trick,
> after a lot of testing Bluetooth now seems to work *all* the time.
>
> The only problem is that the solution ended up being masked under a very
> stringent check; when there are probably hundreds of fake dongle
> models out there that benefit from a good reset. Make it so.
>
>
> Fixes: 81cac64ba258a ("Bluetooth: Deal with USB devices that are faking CSR vendor")
> Fixes: cde1a8a992875 ("Bluetooth: btusb: Fix and detect most of the Chinese Bluetooth controllers")
> Fixes: d74e0ae7e0303 ("Bluetooth: btusb: Fix detection of some fake CSR controllers with a bcdDevice val of 0x0134")
> Fixes: 0671c0662383e ("Bluetooth: btusb: Add workaround for remote-wakeup issues with Barrot 8041a02 fake CSR controllers")
>
> Cc: stable@vger.kernel.org
> Cc: Hans de Goede <hdegoede@redhat.com>
> Tested-by: Ismael Ferreras Morezuelas <swyterzone@gmail.com>
> Signed-off-by: Ismael Ferreras Morezuelas <swyterzone@gmail.com>
> ---
>
> I've changed the warning line to make it easy to grep and detect if this updated
> workaround is part of the driver. Should make it much more obvious to users in
> case their dongle doesn't work for other reasons. There's a clear then-now.
>
> Easy to narrow other future issues down. Let me know what you think.
>
> drivers/bluetooth/btusb.c | 61 +++++++++++++++++++++------------------
> 1 file changed, 33 insertions(+), 28 deletions(-)
patch has been applied to bluetooth-next tree.
Regards
Marcel
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Bluetooth: btusb: Make the CSR clone chip force-suspend workaround more generic
2021-07-29 19:55 ` Marcel Holtmann
@ 2021-07-29 22:24 ` Ismael Ferreras Morezuelas
0 siblings, 0 replies; 5+ messages in thread
From: Ismael Ferreras Morezuelas @ 2021-07-29 22:24 UTC (permalink / raw)
To: Marcel Holtmann
Cc: Johan Hedberg, Luiz Augusto von Dentz, BlueZ, linux-kernel,
Hans de Goede
On 29/07/2021 21:55, Marcel Holtmann wrote:
> Hi Ismael,
>
> patch has been applied to bluetooth-next tree.
>
> Regards
>
> Marcel
>
Thanks a lot, Marcel! Upstreaming these changes has been very fun,
looking forward to improve the compatibility more in the future.
Every bit helps. The next step will probably entail
adding a HCI_FLT_CLEAR_ALL quirk in there. :)
^ permalink raw reply [flat|nested] 5+ messages in thread