| Message ID | 20241001152137.3071690-1-luiz.dentz@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 81b3e33bb054c09a36d2acf84e144746338a5739 |
| Headers | show |
| Series | [RESEND,v3] Bluetooth: btusb: Don't fail external suspend requests | expand |
| Context | Check | Description |
|---|---|---|
| tedd_an/pre-ci_am | success | Success |
| tedd_an/CheckPatch | success | CheckPatch PASS |
| tedd_an/GitLint | success | Gitlint PASS |
| tedd_an/SubjectPrefix | success | Gitlint PASS |
| tedd_an/BuildKernel | success | BuildKernel PASS |
| tedd_an/CheckAllWarning | success | CheckAllWarning PASS |
| tedd_an/CheckSparse | success | CheckSparse PASS |
Hi Luiz, >-----Original Message----- >From: Luiz Augusto von Dentz <luiz.dentz@gmail.com> >Sent: Tuesday, October 1, 2024 8:52 PM >To: linux-bluetooth@vger.kernel.org >Subject: [RESEND v3] Bluetooth: btusb: Don't fail external suspend requests > >From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> > >Commit 4e0a1d8b0675 >("Bluetooth: btusb: Don't suspend when there are connections") introduces a >check for connections to prevent auto-suspend but that actually ignored the >fact the .suspend callback can be called for external suspend requests which >Documentation/driver-api/usb/power-management.rst states the following: > > 'External suspend calls should never be allowed to fail in this way, only >autosuspend calls. The driver can tell them apart by applying the >:c:func:`PMSG_IS_AUTO` macro to the message argument to the ``suspend`` >method; it will return True for internal PM events > (autosuspend) and False for external PM events.' > >In addition to that align system suspend with USB suspend by using >hci_suspend_dev since otherwise the stack would be expecting events such as >advertising reports which may not be delivered while the transport is >suspended. > >Fixes: 4e0a1d8b0675 ("Bluetooth: btusb: Don't suspend when there are >connections") >Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> Tested-by: Kiran K <kiran.k@intel.com> >--- > drivers/bluetooth/btusb.c | 20 ++++++++++++++++++-- > 1 file changed, 18 insertions(+), 2 deletions(-) > >diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c index >4d3eea414c2a..d14b941bfde8 100644 >--- a/drivers/bluetooth/btusb.c >+++ b/drivers/bluetooth/btusb.c >@@ -4075,16 +4075,29 @@ static void btusb_disconnect(struct usb_interface >*intf) static int btusb_suspend(struct usb_interface *intf, pm_message_t >message) { > struct btusb_data *data = usb_get_intfdata(intf); >+ int err; > > BT_DBG("intf %p", intf); > >- /* Don't suspend if there are connections */ >- if (hci_conn_count(data->hdev)) >+ /* Don't auto-suspend if there are connections; external suspend calls >+ * shall never fail. >+ */ >+ if (PMSG_IS_AUTO(message) && hci_conn_count(data->hdev)) > return -EBUSY; > > if (data->suspend_count++) > return 0; > >+ /* Notify Host stack to suspend; this has to be done before stopping >+ * the traffic since the hci_suspend_dev itself may generate some >+ * traffic. >+ */ >+ err = hci_suspend_dev(data->hdev); >+ if (err) { >+ data->suspend_count--; >+ return err; >+ } >+ > spin_lock_irq(&data->txlock); > if (!(PMSG_IS_AUTO(message) && data->tx_in_flight)) { > set_bit(BTUSB_SUSPENDING, &data->flags); @@ -4092,6 >+4105,7 @@ static int btusb_suspend(struct usb_interface *intf, >pm_message_t message) > } else { > spin_unlock_irq(&data->txlock); > data->suspend_count--; >+ hci_resume_dev(data->hdev); > return -EBUSY; > } > >@@ -4212,6 +4226,8 @@ static int btusb_resume(struct usb_interface *intf) > spin_unlock_irq(&data->txlock); > schedule_work(&data->work); > >+ hci_resume_dev(data->hdev); >+ > return 0; > > failed: >-- >2.46.1 > Thanks, Kiran
Hello: This patch was applied to bluetooth/bluetooth-next.git (master) by Luiz Augusto von Dentz <luiz.von.dentz@intel.com>: On Tue, 1 Oct 2024 11:21:37 -0400 you wrote: > From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> > > Commit 4e0a1d8b0675 > ("Bluetooth: btusb: Don't suspend when there are connections") > introduces a check for connections to prevent auto-suspend but that > actually ignored the fact the .suspend callback can be called for > external suspend requests which > Documentation/driver-api/usb/power-management.rst states the following: > > [...] Here is the summary with links: - [RESEND,v3] Bluetooth: btusb: Don't fail external suspend requests https://git.kernel.org/bluetooth/bluetooth-next/c/81b3e33bb054 You are awesome, thank you!
diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c index 4d3eea414c2a..d14b941bfde8 100644 --- a/drivers/bluetooth/btusb.c +++ b/drivers/bluetooth/btusb.c @@ -4075,16 +4075,29 @@ static void btusb_disconnect(struct usb_interface *intf) static int btusb_suspend(struct usb_interface *intf, pm_message_t message) { struct btusb_data *data = usb_get_intfdata(intf); + int err; BT_DBG("intf %p", intf); - /* Don't suspend if there are connections */ - if (hci_conn_count(data->hdev)) + /* Don't auto-suspend if there are connections; external suspend calls + * shall never fail. + */ + if (PMSG_IS_AUTO(message) && hci_conn_count(data->hdev)) return -EBUSY; if (data->suspend_count++) return 0; + /* Notify Host stack to suspend; this has to be done before stopping + * the traffic since the hci_suspend_dev itself may generate some + * traffic. + */ + err = hci_suspend_dev(data->hdev); + if (err) { + data->suspend_count--; + return err; + } + spin_lock_irq(&data->txlock); if (!(PMSG_IS_AUTO(message) && data->tx_in_flight)) { set_bit(BTUSB_SUSPENDING, &data->flags); @@ -4092,6 +4105,7 @@ static int btusb_suspend(struct usb_interface *intf, pm_message_t message) } else { spin_unlock_irq(&data->txlock); data->suspend_count--; + hci_resume_dev(data->hdev); return -EBUSY; } @@ -4212,6 +4226,8 @@ static int btusb_resume(struct usb_interface *intf) spin_unlock_irq(&data->txlock); schedule_work(&data->work); + hci_resume_dev(data->hdev); + return 0; failed: