Hi Alex,

On Sep 25, 2020, at 16:23, 陆朱伟 [off-list ref] wrote:

Hi Kai-Heng,

quoted

On September 25, 2020 at 15:56, Kai-Heng Feng wrote:

Hi Alex,

quoted

On Sep 25, 2020, at 15:42, 陆朱伟 [off-list ref] wrote:

Hi Kai-Heng,

quoted

On 25 September 2020 at 15:14, Kai-Heng Feng wrote:

Hi Alex,

[snipped]

quoted

quoted

Apparently for my case, RTL8821CE, firmware was kept without setting
remote wakeup.

So you got the btusb disconnect and reprobe sequence after resume, and "

Bluetooth: hci0: command 0x1001 tx timeout " before firmware downloading ?

USB power wasn't lost, but it got USB warm reset because btusb driver
explicitly flagged "reset_resume = 1".
Then the issue appeared as "Bluetooth: hci0: command 0x1001 tx timeout",
before downloading firmware.

quoted

quoted

Is it okay to also set remote wakeup for global suspend to retain the
firmware?

Yes, it's ok.

Abhishek, does setting remote wakeup during global suspend works for you?

It depends on your desire on power consumption during global suspend.
The BT controller takes less power if firmware was lost during global suspend.

For my case, the firmware is retained after S3, despite of "reset_resume = 1":

[ 30.164036] ACPI: Waking up from system sleep state S3 
[ 30.167913] ACPI: EC: interrupt unblocked
[ 31.284138] ACPI: EC: event unblocked
...
[   31.467484] usb 1-14: reset full-speed USB device number 3 using xhci_hcd
...
[   32.732934] Bluetooth: hci0: RTL: examining hci_ver=08 hci_rev=826c lmp_ver=08 lmp_subver=a99e
[   32.732937] Bluetooth: hci0: RTL: unknown IC info, lmp subver a99e, hci rev 826c, hci ver 0008
[   32.732937] Bluetooth: hci0: RTL: assuming no firmware upload needed

Kai-Heng

quoted

quoted

quoted

If firmware was retained, does USB warm reset affect BT controller in
anyway?

USB warm reset shouldn't affect BT controller.
But hci device will not work after resume, because btrtl will find "unknown

IC info, lmp subvert ..." and return error when hci device setup is called.

quoted

Tips: The lmp subver in controller changes after firmware downloading.

And driver will find " unknown IC info, lmp subver  ..." when setup is called
with firmware retained.

This should already be fixed by "Bluetooth: btrtl: Restore old logic to assume
firmware is already loaded".

Kai-Heng

quoted

quoted

Kai-Heng

quoted

quoted

Kai-Heng

quoted

@Alex -- What is the common behavior for Realtek controllers?

Should

quoted

quoted

quoted

quoted

quoted

we set BTUSB_WAKEUP_DISABLE only on RTL8822CE or should we

unset

quoted

quoted

it

quoted

quoted

quoted

only on RTL8821CE?

quoted

quoted

I would prefer this doesn't get accepted in its current state.

Of course.
I think we need to find the root cause for your case before applying

this

quoted

quoted

quoted

quoted

one.

quoted

quoted

Kai-Heng

quoted

Abhishek

On Wed, Sep 23, 2020 at 10:56 AM Kai-Heng Feng
[off-list ref] wrote:

quoted

Realtek bluetooth controller may fail to work after system sleep:
[ 1272.707670] Bluetooth: hci0: command 0x1001 tx timeout
[ 1280.835712] Bluetooth: hci0: RTL:

HCI_OP_READ_LOCAL_VERSION

quoted

quoted

quoted

quoted

failed (-110)

quoted

quoted

quoted

quoted

If platform firmware doesn't cut power off during suspend, the

firmware

quoted

quoted

quoted

quoted

is considered retained in controller but the driver is still asking USB
core to perform a reset-resume. This can make bluetooth

controller

quoted

quoted

quoted

quoted

quoted

quoted

quoted

quoted

unusable.

So avoid unnecessary reset to resolve the issue.

For devices that really lose power during suspend, USB core will

detect

quoted

quoted

quoted

quoted

quoted

quoted

and handle reset-resume correctly.

Signed-off-by: Kai-Heng Feng <redacted>
---
drivers/bluetooth/btusb.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index 8d2608ddfd08..de86ef4388f9 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c

@@ -4255,17 +4255,15 @@ static int btusb_suspend(struct

usb_interface *intf, pm_message_t message)

quoted

quoted

quoted

quoted

           enable_irq(data->oob_wake_irq);
   }

-       /* For global suspend, Realtek devices lose the loaded fw
-        * in them. But for autosuspend, firmware should remain.
-        * Actually, it depends on whether the usb host sends
+       /* For global suspend, Realtek devices lose the loaded fw in

them

quoted

quoted

if

quoted

quoted

quoted

quoted

+        * platform firmware cut power off. But for autosuspend,

firmware

quoted

quoted

quoted

quoted

+        * should remain.  Actually, it depends on whether the usb

host

quoted

quoted

quoted

quoted

sends

quoted

quoted

quoted

quoted

    * set feature (enable wakeup) or not.
    */
   if (test_bit(BTUSB_WAKEUP_DISABLE, &data->flags)) {
           if (PMSG_IS_AUTO(message) &&
               device_can_wakeup(&data->udev->dev))
                   data->udev->do_remote_wakeup = 1;
-               else if (!PMSG_IS_AUTO(message))
-                       data->udev->reset_resume = 1;
   }

   return 0;
--
2.17.1

------Please consider the environment before printing this e-mail.