linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] Bluetooth: Add helpers to enable or disable LE Advertising
@ 2019-04-22  9:20 Kai-Heng Feng
  2019-04-22  9:20 ` [PATCH 2/2] Bluetooth: btusb: Disable LE Advertising on system suspend Kai-Heng Feng
  0 siblings, 1 reply; 5+ messages in thread
From: Kai-Heng Feng @ 2019-04-22  9:20 UTC (permalink / raw)
  To: marcel, johan.hedberg; +Cc: linux-bluetooth, linux-kernel, Kai-Heng Feng

This patch adds helpers to enable or disable LE Advertising.
To be used by later patch.

Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
---
 include/net/bluetooth/hci_core.h |  3 ++
 net/bluetooth/hci_core.c         | 47 ++++++++++++++++++++++++++++++++
 2 files changed, 50 insertions(+)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index e5ea633ea368..ef92dd12f816 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -269,6 +269,7 @@ struct hci_dev {
 	__u16		le_max_rx_time;
 	__u8		le_max_key_size;
 	__u8		le_min_key_size;
+	__u8		le_events[8];
 	__u16		discov_interleaved_timeout;
 	__u16		conn_info_min_age;
 	__u16		conn_info_max_age;
@@ -1141,6 +1142,8 @@ void hci_init_sysfs(struct hci_dev *hdev);
 void hci_conn_init_sysfs(struct hci_conn *conn);
 void hci_conn_add_sysfs(struct hci_conn *conn);
 void hci_conn_del_sysfs(struct hci_conn *conn);
+int hci_enable_le_advertising(struct hci_dev *hdev);
+int hci_disable_le_advertising(struct hci_dev *hdev);
 
 #define SET_HCIDEV_DEV(hdev, pdev) ((hdev)->dev.parent = (pdev))
 
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 7352fe85674b..0bed66908588 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -411,6 +412,53 @@ static void hci_setup_event_mask(struct hci_request *req)
 	hci_req_add(req, HCI_OP_SET_EVENT_MASK, sizeof(events), events);
 }
 
+static int hci_enable_le_advertising_req(struct hci_request *req, unsigned long opt)
+{
+	struct hci_dev *hdev = req->hdev;
+	u8 events[8];
+
+	memcpy(events, hdev->le_events, sizeof(events));
+
+	hci_req_add(req, HCI_OP_LE_SET_EVENT_MASK, sizeof(events),
+		    events);
+
+	return 0;
+}
+
+static int hci_disable_le_advertising_req(struct hci_request *req, unsigned long opt)
+{
+	struct hci_dev *hdev = req->hdev;
+
+	u8 events[8];
+
+	memcpy(events, hdev->le_events, sizeof(events));
+
+	events[0] &= ~(u8)0x02;	/* LE Advertising Report */
+
+	hci_req_add(req, HCI_OP_LE_SET_EVENT_MASK, sizeof(events),
+		    events);
+
+	return 0;
+}
+
+int hci_enable_le_advertising(struct hci_dev *hdev)
+{
+	if (!lmp_le_capable(hdev))
+		return 0;
+
+	return __hci_req_sync(hdev, hci_enable_le_advertising_req, 0, HCI_CMD_TIMEOUT, NULL);
+}
+EXPORT_SYMBOL(hci_enable_le_advertising);
+
+int hci_disable_le_advertising(struct hci_dev *hdev)
+{
+	if (!lmp_le_capable(hdev))
+		return 0;
+
+	return __hci_req_sync(hdev, hci_disable_le_advertising_req, 0, HCI_CMD_TIMEOUT, NULL);
+}
+EXPORT_SYMBOL(hci_disable_le_advertising);
+
 static int hci_init2_req(struct hci_request *req, unsigned long opt)
 {
 	struct hci_dev *hdev = req->hdev;
@@ -771,6 +818,8 @@ static int hci_init3_req(struct hci_request *req, unsigned long opt)
 		}
 
 		hci_set_le_support(req);
+
+		memcpy(hdev->le_events, events, sizeof(events));
 	}
 
 	/* Read features beyond page 1 if available */
-- 
2.17.1


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

* [PATCH 2/2] Bluetooth: btusb: Disable LE Advertising on system suspend
  2019-04-22  9:20 [PATCH 1/2] Bluetooth: Add helpers to enable or disable LE Advertising Kai-Heng Feng
@ 2019-04-22  9:20 ` Kai-Heng Feng
  2019-04-23 16:28   ` Marcel Holtmann
  0 siblings, 1 reply; 5+ messages in thread
From: Kai-Heng Feng @ 2019-04-22  9:20 UTC (permalink / raw)
  To: marcel, johan.hedberg; +Cc: linux-bluetooth, linux-kernel, Kai-Heng Feng

System may freeze during suspend, and it's caused by btusb early wakeup:

kernel: pci_pm_suspend(): hcd_pci_suspend+0x0/0x30 returns -16
kernel: dpm_run_callback(): pci_pm_suspend+0x0/0x130 returns -16
kernel: PM: Device 0000:00:14.0 failed to suspend async: error -16
kernel: PM: Some devices failed to suspend, or early wake event detected
kernel: usb usb1: usb resume
kernel: hub 1-0:1.0: hub_resume
kernel: usb usb1-port1: status 0507 change 0000
kernel: usb usb1-port6: status 0103 change 0004
kernel: usb usb1-port10: status 0107 change 0000

where btusb is connecte to usb1-port6.

The expirement shows that the early wakeup is caused by LE Advertising
packet.

Disabling it via event mask can prevent the issue from happening.

BugLink: https://bugs.launchpad.net/bugs/1823029
Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
---
 drivers/bluetooth/btusb.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index 10c8f9872ee5..f03fcf5687e4 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -490,6 +490,7 @@ struct btusb_data {
 	int (*setup_on_usb)(struct hci_dev *hdev);
 
 	int oob_wake_irq;   /* irq for out-of-band wake-on-bt */
+	bool suspended;
 };
 
 static inline void btusb_free_frags(struct btusb_data *data)
@@ -3316,12 +3317,18 @@ 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);
+	struct hci_dev *hdev = data->hdev;
 
 	BT_DBG("intf %p", intf);
 
 	if (data->suspend_count++)
 		return 0;
 
+	if (!PMSG_IS_AUTO(message)) {
+		hci_disable_le_advertising(hdev);
+		data->suspended = true;
+	}
+
 	spin_lock_irq(&data->txlock);
 	if (!(PMSG_IS_AUTO(message) && data->tx_in_flight)) {
 		set_bit(BTUSB_SUSPENDING, &data->flags);
@@ -3427,6 +3434,11 @@ static int btusb_resume(struct usb_interface *intf)
 	spin_unlock_irq(&data->txlock);
 	schedule_work(&data->work);
 
+	if (data->suspended) {
+		hci_enable_le_advertising(hdev);
+		data->suspended = false;
+	}
+
 	return 0;
 
 failed:
-- 
2.17.1


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

* Re: [PATCH 2/2] Bluetooth: btusb: Disable LE Advertising on system suspend
  2019-04-22  9:20 ` [PATCH 2/2] Bluetooth: btusb: Disable LE Advertising on system suspend Kai-Heng Feng
