From 30459b28fae5ce2747a6a7af58edb6c9cd700cf2 Mon Sep 17 00:00:00 2001 From: Andrzej Kaczmarek Date: Fri, 20 Apr 2018 13:14:57 +0200 Subject: [PATCH 1/2] nimble/phy: Fix starting late RX Since c33701ae we can properly receive packets event if scanning was started on late RX. However, the problem is that radio is started manually there so phy_start_cputime is not correctly and, what's even worse, TIMER0 is not running thus we cannot properly calculate RX start time when PDU is received. This does not cause any major problem with legacy adveritising (we may just miss sending CONNECT_REQ or SCAN_REQ once) but if we schedule scan for AUX in extended advertising, the result of invalid calculations is stored in scheduler item. In particular, due to TIMER0 being stopped, our usecs reference time for EVENT_ADDRESS is 0 and calculation of an actual PDU start time wraps it around so the scan is scheduled far far in the future, which in turn quickly leads to exhaustion of mempool for auxscan items and we basically stop extended scanning (until any of incorrectly scheduled items executes). To fix this, on late RX we just setup RTC0 and TIMER0 to start as soon as possible taking limitations of both timer into account (see comments in code). --- nimble/drivers/nrf52/src/ble_phy.c | 68 ++++++++++++++++++++++++------ 1 file changed, 56 insertions(+), 12 deletions(-) diff --git a/nimble/drivers/nrf52/src/ble_phy.c b/nimble/drivers/nrf52/src/ble_phy.c index 39a98128a..b3bdca883 100644 --- a/nimble/drivers/nrf52/src/ble_phy.c +++ b/nimble/drivers/nrf52/src/ble_phy.c @@ -554,6 +554,49 @@ ble_phy_set_start_time(uint32_t cputime, uint8_t rem_usecs, bool tx) return 0; } +static int +ble_phy_set_start_now(void) +{ + uint32_t cntr; + + /* Read current RTC0 state */ + cntr = NRF_RTC0->COUNTER; + + /* + * Set TIMER0 to fire immediately. We can't set CC to 0 as compare will not + * occur in such case. + */ + NRF_TIMER0->TASKS_CLEAR = 1; + NRF_TIMER0->CC[0] = 1; + NRF_TIMER0->EVENTS_COMPARE[0] = 0; + + /* + * Set RTC compare to start TIMER0. We need to set it to at least N+2 ticks + * from current value to guarantee triggering compare event, but let's set + * it to N+3 to account for possible extra tick on RTC0 during these + * operations. + */ + NRF_RTC0->EVENTS_COMPARE[0] = 0; + NRF_RTC0->CC[0] = cntr + 3; + NRF_RTC0->EVTENSET = RTC_EVTENSET_COMPARE0_Msk; + + /* Enable PPI */ + NRF_PPI->CHENSET = PPI_CHEN_CH31_Msk; + + /* + * Store the cputime at which we set the RTC + * + * XXX Compare event may be triggered on previous CC value (if it was set to + * less than N+2) so in rare cases actual start time may be 2 ticks earlier + * than what we expect. Since this is only used on RX, it may cause AUX scan + * to be scheduled 1 or 2 ticks too late so we'll miss it - it's acceptable + * for now. + */ + g_ble_phy_data.phy_start_cputime = cntr + 3; + + return 0; +} + /** * Function is used to set PPI so that we can time out waiting for a reception * to occur. This happens for two reasons: we have sent a packet and we are @@ -1393,10 +1436,8 @@ ble_phy_rx(void) /* Setup for rx */ ble_phy_rx_xcvr_setup(); - /* Start the receive task in the radio if not automatically going to rx */ - if ((NRF_PPI->CHEN & PPI_CHEN_CH21_Msk) == 0) { - NRF_RADIO->TASKS_RXEN = 1; - } + /* PPI to start radio automatically shall be set here */ + assert(NRF_PPI->CHEN & PPI_CHEN_CH21_Msk); ble_ll_log(BLE_LL_LOG_ID_PHY_RX, g_ble_phy_data.phy_encrypted, 0, 0); @@ -1519,17 +1560,15 @@ ble_phy_rx_set_start_time(uint32_t cputime, uint8_t rem_usecs) if (ble_phy_set_start_time(cputime, rem_usecs, false) != 0) { STATS_INC(ble_phy_stats, rx_late); - /* - * Disable PPI so ble_phy_rx() will start RX immediately after - * configuring receiver. - */ - NRF_PPI->CHENCLR = PPI_CHEN_CH21_Msk; + /* We're late so let's just try to start RX as soon as possible */ + ble_phy_set_start_now(); + late = true; - } else { - /* Enable PPI to automatically start RXEN */ - NRF_PPI->CHENSET = PPI_CHEN_CH21_Msk; } + /* Enable PPI to automatically start RXEN */ + NRF_PPI->CHENSET = PPI_CHEN_CH21_Msk; + /* Start rx */ rc = ble_phy_rx(); @@ -1816,6 +1855,11 @@ void ble_phy_restart_rx(void) { ble_phy_disable_irq_and_ppi(); + + ble_phy_set_start_now(); + /* Enable PPI to automatically start RXEN */ + NRF_PPI->CHENSET = PPI_CHEN_CH21_Msk; + ble_phy_rx(); } From 060be7fb6d85cd3a3e444e456e21e8a589b5d65c Mon Sep 17 00:00:00 2001 From: Andrzej Kaczmarek Date: Fri, 20 Apr 2018 14:10:45 +0200 Subject: [PATCH 2/2] nimble/phy: Fix PDU start time calculation Not sure why compiler did not catch uninitialized 'ticks' here, but we should adjust value read from 'phy_start_cputime' instead. --- nimble/drivers/nrf52/src/ble_phy.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/nimble/drivers/nrf52/src/ble_phy.c b/nimble/drivers/nrf52/src/ble_phy.c index b3bdca883..b9756b575 100644 --- a/nimble/drivers/nrf52/src/ble_phy.c +++ b/nimble/drivers/nrf52/src/ble_phy.c @@ -1060,11 +1060,13 @@ ble_phy_rx_start_isr(void) * it is possible that actual transmission started before TIMER0 was * running - need to take this into account. */ + ble_hdr->beg_cputime = g_ble_phy_data.phy_start_cputime; + usecs = NRF_TIMER0->CC[1]; pdu_usecs = ble_phy_mode_pdu_start_off(ble_hdr->rxinfo.phy_mode) + g_ble_phy_t_rxaddrdelay[ble_hdr->rxinfo.phy_mode]; if (usecs < pdu_usecs) { - ticks--; + g_ble_phy_data.phy_start_cputime--; usecs += 30; } usecs -= pdu_usecs; @@ -1076,7 +1078,7 @@ ble_phy_rx_start_isr(void) ++ticks; } - ble_hdr->beg_cputime = g_ble_phy_data.phy_start_cputime + ticks; + ble_hdr->beg_cputime += ticks; ble_hdr->rem_usecs = usecs; /* XXX: I wonder if we always have the 1st byte. If we need to wait for