Skip to content

Missed doc change in #850 #2302

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Mar 18, 2025
Merged

Missed doc change in #850 #2302

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/rp2_common/hardware_gpio/include/hardware/gpio.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -615,6 +615,8 @@ static inline void gpio_add_raw_irq_handler(uint gpio, irq_handler_t handler) {
*
* 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 🤦

*
* @param gpio_mask a bit mask of the GPIO numbers that will now be passed to the default callback for this core
* @param handler the handler to remove from the list of GPIO IRQ handlers for this core
*/
Expand Down