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

Make set_interrupt panic free #2760

Open
bjoernQ opened this issue Dec 13, 2024 · 6 comments
Open

Make set_interrupt panic free #2760

bjoernQ opened this issue Dec 13, 2024 · 6 comments
Labels
peripheral:interrupt Interrupt peripheral(s)

Comments

@bjoernQ
Copy link
Contributor

bjoernQ commented Dec 13, 2024

set_interrupt implementations can panic because Priority contains the None variant.
IIRC we use it internally for disabled interrupts so we cannot just remove it (also we transmute there - need to be cautious about that!)

One way would be to have an internal / HW priority and a user-facing priority - the later wouldn't include None (other solutions are also possible)

@bugadani
Copy link
Contributor

The Xtensa interrupt_level_to_cpu_interrupt also returns an error if you want to set an edge-triggered interrupt at Priority2. Not sure what we can do about that, and this may be relevant for ESP32 and S2.

@bugadani
Copy link
Contributor

Also, exposing Priority::None is interesting in combination with #2684 - although I'm not sure if doing that makes any more sense than using a RefCell in this particular case.

@tom-borcin tom-borcin added the investigation Not needed for 1.0, but don't know if we can support without breaking the driver. label Dec 16, 2024
@jessebraham
Copy link
Member

Why is this a 1.0 blocker if we do not plan to stabilize interrupts?

@MabezDev
Copy link
Member

MabezDev commented Jan 8, 2025

Whilst we aren't stabilizing the interrupt module, drivers in blocking mode should be able to set a interrupt handler. As it turns out, they can't right now: #2905. Once that's fixed, it becomes a bit more clear why this is a 1.0 blocker.

This has also made me realize we do need to stabilize some things in the interrupt module, namely Priority and InterruptHandler.

@bugadani
Copy link
Contributor

bugadani commented Jan 8, 2025

This has also made me realize we do need to stabilize some things in the interrupt module, namely Priority and InterruptHandler.

What about the #[handler] macro? While we don't need it stable, it's probably good enough in its current form to be stable, unless you have concerns?

@MabezDev
Copy link
Member

As per #2900 (comment) we're not planning to stabilize interrupts, therefore I'm removing the 1.0-blocker label

@MabezDev MabezDev added peripheral:interrupt Interrupt peripheral(s) and removed 1.0-blocker investigation Not needed for 1.0, but don't know if we can support without breaking the driver. labels Jan 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
peripheral:interrupt Interrupt peripheral(s)
Projects
Status: Todo
Development

No branches or pull requests

5 participants