All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH BlueZ 0/3] Fix wake_allowed reported error and not being set after pairing
@ 2025-03-25 17:28 Ludovico de Nittis
  2025-03-25 17:28 ` [PATCH BlueZ 1/3] adapter: Preserve pending flags when setting the Device Privacy Mode Ludovico de Nittis
                   ` (4 more replies)
  0 siblings, 5 replies; 7+ messages in thread
From: Ludovico de Nittis @ 2025-03-25 17:28 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Ludovico de Nittis

When a new HID or HOG device is in range, Bluez reports the following error:
```
src/device.c:set_wake_allowed_complete() Set device flags return status:
Invalid Parameters
```

Also, a side effect of that issue, when pairing a HID or HOG device, the
wake_allowed property doesn't get enabled by default, as expected.
It requires a Bluez restart in order to get that property correctly set.

This patch series addresses those issues.

Ludovico de Nittis (3):
  adapter: Preserve pending flags when setting the Device Privacy Mode
  device: Preserve pending flags when setting the wake allowed
  device: Try to set the wake_allowed property only for bonded devices

 src/adapter.c |  5 ++++-
 src/device.c  | 19 +++++++++++++++----
 2 files changed, 19 insertions(+), 5 deletions(-)

-- 
2.49.0


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

* [PATCH BlueZ 1/3] adapter: Preserve pending flags when setting the Device Privacy Mode
  2025-03-25 17:28 [PATCH BlueZ 0/3] Fix wake_allowed reported error and not being set after pairing Ludovico de Nittis
@ 2025-03-25 17:28 ` Ludovico de Nittis
  2025-03-25 18:34   ` Fix wake_allowed reported error and not being set after pairing bluez.test.bot
  2025-03-25 17:28 ` [PATCH BlueZ 2/3] device: Preserve pending flags when setting the wake allowed Ludovico de Nittis
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 7+ messages in thread
From: Ludovico de Nittis @ 2025-03-25 17:28 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Ludovico de Nittis

If there are already flags that are pending to be applied, we should
keep them to avoid overwriting them.
At that point we only want to add the Device Privacy Mode on top of the
existing flags.
---
 src/adapter.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/adapter.c b/src/adapter.c
index 5d4117a49..d4e42eed8 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -5630,8 +5630,11 @@ static void add_device_complete(uint8_t status, uint16_t length,
 	if (btd_opts.device_privacy) {
 		uint32_t flags = btd_device_get_current_flags(dev);
 
-		/* Set Device Privacy Mode has not set the flag yet. */
+		/* Set Device Privacy Mode if it has not set the flag yet. */
 		if (!(flags & DEVICE_FLAG_DEVICE_PRIVACY)) {
+			/* Include the pending flags, or they may get overwritten. */
+			flags |= btd_device_get_pending_flags(dev);
+
 			adapter_set_device_flags(adapter, dev, flags |
 						DEVICE_FLAG_DEVICE_PRIVACY,
 						set_device_privacy_complete,
-- 
2.49.0


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

* [PATCH BlueZ 2/3] device: Preserve pending flags when setting the wake allowed
  2025-03-25 17:28 [PATCH BlueZ 0/3] Fix wake_allowed reported error and not being set after pairing Ludovico de Nittis
  2025-03-25 17:28 ` [PATCH BlueZ 1/3] adapter: Preserve pending flags when setting the Device Privacy Mode Ludovico de Nittis
@ 2025-03-25 17:28 ` Ludovico de Nittis
  2025-03-25 17:28 ` [PATCH BlueZ 3/3] device: Try to set the wake_allowed property only for bonded devices Ludovico de Nittis
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 7+ messages in thread
From: Ludovico de Nittis @ 2025-03-25 17:28 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Ludovico de Nittis

If there are already flags that are pending to be applied, we should
keep them to avoid overwriting them.
In device_set_wake_allowed() we only want to either add or remove the
remote wakeup flag, while keeping the existing flags as-is.
---
 src/device.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/src/device.c b/src/device.c
index e8bff718c..474ec5763 100644
--- a/src/device.c
+++ b/src/device.c
@@ -1605,6 +1605,10 @@ void device_set_wake_allowed(struct btd_device *device, bool wake_allowed,
 	device->pending_wake_allowed = wake_allowed;
 
 	flags = device->current_flags;
+
+	/* Include the pending flags, or they may get overwritten. */
+	flags |= device->pending_flags;
+
 	if (wake_allowed)
 		flags |= DEVICE_FLAG_REMOTE_WAKEUP;
 	else
-- 
2.49.0


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

* [PATCH BlueZ 3/3] device: Try to set the wake_allowed property only for bonded devices
  2025-03-25 17:28 [PATCH BlueZ 0/3] Fix wake_allowed reported error and not being set after pairing Ludovico de Nittis
  2025-03-25 17:28 ` [PATCH BlueZ 1/3] adapter: Preserve pending flags when setting the Device Privacy Mode Ludovico de Nittis
  2025-03-25 17:28 ` [PATCH BlueZ 2/3] device: Preserve pending flags when setting the wake allowed Ludovico de Nittis
@ 2025-03-25 17:28 ` Ludovico de Nittis
  2025-03-25 19:38 ` [PATCH BlueZ 0/3] Fix wake_allowed reported error and not being set after pairing Luiz Augusto von Dentz
  2025-03-26 15:10 ` patchwork-bot+bluetooth
  4 siblings, 0 replies; 7+ messages in thread
From: Ludovico de Nittis @ 2025-03-25 17:28 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Ludovico de Nittis

When the function `device_set_wake_support()` is called, we don't have
the guarantees for the device to be already bonded.

For example, that function gets called by `hog_probe()`, that is also
triggered when bluez scans for new devices. In that instance, we don't
want to try setting the `wake_allowed` property, because those devices
are only in range of the host and are not connected, paired or bonded
yet.

This fixes the following Bluez error when we scan for new devices and a
new hog or hid is in range:
```
src/device.c:set_wake_allowed_complete() Set device flags return status:
Invalid Parameters
```

Additionally, because that initial `device_set_allowed()` call can fail,
this commit fixes the issue of hog and hid devices that, after the first
pairing, were unexpectedly showing `WakeAllowed: no`. And it required a
reboot to let that property be set to the expected `WakeAllowed: yes` by
default.
---
 src/device.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/src/device.c b/src/device.c
index 474ec5763..727b668af 100644
--- a/src/device.c
+++ b/src/device.c
@@ -1526,10 +1526,13 @@ void device_set_wake_support(struct btd_device *device, bool wake_support)
 	if (device->wake_override == WAKE_FLAG_DEFAULT)
 		device_set_wake_override(device, wake_support);
 
-	/* Set wake_allowed according to the override value. */
-	device_set_wake_allowed(device,
-				device->wake_override == WAKE_FLAG_ENABLED,
-				-1U);
+	/* Set wake_allowed according to the override value.
+	 * Limit this to bonded device to avoid trying to set it
+	 * to new devices that are simply in range. */
+	if (device_is_bonded(device, device->bdaddr_type))
+		device_set_wake_allowed(device,
+					device->wake_override == WAKE_FLAG_ENABLED,
+					-1U);
 }
 
 static bool device_get_wake_allowed(struct btd_device *device)
@@ -6561,6 +6564,10 @@ void device_bonding_complete(struct btd_device *device, uint8_t bdaddr_type,
 
 	device_auth_req_free(device);
 
+	/* Enable the wake_allowed property if required */
+	if (device->wake_override == WAKE_FLAG_ENABLED)
+		device_set_wake_allowed(device, true, -1U);
+
 	/* If we're already paired nothing more is needed */
 	if (state->paired)
 		return;
-- 
2.49.0


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

* RE: Fix wake_allowed reported error and not being set after pairing
  2025-03-25 17:28 ` [PATCH BlueZ 1/3] adapter: Preserve pending flags when setting the Device Privacy Mode Ludovico de Nittis
@ 2025-03-25 18:34   ` bluez.test.bot
  0 siblings, 0 replies; 7+ messages in thread
From: bluez.test.bot @ 2025-03-25 18:34 UTC (permalink / raw)
  To: linux-bluetooth, ludovico.denittis

[-- Attachment #1: Type: text/plain, Size: 1260 bytes --]

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

---Test result---

Test Summary:
CheckPatch                    PENDING   0.24 seconds
GitLint                       PENDING   0.22 seconds
BuildEll                      PASS      20.76 seconds
BluezMake                     PASS      1503.05 seconds
MakeCheck                     PASS      13.63 seconds
MakeDistcheck                 PASS      171.29 seconds
CheckValgrind                 PASS      225.50 seconds
CheckSmatch                   PASS      307.78 seconds
bluezmakeextell               PASS      98.94 seconds
IncrementalBuild              PENDING   0.32 seconds
ScanBuild                     PASS      876.53 seconds

Details
##############################
Test: CheckPatch - PENDING
Desc: Run checkpatch.pl script
Output:

##############################
Test: GitLint - PENDING
Desc: Run gitlint
Output:

##############################
Test: IncrementalBuild - PENDING
Desc: Incremental build with the patches in the series
Output:



---
Regards,
Linux Bluetooth


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

* Re: [PATCH BlueZ 0/3] Fix wake_allowed reported error and not being set after pairing
  2025-03-25 17:28 [PATCH BlueZ 0/3] Fix wake_allowed reported error and not being set after pairing Ludovico de Nittis
                   ` (2 preceding siblings ...)
  2025-03-25 17:28 ` [PATCH BlueZ 3/3] device: Try to set the wake_allowed property only for bonded devices Ludovico de Nittis
@ 2025-03-25 19:38 ` Luiz Augusto von Dentz
  2025-03-26 15:10 ` patchwork-bot+bluetooth
  4 siblings, 0 replies; 7+ messages in thread
From: Luiz Augusto von Dentz @ 2025-03-25 19:38 UTC (permalink / raw)
  To: Ludovico de Nittis; +Cc: linux-bluetooth

Hi Ludovico,

On Tue, Mar 25, 2025 at 1:30 PM Ludovico de Nittis
<ludovico.denittis@collabora.com> wrote:
>
> When a new HID or HOG device is in range, Bluez reports the following error:
> ```
> src/device.c:set_wake_allowed_complete() Set device flags return status:
> Invalid Parameters
> ```
>
> Also, a side effect of that issue, when pairing a HID or HOG device, the
> wake_allowed property doesn't get enabled by default, as expected.
> It requires a Bluez restart in order to get that property correctly set.
>
> This patch series addresses those issues.
>
> Ludovico de Nittis (3):
>   adapter: Preserve pending flags when setting the Device Privacy Mode
>   device: Preserve pending flags when setting the wake allowed
>   device: Try to set the wake_allowed property only for bonded devices
>
>  src/adapter.c |  5 ++++-
>  src/device.c  | 19 +++++++++++++++----
>  2 files changed, 19 insertions(+), 5 deletions(-)
>
> --
> 2.49.0

One thing I just realized is that we do not persist the flags accross
restart/reboot, so while at it it might be worth adding support for
it, the other remark is that perhaps we should store things like
pending flags into struct mgmt so we can add unit test to make things
a little more future-proof in this respect, any these can probably
come later so we don't need to delay including these fixes.

-- 
Luiz Augusto von Dentz

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

* Re: [PATCH BlueZ 0/3] Fix wake_allowed reported error and not being set after pairing
  2025-03-25 17:28 [PATCH BlueZ 0/3] Fix wake_allowed reported error and not being set after pairing Ludovico de Nittis
                   ` (3 preceding siblings ...)
  2025-03-25 19:38 ` [PATCH BlueZ 0/3] Fix wake_allowed reported error and not being set after pairing Luiz Augusto von Dentz
@ 2025-03-26 15:10 ` patchwork-bot+bluetooth
  4 siblings, 0 replies; 7+ messages in thread
From: patchwork-bot+bluetooth @ 2025-03-26 15:10 UTC (permalink / raw)
  To: Ludovico de Nittis; +Cc: linux-bluetooth

Hello:

This series was applied to bluetooth/bluez.git (master)
by Luiz Augusto von Dentz <luiz.von.dentz@intel.com>:

On Tue, 25 Mar 2025 18:28:43 +0100 you wrote:
> When a new HID or HOG device is in range, Bluez reports the following error:
> ```
> src/device.c:set_wake_allowed_complete() Set device flags return status:
> Invalid Parameters
> ```
> 
> Also, a side effect of that issue, when pairing a HID or HOG device, the
> wake_allowed property doesn't get enabled by default, as expected.
> It requires a Bluez restart in order to get that property correctly set.
> 
> [...]

Here is the summary with links:
  - [BlueZ,1/3] adapter: Preserve pending flags when setting the Device Privacy Mode
    (no matching commit)
  - [BlueZ,2/3] device: Preserve pending flags when setting the wake allowed
    https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=41cfe55ea759
  - [BlueZ,3/3] device: Try to set the wake_allowed property only for bonded devices
    (no matching commit)

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



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

end of thread, other threads:[~2025-03-26 15:10 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-03-25 17:28 [PATCH BlueZ 0/3] Fix wake_allowed reported error and not being set after pairing Ludovico de Nittis
2025-03-25 17:28 ` [PATCH BlueZ 1/3] adapter: Preserve pending flags when setting the Device Privacy Mode Ludovico de Nittis
2025-03-25 18:34   ` Fix wake_allowed reported error and not being set after pairing bluez.test.bot
2025-03-25 17:28 ` [PATCH BlueZ 2/3] device: Preserve pending flags when setting the wake allowed Ludovico de Nittis
2025-03-25 17:28 ` [PATCH BlueZ 3/3] device: Try to set the wake_allowed property only for bonded devices Ludovico de Nittis
2025-03-25 19:38 ` [PATCH BlueZ 0/3] Fix wake_allowed reported error and not being set after pairing Luiz Augusto von Dentz
2025-03-26 15:10 ` patchwork-bot+bluetooth

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.