* [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).