Skip to content
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.

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

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

Open
benwillcocks opened this issue Jul 6, 2021 · 1 comment

Comments

@benwillcocks
Copy link

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]) callbacksPioApin;
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
Copy link
Author

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);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant