Skip to content

Commit

Permalink
[wip] Bluetooth: conn: move auto-init procedures to syswq
Browse files Browse the repository at this point in the history
TBD

Signed-off-by: Jonathan Rico <[email protected]>
  • Loading branch information
jori-nordic committed Aug 28, 2024
1 parent ad70ff7 commit 2c2a47f
Show file tree
Hide file tree
Showing 4 changed files with 127 additions and 93 deletions.
103 changes: 103 additions & 0 deletions subsys/bluetooth/host/conn.c
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,10 @@ static void tx_notify(struct bt_conn *conn)
}
#endif /* CONFIG_BT_CONN_TX */

#if defined(CONFIG_BT_CONN)
static void auto_initiated_procedures(struct k_work *work);
#endif /* defined(CONFIG_BT_CONN) */

Check notice on line 308 in subsys/bluetooth/host/conn.c

View workflow job for this annotation

GitHub Actions / Run compliance checks on patch series (PR)

You may want to run clang-format on this change

subsys/bluetooth/host/conn.c:308 -#endif /* defined(CONFIG_BT_CONN) */ +#endif /* defined(CONFIG_BT_CONN) */
struct bt_conn *bt_conn_new(struct bt_conn *conns, size_t size)
{
struct bt_conn *conn = NULL;
Expand All @@ -322,6 +326,7 @@ struct bt_conn *bt_conn_new(struct bt_conn *conns, size_t size)

#if defined(CONFIG_BT_CONN)
k_work_init_delayable(&conn->deferred_work, deferred_work);
k_work_init(&conn->auto_initiated_procedures, auto_initiated_procedures);
#endif /* CONFIG_BT_CONN */
#if defined(CONFIG_BT_CONN_TX)
k_work_init(&conn->tx_complete_work, tx_complete_work);
Expand Down Expand Up @@ -1153,6 +1158,104 @@ struct bt_conn *conn_lookup_handle(struct bt_conn *conns, size_t size,
return NULL;
}

#if defined(CONFIG_BT_CONN)
/* We don't want the application to get a PHY update callback upon connection
* establishment on 2M PHY. Therefore we must prevent issuing LE Set PHY
* in this scenario.
*/
static bool skip_auto_phy_update_on_conn_establishment(struct bt_conn *conn)
{
#if defined(CONFIG_BT_USER_PHY_UPDATE)
if (IS_ENABLED(CONFIG_BT_AUTO_PHY_UPDATE) &&
IS_ENABLED(CONFIG_BT_EXT_ADV) &&
BT_DEV_FEAT_LE_EXT_ADV(bt_dev.le.features)) {

Check notice on line 1171 in subsys/bluetooth/host/conn.c

View workflow job for this annotation

GitHub Actions / Run compliance checks on patch series (PR)

You may want to run clang-format on this change

subsys/bluetooth/host/conn.c:1171 - if (IS_ENABLED(CONFIG_BT_AUTO_PHY_UPDATE) && - IS_ENABLED(CONFIG_BT_EXT_ADV) && + if (IS_ENABLED(CONFIG_BT_AUTO_PHY_UPDATE) && IS_ENABLED(CONFIG_BT_EXT_ADV) &&
if (conn->le.phy.tx_phy == BT_HCI_LE_PHY_2M &&
conn->le.phy.rx_phy == BT_HCI_LE_PHY_2M) {
return true;
}
}
#else
ARG_UNUSED(conn);
#endif /* defined(CONFIG_BT_USER_PHY_UPDATE) */

return false;
}

static void auto_initiated_procedures(struct k_work *work)
{
int err;
struct bt_conn *conn = CONTAINER_OF(work, struct bt_conn, auto_initiated_procedures);

if (conn->state != BT_CONN_CONNECTED) {
/* It is possible that connection was disconnected directly from
* connected callback so we must check state before doing
* connection parameters update.
*/
goto exit;
}

if (!atomic_test_bit(conn->flags, BT_CONN_AUTO_FEATURE_EXCH) &&
((conn->role == BT_HCI_ROLE_CENTRAL) ||
BT_FEAT_LE_PER_INIT_FEAT_XCHG(bt_dev.le.features))) {
err = bt_hci_le_read_remote_features(conn);
if (err) {
LOG_ERR("Failed read remote features (%d)", err);
}
if (conn->state != BT_CONN_CONNECTED) {
goto exit;
}
}

if (IS_ENABLED(CONFIG_BT_REMOTE_VERSION) &&
!atomic_test_bit(conn->flags, BT_CONN_AUTO_VERSION_INFO)) {
err = bt_hci_read_remote_version(conn);
if (err) {
LOG_ERR("Failed read remote version (%d)", err);
}
if (conn->state != BT_CONN_CONNECTED) {
goto exit;
}
}

if (IS_ENABLED(CONFIG_BT_AUTO_PHY_UPDATE) &&
BT_FEAT_LE_PHY_2M(bt_dev.le.features) &&
!skip_auto_phy_update_on_conn_establishment(conn)) {
err = bt_le_set_phy(conn, 0U, BT_HCI_LE_PHY_PREFER_2M,
BT_HCI_LE_PHY_PREFER_2M,
BT_HCI_LE_PHY_CODED_ANY);

Check notice on line 1225 in subsys/bluetooth/host/conn.c

View workflow job for this annotation

GitHub Actions / Run compliance checks on patch series (PR)

You may want to run clang-format on this change

subsys/bluetooth/host/conn.c:1225 - if (IS_ENABLED(CONFIG_BT_AUTO_PHY_UPDATE) && - BT_FEAT_LE_PHY_2M(bt_dev.le.features) && + if (IS_ENABLED(CONFIG_BT_AUTO_PHY_UPDATE) && BT_FEAT_LE_PHY_2M(bt_dev.le.features) && !skip_auto_phy_update_on_conn_establishment(conn)) { - err = bt_le_set_phy(conn, 0U, BT_HCI_LE_PHY_PREFER_2M, - BT_HCI_LE_PHY_PREFER_2M, + err = bt_le_set_phy(conn, 0U, BT_HCI_LE_PHY_PREFER_2M, BT_HCI_LE_PHY_PREFER_2M,
if (err) {
LOG_ERR("Failed LE Set PHY (%d)", err);
}
if (conn->state != BT_CONN_CONNECTED) {
goto exit;
}
}

if (IS_ENABLED(CONFIG_BT_AUTO_DATA_LEN_UPDATE) &&
BT_FEAT_LE_DLE(bt_dev.le.features)) {
if (bt_drv_quirk_no_auto_dle()) {
uint16_t tx_octets, tx_time;

err = bt_hci_le_read_max_data_len(&tx_octets, &tx_time);
if (!err) {
err = bt_le_set_data_len(conn,
tx_octets, tx_time);
if (err) {

Check notice on line 1243 in subsys/bluetooth/host/conn.c

View workflow job for this annotation

GitHub Actions / Run compliance checks on patch series (PR)

You may want to run clang-format on this change

subsys/bluetooth/host/conn.c:1243 - if (IS_ENABLED(CONFIG_BT_AUTO_DATA_LEN_UPDATE) && - BT_FEAT_LE_DLE(bt_dev.le.features)) { + if (IS_ENABLED(CONFIG_BT_AUTO_DATA_LEN_UPDATE) && BT_FEAT_LE_DLE(bt_dev.le.features)) { if (bt_drv_quirk_no_auto_dle()) { uint16_t tx_octets, tx_time; err = bt_hci_le_read_max_data_len(&tx_octets, &tx_time); if (!err) { - err = bt_le_set_data_len(conn, - tx_octets, tx_time); + err = bt_le_set_data_len(conn, tx_octets, tx_time);
LOG_ERR("Failed to set data len (%d)", err);
}
}
} else {
/* No need to auto-initiate DLE procedure.
* It is done by the controller.
*/
}
}

exit:
bt_conn_unref(conn);
}
#endif /* defined(CONFIG_BT_CONN) */

Check notice on line 1258 in subsys/bluetooth/host/conn.c

View workflow job for this annotation

GitHub Actions / Run compliance checks on patch series (PR)

You may want to run clang-format on this change

subsys/bluetooth/host/conn.c:1258 -#endif /* defined(CONFIG_BT_CONN) */ +#endif /* defined(CONFIG_BT_CONN) */
void bt_conn_set_state(struct bt_conn *conn, bt_conn_state_t state)
{
bt_conn_state_t old_state;
Expand Down
8 changes: 8 additions & 0 deletions subsys/bluetooth/host/conn_internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,14 @@ struct bt_conn {
*/
struct k_work_delayable deferred_work;

/* Executes procedures after a connection is established:
* - read remote features
* - read remote version
* - update PHY
* - update data length
*/
struct k_work auto_initiated_procedures;

union {
struct bt_conn_le le;
#if defined(CONFIG_BT_CLASSIC)
Expand Down
103 changes: 10 additions & 93 deletions subsys/bluetooth/host/hci_core.c
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ static bool drv_quirk_no_reset(void)
return ((BT_DT_HCI_QUIRKS_GET(DT_CHOSEN(zephyr_bt_hci)) & BT_HCI_QUIRK_NO_RESET) != 0);
}

__maybe_unused static bool drv_quirk_no_auto_dle(void)
__maybe_unused bool bt_drv_quirk_no_auto_dle(void)
{
return ((BT_DT_HCI_QUIRKS_GET(DT_CHOSEN(zephyr_bt_hci)) & BT_HCI_QUIRK_NO_AUTO_DLE) != 0);
}
Expand All @@ -140,7 +140,7 @@ static bool drv_quirk_no_reset(void)
return ((bt_dev.drv->quirks & BT_QUIRK_NO_RESET) != 0);
}

__maybe_unused static bool drv_quirk_no_auto_dle(void)
__maybe_unused bool bt_drv_quirk_no_auto_dle(void)
{
return ((bt_dev.drv->quirks & BT_QUIRK_NO_AUTO_DLE) != 0);
}
Expand Down Expand Up @@ -493,7 +493,7 @@ int bt_hci_le_rand(void *buffer, size_t len)
return 0;
}

static int hci_le_read_max_data_len(uint16_t *tx_octets, uint16_t *tx_time)
int bt_hci_le_read_max_data_len(uint16_t *tx_octets, uint16_t *tx_time)
{
struct bt_hci_rp_le_read_max_data_len *rp;
struct net_buf *rsp;
Expand Down Expand Up @@ -1022,7 +1022,7 @@ static void hci_disconn_complete(struct net_buf *buf)
bt_conn_unref(conn);
}

static int hci_le_read_remote_features(struct bt_conn *conn)
int bt_hci_le_read_remote_features(struct bt_conn *conn)
{
struct bt_hci_cp_le_read_remote_features *cp;
struct net_buf *buf;
Expand All @@ -1038,7 +1038,7 @@ static int hci_le_read_remote_features(struct bt_conn *conn)
return bt_hci_cmd_send_sync(BT_HCI_OP_LE_READ_REMOTE_FEATURES, buf, NULL);
}

static int hci_read_remote_version(struct bt_conn *conn)
int bt_hci_read_remote_version(struct bt_conn *conn)
{
struct bt_hci_cp_read_remote_version_info *cp;
struct net_buf *buf;
Expand Down Expand Up @@ -1172,89 +1172,6 @@ static struct bt_conn *find_pending_connect(uint8_t role, bt_addr_le_t *peer_add
return NULL;
}

/* We don't want the application to get a PHY update callback upon connection
* establishment on 2M PHY. Therefore we must prevent issuing LE Set PHY
* in this scenario.
*/
static bool skip_auto_phy_update_on_conn_establishment(struct bt_conn *conn)
{
#if defined(CONFIG_BT_USER_PHY_UPDATE)
if (IS_ENABLED(CONFIG_BT_AUTO_PHY_UPDATE) &&
IS_ENABLED(CONFIG_BT_EXT_ADV) &&
BT_DEV_FEAT_LE_EXT_ADV(bt_dev.le.features)) {
if (conn->le.phy.tx_phy == BT_HCI_LE_PHY_2M &&
conn->le.phy.rx_phy == BT_HCI_LE_PHY_2M) {
return true;
}
}
#else
ARG_UNUSED(conn);
#endif /* defined(CONFIG_BT_USER_PHY_UPDATE) */

return false;
}

static void conn_auto_initiate(struct bt_conn *conn)
{
int err;

if (conn->state != BT_CONN_CONNECTED) {
/* It is possible that connection was disconnected directly from
* connected callback so we must check state before doing
* connection parameters update.
*/
return;
}

if (!atomic_test_bit(conn->flags, BT_CONN_AUTO_FEATURE_EXCH) &&
((conn->role == BT_HCI_ROLE_CENTRAL) ||
BT_FEAT_LE_PER_INIT_FEAT_XCHG(bt_dev.le.features))) {
err = hci_le_read_remote_features(conn);
if (err) {
LOG_ERR("Failed read remote features (%d)", err);
}
}

if (IS_ENABLED(CONFIG_BT_REMOTE_VERSION) &&
!atomic_test_bit(conn->flags, BT_CONN_AUTO_VERSION_INFO)) {
err = hci_read_remote_version(conn);
if (err) {
LOG_ERR("Failed read remote version (%d)", err);
}
}

if (IS_ENABLED(CONFIG_BT_AUTO_PHY_UPDATE) &&
BT_FEAT_LE_PHY_2M(bt_dev.le.features) &&
!skip_auto_phy_update_on_conn_establishment(conn)) {
err = bt_le_set_phy(conn, 0U, BT_HCI_LE_PHY_PREFER_2M,
BT_HCI_LE_PHY_PREFER_2M,
BT_HCI_LE_PHY_CODED_ANY);
if (err) {
LOG_ERR("Failed LE Set PHY (%d)", err);
}
}

if (IS_ENABLED(CONFIG_BT_AUTO_DATA_LEN_UPDATE) &&
BT_FEAT_LE_DLE(bt_dev.le.features)) {
if (drv_quirk_no_auto_dle()) {
uint16_t tx_octets, tx_time;

err = hci_le_read_max_data_len(&tx_octets, &tx_time);
if (!err) {
err = bt_le_set_data_len(conn,
tx_octets, tx_time);
if (err) {
LOG_ERR("Failed to set data len (%d)", err);
}
}
} else {
/* No need to auto-initiate DLE procedure.
* It is done by the controller.
*/
}
}
}

static void le_conn_complete_cancel(uint8_t err)
{
int ret;
Expand Down Expand Up @@ -1561,8 +1478,8 @@ void bt_hci_le_enh_conn_complete(struct bt_hci_evt_le_enh_conn_complete *evt)

bt_conn_connected(conn);

/* Start auto-initiated procedures */
conn_auto_initiate(conn);
/* Schedule auto-initiated procedures */
k_work_submit(&bt_conn_ref(conn)->auto_initiated_procedures);

bt_conn_unref(conn);

Expand Down Expand Up @@ -1657,8 +1574,8 @@ void bt_hci_le_enh_conn_complete_sync(struct bt_hci_evt_le_enh_conn_complete_v2
*/
bt_conn_unref(conn);

/* Start auto-initiated procedures */
conn_auto_initiate(conn);
/* Schedule auto-initiated procedures */
k_work_submit(&bt_conn_ref(conn)->auto_initiated_procedures);
}
#endif /* CONFIG_BT_PER_ADV_SYNC_RSP */

Expand Down Expand Up @@ -3629,7 +3546,7 @@ static int le_init(void)
struct bt_hci_cp_le_write_default_data_len *cp;
uint16_t tx_octets, tx_time;

err = hci_le_read_max_data_len(&tx_octets, &tx_time);
err = bt_hci_le_read_max_data_len(&tx_octets, &tx_time);
if (err) {
return err;
}
Expand Down
6 changes: 6 additions & 0 deletions subsys/bluetooth/host/hci_core.h
Original file line number Diff line number Diff line change
Expand Up @@ -564,6 +564,12 @@ void bt_hci_le_df_cte_req_failed(struct net_buf *buf);
void bt_hci_le_per_adv_subevent_data_request(struct net_buf *buf);
void bt_hci_le_per_adv_response_report(struct net_buf *buf);

int bt_hci_read_remote_version(struct bt_conn *conn);
int bt_hci_le_read_remote_features(struct bt_conn *conn);
int bt_hci_le_read_max_data_len(uint16_t *tx_octets, uint16_t *tx_time);

bool bt_drv_quirk_no_auto_dle(void);

void bt_tx_irq_raise(void);
void bt_send_one_host_num_completed_packets(uint16_t handle);
void bt_acl_set_ncp_sent(struct net_buf *packet, bool value);

0 comments on commit 2c2a47f

Please sign in to comment.