mbox series

[RFC,v6,0/5] Bluetooth: Handle system suspend gracefully

Message ID 20200311155404.209990-1-abhishekpandit@chromium.org (mailing list archive)
Headers show
Series Bluetooth: Handle system suspend gracefully | expand

Message

Abhishek Pandit-Subedi March 11, 2020, 3:53 p.m. UTC
Hi linux-bluetooth,

This patch series prepares the Bluetooth controller for system suspend
by disconnecting all devices and preparing the event filter and LE
whitelist with devices that can wake the system from suspend.

The main motivation for doing this is so we can enable Bluetooth as
a wake up source during suspend without it being noisy. Bluetooth should
wake the system when a HID device receives user input but otherwise not
send any events to the host.

This patch series was tested on several Chromebooks with both btusb and
hci_serdev on kernel 4.19. The set of tests was basically the following:
* Reconnects after suspend succeed
* HID devices can wake the system from suspend (needs some related bluez
  changes to call the Set Wake Capable management command)
* System properly pauses and unpauses discovery + advertising around
  suspend
* System does not wake from any events from non wakeable devices

Series 2 has refactored the change into multiple smaller commits as
requested. I tried to simplify some of the whitelist filtering edge
cases but unfortunately it remains quite complex.

Series 3 has refactored it further and should have resolved the
whitelisting complexity in series 2.

Series 4 adds a fix to check for powered down and powering down adapters.

Series 5 moves set_wake_capable to the last patch in the series and
changes BT_DBG to bt_dev_dbg.

Please review and provide any feedback.

Thanks
Abhishek


Changes in v6:
* Removed unused variables in hci_req_prepare_suspend
* Add int old_state to this patch

Changes in v5:
* Convert BT_DBG to bt_dev_dbg
* Added wakeable list and changed BT_DBG to bt_dev_dbg
* Add wakeable to hci_conn_params and change BT_DBG to bt_dev_dbg
* Changed BT_DBG to bt_dev_dbg
* Wakeable entries moved to other commits
* Patch moved to end of series

Changes in v4:
* Added check for mgmt_powering_down and hdev_is_powered in notifier

Changes in v3:
* Refactored to only handle BR/EDR devices
* Split LE changes into its own commit
* Added wakeable property to le_conn_param
* Use wakeable list for BR/EDR and wakeable property for LE

Changes in v2:
* Moved pm notifier registration into its own patch and moved params out
  of separate suspend_state
* Refactored filters and whitelist settings to its own patch
* Refactored update_white_list to have clearer edge cases
* Add connected devices to whitelist (previously missing corner case)
* Refactored pause discovery + advertising into its own patch

Abhishek Pandit-Subedi (5):
  Bluetooth: Handle PM_SUSPEND_PREPARE and PM_POST_SUSPEND
  Bluetooth: Handle BR/EDR devices during suspend
  Bluetooth: Handle LE devices during suspend
  Bluetooth: Pause discovery and advertising during suspend
  Bluetooth: Add mgmt op set_wake_capable

 include/net/bluetooth/hci.h      |  17 +-
 include/net/bluetooth/hci_core.h |  43 ++++
 include/net/bluetooth/mgmt.h     |   7 +
 net/bluetooth/hci_core.c         | 102 ++++++++++
 net/bluetooth/hci_event.c        |  24 +++
 net/bluetooth/hci_request.c      | 331 ++++++++++++++++++++++++++-----
 net/bluetooth/hci_request.h      |   2 +
 net/bluetooth/mgmt.c             |  92 +++++++++
 8 files changed, 558 insertions(+), 60 deletions(-)

Comments

Marcel Holtmann March 11, 2020, 5:05 p.m. UTC | #1
Hi Abhishek,

