Skip to content

Conversation

@kilograham
Copy link
Contributor

found this dangling while cleaning up github branchs

@kilograham kilograham added this to the 2.1.2 milestone Feb 19, 2025
*
* This method removes such a callback, and enables the "default" callback for the specified GPIOs.
*
* \note You should always use the same gpio_mask as you used when you added the raw IRQ handler.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wondered why this was the case, since it seems like you should be able to do:

gpio_add_raw_irq_handler_masked((1<<0) | (1<<1) | (1<< 2), my_handler);
gpio_remove_raw_irq_handler_masked((1<<1), my_handler);

and then expect to have my_handler still running for GPIOs 0 and 2. But looking at the code for gpio_remove_raw_irq_handler_masked I see that it just does a blunt:

irq_remove_handler(IO_IRQ_BANK0, handler);

rather than only removing the handler when all of its masked pins have been removed (IYSWIM).
Would supporting this more refined approach (which I guess would require storing a mapping from handlers to masks) be too slow or use up too much memory?

And just out of curiosity, how does

void gpio_remove_raw_irq_handler_masked(uint32_t gpio_mask, irq_handler_t handler) {
    assert(raw_irq_mask[get_core_num()] & gpio_mask); // should not remove handlers that are not added
    irq_remove_handler(IO_IRQ_BANK0, handler);
    raw_irq_mask[get_core_num()] &= ~gpio_mask;
}

re-enable the "default" callback for each of the pins in the mask? Similarly, I can't see how

void gpio_add_raw_irq_handler_with_order_priority_masked(uint32_t gpio_mask, irq_handler_t handler, uint8_t order_priority) {
    hard_assert(!(raw_irq_mask[get_core_num()] & gpio_mask)); // should not add multiple handlers for the same event
    raw_irq_mask[get_core_num()] |= gpio_mask;
    irq_add_shared_handler(IO_IRQ_BANK0, handler, order_priority);
}

void gpio_add_raw_irq_handler_masked(uint32_t gpio_mask, irq_handler_t handler) {
    gpio_add_raw_irq_handler_with_order_priority_masked(gpio_mask, handler, GPIO_RAW_IRQ_HANDLER_DEFAULT_ORDER_PRIORITY);
}

disables the "default" callback for the masked pins? 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. it does track them
  2. the raw_irq_mask array determines this

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2. the raw_irq_mask array determines this

Oh, silly me. I hadn't spotted that gpio_default_irq_handler also checks against raw_irq_mask 🤦

@kilograham kilograham merged commit a995b97 into develop Mar 18, 2025
4 checks passed
@kilograham kilograham deleted the gpio_irq_improvements branch March 18, 2025 18:41
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

Successfully merging this pull request may close these issues.

3 participants