Linux-Bluetooth Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v2] Bluetooth: btusb: Fix suspend issue for Realtek devices
@ 2019-08-02 12:02 Alex Lu
  2019-08-12 16:33 ` Marcel Holtmann
  0 siblings, 1 reply; 5+ messages in thread
From: Alex Lu @ 2019-08-02 12:02 UTC (permalink / raw)
  To: Marcel Holtmann; +Cc: Johan Hedberg, linux-bluetooth, linux-kernel, Max Chou

From: Alex Lu <alex_lu@realsil.com.cn>

From the perspective of controller, global suspend means there is no
SET_FEATURE (DEVICE_REMOTE_WAKEUP) and controller would drop the
firmware. It would consume less power. So we should not send this kind
of SET_FEATURE when host goes to suspend state.
Otherwise, when making device enter selective suspend, host should send
SET_FEATURE to make sure the firmware remains.

Signed-off-by: Alex Lu <alex_lu@realsil.com.cn>
---
 drivers/bluetooth/btusb.c | 34 ++++++++++++++++++++++++++++++----
 1 file changed, 30 insertions(+), 4 deletions(-)

diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index 50aed5259c2b..1995e26fa4cd 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -426,6 +426,7 @@ static const struct dmi_system_id btusb_needs_reset_resume_table[] = {
 #define BTUSB_DIAG_RUNNING	10
 #define BTUSB_OOB_WAKE_ENABLED	11
 #define BTUSB_HW_RESET_ACTIVE	12
+#define BTUSB_WAKEUP_DISABLE	13
 
 struct btusb_data {
 	struct hci_dev       *hdev;
@@ -1165,6 +1166,13 @@ static int btusb_open(struct hci_dev *hdev)
 	 */
 	device_wakeup_enable(&data->udev->dev);
 
+	/* Disable device remote wakeup when host is suspended
+	 * For Realtek chips, global suspend without
+	 * SET_FEATURE (DEVICE_REMOTE_WAKEUP) can save more power in device.
+	 */
+	if (test_bit(BTUSB_WAKEUP_DISABLE, &data->flags))
+		device_wakeup_disable(&data->udev->dev);
+
 	if (test_and_set_bit(BTUSB_INTR_RUNNING, &data->flags))
 		goto done;
 
@@ -1227,6 +1235,11 @@ static int btusb_close(struct hci_dev *hdev)
 		goto failed;
 
 	data->intf->needs_remote_wakeup = 0;
+
+	/* Enable remote wake up for auto-suspend */
+	if (test_bit(BTUSB_WAKEUP_DISABLE, &data->flags))
+		data->intf->needs_remote_wakeup = 1;
+
 	device_wakeup_disable(&data->udev->dev);
 	usb_autopm_put_interface(data->intf);
 
@@ -3185,11 +3198,11 @@ static int btusb_probe(struct usb_interface *intf,
 	if (id->driver_info & BTUSB_REALTEK) {
 		hdev->setup = btrtl_setup_realtek;
 
-		/* Realtek devices lose their updated firmware over suspend,
-		 * but the USB hub doesn't notice any status change.
-		 * Explicitly request a device reset on resume.
+		/* Realtek devices lose their updated firmware over global
+		 * suspend that means host doesn't send SET_FEATURE
+		 * (DEVICE_REMOTE_WAKEUP)
 		 */
-		interface_to_usbdev(intf)->quirks |= USB_QUIRK_RESET_RESUME;
+		set_bit(BTUSB_WAKEUP_DISABLE, &data->flags);
 	}
 #endif
 
@@ -3363,6 +3376,19 @@ static int btusb_suspend(struct usb_interface *intf, pm_message_t message)
 		enable_irq(data->oob_wake_irq);
 	}
 
+	/* For global suspend, Realtek devices lose the loaded fw
+	 * in them. But for autosuspend, firmware should remain.
+	 * Actually, it depends on whether the usb host sends
+	 * set feature (enable wakeup) or not.
+	 */
+	if (test_bit(BTUSB_WAKEUP_DISABLE, &data->flags)) {
+		if (PMSG_IS_AUTO(message) &&
+		    device_can_wakeup(&data->udev->dev))
+			data->udev->do_remote_wakeup = 1;
+		else if (!PMSG_IS_AUTO(message))
+			data->udev->reset_resume = 1;
+	}
+
 	return 0;
 }
 
-- 
2.19.2


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] Bluetooth: btusb: Fix suspend issue for Realtek devices
  2019-08-02 12:02 [PATCH v2] Bluetooth: btusb: Fix suspend issue for Realtek devices Alex Lu
@ 2019-08-12 16:33 ` Marcel Holtmann
  2019-08-12 21:46   ` Dmitry Torokhov
  2019-08-14  7:33   ` 陆朱伟
  0 siblings, 2 replies; 5+ messages in thread