> This patch series prepares the Bluetooth controller for system suspend
> by disconnecting all devices and preparing the event filter and LE
> whitelist with devices that can wake the system from suspend.
> 
> The main motivation for doing this is so we can enable Bluetooth as
> a wake up source during suspend without it being noisy. Bluetooth should
> wake the system when a HID device receives user input but otherwise not
> send any events to the host.
> 
> This patch series was tested on several Chromebooks with both btusb and
> hci_serdev on kernel 4.19. The set of tests was basically the following:
> * Reconnects after suspend succeed
> * HID devices can wake the system from suspend (needs some related bluez
>  changes to call the Set Wake Capable management command)
> * System properly pauses and unpauses discovery + advertising around
>  suspend
> * System does not wake from any events from non wakeable devices
> 
> Series 2 has refactored the change into multiple smaller commits as
> requested. I tried to simplify some of the whitelist filtering edge
> cases but unfortunately it remains quite complex.
> 
> Series 3 has refactored it further and should have resolved the
> whitelisting complexity in series 2.
> 
> Series 4 adds a fix to check for powered down and powering down adapters.
> 
> Series 5 moves set_wake_capable to the last patch in the series and
> changes BT_DBG to bt_dev_dbg.
> 
> Please review and provide any feedback.
> 
> Thanks
> Abhishek
> 
> 
> Changes in v6:
> * Removed unused variables in hci_req_prepare_suspend
> * Add int old_state to this patch
> 
> Changes in v5:
> * Convert BT_DBG to bt_dev_dbg
> * Added wakeable list and changed BT_DBG to bt_dev_dbg
> * Add wakeable to hci_conn_params and change BT_DBG to bt_dev_dbg
> * Changed BT_DBG to bt_dev_dbg
> * Wakeable entries moved to other commits
> * Patch moved to end of series
> 
> Changes in v4:
> * Added check for mgmt_powering_down and hdev_is_powered in notifier
> 
> Changes in v3:
> * Refactored to only handle BR/EDR devices
> * Split LE changes into its own commit
> * Added wakeable property to le_conn_param
> * Use wakeable list for BR/EDR and wakeable property for LE
> 
> Changes in v2:
> * Moved pm notifier registration into its own patch and moved params out
>  of separate suspend_state
> * Refactored filters and whitelist settings to its own patch
> * Refactored update_white_list to have clearer edge cases
> * Add connected devices to whitelist (previously missing corner case)
> * Refactored pause discovery + advertising into its own patch
> 
> Abhishek Pandit-Subedi (5):
>  Bluetooth: Handle PM_SUSPEND_PREPARE and PM_POST_SUSPEND
>  Bluetooth: Handle BR/EDR devices during suspend
>  Bluetooth: Handle LE devices during suspend
>  Bluetooth: Pause discovery and advertising during suspend
>  Bluetooth: Add mgmt op set_wake_capable
> 
> include/net/bluetooth/hci.h      |  17 +-
> include/net/bluetooth/hci_core.h |  43 ++++
> include/net/bluetooth/mgmt.h     |   7 +
> net/bluetooth/hci_core.c         | 102 ++++++++++
> net/bluetooth/hci_event.c        |  24 +++
> net/bluetooth/hci_request.c      | 331 ++++++++++++++++++++++++++-----
> net/bluetooth/hci_request.h      |   2 +
> net/bluetooth/mgmt.c             |  92 +++++++++
> 8 files changed, 558 insertions(+), 60 deletions(-)

patches 1-4 have been applied to bluetooth-next tree.

I skipped patch 5 since now we have to discuss how best the API for setting the wakeable devices will be. Care to start up a discussion thread for that?

Regards

Marcel
Abhishek Pandit-Subedi March 11, 2020, 5:20 p.m. UTC | #2
Let's continue the discussion on the bluez list.

I will update the patch series so that the "set_wake_capable"
management change is last in the series there as well and send out an
update today.

Thanks
Abhishek

