In theory the firmware is supposed to power off the bluetooth card
when we use rfkill to block it. This doesn't work on a lot of laptops
though, leading to weird issues after turning off bluetooth, like the
connection timing out on the peripherals which were connected, and
bluetooth not connecting properly when the adapter is turned on again
quickly after rfkilling.
This series hooks into the rfkill driver from the bluetooth subsystem
to send a HCI_POWER_OFF command to the adapter before actually submitting
the rfkill to the firmware and killing the HCI connection.
Jonas Dreßler (4):
Bluetooth: HCI: Remove HCI_POWER_OFF_TIMEOUT
Bluetooth: mgmt: Remove leftover queuing of power_off work
Bluetooth: HCI: Add new state HCI_POWERING_DOWN
hci: Queue a HCI power-off command before rfkilling adapters
include/net/bluetooth/hci.h | 2 +-
net/bluetooth/hci_core.c | 33 ++++++++++++++++++++++++++++++---
net/bluetooth/hci_sync.c | 16 +++++++++++-----
net/bluetooth/mgmt.c | 30 ++++++++++++++----------------
4 files changed, 56 insertions(+), 25 deletions(-)
--
2.43.0
Queuing of power_off work was introduced in these functions with commits
8b064a3ad377c016a17e74f676e7a204c2b8c9f2 and
c9910d0fb4fc2ede468b26d45a1d50c309897770 in an effort to clean up state
and do things like disconnecting devices before actually powering off
the device.
After that, commit a3172b7eb4a2719711187cfca12097d2326e85a7 introduced a
timeout to ensure that the device actually got powered off, even if some
of the cleanup work would never complete.
This code later got refactored with commit
cf75ad8b41d2aa06f98f365d42a3ae8b059daddd, which made powering off the
device synchronous and removed the need for initiating the power_off
work from other places. The timeout mentioned above got removed too,
because we now also made use of the command timeout during power on/off.
These days the power_off work still exists, but it only seems to only be
used for HCI_AUTO_OFF functionality, which is why we never noticed
those two leftover places where we queue power_off work. So let's remove
that code.
Signed-off-by: Jonas Dreßler <[email protected]>
---
net/bluetooth/mgmt.c | 16 ----------------
1 file changed, 16 deletions(-)
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index d4498037f..c5291e139 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -9760,14 +9760,6 @@ void mgmt_device_disconnected(struct hci_dev *hdev, bdaddr_t *bdaddr,
struct mgmt_ev_device_disconnected ev;
struct sock *sk = NULL;
- /* The connection is still in hci_conn_hash so test for 1
- * instead of 0 to know if this is the last one.
- */
- if (mgmt_powering_down(hdev) && hci_conn_count(hdev) == 1) {
- cancel_delayed_work(&hdev->power_off);
- queue_work(hdev->req_workqueue, &hdev->power_off.work);
- }
-
if (!mgmt_connected)
return;
@@ -9824,14 +9816,6 @@ void mgmt_connect_failed(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 link_type,
{
struct mgmt_ev_connect_failed ev;
- /* The connection is still in hci_conn_hash so test for 1
- * instead of 0 to know if this is the last one.
- */
- if (mgmt_powering_down(hdev) && hci_conn_count(hdev) == 1) {
- cancel_delayed_work(&hdev->power_off);
- queue_work(hdev->req_workqueue, &hdev->power_off.work);
- }
-
bacpy(&ev.addr.bdaddr, bdaddr);
ev.addr.type = link_to_bdaddr(link_type, addr_type);
ev.status = mgmt_status(status);
--
2.43.0
On a lot of platforms (at least the MS Surface devices, M1 macbooks, and
a few ThinkPads) firmware doesn't do its job when rfkilling a device
and the bluetooth adapter is not actually shut down on rfkill. This leads
to connected devices remaining in connected state and the bluetooth
connection eventually timing out after rfkilling an adapter.
Use the rfkill hook in the HCI driver to actually power the device off
before rfkilling it.
Note that the wifi subsystem is doing something similar by calling
cfg80211_shutdown_all_interfaces()
in it's rfkill set_block callback (see cfg80211_rfkill_set_block).
Signed-off-by: Jonas Dreßler <[email protected]>
---
net/bluetooth/hci_core.c | 33 ++++++++++++++++++++++++++++++---
1 file changed, 30 insertions(+), 3 deletions(-)
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 1ec83985f..1c91d02f7 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -543,6 +543,23 @@ int hci_dev_open(__u16 dev)
return err;
}
+static int set_powered_off_sync(struct hci_dev *hdev, void *data)
+{
+ return hci_set_powered_sync(hdev, false);
+}
+
+static void set_powered_off_sync_complete(struct hci_dev *hdev, void *data, int err)
+{
+ if (err)
+ bt_dev_err(hdev, "Powering HCI device off before rfkilling failed (%d)", err);
+}
+
+static int hci_dev_do_poweroff(struct hci_dev *hdev)
+{
+ return hci_cmd_sync_queue(hdev, set_powered_off_sync,
+ NULL, set_powered_off_sync_complete);
+}
+
int hci_dev_do_close(struct hci_dev *hdev)
{
int err;
@@ -943,17 +960,27 @@ int hci_get_dev_info(void __user *arg)
static int hci_rfkill_set_block(void *data, bool blocked)
{
struct hci_dev *hdev = data;
+ int err;
BT_DBG("%p name %s blocked %d", hdev, hdev->name, blocked);
if (hci_dev_test_flag(hdev, HCI_USER_CHANNEL))
return -EBUSY;
+ if (blocked == hci_dev_test_flag(hdev, HCI_RFKILLED))
+ return 0;
+
if (blocked) {
- hci_dev_set_flag(hdev, HCI_RFKILLED);
if (!hci_dev_test_flag(hdev, HCI_SETUP) &&
- !hci_dev_test_flag(hdev, HCI_CONFIG))
- hci_dev_do_close(hdev);
+ !hci_dev_test_flag(hdev, HCI_CONFIG)) {
+ err = hci_dev_do_poweroff(hdev);
+ if (err) {
+ bt_dev_err(hdev, "Powering off device before rfkilling failed (%d)",
+ err);
+ }
+ }
+
+ hci_dev_set_flag(hdev, HCI_RFKILLED);
} else {
hci_dev_clear_flag(hdev, HCI_RFKILLED);
}
--
2.43.0
Add a new state HCI_POWERING_DOWN that indicates that the device is
currently powering down, this will be useful for the next commit.
Signed-off-by: Jonas Dreßler <[email protected]>
---
include/net/bluetooth/hci.h | 1 +
net/bluetooth/hci_sync.c | 16 +++++++++++-----
net/bluetooth/mgmt.c | 14 ++++++++++++++
3 files changed, 26 insertions(+), 5 deletions(-)
diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index cf5d6230c..e08afd870 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -361,6 +361,7 @@ enum {
HCI_SETUP,
HCI_CONFIG,
HCI_DEBUGFS_CREATED,
+ HCI_POWERING_DOWN,
HCI_AUTO_OFF,
HCI_RFKILLED,
HCI_MGMT,
diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
index e6eee1808..c920de0a2 100644
--- a/net/bluetooth/hci_sync.c
+++ b/net/bluetooth/hci_sync.c
@@ -5389,27 +5389,33 @@ static int hci_power_off_sync(struct hci_dev *hdev)
if (!test_bit(HCI_UP, &hdev->flags))
return 0;
+ hci_dev_set_flag(hdev, HCI_POWERING_DOWN);
+
if (test_bit(HCI_ISCAN, &hdev->flags) ||
test_bit(HCI_PSCAN, &hdev->flags)) {
err = hci_write_scan_enable_sync(hdev, 0x00);
if (err)
- return err;
+ goto out;
}
err = hci_clear_adv_sync(hdev, NULL, false);
if (err)
- return err;
+ goto out;
err = hci_stop_discovery_sync(hdev);
if (err)
- return err;
+ goto out;
/* Terminated due to Power Off */
err = hci_disconnect_all_sync(hdev, HCI_ERROR_REMOTE_POWER_OFF);
if (err)
- return err;
+ goto out;
+
+ err = hci_dev_close_sync(hdev);
- return hci_dev_close_sync(hdev);
+out:
+ hci_dev_clear_flag(hdev, HCI_POWERING_DOWN);
+ return err;
}
int hci_set_powered_sync(struct hci_dev *hdev, u8 val)
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index c5291e139..8f42ee059 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -1382,6 +1382,14 @@ static int set_powered(struct sock *sk, struct hci_dev *hdev, void *data,
hci_dev_lock(hdev);
+ if (!cp->val) {
+ if (hci_dev_test_flag(hdev, HCI_POWERING_DOWN)) {
+ err = mgmt_cmd_status(sk, hdev->id, MGMT_OP_SET_POWERED,
+ MGMT_STATUS_BUSY);
+ goto failed;
+ }
+ }
+
if (pending_find(MGMT_OP_SET_POWERED, hdev)) {
err = mgmt_cmd_status(sk, hdev->id, MGMT_OP_SET_POWERED,
MGMT_STATUS_BUSY);
@@ -9742,6 +9750,9 @@ bool mgmt_powering_down(struct hci_dev *hdev)
struct mgmt_pending_cmd *cmd;
struct mgmt_mode *cp;
+ if (hci_dev_test_flag(hdev, HCI_POWERING_DOWN))
+ return true;
+
cmd = pending_find(MGMT_OP_SET_POWERED, hdev);
if (!cmd)
return false;
@@ -10049,6 +10060,9 @@ void mgmt_set_local_name_complete(struct hci_dev *hdev, u8 *name, u8 status)
/* If this is a HCI command related to powering on the
* HCI dev don't send any mgmt signals.
*/
+ if (hci_dev_test_flag(hdev, HCI_POWERING_DOWN))
+ return;
+
if (pending_find(MGMT_OP_SET_POWERED, hdev))
return;
}
--
2.43.0
With commit cf75ad8b41d2aa06f98f365d42a3ae8b059daddd, the power off
sequence got refactored so that this was no longer necessary, let's
remove the leftover define from the header too.
Signed-off-by: Jonas Dreßler <[email protected]>
---
include/net/bluetooth/hci.h | 1 -
1 file changed, 1 deletion(-)
diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index 111e8f8e5..cf5d6230c 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -426,7 +426,6 @@ enum {
#define HCI_NCMD_TIMEOUT msecs_to_jiffies(4000) /* 4 seconds */
#define HCI_ACL_TX_TIMEOUT msecs_to_jiffies(45000) /* 45 seconds */
#define HCI_AUTO_OFF_TIMEOUT msecs_to_jiffies(2000) /* 2 seconds */
-#define HCI_POWER_OFF_TIMEOUT msecs_to_jiffies(5000) /* 5 seconds */
#define HCI_LE_CONN_TIMEOUT msecs_to_jiffies(20000) /* 20 seconds */
#define HCI_LE_AUTOCONN_TIMEOUT msecs_to_jiffies(4000) /* 4 seconds */
--
2.43.0
This is automated email and please do not reply to this email!
Dear submitter,
Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=813840
---Test result---
Test Summary:
CheckPatch PASS 5.24 seconds
GitLint PASS 1.32 seconds
SubjectPrefix FAIL 0.75 seconds
BuildKernel PASS 27.69 seconds
CheckAllWarning PASS 30.59 seconds
CheckSparse PASS 36.30 seconds
CheckSmatch PASS 99.67 seconds
BuildKernel32 PASS 26.96 seconds
TestRunnerSetup PASS 432.67 seconds
TestRunner_l2cap-tester PASS 22.79 seconds
TestRunner_iso-tester PASS 40.66 seconds
TestRunner_bnep-tester PASS 6.78 seconds
TestRunner_mgmt-tester PASS 160.29 seconds
TestRunner_rfcomm-tester PASS 10.76 seconds
TestRunner_sco-tester PASS 14.47 seconds
TestRunner_ioctl-tester PASS 11.93 seconds
TestRunner_mesh-tester PASS 8.73 seconds
TestRunner_smp-tester PASS 9.68 seconds
TestRunner_userchan-tester PASS 7.19 seconds
IncrementalBuild PASS 60.21 seconds
Details
##############################
Test: SubjectPrefix - FAIL
Desc: Check subject contains "Bluetooth" prefix
Output:
"Bluetooth: " prefix is not specified in the subject
---
Regards,
Linux Bluetooth