From: Marcel Holtmann @ 2019-08-12 16:33 UTC (permalink / raw)
  To: Alex Lu; +Cc: Johan Hedberg, linux-bluetooth, linux-kernel, Max Chou

Hi Alex,

> From the perspective of controller, global suspend means there is no
> SET_FEATURE (DEVICE_REMOTE_WAKEUP) and controller would drop the
> firmware. It would consume less power. So we should not send this kind
> of SET_FEATURE when host goes to suspend state.
> Otherwise, when making device enter selective suspend, host should send
> SET_FEATURE to make sure the firmware remains.
> 
> Signed-off-by: Alex Lu <alex_lu@realsil.com.cn>
> ---
> drivers/bluetooth/btusb.c | 34 ++++++++++++++++++++++++++++++----
> 1 file changed, 30 insertions(+), 4 deletions(-)

this one doesn’t apply cleanly to bluetooth-next. Can you please send a version that does.

Regards

Marcel


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] Bluetooth: btusb: Fix suspend issue for Realtek devices
  2019-08-12 16:33 ` Marcel Holtmann
@ 2019-08-12 21:46   ` Dmitry Torokhov
  2019-08-14  8:03     ` 答复: " 陆朱伟
  2019-08-14  7:33   ` 陆朱伟
  1 sibling, 1 reply; 5+ messages in thread
From: Dmitry Torokhov @ 2019-08-12 21:46 UTC (permalink / raw)
  To: Marcel Holtmann; +Cc: Alex Lu, Johan Hedberg, linux-bluetooth, lkml, Max Chou

On Mon, Aug 12, 2019 at 9:36 AM Marcel Holtmann <marcel@holtmann.org> wrote:
>
> Hi Alex,
>
> > From the perspective of controller, global suspend means there is no
> > SET_FEATURE (DEVICE_REMOTE_WAKEUP) and controller would drop the
> > firmware. It would consume less power. So we should not send this kind
> > of SET_FEATURE when host goes to suspend state.
> > Otherwise, when making device enter selective suspend, host should send
> > SET_FEATURE to make sure the firmware remains.
> >
> > Signed-off-by: Alex Lu <alex_lu@realsil.com.cn>
> > ---
> > drivers/bluetooth/btusb.c | 34 ++++++++++++++++++++++++++++++----
> > 1 file changed, 30 insertions(+), 4 deletions(-)
>
> this one doesn’t apply cleanly to bluetooth-next. Can you please send a version that does.

Is this a chip issue or system issue? I.e. if in some system BT
controller is wired so that it loses power over system suspend, this
is quite different form chip itself losing firmware in certain
situations, and this smells like a system issue and thus needs to be
addressed on system level.

Thanks.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 5+ messages in thread

* 答复: [PATCH v2] Bluetooth: btusb: Fix suspend issue for Realtek devices
  2019-08-12 16:33 ` Marcel Holtmann
  2019-08-12 21:46   ` Dmitry Torokhov
