[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>
---
 drivers/bluetooth/btusb.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

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!