Skip to content

Commit 0cb5914

Browse files
committed
Reduce ISR overhead
Reduce the number of cycles spent in the GPIOTE ISR, by reading common volatile registers across AHB only once, and using a safer hack to prevent spurious ISR firing when the last channel's event was cleared.
1 parent 3ac53c5 commit 0cb5914

File tree

1 file changed

+46
-18
lines changed

1 file changed

+46
-18
lines changed

cores/nRF5/WInterrupts.c

Lines changed: 46 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -179,30 +179,58 @@ void GPIOTE_IRQHandler()
179179
SEGGER_SYSVIEW_RecordEnterISR();
180180
#endif
181181

182-
uint32_t event = offsetof(NRF_GPIOTE_Type, EVENTS_IN[0]);
183-
182+
// Read this once (not 8x), as it's a volatile read
183+
// across the AHB, which adds up to 3 cycles.
184+
uint32_t const enabledInterruptMask = NRF_GPIOTE->INTENSET;
184185
for (int ch = 0; ch < NUMBER_OF_GPIO_TE; ch++) {
185-
if ((*(uint32_t *)((uint32_t)NRF_GPIOTE + event) == 0x1UL) && (NRF_GPIOTE->INTENSET & (1 << ch))) {
186-
if (channelMap[ch] != -1 && callbacksInt[ch]) {
187-
if ( callbackDeferred[ch] ) {
188-
// Adafruit defer callback to non-isr if configured so
189-
ada_callback(NULL, 0, callbacksInt[ch]);
190-
}else{
191-
callbacksInt[ch]();
192-
}
186+
// only process where the interrupt is enabled and the event register is set
187+
// check interrupt enabled mask first, as already read that IOM value, to
188+
// reduce delays from AHB (16MHz) reads.
189+
if ( 0 == (enabledInterruptMask & (1 << ch))) continue;
190+
if ( 0 == NRF_GPIOTE->EVENTS_IN[ch]) continue;
191+
192+
// If the event was set and interrupts are enabled,
193+
// call the callback function only if it exists,
194+
// but ALWAYS clear the event to prevent an interrupt storm.
195+
if (channelMap[ch] != -1 && callbacksInt[ch]) {
196+
if ( callbackDeferred[ch] ) {
197+
// Adafruit defer callback to non-isr if configured so
198+
ada_callback(NULL, 0, callbacksInt[ch]);
199+
} else {
200+
callbacksInt[ch]();
193201
}
194-
195-
*(uint32_t *)((uint32_t)NRF_GPIOTE + event) = 0;
196-
#if __CORTEX_M == 0x04
197-
volatile uint32_t dummy = *((volatile uint32_t *)((uint32_t)NRF_GPIOTE + event));
198-
(void)dummy;
199-
#endif
200202
}
201203

202-
event = (uint32_t)((uint32_t)event + 4);
204+
// clear the event
205+
NRF_GPIOTE->EVENTS_IN[ch] = 0;
203206
}
207+
#if __CORTEX_M == 0x04
208+
// To quote from nRF52840 product specification:
209+
// --------------------------------------------------------------------------
210+
// Note: To avoid an interrupt reoccurring before a new event has come,
211+
// the program should perform a read from one of the peripheral
212+
// registers. For example, the event register that has been cleared,
213+
// or the INTENCLR register that has been used to disable the
214+
// interrupt. This will cause a one to three cycle delay and ensure
215+
// the interrupt is cleared before exiting the interrupt handler.
216+
// Care should be taken to ensure the compiler does not remove the read
217+
// operation as an optimization. If the program can guarantee a four-cycle
218+
// delay after event being cleared or interrupt disabled in any other way,
219+
// then a read of a register is not required.
220+
// --------------------------------------------------------------------------
221+
//
222+
// Unfortunately, simply marking a variable as volatile is not enough to
223+
// prevent GCC from reordering or combining memory accesses, without also
224+
// having an explicit sequence point.
225+
// See https://gcc.gnu.org/onlinedocs/gcc/Volatiles.html
226+
//
227+
// Here, ensure a sequence point by adding a memory barrier
228+
// followed with four nops before exiting the ISR
229+
asm volatile ("" : : : "memory");
230+
__asm__ __volatile__ ("nop\n\tnop\n\tnop\n\tnop\n");
231+
#endif
204232

205233
#if CFG_SYSVIEW
206-
SEGGER_SYSVIEW_RecordExitISR();
234+
SEGGER_SYSVIEW_RecordExitISR();
207235
#endif
208236
}

0 commit comments

Comments
 (0)