Bluetooth: Host: Add role-specific auto PHY update options #97198

jhedberg · 2025-10-08T13:15:00Z

The BT_AUTO_PHY_UPDATE option was problematic in the sense that it didn't
really allow any kind of fine-tuning of what automation is desired. In
particular, it didn't allow specifying which PHY was the preferred one (it
was hardcoded to 2M) and also didn't allow specifying role-specific
(central vs peripheral) preferences.

To solve this, deprecate the old option, and instead introduce
role-specific options that allow specifying 1M/2M/Coded/None as the
preference. The new options have slightly different defaults than the new:
for central role 2M stays as the preference, however since it's uncommon to
require automated changes on the peripheral side the default is set to
None there.

subsys/bluetooth/host/Kconfig

doc/releases/migration-guide-4.3.rst

subsys/bluetooth/host/conn.c

subsys/bluetooth/host/Kconfig

jhedberg · 2025-10-09T09:47:28Z

Posting here the latest from Discord:

There shouldn't be a need for any of this automation in the host, neither for the PHY nor for the data length, due to the existence of HCI_LE_Set_Default_PHY and HCI_LE_Write_Suggested_Default_Data_Length commands that could either be included as part of the host's HCI init (taking their values from Kconfig), or from the application, for which we already have existing APIs.

Actually, we already have HCI_LE_Write_Suggested_Default_Data_Length in the HCI init, so the code in conn.c is (as the code comment there indicates) only needed for buggy controllers that don't otherwise take care of this. Either way, the Kconfig options for these become questionable.

The only place where the CONFIG_BT_USER_DATA_LEN_UPDATE and CONFIG_BT_USER_PHY_UPDATE options are slightly useful are for memory saving in the bt_conn_cb struct. However, they're really not related to triggers from the app side, rather a peripheral app may be interested in these e.g. if the central goes and changes the values during a connection.

jhedberg · 2025-10-09T11:32:56Z

Conclusion from the Bluetooth WG meeting:

The set_default HCI commands don't actually cause any procedures to be triggered in themselves, so the explicit triggering by the host still has a purpose. I'll therefore roll back to my earlier plan to just do the role split for the phy update. For data length, we can either remove or deprecate the "auto" Kconfig option, since it serves no purpose now that the controller "no_auto_dle" quirk comes from DTS, i.e. is build-time resolvable in itself.

Another improvement that can be done (follow-up PR) is to cache the controller max data length, since we read it already during HCI init, i.e. no need to redo this for every connection.

cvinayak · 2025-10-09T11:45:00Z

Just re-iterating my statements from the WG meeting.

just do the role split for the phy update.

Agree. And keep the feature available (default disabled is ok) for users needing auto PHY update being handled by Host on connection establishment (i.e. while connected callback is generated to applications). We do not want such users investing in developing a solution themself when migrating to new Zephyr release.

For data length, we can either remove or deprecate the "auto" Kconfig option, since it serves no purpose now that the controller "no_auto_dle" quirk comes from DTS, i.e. is build-time resolvable in itself.

Agree.

Another improvement that can be done (follow-up PR) is to cache the controller max data length, since we read it already during HCI init, i.e. no need to redo this for every connection.

Agree.

alwa-nordic · 2025-10-09T11:48:46Z

Another improvement that can be done (follow-up PR) is to cache the controller max data length, since we read it already during HCI init, i.e. no need to redo this for every connection.

HCI spec does not prohibit asking for a larger value than the Controller supports. We can always do bt_le_set_data_len(conn, 0x00fb, 0x4290), which are the HCI-defined absolute limits.

Thalley · 2025-10-09T11:57:08Z

HCI spec does not prohibit asking for a larger value than the Controller supports. We can always do bt_le_set_data_len(conn, 0x00fb, 0x4290), which are the HCI-defined absolute limits.

From 7.8.33 LE Set Data Length command (similar text exists for 7.8.35 LE Write Suggested Default Data Length command):

