Skip to content

Commit 01354c0

Browse files
jori-nordicnashif
jori-nordic
authored and
nashif
committed
Bluetooth: conn: move auto-init procedures to system workqueue
`conn_auto_initiate()` starts a bunch of controller procedures (read: HCI commands) that are fired off right after connection establishment. Right now, it's called from the RX context, which is the same context where resources (cmd & acl buffers) are freed. This not ideal. But the procedures are all async, so it should be fine to schedule this function on the system workqueue, where we have less risk of deadlocks. Signed-off-by: Jonathan Rico <[email protected]>
1 parent 5ec6024 commit 01354c0

File tree

4 files changed

+137
-96
lines changed

4 files changed

+137
-96
lines changed

‎subsys/bluetooth/host/conn.c‎

Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1660,9 +1660,133 @@ int bt_conn_disconnect(struct bt_conn *conn, uint8_t reason)
16601660

16611661
/* Group Connected BT_CONN only in this */
16621662
#if defined(CONFIG_BT_CONN)
1663+
/* We don't want the application to get a PHY update callback upon connection
1664+
* establishment on 2M PHY. Therefore we must prevent issuing LE Set PHY
1665+
* in this scenario.
1666+
*/
1667+
static bool skip_auto_phy_update_on_conn_establishment(struct bt_conn *conn)
1668+
{
1669+
#if defined(CONFIG_BT_USER_PHY_UPDATE)
1670+
if (IS_ENABLED(CONFIG_BT_AUTO_PHY_UPDATE) &&
1671+
IS_ENABLED(CONFIG_BT_EXT_ADV) &&
1672+
BT_DEV_FEAT_LE_EXT_ADV(bt_dev.le.features)) {
1673+
if (conn->le.phy.tx_phy == BT_HCI_LE_PHY_2M &&
1674+
conn->le.phy.rx_phy == BT_HCI_LE_PHY_2M) {
1675+
return true;
1676+
}
1677+
}
1678+
#else
1679+
ARG_UNUSED(conn);
1680+
#endif /* defined(CONFIG_BT_USER_PHY_UPDATE) */
1681+
1682+
return false;
1683+
}
1684+
1685+
static void perform_auto_initiated_procedures(struct bt_conn *conn, void *unused)
1686+
{
1687+
int err;
1688+
1689+
ARG_UNUSED(unused);
1690+
1691+
LOG_DBG("[%p] Running auto-initiated procedures", conn);
1692+
1693+
if (conn->state != BT_CONN_CONNECTED) {
1694+
/* It is possible that connection was disconnected directly from
1695+
* connected callback so we must check state before doing
1696+
* connection parameters update.
1697+
*/
1698+
return;
1699+
}
1700+
1701+
if (atomic_test_and_set_bit(conn->flags, BT_CONN_AUTO_INIT_PROCEDURES_DONE)) {
1702+
/* We have already run the auto-initiated procedures */
1703+
return;
1704+
}
1705+
1706+
if (!atomic_test_bit(conn->flags, BT_CONN_LE_FEATURES_EXCHANGED) &&
1707+
((conn->role == BT_HCI_ROLE_CENTRAL) ||
1708+
BT_FEAT_LE_PER_INIT_FEAT_XCHG(bt_dev.le.features))) {
1709+
err = bt_hci_le_read_remote_features(conn);
1710+
if (err) {
1711+
LOG_ERR("Failed read remote features (%d)", err);
1712+
}
1713+
if (conn->state != BT_CONN_CONNECTED) {
1714+
return;
1715+
}
1716+
}
1717+
1718+
if (IS_ENABLED(CONFIG_BT_REMOTE_VERSION) &&
1719+
!atomic_test_bit(conn->flags, BT_CONN_AUTO_VERSION_INFO)) {
1720+
err = bt_hci_read_remote_version(conn);
1721+
if (err) {
1722+
LOG_ERR("Failed read remote version (%d)", err);
1723+
}
1724+
if (conn->state != BT_CONN_CONNECTED) {
1725+
return;
1726+
}
1727+
}
1728+
1729+
if (IS_ENABLED(CONFIG_BT_AUTO_PHY_UPDATE) &&
1730+
BT_FEAT_LE_PHY_2M(bt_dev.le.features) &&
1731+
!skip_auto_phy_update_on_conn_establishment(conn)) {
1732+
err = bt_le_set_phy(conn, 0U, BT_HCI_LE_PHY_PREFER_2M,
1733+
BT_HCI_LE_PHY_PREFER_2M,
1734+
BT_HCI_LE_PHY_CODED_ANY);
1735+
if (err) {
1736+
LOG_ERR("Failed LE Set PHY (%d)", err);
1737+
}
1738+
if (conn->state != BT_CONN_CONNECTED) {
1739+
return;
1740+
}
1741+
}
1742+
1743+
if (IS_ENABLED(CONFIG_BT_AUTO_DATA_LEN_UPDATE) &&
1744+
BT_FEAT_LE_DLE(bt_dev.le.features)) {
1745+
if (bt_drv_quirk_no_auto_dle()) {
1746+
uint16_t tx_octets, tx_time;
1747+
1748+
err = bt_hci_le_read_max_data_len(&tx_octets, &tx_time);
1749+
if (!err) {
1750+
err = bt_le_set_data_len(conn,
1751+
tx_octets, tx_time);
1752+
if (err) {
1753+
LOG_ERR("Failed to set data len (%d)", err);
1754+
}
1755+
}
1756+
} else {
1757+
/* No need to auto-initiate DLE procedure.
1758+
* It is done by the controller.
1759+
*/
1760+
}
1761+
}
1762+
1763+
LOG_DBG("[%p] Successfully ran auto-initiated procedures", conn);
1764+
}
1765+
1766+
/* Executes procedures after a connection is established:
1767+
* - read remote features
1768+
* - read remote version
1769+
* - update PHY
1770+
* - update data length
1771+
*/
1772+
static void auto_initiated_procedures(struct k_work *unused)
1773+
{
1774+
ARG_UNUSED(unused);
1775+
1776+
bt_conn_foreach(BT_CONN_TYPE_LE, perform_auto_initiated_procedures, NULL);
1777+
}
1778+
1779+
static K_WORK_DEFINE(procedures_on_connect, auto_initiated_procedures);
1780+
1781+
static void schedule_auto_initiated_procedures(struct bt_conn *conn)
1782+
{
1783+
LOG_DBG("[%p] Scheduling auto-init procedures", conn);
1784+
k_work_submit(&procedures_on_connect);
1785+
}
16631786

