Re: [PATCH v2 2/2] Bluetooth: btusb: Add workaround for remote-wakeup issues with some fake CSR controllers

[Hans de Goede <hdegoede@redhat.com>]

Hi,

On 11/23/20 11:13 AM, Hans de Goede wrote:
> With the recent btusb change to detect and deal with more fake CSR
> controllers, I decided to see if one of the fakes which I have
> lying around 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 bcdDevice value of 0x8891,
> a lmp_subver of 0x0x1012, a hci_rev of 0x0810 and a hci_ver of
> BLUETOOTH_VER_4_0.

I just realized that I should have opened this one and add the
chipmarkings to the commit msg here too. So I've just opened it
and took a look.

This one has a Barrot 8041a02 chip and searching for 8041a92 shows that
the internet is full of reports of how crappy it is.

I guess this patch probably will get some review remarks anyways,
but let me know if you want a v3 with just the chip added to the
commit msg.

Regards,

Hans


> 
> 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>
> ---
>  drivers/bluetooth/btusb.c | 32 ++++++++++++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
> 
> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> index ac7fede4f951..48e404dfa246 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,37 @@ 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);
> +
> +		/*
> +		 * Some of 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 (device_can_wakeup(&data->udev->dev)) {
> +			pm_runtime_allow(&data->udev->dev);
> +
> +			ret = pm_runtime_suspend(&data->udev->dev);
> +			if (ret >= 0)
> +				msleep(200);
> +			else
> +				bt_dev_warn(hdev, "Failed to suspend the device for CSR clone 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);
>