Skip to content

nimble/phy/nrf5x: fix race conditions in TIMER0 setup #2036

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

nimble/phy/nrf5x: fix race conditions in TIMER0 setup #2036

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 45 additions & 24 deletions nimble/drivers/nrf5x/src/ble_phy.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -583,6 +583,31 @@ ble_phy_rxpdu_copy(uint8_t *dptr, struct os_mbuf *rxpdu)
sizeof(struct ble_mbuf_hdr));
}

/**
* There is no way on nRF5 to disable a COMPARE event entirely. So we fake
* it, by setting it as far in the future as possible (by triggering a
* capture to it), and then clearing the event if it was set.
*/
static void
timer0_safely_reset(int cc)
{
nrf_timer_task_trigger(NRF_TIMER0, nrf_timer_capture_task_get(cc));
NRF_TIMER0->EVENTS_COMPARE[cc] = 0;
}

/**
* It is possible that we set up a timer, but we were too late -- it didn't
* fire. Detect when this happens, reliably, by capturing the current time
* to a scratch counter and comparing with wraparound.
*/
static bool
timer0_did_miss(int cc, int cc_scratch)
{
nrf_timer_task_trigger(NRF_TIMER0, nrf_timer_capture_task_get(cc_scratch));
uint32_t ticks_rem = NRF_TIMER0->CC[cc] - NRF_TIMER0->CC[cc_scratch];
return (ticks_rem >= 0x80000000) && !NRF_TIMER0->EVENTS_COMPARE[cc];
}

/**
* Called when we want to wait if the radio is in either the rx or tx
* disable states. We want to wait until that state is over before doing
Expand Down Expand Up @@ -885,13 +910,13 @@ ble_phy_wfr_enable(int txrx, uint8_t tx_phy_mode, uint32_t wfr_usecs)
/* Adjust for delay between actual access address RX and EVENT_ADDRESS */
end_time += g_ble_phy_t_rxaddrdelay[phy];

/* wfr_secs is the time from rxen until timeout */
nrf_timer_cc_set(NRF_TIMER0, 3, end_time);
NRF_TIMER0->EVENTS_COMPARE[3] = 0;

/* Enable wait for response PPI */
timer0_safely_reset(3);
phy_ppi_wfr_enable();

/* wfr_secs is the time from rxen until timeout */
nrf_timer_cc_set(NRF_TIMER0, 3, end_time);

/*
* It may happen that if CPU is halted for a brief moment (e.g. during flash
* erase or write), TIMER0 already counted past CC[3] and thus wfr will not
Expand All @@ -904,8 +929,7 @@ ble_phy_wfr_enable(int txrx, uint8_t tx_phy_mode, uint32_t wfr_usecs)
* CC[1] is only used as a reference on RX start, we do not need it here so
* it can be used to read TIMER0 counter.
*/
nrf_timer_task_trigger(NRF_TIMER0, NRF_TIMER_TASK_CAPTURE1);
if (NRF_TIMER0->CC[1] > NRF_TIMER0->CC[3]) {
if (timer0_did_miss(3, 1 /* scratch */)) {
phy_ppi_wfr_disable();
nrf_radio_task_trigger(NRF_RADIO, NRF_RADIO_TASK_DISABLE);
}
Expand Down Expand Up @@ -1096,15 +1120,15 @@ ble_phy_tx_end_isr(void)

#if PHY_USE_FEM_LNA
fem_time = rx_time - MYNEWT_VAL(BLE_FEM_LNA_TURN_ON_US);
nrf_timer_cc_set(NRF_TIMER0, 2, fem_time);
NRF_TIMER0->EVENTS_COMPARE[2] = 0;
timer0_safely_reset(2);
phy_fem_enable_lna();
nrf_timer_cc_set(NRF_TIMER0, 2, fem_time);
#endif

radio_time = rx_time - BLE_PHY_T_RXENFAST;
nrf_timer_cc_set(NRF_TIMER0, 0, radio_time);
NRF_TIMER0->EVENTS_COMPARE[0] = 0;
timer0_safely_reset(0);
phy_ppi_timer0_compare0_to_radio_rxen_enable();
nrf_timer_cc_set(NRF_TIMER0, 0, radio_time);

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

radio_time = tx_time - BLE_PHY_T_TXENFAST;
nrf_timer_cc_set(NRF_TIMER0, 0, radio_time);
NRF_TIMER0->EVENTS_COMPARE[0] = 0;
timer0_safely_reset(0);
phy_ppi_timer0_compare0_to_radio_txen_enable();
nrf_timer_cc_set(NRF_TIMER0, 0, radio_time);

#if PHY_USE_FEM_PA
nrf_timer_cc_set(NRF_TIMER0, 2, fem_time);
NRF_TIMER0->EVENTS_COMPARE[2] = 0;
timer0_safely_reset(2);
phy_fem_enable_pa();
nrf_timer_cc_set(NRF_TIMER0, 2, fem_time);
#endif

nrf_timer_task_trigger(NRF_TIMER0, NRF_TIMER_TASK_CAPTURE3);
if (NRF_TIMER0->CC[3] > NRF_TIMER0->CC[0]) {
if (timer0_did_miss(0, 3 /* scratch */)) {
phy_ppi_timer0_compare0_to_radio_txen_disable();
g_ble_phy_data.phy_transition_late = 1;
}
Expand Down Expand Up @@ -1292,14 +1315,14 @@ ble_phy_rx_end_isr(void)
tx_time -= g_ble_phy_t_txdelay[g_ble_phy_data.phy_cur_phy_mode];

radio_time = tx_time - BLE_PHY_T_TXENFAST;
nrf_timer_cc_set(NRF_TIMER0, 0, radio_time);
NRF_TIMER0->EVENTS_COMPARE[0] = 0;
timer0_safely_reset(0);
phy_ppi_timer0_compare0_to_radio_txen_enable();
nrf_timer_cc_set(NRF_TIMER0, 0, radio_time);

#if PHY_USE_FEM_PA
nrf_timer_cc_set(NRF_TIMER0, 2, fem_time);
NRF_TIMER0->EVENTS_COMPARE[2] = 0;
timer0_safely_reset(2);
phy_fem_enable_pa();
nrf_timer_cc_set(NRF_TIMER0, 2, fem_time);
#endif

/* Need to check if TIMER0 did not already count past CC[0] and/or CC[2], so
Expand All @@ -1308,11 +1331,9 @@ ble_phy_rx_end_isr(void)
*
* Note: CC[3] is used only for wfr which we do not need here.
*/
nrf_timer_task_trigger(NRF_TIMER0, NRF_TIMER_TASK_CAPTURE3);
is_late = (NRF_TIMER0->CC[3] > radio_time) && !NRF_TIMER0->EVENTS_COMPARE[0];
is_late = timer0_did_miss(0, 3 /* scratch */);
#if PHY_USE_FEM_PA
is_late = is_late ||
((NRF_TIMER0->CC[3] > fem_time) && !NRF_TIMER0->EVENTS_COMPARE[2]);
is_late = is_late || timer0_did_miss(2, 3 /* scratch */);
#endif
if (is_late) {
phy_ppi_timer0_compare0_to_radio_txen_disable();
Expand Down
Loading