16641787
void bt_conn_connected(struct bt_conn *conn)
16651788
{
1789+
schedule_auto_initiated_procedures(conn);
16661790
bt_l2cap_connected(conn);
16671791
notify_connected(conn);
16681792
}

‎subsys/bluetooth/host/conn_internal.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@ enum {
6262
BT_CONN_BR_NOBOND, /* SSP no bond pairing tracker */
6363
BT_CONN_BR_PAIRING_INITIATOR, /* local host starts authentication */
6464
BT_CONN_CLEANUP, /* Disconnected, pending cleanup */
65+
BT_CONN_AUTO_INIT_PROCEDURES_DONE, /* Auto-initiated procedures have run */
6566
BT_CONN_PERIPHERAL_PARAM_UPDATE, /* If periph param update timer fired */
6667
BT_CONN_PERIPHERAL_PARAM_AUTO_UPDATE, /* If periph param auto update on timer fired */
6768
BT_CONN_PERIPHERAL_PARAM_SET, /* If periph param were set from app */

‎subsys/bluetooth/host/hci_core.c‎

Lines changed: 6 additions & 96 deletions
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@ static bool drv_quirk_no_reset(void)
130130
return ((BT_DT_HCI_QUIRKS_GET(DT_CHOSEN(zephyr_bt_hci)) & BT_HCI_QUIRK_NO_RESET) != 0);
131131
}
132132

133-
__maybe_unused static bool drv_quirk_no_auto_dle(void)
133+
bool bt_drv_quirk_no_auto_dle(void)
134134
{
135135
return ((BT_DT_HCI_QUIRKS_GET(DT_CHOSEN(zephyr_bt_hci)) & BT_HCI_QUIRK_NO_AUTO_DLE) != 0);
136136
}
@@ -140,7 +140,7 @@ static bool drv_quirk_no_reset(void)
140140
return ((bt_dev.drv->quirks & BT_QUIRK_NO_RESET) != 0);
141141
}
142142

