diff --git a/subsys/bluetooth/controller/ll_sw/ull_conn.c b/subsys/bluetooth/controller/ll_sw/ull_conn.c index f260c71eb5d..2e68206dc9d 100644 --- a/subsys/bluetooth/controller/ll_sw/ull_conn.c +++ b/subsys/bluetooth/controller/ll_sw/ull_conn.c @@ -1672,13 +1672,8 @@ void ull_conn_done(struct node_rx_event_done *done) if (err == BT_HCI_ERR_CMD_DISALLOWED) { /* Conditions has changed e.g. PHY was changed to CODED. * New CTE REQ is not possible. Disable the periodic requests. - * - * If the CTE REQ is in active state, let it complete and disable - * in regular control procedure way. */ - if (!conn->llcp.cte_req.is_active) { - ull_cp_cte_req_set_disable(conn); - } + ull_cp_cte_req_set_disable(conn); } } } diff --git a/subsys/bluetooth/controller/ll_sw/ull_conn_types.h b/subsys/bluetooth/controller/ll_sw/ull_conn_types.h index 395624f07e2..54c7bb36b14 100644 --- a/subsys/bluetooth/controller/ll_sw/ull_conn_types.h +++ b/subsys/bluetooth/controller/ll_sw/ull_conn_types.h @@ -428,15 +428,12 @@ struct llcp_struct { /* Procedure may be active periodically, active state must be stored. * If procedure is active, request parameters update may not be issued. */ - uint8_t is_enabled:1; - uint8_t is_active:1; + volatile uint8_t is_enabled; uint8_t cte_type; /* Minimum requested CTE length in 8us units */ uint8_t min_cte_len; uint16_t req_interval; uint16_t req_expire; - void *disable_param; - void (*disable_cb)(void *param); } cte_req; #endif /* CONFIG_BT_CTLR_DF_CONN_CTE_REQ */ diff --git a/subsys/bluetooth/controller/ll_sw/ull_df.c b/subsys/bluetooth/controller/ll_sw/ull_df.c index c80cc56d433..dc2cddd3ecb 100644 --- a/subsys/bluetooth/controller/ll_sw/ull_df.c +++ b/subsys/bluetooth/controller/ll_sw/ull_df.c @@ -1123,12 +1123,12 @@ uint8_t ll_df_set_conn_cte_rx_params(uint16_t handle, uint8_t sampling_enable, } #endif /* CONFIG_BT_CTLR_DF_CONN_CTE_RX */ -#if defined(CONFIG_BT_CTLR_DF_CONN_CTE_REQ) || defined(CONFIG_BT_CTLR_DF_CONN_CTE_RSP) +#if defined(CONFIG_BT_CTLR_DF_CONN_CTE_RSP) static void df_conn_cte_req_disable(void *param) { k_sem_give(param); } -#endif /* CONFIG_BT_CTLR_DF_CONN_CTE_REQ || CONFIG_BT_CTLR_DF_CONN_CTE_RSP */ +#endif /* CONFIG_BT_CTLR_DF_CONN_CTE_RSP */ #if defined(CONFIG_BT_CTLR_DF_CONN_CTE_REQ) /* @brief Function enables or disables CTE request control procedure for a connection. @@ -1165,18 +1165,6 @@ uint8_t ll_df_set_conn_cte_req_enable(uint16_t handle, uint8_t enable, if (!enable) { ull_cp_cte_req_set_disable(conn); - if (conn->llcp.cte_req.is_active) { - struct k_sem sem; - - k_sem_init(&sem, 0U, 1U); - conn->llcp.cte_req.disable_param = &sem; - conn->llcp.cte_req.disable_cb = df_conn_cte_req_disable; - - if (!conn->llcp.cte_req.is_active) { - k_sem_take(&sem, K_FOREVER); - } - } - return BT_HCI_ERR_SUCCESS; } else { if (!conn->lll.df_rx_cfg.is_initialized) { diff --git a/subsys/bluetooth/controller/ll_sw/ull_llcp.c b/subsys/bluetooth/controller/ll_sw/ull_llcp.c index 56bb01bf80e..08bd2262dcf 100644 --- a/subsys/bluetooth/controller/ll_sw/ull_llcp.c +++ b/subsys/bluetooth/controller/ll_sw/ull_llcp.c @@ -485,9 +485,6 @@ void ull_llcp_init(struct ll_conn *conn) #if defined(CONFIG_BT_CTLR_DF_CONN_CTE_REQ) conn->llcp.cte_req.is_enabled = 0U; conn->llcp.cte_req.req_expire = 0U; - conn->llcp.cte_req.is_active = 0U; - conn->llcp.cte_req.disable_param = NULL; - conn->llcp.cte_req.disable_cb = NULL; #endif /* CONFIG_BT_CTLR_DF_CONN_CTE_REQ */ #if defined(CONFIG_BT_CTLR_DF_CONN_CTE_RSP) conn->llcp.cte_rsp.is_enabled = 0U; @@ -981,8 +978,6 @@ uint8_t ull_cp_cte_req(struct ll_conn *conn, uint8_t min_cte_len, uint8_t cte_ty ctx->data.cte_req.min_len = min_cte_len; ctx->data.cte_req.type = cte_type; - conn->llcp.cte_req.is_active = 1U; - llcp_lr_enqueue(conn, ctx); return BT_HCI_ERR_SUCCESS; diff --git a/subsys/bluetooth/controller/ll_sw/ull_llcp_common.c b/subsys/bluetooth/controller/ll_sw/ull_llcp_common.c index b781807b633..b26a0ced761 100644 --- a/subsys/bluetooth/controller/ll_sw/ull_llcp_common.c +++ b/subsys/bluetooth/controller/ll_sw/ull_llcp_common.c @@ -93,6 +93,9 @@ enum { RP_COMMON_EVT_REQUEST, }; +static void lp_comm_ntf(struct ll_conn *conn, struct proc_ctx *ctx); +static void lp_comm_terminate_invalid_pdu(struct ll_conn *conn, struct proc_ctx *ctx); + /* * LLCP Local Procedure Common FSM */ @@ -205,23 +208,6 @@ static void lp_comm_complete_cte_req_finalize(struct ll_conn *conn) { llcp_rr_set_paused_cmd(conn, PROC_NONE); llcp_lr_complete(conn); - - conn->llcp.cte_req.is_active = 0U; - - /* Disable the CTE request procedure when it is completed in case it was executed as - * non-periodic. - */ - if (conn->llcp.cte_req.req_interval == 0U) { - conn->llcp.cte_req.is_enabled = 0U; - } - - /* If disable_cb is not NULL then there is waiting CTE REQ disable request - * from host. Execute the callback to notify waiting thread that the - * procedure is inactive. - */ - if (conn->llcp.cte_req.disable_cb) { - conn->llcp.cte_req.disable_cb(conn->llcp.cte_req.disable_param); - } } static void lp_comm_ntf_cte_req(struct ll_conn *conn, struct proc_ctx *ctx, struct pdu_data *pdu) @@ -241,6 +227,56 @@ static void lp_comm_ntf_cte_req(struct ll_conn *conn, struct proc_ctx *ctx, stru LL_ASSERT(0); } } + +static void lp_comm_complete_cte_req(struct ll_conn *conn, struct proc_ctx *ctx) +{ + if (conn->llcp.cte_req.is_enabled) { + if (ctx->response_opcode == PDU_DATA_LLCTRL_TYPE_CTE_RSP) { + if (ctx->data.cte_remote_rsp.has_cte) { + if (conn->llcp.cte_req.req_interval != 0U) { + conn->llcp.cte_req.req_expire = + conn->llcp.cte_req.req_interval; + } + ctx->state = LP_COMMON_STATE_IDLE; + } else if (llcp_ntf_alloc_is_available()) { + lp_comm_ntf(conn, ctx); + ull_cp_cte_req_set_disable(conn); + ctx->state = LP_COMMON_STATE_IDLE; + } else { + ctx->state = LP_COMMON_STATE_WAIT_NTF; + } + } else if (ctx->response_opcode == PDU_DATA_LLCTRL_TYPE_REJECT_EXT_IND && + ctx->reject_ext_ind.reject_opcode == PDU_DATA_LLCTRL_TYPE_CTE_REQ) { + if (llcp_ntf_alloc_is_available()) { + lp_comm_ntf(conn, ctx); + ull_cp_cte_req_set_disable(conn); + ctx->state = LP_COMMON_STATE_IDLE; + } else { + ctx->state = LP_COMMON_STATE_WAIT_NTF; + } + } else if (ctx->response_opcode == PDU_DATA_LLCTRL_TYPE_UNUSED) { + /* This path is related with handling disable the CTE REQ when PHY + * has been changed to CODED PHY. BT 5.3 Core Vol 4 Part E 7.8.85 + * says CTE REQ has to be automatically disabled as if it had been requested + * by Host. There is no notification send to Host. + */ + ull_cp_cte_req_set_disable(conn); + ctx->state = LP_COMMON_STATE_IDLE; + } else { + /* Illegal response opcode */ + lp_comm_terminate_invalid_pdu(conn, ctx); + } + } else { + /* The CTE_REQ was disabled by Host after the request was send. + * It does not matter if response has arrived, it should not be handled. + */ + ctx->state = LP_COMMON_STATE_IDLE; + } + + if (ctx->state == LP_COMMON_STATE_IDLE) { + lp_comm_complete_cte_req_finalize(conn); + } +} #endif /* CONFIG_BT_CTLR_DF_CONN_CTE_REQ */ static void lp_comm_ntf(struct ll_conn *conn, struct proc_ctx *ctx) @@ -388,45 +424,7 @@ static void lp_comm_complete(struct ll_conn *conn, struct proc_ctx *ctx, uint8_t #endif /* CONFIG_BT_CTLR_DATA_LENGTH */ #if defined(CONFIG_BT_CTLR_DF_CONN_CTE_REQ) case PROC_CTE_REQ: - if (ctx->response_opcode == PDU_DATA_LLCTRL_TYPE_CTE_RSP) { - if (ctx->data.cte_remote_rsp.has_cte) { - if (conn->llcp.cte_req.req_interval != 0U) { - conn->llcp.cte_req.req_expire = - conn->llcp.cte_req.req_interval; - } - ctx->state = LP_COMMON_STATE_IDLE; - } else if (llcp_ntf_alloc_is_available()) { - lp_comm_ntf(conn, ctx); - ull_cp_cte_req_set_disable(conn); - ctx->state = LP_COMMON_STATE_IDLE; - } else { - ctx->state = LP_COMMON_STATE_WAIT_NTF; - } - } else if (ctx->response_opcode == PDU_DATA_LLCTRL_TYPE_REJECT_EXT_IND && - ctx->reject_ext_ind.reject_opcode == PDU_DATA_LLCTRL_TYPE_CTE_REQ) { - if (llcp_ntf_alloc_is_available()) { - lp_comm_ntf(conn, ctx); - ull_cp_cte_req_set_disable(conn); - ctx->state = LP_COMMON_STATE_IDLE; - } else { - ctx->state = LP_COMMON_STATE_WAIT_NTF; - } - } else if (ctx->response_opcode == PDU_DATA_LLCTRL_TYPE_UNUSED) { - /* This path is related with handling disable the CTE REQ when PHY - * has been changed to CODED PHY. BT 5.3 Core Vol 4 Part E 7.8.85 - * says CTE REQ has to be automatically disabled as if it had been requested - * by Host. There is no notification send to Host. - */ - ull_cp_cte_req_set_disable(conn); - ctx->state = LP_COMMON_STATE_IDLE; - } else { - /* Illegal response opcode */ - lp_comm_terminate_invalid_pdu(conn, ctx); - } - - if (ctx->state == LP_COMMON_STATE_IDLE) { - lp_comm_complete_cte_req_finalize(conn); - } + lp_comm_complete_cte_req(conn, ctx); break; #endif /* CONFIG_BT_CTLR_DF_CONN_CTE_REQ */ default: @@ -518,10 +516,11 @@ static void lp_comm_send_req(struct ll_conn *conn, struct proc_ctx *ctx, uint8_t #endif /* CONFIG_BT_CTLR_DATA_LENGTH */ #if defined(CONFIG_BT_CTLR_DF_CONN_CTE_REQ) case PROC_CTE_REQ: + if (conn->llcp.cte_req.is_enabled && #if defined(CONFIG_BT_CTLR_PHY) - if (conn->lll.phy_rx != PHY_CODED) { + conn->lll.phy_rx != PHY_CODED) { #else - if (1) { + 1) { #endif /* CONFIG_BT_CTLR_PHY */ if (llcp_lr_ispaused(conn) || !llcp_tx_alloc_peek(conn, ctx) || (llcp_rr_get_paused_cmd(conn) == PROC_CTE_REQ)) { diff --git a/tests/bluetooth/controller/ctrl_cte_req/src/main.c b/tests/bluetooth/controller/ctrl_cte_req/src/main.c index 7474890401b..bd40462a1fb 100644 --- a/tests/bluetooth/controller/ctrl_cte_req/src/main.c +++ b/tests/bluetooth/controller/ctrl_cte_req/src/main.c @@ -84,7 +84,10 @@ void test_cte_req_central_local(void) /* Connect */ ull_cp_state_set(&conn, ULL_CP_CONNECTED); + /* Initiate an CTE Request Procedure */ + conn.llcp.cte_req.is_enabled = 1U; + err = ull_cp_cte_req(&conn, local_cte_req.min_cte_len_req, local_cte_req.cte_type_req); zassert_equal(err, BT_HCI_ERR_SUCCESS, NULL); @@ -261,6 +264,8 @@ void test_cte_req_peripheral_local(void) ull_cp_state_set(&conn, ULL_CP_CONNECTED); /* Initiate an CTE Request Procedure */ + conn.llcp.cte_req.is_enabled = 1U; + err = ull_cp_cte_req(&conn, local_cte_req.min_cte_len_req, local_cte_req.cte_type_req); zassert_equal(err, BT_HCI_ERR_SUCCESS, NULL); @@ -475,6 +480,8 @@ void test_cte_req_rejected_inv_ll_param_central_local(void) ull_cp_state_set(&conn, ULL_CP_CONNECTED); /* Initiate an CTE Request Procedure */ + conn.llcp.cte_req.is_enabled = 1U; + err = ull_cp_cte_req(&conn, local_cte_req.min_cte_len_req, local_cte_req.cte_type_req); zassert_equal(err, BT_HCI_ERR_SUCCESS, NULL); @@ -549,6 +556,8 @@ void test_cte_req_rejected_inv_ll_param_peripheral_local(void) ull_cp_state_set(&conn, ULL_CP_CONNECTED); /* Initiate an CTE Request Procedure */ + conn.llcp.cte_req.is_enabled = 1U; + err = ull_cp_cte_req(&conn, local_cte_req.min_cte_len_req, local_cte_req.cte_type_req); zassert_equal(err, BT_HCI_ERR_SUCCESS, NULL); @@ -1124,6 +1133,8 @@ static void test_local_cte_req_wait_for_phy_update_complete_and_disable(uint8_t zassert_equal(err, BT_HCI_ERR_SUCCESS, NULL); /* Initiate an CTE Request Procedure */ + conn.llcp.cte_req.is_enabled = 1U; + err = ull_cp_cte_req(&conn, local_cte_req.min_cte_len_req, local_cte_req.cte_type_req); zassert_equal(err, BT_HCI_ERR_SUCCESS, NULL); @@ -1189,6 +1200,8 @@ static void test_local_cte_req_wait_for_phy_update_complete(uint8_t role) zassert_equal(err, BT_HCI_ERR_SUCCESS, NULL); /* Initiate an CTE Request Procedure */ + conn.llcp.cte_req.is_enabled = 1U; + err = ull_cp_cte_req(&conn, local_cte_req.min_cte_len_req, local_cte_req.cte_type_req); zassert_equal(err, BT_HCI_ERR_SUCCESS, NULL); @@ -1235,6 +1248,8 @@ static void test_local_phy_update_wait_for_cte_req_complete(uint8_t role) ull_cp_state_set(&conn, ULL_CP_CONNECTED); /* Initiate an CTE Request Procedure */ + conn.llcp.cte_req.is_enabled = 1U; + err = ull_cp_cte_req(&conn, local_cte_req.min_cte_len_req, local_cte_req.cte_type_req); zassert_equal(err, BT_HCI_ERR_SUCCESS, NULL); @@ -1385,6 +1400,8 @@ static void test_cte_req_wait_for_remote_phy_update_complete_and_disable(uint8_t event_done(&conn); /* Initiate an CTE Request Procedure */ + conn.llcp.cte_req.is_enabled = 1U; + err = ull_cp_cte_req(&conn, local_cte_req.min_cte_len_req, local_cte_req.cte_type_req); zassert_equal(err, BT_HCI_ERR_SUCCESS, NULL); @@ -1439,6 +1456,8 @@ static void test_cte_req_wait_for_remote_phy_update_complete(uint8_t role) event_done(&conn); /* Initiate an CTE Request Procedure */ + conn.llcp.cte_req.is_enabled = 1U; + err = ull_cp_cte_req(&conn, local_cte_req.min_cte_len_req, local_cte_req.cte_type_req); zassert_equal(err, BT_HCI_ERR_SUCCESS, NULL);