@ 2019-04-23 16:28   ` Marcel Holtmann
  2019-04-25 16:27     ` Kai Heng Feng
  0 siblings, 1 reply; 5+ messages in thread
From: Marcel Holtmann @ 2019-04-23 16:28 UTC (permalink / raw)
  To: Kai-Heng Feng; +Cc: Johan Hedberg, linux-bluetooth, linux-kernel

Hi Kai-Heng,

> System may freeze during suspend, and it's caused by btusb early wakeup:
> 
> kernel: pci_pm_suspend(): hcd_pci_suspend+0x0/0x30 returns -16
> kernel: dpm_run_callback(): pci_pm_suspend+0x0/0x130 returns -16
> kernel: PM: Device 0000:00:14.0 failed to suspend async: error -16
> kernel: PM: Some devices failed to suspend, or early wake event detected
> kernel: usb usb1: usb resume
> kernel: hub 1-0:1.0: hub_resume
> kernel: usb usb1-port1: status 0507 change 0000
> kernel: usb usb1-port6: status 0103 change 0004
> kernel: usb usb1-port10: status 0107 change 0000
> 
> where btusb is connecte to usb1-port6.
> 
> The expirement shows that the early wakeup is caused by LE Advertising
> packet.
> 
> Disabling it via event mask can prevent the issue from happening.
> 
> BugLink: https://bugs.launchpad.net/bugs/1823029
> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
> ---
> drivers/bluetooth/btusb.c | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
> 
> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> index 10c8f9872ee5..f03fcf5687e4 100644
> --- a/drivers/bluetooth/btusb.c
> +++ b/drivers/bluetooth/btusb.c
> @@ -490,6 +490,7 @@ struct btusb_data {
> 	int (*setup_on_usb)(struct hci_dev *hdev);
> 
> 	int oob_wake_irq;   /* irq for out-of-band wake-on-bt */
> +	bool suspended;
> };
> 
> static inline void btusb_free_frags(struct btusb_data *data)
> @@ -3316,12 +3317,18 @@ 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);
> +	struct hci_dev *hdev = data->hdev;
> 
> 	BT_DBG("intf %p", intf);
> 
> 	if (data->suspend_count++)
> 		return 0;
> 
> +	if (!PMSG_IS_AUTO(message)) {
> +		hci_disable_le_advertising(hdev);
> +		data->suspended = true;
> +	}
> +
> 	spin_lock_irq(&data->txlock);
> 	if (!(PMSG_IS_AUTO(message) && data->tx_in_flight)) {
> 		set_bit(BTUSB_SUSPENDING, &data->flags);
> @@ -3427,6 +3434,11 @@ static int btusb_resume(struct usb_interface *intf)
> 	spin_unlock_irq(&data->txlock);
> 	schedule_work(&data->work);
> 
> +	if (data->suspended) {
> +		hci_enable_le_advertising(hdev);
> +		data->suspended = false;
> +	}
> +
> 	return 0;

this is a clear NAK. Please stop hacking things.

Lets use hci_suspend_dev and hci_resume_dev and make it actually do something to disable the events for advertising.

Regards

Marcel


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

* Re: [PATCH 2/2] Bluetooth: btusb: Disable LE Advertising on system suspend
  2019-04-23 16:28   ` Marcel Holtmann
@ 2019-04-25 16:27     ` Kai Heng Feng
  2019-04-25 16:35       ` Marcel Holtmann
  0 siblings, 1 reply; 5+ messages in thread
From: Kai Heng Feng @ 2019-04-25 16:27 UTC (permalink / raw)
  To: Marcel Holtmann; +Cc: Johan Hedberg, linux-bluetooth, linux-kernel

Hi Marcel,

> On Apr 24, 2019, at 12:28 AM, Marcel Holtmann <marcel@holtmann.org> wrote:
> 
> Hi Kai-Heng,
> 
>> System may freeze during suspend, and it's caused by btusb early wakeup:
>> 
>> kernel: pci_pm_suspend(): hcd_pci_suspend+0x0/0x30 returns -16
>> kernel: dpm_run_callback(): pci_pm_suspend+0x0/0x130 returns -16
>> kernel: PM: Device 0000:00:14.0 failed to suspend async: error -16
>> kernel: PM: Some devices failed to suspend, or early wake event detected
>> kernel: usb usb1: usb resume
>> kernel: hub 1-0:1.0: hub_resume
>> kernel: usb usb1-port1: status 0507 change 0000
>> kernel: usb usb1-port6: status 0103 change 0004
>> kernel: usb usb1-port10: status 0107 change 0000
>> 
>> where btusb is connecte to usb1-port6.
>> 
>> The expirement shows that the early wakeup is caused by LE Advertising
>> packet.
>> 
>> Disabling it via event mask can prevent the issue from happening.
>> 
>> BugLink: https://bugs.launchpad.net/bugs/1823029
>> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
>> ---
>> drivers/bluetooth/btusb.c | 12 ++++++++++++
>> 1 file changed, 12 insertions(+)
>> 
>> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
>> index 10c8f9872ee5..f03fcf5687e4 100644
>> --- a/drivers/bluetooth/btusb.c
>> +++ b/drivers/bluetooth/btusb.c
>> @@ -490,6 +490,7 @@ struct btusb_data {
>> 	int (*setup_on_usb)(struct hci_dev *hdev);
>> 
>> 	int oob_wake_irq;   /* irq for out-of-band wake-on-bt */
>> +	bool suspended;
>> };
>> 
>> static inline void btusb_free_frags(struct btusb_data *data)
>> @@ -3316,12 +3317,18 @@ 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);
>> +	struct hci_dev *hdev = data->hdev;
>> 
>> 	BT_DBG("intf %p", intf);
>> 
>> 	if (data->suspend_count++)
>> 		return 0;
>> 
>> +	if (!PMSG_IS_AUTO(message)) {
>> +		hci_disable_le_advertising(hdev);
>> +		data->suspended = true;
>> +	}
>> +
>> 	spin_lock_irq(&data->txlock);
>> 	if (!(PMSG_IS_AUTO(message) && data->tx_in_flight)) {
>> 		set_bit(BTUSB_SUSPENDING, &data->flags);
>> @@ -3427,6 +3434,11 @@ static int btusb_resume(struct usb_interface *intf)
>> 	spin_unlock_irq(&data->txlock);
>> 	schedule_work(&data->work);
>> 
>> +	if (data->suspended) {
>> +		hci_enable_le_advertising(hdev);
>> +		data->suspended = false;
>> +	}
>> +
>> 	return 0;
> 
> this is a clear NAK. Please stop hacking things.
> 
> Lets use hci_suspend_dev and hci_resume_dev and make it actually do something to disable the events for advertising.

Do you mean hci_disable_le_advertising() should be called by hci_suspend_dev(), which should be called by btusb_suspend()?

I’ve tried calling hci_suspend_dev() without disabling advertising, the issue still presents.

Kai-Heng 

> 
> Regards
> 
> Marcel


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

* Re: [PATCH 2/2] Bluetooth: btusb: Disable LE Advertising on system suspend
  2019-04-25 16:27     ` Kai Heng Feng
@ 2019-04-25 16:35       ` Marcel Holtmann
  0 siblings, 0 replies; 5+ messages in thread
From: Marcel Holtmann @ 2019-04-25 16:35 UTC (permalink / raw)
  To: Kai Heng Feng; +Cc: Johan Hedberg, linux-bluetooth, linux-kernel

Hi Kai-Heng,

>>> System may freeze during suspend, and it's caused by btusb early wakeup:
>>> 
>>> kernel: pci_pm_suspend(): hcd_pci_suspend+0x0/0x30 returns -16
>>> kernel: dpm_run_callback(): pci_pm_suspend+0x0/0x130 returns -16
>>> kernel: PM: Device 0000:00:14.0 failed to suspend async: error -16
>>> kernel: PM: Some devices failed to suspend, or early wake event detected
>>> kernel: usb usb1: usb resume
>>> kernel: hub 1-0:1.0: hub_resume
>>> kernel: usb usb1-port1: status 0507 change 0000
>>> kernel: usb usb1-port6: status 0103 change 0004
>>> kernel: usb usb1-port10: status 0107 change 0000
>>> 
>>> where btusb is connecte to usb1-port6.
>>> 
>>> The expirement shows that the early wakeup is caused by LE Advertising
>>> packet.
>>> 
>>> Disabling it via event mask can prevent the issue from happening.
>>> 
>>> BugLink: https://bugs.launchpad.net/bugs/1823029
>>> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
>>> ---
>>> drivers/bluetooth/btusb.c | 12 ++++++++++++
>>> 1 file changed, 12 insertions(+)
>>> 
>>> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
>>> index 10c8f9872ee5..f03fcf5687e4 100644
>>> --- a/drivers/bluetooth/btusb.c
>>> +++ b/drivers/bluetooth/btusb.c
>>> @@ -490,6 +490,7 @@ struct btusb_data {
>>> 	int (*setup_on_usb)(struct hci_dev *hdev);
>>> 
>>> 	int oob_wake_irq;   /* irq for out-of-band wake-on-bt */
>>> +	bool suspended;
>>> };
>>> 
>>> static inline void btusb_free_frags(struct btusb_data *data)
>>> @@ -3316,12 +3317,18 @@ 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);
>>> +	struct hci_dev *hdev = data->hdev;
>>> 
>>> 	BT_DBG("intf %p", intf);
>>> 
>>> 	if (data->suspend_count++)
>>> 		return 0;
>>> 
>>> +	if (!PMSG_IS_AUTO(message)) {
>>> +		hci_disable_le_advertising(hdev);
>>> +		data->suspended = true;
>>> +	}
>>> +
>>> 	spin_lock_irq(&data->txlock);
>>> 	if (!(PMSG_IS_AUTO(message) && data->tx_in_flight)) {
>>> 		set_bit(BTUSB_SUSPENDING, &data->flags);
>>> @@ -3427,6 +3434,11 @@ static int btusb_resume(struct usb_interface *intf)
>>> 	spin_unlock_irq(&data->txlock);
>>> 	schedule_work(&data->work);
>>> 
>>> +	if (data->suspended) {
>>> +		hci_enable_le_advertising(hdev);
>>> +		data->suspended = false;
>>> +	}
>>> +
>>> 	return 0;
>> 
>> this is a clear NAK. Please stop hacking things.
>> 
>> Lets use hci_suspend_dev and hci_resume_dev and make it actually do something to disable the events for advertising.
> 
> Do you mean hci_disable_le_advertising() should be called by hci_suspend_dev(), which should be called by btusb_suspend()?
> 
> I’ve tried calling hci_suspend_dev() without disabling advertising, the issue still presents.

we have to define what the behavior of hci_suspend_dev is suppose to be. In general you want to wake up from LE Advertising, but most likely only ones that are passing the whitelist. Same goes for BR/EDR and HID wakeups btw.

Anyway this is the way to go since I will not allow doing any hacking from a HCI transport driver. And that is what btusb.c is. It is just a transport, it is not suppose to know anything about HCI internals unless told from the core.

Regards

Marcel


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

end of thread, other threads:[~2019-04-25 16:35 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-22  9:20 [PATCH 1/2] Bluetooth: Add helpers to enable or disable LE Advertising Kai-Heng Feng
2019-04-22  9:20 ` [PATCH 2/2] Bluetooth: btusb: Disable LE Advertising on system suspend Kai-Heng Feng
2019-04-23 16:28   ` Marcel Holtmann
2019-04-25 16:27     ` Kai Heng Feng
2019-04-25 16:35       ` Marcel Holtmann

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).