Hi Alex,
> On Sep 25, 2020, at 15:42, 陆朱伟 <alex_lu@xxxxxxxxxxxxxx> wrote:
>
> Hi Kai-Heng,
>
>> On 25 September 2020 at 15:14, Kai-Heng Feng wrote:
>>
>> Hi Alex,
[snipped]
>> 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.
>
>> 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?
>
>> 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.
> 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
>
>>
>> Kai-Heng
>>
>>>
>>>>
>>>> Kai-Heng
>>>>
>>>>>
>>>>> @Alex -- What is the common behavior for Realtek controllers? Should
>>>>> we set BTUSB_WAKEUP_DISABLE only on RTL8822CE or should we unset
>> it
>>>>> only on RTL8821CE?
>>>>>
>>>>>>>
>>>>>>> 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
>>>> one.
>>>>>>
>>>>>> Kai-Heng
>>>>>>
>>>>>>>
>>>>>>> Abhishek
>>>>>>>
>>>>>>> On Wed, Sep 23, 2020 at 10:56 AM Kai-Heng Feng
>>>>>>> <kai.heng.feng@xxxxxxxxxxxxx> wrote:
>>>>>>>>
>>>>>>>> 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
>>>> failed (-110)
>>>>>>>>
>>>>>>>> If platform firmware doesn't cut power off during suspend, the
>>>> firmware
>>>>>>>> is considered retained in controller but the driver is still asking USB
>>>>>>>> core to perform a reset-resume. This can make bluetooth controller
>>>>>>>> unusable.
>>>>>>>>
>>>>>>>> So avoid unnecessary reset to resolve the issue.
>>>>>>>>
>>>>>>>> For devices that really lose power during suspend, USB core will
>> detect
>>>>>>>> and handle reset-resume correctly.
>>>>>>>>
>>>>>>>> Signed-off-by: Kai-Heng Feng <kai.heng.feng@xxxxxxxxxxxxx>
>>>>>>>> ---
>>>>>>>> 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)
>>>>>>>> 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
>>>> if
>>>>>>>> + * platform firmware cut power off. But for autosuspend,
>>>> firmware
>>>>>>>> + * should remain. Actually, it depends on whether the usb host
>>>> sends
>>>>>>>> * 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.
