Skip to content

Commit 0474bee

Browse files
nimble/phy/nrf5x: Fix races in TIMER0 setup
In cases where RADIO ISR is delayed during transitions it's possible that TIMER0 is not set properly and that could stall phy in waiting state. This is more likely to happen on non-Mynewt systems where IRQs can be configured/handled differently than on Mynewt. There are few issues handled here: 1. Make sure we always set CC register in proper order, i.e. clear COMPARE event, then enable related PPI, then set the proper CC value. This ensures that whenever COMPARE event is triggered, radio is ready to handle it. 2. Handle case where CC may be set to the current value of TIMER0. This would not trigger COMPARE event so it should be handled as a miss. 3. Reset all CC registers to 0 on start of each event. This prevents spurious triggers of COMPARE event due to some leftovers from previous event. We don't need to do this on each TX/RX because at the start of each TX/RX the values in CC registers are either start times or captured timer of previous TX/RX thus they are always in the past.
1 parent b973df0 commit 0474bee

1 file changed

Lines changed: 46 additions & 24 deletions

File tree

nimble/drivers/nrf5x/src/ble_phy.c

Lines changed: 46 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -349,6 +349,32 @@ struct nrf_ccm_data
349349
struct nrf_ccm_data g_nrf_ccm_data;
350350
#endif
351351

352+
static void
353+
timer0_reset(void)
354+
{
355+
/* Reset CC registers to avoid triggering spurious COMPARE events */
356+
NRF_TIMER0->CC[0] = 0;
357+
NRF_TIMER0->CC[1] = 0;
358+
NRF_TIMER0->CC[2] = 0;
359+
NRF_TIMER0->CC[3] = 0;
360+
}
361+
362+
static bool
363+
timer0_did_miss(int cc, int cc_scratch)
364+
{
365+
uint32_t now, target;
366+
367+
nrf_timer_task_trigger(NRF_TIMER0, nrf_timer_capture_task_get(cc_scratch));
368+
target = NRF_TIMER0->CC[cc];
369+
now = NRF_TIMER0->CC[cc_scratch];
370+
371+
/* COMPARE event is not triggered when CC is set to the current value of
372+
* TIMER0 (at the time it was set), so the case of equal CC values without
373+
* a COMPARE event should be also considered a miss here.
374+
*/
375+
return (now >= target) && !NRF_TIMER0->EVENTS_COMPARE[cc];
376+
}
377+
352378
#if MYNEWT_VAL(BLE_LL_PHY)
353379

354380
/* Packet start offset (in usecs). This is the preamble plus access address.
@@ -706,6 +732,8 @@ ble_phy_set_start_time(uint32_t cputime, uint8_t rem_us, bool tx)
706732
return -1;
707733
}
708734

735+
timer0_reset();
736+
709737
/* Clear and set TIMER0 to fire off at proper time */
710738
nrf_timer_task_trigger(NRF_TIMER0, NRF_TIMER_TASK_CLEAR);
711739
nrf_timer_cc_set(NRF_TIMER0, 0, radio_rem_us + rem_us_corr);
@@ -885,27 +913,24 @@ ble_phy_wfr_enable(int txrx, uint8_t tx_phy_mode, uint32_t wfr_usecs)
885913
/* Adjust for delay between actual access address RX and EVENT_ADDRESS */
886914
end_time += g_ble_phy_t_rxaddrdelay[phy];
887915

888-
/* wfr_secs is the time from rxen until timeout */
889-
nrf_timer_cc_set(NRF_TIMER0, 3, end_time);
890-
NRF_TIMER0->EVENTS_COMPARE[3] = 0;
891-
892916
/* Enable wait for response PPI */
917+
NRF_TIMER0->EVENTS_COMPARE[3] = 0;
893918
phy_ppi_wfr_enable();
919+
nrf_timer_cc_set(NRF_TIMER0, 3, end_time);
894920

