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

Missed doc change in #850 #2302

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open
Changes from all commits
Commits
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
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? 🤔

*
* @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
Loading