From bcd28e0a866b44b3db10839cad7cc68f514ae4f4 Mon Sep 17 00:00:00 2001 From: Vinayak Kariappa Chettimada Date: Sat, 6 Jul 2024 02:21:13 +0200 Subject: [PATCH] Bluetooth: Controller: Fix sw switch single timer for spurious TXEN/RXEN Fix software tIFS switching using single timer from triggering spurious TXEN/RXEN if the first remainder is shorter than the tIFS delay calculated in the sw_switch() function. Signed-off-by: Vinayak Kariappa Chettimada --- .../ll_sw/nordic/hal/nrf5/radio/radio.c | 56 +++++++++++++++++-- .../hal/nrf5/radio/radio_nrf5_resources.h | 15 +++++ .../controller/ll_sw/nordic/lll/lll_scan.c | 1 + 3 files changed, 68 insertions(+), 4 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 c63883bf4b2..73b4fbbdd3d 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 @@ -1337,6 +1337,21 @@ uint32_t radio_tmr_start(uint8_t trx, uint32_t ticks_start, uint32_t remainder) { hal_ticker_remove_jitter(&ticks_start, &remainder); +#if defined(CONFIG_BT_CTLR_SW_SWITCH_SINGLE_TIMER) + /* When using single timer for software tIFS switching, ensure that + * the timer compare value is large enough to consider radio ISR + * latency so that the ISR is able to disable the PPI/DPPI that again + * could trigger the TXEN/RXEN task. + * The timer is cleared on Radio End and if the PPI/DPPI is not disabled + * by the Radio ISR, the compare will trigger again. + */ + uint32_t latency_ticks; + + latency_ticks = HAL_TICKER_US_TO_TICKS(HAL_RADIO_ISR_LATENCY_MAX_US); + ticks_start -= latency_ticks; + remainder += HAL_TICKER_TICKS_TO_US(latency_ticks); +#endif /* !CONFIG_BT_CTLR_SW_SWITCH_SINGLE_TIMER */ + nrf_timer_task_trigger(EVENT_TIMER, NRF_TIMER_TASK_CLEAR); EVENT_TIMER->MODE = 0; EVENT_TIMER->PRESCALER = HAL_EVENT_TIMER_PRESCALER_VALUE; @@ -1388,11 +1403,27 @@ uint32_t radio_tmr_start_tick(uint8_t trx, uint32_t tick) { uint32_t remainder_us; + /* Setup compare event with min. 1 us offset */ + remainder_us = 1; + +#if defined(CONFIG_BT_CTLR_SW_SWITCH_SINGLE_TIMER) + /* When using single timer for software tIFS switching, ensure that + * the timer compare value is large enough to consider radio ISR + * latency so that the ISR is able to disable the PPI/DPPI that again + * could trigger the TXEN/RXEN task. + * The timer is cleared on Radio End and if the PPI/DPPI is not disabled + * by the Radio ISR, the compare will trigger again. + */ + uint32_t latency_ticks; + + latency_ticks = HAL_TICKER_US_TO_TICKS(HAL_RADIO_ISR_LATENCY_MAX_US); + ticks_start -= latency_ticks; + remainder_us += HAL_TICKER_TICKS_TO_US(latency_ticks); +#endif /* !CONFIG_BT_CTLR_SW_SWITCH_SINGLE_TIMER */ + nrf_timer_task_trigger(EVENT_TIMER, NRF_TIMER_TASK_STOP); nrf_timer_task_trigger(EVENT_TIMER, NRF_TIMER_TASK_CLEAR); - /* Setup compare event with min. 1 us offset */ - remainder_us = 1; nrf_timer_cc_set(EVENT_TIMER, 0, remainder_us); nrf_rtc_cc_set(NRF_RTC, 2, tick); @@ -1452,6 +1483,7 @@ uint32_t radio_tmr_start_us(uint8_t trx, uint32_t start_us) /* start_us could be the current count in the timer */ uint32_t now_us = start_us; + uint32_t actual_us; /* Setup PPI while determining the latency in doing so */ do { @@ -1459,8 +1491,24 @@ uint32_t radio_tmr_start_us(uint8_t trx, uint32_t start_us) start_us = (now_us << 1) - start_us; /* Setup compare event with min. 1 us offset */ + actual_us = start_us + 1U; + +#if defined(CONFIG_BT_CTLR_SW_SWITCH_SINGLE_TIMER) + /* When using single timer for software tIFS switching, ensure that + * the timer compare value is large enough to consider radio ISR + * latency so that the ISR is able to disable the PPI/DPPI that again + * could trigger the TXEN/RXEN task. + * The timer is cleared on Radio End and if the PPI/DPPI is not disabled + * by the Radio ISR, the compare will trigger again. + */ + uint32_t latency_ticks; + + latency_ticks = HAL_TICKER_US_TO_TICKS(HAL_RADIO_ISR_LATENCY_MAX_US); + actual_us += HAL_TICKER_TICKS_TO_US(latency_ticks); +#endif /* !CONFIG_BT_CTLR_SW_SWITCH_SINGLE_TIMER */ + nrf_timer_event_clear(EVENT_TIMER, NRF_TIMER_EVENT_COMPARE0); - nrf_timer_cc_set(EVENT_TIMER, 0, start_us + 1U); + nrf_timer_cc_set(EVENT_TIMER, 0, actual_us); /* Capture the current time */ nrf_timer_task_trigger(EVENT_TIMER, @@ -1469,7 +1517,7 @@ uint32_t radio_tmr_start_us(uint8_t trx, uint32_t start_us) now_us = EVENT_TIMER->CC[HAL_EVENT_TIMER_SAMPLE_CC_OFFSET]; } while ((now_us > start_us) && (EVENT_TIMER->EVENTS_COMPARE[0] == 0U)); - return start_us + 1U; + return actual_us; } uint32_t radio_tmr_start_now(uint8_t trx) diff --git a/subsys/bluetooth/controller/ll_sw/nordic/hal/nrf5/radio/radio_nrf5_resources.h b/subsys/bluetooth/controller/ll_sw/nordic/hal/nrf5/radio/radio_nrf5_resources.h index 15faa3a53b8..7a8fd0e8f13 100644 --- a/subsys/bluetooth/controller/ll_sw/nordic/hal/nrf5/radio/radio_nrf5_resources.h +++ b/subsys/bluetooth/controller/ll_sw/nordic/hal/nrf5/radio/radio_nrf5_resources.h @@ -11,6 +11,9 @@ #define EVENT_TIMER_ID 0 #define EVENT_TIMER _CONCAT(NRF_TIMER, EVENT_TIMER_ID) +/* Single timer feature not supported when using h/w tIFS switching */ +#define HAL_RADIO_ISR_LATENCY_MAX_US 0U + /* Wrapper for EVENTS_END event generated by Radio peripheral at the very end of the transmission * or reception of a PDU on air. In case of regular PDU it is generated when last bit of CRC is * received or transmitted. @@ -32,6 +35,12 @@ #define SW_SWITCH_TIMER EVENT_TIMER +/* Radio ISR Latency to be considered with single timer used so that the PPI/ + * DPPI is disabled in time when the timer is cleared on radio end, so that + * the timer compare should not trigger TXEN/RXEN immediately on radio end. + */ +#define HAL_RADIO_ISR_LATENCY_MAX_US 150U + #if defined(CONFIG_BT_CTLR_PHY_CODED) #define SW_SWITCH_TIMER_EVTS_COMP_BASE 3 #define SW_SWITCH_TIMER_EVTS_COMP_S2_BASE 5 @@ -73,6 +82,12 @@ #define EVENT_TIMER _CONCAT(NRF_TIMER, EVENT_TIMER_ID) #define SW_SWITCH_TIMER NRF_TIMER1 + +/* When using dedicated timer used for tIFS switching, compensation to avoid + * spurious TXEN/RXEN due to timer being clear is not needed. + */ +#define HAL_RADIO_ISR_LATENCY_MAX_US 0U + #define SW_SWITCH_TIMER_EVTS_COMP_BASE 0 #if defined(CONFIG_BT_CTLR_PHY_CODED) 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 f4a445d556c..ce37381dda1 100644 --- a/subsys/bluetooth/controller/ll_sw/nordic/lll/lll_scan.c +++ b/subsys/bluetooth/controller/ll_sw/nordic/lll/lll_scan.c @@ -939,6 +939,7 @@ static void isr_window(void *param) uint32_t ticks_at_start; ticks_at_start = ticker_ticks_now_get() + + HAL_TICKER_US_TO_TICKS(HAL_RADIO_ISR_LATENCY_MAX_US) + HAL_TICKER_CNTR_CMP_OFFSET_MIN; remainder_us = radio_tmr_start_tick(0, ticks_at_start); #else /* !CONFIG_BT_CENTRAL && !CONFIG_BT_CTLR_ADV_EXT */