arduino / ArduinoCore-sam

80 stars 107 forks source link

If interrupt attached to 2 or more pins of a port, detachInterrupt() doesn't work correctly #122

Open benwillcocks opened 3 years ago

benwillcocks commented 3 years ago

Here's the problem code in WInterrupts.c - note PIOB/C/D handlers have the same problem:

void PIOA_Handler(void) { uint32_t isr = PIOA->PIO_ISR; // this line should be: uint32_t isr = PIOA->PIO_ISR & PIOA->PIO_IMR; uint8_t leading_zeros; while((leading_zeros=__CLZ(isr))<32) { uint8_t pin=32-leading_zeros-1; if(callbacksPioA[pin]) callbacksPioA[pin](); isr=isr&(~(1<<pin)); } }

A '1' in PIO_ISR means an edge has been detected on the corresponding pin, regardless of whether the interrupt is enabled by PIO_IMR. If no interrupts are attached, there's obviously no problem as PIOx_Handler doesn't get invoked. As soon as one interrupt is attached, PIOx_Handler will attempt to invoke callbacks for all pins where edges have been detected. I'm guessing callbacksPioA[] is initialised to NULL, so the bug will normally go unnoticed. However, if you attach interrupts to two or more pins on the same port, then use detachInterrupt() to remove one of them, you find that the ISR which should have been detached continues to get invoked!

benwillcocks commented 3 years ago

I also noticed the neat __CLZ() trick used in PIOx_Handler() could be used to improve the efficiency of attachInterrupt(). This code:

uint32_t pos = 0; uint32_t t; for (t = mask; t>1; t>>=1, pos++) ;

could be replaced by:

uint32_t pos = 31 - __CLZ(mask);