> On Sep 25, 2020, at 14:40, 陆朱伟 <alex_lu@xxxxxxxxxxxxxx> 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 >> <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. 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 >>>> <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
