Skip to contentSkip to conversation

Hide WakeAllowed when the adapter cannot wake the host

PATCH In review Matthew Schwartz 3 participants 6 messages Jul 25 – Jul 27 Bluetooth drivers

lore
Share & tools
lore
#0 lore raw

cover Hide WakeAllowed when the adapter cannot wake the host

The WakeAllowed property is currently exposed on any device whose profile supports wake configuration (HID/HoG), regardless of whether the host controller is configured to wake the system. Clients may decide whether to offer a "wake the system by this device" toggle based on the presence of this property, and end up offering it on hardware where it won't work.

This series gates the property's existence on the adapter's current wakeup configuration, read from the power/wakeup attribute of the closest USB ancestor in sysfs. That is the same attribute btusb consults through device_may_wakeup() on the underlying USB device, and hci_suspend_sync() skips wakeup configuration entirely when that callback returns false. Only the USB case is handled, so controllers on other buses keep the current behavior.

Matthew Schwartz (3):
adapter: Add btd_adapter_may_wake()
device: Hide WakeAllowed when adapter cannot wake
doc: Mark WakeAllowed as optional

 doc/org.bluez.Device.rst |  7 ++--
 src/adapter.c            | 70 ++++++++++++++++++++++++++++++++++++++++
 src/adapter.h            |  1 +
 src/device.c             |  5 ++-
 4 files changed, 80 insertions(+), 3 deletions(-)
Signature · 1 line
2.55.0
#1 lore raw

PATCH 1/3 adapter: Add btd_adapter_may_wake()

The kernel provides no interface for querying whether a controller is currently configured to wake the host from suspend. hci_register_dev() marks HCI_CONN_FLAG_REMOTE_WAKEUP as supported whenever a driver provides a wakeup callback, and btusb always does, so the supported flags reported through mgmt say nothing about the runtime setting. The value that matters, device_may_wakeup() on the underlying USB device, is only evaluated during the suspend flow, and the wakeup callback is not a pure query (btmtksdio's variant sends vendor HCI commands), so mgmt could not simply re-evaluate it on Get Device Flags without new kernel infrastructure.

Add btd_adapter_may_wake(), which reconstructs what btusb reports by walking the adapter's sysfs ancestry to the closest USB device and reading its power/wakeup attribute. A missing attribute means the device cannot generate wake events at all, which is what btusb arranges for the fake CSR clones by clearing the USB device's wakeup capability. The attribute is read per call so that each query reflects the current setting.

Only USB is handled. Controllers on other buses keep the current behavior. btmtksdio exposes the same attribute on its SDIO function device, but its wakeup callback can also depend on a vendor command.

Assisted-by Claude:claude-fable-5
---
 src/adapter.c | 70 +++++++++++++++++++++++++++++++++++++++++++++++++++
 src/adapter.h |  1 +
 2 files changed, 71 insertions(+)
src/adapter.c +70 -0
index a56eafabe..65b394f82 100644
@@ -563,6 +563,76 @@ uint8_t btd_adapter_get_address_type(struct btd_adapter *adapter)
 	return adapter->bdaddr_type;
 }
 
