From 8573d3671eff835fc971dec2a4b896adc3d916a8 Mon Sep 17 00:00:00 2001 From: Vinayak Kariappa Chettimada Date: Thu, 29 Feb 2024 14:36:10 +0100 Subject: [PATCH] Bluetooth: Controller: Fix missing radio timer comp and range delay Fix missing PPI to timer start compensation and missing inclusion of range delay in the calculation of packet header receive timeout value. Signed-off-by: Vinayak Kariappa Chettimada --- .../ll_sw/nordic/hal/nrf5/radio/radio.c | 5 ++-- .../ll_sw/nordic/hal/nrf5/radio/radio_nrf5.h | 25 ++++++++++++------- .../controller/ll_sw/nordic/lll/lll_adv.c | 6 +++-- .../controller/ll_sw/nordic/lll/lll_adv_aux.c | 5 ++-- .../ll_sw/nordic/lll/lll_central_iso.c | 4 +-- .../controller/ll_sw/nordic/lll/lll_conn.c | 7 +++--- .../controller/ll_sw/nordic/lll/lll_scan.c | 6 +++-- .../ll_sw/nordic/lll/lll_scan_aux.c | 5 ++-- .../ll_sw/nordic/lll/lll_sync_iso.c | 2 +- .../ll_sw/nordic/lll/lll_tim_internal.h | 6 ----- 10 files changed, 40 insertions(+), 31 deletions(-) diff --git a/subsys/bluetooth/controller/ll_sw/nordic/hal/nrf5/radio/radio.c b/subsys/bluetooth/controller/ll_sw/nordic/hal/nrf5/radio/radio.c index 97919a18dd3656..b681fa67a14941 100644 --- a/subsys/bluetooth/controller/ll_sw/nordic/hal/nrf5/radio/radio.c +++ b/subsys/bluetooth/controller/ll_sw/nordic/hal/nrf5/radio/radio.c @@ -874,8 +874,9 @@ void sw_switch(uint8_t dir_curr, uint8_t dir_next, uint8_t phy_curr, uint8_t fla } if (delay < SW_SWITCH_TIMER->CC[cc]) { - nrf_timer_cc_set(SW_SWITCH_TIMER, cc, - (SW_SWITCH_TIMER->CC[cc] - delay)); + nrf_timer_cc_set(SW_SWITCH_TIMER, + cc, + (SW_SWITCH_TIMER->CC[cc] - delay - HAL_RADIO_TMR_START_DELAY_US)); } else { nrf_timer_cc_set(SW_SWITCH_TIMER, cc, 1); } diff --git a/subsys/bluetooth/controller/ll_sw/nordic/hal/nrf5/radio/radio_nrf5.h b/subsys/bluetooth/controller/ll_sw/nordic/hal/nrf5/radio/radio_nrf5.h index 842b4189184d50..dce326d469aa90 100644 --- a/subsys/bluetooth/controller/ll_sw/nordic/hal/nrf5/radio/radio_nrf5.h +++ b/subsys/bluetooth/controller/ll_sw/nordic/hal/nrf5/radio/radio_nrf5.h @@ -10,7 +10,9 @@ /* Common radio resources */ #include "radio_nrf5_resources.h" -/* Helpers for radio timing conversions */ +/* Helpers for radio timing conversions. + * These has to come before the radio_*.h include below. + */ #define HAL_RADIO_NS2US_CEIL(ns) ((ns + 999)/1000) #define HAL_RADIO_NS2US_ROUND(ns) ((ns + 500)/1000) @@ -42,7 +44,9 @@ #error "Unsupported SoC." #endif -/* Define to reset PPI registration */ +/* Define to reset PPI registration. + * This has to come before the ppi/dppi includes below. + */ #define NRF_PPI_NONE 0 /* This has to come before the ppi/dppi includes below. */ @@ -66,13 +70,6 @@ #include "radio_nrf5_txp.h" -/* SoC specific Radio PDU length field maximum value */ -#if defined(CONFIG_SOC_SERIES_NRF51X) -#define HAL_RADIO_PDU_LEN_MAX (BIT(5) - 1) -#else -#define HAL_RADIO_PDU_LEN_MAX (BIT(8) - 1) -#endif - /* Common NRF_RADIO power-on reset value. Refer to Product Specification, * RADIO Registers section for the documented reset values. * @@ -80,3 +77,13 @@ * In the future if MDK or nRFx header include these, use them instead. */ #define HAL_RADIO_RESET_VALUE_PCNF1 0x00000000UL + +/* SoC specific Radio PDU length field maximum value */ +#if defined(CONFIG_SOC_SERIES_NRF51X) +#define HAL_RADIO_PDU_LEN_MAX (BIT(5) - 1) +#else +#define HAL_RADIO_PDU_LEN_MAX (BIT(8) - 1) +#endif + +/* This is delay between PPI task START and timer actual start counting. */ +#define HAL_RADIO_TMR_START_DELAY_US 1U diff --git a/subsys/bluetooth/controller/ll_sw/nordic/lll/lll_adv.c b/subsys/bluetooth/controller/ll_sw/nordic/lll/lll_adv.c index 851c82692b81d0..25c085684805e4 100644 --- a/subsys/bluetooth/controller/ll_sw/nordic/lll/lll_adv.c +++ b/subsys/bluetooth/controller/ll_sw/nordic/lll/lll_adv.c @@ -1196,8 +1196,10 @@ static void isr_tx(void *param) } #endif /* CONFIG_BT_CTLR_PRIVACY */ - /* +/- 2us active clock jitter, +1 us hcto compensation */ - hcto = radio_tmr_tifs_base_get() + EVENT_IFS_US + 4 + 1; + /* +/- 2us active clock jitter, +1 us PPI to timer start compensation */ + hcto = radio_tmr_tifs_base_get() + EVENT_IFS_US + + (EVENT_CLOCK_JITTER_US << 1) + RANGE_DELAY_US + + HAL_RADIO_TMR_START_DELAY_US; hcto += radio_rx_chain_delay_get(phy_p, 0); hcto += addr_us_get(phy_p); hcto -= radio_tx_chain_delay_get(phy_p, 0); diff --git a/subsys/bluetooth/controller/ll_sw/nordic/lll/lll_adv_aux.c b/subsys/bluetooth/controller/ll_sw/nordic/lll/lll_adv_aux.c index 6b61c3b559f9cd..e16cddb5d3a1cb 100644 --- a/subsys/bluetooth/controller/ll_sw/nordic/lll/lll_adv_aux.c +++ b/subsys/bluetooth/controller/ll_sw/nordic/lll/lll_adv_aux.c @@ -509,9 +509,10 @@ static void isr_tx_rx(void *param) } #endif /* CONFIG_BT_CTLR_PRIVACY */ - /* +/- 2us active clock jitter, +1 us hcto compensation */ + /* +/- 2us active clock jitter, +1 us PPI to timer start compensation */ hcto = radio_tmr_tifs_base_get() + EVENT_IFS_US + - (EVENT_CLOCK_JITTER_US << 1U) + 1U; + (EVENT_CLOCK_JITTER_US << 1) + RANGE_DELAY_US + + HAL_RADIO_TMR_START_DELAY_US; hcto += radio_rx_chain_delay_get(lll->phy_s, PHY_FLAGS_S8); hcto += addr_us_get(lll->phy_s); hcto -= radio_tx_chain_delay_get(lll->phy_s, PHY_FLAGS_S8); diff --git a/subsys/bluetooth/controller/ll_sw/nordic/lll/lll_central_iso.c b/subsys/bluetooth/controller/ll_sw/nordic/lll/lll_central_iso.c index 751c3915fc20ad..48247b18e6f9fc 100644 --- a/subsys/bluetooth/controller/ll_sw/nordic/lll/lll_central_iso.c +++ b/subsys/bluetooth/controller/ll_sw/nordic/lll/lll_central_iso.c @@ -500,10 +500,10 @@ static void isr_tx(void *param) /* assert if radio packet ptr is not set and radio started rx */ LL_ASSERT(!radio_is_ready()); - /* +/- 2us active clock jitter, +1 us hcto compensation */ + /* +/- 2us active clock jitter, +1 us PPI to timer start compensation */ hcto = radio_tmr_tifs_base_get() + EVENT_IFS_US + (EVENT_CLOCK_JITTER_US << 1) + RANGE_DELAY_US + - HCTO_START_DELAY_US; + HAL_RADIO_TMR_START_DELAY_US; #if defined(CONFIG_BT_CTLR_PHY) hcto += radio_rx_chain_delay_get(cis_lll->rx.phy, PHY_FLAGS_S8); 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 1f3cd8a04afb64..faf54dac417145 100644 --- a/subsys/bluetooth/controller/ll_sw/nordic/lll/lll_conn.c +++ b/subsys/bluetooth/controller/ll_sw/nordic/lll/lll_conn.c @@ -579,9 +579,10 @@ void lll_conn_isr_tx(void *param) } #endif /* CONFIG_BT_CTLR_DF_CONN_CTE_TX */ - /* +/- 2us active clock jitter, +1 us hcto compensation */ - hcto = radio_tmr_tifs_base_get() + EVENT_IFS_US + (EVENT_CLOCK_JITTER_US << 1) + - RANGE_DELAY_US + HCTO_START_DELAY_US; + /* +/- 2us active clock jitter, +1 us PPI to timer start compensation */ + hcto = radio_tmr_tifs_base_get() + EVENT_IFS_US + + (EVENT_CLOCK_JITTER_US << 1) + RANGE_DELAY_US + + HAL_RADIO_TMR_START_DELAY_US; #if defined(CONFIG_BT_CTLR_DF_CONN_CTE_TX) hcto += cte_len; #endif /* CONFIG_BT_CTLR_DF_CONN_CTE_TX */ diff --git a/subsys/bluetooth/controller/ll_sw/nordic/lll/lll_scan.c b/subsys/bluetooth/controller/ll_sw/nordic/lll/lll_scan.c index 7279d407ad8a9f..a4e1a2bd705ad2 100644 --- a/subsys/bluetooth/controller/ll_sw/nordic/lll/lll_scan.c +++ b/subsys/bluetooth/controller/ll_sw/nordic/lll/lll_scan.c @@ -805,8 +805,10 @@ static void isr_tx(void *param) } #endif /* CONFIG_BT_CTLR_PRIVACY */ - /* +/- 2us active clock jitter, +1 us hcto compensation */ - hcto = radio_tmr_tifs_base_get() + EVENT_IFS_US + 4 + 1; + /* +/- 2us active clock jitter, +1 us PPI to timer start compensation */ + hcto = radio_tmr_tifs_base_get() + EVENT_IFS_US + + (EVENT_CLOCK_JITTER_US << 1) + RANGE_DELAY_US + + HAL_RADIO_TMR_START_DELAY_US; hcto += radio_rx_chain_delay_get(0, 0); hcto += addr_us_get(0); hcto -= radio_tx_chain_delay_get(0, 0); diff --git a/subsys/bluetooth/controller/ll_sw/nordic/lll/lll_scan_aux.c b/subsys/bluetooth/controller/ll_sw/nordic/lll/lll_scan_aux.c index 57132835fba664..a40e90e20a09ac 100644 --- a/subsys/bluetooth/controller/ll_sw/nordic/lll/lll_scan_aux.c +++ b/subsys/bluetooth/controller/ll_sw/nordic/lll/lll_scan_aux.c @@ -1350,9 +1350,10 @@ static void isr_tx(struct lll_scan_aux *lll_aux, void *pdu_rx, } #endif /* CONFIG_BT_CTLR_PRIVACY */ - /* +/- 2us active clock jitter, +1 us hcto compensation */ + /* +/- 2us active clock jitter, +1 us PPI to timer start compensation */ hcto = radio_tmr_tifs_base_get() + EVENT_IFS_US + - (EVENT_CLOCK_JITTER_US << 1U) + RANGE_DELAY_US + 1U; + (EVENT_CLOCK_JITTER_US << 1) + RANGE_DELAY_US + + HAL_RADIO_TMR_START_DELAY_US; hcto += radio_rx_chain_delay_get(lll_aux->phy, PHY_FLAGS_S8); hcto += addr_us_get(lll_aux->phy); hcto -= radio_tx_chain_delay_get(lll_aux->phy, PHY_FLAGS_S8); 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 2c1125e02d4039..e9111981d6f34d 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 @@ -1004,7 +1004,7 @@ static void isr_rx(void *param) * the current subevent we are listening. */ hcto += (((EVENT_CLOCK_JITTER_US << 1) * nse) << 1) + - RANGE_DELAY_US + HCTO_START_DELAY_US; + RANGE_DELAY_US + HAL_RADIO_TMR_START_DELAY_US; } else { /* First subevent PDU was not received, hence setup radio packet * timer header complete timeout from where the first subevent diff --git a/subsys/bluetooth/controller/ll_sw/nordic/lll/lll_tim_internal.h b/subsys/bluetooth/controller/ll_sw/nordic/lll/lll_tim_internal.h index 41aa2b0454bae4..eb1858ca1bb24d 100644 --- a/subsys/bluetooth/controller/ll_sw/nordic/lll/lll_tim_internal.h +++ b/subsys/bluetooth/controller/ll_sw/nordic/lll/lll_tim_internal.h @@ -12,12 +12,6 @@ #define RANGE_DISTANCE 1000 /* meters */ #define RANGE_DELAY_US (2 * RANGE_DISTANCE * 4 / 1000) -/* This is a compensation of delay between PPI task START and timer start counting. - * HCTO is a timer used to stop Radio peripheral in receive mode if packet address was not - * received. - */ -#define HCTO_START_DELAY_US 1U - static inline uint32_t addr_us_get(uint8_t phy) { switch (phy) {