895921
/*
896-
* It may happen that if CPU is halted for a brief moment (e.g. during flash
897-
* erase or write), TIMER0 already counted past CC[3] and thus wfr will not
898-
* fire as expected. In case this happened, let's just disable PPIs for wfr
899-
* and trigger wfr manually (i.e. disable radio).
922+
* It may happen that if CPU is halted for a brief moment (e.g. during
923+
* flash, erase or write), TIMER0 already counted past CC[3] and thus wfr
924+
* will not fire as expected. In case this happened, let's just disable
925+
* PPIs for wfr and trigger wfr manually (i.e. disable radio).
900926
*
901927
* Note that the same applies to RX start time set in CC[0] but since it
902928
* should fire earlier than wfr, fixing wfr is enough.
903929
*
904-
* CC[1] is only used as a reference on RX start, we do not need it here so
905-
* it can be used to read TIMER0 counter.
930+
* CC[1] is only used as a reference on RX start. We do not need it here,
931+
* so we can use it as a scratch register.
906932
*/
907-
nrf_timer_task_trigger(NRF_TIMER0, NRF_TIMER_TASK_CAPTURE1);
908-
if (NRF_TIMER0->CC[1] > NRF_TIMER0->CC[3]) {
933+
if (timer0_did_miss(3, 1)) {
909934
phy_ppi_wfr_disable();
910935
nrf_radio_task_trigger(NRF_RADIO, NRF_RADIO_TASK_DISABLE);
911936
}
@@ -1096,15 +1121,15 @@ ble_phy_tx_end_isr(void)
10961121

10971122
#if PHY_USE_FEM_LNA
10981123
fem_time = rx_time - MYNEWT_VAL(BLE_FEM_LNA_TURN_ON_US);
1099-
nrf_timer_cc_set(NRF_TIMER0, 2, fem_time);
11001124
NRF_TIMER0->EVENTS_COMPARE[2] = 0;
11011125
phy_fem_enable_lna();
1126+
nrf_timer_cc_set(NRF_TIMER0, 2, fem_time);
11021127
#endif
11031128

11041129
radio_time = rx_time - BLE_PHY_T_RXENFAST;
1105-
nrf_timer_cc_set(NRF_TIMER0, 0, radio_time);
11061130
NRF_TIMER0->EVENTS_COMPARE[0] = 0;
11071131
phy_ppi_timer0_compare0_to_radio_rxen_enable();
1132+
nrf_timer_cc_set(NRF_TIMER0, 0, radio_time);
11081133

11091134
/* In case TIMER0 did already count past CC[0] and/or CC[2], radio
11101135
* and/or LNA may not be enabled. In any case we won't be stuck since
@@ -1142,18 +1167,17 @@ ble_phy_tx_end_isr(void)
11421167
tx_time -= g_ble_phy_t_txdelay[g_ble_phy_data.phy_cur_phy_mode];
11431168

11441169
radio_time = tx_time - BLE_PHY_T_TXENFAST;
1145-
nrf_timer_cc_set(NRF_TIMER0, 0, radio_time);
11461170
NRF_TIMER0->EVENTS_COMPARE[0] = 0;
11471171
phy_ppi_timer0_compare0_to_radio_txen_enable();
1172+
nrf_timer_cc_set(NRF_TIMER0, 0, radio_time);
11481173

11491174
#if PHY_USE_FEM_PA
1150-
nrf_timer_cc_set(NRF_TIMER0, 2, fem_time);
11511175
NRF_TIMER0->EVENTS_COMPARE[2] = 0;
11521176
phy_fem_enable_pa();
1177+
nrf_timer_cc_set(NRF_TIMER0, 2, fem_time);
11531178
#endif
11541179

1155-
nrf_timer_task_trigger(NRF_TIMER0, NRF_TIMER_TASK_CAPTURE3);
1156-
if (NRF_TIMER0->CC[3] > NRF_TIMER0->CC[0]) {
1180+
if (timer0_did_miss(0, 3)) {
11571181
phy_ppi_timer0_compare0_to_radio_txen_disable();
11581182
g_ble_phy_data.phy_transition_late = 1;
11591183
}
@@ -1292,14 +1316,14 @@ ble_phy_rx_end_isr(void)
12921316
tx_time -= g_ble_phy_t_txdelay[g_ble_phy_data.phy_cur_phy_mode];
12931317

12941318
radio_time = tx_time - BLE_PHY_T_TXENFAST;
1295-
nrf_timer_cc_set(NRF_TIMER0, 0, radio_time);
12961319
NRF_TIMER0->EVENTS_COMPARE[0] = 0;
12971320
phy_ppi_timer0_compare0_to_radio_txen_enable();
1321+
nrf_timer_cc_set(NRF_TIMER0, 0, radio_time);
12981322

12991323
#if PHY_USE_FEM_PA
1300-
nrf_timer_cc_set(NRF_TIMER0, 2, fem_time);
13011324
NRF_TIMER0->EVENTS_COMPARE[2] = 0;
13021325
phy_fem_enable_pa();
1326+
nrf_timer_cc_set(NRF_TIMER0, 2, fem_time);
13031327
#endif
13041328

13051329
/* Need to check if TIMER0 did not already count past CC[0] and/or CC[2], so
@@ -1308,11 +1332,9 @@ ble_phy_rx_end_isr(void)
13081332
*
13091333
* Note: CC[3] is used only for wfr which we do not need here.
13101334
*/
1311-
nrf_timer_task_trigger(NRF_TIMER0, NRF_TIMER_TASK_CAPTURE3);
1312-
is_late = (NRF_TIMER0->CC[3] > radio_time) && !NRF_TIMER0->EVENTS_COMPARE[0];
1335+
is_late = timer0_did_miss(0, 3);
13131336
#if PHY_USE_FEM_PA
1314-
is_late = is_late ||
1315-
((NRF_TIMER0->CC[3] > fem_time) && !NRF_TIMER0->EVENTS_COMPARE[2]);
1337+
is_late = is_late || timer0_did_miss(2, 3);
13161338
#endif
13171339
if (is_late) {
13181340
phy_ppi_timer0_compare0_to_radio_txen_disable();

0 commit comments

Comments
 (0)