Hi Hans,
Thanks for the pointers. I'll try to spin up a patch to fix these
issues (or ask for some help on my team).
Thanks,
Abhishek
On Thu, Jan 28, 2021 at 4:00 AM Hans de Goede <hdegoede@xxxxxxxxxx> wrote:
>
> Hi All,
>
> While debugging an rtl8723bs bluetooth issue I noticed that last year
> the bluetooth core has grown a new hci_suspend_notifier() mechanism and
> I noticed a number of possible issues / improvements with that mechanism
> which I noticed:
>
> 1. HCI_OP_WRITE_SCAN_ENABLE gets send to the HCI in some places without
> a hci_dev_test_flag(hdev, HCI_BREDR_ENABLED) check
>
> 2. HCI_OP_SET_EVENT_FLT gets end to the HCI in some places without
> a hci_dev_test_flag(hdev, HCI_BREDR_ENABLED) check
>
> 3. hci_req_set_event_filter sends the following requests:
> 1 HCI_OP_SET_EVENT_FLT
> 2 HCI_OP_WRITE_SCAN_ENABLE (if the scan parameters have changed only)
> 3 HCI_OP_SET_EVENT_FLT (if their are relevant whitelist entries_
> 4 HCI_OP_WRITE_SCAN_ENABLE unconditionally
>
> It seems to me that 2. is unnecessary since it will immediately be
> followed by 4. and 4. misses the check to see if the scan parameters
> need updating which 2 does (this check is done in __hci_req_update_scan()
>
>
> 4. There is another unconditional, possibly unnecessary HCI_OP_WRITE_SCAN_ENABLE
> in the if (next == BT_SUSPEND_DISCONNECT) {} block of hci_req_prepare_suspend()
> Note if this is made conditional then the:
> set_bit(SUSPEND_SCAN_DISABLE, hdev->suspend_tasks);
> should also be made conditional since then req might not contain any requests
> in which case suspend_req_complete() will not run.
>
> Regards,
>
> Hans
>
