From 2402c52e1dc60ec7a06e15a29051dee8e5b0afd5 Mon Sep 17 00:00:00 2001 From: Vinayak Kariappa Chettimada Date: Mon, 30 Jan 2023 08:16:26 +0530 Subject: [PATCH 1/3] Bluetooth: Controller: Fix addr type in PA sync established event The address type in the periodic adveritising sync established event is incorrectly set to 0x01 (BT_ADDR_LE_RANDOM) when the address is a resolved one, where it should have been BT_ADDR_LE_RANDOM_ID. The address type has been fixed by offsetting by 2 to get BT_ADDR_LE_PUBLIC_ID or BT_ADDR_LE_RANDOM_ID when the address has been resolved. Signed-off-by: Vinayak Kariappa Chettimada (cherry picked from commit 278b07c9710d1e5af18fcdc4560936187e0e83af) Signed-off-by: Vinayak Kariappa Chettimada --- subsys/bluetooth/controller/ll_sw/ull_scan_types.h | 2 +- subsys/bluetooth/controller/ll_sw/ull_sync.c | 8 +++++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/subsys/bluetooth/controller/ll_sw/ull_scan_types.h b/subsys/bluetooth/controller/ll_sw/ull_scan_types.h index d10ef58b0c0..a4247ecf304 100644 --- a/subsys/bluetooth/controller/ll_sw/ull_scan_types.h +++ b/subsys/bluetooth/controller/ll_sw/ull_scan_types.h @@ -22,7 +22,7 @@ struct ll_scan_set { struct { uint8_t sid; - uint8_t adv_addr_type:1; + uint8_t adv_addr_type:2; uint8_t filter_policy:1; uint8_t cancelled:1; uint8_t state:2; diff --git a/subsys/bluetooth/controller/ll_sw/ull_sync.c b/subsys/bluetooth/controller/ll_sw/ull_sync.c index 8e6108fec30..2185b04a1a7 100644 --- a/subsys/bluetooth/controller/ll_sw/ull_sync.c +++ b/subsys/bluetooth/controller/ll_sw/ull_sync.c @@ -618,6 +618,9 @@ void ull_sync_setup_addr_check(struct ll_scan_set *scan, uint8_t addr_type, */ scan->periodic.adv_addr_type = addr_type; + /* Mark it as identity address from RPA (0x02, 0x03) */ + scan->periodic.adv_addr_type += 2U; + /* Address matched */ scan->periodic.state = LL_SYNC_STATE_ADDR_MATCH; } @@ -634,6 +637,9 @@ void ull_sync_setup_addr_check(struct ll_scan_set *scan, uint8_t addr_type, ll_rl_id_addr_get(rl_idx, &addr_type, addr); if ((addr_type == scan->periodic.adv_addr_type) && !memcmp(addr, scan->periodic.adv_addr, BDADDR_SIZE)) { + /* Mark it as identity address from RPA (0x02, 0x03) */ + scan->periodic.adv_addr_type += 2U; + /* Identity address matched */ scan->periodic.state = LL_SYNC_STATE_ADDR_MATCH; } @@ -699,7 +705,7 @@ void ull_sync_setup(struct ll_scan_set *scan, struct ll_scan_aux_set *aux, /* Remember the peer address. * NOTE: Peer identity address is copied here when privacy is enable. */ - sync->peer_id_addr_type = scan->periodic.adv_addr_type; + sync->peer_id_addr_type = scan->periodic.adv_addr_type & 0x01; (void)memcpy(sync->peer_id_addr, scan->periodic.adv_addr, sizeof(sync->peer_id_addr)); #endif /* CONFIG_BT_CTLR_CHECK_SAME_PEER_SYNC || From 0b9cd4ce58f1d007836d44500d8ce7a402b52179 Mon Sep 17 00:00:00 2001 From: Vinayak Kariappa Chettimada Date: Fri, 1 Mar 2024 04:47:07 +0100 Subject: [PATCH 2/3] Bluetooth: Controller: Fix assertion establishing periodic sync When the AUX_ADV_IND and AUX_SYNC_IND are close to each other, the duration between them is not sufficient to schedule a new instance of ticker to establish synchronization. This processing time introduces latencies detected by the prepare callback. When the sync offset is low, schedule the start of the reception to next periodic interval. Signed-off-by: Vinayak Kariappa Chettimada (cherry picked from commit bd5e906f681a63b1ab0d3beae54f98e4e6ee9a59) Signed-off-by: Vinayak Kariappa Chettimada --- subsys/bluetooth/controller/ll_sw/ull_sync.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/subsys/bluetooth/controller/ll_sw/ull_sync.c b/subsys/bluetooth/controller/ll_sw/ull_sync.c index 2185b04a1a7..9eb28ed8845 100644 --- a/subsys/bluetooth/controller/ll_sw/ull_sync.c +++ b/subsys/bluetooth/controller/ll_sw/ull_sync.c @@ -672,6 +672,7 @@ void ull_sync_setup(struct ll_scan_set *scan, struct ll_scan_aux_set *aux, struct lll_sync *lll; uint16_t sync_handle; uint32_t interval_us; + uint32_t overhead_us; struct pdu_adv *pdu; uint16_t interval; uint8_t chm_last; @@ -799,6 +800,22 @@ void ull_sync_setup(struct ll_scan_set *scan, struct ll_scan_aux_set *aux, sync_offset_us -= EVENT_JITTER_US; sync_offset_us -= ready_delay_us; + /* Minimum prepare tick offset + minimum preempt tick offset are the + * overheads before ULL scheduling can setup radio for reception + */ + overhead_us = HAL_TICKER_TICKS_TO_US(HAL_TICKER_CNTR_CMP_OFFSET_MIN << 1); + + /* CPU execution overhead to setup the radio for reception */ + overhead_us += EVENT_OVERHEAD_END_US + EVENT_OVERHEAD_START_US; + + /* If not sufficient CPU processing time, skip to receiving next + * event. + */ + if ((sync_offset_us - ftr->radio_end_us) < overhead_us) { + sync_offset_us += interval_us; + lll->event_counter++; + } + interval_us -= lll->window_widening_periodic_us; /* TODO: active_to_start feature port */ From a072fa2d11d44c7cc4ea320e17703a5ebce4e1e6 Mon Sep 17 00:00:00 2001 From: Vinayak Kariappa Chettimada Date: Thu, 30 Jan 2025 09:44:55 +0100 Subject: [PATCH 3/3] Bluetooth: Controller: Fix device address in Periodic Adv Sync Fix incorrect device address reported in the LE Periodic Advertising Sync Established event when using Periodic Advertiser List. During Extended Scanning there can be an ADV_EXT_IND PDU received between currently being received ADV_EXT_IND PDU and AUX_ADV_IND PDU; if the one received between has an address match then incorrectly the Periodic Synchronization was established to the device whos AUX_ADV_IND PDU is being received. Fix by storing the auxiliary context that has the address match and compare with it when matching the SID in SyncInfo of AUX_ADV_IND PDU being received prior to creating the synchronization. Signed-off-by: Vinayak Kariappa Chettimada --- .../bluetooth/controller/ll_sw/ull_scan_aux.c | 41 ++++++++++++++++--- .../controller/ll_sw/ull_scan_types.h | 5 +++ subsys/bluetooth/controller/ll_sw/ull_sync.c | 17 +++++--- .../controller/ll_sw/ull_sync_internal.h | 2 +- 4 files changed, 53 insertions(+), 12 deletions(-) diff --git a/subsys/bluetooth/controller/ll_sw/ull_scan_aux.c b/subsys/bluetooth/controller/ll_sw/ull_scan_aux.c index db510e6a3d3..c1b484d757e 100644 --- a/subsys/bluetooth/controller/ll_sw/ull_scan_aux.c +++ b/subsys/bluetooth/controller/ll_sw/ull_scan_aux.c @@ -361,19 +361,30 @@ void ull_scan_aux_setup(memq_link_t *link, struct node_rx_hdr *rx) ptr = h->data; +#if defined(CONFIG_BT_CTLR_SYNC_PERIODIC) + bool is_aux_addr_match = false; +#endif /* CONFIG_BT_CTLR_SYNC_PERIODIC */ + if (h->adv_addr) { #if defined(CONFIG_BT_CTLR_SYNC_PERIODIC) /* Check if Periodic Advertising Synchronization to be created */ if (sync && (scan->periodic.state != LL_SYNC_STATE_CREATED)) { - /* Check address and update internal state */ #if defined(CONFIG_BT_CTLR_PRIVACY) - ull_sync_setup_addr_check(scan, pdu->tx_addr, ptr, - ftr->rl_idx); + uint8_t rl_idx = ftr->rl_idx; #else /* !CONFIG_BT_CTLR_PRIVACY */ - ull_sync_setup_addr_check(scan, pdu->tx_addr, ptr, 0U); + uint8_t rl_idx = 0U; #endif /* !CONFIG_BT_CTLR_PRIVACY */ + /* Check address and update internal state */ + is_aux_addr_match = + ull_sync_setup_addr_check(scan, pdu->tx_addr, + ptr, rl_idx); + if (is_aux_addr_match) { + scan->periodic.state = LL_SYNC_STATE_ADDR_MATCH; + } else { + scan->periodic.state = LL_SYNC_STATE_IDLE; + } } #endif /* CONFIG_BT_CTLR_SYNC_PERIODIC */ @@ -406,14 +417,22 @@ void ull_scan_aux_setup(memq_link_t *link, struct node_rx_hdr *rx) si = (void *)ptr; ptr += sizeof(*si); +#if defined(CONFIG_BT_CTLR_SYNC_PERIODIC) /* Check if Periodic Advertising Synchronization to be created. * Setup synchronization if address and SID match in the * Periodic Advertiser List or with the explicitly supplied. + * + * is_aux_addr_match, device address in auxiliary channel PDU; + * scan->periodic.param has not been assigned yet. + * Otherwise, address was in primary channel PDU and we are now + * checking SID (in SyncInfo) in auxiliary channel PDU. */ - if (IS_ENABLED(CONFIG_BT_CTLR_SYNC_PERIODIC) && sync && adi && - ull_sync_setup_sid_match(scan, adi->sid)) { + if (sync && aux && (is_aux_addr_match || + (scan->periodic.param == aux)) && + adi && ull_sync_setup_sid_match(scan, adi->sid)) { ull_sync_setup(scan, aux, rx, si); } +#endif /* CONFIG_BT_CTLR_SYNC_PERIODIC */ } if (h->tx_pwr) { @@ -522,6 +541,15 @@ void ull_scan_aux_setup(memq_link_t *link, struct node_rx_hdr *rx) aux->parent = lll ? (void *)lll : (void *)sync_lll; #if defined(CONFIG_BT_CTLR_SYNC_PERIODIC) + /* Store the aux context that has Periodic Advertising + * Synchronization address match. + */ + if (sync && (scan->periodic.state == LL_SYNC_STATE_ADDR_MATCH)) { + scan->periodic.param = aux; + } + + /* Store the node rx allocated for incomplete report, if needed. + */ aux->rx_incomplete = rx_incomplete; rx_incomplete = NULL; #endif /* CONFIG_BT_CTLR_SYNC_PERIODIC */ @@ -708,6 +736,7 @@ void ull_scan_aux_setup(memq_link_t *link, struct node_rx_hdr *rx) #if defined(CONFIG_BT_CTLR_SYNC_PERIODIC) if (sync && (scan->periodic.state != LL_SYNC_STATE_CREATED)) { scan->periodic.state = LL_SYNC_STATE_IDLE; + scan->periodic.param = NULL; } #endif /* CONFIG_BT_CTLR_SYNC_PERIODIC */ diff --git a/subsys/bluetooth/controller/ll_sw/ull_scan_types.h b/subsys/bluetooth/controller/ll_sw/ull_scan_types.h index a4247ecf304..8b07b4324c2 100644 --- a/subsys/bluetooth/controller/ll_sw/ull_scan_types.h +++ b/subsys/bluetooth/controller/ll_sw/ull_scan_types.h @@ -34,6 +34,11 @@ struct ll_scan_set { * cancelling sync create, hence the volatile keyword. */ struct ll_sync_set *volatile sync; + + /* Non-NULL when Periodic Advertising Synchronisation address + * matched. + */ + void *param; } periodic; #endif }; diff --git a/subsys/bluetooth/controller/ll_sw/ull_sync.c b/subsys/bluetooth/controller/ll_sw/ull_sync.c index 9eb28ed8845..cc67829cc35 100644 --- a/subsys/bluetooth/controller/ll_sw/ull_sync.c +++ b/subsys/bluetooth/controller/ll_sw/ull_sync.c @@ -157,11 +157,13 @@ uint8_t ll_sync_create(uint8_t options, uint8_t sid, uint8_t adv_addr_type, scan->periodic.cancelled = 0U; scan->periodic.state = LL_SYNC_STATE_IDLE; + scan->periodic.param = NULL; scan->periodic.filter_policy = options & BT_HCI_LE_PER_ADV_CREATE_SYNC_FP_USE_LIST; if (IS_ENABLED(CONFIG_BT_CTLR_PHY_CODED)) { scan_coded->periodic.cancelled = 0U; scan_coded->periodic.state = LL_SYNC_STATE_IDLE; + scan_coded->periodic.param = NULL; scan_coded->periodic.filter_policy = scan->periodic.filter_policy; } @@ -591,7 +593,7 @@ void ull_sync_release(struct ll_sync_set *sync) mem_release(sync, &sync_free); } -void ull_sync_setup_addr_check(struct ll_scan_set *scan, uint8_t addr_type, +bool ull_sync_setup_addr_check(struct ll_scan_set *scan, uint8_t addr_type, uint8_t *addr, uint8_t rl_idx) { /* Check if Periodic Advertiser list to be used */ @@ -607,7 +609,7 @@ void ull_sync_setup_addr_check(struct ll_scan_set *scan, uint8_t addr_type, BDADDR_SIZE); /* Address matched */ - scan->periodic.state = LL_SYNC_STATE_ADDR_MATCH; + return true; /* Check in Resolving List */ } else if (IS_ENABLED(CONFIG_BT_CTLR_PRIVACY) && @@ -622,14 +624,14 @@ void ull_sync_setup_addr_check(struct ll_scan_set *scan, uint8_t addr_type, scan->periodic.adv_addr_type += 2U; /* Address matched */ - scan->periodic.state = LL_SYNC_STATE_ADDR_MATCH; + return true; } /* Check with explicitly supplied address */ } else if ((addr_type == scan->periodic.adv_addr_type) && !memcmp(addr, scan->periodic.adv_addr, BDADDR_SIZE)) { /* Address matched */ - scan->periodic.state = LL_SYNC_STATE_ADDR_MATCH; + return true; /* Check identity address with explicitly supplied address */ } else if (IS_ENABLED(CONFIG_BT_CTLR_PRIVACY) && @@ -641,9 +643,11 @@ void ull_sync_setup_addr_check(struct ll_scan_set *scan, uint8_t addr_type, scan->periodic.adv_addr_type += 2U; /* Identity address matched */ - scan->periodic.state = LL_SYNC_STATE_ADDR_MATCH; + return true; } } + + return false; } bool ull_sync_setup_sid_match(struct ll_scan_set *scan, uint8_t sid) @@ -756,6 +760,7 @@ void ull_sync_setup(struct ll_scan_set *scan, struct ll_scan_aux_set *aux, /* Set the state to sync create */ scan->periodic.state = LL_SYNC_STATE_CREATED; + scan->periodic.param = NULL; if (IS_ENABLED(CONFIG_BT_CTLR_PHY_CODED)) { struct ll_scan_set *scan_1m; @@ -765,8 +770,10 @@ void ull_sync_setup(struct ll_scan_set *scan, struct ll_scan_aux_set *aux, scan_coded = ull_scan_set_get(SCAN_HANDLE_PHY_CODED); scan_coded->periodic.state = LL_SYNC_STATE_CREATED; + scan_coded->periodic.param = NULL; } else { scan_1m->periodic.state = LL_SYNC_STATE_CREATED; + scan_1m->periodic.param = NULL; } } diff --git a/subsys/bluetooth/controller/ll_sw/ull_sync_internal.h b/subsys/bluetooth/controller/ll_sw/ull_sync_internal.h index 62d50bd8021..48c5e8aaef7 100644 --- a/subsys/bluetooth/controller/ll_sw/ull_sync_internal.h +++ b/subsys/bluetooth/controller/ll_sw/ull_sync_internal.h @@ -9,7 +9,7 @@ int ull_sync_reset(void); uint16_t ull_sync_handle_get(struct ll_sync_set *sync); struct ll_sync_set *ull_sync_is_enabled_get(uint16_t handle); void ull_sync_release(struct ll_sync_set *sync); -void ull_sync_setup_addr_check(struct ll_scan_set *scan, uint8_t addr_type, +bool ull_sync_setup_addr_check(struct ll_scan_set *scan, uint8_t addr_type, uint8_t *addr, uint8_t rl_idx); bool ull_sync_setup_sid_match(struct ll_scan_set *scan, uint8_t sid); void ull_sync_setup(struct ll_scan_set *scan, struct ll_scan_aux_set *aux,