Skip to content

Commit ddaef40

Browse files
cvinayakcvinayak
committed
Bluetooth: Controller: Fix LLL events pending check from ULL Low
ULL reference count is checked in ULL_LOW context to decide if LLL events are pending, but the reference count can be decremented by the ULL HIGH execution context which can prevent the set `disabled_cb` function not being called due to no pending event to produce the done events. Fixed by checking the reference count in the ULL HIGH execution context using a mayfly to schedule the check. Signed-off-by: Vinayak Kariappa Chettimada <[email protected]>
1 parent 6206ae2 commit ddaef40

File tree

3 files changed

+125
-91
lines changed

3 files changed

+125
-91
lines changed

‎subsys/bluetooth/controller/ll_sw/ull_adv.c‎

Lines changed: 71 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@ static void ticker_update_op_cb(uint32_t status, void *param);
7474
static void ticker_stop_cb(uint32_t ticks_at_expire, uint32_t remainder,
7575
uint16_t lazy, uint8_t force, void *param);
7676
static void ticker_stop_op_cb(uint32_t status, void *param);
77+
static void adv_disable(void *param);
7778
static void disabled_cb(void *param);
7879
static void conn_release(struct ll_adv_set *adv);
7980
#endif /* CONFIG_BT_PERIPHERAL */
@@ -83,8 +84,10 @@ static void adv_max_events_duration_set(struct ll_adv_set *adv,
8384
uint16_t duration,
8485
uint8_t max_ext_adv_evts);
8586
static void ticker_stop_aux_op_cb(uint32_t status, void *param);
87+
static void aux_disable(void *param);
8688
static void aux_disabled_cb(void *param);
8789
static void ticker_stop_ext_op_cb(uint32_t status, void *param);
90+
static void ext_disable(void *param);
8891
static void ext_disabled_cb(void *param);
8992
#endif /* CONFIG_BT_CTLR_ADV_EXT */
9093

@@ -2195,9 +2198,8 @@ static void ticker_stop_cb(uint32_t ticks_at_expire, uint32_t remainder,
21952198
static void ticker_stop_op_cb(uint32_t status, void *param)
21962199
{
21972200
static memq_link_t link;
2198-
static struct mayfly mfy = {0, 0, &link, NULL, NULL};
2199-
struct ll_adv_set *adv;
2200-
struct ull_hdr *hdr;
2201+
static struct mayfly mfy = {0, 0, &link, NULL, adv_disable};
2202+
uint32_t ret;
22012203

22022204
/* Ignore if race between thread and ULL */
22032205
if (status != TICKER_STATUS_SUCCESS) {
@@ -2213,33 +2215,42 @@ static void ticker_stop_op_cb(uint32_t status, void *param)
22132215
}
22142216
#endif /* CONFIG_BT_HCI_MESH_EXT */
22152217

2216-
/* NOTE: We are in ULL_LOW which can be pre-empted by ULL_HIGH.
2217-
* As we are in the callback after successful stop of the
2218-
* ticker, the ULL reference count will not be modified
2219-
* further hence it is safe to check and act on either the need
2220-
* to call lll_disable or not.
2221-
*/
2218+
/* Check if any pending LLL events that need to be aborted */
2219+
mfy.param = param;
2220+
ret = mayfly_enqueue(TICKER_USER_ID_ULL_LOW,
2221+
TICKER_USER_ID_ULL_HIGH, 0, &mfy);
2222+
LL_ASSERT(!ret);
2223+
}
2224+
2225+
static void adv_disable(void *param)
2226+
{
2227+
struct ll_adv_set *adv;
2228+
struct ull_hdr *hdr;
2229+
2230+
/* Check ref count to determine if any pending LLL events in pipeline */
22222231
adv = param;
22232232
hdr = &adv->ull;
2224-
mfy.param = &adv->lll;
22252233
if (ull_ref_get(hdr)) {
2234+
static memq_link_t link;
2235+
static struct mayfly mfy = {0, 0, &link, NULL, lll_disable};
22262236
uint32_t ret;
22272237

2238+
mfy.param = &adv->lll;
2239+
2240+
/* Setup disabled callback to be called when ref count
2241+
* returns to zero.
2242+
*/
22282243
LL_ASSERT(!hdr->disabled_cb);
22292244
hdr->disabled_param = mfy.param;
22302245
hdr->disabled_cb = disabled_cb;
22312246

2232-
mfy.fp = lll_disable;
2233-
ret = mayfly_enqueue(TICKER_USER_ID_ULL_LOW,
2247+
/* Trigger LLL disable */
2248+
ret = mayfly_enqueue(TICKER_USER_ID_ULL_HIGH,
22342249
TICKER_USER_ID_LLL, 0, &mfy);
22352250
LL_ASSERT(!ret);
22362251
} else {
2237-
uint32_t ret;
2238-
2239-
mfy.fp = disabled_cb;
2240-
ret = mayfly_enqueue(TICKER_USER_ID_ULL_LOW,
2241-
TICKER_USER_ID_ULL_HIGH, 0, &mfy);
2242-
LL_ASSERT(!ret);
2252+
/* No pending LLL events */
2253+
disabled_cb(&adv->lll);
22432254
}
22442255
}
22452256

@@ -2325,21 +2336,27 @@ static void adv_max_events_duration_set(struct ll_adv_set *adv,
23252336
}
23262337

23272338
static void ticker_stop_aux_op_cb(uint32_t status, void *param)
2339+
{
2340+
static memq_link_t link;
2341+
static struct mayfly mfy = {0, 0, &link, NULL, aux_disable};
2342+
uint32_t ret;
2343+
2344+
LL_ASSERT(status == TICKER_STATUS_SUCCESS);
2345+
2346+
/* Check if any pending LLL events that need to be aborted */
2347+
mfy.param = param;
2348+
ret = mayfly_enqueue(TICKER_USER_ID_ULL_LOW,
2349+
TICKER_USER_ID_ULL_HIGH, 0, &mfy);
2350+
LL_ASSERT(!ret);
2351+
}
2352+
2353+
static void aux_disable(void *param)
23282354
{
23292355
struct lll_adv_aux *lll_aux;
23302356
struct ll_adv_aux_set *aux;
23312357
struct ll_adv_set *adv;
23322358
struct ull_hdr *hdr;
2333-
uint32_t ret;
2334-
2335-
LL_ASSERT(status == TICKER_STATUS_SUCCESS);
23362359

2337-
/* NOTE: We are in ULL_LOW which can be pre-empted by ULL_HIGH.
2338-
* As we are in the callback after successful stop of the
2339-
* ticker, the ULL reference count will not be modified
2340-
* further hence it is safe to check and act on either the need
2341-
* to call lll_disable or not.
2342-
*/
23432360
adv = param;
23442361
lll_aux = adv->lll.aux;
23452362
aux = HDR_LLL2ULL(lll_aux);
@@ -2349,15 +2366,7 @@ static void ticker_stop_aux_op_cb(uint32_t status, void *param)
23492366
hdr->disabled_param = adv;
23502367
hdr->disabled_cb = aux_disabled_cb;
23512368
} else {
2352-
uint8_t handle;
2353-
2354-
handle = ull_adv_handle_get(adv);
2355-
ret = ticker_stop(TICKER_INSTANCE_ID_CTLR,
2356-
TICKER_USER_ID_ULL_LOW,
2357-
(TICKER_ID_ADV_BASE + handle),
2358-
ticker_op_ext_stop_cb, adv);
2359-
LL_ASSERT((ret == TICKER_STATUS_SUCCESS) ||
2360-
(ret == TICKER_STATUS_BUSY));
2369+
aux_disabled_cb(param);
23612370
}
23622371
}
23632372

@@ -2378,9 +2387,8 @@ static void aux_disabled_cb(void *param)
23782387
static void ticker_stop_ext_op_cb(uint32_t status, void *param)
23792388
{
23802389
static memq_link_t link;
2381-
static struct mayfly mfy = {0, 0, &link, NULL, NULL};
2382-
struct ll_adv_set *adv;
2383-
struct ull_hdr *hdr;
2390+
static struct mayfly mfy = {0, 0, &link, NULL, ext_disable};
2391+
uint32_t ret;
23842392

23852393
/* Ignore if race between thread and ULL */
23862394
if (status != TICKER_STATUS_SUCCESS) {
@@ -2389,33 +2397,42 @@ static void ticker_stop_ext_op_cb(uint32_t status, void *param)
23892397
return;
23902398
}
23912399

2392-
/* NOTE: We are in ULL_LOW which can be pre-empted by ULL_HIGH.
2393-
* As we are in the callback after successful stop of the
2394-
* ticker, the ULL reference count will not be modified
2395-
* further hence it is safe to check and act on either the need
2396-
* to call lll_disable or not.
2397-
*/
2400+
/* Check if any pending LLL events that need to be aborted */
2401+
mfy.param = param;
2402+
ret = mayfly_enqueue(TICKER_USER_ID_ULL_LOW,
2403+
TICKER_USER_ID_ULL_HIGH, 0, &mfy);
2404+
LL_ASSERT(!ret);
2405+
}
2406+
2407+
static void ext_disable(void *param)
2408+
{
2409+
struct ll_adv_set *adv;
2410+
struct ull_hdr *hdr;
2411+
2412+
/* Check ref count to determine if any pending LLL events in pipeline */
23982413
adv = param;
23992414
hdr = &adv->ull;
2400-
mfy.param = &adv->lll;
24012415
if (ull_ref_get(hdr)) {
2416+
static memq_link_t link;
2417+
static struct mayfly mfy = {0, 0, &link, NULL, lll_disable};
24022418
uint32_t ret;
24032419

2420+
mfy.param = &adv->lll;
2421+
2422+
/* Setup disabled callback to be called when ref count
2423+
* returns to zero.
2424+
*/
24042425
LL_ASSERT(!hdr->disabled_cb);
24052426
hdr->disabled_param = mfy.param;
24062427
hdr->disabled_cb = ext_disabled_cb;
24072428

2408-
mfy.fp = lll_disable;
2409-
ret = mayfly_enqueue(TICKER_USER_ID_ULL_LOW,
2429+
/* Trigger LLL disable */
2430+
ret = mayfly_enqueue(TICKER_USER_ID_ULL_HIGH,
24102431
TICKER_USER_ID_LLL, 0, &mfy);
24112432
LL_ASSERT(!ret);
24122433
} else {
2413-
uint32_t ret;
2414-
2415-
mfy.fp = ext_disabled_cb;
2416-
ret = mayfly_enqueue(TICKER_USER_ID_ULL_LOW,
2417-
TICKER_USER_ID_ULL_HIGH, 0, &mfy);
2418-
LL_ASSERT(!ret);
2434+
/* No pending LLL events */
2435+
ext_disabled_cb(&adv->lll);
24192436
}
24202437
}
24212438

‎subsys/bluetooth/controller/ll_sw/ull_conn.c‎

Lines changed: 27 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@ static void conn_cleanup(struct ll_conn *conn, uint8_t reason);
8484
static void conn_cleanup_finalize(struct ll_conn *conn);
8585
static void tx_ull_flush(struct ll_conn *conn);
8686
static void ticker_stop_op_cb(uint32_t status, void *param);
87+
static void conn_disable(void *param);
8788
static void disabled_cb(void *param);
8889
static void tx_lll_flush(void *param);
8990

@@ -2039,39 +2040,47 @@ static void tx_ull_flush(struct ll_conn *conn)
20392040
static void ticker_stop_op_cb(uint32_t status, void *param)
20402041
{
20412042
static memq_link_t link;
2042-
static struct mayfly mfy = {0, 0, &link, NULL, NULL};
2043-
struct ll_conn *conn;
2044-
struct ull_hdr *hdr;
2043+
static struct mayfly mfy = {0, 0, &link, NULL, conn_disable};
2044+
uint32_t ret;
20452045

20462046
LL_ASSERT(status == TICKER_STATUS_SUCCESS);
20472047

2048-
/* NOTE: We are in ULL_LOW which can be pre-empted by ULL_HIGH.
2049-
* As we are in the callback after successful stop of the
2050-
* ticker, the ULL reference count will not be modified
2051-
* further hence it is safe to check and act on either the need
2052-
* to call lll_disable or not.
2053-
*/
2048+
/* Check if any pending LLL events that need to be aborted */
2049+
mfy.param = param;
2050+
ret = mayfly_enqueue(TICKER_USER_ID_ULL_LOW,
2051+
TICKER_USER_ID_ULL_HIGH, 0, &mfy);
2052+
LL_ASSERT(!ret);
2053+
}
2054+
2055+
static void conn_disable(void *param)
2056+
{
2057+
struct ll_conn *conn;
2058+
struct ull_hdr *hdr;
2059+
2060+
/* Check ref count to determine if any pending LLL events in pipeline */
20542061
conn = param;
20552062
hdr = &conn->ull;
2056-
mfy.param = &conn->lll;
20572063
if (ull_ref_get(hdr)) {
2064+
static memq_link_t link;
2065+
static struct mayfly mfy = {0, 0, &link, NULL, lll_disable};
20582066
uint32_t ret;
20592067

2068+
mfy.param = &conn->lll;
2069+
2070+
/* Setup disabled callback to be called when ref count
2071+
* returns to zero.
2072+
*/
20602073
LL_ASSERT(!hdr->disabled_cb);
20612074
hdr->disabled_param = mfy.param;
20622075
hdr->disabled_cb = disabled_cb;
20632076

2064-
mfy.fp = lll_disable;
2065-
ret = mayfly_enqueue(TICKER_USER_ID_ULL_LOW,
2077+
/* Trigger LLL disable */
2078+
ret = mayfly_enqueue(TICKER_USER_ID_ULL_HIGH,
20662079
TICKER_USER_ID_LLL, 0, &mfy);
20672080
LL_ASSERT(!ret);
20682081
} else {
2069-
uint32_t ret;
2070-
2071-
mfy.fp = disabled_cb;
2072-
ret = mayfly_enqueue(TICKER_USER_ID_ULL_LOW,
2073-
TICKER_USER_ID_ULL_HIGH, 0, &mfy);
2074-
LL_ASSERT(!ret);
2082+
/* No pending LLL events */
2083+
disabled_cb(&conn->lll);
20752084
}
20762085
}
20772086

‎subsys/bluetooth/controller/ll_sw/ull_scan.c‎

Lines changed: 27 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ static uint8_t duration_period_setup(struct ll_scan_set *scan,
6464
static uint8_t duration_period_update(struct ll_scan_set *scan,
6565
uint8_t is_update);
6666
static void ticker_stop_ext_op_cb(uint32_t status, void *param);
67+
static void ext_disable(void *param);
6768
static void ext_disabled_cb(void *param);
6869
#endif /* CONFIG_BT_CTLR_ADV_EXT */
6970

@@ -840,9 +841,8 @@ static uint8_t duration_period_update(struct ll_scan_set *scan,
840841
static void ticker_stop_ext_op_cb(uint32_t status, void *param)
841842
{
842843
static memq_link_t link;
843-
static struct mayfly mfy = {0, 0, &link, NULL, NULL};
844-
struct ll_scan_set *scan;
845-
struct ull_hdr *hdr;
844+
static struct mayfly mfy = {0, 0, &link, NULL, ext_disable};
845+
uint32_t ret;
846846

847847
/* Ignore if race between thread and ULL */
848848
if (status != TICKER_STATUS_SUCCESS) {
@@ -851,34 +851,42 @@ static void ticker_stop_ext_op_cb(uint32_t status, void *param)
851851
return;
852852
}
853853

854-
/* NOTE: We are in ULL_LOW which can be pre-empted by ULL_HIGH.
855-
* As we are in the callback after successful stop of the
856-
* ticker, the ULL reference count will not be modified
857-
* further hence it is safe to check and act on either the need
858-
* to call lll_disable or not.
859-
*/
854+
/* Check if any pending LLL events that need to be aborted */
855+
mfy.param = param;
856+
ret = mayfly_enqueue(TICKER_USER_ID_ULL_LOW,
857+
TICKER_USER_ID_ULL_HIGH, 0, &mfy);
858+
LL_ASSERT(!ret);
859+
}
860+
861+
static void ext_disable(void *param)
862+
{
863+
struct ll_scan_set *scan;
864+
struct ull_hdr *hdr;
865+
866+
/* Check ref count to determine if any pending LLL events in pipeline */
860867
scan = param;
861868
hdr = &scan->ull;
862-
mfy.param = &scan->lll;
863869
if (ull_ref_get(hdr)) {
870+
static memq_link_t link;
871+
static struct mayfly mfy = {0, 0, &link, NULL, lll_disable};
864872
uint32_t ret;
865873

866-
LL_ASSERT(!hdr->disabled_cb);
874+
mfy.param = &scan->lll;
867875

876+
/* Setup disabled callback to be called when ref count
877+
* returns to zero.
878+
*/
879+
LL_ASSERT(!hdr->disabled_cb);
868880
hdr->disabled_param = mfy.param;
869881
hdr->disabled_cb = ext_disabled_cb;
870882

871-
mfy.fp = lll_disable;
872-
ret = mayfly_enqueue(TICKER_USER_ID_ULL_LOW,
883+
/* Trigger LLL disable */
884+
ret = mayfly_enqueue(TICKER_USER_ID_ULL_HIGH,
873885
TICKER_USER_ID_LLL, 0, &mfy);
874886
LL_ASSERT(!ret);
875887
} else {
876-
uint32_t ret;
877-
878-
mfy.fp = ext_disabled_cb;
879-
ret = mayfly_enqueue(TICKER_USER_ID_ULL_LOW,
880-
TICKER_USER_ID_ULL_HIGH, 0, &mfy);
881-
LL_ASSERT(!ret);
888+
/* No pending LLL events */
889+
ext_disabled_cb(&scan->lll);
882890
}
883891
}
884892

0 commit comments

Comments
 (0)