| Message ID | 20201203110944.49307-2-hdegoede@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v3] Bluetooth: btusb: Add workaround for remote-wakeup issues with Barrot 8041a02 fake CSR controllers | 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=395391
---Test result---
##############################
Test: CheckPatch - PASS
##############################
Test: CheckGitLint - FAIL
Output:
Bluetooth: btusb: Add workaround for remote-wakeup issues with Barrot 8041a02 fake CSR controllers
1: T1 Title exceeds max length (98>72): "Bluetooth: btusb: Add workaround for remote-wakeup issues with Barrot 8041a02 fake CSR controllers"
##############################
Test: CheckBuildK - FAIL
Output:
drivers/bluetooth/btusb.c: In function ‘btusb_setup_csr’:
drivers/bluetooth/btusb.c:1893:7: error: ‘bcdDevice’ undeclared (first use in this function); did you mean ‘device’?
1893 | if (bcdDevice == 0x8891 &&
| ^~~~~~~~~
| device
drivers/bluetooth/btusb.c:1893:7: note: each undeclared identifier is reported only once for each function it appears in
drivers/bluetooth/btusb.c:1899:22: error: ‘data’ undeclared (first use in this function); did you mean ‘_data’?
1899 | pm_runtime_allow(&data->udev->dev);
| ^~~~
| _data
make[2]: *** [scripts/Makefile.build:283: drivers/bluetooth/btusb.o] Error 1
make[1]: *** [scripts/Makefile.build:500: drivers/bluetooth] Error 2
make: *** [Makefile:1799: drivers] Error 2
---
Regards,
Linux Bluetooth
Hi Hans,
I love your patch! Yet something to improve:
[auto build test ERROR on bluetooth-next/master]
[also build test ERROR on v5.10-rc6 next-20201203]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Hans-de-Goede/Bluetooth-btusb-Add-workaround-for-remote-wakeup-issues-with-Barrot-8041a02-fake-CSR-controllers/20201203-191712
base: https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git master
config: arm-randconfig-r032-20201203 (attached as .config)
compiler: arm-linux-gnueabi-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/8dd4bd66a9bd4a811d7389743b7495719c6e4de3
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Hans-de-Goede/Bluetooth-btusb-Add-workaround-for-remote-wakeup-issues-with-Barrot-8041a02-fake-CSR-controllers/20201203-191712
git checkout 8dd4bd66a9bd4a811d7389743b7495719c6e4de3
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=arm
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
drivers/bluetooth/btusb.c: In function 'btusb_setup_csr':
>> drivers/bluetooth/btusb.c:1893:7: error: 'bcdDevice' undeclared (first use in this function); did you mean 'device'?
1893 | if (bcdDevice == 0x8891 &&
| ^~~~~~~~~
| device
drivers/bluetooth/btusb.c:1893:7: note: each undeclared identifier is reported only once for each function it appears in
>> drivers/bluetooth/btusb.c:1899:22: error: 'data' undeclared (first use in this function); did you mean '_data'?
1899 | pm_runtime_allow(&data->udev->dev);
| ^~~~
| _data
vim +1893 drivers/bluetooth/btusb.c
1788
1789 static int btusb_setup_csr(struct hci_dev *hdev)
1790 {
1791 struct hci_rp_read_local_version *rp;
1792 struct sk_buff *skb;
1793 bool is_fake = false;
1794 int ret;
1795
1796 BT_DBG("%s", hdev->name);
1797
1798 skb = __hci_cmd_sync(hdev, HCI_OP_READ_LOCAL_VERSION, 0, NULL,
1799 HCI_INIT_TIMEOUT);
1800 if (IS_ERR(skb)) {
1801 int err = PTR_ERR(skb);
1802 bt_dev_err(hdev, "CSR: Local version failed (%d)", err);
1803 return err;
1804 }
1805
1806 if (skb->len != sizeof(struct hci_rp_read_local_version)) {
1807 bt_dev_err(hdev, "CSR: Local version length mismatch");
1808 kfree_skb(skb);
1809 return -EIO;
1810 }
1811
1812 rp = (struct hci_rp_read_local_version *)skb->data;
1813
1814 /* Detect a wide host of Chinese controllers that aren't CSR.
1815 *
1816 * Known fake bcdDevices: 0x0100, 0x0134, 0x1915, 0x2520, 0x7558, 0x8891
1817 *
1818 * The main thing they have in common is that these are really popular low-cost
1819 * options that support newer Bluetooth versions but rely on heavy VID/PID
1820 * squatting of this poor old Bluetooth 1.1 device. Even sold as such.
1821 *
1822 * We detect actual CSR devices by checking that the HCI manufacturer code
1823 * is Cambridge Silicon Radio (10) and ensuring that LMP sub-version and
1824 * HCI rev values always match. As they both store the firmware number.
1825 */
1826 if (le16_to_cpu(rp->manufacturer) != 10 ||
1827 le16_to_cpu(rp->hci_rev) != le16_to_cpu(rp->lmp_subver))
1828 is_fake = true;
1829
1830 /* Known legit CSR firmware build numbers and their supported BT versions:
1831 * - 1.1 (0x1) -> 0x0073, 0x020d, 0x033c, 0x034e
1832 * - 1.2 (0x2) -> 0x04d9, 0x0529
1833 * - 2.0 (0x3) -> 0x07a6, 0x07ad, 0x0c5c
1834 * - 2.1 (0x4) -> 0x149c, 0x1735, 0x1899 (0x1899 is a BlueCore4-External)
1835 * - 4.0 (0x6) -> 0x1d86, 0x2031, 0x22bb
1836 *
1837 * e.g. Real CSR dongles with LMP subversion 0x73 are old enough that
1838 * support BT 1.1 only; so it's a dead giveaway when some
1839 * third-party BT 4.0 dongle reuses it.
1840 */
1841 else if (le16_to_cpu(rp->lmp_subver) <= 0x034e &&
1842 le16_to_cpu(rp->hci_ver) > BLUETOOTH_VER_1_1)
1843 is_fake = true;
1844
1845 else if (le16_to_cpu(rp->lmp_subver) <= 0x0529 &&
1846 le16_to_cpu(rp->hci_ver) > BLUETOOTH_VER_1_2)
1847 is_fake = true;
1848
1849 else if (le16_to_cpu(rp->lmp_subver) <= 0x0c5c &&
1850 le16_to_cpu(rp->hci_ver) > BLUETOOTH_VER_2_0)
1851 is_fake = true;
1852
1853 else if (le16_to_cpu(rp->lmp_subver) <= 0x1899 &&
1854 le16_to_cpu(rp->hci_ver) > BLUETOOTH_VER_2_1)
1855 is_fake = true;
1856
1857 else if (le16_to_cpu(rp->lmp_subver) <= 0x22bb &&
1858 le16_to_cpu(rp->hci_ver) > BLUETOOTH_VER_4_0)
1859 is_fake = true;
1860
1861 if (is_fake) {
1862 bt_dev_warn(hdev, "CSR: Unbranded CSR clone detected; adding workarounds...");
1863
1864 /* Generally these clones have big discrepancies between
1865 * advertised features and what's actually supported.
1866 * Probably will need to be expanded in the future;
1867 * without these the controller will lock up.
1868 */
1869 set_bit(HCI_QUIRK_BROKEN_STORED_LINK_KEY, &hdev->quirks);
1870 set_bit(HCI_QUIRK_BROKEN_ERR_DATA_REPORTING, &hdev->quirks);
1871
1872 /* Clear the reset quirk since this is not an actual
1873 * early Bluetooth 1.1 device from CSR.
1874 */
1875 clear_bit(HCI_QUIRK_RESET_ON_CLOSE, &hdev->quirks);
1876 clear_bit(HCI_QUIRK_SIMULTANEOUS_DISCOVERY, &hdev->quirks);
1877
1878 /*
1879 * Special workaround for clones with a Barrot 8041a02 chip,
1880 * these clones are really messed-up:
1881 * 1. Their bulk rx endpoint will never report any data unless
1882 * the device was suspended at least once (yes really).
1883 * 2. They will not wakeup when autosuspended and receiving data
1884 * on their bulk rx endpoint from e.g. a keyboard or mouse
1885 * (IOW remote-wakeup support is broken for the bulk endpoint).
1886 *
1887 * To fix 1. enable runtime-suspend, force-suspend the
1888 * hci and then wake-it up by disabling runtime-suspend.
1889 *
1890 * To fix 2. clear the hci's can_wake flag, this way the hci
1891 * will still be autosuspended when it is not open.
1892 */
> 1893 if (bcdDevice == 0x8891 &&
1894 le16_to_cpu(rp->lmp_subver) == 0x1012 &&
1895 le16_to_cpu(rp->hci_rev) == 0x0810 &&
1896 le16_to_cpu(rp->hci_ver) == BLUETOOTH_VER_4_0) {
1897 bt_dev_warn(hdev, "CSR: detected a fake CSR dongle using a Barrot 8041a02 chip, this chip is very buggy and may have issues\n");
1898
> 1899 pm_runtime_allow(&data->udev->dev);
1900
1901 ret = pm_runtime_suspend(&data->udev->dev);
1902 if (ret >= 0)
1903 msleep(200);
1904 else
1905 bt_dev_err(hdev, "Failed to suspend the device for Barrot 8041a02 receive-issue workaround\n");
1906
1907 pm_runtime_forbid(&data->udev->dev);
1908
1909 device_set_wakeup_capable(&data->udev->dev, false);
1910 /* Re-enable autosuspend if this was requested */
1911 if (enable_autosuspend)
1912 usb_enable_autosuspend(data->udev);
1913 }
1914 }
1915
1916 kfree_skb(skb);
1917
1918 return 0;
1919 }
1920
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c index ac7fede4f951..236aefaf925c 100644 --- a/drivers/bluetooth/btusb.c +++ b/drivers/bluetooth/btusb.c @@ -1768,6 +1768,7 @@ static int btusb_setup_csr(struct hci_dev *hdev) struct hci_rp_read_local_version *rp; struct sk_buff *skb; bool is_fake = false; + int ret; BT_DBG("%s", hdev->name); @@ -1856,6 +1857,43 @@ static int btusb_setup_csr(struct hci_dev *hdev) */ clear_bit(HCI_QUIRK_RESET_ON_CLOSE, &hdev->quirks); 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). + * 2. They will not wakeup when autosuspended and receiving data + * 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. + * + * To fix 2. clear the hci's can_wake flag, this way the hci + * will still be autosuspended when it is not open. + */ + 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\n"); + + 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\n"); + + 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);
With the recent btusb change to detect and deal with more fake CSR controllers, I decided to see if fake CSR controllers with Barrot 8041a02 chips would now work. After much experimentation I came to the conclusion that it works, if I have autosuspend enabled initially and then disable it after the device has suspended at least once. Yes this is very weird, but I've tried many things, like manually clearing the remote-wakeup feature. Doing a runtime-resume + runtime suspend is the only way to get the receiver to actually report received data (and/or pairing info) through its bulk rx endpoint. But the funkyness of the bulk-endpoint does not stop there, I mainly found out about this problem, because with autosuspend enabled (which usually ensures the suspend at least once condition is met), the receiver stops reporting received data through its bulk rx endpoint as soon as autosuspend kicks in. So I initially just disabled autosuspend, but then the receiver does not work at all. This was with a fake CSR receiver with a Barrot 8041a02 chip with a bcdDevice value of 0x8891, a lmp_subver of 0x1012, a hci_rev of 0x0810 and a hci_ver of BLUETOOTH_VER_4_0. Summarizing this specific fake CSR receiver has the following 2 issues: 1. The bulk rx endpoint will never report any data unless the device was suspended at least once. 2. They will not wakeup when autosuspended and receiving data on their bulk rx endpoint from e.g. a keyboard or mouse (IOW remote-wakeup support is broken for the bulk endpoint). Add a workaround for 1. which enables runtime-suspend, force-suspends the hci and then wakes-it up by disabling runtime-suspend again. Add a workaround for 2. which clears the hci's can_wake flag, this way the hci will still be autosuspended when it is not open. Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- Changes in v3: - Add info about which fake CSR chip this happens on to the commit message and source code comment - Only apply the workaround on this specific chip --- drivers/bluetooth/btusb.c | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+)