diff mbox series

[v1] Bluetooth: Set missing suspend task bits

Message ID 20201204111038.v1.1.I4557a89427f61427e65d85bc51cca9e65607488e@changeid
State New
Headers show
Series [v1] Bluetooth: Set missing suspend task bits | expand

Commit Message

Howard Chung Dec. 4, 2020, 3:14 a.m. UTC
From: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>

When suspending, mark SUSPEND_SCAN_ENABLE and SUSPEND_SCAN_DISABLE tasks
correctly when either classic or le scanning is modified.

Signed-off-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
Signed-off-by: Howard Chung <howardchung@google.com>
Reviewed-by: Alain Michaud <alainm@chromium.org>
---

 net/bluetooth/hci_request.c | 8 ++++++++
 1 file changed, 8 insertions(+)

Comments

Dmitry Osipenko Dec. 21, 2020, 2:35 p.m. UTC | #1
04.12.2020 06:14, Howard Chung пишет:
> From: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>

> 

> When suspending, mark SUSPEND_SCAN_ENABLE and SUSPEND_SCAN_DISABLE tasks

> correctly when either classic or le scanning is modified.

> 

> Signed-off-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>

> Signed-off-by: Howard Chung <howardchung@google.com>

> Reviewed-by: Alain Michaud <alainm@chromium.org>

> ---

> 

>  net/bluetooth/hci_request.c | 8 ++++++++

>  1 file changed, 8 insertions(+)

> 

> diff --git a/net/bluetooth/hci_request.c b/net/bluetooth/hci_request.c

> index 80dc451d6e124..71bffd7454720 100644

> --- a/net/bluetooth/hci_request.c

> +++ b/net/bluetooth/hci_request.c

> @@ -707,6 +707,9 @@ void hci_req_add_le_scan_disable(struct hci_request *req, bool rpa_le_conn)

>  		return;

>  	}

>  

> +	if (hdev->suspended)

> +		set_bit(SUSPEND_SCAN_DISABLE, hdev->suspend_tasks);

> +

>  	if (use_ext_scan(hdev)) {

>  		struct hci_cp_le_set_ext_scan_enable cp;

>  

> @@ -1159,6 +1162,11 @@ static void hci_req_set_event_filter(struct hci_request *req)

>  		scan = SCAN_PAGE;

>  	}

>  

> +	if (scan)

> +		set_bit(SUSPEND_SCAN_ENABLE, hdev->suspend_tasks);

> +	else

> +		set_bit(SUSPEND_SCAN_DISABLE, hdev->suspend_tasks);

> +

>  	hci_req_add(req, HCI_OP_WRITE_SCAN_ENABLE, 1, &scan);

>  }

>  

> 


Hi,

This commit caused a regression on entering into suspend for Broadcom
Bluetooth 4330 on Nexus 7:

 Bluetooth: hci0: Timed out waiting for suspend events
 Bluetooth: hci0: Suspend timeout bit: 4
 Bluetooth: hci0: Suspend notifier action (3) failed: -110

I don't see this problem using BCM4329 chip on another device.

Please fix, thanks in advance.
Abhishek Pandit-Subedi Dec. 21, 2020, 5:58 p.m. UTC | #2
Hi Dmitry,

https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git/commit/?id=295fa2a5647b13681594bb1bcc76c74619035218
should fix this issue.

Your issue seems the same as the one I encountered -- the
SUSPEND_DISABLE bit (0x4) wasn't being cleared by the request
completion handler.

Thanks,
Abhishek

On Mon, Dec 21, 2020 at 6:35 AM Dmitry Osipenko <digetx@gmail.com> wrote:
>

> 04.12.2020 06:14, Howard Chung пишет:

> > From: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>

> >

> > When suspending, mark SUSPEND_SCAN_ENABLE and SUSPEND_SCAN_DISABLE tasks

> > correctly when either classic or le scanning is modified.

> >

> > Signed-off-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>

> > Signed-off-by: Howard Chung <howardchung@google.com>

> > Reviewed-by: Alain Michaud <alainm@chromium.org>

> > ---

> >

> >  net/bluetooth/hci_request.c | 8 ++++++++

> >  1 file changed, 8 insertions(+)

> >

> > diff --git a/net/bluetooth/hci_request.c b/net/bluetooth/hci_request.c

> > index 80dc451d6e124..71bffd7454720 100644

> > --- a/net/bluetooth/hci_request.c

> > +++ b/net/bluetooth/hci_request.c

> > @@ -707,6 +707,9 @@ void hci_req_add_le_scan_disable(struct hci_request *req, bool rpa_le_conn)

> >               return;

> >       }

> >

> > +     if (hdev->suspended)

> > +             set_bit(SUSPEND_SCAN_DISABLE, hdev->suspend_tasks);

> > +

> >       if (use_ext_scan(hdev)) {

> >               struct hci_cp_le_set_ext_scan_enable cp;

> >

> > @@ -1159,6 +1162,11 @@ static void hci_req_set_event_filter(struct hci_request *req)

> >               scan = SCAN_PAGE;

> >       }

> >

> > +     if (scan)

> > +             set_bit(SUSPEND_SCAN_ENABLE, hdev->suspend_tasks);

> > +     else

> > +             set_bit(SUSPEND_SCAN_DISABLE, hdev->suspend_tasks);

> > +

> >       hci_req_add(req, HCI_OP_WRITE_SCAN_ENABLE, 1, &scan);

> >  }

> >

> >

>

> Hi,

>

> This commit caused a regression on entering into suspend for Broadcom

> Bluetooth 4330 on Nexus 7:

>

>  Bluetooth: hci0: Timed out waiting for suspend events

>  Bluetooth: hci0: Suspend timeout bit: 4

>  Bluetooth: hci0: Suspend notifier action (3) failed: -110

>

> I don't see this problem using BCM4329 chip on another device.

>

> Please fix, thanks in advance.
Dmitry Osipenko Dec. 21, 2020, 6:19 p.m. UTC | #3
21.12.2020 20:58, Abhishek Pandit-Subedi пишет:
> Hi Dmitry,

> 

> https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git/commit/?id=295fa2a5647b13681594bb1bcc76c74619035218

> should fix this issue.

> 

> Your issue seems the same as the one I encountered -- the

> SUSPEND_DISABLE bit (0x4) wasn't being cleared by the request

> completion handler.


Hello Abhishek,

It fixes the problem using today's linux-next, which already includes
that commit, thank you.
diff mbox series

Patch

diff --git a/net/bluetooth/hci_request.c b/net/bluetooth/hci_request.c
index 80dc451d6e124..71bffd7454720 100644
--- a/net/bluetooth/hci_request.c
+++ b/net/bluetooth/hci_request.c
@@ -707,6 +707,9 @@  void hci_req_add_le_scan_disable(struct hci_request *req, bool rpa_le_conn)
 		return;
 	}
 
+	if (hdev->suspended)
+		set_bit(SUSPEND_SCAN_DISABLE, hdev->suspend_tasks);
+
 	if (use_ext_scan(hdev)) {
 		struct hci_cp_le_set_ext_scan_enable cp;
 
@@ -1159,6 +1162,11 @@  static void hci_req_set_event_filter(struct hci_request *req)
 		scan = SCAN_PAGE;
 	}
 
+	if (scan)
+		set_bit(SUSPEND_SCAN_ENABLE, hdev->suspend_tasks);
+	else
+		set_bit(SUSPEND_SCAN_DISABLE, hdev->suspend_tasks);
+
 	hci_req_add(req, HCI_OP_WRITE_SCAN_ENABLE, 1, &scan);
 }