Hi, On Fri, Jan 29, 2021 at 6:28 AM Hans de Goede <hdegoede@xxxxxxxxxx> wrote: > > Hi, > > On 1/29/21 9:25 AM, Max Chou wrote: > > Hi, > > > >> I'm not sure we should enable this for all RTL devices rather than based on the specific project. RTL8822C will also be using hci_h5 but intends to support wake on bt (meaning it shouldn't be losing firmware around suspend). > > > >> + Max Chou: You are proposing a change to add project id to btrtl. > >> Should we use that instead to set this quirk for 8723 devices (and others which lose fw around suspend)? > >> (https://patchwork.kernel.org/project/bluetooth/patch/20210127030152.3940-1-max.chou@xxxxxxxxxxx/) > > > > Agree. Recommend to use the same way as Abhishek mentions that limiting the quirk only for RTL8723B devices if this patch is necessary. > > Therefore, some of the projects use RTL8822C devices with BT UART interface would apply BT wakes Host. > > So far, I've not heard the issue as this topic. > > So I just checked because I was not aware that the hci_h5 code was also being used for the RTL8822C. > I mainly focus on x86/ACPI use and there is no 8822 ACPI device-id in the h5_acpi_match table, but > there is indeed a match for this in the rtl_bluetooth_of_match table. > > But ATM the code is treating the 8822 exactly the same as the 8723, including doing a device_reprobe > on resume. So to me it makes sense to set the HCI_UART_NO_SUSPEND_NOTIFIER unconditionally to, as > it is used because of the device_reprobe being done. > > Now it might be a good idea to opt out of the device_reprobe for 8822 devices, and/or maybe even for > all devicetree enumerated cases (the device being completely shutoff is an ACPI thing, with dt/of we > should have more control). > > To me it seems that since for now the device_reprobe() is unconditional that the matching setting > of the HCI_UART_NO_SUSPEND_NOTIFIER flag should be unconditional too; and then when the device_reprobe() > stuff is made unconditional, then we can make the setting of the flag unconditional using the same check. I didn't realize this was currently unconditional in code. In that case, I think it's fine for you to add the flag unconditionally in hci_h5. When we add support for wake on bt to the h5 driver, we should make this flag conditional based on whether the driver reprobes on resume. I will go back and +1 the original patch. Thanks, Abhishek > > With that said if people really want it I'm happy to respin this to only apply to the 8723 case, > but that seems weird given that the device_reprobe ATM is being done on the 8822 too. > > > +Clair: Have you met this issue during suspend/resume when BT controller is RTL8822C with BT UART interface? > > Please see the patchwork. https://patchwork.kernel.org/project/bluetooth/list/?series=423915 > > Regards, > > Hans >
