[PATCH v3 0/1] Bluetooth: btusb: Add workaround for Barrot 8041a02 fake CSR controllers

[PATCH v3 0/1] Bluetooth: btusb: Add workaround for Barrot 8041a02 fake CSR controllers

[Hans de Goede]

Hi All,

Here is a new version of my patch to make fake CSR controllers with a
Barrot 8041a02 chip work under Linux.

Changes since the previous version:

- Post as a separate patch, instead of together with the unrelated fix
  for a regression on some other fake CSR devices
- 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

Regards,

Hans

[PATCH v3] Bluetooth: btusb: Add workaround for remote-wakeup issues with Barrot 8041a02 fake CSR controllers

[Hans de Goede]

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(+)

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);
-- 
2.28.0

RE: [v3] Bluetooth: btusb: Add workaround for remote-wakeup issues with Barrot 8041a02 fake CSR controllers

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 1576 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=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

Re: [PATCH v3] Bluetooth: btusb: Add workaround for remote-wakeup issues with Barrot 8041a02 fake CSR controllers

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 7890 bytes --]

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

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 22577 bytes --]

Re: [PATCH v3 0/1] Bluetooth: btusb: Add workaround for Barrot 8041a02 fake CSR controllers

[Marcel Holtmann]

Hi Hans,

> Here is a new version of my patch to make fake CSR controllers with a
> Barrot 8041a02 chip work under Linux.
> 
> Changes since the previous version:
> 
> - Post as a separate patch, instead of together with the unrelated fix
>  for a regression on some other fake CSR devices
> - 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

can you send the full series? The kbot seems to be complaining.

Regards

Marcel