From 4c674aa1f2b88a2831d1e2cbf0e7d1578be038c4 Mon Sep 17 00:00:00 2001 From: Vinayak Kariappa Chettimada Date: Tue, 12 Apr 2022 10:57:08 +0530 Subject: [PATCH] Bluetooth: Controller: Fix instant based procedure complete event Fix instant based procedure complete event generation to be held until after the on-air instant has elapsed. Signed-off-by: Vinayak Kariappa Chettimada --- .../bluetooth/controller/Kconfig.ll_sw_split | 1 + subsys/bluetooth/controller/ll_sw/lll_conn.h | 8 ++ .../controller/ll_sw/nordic/lll/lll_conn.c | 7 + subsys/bluetooth/controller/ll_sw/ull_adv.c | 6 + .../bluetooth/controller/ll_sw/ull_central.c | 6 + subsys/bluetooth/controller/ll_sw/ull_conn.c | 127 ++++++++++++++++-- .../controller/ll_sw/ull_conn_types.h | 4 + 7 files changed, 150 insertions(+), 9 deletions(-) diff --git a/subsys/bluetooth/controller/Kconfig.ll_sw_split b/subsys/bluetooth/controller/Kconfig.ll_sw_split index c84f98cf118..b50144ce57f 100644 --- a/subsys/bluetooth/controller/Kconfig.ll_sw_split +++ b/subsys/bluetooth/controller/Kconfig.ll_sw_split @@ -563,6 +563,7 @@ config BT_CTLR_LLID_DATA_START_EMPTY config BT_CTLR_RX_ENQUEUE_HOLD bool "Procedure Complete after on-air instant" + depends on BT_LL_SW_LLCP_LEGACY default y if BT_HCI_RAW help Hold enqueue of Procedure Complete events with instant until after the diff --git a/subsys/bluetooth/controller/ll_sw/lll_conn.h b/subsys/bluetooth/controller/ll_sw/lll_conn.h index 5c6874c6604..74b4735b248 100644 --- a/subsys/bluetooth/controller/ll_sw/lll_conn.h +++ b/subsys/bluetooth/controller/ll_sw/lll_conn.h @@ -140,6 +140,14 @@ struct lll_conn { #endif /* CONFIG_BT_CTLR_CONN_RSSI_EVENT */ #endif /* CONFIG_BT_CTLR_CONN_RSSI */ +#if defined(CONFIG_BT_CTLR_RX_ENQUEUE_HOLD) +#define RX_HOLD_MASK 3U +#define RX_HOLD_REQ 1U +#define RX_HOLD_ACK 2U + uint8_t rx_hold_req; + uint8_t rx_hold_ack; +#endif /* CONFIG_BT_CTLR_RX_ENQUEUE_HOLD */ + #if defined(CONFIG_BT_CTLR_CONN_META) struct lll_conn_meta conn_meta; #endif /* CONFIG_BT_CTLR_CONN_META */ diff --git a/subsys/bluetooth/controller/ll_sw/nordic/lll/lll_conn.c b/subsys/bluetooth/controller/ll_sw/nordic/lll/lll_conn.c index c0db5dceeda..02eaa450067 100644 --- a/subsys/bluetooth/controller/ll_sw/nordic/lll/lll_conn.c +++ b/subsys/bluetooth/controller/ll_sw/nordic/lll/lll_conn.c @@ -395,6 +395,13 @@ lll_conn_isr_rx_exit: is_ull_rx = 0U; +#if defined(CONFIG_BT_CTLR_RX_ENQUEUE_HOLD) + if (((lll->rx_hold_req - lll->rx_hold_ack) & RX_HOLD_MASK) == + RX_HOLD_REQ) { + lll->rx_hold_ack--; + } +#endif /* CONFIG_BT_CTLR_RX_ENQUEUE_HOLD */ + if (tx_release) { LL_ASSERT(lll->handle != 0xFFFF); diff --git a/subsys/bluetooth/controller/ll_sw/ull_adv.c b/subsys/bluetooth/controller/ll_sw/ull_adv.c index e0cf99cdc46..042c9b8e17f 100644 --- a/subsys/bluetooth/controller/ll_sw/ull_adv.c +++ b/subsys/bluetooth/controller/ll_sw/ull_adv.c @@ -1092,6 +1092,12 @@ uint8_t ll_adv_enable(uint8_t enable) */ conn->llcp_terminate.node_rx.hdr.link = link; +#if defined(CONFIG_BT_CTLR_RX_ENQUEUE_HOLD) + conn->llcp_rx_hold = NULL; + conn_lll->rx_hold_req = 0U; + conn_lll->rx_hold_ack = 0U; +#endif /* CONFIG_BT_CTLR_RX_ENQUEUE_HOLD */ + #if defined(CONFIG_BT_CTLR_LE_ENC) conn_lll->enc_rx = conn_lll->enc_tx = 0U; conn->llcp_enc.req = conn->llcp_enc.ack = 0U; diff --git a/subsys/bluetooth/controller/ll_sw/ull_central.c b/subsys/bluetooth/controller/ll_sw/ull_central.c index 28b8960a2bf..e2dc5d1bfe1 100644 --- a/subsys/bluetooth/controller/ll_sw/ull_central.c +++ b/subsys/bluetooth/controller/ll_sw/ull_central.c @@ -325,6 +325,12 @@ uint8_t ll_create_connection(uint16_t scan_interval, uint16_t scan_window, */ conn->llcp_terminate.node_rx.hdr.link = link; +#if defined(CONFIG_BT_CTLR_RX_ENQUEUE_HOLD) + conn->llcp_rx_hold = NULL; + conn_lll->rx_hold_req = 0U; + conn_lll->rx_hold_ack = 0U; +#endif /* CONFIG_BT_CTLR_RX_ENQUEUE_HOLD */ + #if defined(CONFIG_BT_CTLR_LE_ENC) conn_lll->enc_rx = conn_lll->enc_tx = 0U; conn->llcp_enc.req = conn->llcp_enc.ack = 0U; diff --git a/subsys/bluetooth/controller/ll_sw/ull_conn.c b/subsys/bluetooth/controller/ll_sw/ull_conn.c index 0fec916a8bc..475a2139f7a 100644 --- a/subsys/bluetooth/controller/ll_sw/ull_conn.c +++ b/subsys/bluetooth/controller/ll_sw/ull_conn.c @@ -73,6 +73,10 @@ #include "hal/debug.h" static int init_reset(void); +#if defined(CONFIG_BT_CTLR_RX_ENQUEUE_HOLD) +static bool rx_hold_is_done(struct ll_conn *conn); +static void rx_hold_flush(struct ll_conn *conn); +#endif /* CONFIG_BT_CTLR_RX_ENQUEUE_HOLD */ #if !defined(CONFIG_BT_CTLR_LOW_LAT_ULL) static void tx_demux_sched(struct ll_conn *conn); #endif /* CONFIG_BT_CTLR_LOW_LAT_ULL */ @@ -1010,6 +1014,12 @@ int ull_conn_rx(memq_link_t *link, struct node_rx_pdu **rx) return 0; } +#if defined(CONFIG_BT_CTLR_RX_ENQUEUE_HOLD) + if (conn->llcp_rx_hold && rx_hold_is_done(conn)) { + rx_hold_flush(conn); + } +#endif /* CONFIG_BT_CTLR_RX_ENQUEUE_HOLD */ + pdu_rx = (void *)(*rx)->pdu; switch (pdu_rx->ll_id) { @@ -1379,6 +1389,16 @@ void ull_conn_done(struct node_rx_event_done *done) return; } +#if defined(CONFIG_BT_CTLR_RX_ENQUEUE_HOLD) + if (conn->llcp_rx_hold && rx_hold_is_done(conn)) { + rx_hold_flush(conn); + +#if !defined(CONFIG_BT_CTLR_LOW_LAT_ULL) + ll_rx_sched(); +#endif /* !CONFIG_BT_CTLR_LOW_LAT_ULL */ + } +#endif /* CONFIG_BT_CTLR_RX_ENQUEUE_HOLD */ + #if defined(CONFIG_BT_CTLR_LE_ENC) /* Check authenticated payload expiry or MIC failure */ switch (done->extra.mic_state) { @@ -1936,6 +1956,12 @@ void ull_conn_tx_ack(uint16_t handle, memq_link_t *link, struct node_tx *tx) if (handle != LLL_HANDLE_INVALID) { struct ll_conn *conn = ll_conn_get(handle); +#if defined(CONFIG_BT_CTLR_RX_ENQUEUE_HOLD) + if (conn->llcp_rx_hold && rx_hold_is_done(conn)) { + rx_hold_flush(conn); + } +#endif /* CONFIG_BT_CTLR_RX_ENQUEUE_HOLD */ + #if defined(CONFIG_BT_LL_SW_LLCP_LEGACY) ctrl_tx_ack(conn, &tx, pdu_tx); #else /* CONFIG_BT_LL_SW_LLCP_LEGACY */ @@ -1964,6 +1990,14 @@ void ull_conn_tx_ack(uint16_t handle, memq_link_t *link, struct node_tx *tx) pdu_tx->ll_id = PDU_DATA_LLID_RESV; } else { LL_ASSERT(handle != LLL_HANDLE_INVALID); + +#if defined(CONFIG_BT_CTLR_RX_ENQUEUE_HOLD) + struct ll_conn *conn = ll_conn_get(handle); + + if (conn->llcp_rx_hold && rx_hold_is_done(conn)) { + rx_hold_flush(conn); + } +#endif /* CONFIG_BT_CTLR_RX_ENQUEUE_HOLD */ } ll_tx_ack_put(handle, tx); @@ -2154,6 +2188,72 @@ static int init_reset(void) return 0; } +#if defined(CONFIG_BT_LL_SW_LLCP_LEGACY) +static void rx_hold_put(struct ll_conn *conn, memq_link_t *link, + struct node_rx_pdu *rx) +{ +#if defined(CONFIG_BT_CTLR_RX_ENQUEUE_HOLD) + struct node_rx_pdu *rx_last; + struct lll_conn *lll; + + link->mem = NULL; + rx->hdr.link = link; + + rx_last = conn->llcp_rx_hold; + while (rx_last && rx_last->hdr.link && rx_last->hdr.link->mem) { + rx_last = rx_last->hdr.link->mem; + } + + if (rx_last) { + rx_last->hdr.link->mem = rx; + } else { + conn->llcp_rx_hold = rx; + } + + lll = &conn->lll; + if (lll->rx_hold_req == lll->rx_hold_ack) { + lll->rx_hold_req++; + } + +#else /* !CONFIG_BT_CTLR_RX_ENQUEUE_HOLD */ + ARG_UNUSED(conn); + + ll_rx_put(link, rx); +#endif /* !CONFIG_BT_CTLR_RX_ENQUEUE_HOLD */ +} + +#if defined(CONFIG_BT_CTLR_RX_ENQUEUE_HOLD) +static bool rx_hold_is_done(struct ll_conn *conn) +{ + return ((conn->lll.rx_hold_req - + conn->lll.rx_hold_ack) & RX_HOLD_MASK) == RX_HOLD_ACK; +} + +static void rx_hold_flush(struct ll_conn *conn) +{ + struct node_rx_pdu *rx; + struct lll_conn *lll; + + rx = conn->llcp_rx_hold; + do { + struct node_rx_hdr *hdr; + + /* traverse to next rx node */ + hdr = &rx->hdr; + rx = hdr->link->mem; + + /* enqueue rx node towards Thread */ + ll_rx_put(hdr->link, hdr); + } while (rx); + + conn->llcp_rx_hold = NULL; + lll = &conn->lll; + lll->rx_hold_req = 0U; + lll->rx_hold_ack = 0U; +} +#endif /* CONFIG_BT_CTLR_RX_ENQUEUE_HOLD */ +#endif /* CONFIG_BT_LL_SW_LLCP_LEGACY */ + #if !defined(CONFIG_BT_CTLR_LOW_LAT_ULL) static void tx_demux_sched(struct ll_conn *conn) { @@ -3121,14 +3221,21 @@ static inline int event_conn_upd_prep(struct ll_conn *conn, uint16_t lazy, cu->interval = conn->llcp_cu.interval; cu->latency = conn->llcp_cu.latency; cu->timeout = conn->llcp_cu.timeout; + + /* hold node rx until the instant's anchor point sync */ + rx_hold_put(conn, rx->hdr.link, rx); + + if (!IS_ENABLED(CONFIG_BT_CTLR_RX_ENQUEUE_HOLD)) { + ll_rx_sched(); + } } else { /* Mark for buffer for release */ rx->hdr.type = NODE_RX_TYPE_RELEASE; - } - /* enqueue rx node towards Thread */ - ll_rx_put(rx->hdr.link, rx); - ll_rx_sched(); + /* enqueue rx node towards Thread */ + ll_rx_put(rx->hdr.link, rx); + ll_rx_sched(); + } #if defined(CONFIG_BT_CTLR_XTAL_ADVANCED) /* restore to normal prepare */ @@ -4672,8 +4779,8 @@ static inline void event_phy_upd_ind_prep(struct ll_conn *conn, upd->tx = lll->phy_tx; upd->rx = lll->phy_rx; - /* enqueue rx node towards Thread */ - ll_rx_put(rx->hdr.link, rx); + /* hold node rx until the instant's anchor point sync */ + rx_hold_put(conn, rx->hdr.link, rx); #if defined(CONFIG_BT_CTLR_DATA_LENGTH) /* get a rx node for ULL->LL */ @@ -4716,11 +4823,13 @@ static inline void event_phy_upd_ind_prep(struct ll_conn *conn, lr->max_rx_time = sys_cpu_to_le16(lll->max_rx_time); lr->max_tx_time = sys_cpu_to_le16(lll->max_tx_time); - /* enqueue rx node towards Thread */ - ll_rx_put(rx->hdr.link, rx); + /* hold node rx until the instant's anchor point sync */ + rx_hold_put(conn, rx->hdr.link, rx); #endif /* CONFIG_BT_CTLR_DATA_LENGTH */ - ll_rx_sched(); + if (!IS_ENABLED(CONFIG_BT_CTLR_RX_ENQUEUE_HOLD)) { + ll_rx_sched(); + } } } #endif /* CONFIG_BT_CTLR_PHY */ diff --git a/subsys/bluetooth/controller/ll_sw/ull_conn_types.h b/subsys/bluetooth/controller/ll_sw/ull_conn_types.h index 85cbafc34de..9bd5f7b18b2 100644 --- a/subsys/bluetooth/controller/ll_sw/ull_conn_types.h +++ b/subsys/bluetooth/controller/ll_sw/ull_conn_types.h @@ -153,6 +153,10 @@ struct ll_conn { struct node_rx_pdu *llcp_rx; +#if defined(CONFIG_BT_CTLR_RX_ENQUEUE_HOLD) + struct node_rx_pdu *llcp_rx_hold; +#endif /* CONFIG_BT_CTLR_RX_ENQUEUE_HOLD */ + struct { uint8_t req; uint8_t ack;