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 > <kai.heng.feng@xxxxxxxxxxxxx> wrote: > > > > [+Cc linux-usb] > > > > Hi Abhishek, > > > > > On Sep 24, 2020, at 04:41, Abhishek Pandit-Subedi > <abhishekpandit@xxxxxxxxxxxx> 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 > > > <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 > > >> > > >