This command allows the Host to suggest the maximum transmission payload size and
maximum packet transmission time (connMaxTxOctets and connMaxTxTime - see [Vol
6] Part B, Section 4.5.10) to be used for LL Data PDUs on a given connection. The
Controller may use smaller or larger values based on local information.

I think that make might sense to do. We probably want to use the largest data length, and since the host isn't using the results of bt_hci_le_read_max_data_len for anything anyways, then it's kind of moot to perform that (not to mention that even setting the data length to the maximum value supported by the controller, doesn't ensure that it is indeed the value that the controller supports.
Of course as @jhedberg mentioned, some controllers may have some odd behavior (e.g. could just ignore the value from the host if it's outside it's range, instead of applying the maximum it supports).

jhedberg · 2025-10-10T08:54:00Z

@Thalley @cvinayak I've pushed an update to implement the initial change as discussed

Thalley · 2025-10-10T09:12:12Z

subsys/bluetooth/host/conn.c

+	switch (conn->role) {
+#if defined(AUTO_PHY_CENTRAL)
+	case BT_HCI_ROLE_CENTRAL:
+		if (AUTO_PHY_CENTRAL_SUPPORTED(bt_dev.le.features) &&
+		    phy_change_needed(conn, AUTO_PHY_CENTRAL)) {
+			err = bt_le_set_phy(conn, 0U, AUTO_PHY_CENTRAL_PREF,
+					    AUTO_PHY_CENTRAL_PREF, BT_HCI_LE_PHY_CODED_ANY);
 		}
-		if (conn->state != BT_CONN_CONNECTED) {
-			return;
+		break;
+#endif
+#if defined(AUTO_PHY_PERIPHERAL)
+	case BT_HCI_ROLE_PERIPHERAL:
+		if (AUTO_PHY_PERIPHERAL_SUPPORTED(bt_dev.le.features) &&
+		    phy_change_needed(conn, AUTO_PHY_PERIPHERAL)) {
+			err = bt_le_set_phy(conn, 0U, AUTO_PHY_PERIPHERAL_PREF,
+					    AUTO_PHY_PERIPHERAL_PREF, BT_HCI_LE_PHY_CODED_ANY);
 		}
+		break;
+#endif
+	default:
+		/* No PHY preferences set for the given role */
+		break;
+	}


Given that phy_change_needed takes conn, the conn->role check could be moved to that function which would make the code here significantly simpler

I'm all for finding ways to simplify this. However, I'm not sure how that exact proposal helps, since we also need the role to know what to pass to the bt_le_set_phy() call.

since we also need the role to know what to pass to the bt_le_set_phy() call.

Since the value is exclusively based on the conn->role, then we don't need to pass it along at all. The function can just take the conn pointer and nothing more :)

Thalley · 2025-10-10T09:13:38Z

subsys/bluetooth/host/conn.c

 */
-static bool uses_symmetric_2mbit_phy(struct bt_conn *conn)
+#if defined(AUTO_PHY_CENTRAL) || defined(AUTO_PHY_PERIPHERAL)
+static bool phy_change_needed(struct bt_conn *conn, uint8_t phy)


Minor: "Needed" is a bit strongly worded here; we never really need to update the PHY, do we?

(Not a blocker, and the name is OK, but otherwise consider can_update_phy or something instead?)

I'm not sure "can" is any more intuitive here. This is whether we already have the PHY we prefer. If we do, then we don't "need" to do the procedure. What other word would be better?

If we end up moving everything related to the PHY update into the function (see #97198 (comment)), it would probably just be do_auto_phy_update :D

Yes, I can certainly refactor this so that everything is in a single PHY-related function. It might have a slight impact on code size, but likely not much (if any)

Thalley · 2025-10-10T09:15:24Z

subsys/bluetooth/host/Kconfig

+config BT_AUTO_PHY_PERIPHERAL_NONE
+	bool "No PHY preference"


I would consider putting "None" as the first choice

Btw, the _NONE options shouldn't really be needed since Kconfig has the optional keyword for choice statements which make it possible to not have any of the possible options selected. The problem with optional (as I discovered) is that it then doesn't allow us to have a default where one of the options would anyway be selected. The only effect of a default for optional is that when you select the choice on the top-level in menuconfig, then the default is what'll be initially enabled, however the top-level doesn't implicitly get enabled by specifying a default.

jhedberg · 2025-10-13T11:01:00Z

@aescolar @cvinayak I think I identified an issue. It should hopefully now be fixed with the update I just pushed (also simpler code with using IS_ENABLED() instead of COND_CODE_0())

jhedberg · 2025-10-13T11:14:55Z

@aescolar @cvinayak I think I identified an issue. It should hopefully now be fixed with the update I just pushed (also simpler code with using IS_ENABLED() instead of COND_CODE_0())

Still failing. So now I'm at a loss of how to proceed with debugging this, since I can't run babblesim myself.

cvinayak · 2025-10-13T11:16:16Z

subsys/bluetooth/common/hci_common_internal.h

+#define BT_BUF_CMD_TX_AUTO_PHY_UPDATE      (!IS_ENABLED(CONFIG_BT_AUTO_PHY_PERIPHERAL_NONE) && \
+					    !IS_ENABLED(CONFIG_BT_AUTO_PHY_CENTRAL_NONE))


Suggested change

#define BT_BUF_CMD_TX_AUTO_PHY_UPDATE (!IS_ENABLED(CONFIG_BT_AUTO_PHY_PERIPHERAL_NONE) && \

!IS_ENABLED(CONFIG_BT_AUTO_PHY_CENTRAL_NONE))

#define BT_BUF_CMD_TX_AUTO_PHY_UPDATE (1)

Will the bsim test pass locally if hardcoding the buffer count? if it still fails, then there must be a bug in Host or in the Controller that is being exposed by the changes in this PR.

The above had a bug: it should have been || instead of &&. Let's see one more time. As for "locally", I have no idea since I don't have a development environment supported by babblesim.

Still failing, but looks like a different error. So probably this particular issue is solved.

d_00: @00:00:00.004938 Advertising failed to start (err -114)

This looks suspicious... let me checkout your branch and test it.

@jhedberg please fixup

commit e6eb3c52637751ec2c0e613d0610127ab0ffd826 Author: Johan Hedberg <[email protected]> Date: Fri Oct 10 15:38:56 2025 +0300 fixup! Bluetooth: samples/tests: Update auto PHY usage to recent changes Remove deprecated Kconfig option usage, and replace it with corresponding options which yield the same behavior as before. Signed-off-by: Johan Hedberg <[email protected]> diff --git a/tests/bsim/bluetooth/ll/conn/prj_split_tx_defer.conf b/tests/bsim/bluetooth/ll/conn/prj_split_tx_defer.conf index 22dc32be4c4..6b6ecfc2b23 100644 --- a/tests/bsim/bluetooth/ll/conn/prj_split_tx_defer.conf +++ b/tests/bsim/bluetooth/ll/conn/prj_split_tx_defer.conf @@ -13,6 +13,8 @@ CONFIG_BT_L2CAP_DYNAMIC_CHANNEL=y CONFIG_BT_DEVICE_NAME="bsim_test_split" CONFIG_BT_L2CAP_TX_BUF_COUNT=6 +CONFIG_BT_AUTO_PHY_PERIPHERAL_2M=y + CONFIG_BT_CTLR_PRIVACY=n CONFIG_BT_CTLR_ADVANCED_FEATURES=y

d_00: @00:00:00.004938 Advertising failed to start (err -114)

This looks suspicious... let me checkout your branch and test it.

Fix for this unrelated issue: #97479

jhedberg · 2025-10-13T12:31:19Z

@cvinayak @aescolar any idea about the latest failure:

d_00: @00:00:00.004938  Advertising failed to start (err -114)

That's super odd, since #define ENETUNREACH 114, yet there's nothing in the Zephyr host or the HCI drivers that would use that error code.

Thalley · 2025-10-13T12:35:33Z

@cvinayak @aescolar any idea about the latest failure:
d_00: @00:00:00.004938  Advertising failed to start (err -114)
That's super odd, since #define ENETUNREACH 114, yet there's nothing in the Zephyr host or the HCI drivers that would use that error code.

Keep in mind that 114 in BSIM is not the errno values from Zephyr, but rather the host system. On my system it corresponds to EALREADY

aescolar · 2025-10-13T12:42:11Z

@cvinayak @aescolar any idea about the latest failure:
d_00: @00:00:00.004938  Advertising failed to start (err -114)
That's super odd, since #define ENETUNREACH 114, yet there's nothing in the Zephyr host or the HCI drivers that would use that error code.

This is the call stack when that error is returned:

adv_start_legacy() at subsys/bluetooth/host/adv.c:915 0x808eff9	
bt_le_adv_start() at subsys/bluetooth/host/adv.c:1,285 0x808f8b6	
start_advertising() at tests/bsim/bluetooth/ll/conn/src/test_connect2.c:129 0x804fe16	
bt_ready() at tests/bsim/bluetooth/ll/conn/src/test_connect2.c:160 0x8050012	
test_con2_main() at tests/bsim/bluetooth/ll/conn/src/test_connect2.c:206 0x80502b6

Thalley · 2025-10-13T12:46:42Z

@cvinayak @aescolar any idea about the latest failure:
d_00: @00:00:00.004938  Advertising failed to start (err -114)
That's super odd, since #define ENETUNREACH 114, yet there's nothing in the Zephyr host or the HCI drivers that would use that error code.
Keep in mind that 114 in BSIM is not the errno values from Zephyr, but rather the host system. On my system it corresponds to EALREADY

Discussed a while ago with @alwa-nordic to use one of the Zephyr LIBC implementations for BSIM tests to help make it easier to lookup the error from the value.

We can probably also find a way to convert the values to strings using strerror in some way

There's a regression from commit 8cfad44 which introduced trying to enable advertising twice. Remove the other attempt. Signed-off-by: Johan Hedberg <[email protected]>

The BT_AUTO_PHY_UPDATE option was problematic in the sense that it didn't really allow any kind of fine-tuning of what automation is desired. In particular, it didn't allow specifying which PHY was the preferred one (it was hardcoded to 2M) and also didn't allow specifying role-specific (central vs peripheral) preferences. To solve this, deprecate the old option, and instead introduce role-specific options that allow specifying 1M/2M/Coded/None as the preference. The new options have slightly different defaults than the new: for central role 2M stays as the preference, however since it's uncommon to require automated changes on the peripheral side the default is set to None there. Signed-off-by: Johan Hedberg <[email protected]>

Document the changes related to Bluetooth auto PHY update Kconfig options. Signed-off-by: Johan Hedberg <[email protected]>

Remove deprecated Kconfig option usage, and replace it with corresponding options which yield the same behavior as before. Signed-off-by: Johan Hedberg <[email protected]>

Make sure to consider both the new peripheral and central Kconfig options when calculating needed TX buffers. Signed-off-by: Johan Hedberg <[email protected]>

jhedberg · 2025-10-13T12:56:51Z

@Thalley @cvinayak There was a test case bug that @aescolar helped to uncover. See the first commit in this PR for the details. Fingers crossed that everything will be green now.

aescolar · 2025-10-13T13:19:48Z

We can probably also find a way to convert the values to strings using strerror in some way

I'd say that would be nicer. Otherwise the next thing would be someone looking at one libc errno.h or another and getting confused. Just faster to see the error name printed.

jhedberg · 2025-10-13T13:35:13Z

@Thalley @cvinayak There was a test case bug that @aescolar helped to uncover. See the first commit in this PR for the details. Fingers crossed that everything will be green now.

Turns out this was a red herring :( It was a bug, but not the cause of the test case failure. Only other idea I have is to 100% restore the PHY config for this test case, in case it depends on both sides to have "auto-phy 2M" enabled.

Restore the auto-phy config for split_tx_defer test case, since it seems to fail otherwise. Signed-off-by: Johan Hedberg <[email protected]>

jhedberg · 2025-10-13T14:02:33Z

Now we have

d_00: @00:00:07.159852 ERROR: (WEST_TOPDIR/zephyr/tests/bsim/bluetooth/audio/src/bap_scan_delegator_test.c:141): PA timeout

Thalley · 2025-10-13T14:10:17Z

Now we have

d_00: @00:00:07.159852 ERROR: (WEST_TOPDIR/zephyr/tests/bsim/bluetooth/audio/src/bap_scan_delegator_test.c:141): PA timeout

I also get that for #91587.
When the timing changes, the scheduling changes, and sometimes that causes issues like this.

Sometimes it helps nudging the -start_offset and -rs values in the test script, but it's pretty hard to know what values to set (I managed to get the above to pass locally, but fail in CI). Maybe @cvinayak can help provide some reasonable values there?

sonarqubecloud · 2025-10-13T14:28:27Z

Quality Gate passed

Issues
4 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

aescolar · 2025-10-13T16:47:16Z

(I managed to get the above to pass locally, but fail in CI)

@Thalley that should not be happening unless there is unitialized memory in play (or the test is killed in CI due to taking too long).

Thalley · 2025-10-13T17:25:12Z

(I managed to get the above to pass locally, but fail in CI)

@Thalley that should not be happening unless there is unitialized memory in play (or the test is killed in CI due to taking too long).

I've seen that happen on many occasions over the years, and I've never been able to find any memory issues with valgrind, ASAN or UBSAN :)

zephyrbot added area: Bluetooth Release Notes To be mentioned in the release notes area: Bluetooth Host Bluetooth Host (excluding BR/EDR) labels Oct 8, 2025

zephyrbot requested review from HaavardRei, PavelVPV, Thalley, alwa-nordic, cfriedt, cvinayak, hermabe, kartben, rugeGerritsen and sjanc October 8, 2025 13:16

zephyrbot assigned jhedberg and alwa-nordic Oct 8, 2025

jhedberg requested a review from nordicjm October 8, 2025 13:21

Thalley reviewed Oct 8, 2025

View reviewed changes

subsys/bluetooth/host/Kconfig Outdated Show resolved Hide resolved

subsys/bluetooth/host/Kconfig Show resolved Hide resolved

jhedberg force-pushed the periph_phy_update branch from c33aaac to b93d436 Compare October 8, 2025 13:52

Thalley reviewed Oct 8, 2025

View reviewed changes

doc/releases/migration-guide-4.3.rst Outdated Show resolved Hide resolved

subsys/bluetooth/host/conn.c Outdated Show resolved Hide resolved

subsys/bluetooth/host/conn.c Outdated Show resolved Hide resolved

subsys/bluetooth/host/Kconfig Show resolved Hide resolved

jhedberg added the Bluetooth Review Discussion in the Bluetooth WG meeting required label Oct 9, 2025

jhedberg force-pushed the periph_phy_update branch from b93d436 to d6b78bb Compare October 10, 2025 08:52

jhedberg changed the title ~~Bluetooth: Host: Limit auto-phy update to central by default~~ Bluetooth: Host: Add role-specific auto PHY update options Oct 10, 2025

Thalley reviewed Oct 10, 2025

View reviewed changes

jhedberg force-pushed the periph_phy_update branch from d6b78bb to cc810c8 Compare October 10, 2025 09:47

jhedberg force-pushed the periph_phy_update branch from b6b0b5f to 8b36c01 Compare October 13, 2025 11:02

cvinayak reviewed Oct 13, 2025

View reviewed changes

jhedberg force-pushed the periph_phy_update branch 2 times, most recently from ae17c44 to 6b578db Compare October 13, 2025 12:05

jhedberg added 5 commits October 13, 2025 15:55

Bluetooth: tests: bsim: Fix double advertising in test_connect2

0e0f425

There's a regression from commit 8cfad44 which introduced trying to enable advertising twice. Remove the other attempt. Signed-off-by: Johan Hedberg <[email protected]>

doc: release-notes-4.3: Document Bluetooth auto PHY update changes

6bd4b21

Document the changes related to Bluetooth auto PHY update Kconfig options. Signed-off-by: Johan Hedberg <[email protected]>

Bluetooth: samples/tests: Update auto PHY usage to recent changes

38fb28e

Remove deprecated Kconfig option usage, and replace it with corresponding options which yield the same behavior as before. Signed-off-by: Johan Hedberg <[email protected]>

Bluetooth: Update TX buffer calculation with auto PHY changes

dff2e4c

Make sure to consider both the new peripheral and central Kconfig options when calculating needed TX buffers. Signed-off-by: Johan Hedberg <[email protected]>

jhedberg force-pushed the periph_phy_update branch from 6b578db to dff2e4c Compare October 13, 2025 12:56

Thalley previously approved these changes Oct 13, 2025

View reviewed changes

Bluetooth: tests: bsim: Restore auto PHY config

646596c

Restore the auto-phy config for split_tx_defer test case, since it seems to fail otherwise. Signed-off-by: Johan Hedberg <[email protected]>

jhedberg dismissed Thalley’s stale review via 646596c October 13, 2025 13:37

jhedberg closed this Oct 13, 2025

github-project-automation bot moved this from In Review to Done in Bluetooth LE Audio Oct 13, 2025

jhedberg reopened this Oct 13, 2025

		#define BT_BUF_CMD_TX_AUTO_PHY_UPDATE (!IS_ENABLED(CONFIG_BT_AUTO_PHY_PERIPHERAL_NONE) && \
		!IS_ENABLED(CONFIG_BT_AUTO_PHY_CENTRAL_NONE))

	#define BT_BUF_CMD_TX_AUTO_PHY_UPDATE (!IS_ENABLED(CONFIG_BT_AUTO_PHY_PERIPHERAL_NONE) && \
	!IS_ENABLED(CONFIG_BT_AUTO_PHY_CENTRAL_NONE))
	#define BT_BUF_CMD_TX_AUTO_PHY_UPDATE (1)

