Bluetooth: Always set event mask on suspend

Commit Message

Abhishek Pandit-Subedi July 25, 2022, 8:50 p.m. UTC

From: Abhishek Pandit-Subedi <[e-mail]>

When suspending, always set the event mask once disconnects are
successful. Otherwise, if wakeup is disallowed, the event mask is not
set before suspend continues and can result in an early wakeup.

Signed-off-by: Abhishek Pandit-Subedi <[e-mail]>
---
Observed on ChromeOS as follows:

< HCI Command: Disconnect (0x01|0x0006) plen 3
        Handle: 256
        Reason: Remote Device Terminated due to Power Off (0x15)
> HCI Event: Command Status (0x0f) plen 4
      Disconnect (0x01|0x0006) ncmd 1
        Status: Success (0x00)
@ MGMT Event: Device Disconnected (0x000c) plen 8
        BR/EDR Address: 04:52:C7:C3:65:B5 (Bose Corporation)
        Reason: Connection terminated by local host for suspend (0x05)
@ MGMT Event: Controller Suspended (0x002d) plen 1
        Suspend state: Disconnected and not scanning (1)
> HCI Event: Disconnect Complete (0x05) plen 4
        Status: Success (0x00)
        Handle: 256
        Reason: Connection Terminated By Local Host (0x16)

The expectation is that we should see Set Event Mask before completing
the suspend so that the `Disconnect Complete` doesn't wake us up.


 net/bluetooth/hci_sync.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Comments

[e-mail] July 25, 2022, 10:06 p.m. UTC | #1

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=662855

---Test result---

Test Summary:
CheckPatch                    PASS      1.09 seconds
GitLint                       PASS      0.46 seconds
SubjectPrefix                 PASS      0.30 seconds
BuildKernel                   PASS      44.63 seconds
BuildKernel32                 PASS      38.89 seconds
Incremental Build with patchesPASS      53.29 seconds
TestRunner: Setup             PASS      648.91 seconds
TestRunner: l2cap-tester      PASS      21.11 seconds
TestRunner: bnep-tester       PASS      8.16 seconds
TestRunner: mgmt-tester       PASS      128.88 seconds
TestRunner: rfcomm-tester     PASS      12.28 seconds
TestRunner: sco-tester        PASS      11.97 seconds
TestRunner: smp-tester        PASS      11.86 seconds
TestRunner: userchan-tester   PASS      8.47 seconds



---
Regards,
Linux Bluetooth

Luiz Augusto von Dentz July 25, 2022, 10:28 p.m. UTC | #2

Hi Abhishek,

On Mon, Jul 25, 2022 at 1:50 PM Abhishek Pandit-Subedi
<[e-mail]> wrote:
>
> From: Abhishek Pandit-Subedi <[e-mail]>
>
> When suspending, always set the event mask once disconnects are
> successful. Otherwise, if wakeup is disallowed, the event mask is not
> set before suspend continues and can result in an early wakeup.
>

Please include the commit hash it fixes, also it may be a good idea to
tag it for stable as well.

> Signed-off-by: Abhishek Pandit-Subedi <[e-mail]>
> ---
> Observed on ChromeOS as follows:
>
> < HCI Command: Disconnect (0x01|0x0006) plen 3
>         Handle: 256
>         Reason: Remote Device Terminated due to Power Off (0x15)
> > HCI Event: Command Status (0x0f) plen 4
>       Disconnect (0x01|0x0006) ncmd 1
>         Status: Success (0x00)
> @ MGMT Event: Device Disconnected (0x000c) plen 8
>         BR/EDR Address: 04:52:C7:C3:65:B5 (Bose Corporation)
>         Reason: Connection terminated by local host for suspend (0x05)
> @ MGMT Event: Controller Suspended (0x002d) plen 1
>         Suspend state: Disconnected and not scanning (1)
> > HCI Event: Disconnect Complete (0x05) plen 4
>         Status: Success (0x00)
>         Handle: 256
>         Reason: Connection Terminated By Local Host (0x16)
>
> The expectation is that we should see Set Event Mask before completing
> the suspend so that the `Disconnect Complete` doesn't wake us up.
>
>
>  net/bluetooth/hci_sync.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
> index 148ce629a59f..e6d804b82b67 100644
> --- a/net/bluetooth/hci_sync.c
> +++ b/net/bluetooth/hci_sync.c
> @@ -5297,6 +5297,9 @@ int hci_suspend_sync(struct hci_dev *hdev)
>                 return err;
>         }
>
> +       /* Update event mask so only the allowed event can wakeup the host */
> +       hci_set_event_mask_sync(hdev);
> +
>         /* Only configure accept list if disconnect succeeded and wake
>          * isn't being prevented.
>          */
> @@ -5308,9 +5311,6 @@ int hci_suspend_sync(struct hci_dev *hdev)
>         /* Unpause to take care of updating scanning params */
>         hdev->scanning_paused = false;
>
> -       /* Update event mask so only the allowed event can wakeup the host */
> -       hci_set_event_mask_sync(hdev);
> -
>         /* Enable event filter for paired devices */
>         hci_update_event_filter_sync(hdev);
>
> --
> 2.37.1.359.gd136c6c3e2-goog
>

Patch

diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
index 148ce629a59f..e6d804b82b67 100644
--- a/net/bluetooth/hci_sync.c
+++ b/net/bluetooth/hci_sync.c
@@ -5297,6 +5297,9 @@  int hci_suspend_sync(struct hci_dev *hdev)
 		return err;
 	}
 
+	/* Update event mask so only the allowed event can wakeup the host */
+	hci_set_event_mask_sync(hdev);
+
 	/* Only configure accept list if disconnect succeeded and wake
 	 * isn't being prevented.
 	 */
@@ -5308,9 +5311,6 @@  int hci_suspend_sync(struct hci_dev *hdev)
 	/* Unpause to take care of updating scanning params */
 	hdev->scanning_paused = false;
 
-	/* Update event mask so only the allowed event can wakeup the host */
-	hci_set_event_mask_sync(hdev);
-
 	/* Enable event filter for paired devices */
 	hci_update_event_filter_sync(hdev);