143-
__maybe_unused static bool drv_quirk_no_auto_dle(void)
143+
bool bt_drv_quirk_no_auto_dle(void)
144144
{
145145
return ((bt_dev.drv->quirks & BT_QUIRK_NO_AUTO_DLE) != 0);
146146
}
@@ -493,7 +493,7 @@ int bt_hci_le_rand(void *buffer, size_t len)
493493
return 0;
494494
}
495495

496-
static int hci_le_read_max_data_len(uint16_t *tx_octets, uint16_t *tx_time)
496+
int bt_hci_le_read_max_data_len(uint16_t *tx_octets, uint16_t *tx_time)
497497
{
498498
struct bt_hci_rp_le_read_max_data_len *rp;
499499
struct net_buf *rsp;
@@ -1022,7 +1022,7 @@ static void hci_disconn_complete(struct net_buf *buf)
10221022
bt_conn_unref(conn);
10231023
}
10241024

1025-
static int hci_le_read_remote_features(struct bt_conn *conn)
1025+
int bt_hci_le_read_remote_features(struct bt_conn *conn)
10261026
{
10271027
struct bt_hci_cp_le_read_remote_features *cp;
10281028
struct net_buf *buf;
@@ -1039,7 +1039,7 @@ static int hci_le_read_remote_features(struct bt_conn *conn)
10391039
return bt_hci_cmd_send_sync(BT_HCI_OP_LE_READ_REMOTE_FEATURES, buf, NULL);
10401040
}
10411041