On Wed, Mar 11, 2020 at 10:05 AM Marcel Holtmann <marcel@holtmann.org> wrote:
>
> Hi Abhishek,
>
> > This patch series prepares the Bluetooth controller for system suspend
> > by disconnecting all devices and preparing the event filter and LE
> > whitelist with devices that can wake the system from suspend.
> >
> > The main motivation for doing this is so we can enable Bluetooth as
> > a wake up source during suspend without it being noisy. Bluetooth should
> > wake the system when a HID device receives user input but otherwise not
> > send any events to the host.
> >
> > This patch series was tested on several Chromebooks with both btusb and
> > hci_serdev on kernel 4.19. The set of tests was basically the following:
> > * Reconnects after suspend succeed
> > * HID devices can wake the system from suspend (needs some related bluez
> >  changes to call the Set Wake Capable management command)
> > * System properly pauses and unpauses discovery + advertising around
> >  suspend
> > * System does not wake from any events from non wakeable devices
> >
> > Series 2 has refactored the change into multiple smaller commits as
> > requested. I tried to simplify some of the whitelist filtering edge
> > cases but unfortunately it remains quite complex.
> >
> > Series 3 has refactored it further and should have resolved the
> > whitelisting complexity in series 2.
> >
> > Series 4 adds a fix to check for powered down and powering down adapters.
> >
> > Series 5 moves set_wake_capable to the last patch in the series and
> > changes BT_DBG to bt_dev_dbg.
> >
> > Please review and provide any feedback.
> >
> > Thanks
> > Abhishek
> >
> >
> > Changes in v6:
> > * Removed unused variables in hci_req_prepare_suspend
> > * Add int old_state to this patch
> >
> > Changes in v5:
> > * Convert BT_DBG to bt_dev_dbg
> > * Added wakeable list and changed BT_DBG to bt_dev_dbg
> > * Add wakeable to hci_conn_params and change BT_DBG to bt_dev_dbg
> > * Changed BT_DBG to bt_dev_dbg
> > * Wakeable entries moved to other commits
> > * Patch moved to end of series
> >
> > Changes in v4:
> > * Added check for mgmt_powering_down and hdev_is_powered in notifier
> >
> > Changes in v3:
> > * Refactored to only handle BR/EDR devices
> > * Split LE changes into its own commit
> > * Added wakeable property to le_conn_param
> > * Use wakeable list for BR/EDR and wakeable property for LE
> >
> > Changes in v2:
> > * Moved pm notifier registration into its own patch and moved params out
> >  of separate suspend_state
> > * Refactored filters and whitelist settings to its own patch
> > * Refactored update_white_list to have clearer edge cases
> > * Add connected devices to whitelist (previously missing corner case)
> > * Refactored pause discovery + advertising into its own patch
> >
> > Abhishek Pandit-Subedi (5):
> >  Bluetooth: Handle PM_SUSPEND_PREPARE and PM_POST_SUSPEND
> >  Bluetooth: Handle BR/EDR devices during suspend
> >  Bluetooth: Handle LE devices during suspend
> >  Bluetooth: Pause discovery and advertising during suspend
> >  Bluetooth: Add mgmt op set_wake_capable
> >
> > include/net/bluetooth/hci.h      |  17 +-
> > include/net/bluetooth/hci_core.h |  43 ++++
> > include/net/bluetooth/mgmt.h     |   7 +
> > net/bluetooth/hci_core.c         | 102 ++++++++++
> > net/bluetooth/hci_event.c        |  24 +++
> > net/bluetooth/hci_request.c      | 331 ++++++++++++++++++++++++++-----
> > net/bluetooth/hci_request.h      |   2 +
> > net/bluetooth/mgmt.c             |  92 +++++++++
> > 8 files changed, 558 insertions(+), 60 deletions(-)
>
> patches 1-4 have been applied to bluetooth-next tree.
>
> I skipped patch 5 since now we have to discuss how best the API for setting the wakeable devices will be. Care to start up a discussion thread for that?
>
> Regards
>
> Marcel
>
youling 257 April 9, 2020, 5:14 p.m. UTC | #3
This patch series cause a suspend problem on BYT/CHT device, https://bugzilla.kernel.org/show_bug.cgi?id=207169