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 <[email protected]>
---
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
Hi Kai-Heng,
Which Realtek controller is this on?
Specifically for RTL8822CE, we tested without reset_resume being set
and that was causing the controller being reset without bluez ever
learning about it (resulting in devices being unusable without
toggling the BT power).
If the firmware doesn't cut off power during suspend, maybe you
shouldn't set the BTUSB_WAKEUP_DISABLE flag for that controller.
I would prefer this doesn't get accepted in its current state.
Abhishek
On Wed, Sep 23, 2020 at 10:56 AM Kai-Heng Feng
<[email protected]> 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 <[email protected]>
> ---
> 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
>
[+Cc linux-usb]
Hi Abhishek,
> On Sep 24, 2020, at 04:41, Abhishek Pandit-Subedi <[email protected]> wrote:
>
> Hi Kai-Heng,
>
> Which Realtek controller is this on?'
The issue happens on 8821CE.
>
> Specifically for RTL8822CE, we tested without reset_resume being set
> and that was causing the controller being reset without bluez ever
> learning about it (resulting in devices being unusable without
> toggling the BT power).
The reset is done by the kernel, so how does that affect bluez?
From what you described, it sounds more like runtime resume since bluez is already running.
If we need reset resume for runtime resume, maybe it's another bug which needs to be addressed?
> If the firmware doesn't cut off power during suspend, maybe you
> shouldn't set the BTUSB_WAKEUP_DISABLE flag for that controller.
We don't know beforehand if the platform firmware (BIOS for my case) will cut power off or not.
In general, laptops will cut off the USB power during S3.
When AC is plugged, some laptops cuts USB power off and some don't. This also applies to many desktops. Not to mention there can be BIOS options to control USB power under S3/S4/S5...
So we don't know beforehand.
>
> 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
> <[email protected]> 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 <[email protected]>
>> ---
>> 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
>>
Hi Abhishek,
> On Sep 25, 2020, at 11:33, Abhishek Pandit-Subedi <[email protected]> wrote:
>
> + Alex Lu (who contributed the original change)
>
> Hi Kai-Heng,
>
>
> On Thu, Sep 24, 2020 at 12:10 AM Kai-Heng Feng
> <[email protected]> wrote:
>>
>> [+Cc linux-usb]
>>
>> Hi Abhishek,
>>
>>> On Sep 24, 2020, at 04:41, Abhishek Pandit-Subedi <[email protected]> wrote:
>>>
>>> Hi Kai-Heng,
>>>
>>> Which Realtek controller is this on?'
>>
>> The issue happens on 8821CE.
>>
>>>
>>> Specifically for RTL8822CE, we tested without reset_resume being set
>>> and that was causing the controller being reset without bluez ever
>>> learning about it (resulting in devices being unusable without
>>> toggling the BT power).
>>
>> The reset is done by the kernel, so how does that affect bluez?
>>
>> From what you described, it sounds more like runtime resume since bluez is already running.
>> If we need reset resume for runtime resume, maybe it's another bug which needs to be addressed?
>
> From btusb.c: https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git/tree/drivers/bluetooth/btusb.c#n4189
> /* Realtek devices lose their updated firmware over global
> * suspend that means host doesn't send SET_FEATURE
> * (DEVICE_REMOTE_WAKEUP)
> */
>
> Runtime suspend always requires remote wakeup to be set and reset
> resume isn't used there.
Thanks for the clarification.
>
> During system suspend, when remote wakeup is not set, RTL8822CE loses
> the FW loaded by the driver and any state currently in the controller.
> This causes the kernel and the controller state to go out of sync.
> One of the issues we observed on the Realtek controller without the
> reset resume quirk was that paired or connected devices would just
> stop working after resume.
>
>>
>>> If the firmware doesn't cut off power during suspend, maybe you
>>> shouldn't set the BTUSB_WAKEUP_DISABLE flag for that controller.
>>
>> We don't know beforehand if the platform firmware (BIOS for my case) will cut power off or not.
>>
>> In general, laptops will cut off the USB power during S3.
>> When AC is plugged, some laptops cuts USB power off and some don't. This also applies to many desktops. Not to mention there can be BIOS options to control USB power under S3/S4/S5...
>>
>> So we don't know beforehand.
>>
>
> I think the confusion here stems from what is actually being turned
> off between our two boards and what we're referring to as firmware :)
Yes :)
>
> In your case, the Realtek controller retains firmware unless the
> platform cuts of power to USB (which it does during S3).
Not all platform firmware (i.e. BIOS for x86) cut USB power during S3, as I describe in last reply.
> In my case, the Realtek controller loses firmware when Remote Wakeup
> isn't set, even if the platform doesn't cut power to USB.
Thanks for the clarification, I believe it's a case that should to be handled separately.
>
> In your case, since you don't need to enforce the 'Remote Wakeup' bit,
> if you unset the BTUSB_WAKEUP_DISABLE for that VID:PID, you should get
> the desirable behavior (which is actually the default behavior; remote
> wake will always be asserted instead of only during Runtime Suspend).
So we have three cases here. Assuming reset_resume isn't flagged by btusb:
1) Both USB power and BT firmware were lost during suspend.
USB core finds out power was lost, try to reset resume the device. Since btusb doesn't have reset_resume callback, USB core calls probe instead.
2) Both USB power and BT firmware were kept during suspend. This is my case.
Regular resume handles everything.
3) USB power was kept but BT firmware was lost. This is your case.
USB core finds out power was kept, use regular resume. However the BT firmware was lost, so resume fails.
For this case, maybe we can use btrtl_setup_realtek() in btusb_resume()? It won't re-upload firmware if firmware is retained, and will do proper initializing if firmware was lost.
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
>>> <[email protected]> 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 <[email protected]>
>>>> ---
>>>> 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
Hi Abhishek,
> On September 25, 2020 at 11:34, Abhishek Pandit-Subedi wrote:
>
> + Alex Lu (who contributed the original change)
>
> Hi Kai-Heng,
>
>
> On Thu, Sep 24, 2020 at 12:10 AM Kai-Heng Feng
> <[email protected]> wrote:
> >
> > [+Cc linux-usb]
> >
> > Hi Abhishek,
> >
> > > On Sep 24, 2020, at 04:41, Abhishek Pandit-Subedi
> <[email protected]> wrote:
> > >
> > > Hi Kai-Heng,
> > >
> > > Which Realtek controller is this on?'
> >
> > The issue happens on 8821CE.
> >
> > >
> > > Specifically for RTL8822CE, we tested without reset_resume being set
> > > and that was causing the controller being reset without bluez ever
> > > learning about it (resulting in devices being unusable without
> > > toggling the BT power).
> >
> > The reset is done by the kernel, so how does that affect bluez?
> >
> > From what you described, it sounds more like runtime resume since bluez
> is already running.
> > If we need reset resume for runtime resume, maybe it's another bug
> which needs to be addressed?
>
> From btusb.c:
> https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-
> next.git/tree/drivers/bluetooth/btusb.c#n4189
> /* Realtek devices lose their updated firmware over global
> * suspend that means host doesn't send SET_FEATURE
> * (DEVICE_REMOTE_WAKEUP)
> */
>
> Runtime suspend always requires remote wakeup to be set and reset
> resume isn't used there.
>
> During system suspend, when remote wakeup is not set, RTL8822CE loses
> the FW loaded by the driver and any state currently in the controller.
> This causes the kernel and the controller state to go out of sync.
> One of the issues we observed on the Realtek controller without the
> reset resume quirk was that paired or connected devices would just
> stop working after resume.
>
> >
> > > If the firmware doesn't cut off power during suspend, maybe you
> > > shouldn't set the BTUSB_WAKEUP_DISABLE flag for that controller.
> >
> > We don't know beforehand if the platform firmware (BIOS for my case) will
> cut power off or not.
> >
> > In general, laptops will cut off the USB power during S3.
> > When AC is plugged, some laptops cuts USB power off and some don't. This
> also applies to many desktops. Not to mention there can be BIOS options to
> control USB power under S3/S4/S5...
> >
> > So we don't know beforehand.
> >
>
> I think the confusion here stems from what is actually being turned
> off between our two boards and what we're referring to as firmware :)
>
> In your case, the Realtek controller retains firmware unless the
> platform cuts of power to USB (which it does during S3).
> In my case, the Realtek controller loses firmware when Remote Wakeup
> isn't set, even if the platform doesn't cut power to USB.
>
> In your case, since you don't need to enforce the 'Remote Wakeup' bit,
> if you unset the BTUSB_WAKEUP_DISABLE for that VID:PID, you should get
> the desirable behavior (which is actually the default behavior; remote
> wake will always be asserted instead of only during Runtime Suspend).
>
> @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?
>
According to the feedback from our firmware engineers, all Realtek controllers should have the same behavior on suspend and resume.
@ Kai-Heng, " Bluetooth: hci0: command 0x1001 tx timeout " is irrelevant to firmware loss or not. The rom code in controller supports this command.
> > >
> > > 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
> > > <[email protected]> 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 <[email protected]>
> > >> ---
> > >> 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
> > >>
> >
>
> On Sep 25, 2020, at 14:40, 陆朱伟 <[email protected]> wrote:
>
> Hi Abhishek,
>
>> On September 25, 2020 at 11:34, Abhishek Pandit-Subedi wrote:
>>
>> + Alex Lu (who contributed the original change)
>>
>> Hi Kai-Heng,
>>
>>
>> On Thu, Sep 24, 2020 at 12:10 AM Kai-Heng Feng
>> <[email protected]> wrote:
>>>
>>> [+Cc linux-usb]
>>>
>>> Hi Abhishek,
>>>
>>>> On Sep 24, 2020, at 04:41, Abhishek Pandit-Subedi
>> <[email protected]> wrote:
>>>>
>>>> Hi Kai-Heng,
>>>>
>>>> Which Realtek controller is this on?'
>>>
>>> The issue happens on 8821CE.
>>>
>>>>
>>>> Specifically for RTL8822CE, we tested without reset_resume being set
>>>> and that was causing the controller being reset without bluez ever
>>>> learning about it (resulting in devices being unusable without
>>>> toggling the BT power).
>>>
>>> The reset is done by the kernel, so how does that affect bluez?
>>>
>>> From what you described, it sounds more like runtime resume since bluez
>> is already running.
>>> If we need reset resume for runtime resume, maybe it's another bug
>> which needs to be addressed?
>>
>> From btusb.c:
>> https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-
>> next.git/tree/drivers/bluetooth/btusb.c#n4189
>> /* Realtek devices lose their updated firmware over global
>> * suspend that means host doesn't send SET_FEATURE
>> * (DEVICE_REMOTE_WAKEUP)
>> */
>>
>> Runtime suspend always requires remote wakeup to be set and reset
>> resume isn't used there.
>>
>> During system suspend, when remote wakeup is not set, RTL8822CE loses
>> the FW loaded by the driver and any state currently in the controller.
>> This causes the kernel and the controller state to go out of sync.
>> One of the issues we observed on the Realtek controller without the
>> reset resume quirk was that paired or connected devices would just
>> stop working after resume.
>>
>>>
>>>> If the firmware doesn't cut off power during suspend, maybe you
>>>> shouldn't set the BTUSB_WAKEUP_DISABLE flag for that controller.
>>>
>>> We don't know beforehand if the platform firmware (BIOS for my case) will
>> cut power off or not.
>>>
>>> In general, laptops will cut off the USB power during S3.
>>> When AC is plugged, some laptops cuts USB power off and some don't. This
>> also applies to many desktops. Not to mention there can be BIOS options to
>> control USB power under S3/S4/S5...
>>>
>>> So we don't know beforehand.
>>>
>>
>> I think the confusion here stems from what is actually being turned
>> off between our two boards and what we're referring to as firmware :)
>>
>> In your case, the Realtek controller retains firmware unless the
>> platform cuts of power to USB (which it does during S3).
>> In my case, the Realtek controller loses firmware when Remote Wakeup
>> isn't set, even if the platform doesn't cut power to USB.
>>
>> In your case, since you don't need to enforce the 'Remote Wakeup' bit,
>> if you unset the BTUSB_WAKEUP_DISABLE for that VID:PID, you should get
>> the desirable behavior (which is actually the default behavior; remote
>> wake will always be asserted instead of only during Runtime Suspend).
>>
>> @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?
>>
>
> According to the feedback from our firmware engineers, all Realtek controllers should have the same behavior on suspend and resume.
> @ Kai-Heng, " Bluetooth: hci0: command 0x1001 tx timeout " is irrelevant to firmware loss or not. The rom code in controller supports this command.
Does USB warm-reset affect the Bluetooth controller?
Kai-Heng
>
>>>>
>>>> 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
>>>> <[email protected]> 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 <[email protected]>
>>>>> ---
>>>>> 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
Hi Kai-Heng,
> On September 25, 2020 14:04, Kai-Heng Feng wrote:
>
> Hi Abhishek,
> > On Sep 25, 2020, at 11:33, Abhishek Pandit-Subedi
> <[email protected]> wrote:
> >
> > + Alex Lu (who contributed the original change)
> >
> > Hi Kai-Heng,
> >
> >
> > On Thu, Sep 24, 2020 at 12:10 AM Kai-Heng Feng
> > <[email protected]> wrote:
> >>
> >> [+Cc linux-usb]
> >>
> >> Hi Abhishek,
> >>
> >>> On Sep 24, 2020, at 04:41, Abhishek Pandit-Subedi
> <[email protected]> wrote:
> >>>
> >>> Hi Kai-Heng,
> >>>
> >>> Which Realtek controller is this on?'
> >>
> >> The issue happens on 8821CE.
> >>
> >>>
> >>> Specifically for RTL8822CE, we tested without reset_resume being set
> >>> and that was causing the controller being reset without bluez ever
> >>> learning about it (resulting in devices being unusable without
> >>> toggling the BT power).
> >>
> >> The reset is done by the kernel, so how does that affect bluez?
> >>
> >> From what you described, it sounds more like runtime resume since bluez
> is already running.
> >> If we need reset resume for runtime resume, maybe it's another bug
> which needs to be addressed?
> >
> > From btusb.c:
> https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-
> next.git/tree/drivers/bluetooth/btusb.c#n4189
> > /* Realtek devices lose their updated firmware over global
> > * suspend that means host doesn't send SET_FEATURE
> > * (DEVICE_REMOTE_WAKEUP)
> > */
> >
> > Runtime suspend always requires remote wakeup to be set and reset
> > resume isn't used there.
>
> Thanks for the clarification.
>
> >
> > During system suspend, when remote wakeup is not set, RTL8822CE loses
> > the FW loaded by the driver and any state currently in the controller.
> > This causes the kernel and the controller state to go out of sync.
> > One of the issues we observed on the Realtek controller without the
> > reset resume quirk was that paired or connected devices would just
> > stop working after resume.
> >
> >>
> >>> If the firmware doesn't cut off power during suspend, maybe you
> >>> shouldn't set the BTUSB_WAKEUP_DISABLE flag for that controller.
> >>
> >> We don't know beforehand if the platform firmware (BIOS for my case)
> will cut power off or not.
> >>
> >> In general, laptops will cut off the USB power during S3.
> >> When AC is plugged, some laptops cuts USB power off and some don't.
> This also applies to many desktops. Not to mention there can be BIOS
> options to control USB power under S3/S4/S5...
> >>
> >> So we don't know beforehand.
> >>
> >
> > I think the confusion here stems from what is actually being turned
> > off between our two boards and what we're referring to as firmware :)
>
> Yes :)
>
> >
> > In your case, the Realtek controller retains firmware unless the
> > platform cuts of power to USB (which it does during S3).
>
> Not all platform firmware (i.e. BIOS for x86) cut USB power during S3, as I
> describe in last reply.
>
> > In my case, the Realtek controller loses firmware when Remote Wakeup
> > isn't set, even if the platform doesn't cut power to USB.
>
> Thanks for the clarification, I believe it's a case that should to be handled
> separately.
>
> >
> > In your case, since you don't need to enforce the 'Remote Wakeup' bit,
> > if you unset the BTUSB_WAKEUP_DISABLE for that VID:PID, you should get
> > the desirable behavior (which is actually the default behavior; remote
> > wake will always be asserted instead of only during Runtime Suspend).
>
> So we have three cases here. Assuming reset_resume isn't flagged by btusb:
>
> 1) Both USB power and BT firmware were lost during suspend.
> USB core finds out power was lost, try to reset resume the device. Since
> btusb doesn't have reset_resume callback, USB core calls probe instead.
>
> 2) Both USB power and BT firmware were kept during suspend. This is my
> case.
> Regular resume handles everything.
>
> 3) USB power was kept but BT firmware was lost. This is your case.
> USB core finds out power was kept, use regular resume. However the BT
> firmware was lost, so resume fails.
> For this case, maybe we can use btrtl_setup_realtek() in btusb_resume()? It
> won't re-upload firmware if firmware is retained, and will do proper
> initializing if firmware was lost.
In my opinions,
For the 3), there are two cases, one is that firmware was lost in auto suspend. That should never happen, because the data->intf->needs_remote_wakeup is set in btusb_open() and btusb_close(). The flag means that host will send remote wakeup during autosuspend, and firmware wouldn't drop.
Another case is firmware loss in global suspend. I think that's no problem, driver sets data->udev->reset_resume in btusb_suspend() and btusb will reprobe after resume.
>
> 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
> >>> <[email protected]> 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 <[email protected]>
> >>>> ---
> >>>> 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.
Hi Alex,
> On Sep 25, 2020, at 15:04, 陆朱伟 <[email protected]> wrote:
>
> Hi Kai-Heng,
>
>> On September 25, 2020 14:04, Kai-Heng Feng wrote:
>>
>> Hi Abhishek,
>>> On Sep 25, 2020, at 11:33, Abhishek Pandit-Subedi
>> <[email protected]> wrote:
>>>
>>> + Alex Lu (who contributed the original change)
>>>
>>> Hi Kai-Heng,
>>>
>>>
>>> On Thu, Sep 24, 2020 at 12:10 AM Kai-Heng Feng
>>> <[email protected]> wrote:
>>>>
>>>> [+Cc linux-usb]
>>>>
>>>> Hi Abhishek,
>>>>
>>>>> On Sep 24, 2020, at 04:41, Abhishek Pandit-Subedi
>> <[email protected]> wrote:
>>>>>
>>>>> Hi Kai-Heng,
>>>>>
>>>>> Which Realtek controller is this on?'
>>>>
>>>> The issue happens on 8821CE.
>>>>
>>>>>
>>>>> Specifically for RTL8822CE, we tested without reset_resume being set
>>>>> and that was causing the controller being reset without bluez ever
>>>>> learning about it (resulting in devices being unusable without
>>>>> toggling the BT power).
>>>>
>>>> The reset is done by the kernel, so how does that affect bluez?
>>>>
>>>> From what you described, it sounds more like runtime resume since bluez
>> is already running.
>>>> If we need reset resume for runtime resume, maybe it's another bug
>> which needs to be addressed?
>>>
>>> From btusb.c:
>> https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-
>> next.git/tree/drivers/bluetooth/btusb.c#n4189
>>> /* Realtek devices lose their updated firmware over global
>>> * suspend that means host doesn't send SET_FEATURE
>>> * (DEVICE_REMOTE_WAKEUP)
>>> */
>>>
>>> Runtime suspend always requires remote wakeup to be set and reset
>>> resume isn't used there.
>>
>> Thanks for the clarification.
>>
>>>
>>> During system suspend, when remote wakeup is not set, RTL8822CE loses
>>> the FW loaded by the driver and any state currently in the controller.
>>> This causes the kernel and the controller state to go out of sync.
>>> One of the issues we observed on the Realtek controller without the
>>> reset resume quirk was that paired or connected devices would just
>>> stop working after resume.
>>>
>>>>
>>>>> If the firmware doesn't cut off power during suspend, maybe you
>>>>> shouldn't set the BTUSB_WAKEUP_DISABLE flag for that controller.
>>>>
>>>> We don't know beforehand if the platform firmware (BIOS for my case)
>> will cut power off or not.
>>>>
>>>> In general, laptops will cut off the USB power during S3.
>>>> When AC is plugged, some laptops cuts USB power off and some don't.
>> This also applies to many desktops. Not to mention there can be BIOS
>> options to control USB power under S3/S4/S5...
>>>>
>>>> So we don't know beforehand.
>>>>
>>>
>>> I think the confusion here stems from what is actually being turned
>>> off between our two boards and what we're referring to as firmware :)
>>
>> Yes :)
>>
>>>
>>> In your case, the Realtek controller retains firmware unless the
>>> platform cuts of power to USB (which it does during S3).
>>
>> Not all platform firmware (i.e. BIOS for x86) cut USB power during S3, as I
>> describe in last reply.
>>
>>> In my case, the Realtek controller loses firmware when Remote Wakeup
>>> isn't set, even if the platform doesn't cut power to USB.
>>
>> Thanks for the clarification, I believe it's a case that should to be handled
>> separately.
>>
>>>
>>> In your case, since you don't need to enforce the 'Remote Wakeup' bit,
>>> if you unset the BTUSB_WAKEUP_DISABLE for that VID:PID, you should get
>>> the desirable behavior (which is actually the default behavior; remote
>>> wake will always be asserted instead of only during Runtime Suspend).
>>
>> So we have three cases here. Assuming reset_resume isn't flagged by btusb:
>>
>> 1) Both USB power and BT firmware were lost during suspend.
>> USB core finds out power was lost, try to reset resume the device. Since
>> btusb doesn't have reset_resume callback, USB core calls probe instead.
>>
>> 2) Both USB power and BT firmware were kept during suspend. This is my
>> case.
>> Regular resume handles everything.
>>
>> 3) USB power was kept but BT firmware was lost. This is your case.
>> USB core finds out power was kept, use regular resume. However the BT
>> firmware was lost, so resume fails.
>> For this case, maybe we can use btrtl_setup_realtek() in btusb_resume()? It
>> won't re-upload firmware if firmware is retained, and will do proper
>> initializing if firmware was lost.
>
> In my opinions,
> For the 3), there are two cases, one is that firmware was lost in auto suspend. That should never happen, because the data->intf->needs_remote_wakeup is set in btusb_open() and btusb_close(). The flag means that host will send remote wakeup during autosuspend, and firmware wouldn't drop.
> Another case is firmware loss in global suspend. I think that's no problem, driver sets data->udev->reset_resume in btusb_suspend() and btusb will reprobe after resume.
Apparently for my case, RTL8821CE, firmware was kept without setting remote wakeup.
Is it okay to also set remote wakeup for global suspend to retain the firmware?
If firmware was retained, does USB warm reset affect BT controller in anyway?
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
>>>>> <[email protected]> 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 <[email protected]>
>>>>>> ---
>>>>>> 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.
Hi Kai-Heng,
> On 25 September 2020 at 15:14, Kai-Heng Feng wrote:
>
> Hi Alex,
>
> > On Sep 25, 2020, at 15:04, 陆朱伟 <[email protected]> wrote:
> >
> > Hi Kai-Heng,
> >
> >> On September 25, 2020 14:04, Kai-Heng Feng wrote:
> >>
> >> Hi Abhishek,
> >>> On Sep 25, 2020, at 11:33, Abhishek Pandit-Subedi
> >> <[email protected]> wrote:
> >>>
> >>> + Alex Lu (who contributed the original change)
> >>>
> >>> Hi Kai-Heng,
> >>>
> >>>
> >>> On Thu, Sep 24, 2020 at 12:10 AM Kai-Heng Feng
> >>> <[email protected]> wrote:
> >>>>
> >>>> [+Cc linux-usb]
> >>>>
> >>>> Hi Abhishek,
> >>>>
> >>>>> On Sep 24, 2020, at 04:41, Abhishek Pandit-Subedi
> >> <[email protected]> wrote:
> >>>>>
> >>>>> Hi Kai-Heng,
> >>>>>
> >>>>> Which Realtek controller is this on?'
> >>>>
> >>>> The issue happens on 8821CE.
> >>>>
> >>>>>
> >>>>> Specifically for RTL8822CE, we tested without reset_resume being set
> >>>>> and that was causing the controller being reset without bluez ever
> >>>>> learning about it (resulting in devices being unusable without
> >>>>> toggling the BT power).
> >>>>
> >>>> The reset is done by the kernel, so how does that affect bluez?
> >>>>
> >>>> From what you described, it sounds more like runtime resume since
> bluez
> >> is already running.
> >>>> If we need reset resume for runtime resume, maybe it's another bug
> >> which needs to be addressed?
> >>>
> >>> From btusb.c:
> >> https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-
> >> next.git/tree/drivers/bluetooth/btusb.c#n4189
> >>> /* Realtek devices lose their updated firmware over global
> >>> * suspend that means host doesn't send SET_FEATURE
> >>> * (DEVICE_REMOTE_WAKEUP)
> >>> */
> >>>
> >>> Runtime suspend always requires remote wakeup to be set and reset
> >>> resume isn't used there.
> >>
> >> Thanks for the clarification.
> >>
> >>>
> >>> During system suspend, when remote wakeup is not set, RTL8822CE
> loses
> >>> the FW loaded by the driver and any state currently in the controller.
> >>> This causes the kernel and the controller state to go out of sync.
> >>> One of the issues we observed on the Realtek controller without the
> >>> reset resume quirk was that paired or connected devices would just
> >>> stop working after resume.
> >>>
> >>>>
> >>>>> If the firmware doesn't cut off power during suspend, maybe you
> >>>>> shouldn't set the BTUSB_WAKEUP_DISABLE flag for that controller.
> >>>>
> >>>> We don't know beforehand if the platform firmware (BIOS for my case)
> >> will cut power off or not.
> >>>>
> >>>> In general, laptops will cut off the USB power during S3.
> >>>> When AC is plugged, some laptops cuts USB power off and some don't.
> >> This also applies to many desktops. Not to mention there can be BIOS
> >> options to control USB power under S3/S4/S5...
> >>>>
> >>>> So we don't know beforehand.
> >>>>
> >>>
> >>> I think the confusion here stems from what is actually being turned
> >>> off between our two boards and what we're referring to as firmware :)
> >>
> >> Yes :)
> >>
> >>>
> >>> In your case, the Realtek controller retains firmware unless the
> >>> platform cuts of power to USB (which it does during S3).
> >>
> >> Not all platform firmware (i.e. BIOS for x86) cut USB power during S3, as I
> >> describe in last reply.
> >>
> >>> In my case, the Realtek controller loses firmware when Remote Wakeup
> >>> isn't set, even if the platform doesn't cut power to USB.
> >>
> >> Thanks for the clarification, I believe it's a case that should to be handled
> >> separately.
> >>
> >>>
> >>> In your case, since you don't need to enforce the 'Remote Wakeup' bit,
> >>> if you unset the BTUSB_WAKEUP_DISABLE for that VID:PID, you should
> get
> >>> the desirable behavior (which is actually the default behavior; remote
> >>> wake will always be asserted instead of only during Runtime Suspend).
> >>
> >> So we have three cases here. Assuming reset_resume isn't flagged by
> btusb:
> >>
> >> 1) Both USB power and BT firmware were lost during suspend.
> >> USB core finds out power was lost, try to reset resume the device. Since
> >> btusb doesn't have reset_resume callback, USB core calls probe instead.
> >>
> >> 2) Both USB power and BT firmware were kept during suspend. This is my
> >> case.
> >> Regular resume handles everything.
> >>
> >> 3) USB power was kept but BT firmware was lost. This is your case.
> >> USB core finds out power was kept, use regular resume. However the BT
> >> firmware was lost, so resume fails.
> >> For this case, maybe we can use btrtl_setup_realtek() in btusb_resume()?
> It
> >> won't re-upload firmware if firmware is retained, and will do proper
> >> initializing if firmware was lost.
> >
> > In my opinions,
> > For the 3), there are two cases, one is that firmware was lost in auto
> suspend. That should never happen, because the data->intf-
> >needs_remote_wakeup is set in btusb_open() and btusb_close(). The flag
> means that host will send remote wakeup during autosuspend, and firmware
> wouldn't drop.
> > Another case is firmware loss in global suspend. I think that's no problem,
> driver sets data->udev->reset_resume in btusb_suspend() and btusb will
> reprobe after resume.
>
> 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 ?
> Is it okay to also set remote wakeup for global suspend to retain the
> firmware?
Yes, it's ok.
> 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.
>
> 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
> >>>>> <[email protected]> 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 <[email protected]>
> >>>>>> ---
> >>>>>> 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.
Hi Alex,
> On Sep 25, 2020, at 15:42, 陆朱伟 <[email protected]> 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
>>>>>>> <[email protected]> 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 <[email protected]>
>>>>>>>> ---
>>>>>>>> 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.
Hi Kai-Heng,
> On September 25, 2020 at 15:56, Kai-Heng Feng wrote:
>
> Hi Alex,
>
> > On Sep 25, 2020, at 15:42, 陆朱伟 <[email protected]> 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?
It depends on your desire on power consumption during global suspend.
The BT controller takes less power if firmware was lost during global suspend.
>
> >
> >> 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
> >>>>>>> <[email protected]> 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 <[email protected]>
> >>>>>>>> ---
> >>>>>>>> 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.
Hi Alex,
> On Sep 25, 2020, at 16:23, 陆朱伟 <[email protected]> wrote:
>
> Hi Kai-Heng,
>
>> On September 25, 2020 at 15:56, Kai-Heng Feng wrote:
>>
>> Hi Alex,
>>
>>> On Sep 25, 2020, at 15:42, 陆朱伟 <[email protected]> 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?
>
> 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
>
>>
>>>
>>>> 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
>>>>>>>>> <[email protected]> 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 <[email protected]>
>>>>>>>>>> ---
>>>>>>>>>> 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.
>