1042-
static int hci_read_remote_version(struct bt_conn *conn)
1042+
int bt_hci_read_remote_version(struct bt_conn *conn)
10431043
{
10441044
struct bt_hci_cp_read_remote_version_info *cp;
10451045
struct net_buf *buf;
@@ -1173,89 +1173,6 @@ static struct bt_conn *find_pending_connect(uint8_t role, bt_addr_le_t *peer_add
11731173
return NULL;
11741174
}
11751175

1176-
/* We don't want the application to get a PHY update callback upon connection
1177-
* establishment on 2M PHY. Therefore we must prevent issuing LE Set PHY
1178-
* in this scenario.
1179-
*/
1180-
static bool skip_auto_phy_update_on_conn_establishment(struct bt_conn *conn)
1181-
{
1182-
#if defined(CONFIG_BT_USER_PHY_UPDATE)
1183-
if (IS_ENABLED(CONFIG_BT_AUTO_PHY_UPDATE) &&
1184-
IS_ENABLED(CONFIG_BT_EXT_ADV) &&
1185-
BT_DEV_FEAT_LE_EXT_ADV(bt_dev.le.features)) {
1186-
if (conn->le.phy.tx_phy == BT_HCI_LE_PHY_2M &&
1187-
conn->le.phy.rx_phy == BT_HCI_LE_PHY_2M) {
1188-
return true;
1189-
}
1190-
}
1191-
#else
1192-
ARG_UNUSED(conn);
1193-
#endif /* defined(CONFIG_BT_USER_PHY_UPDATE) */
1194-
1195-
return false;
1196-
}
1197-
1198-
static void conn_auto_initiate(struct bt_conn *conn)
1199-
{
1200-
int err;
1201-
1202-
if (conn->state != BT_CONN_CONNECTED) {
1203-
/* It is possible that connection was disconnected directly from
1204-
* connected callback so we must check state before doing
1205-
* connection parameters update.
1206-
*/
1207-
return;
1208-
}
1209-
1210-
if (!atomic_test_bit(conn->flags, BT_CONN_LE_FEATURES_EXCHANGED) &&
1211-
((conn->role == BT_HCI_ROLE_CENTRAL) ||
1212-
BT_FEAT_LE_PER_INIT_FEAT_XCHG(bt_dev.le.features))) {
1213-
err = hci_le_read_remote_features(conn);
1214-
if (err) {
1215-
LOG_ERR("Failed read remote features (%d)", err);
1216-
}
1217-
}
1218-
1219-
if (IS_ENABLED(CONFIG_BT_REMOTE_VERSION) &&
1220-
!atomic_test_bit(conn->flags, BT_CONN_AUTO_VERSION_INFO)) {
1221-
err = hci_read_remote_version(conn);
1222-
if (err) {
1223-
LOG_ERR("Failed read remote version (%d)", err);
1224-
}
1225-
}
1226-
1227-
if (IS_ENABLED(CONFIG_BT_AUTO_PHY_UPDATE) &&
1228-
BT_FEAT_LE_PHY_2M(bt_dev.le.features) &&
1229-
!skip_auto_phy_update_on_conn_establishment(conn)) {
1230-
err = bt_le_set_phy(conn, 0U, BT_HCI_LE_PHY_PREFER_2M,
1231-
BT_HCI_LE_PHY_PREFER_2M,
1232-
BT_HCI_LE_PHY_CODED_ANY);
1233-
if (err) {
1234-
LOG_ERR("Failed LE Set PHY (%d)", err);
1235-
}
1236-
}
1237-
1238-
if (IS_ENABLED(CONFIG_BT_AUTO_DATA_LEN_UPDATE) &&
1239-
BT_FEAT_LE_DLE(bt_dev.le.features)) {
1240-
if (drv_quirk_no_auto_dle()) {
1241-
uint16_t tx_octets, tx_time;
1242-
1243-
err = hci_le_read_max_data_len(&tx_octets, &tx_time);
1244-
if (!err) {
1245-
err = bt_le_set_data_len(conn,
1246-
tx_octets, tx_time);
1247-
if (err) {
1248-
LOG_ERR("Failed to set data len (%d)", err);
1249-
}
1250-
}
1251-
} else {
1252-
/* No need to auto-initiate DLE procedure.
1253-
* It is done by the controller.
1254-
*/
1255-
}
1256-
}
1257-
}
1258-
12591176
static void le_conn_complete_cancel(uint8_t err)
12601177
{
12611178
int ret;
@@ -1561,10 +1478,6 @@ void bt_hci_le_enh_conn_complete(struct bt_hci_evt_le_enh_conn_complete *evt)
15611478
}
15621479

15631480
bt_conn_connected(conn);
1564-
1565-
/* Start auto-initiated procedures */
1566-
conn_auto_initiate(conn);
1567-
15681481
bt_conn_unref(conn);
15691482

15701483
if (IS_ENABLED(CONFIG_BT_CENTRAL) && conn->role == BT_HCI_ROLE_CENTRAL) {
@@ -1657,9 +1570,6 @@ void bt_hci_le_enh_conn_complete_sync(struct bt_hci_evt_le_enh_conn_complete_v2
16571570
* for peripheral connections, we need to release this reference here.
16581571
*/
16591572
bt_conn_unref(conn);
1660-
1661-
/* Start auto-initiated procedures */
1662-
conn_auto_initiate(conn);
16631573
}
16641574
#endif /* CONFIG_BT_PER_ADV_SYNC_RSP */
16651575

@@ -3630,7 +3540,7 @@ static int le_init(void)
36303540
struct bt_hci_cp_le_write_default_data_len *cp;
36313541
uint16_t tx_octets, tx_time;
36323542

3633-
err = hci_le_read_max_data_len(&tx_octets, &tx_time);
3543+
err = bt_hci_le_read_max_data_len(&tx_octets, &tx_time);
36343544
if (err) {
36353545
return err;
36363546
}

‎subsys/bluetooth/host/hci_core.h‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -564,6 +564,12 @@ void bt_hci_le_df_cte_req_failed(struct net_buf *buf);
564564
void bt_hci_le_per_adv_subevent_data_request(struct net_buf *buf);
565565
void bt_hci_le_per_adv_response_report(struct net_buf *buf);
566566

567+
int bt_hci_read_remote_version(struct bt_conn *conn);
568+
int bt_hci_le_read_remote_features(struct bt_conn *conn);
569+
int bt_hci_le_read_max_data_len(uint16_t *tx_octets, uint16_t *tx_time);
570+
571+
bool bt_drv_quirk_no_auto_dle(void);
572+
567573
void bt_tx_irq_raise(void);
568574
void bt_send_one_host_num_completed_packets(uint16_t handle);
569575
void bt_acl_set_ncp_sent(struct net_buf *packet, bool value);

0 commit comments

Comments
 (0)