@ 2019-08-14  7:33   ` 陆朱伟
  1 sibling, 0 replies; 5+ messages in thread
From: 陆朱伟 @ 2019-08-14  7:33 UTC (permalink / raw)
  To: Marcel Holtmann; +Cc: Johan Hedberg, linux-bluetooth, linux-kernel, Max Chou

Hi Marcel,
Okay, I will send a version for bluetooth-next.

Thanks,
BRs.

> Subject: Re: [PATCH v2] Bluetooth: btusb: Fix suspend issue for Realtek devices
> 
> Hi Alex,
> 
> > From the perspective of controller, global suspend means there is no
> > SET_FEATURE (DEVICE_REMOTE_WAKEUP) and controller would drop the
> > firmware. It would consume less power. So we should not send this kind
> > of SET_FEATURE when host goes to suspend state.
> > Otherwise, when making device enter selective suspend, host should send
> > SET_FEATURE to make sure the firmware remains.
> >
> > Signed-off-by: Alex Lu <alex_lu@realsil.com.cn>
> > ---
> > drivers/bluetooth/btusb.c | 34 ++++++++++++++++++++++++++++++----
> > 1 file changed, 30 insertions(+), 4 deletions(-)
> 
> this one doesn’t apply cleanly to bluetooth-next. Can you please send a
> version that does.
> 
> Regards
> 
> Marcel
> 
> 
> ------Please consider the environment before printing this e-mail.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* 答复: [PATCH v2] Bluetooth: btusb: Fix suspend issue for Realtek devices
  2019-08-12 21:46   ` Dmitry Torokhov
@ 2019-08-14  8:03     ` " 陆朱伟
  0 siblings, 0 replies; 5+ messages in thread
From: 陆朱伟 @ 2019-08-14  8:03 UTC (permalink / raw)
  To: Dmitry Torokhov, Marcel Holtmann
  Cc: Johan Hedberg, linux-bluetooth, lkml, Max Chou

Hi Dmitry,
It's only for Realtek devices.
If Realtek device firmware receives SET_FEATURE(device remote wakeup) usb cmd during usb is suspending, it will remains in suspend state.
Otherwise, firmware will drop itself and device will consume less power. But when host resumes, it needs to reload firmware. It can be accomplished by setting usb reset resume for device.

Thanks,
BRs.

> Subject: Re: [PATCH v2] Bluetooth: btusb: Fix suspend issue for Realtek devices
> 
> On Mon, Aug 12, 2019 at 9:36 AM Marcel Holtmann <marcel@holtmann.org>
> wrote:
> >
> > Hi Alex,
> >
> > > From the perspective of controller, global suspend means there is no
> > > SET_FEATURE (DEVICE_REMOTE_WAKEUP) and controller would drop
> the
> > > firmware. It would consume less power. So we should not send this kind
> > > of SET_FEATURE when host goes to suspend state.
> > > Otherwise, when making device enter selective suspend, host should
> send
> > > SET_FEATURE to make sure the firmware remains.
> > >
> > > Signed-off-by: Alex Lu <alex_lu@realsil.com.cn>
> > > ---
> > > drivers/bluetooth/btusb.c | 34 ++++++++++++++++++++++++++++++---
> -
> > > 1 file changed, 30 insertions(+), 4 deletions(-)
> >
> > this one doesn’t apply cleanly to bluetooth-next. Can you please send a
> version that does.
> 
> Is this a chip issue or system issue? I.e. if in some system BT
> controller is wired so that it loses power over system suspend, this
> is quite different form chip itself losing firmware in certain
> situations, and this smells like a system issue and thus needs to be
> addressed on system level.
> 
> Thanks.
> 
> --
> Dmitry
> 
> ------Please consider the environment before printing this e-mail.

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, back to index

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-02 12:02 [PATCH v2] Bluetooth: btusb: Fix suspend issue for Realtek devices Alex Lu
2019-08-12 16:33 ` Marcel Holtmann
2019-08-12 21:46   ` Dmitry Torokhov
2019-08-14  8:03     ` 答复: " 陆朱伟
2019-08-14  7:33   ` 陆朱伟

Linux-Bluetooth Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-bluetooth/0 linux-bluetooth/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-bluetooth linux-bluetooth/ https://lore.kernel.org/linux-bluetooth \
		linux-bluetooth@vger.kernel.org linux-bluetooth@archiver.kernel.org
	public-inbox-index linux-bluetooth


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-bluetooth


AGPL code for this site: git clone https://public-inbox.org/ public-inbox