Received-SPF: pass (google.com: domain of linux-bluetooth-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Hans de Goede <hdegoede@redhat.com>
To:     Marcel Holtmann <marcel@holtmann.org>,
        Johan Hedberg <johan.hedberg@gmail.com>,
        Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
Cc:     Hans de Goede <hdegoede@redhat.com>,
        linux-bluetooth@vger.kernel.org
Subject: Recent(ish) bluetooth hci_suspend_notifier() changes
Date:   Thu, 28 Jan 2021 13:00:07 +0100
Message-Id: <20210128120007.5676-1-hdegoede@redhat.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

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