From 01c9c6716ae9dd07f6f72de7a227879fa9aedd58 Mon Sep 17 00:00:00 2001 From: Vinayak Kariappa Chettimada Date: Thu, 18 Jul 2024 07:56:09 +0200 Subject: [PATCH] Bluetooth: Controller: Fix incorrect elapsed events value Fix incorrect elapsed events value when LLL event prepare_cb was invoked but was aborted before anchor point sync. This caused premature supervision timeouts under applications configured with CONFIG_BT_CTLR_EVENT_OVERHEAD_RESERVE_MAX=n and CONFIG_BT_CTLR_PERIPHERAL_RESERVE_MAX=n. Fixes commit 247037bd3e2a ("Bluetooth: Controller: Fix incorrect elapsed events value"). Signed-off-by: Vinayak Kariappa Chettimada --- .../bluetooth/controller/ll_sw/lll_sync_iso.h | 2 + .../controller/ll_sw/nordic/lll/lll_sync.c | 54 +++++----- .../ll_sw/nordic/lll/lll_sync_iso.c | 100 ++++++++---------- subsys/bluetooth/controller/ll_sw/ull_conn.c | 2 +- subsys/bluetooth/controller/ll_sw/ull_sync.c | 2 +- .../bluetooth/controller/ll_sw/ull_sync_iso.c | 8 +- 6 files changed, 75 insertions(+), 93 deletions(-) diff --git a/subsys/bluetooth/controller/ll_sw/lll_sync_iso.h b/subsys/bluetooth/controller/ll_sw/lll_sync_iso.h index 6e295100437..e350ad31ab6 100644 --- a/subsys/bluetooth/controller/ll_sw/lll_sync_iso.h +++ b/subsys/bluetooth/controller/ll_sw/lll_sync_iso.h @@ -30,8 +30,10 @@ struct lll_sync_iso { uint16_t iso_interval; + uint16_t lazy_prepare; uint16_t latency_prepare; uint16_t latency_event; + union { struct lll_sync_iso_data_chan data_chan; diff --git a/subsys/bluetooth/controller/ll_sw/nordic/lll/lll_sync.c b/subsys/bluetooth/controller/ll_sw/nordic/lll/lll_sync.c index c80591939b1..d549d914f59 100644 --- a/subsys/bluetooth/controller/ll_sw/nordic/lll/lll_sync.c +++ b/subsys/bluetooth/controller/ll_sw/nordic/lll/lll_sync.c @@ -46,7 +46,6 @@ #include "hal/debug.h" static int init_reset(void); -static void prepare(void *param); static int create_prepare_cb(struct lll_prepare_param *p); static int prepare_cb(struct lll_prepare_param *p); static int prepare_cb_common(struct lll_prepare_param *p, uint8_t chan_idx); @@ -100,7 +99,9 @@ void lll_sync_create_prepare(void *param) { int err; - prepare(param); + /* Request to start HF Clock */ + err = lll_hfclock_on(); + LL_ASSERT(err >= 0); /* Invoke common pipeline handling of prepare */ err = lll_prepare(is_abort_cb, abort_cb, create_prepare_cb, 0, param); @@ -111,35 +112,13 @@ void lll_sync_prepare(void *param) { int err; - prepare(param); - - /* Invoke common pipeline handling of prepare */ - err = lll_prepare(is_abort_cb, abort_cb, prepare_cb, 0, param); - LL_ASSERT(!err || err == -EINPROGRESS); -} - -static void prepare(void *param) -{ - struct lll_prepare_param *p; - struct lll_sync *lll; - int err; - /* Request to start HF Clock */ err = lll_hfclock_on(); LL_ASSERT(err >= 0); - p = param; - - lll = p->param; - - lll->lazy_prepare = p->lazy; - - /* Accumulate window widening */ - lll->window_widening_prepare_us += lll->window_widening_periodic_us * - (lll->lazy_prepare + 1U); - if (lll->window_widening_prepare_us > lll->window_widening_max_us) { - lll->window_widening_prepare_us = lll->window_widening_max_us; - } + /* Invoke common pipeline handling of prepare */ + err = lll_prepare(is_abort_cb, abort_cb, prepare_cb, 0, param); + LL_ASSERT(err == 0 || err == -EINPROGRESS); } void lll_sync_aux_prepare_cb(struct lll_sync *lll, @@ -274,6 +253,7 @@ static int create_prepare_cb(struct lll_prepare_param *p) lll = p->param; /* Calculate the current event latency */ + lll->lazy_prepare = p->lazy; lll->skip_event = lll->skip_prepare + lll->lazy_prepare; /* Calculate the current event counter value */ @@ -362,6 +342,7 @@ static int prepare_cb(struct lll_prepare_param *p) lll = p->param; /* Calculate the current event latency */ + lll->lazy_prepare = p->lazy; lll->skip_event = lll->skip_prepare + lll->lazy_prepare; /* Calculate the current event counter value */ @@ -442,6 +423,13 @@ static int prepare_cb_common(struct lll_prepare_param *p, uint8_t chan_idx) lll = p->param; + /* Accumulate window widening */ + lll->window_widening_prepare_us += lll->window_widening_periodic_us * + (lll->lazy_prepare + 1U); + if (lll->window_widening_prepare_us > lll->window_widening_max_us) { + lll->window_widening_prepare_us = lll->window_widening_max_us; + } + /* Current window widening */ lll->window_widening_event_us += lll->window_widening_prepare_us; lll->window_widening_prepare_us = 0; @@ -631,10 +619,20 @@ static void abort_cb(struct lll_prepare_param *prepare_param, void *param) err = lll_hfclock_off(); LL_ASSERT(err >= 0); - /* Accumulate the latency as event is aborted while being in pipeline */ + /* Get reference to LLL connection context */ lll = prepare_param->param; + + /* Accumulate the latency as event is aborted while being in pipeline */ + lll->lazy_prepare = prepare_param->lazy; lll->skip_prepare += (lll->lazy_prepare + 1U); + /* Accumulate window widening */ + lll->window_widening_prepare_us += lll->window_widening_periodic_us * + (prepare_param->lazy + 1); + if (lll->window_widening_prepare_us > lll->window_widening_max_us) { + lll->window_widening_prepare_us = lll->window_widening_max_us; + } + /* Extra done event, to check sync lost */ e = ull_event_done_extra_get(); LL_ASSERT(e); diff --git a/subsys/bluetooth/controller/ll_sw/nordic/lll/lll_sync_iso.c b/subsys/bluetooth/controller/ll_sw/nordic/lll/lll_sync_iso.c index b8ec3f58674..6b4c63fbeba 100644 --- a/subsys/bluetooth/controller/ll_sw/nordic/lll/lll_sync_iso.c +++ b/subsys/bluetooth/controller/ll_sw/nordic/lll/lll_sync_iso.c @@ -40,9 +40,6 @@ #include "hal/debug.h" static int init_reset(void); -static void prepare(void *param); -static void create_prepare_bh(void *param); -static void prepare_bh(void *param); static int create_prepare_cb(struct lll_prepare_param *p); static int prepare_cb(struct lll_prepare_param *p); static int prepare_cb_common(struct lll_prepare_param *p); @@ -101,14 +98,25 @@ int lll_sync_iso_reset(void) void lll_sync_iso_create_prepare(void *param) { - prepare(param); - create_prepare_bh(param); + int err; + + err = lll_hfclock_on(); + LL_ASSERT(err >= 0); + + err = lll_prepare(is_abort_cb, abort_cb, create_prepare_cb, 0U, + param); + LL_ASSERT(err == 0 || err == -EINPROGRESS); } void lll_sync_iso_prepare(void *param) { - prepare(param); - prepare_bh(param); + int err; + + err = lll_hfclock_on(); + LL_ASSERT(err >= 0); + + err = lll_prepare(is_abort_cb, abort_cb, prepare_cb, 0U, param); + LL_ASSERT(err == 0 || err == -EINPROGRESS); } void lll_sync_iso_flush(uint8_t handle, struct lll_sync_iso *lll) @@ -122,53 +130,6 @@ static int init_reset(void) return 0; } -static void prepare(void *param) -{ - struct lll_prepare_param *p; - struct lll_sync_iso *lll; - uint16_t elapsed; - int err; - - err = lll_hfclock_on(); - LL_ASSERT(err >= 0); - - p = param; - - /* Instants elapsed */ - elapsed = p->lazy + 1U; - - lll = p->param; - - /* Save the (latency + 1) for use in event */ - lll->latency_prepare += elapsed; - - /* Accumulate window widening */ - lll->window_widening_prepare_us += lll->window_widening_periodic_us * - elapsed; - if (lll->window_widening_prepare_us > lll->window_widening_max_us) { - lll->window_widening_prepare_us = lll->window_widening_max_us; - } -} - -static void create_prepare_bh(void *param) -{ - int err; - - /* Invoke common pipeline handling of prepare */ - err = lll_prepare(is_abort_cb, abort_cb, create_prepare_cb, 0U, - param); - LL_ASSERT(!err || err == -EINPROGRESS); -} - -static void prepare_bh(void *param) -{ - int err; - - /* Invoke common pipeline handling of prepare */ - err = lll_prepare(is_abort_cb, abort_cb, prepare_cb, 0U, param); - LL_ASSERT(!err || err == -EINPROGRESS); -} - static int create_prepare_cb(struct lll_prepare_param *p) { int err; @@ -225,18 +186,26 @@ static int prepare_cb_common(struct lll_prepare_param *p) lll = p->param; - /* Deduce the latency */ - lll->latency_event = lll->latency_prepare - 1U; + /* Calculate the current event latency */ + lll->lazy_prepare = p->lazy; + lll->latency_event = lll->latency_prepare + lll->lazy_prepare; /* Calculate the current event counter value */ event_counter = (lll->payload_count / lll->bn) + lll->latency_event; /* Update BIS packet counter to next value */ - lll->payload_count += (lll->latency_prepare * lll->bn); + lll->payload_count += (lll->latency_event + 1U) * lll->bn; /* Reset accumulated latencies */ lll->latency_prepare = 0U; + /* Accumulate window widening */ + lll->window_widening_prepare_us += lll->window_widening_periodic_us * + (lll->lazy_prepare + 1U); + if (lll->window_widening_prepare_us > lll->window_widening_max_us) { + lll->window_widening_prepare_us = lll->window_widening_max_us; + } + /* Current window widening */ lll->window_widening_event_us += lll->window_widening_prepare_us; lll->window_widening_prepare_us = 0U; @@ -476,6 +445,7 @@ static int is_abort_cb(void *next, void *curr, lll_prepare_cb_t *resume_cb) static void abort_cb(struct lll_prepare_param *prepare_param, void *param) { struct event_done_extra *e; + struct lll_sync_iso *lll; int err; /* NOTE: This is not a prepare being cancelled */ @@ -484,7 +454,7 @@ static void abort_cb(struct lll_prepare_param *prepare_param, void *param) radio_disable(); if (IS_ENABLED(CONFIG_BT_CTLR_BROADCAST_ISO_ENC)) { - const struct lll_sync_iso *lll = param; + lll = param; if (lll->enc) { radio_ccm_disable(); @@ -500,6 +470,20 @@ static void abort_cb(struct lll_prepare_param *prepare_param, void *param) err = lll_hfclock_off(); LL_ASSERT(err >= 0); + /* Get reference to LLL connection context */ + lll = prepare_param->param; + + /* Accumulate the latency as event is aborted while being in pipeline */ + lll->lazy_prepare = prepare_param->lazy; + lll->latency_prepare += (lll->lazy_prepare + 1U); + + /* Accumulate window widening */ + lll->window_widening_prepare_us += lll->window_widening_periodic_us * + (prepare_param->lazy + 1); + if (lll->window_widening_prepare_us > lll->window_widening_max_us) { + lll->window_widening_prepare_us = lll->window_widening_max_us; + } + /* Extra done event, to check sync lost */ e = ull_event_done_extra_get(); LL_ASSERT(e); diff --git a/subsys/bluetooth/controller/ll_sw/ull_conn.c b/subsys/bluetooth/controller/ll_sw/ull_conn.c index c53f46a1288..c6dd93efd8e 100644 --- a/subsys/bluetooth/controller/ll_sw/ull_conn.c +++ b/subsys/bluetooth/controller/ll_sw/ull_conn.c @@ -1129,7 +1129,7 @@ void ull_conn_done(struct node_rx_event_done *done) #endif /* CONFIG_BT_PERIPHERAL */ } - elapsed_event = latency_event + lll->lazy_prepare + 1U; + elapsed_event = lll->lazy_prepare + 1U; /* Reset supervision countdown */ if (done->extra.crc_valid && !done->extra.is_aborted) { diff --git a/subsys/bluetooth/controller/ll_sw/ull_sync.c b/subsys/bluetooth/controller/ll_sw/ull_sync.c index 16ccd8e0358..0ce9658e8d2 100644 --- a/subsys/bluetooth/controller/ll_sw/ull_sync.c +++ b/subsys/bluetooth/controller/ll_sw/ull_sync.c @@ -1342,7 +1342,7 @@ void ull_sync_done(struct node_rx_event_done *done) sync->sync_expire = 0U; } - elapsed_event = skip_event + lll->lazy_prepare + 1U; + elapsed_event = lll->lazy_prepare + 1U; /* Reset supervision countdown */ if (done->extra.crc_valid) { diff --git a/subsys/bluetooth/controller/ll_sw/ull_sync_iso.c b/subsys/bluetooth/controller/ll_sw/ull_sync_iso.c index 756abaa6141..b3737c8fc4a 100644 --- a/subsys/bluetooth/controller/ll_sw/ull_sync_iso.c +++ b/subsys/bluetooth/controller/ll_sw/ull_sync_iso.c @@ -185,6 +185,7 @@ uint8_t ll_big_sync_create(uint8_t big_handle, uint16_t sync_handle, /* Initialize sync LLL context */ lll = &sync_iso->lll; + lll->lazy_prepare = 0U; lll->latency_prepare = 0U; lll->latency_event = 0U; lll->window_widening_prepare_us = 0U; @@ -762,11 +763,6 @@ void ull_sync_iso_done(struct node_rx_event_done *done) /* Events elapsed used in timeout checks below */ latency_event = lll->latency_event; - if (lll->latency_prepare) { - elapsed_event = latency_event + lll->latency_prepare; - } else { - elapsed_event = latency_event + 1U; - } /* Check for establishmet failure */ if (done->extra.estab_failed) { @@ -800,6 +796,8 @@ void ull_sync_iso_done(struct node_rx_event_done *done) } } + elapsed_event = lll->lazy_prepare + 1U; + /* check timeout */ force = 0U; if (sync_iso->timeout_expire) {