Re: [PR] nimble/phy/nrf5x: fix race conditions in TIMER0 setup [mynewt-nimble]

["andrzej-kaczmarek (via GitHub)" <gi...@apache.org>]

andrzej-kaczmarek commented on code in PR #2036:
URL: https://github.com/apache/mynewt-nimble/pull/2036#discussion_r2107002321


##########
nimble/drivers/nrf5x/src/ble_phy.c:
##########
@@ -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 timer)
+{
+    nrf_timer_task_trigger(NRF_TIMER0, nrf_timer_capture_task_get(timer));
+    NRF_TIMER0->EVENTS_COMPARE[timer] = 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 timer, int scratch)

Review Comment:
   int cc, int scratch_cc



##########
nimble/drivers/nrf5x/src/ble_phy.c:
##########
@@ -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 timer)

Review Comment:
   int cc



##########
nimble/drivers/nrf5x/src/ble_phy.c:
##########
@@ -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 timer)
+{
+    nrf_timer_task_trigger(NRF_TIMER0, nrf_timer_capture_task_get(timer));
+    NRF_TIMER0->EVENTS_COMPARE[timer] = 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 timer, int scratch)
+{
+    nrf_timer_task_trigger(NRF_TIMER0, nrf_timer_capture_task_get(scratch));
+    uint32_t ticks_rem = NRF_TIMER0->CC[timer] - NRF_TIMER0->CC[scratch];
+    return (ticks_rem >= 0x80000000) && !NRF_TIMER0->EVENTS_COMPARE[timer];

Review Comment:
   you can simply compare `NRF_TIMER0->CC[timer] < NRF_TIMER0->CC[scratch]`
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@mynewt.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org