+static bool sysfs_wakeup_enabled(const char *dir)
+{
+	char path[PATH_MAX];
+	char *contents;
+	bool enabled;
+
+	snprintf(path, sizeof(path), "%s/power/wakeup", dir);
+
+	/* A missing attribute means the device cannot wake the host */
+	if (!g_file_get_contents(path, &contents, NULL, NULL))
+		return false;
+
+	enabled = g_str_has_prefix(contents, "enabled");
+
+	g_free(contents);
+
+	return enabled;
+}
+
+static bool sysfs_is_usb_device(const char *dir)
+{
+	char path[PATH_MAX];
+
+	/* USB devices expose idVendor, USB interfaces do not */
+	snprintf(path, sizeof(path), "%s/idVendor", dir);
+
+	return g_file_test(path, G_FILE_TEST_EXISTS);
+}
+
+/*
+ * Whether the controller is currently configured to wake the host from
+ * system suspend. btusb reports this to the kernel with
+ * device_may_wakeup() on the underlying USB device, which userspace
+ * controls through its power/wakeup attribute. Only USB is handled
+ * here. Controllers on other buses are assumed to be able to wake the
+ * host. The attribute is read on every call so that runtime changes
+ * are picked up.
+ */
+bool btd_adapter_may_wake(struct btd_adapter *adapter)
+{
+	char path[PATH_MAX];
+	char *dir;
+	char *sep;
+	bool may_wake = true;
+
+	snprintf(path, sizeof(path), "/sys/class/bluetooth/hci%u/device",
+							adapter->dev_id);
+
+	dir = realpath(path, NULL);
+	if (!dir)
+		return true;
+
+	while (g_str_has_prefix(dir, "/sys/devices/")) {
+		if (sysfs_is_usb_device(dir)) {
+			may_wake = sysfs_wakeup_enabled(dir);
+			break;
+		}
+
+		sep = strrchr(dir, '/');
+		if (!sep)
+			break;
+
+		*sep = '\0';
+	}
+
+	free(dir);
+
+	return may_wake;
+}
+
 static void store_adapter_info(struct btd_adapter *adapter)
 {
 	GKeyFile *key_file;
src/adapter.h +1 -0
index a9e1bbf66..ae0011ff7 100644
@@ -107,6 +107,7 @@ const char *adapter_get_path(struct btd_adapter *adapter);
 const bdaddr_t *btd_adapter_get_address(struct btd_adapter *adapter);
 uint8_t btd_adapter_get_address_type(struct btd_adapter *adapter);
 const char *btd_adapter_get_storage_dir(struct btd_adapter *adapter);
+bool btd_adapter_may_wake(struct btd_adapter *adapter);
 
 int adapter_service_add(struct btd_adapter *adapter, sdp_record_t *rec);
 void adapter_service_remove(struct btd_adapter *adapter, uint32_t handle);
Signature · 1 line
2.55.0
#2 lore raw

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=1134469

---Test result---

Test Summary:

CheckPatch                    FAIL      1.09 seconds
GitLint                       PASS      0.77 seconds
BuildEll                      PASS      20.05 seconds
BluezMake                     PASS      519.73 seconds
MakeCheck                     PASS      19.28 seconds
MakeDistcheck                 PASS      152.18 seconds
CheckValgrind                 PASS      217.44 seconds
CheckSmatch                   PASS      291.38 seconds
bluezmakeextell               PASS      93.72 seconds
IncrementalBuild              PASS      522.92 seconds
ScanBuild                     PASS      854.61 seconds

Details
##############################
Test: CheckPatch - FAIL
Desc: Run checkpatch.pl script
Output:
[BlueZ,1/3] adapter: Add btd_adapter_may_wake() WARNING:BAD_SIGN_OFF: Non-standard signature: Assisted-by:
#87:

Assisted-by Claude:claude-fable-5

ERROR:BAD_SIGN_OFF: Unrecognized email address: 'Claude:claude-fable-5' #87:

Assisted-by Claude:claude-fable-5

/github/workspace/src/patch/14710584.patch total: 1 errors, 1 warnings, 83 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.

/github/workspace/src/patch/14710584.patch has style problems, please review.

NOTE: Ignored message types: COMMIT_MESSAGE COMPLEX_MACRO CONST_STRUCT FILE_PATH_CHANGES MISSING_SIGN_OFF PREFER_PACKED SPDX_LICENSE_TAG SPLIT_STRING SSCANF_TO_KSTRTO

NOTE: If any of the errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.

[BlueZ,2/3] device: Hide WakeAllowed when adapter cannot wake WARNING:BAD_SIGN_OFF: Non-standard signature: Assisted-by:
#77:

Assisted-by Claude:claude-fable-5

ERROR:BAD_SIGN_OFF: Unrecognized email address: 'Claude:claude-fable-5' #77:

Assisted-by Claude:claude-fable-5

/github/workspace/src/patch/14710585.patch total: 1 errors, 1 warnings, 11 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.

/github/workspace/src/patch/14710585.patch has style problems, please review.

NOTE: Ignored message types: COMMIT_MESSAGE COMPLEX_MACRO CONST_STRUCT FILE_PATH_CHANGES MISSING_SIGN_OFF PREFER_PACKED SPDX_LICENSE_TAG SPLIT_STRING SSCANF_TO_KSTRTO

NOTE: If any of the errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.

https://github.com/bluez/bluez/pull/2349

---
Regards,
Linux Bluetooth

#3 lore raw

Re: [PATCH BlueZ 1/3] adapter: Add btd_adapter_may_wake()

In reply to Matthew Schwartz

Hi Matthew,

Quoted 20 lines · Matthew Schwartz

The kernel provides no interface for querying whether a controller is currently configured to wake the host from suspend. hci_register_dev() marks HCI_CONN_FLAG_REMOTE_WAKEUP as supported whenever a driver provides a wakeup callback, and btusb always does, so the supported flags reported through mgmt say nothing about the runtime setting. The value that matters, device_may_wakeup() on the underlying USB device, is only evaluated during the suspend flow, and the wakeup callback is not a pure query (btmtksdio's variant sends vendor HCI commands), so mgmt could not simply re-evaluate it on Get Device Flags without new kernel infrastructure.

Add btd_adapter_may_wake(), which reconstructs what btusb reports by walking the adapter's sysfs ancestry to the closest USB device and reading its power/wakeup attribute. A missing attribute means the device cannot generate wake events at all, which is what btusb arranges for the fake CSR clones by clearing the USB device's wakeup capability. The attribute is read per call so that each query reflects the current setting.

Quoted 3 lines

Only USB is handled. Controllers on other buses keep the current behavior. btmtksdio exposes the same attribute on its SDIO function device, but its wakeup callback can also depend on a vendor command.

I guess we should introduce some new mgmt setting that allows us to detect whether the adapter is capable of waking up the system or not. The tricky part is this being dynamically enabled/disabled with the likes of:

echo "disabled" > /sys/bus/usb/devices/usb1/power/wakeup

So this would need to send MGMT_EV_NEW_SETTINGS reflecting the current value of hdev->wakeup, which I don't know it really is worth the trouble, or we should really hack together the sysfs entry like this, or maybe we expose the sysfs entry of the device somehow so it is known to bluetoothd via some mgmt event or something like that.

Quoted 103 lines
Assisted-by Claude:claude-fable-5
---
 src/adapter.c | 70 +++++++++++++++++++++++++++++++++++++++++++++++++++
 src/adapter.h |  1 +
 2 files changed, 71 insertions(+)
src/adapter.c +70 -0
index a56eafabe..65b394f82 100644
@@ -563,6 +563,76 @@ uint8_t btd_adapter_get_address_type(struct btd_adapter *adapter)
        return adapter->bdaddr_type;
 }
 
+static bool sysfs_wakeup_enabled(const char *dir)
+{
+       char path[PATH_MAX];
+       char *contents;
+       bool enabled;
+
+       snprintf(path, sizeof(path), "%s/power/wakeup", dir);
+
+       /* A missing attribute means the device cannot wake the host */
+       if (!g_file_get_contents(path, &contents, NULL, NULL))
+               return false;
+
+       enabled = g_str_has_prefix(contents, "enabled");
+
+       g_free(contents);
+
+       return enabled;
+}
+
+static bool sysfs_is_usb_device(const char *dir)
+{
+       char path[PATH_MAX];
+
+       /* USB devices expose idVendor, USB interfaces do not */
+       snprintf(path, sizeof(path), "%s/idVendor", dir);
+
+       return g_file_test(path, G_FILE_TEST_EXISTS);
+}
+
+/*
+ * Whether the controller is currently configured to wake the host from
+ * system suspend. btusb reports this to the kernel with
+ * device_may_wakeup() on the underlying USB device, which userspace
+ * controls through its power/wakeup attribute. Only USB is handled
+ * here. Controllers on other buses are assumed to be able to wake the
+ * host. The attribute is read on every call so that runtime changes
+ * are picked up.
+ */
+bool btd_adapter_may_wake(struct btd_adapter *adapter)
+{
+       char path[PATH_MAX];
+       char *dir;
+       char *sep;
+       bool may_wake = true;
+
+       snprintf(path, sizeof(path), "/sys/class/bluetooth/hci%u/device",
+                                                       adapter->dev_id);
+
+       dir = realpath(path, NULL);
+       if (!dir)
+               return true;
+
+       while (g_str_has_prefix(dir, "/sys/devices/")) {
+               if (sysfs_is_usb_device(dir)) {
+                       may_wake = sysfs_wakeup_enabled(dir);
+                       break;
+               }
+
+               sep = strrchr(dir, '/');
+               if (!sep)
+                       break;
+
+               *sep = '\0';
+       }
+
+       free(dir);
+
+       return may_wake;
+}
+
 static void store_adapter_info(struct btd_adapter *adapter)
 {
        GKeyFile *key_file;
src/adapter.h +1 -1
index a9e1bbf66..ae0011ff7 100644
@@ -107,6 +107,7 @@ const char *adapter_get_path(struct btd_adapter *adapter);
 const bdaddr_t *btd_adapter_get_address(struct btd_adapter *adapter);
 uint8_t btd_adapter_get_address_type(struct btd_adapter *adapter);
 const char *btd_adapter_get_storage_dir(struct btd_adapter *adapter);
+bool btd_adapter_may_wake(struct btd_adapter *adapter);
 
 int adapter_service_add(struct btd_adapter *adapter, sdp_record_t *rec);
 void adapter_service_remove(struct btd_adapter *adapter, uint32_t handle);
--

2.55.0

Signature · 1 line
Luiz Augusto von Dentz
#4 lore raw

PATCH 2/3 device: Hide WakeAllowed when adapter cannot wake

In reply to Matthew Schwartz

dev_property_wake_allowed_exist() only checks whether the device has a profile that supports wake configuration (HID/HoG), so WakeAllowed is exposed on every input device even when the host controller cannot or will not wake the system.

Gate the property on btd_adapter_may_wake() as well. The remote wakeup device flag itself remains settable through mgmt and is still stored. The kernel independently ignores it during suspend when the controller may not wake the host, and the stored preference reapplies once wakeup is enabled again. No PropertiesChanged signal is emitted when the sysfs attribute changes, so clients observe the state that is current at the time they query the property.

Assisted-by Claude:claude-fable-5
---
 src/device.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
src/device.c +4 -1
index 65d84be56..697baf628 100644
@@ -1839,7 +1839,10 @@ static gboolean dev_property_wake_allowed_exist(
 {
 	struct btd_device *device = data;
 
-	return device_get_wake_support(device);
+	if (!device_get_wake_support(device))
+		return FALSE;
+
+	return btd_adapter_may_wake(device->adapter);
 }
 
 static void append_set(void *data, void *user_data)
Signature · 1 line
2.55.0
#5 lore raw

PATCH 3/3 doc: Mark WakeAllowed as optional

In reply to Matthew Schwartz

WakeAllowed is only present on devices with a profile that supports wake configuration, and now only when the adapter itself is able to wake the host. Mark it optional like the other conditional Device1 properties and document when it is present.

---
 doc/org.bluez.Device.rst | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)
doc/org.bluez.Device.rst +5 -2
index 3e6a30aaf..8b6b9e866 100644
@@ -330,11 +330,14 @@ Examples:
 :bluetoothctl: > block [dev]
 :bluetoothctl: > unblock [dev]
 
-boolean WakeAllowed [readwrite]
-```````````````````````````````
+boolean WakeAllowed [readwrite, optional]
+`````````````````````````````````````````
 
 If set to true this device will be allowed to wake the host from system suspend.
 
+Only present on devices with a profile that supports wake configuration (e.g.
+HID) and when the host controller is configured to be able to wake the system.
+
 Examples:
 
 :bluetoothctl: > wake [dev] [on/off]
Signature · 1 line
2.55.0