From aa4819994e8cdc34dc1d6b7803588f6f3d908c22 Mon Sep 17 00:00:00 2001 From: Piotr Pryga Date: Tue, 18 Jan 2022 11:42:30 +0100 Subject: [PATCH] Bluetooth: Controller: Add packet TX restrictions for CTE REQ/RSP PDUs There are packet restrictions imposed by PHY update procedure for PDU that includes CTE (BT Core 5.3 : - central/peripheral can't send PDU with CTE after receive or send LLPHY_UPDATE_IND until instant if the PHY after instant is CODED - peripheral can't send PDU including CTE after it sends LL_PHY_REQ PDU until receive LL_PHY_UPDATE_IND, LL_UNKNOWN_RSP, LL_REJECTED_EXT_- IND_PDU if there is a CODE PHY in TX_PHYS - peripheral can't send PDU including CTE after it sends LL_PHY_RSP PDU until receive LL_PHY_UPDATE_IND if there is a CODED PHY in TX_PHYS of LL_PHY_RSP PDU or RX_PHYS of LL_PHY_REQ PDU. The BT 5.3 Core spec defines only one PDU that may include CTE, that is LL_CTE_RSP PDU. To avoid a situation that there is such PDU enqueued for transmission in LLL when packet transmission restrictions should be applied, both procedures in almost all cases will not be executed in parallel. Current implementation always handles remote procedurerequest first. There are possible three scenarios: 1. Remotely requested PHY update. Locally initiated CTE REQ. In this case there is no problem with LL_CTE_RSP waiting in a TX queue in LLL. Both procedures may be executed one after another. 2. Remotely requested CTE REQ. Locally initiated PHY update. In this case the CTE REQ is handled first and it will pause the PHY update procedure until LL_CTE_RSP PDU is acknowledged by remote. Then the CTE REQ procedure will be completed and PHY update continued. 3. Locally initiated PHY update is pending. Arrives remote CTE REQ. In this case the CTE REQ will be paused until localy initiated PHY update is completed. Then the CTE REQ will be continued. Thanks to that there should be no PDU including CTE in LLL TX quueue. That releases us from a situation there is a LL_CTE_RSP PDU in the LLL TX qeueue that must be changed into LL_REJECT_EXT_IND PDU due to change of PHY to CODED PHY after PHY update procedure completes. Signed-off-by: Piotr Pryga --- .../controller/ll_sw/ull_conn_types.h | 3 ++ subsys/bluetooth/controller/ll_sw/ull_llcp.c | 3 ++ .../controller/ll_sw/ull_llcp_common.c | 28 +++++++++++++++---- .../controller/ll_sw/ull_llcp_internal.h | 4 +++ .../bluetooth/controller/ll_sw/ull_llcp_phy.c | 10 +++++-- .../controller/ll_sw/ull_llcp_remote.c | 21 ++++++++++++++ 6 files changed, 61 insertions(+), 8 deletions(-) diff --git a/subsys/bluetooth/controller/ll_sw/ull_conn_types.h b/subsys/bluetooth/controller/ll_sw/ull_conn_types.h index 0da3d2a3c71..b5eca9774a8 100644 --- a/subsys/bluetooth/controller/ll_sw/ull_conn_types.h +++ b/subsys/bluetooth/controller/ll_sw/ull_conn_types.h @@ -367,6 +367,9 @@ struct llcp_struct { uint8_t collision; uint8_t incompat; uint8_t reject_opcode; +#if defined(CONFIG_BT_CTLR_DF_CONN_CTE_RSP) || defined(CONFIG_BT_CTLR_DF_CONN_CTE_REQ) + uint8_t paused_cmd; +#endif /* CONFIG_BT_CTLR_DF_CONN_CTE_RSP || CONFIG_BT_CTLR_DF_CONN_CTE_REQ */ } remote; /* Prepare parameters */ diff --git a/subsys/bluetooth/controller/ll_sw/ull_llcp.c b/subsys/bluetooth/controller/ll_sw/ull_llcp.c index fc3f57a87a1..d65ef4b09f5 100644 --- a/subsys/bluetooth/controller/ll_sw/ull_llcp.c +++ b/subsys/bluetooth/controller/ll_sw/ull_llcp.c @@ -433,6 +433,9 @@ void ull_llcp_init(struct ll_conn *conn) sys_slist_init(&conn->llcp.remote.pend_proc_list); conn->llcp.remote.incompat = INCOMPAT_NO_COLLISION; conn->llcp.remote.collision = 0U; +#if defined(CONFIG_BT_CTLR_DF_CONN_CTE_RSP) + conn->llcp.remote.paused_cmd = PROC_NONE; +#endif /* CONFIG_BT_CTLR_DF_CONN_CTE_RSP */ /* Reset the cached version Information (PROC_VERSION_EXCHANGE) */ memset(&conn->llcp.vex, 0, sizeof(conn->llcp.vex)); diff --git a/subsys/bluetooth/controller/ll_sw/ull_llcp_common.c b/subsys/bluetooth/controller/ll_sw/ull_llcp_common.c index acfbf6367ea..9baa10b6a0c 100644 --- a/subsys/bluetooth/controller/ll_sw/ull_llcp_common.c +++ b/subsys/bluetooth/controller/ll_sw/ull_llcp_common.c @@ -365,6 +365,7 @@ static void lp_comm_complete(struct ll_conn *conn, struct proc_ctx *ctx, uint8_t } if (ctx->state == LP_COMMON_STATE_IDLE) { + llcp_rr_set_paused_cmd(conn, PROC_NONE); llcp_lr_complete(conn); } break; @@ -458,7 +459,8 @@ 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 (ctx->pause || !llcp_tx_alloc_peek(conn, ctx)) { + if (ctx->pause || !llcp_tx_alloc_peek(conn, ctx) || + (llcp_rr_get_paused_cmd(conn) == PROC_CTE_REQ)) { ctx->state = LP_COMMON_STATE_WAIT_TX; } else { lp_comm_tx(conn, ctx); @@ -776,6 +778,9 @@ static void rp_comm_tx(struct ll_conn *conn, struct proc_ctx *ctx) llcp_pdu_encode_reject_ext_ind(pdu, PDU_DATA_LLCTRL_TYPE_CTE_REQ, err_code); ctx->rx_opcode = PDU_DATA_LLCTRL_TYPE_REJECT_EXT_IND; } + + ctx->tx_ack = tx; + break; } #endif /* CONFIG_BT_CTLR_DF_CONN_CTE_RSP */ @@ -934,12 +939,13 @@ static void rp_comm_send_rsp(struct ll_conn *conn, struct proc_ctx *ctx, uint8_t #endif /* CONFIG_BT_CTLR_DATA_LENGTH */ #if defined(CONFIG_BT_CTLR_DF_CONN_CTE_RSP) case PROC_CTE_REQ: - if (ctx->pause || !llcp_tx_alloc_peek(conn, ctx)) { + if (ctx->pause || !llcp_tx_alloc_peek(conn, ctx) || + (llcp_rr_get_paused_cmd(conn) == PROC_CTE_REQ)) { ctx->state = RP_COMMON_STATE_WAIT_TX; } else { + llcp_rr_set_paused_cmd(conn, PROC_PHY_UPDATE); rp_comm_tx(conn, ctx); - llcp_rr_complete(conn); - ctx->state = RP_COMMON_STATE_IDLE; + ctx->state = RP_COMMON_STATE_WAIT_TX_ACK; } break; #endif /* CONFIG_BT_CTLR_DF_CONN_CTE_RSP */ @@ -974,13 +980,13 @@ static void rp_comm_st_wait_tx(struct ll_conn *conn, struct proc_ctx *ctx, uint8 } } -#if defined(CONFIG_BT_CTLR_DATA_LENGTH) static void rp_comm_st_wait_tx_ack(struct ll_conn *conn, struct proc_ctx *ctx, uint8_t evt, void *param) { switch (evt) { case RP_COMMON_EVT_ACK: switch (ctx->proc) { +#if defined(CONFIG_BT_CTLR_DATA_LENGTH) case PROC_DATA_LENGTH_UPDATE: { /* Apply changes in data lengths/times */ uint8_t dle_changed = ull_dle_update_eff(conn); @@ -998,6 +1004,15 @@ static void rp_comm_st_wait_tx_ack(struct ll_conn *conn, struct proc_ctx *ctx, u } break; } +#endif /* CONFIG_BT_CTLR_DATA_LENGTH */ +#if defined(CONFIG_BT_CTLR_DF_CONN_CTE_RSP) + case PROC_CTE_REQ: { + /* add PHY update pause = false here */ + llcp_rr_set_paused_cmd(conn, PROC_NONE); + llcp_rr_complete(conn); + ctx->state = RP_COMMON_STATE_IDLE; + } +#endif /* CONFIG_BT_CTLR_DF_CONN_CTE_RSP */ default: /* Ignore other procedures */ break; @@ -1009,6 +1024,7 @@ static void rp_comm_st_wait_tx_ack(struct ll_conn *conn, struct proc_ctx *ctx, u } } +#if defined(CONFIG_BT_CTLR_DATA_LENGTH) static void rp_comm_st_wait_ntf(struct ll_conn *conn, struct proc_ctx *ctx, uint8_t evt, void *param) { @@ -1033,10 +1049,10 @@ static void rp_comm_execute_fsm(struct ll_conn *conn, struct proc_ctx *ctx, uint case RP_COMMON_STATE_WAIT_TX: rp_comm_st_wait_tx(conn, ctx, evt, param); break; -#if defined(CONFIG_BT_CTLR_DATA_LENGTH) case RP_COMMON_STATE_WAIT_TX_ACK: rp_comm_st_wait_tx_ack(conn, ctx, evt, param); break; +#if defined(CONFIG_BT_CTLR_DATA_LENGTH) case RP_COMMON_STATE_WAIT_NTF: rp_comm_st_wait_ntf(conn, ctx, evt, param); break; diff --git a/subsys/bluetooth/controller/ll_sw/ull_llcp_internal.h b/subsys/bluetooth/controller/ll_sw/ull_llcp_internal.h index 770e883c97e..cab97dcb05d 100644 --- a/subsys/bluetooth/controller/ll_sw/ull_llcp_internal.h +++ b/subsys/bluetooth/controller/ll_sw/ull_llcp_internal.h @@ -20,6 +20,8 @@ enum llcp_proc { PROC_CHAN_MAP_UPDATE, PROC_DATA_LENGTH_UPDATE, PROC_CTE_REQ, + /* A helper enum entry, to use in pause prcedure context */ + PROC_NONE = 0x0, }; #if ((CONFIG_BT_CTLR_LLCP_COMMON_TX_CTRL_BUF_NUM <\ (CONFIG_BT_CTLR_LLCP_TX_PER_CONN_TX_CTRL_BUF_NUM_MAX *\ @@ -446,6 +448,8 @@ void llcp_lr_abort(struct ll_conn *conn); */ void llcp_rr_set_incompat(struct ll_conn *conn, enum proc_incompat incompat); bool llcp_rr_get_collision(struct ll_conn *conn); +void llcp_rr_set_paused_cmd(struct ll_conn *conn, enum llcp_proc); +enum llcp_proc llcp_rr_get_paused_cmd(struct ll_conn *conn); struct proc_ctx *llcp_rr_peek(struct ll_conn *conn); void llcp_rr_pause(struct ll_conn *conn); void llcp_rr_resume(struct ll_conn *conn); diff --git a/subsys/bluetooth/controller/ll_sw/ull_llcp_phy.c b/subsys/bluetooth/controller/ll_sw/ull_llcp_phy.c index fa108ca05c6..bd006d03959 100644 --- a/subsys/bluetooth/controller/ll_sw/ull_llcp_phy.c +++ b/subsys/bluetooth/controller/ll_sw/ull_llcp_phy.c @@ -435,15 +435,18 @@ static void lp_pu_complete(struct ll_conn *conn, struct proc_ctx *ctx, uint8_t e #endif llcp_lr_complete(conn); ctx->state = LP_PU_STATE_IDLE; + llcp_rr_set_paused_cmd(conn, PROC_NONE); } } static void lp_pu_send_phy_req(struct ll_conn *conn, struct proc_ctx *ctx, uint8_t evt, void *param) { - if (ctx->pause || llcp_rr_get_collision(conn) || !llcp_tx_alloc_peek(conn, ctx)) { + if (ctx->pause || llcp_rr_get_collision(conn) || !llcp_tx_alloc_peek(conn, ctx) || + (llcp_rr_get_paused_cmd(conn) == PROC_PHY_UPDATE)) { ctx->state = LP_PU_STATE_WAIT_TX_PHY_REQ; } else { llcp_rr_set_incompat(conn, INCOMPAT_RESOLVABLE); + llcp_rr_set_paused_cmd(conn, PROC_CTE_REQ); lp_pu_tx(conn, ctx, PDU_DATA_LLCTRL_TYPE_PHY_REQ); llcp_tx_pause_data(conn); ctx->state = LP_PU_STATE_WAIT_TX_ACK_PHY_REQ; @@ -832,6 +835,7 @@ static void rp_pu_complete(struct ll_conn *conn, struct proc_ctx *ctx, uint8_t e pu_dle_ntf(conn, ctx); } #endif + llcp_rr_set_paused_cmd(conn, PROC_NONE); llcp_rr_complete(conn); ctx->state = RP_PU_STATE_IDLE; } @@ -855,9 +859,11 @@ static void rp_pu_send_phy_update_ind(struct ll_conn *conn, struct proc_ctx *ctx #if defined(CONFIG_BT_PERIPHERAL) static void rp_pu_send_phy_rsp(struct ll_conn *conn, struct proc_ctx *ctx, uint8_t evt, void *param) { - if (ctx->pause || !llcp_tx_alloc_peek(conn, ctx)) { + if (ctx->pause || !llcp_tx_alloc_peek(conn, ctx) || + (llcp_rr_get_paused_cmd(conn) == PROC_PHY_UPDATE)) { ctx->state = RP_PU_STATE_WAIT_TX_PHY_RSP; } else { + llcp_rr_set_paused_cmd(conn, PROC_CTE_REQ); rp_pu_tx(conn, ctx, PDU_DATA_LLCTRL_TYPE_PHY_RSP); ctx->rx_opcode = PDU_DATA_LLCTRL_TYPE_PHY_UPD_IND; ctx->state = RP_PU_STATE_WAIT_TX_ACK_PHY_RSP; diff --git a/subsys/bluetooth/controller/ll_sw/ull_llcp_remote.c b/subsys/bluetooth/controller/ll_sw/ull_llcp_remote.c index 8438dc11802..0fde9d678e3 100644 --- a/subsys/bluetooth/controller/ll_sw/ull_llcp_remote.c +++ b/subsys/bluetooth/controller/ll_sw/ull_llcp_remote.c @@ -136,6 +136,22 @@ void llcp_rr_set_incompat(struct ll_conn *conn, enum proc_incompat incompat) conn->llcp.remote.incompat = incompat; } +void llcp_rr_set_paused_cmd(struct ll_conn *conn, enum llcp_proc proc) +{ +#if defined(CONFIG_BT_CTLR_DF_CONN_CTE_RSP) || defined(CONFIG_BT_CTLR_DF_CONN_CTE_REQ) + conn->llcp.remote.paused_cmd = proc; +#endif /* CONFIG_BT_CTLR_DF_CONN_CTE_RSP || CONFIG_BT_CTLR_DF_CONN_CTE_REQ */ +} + +enum llcp_proc llcp_rr_get_paused_cmd(struct ll_conn *conn) +{ +#if defined(CONFIG_BT_CTLR_DF_CONN_CTE_RSP) || defined(CONFIG_BT_CTLR_DF_CONN_CTE_REQ) + return conn->llcp.remote.paused_cmd; +#else + return PROC_NONE; +#endif /* CONFIG_BT_CTLR_DF_CONN_CTE_RSP || CONFIG_BT_CTLR_DF_CONN_CTE_REQ */ +} + static enum proc_incompat rr_get_incompat(struct ll_conn *conn) { return conn->llcp.remote.incompat; @@ -266,6 +282,11 @@ void llcp_rr_tx_ack(struct ll_conn *conn, struct proc_ctx *ctx, struct node_tx * llcp_rp_pu_tx_ack(conn, ctx, tx); break; #endif /* CONFIG_BT_CTLR_PHY */ +#if defined(CONFIG_BT_CTLR_DF_CONN_CTE_RSP) + case PROC_CTE_REQ: + llcp_rp_comm_tx_ack(conn, ctx, tx); + break; +#endif /* CONFIG_BT_CTLR_DF_CONN_CTE_RSP */ default: /* Ignore tx_ack */ break;