Bluetooth: Host: Add role-specific auto PHY update options #97198

Are you sure you want to change the base?

Bluetooth: Host: Add role-specific auto PHY update options #97198

Conversation

jhedberg commented Oct 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jhedberg commented Oct 9, 2025

Uh oh!

jhedberg commented Oct 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cvinayak commented Oct 9, 2025

Uh oh!

alwa-nordic commented Oct 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Thalley commented Oct 9, 2025

Uh oh!

jhedberg commented Oct 10, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jhedberg commented Oct 13, 2025

Uh oh!

jhedberg commented Oct 13, 2025

Uh oh!

cvinayak Oct 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jhedberg Oct 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jhedberg commented Oct 13, 2025

Uh oh!

Thalley commented Oct 13, 2025

Uh oh!

aescolar commented Oct 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Thalley commented Oct 13, 2025

Uh oh!

jhedberg commented Oct 13, 2025

Uh oh!

aescolar commented Oct 13, 2025

Uh oh!

jhedberg commented Oct 8, 2025 •

edited

Loading

jhedberg commented Oct 9, 2025 •

edited

Loading

alwa-nordic commented Oct 9, 2025 •

edited

Loading

cvinayak Oct 13, 2025 •

edited

Loading

jhedberg Oct 13, 2025 •

edited

Loading

aescolar commented Oct 13, 2025 •

edited

Loading