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

Provide a higher level interrupt mechanism #80

Closed
MabezDev opened this issue Jun 10, 2022 · 10 comments
Closed

Provide a higher level interrupt mechanism #80

MabezDev opened this issue Jun 10, 2022 · 10 comments
Assignees

Comments

@MabezDev
Copy link
Member

The current interrupt implementation is quite currently low level. The PAC's we generate also generate a table of Peripheral interrupts with handlers held in static storage. We should be able to provide a higher level of abstraction such that we could declare an interrupt for a peripheral with a simple proc-macro. For example

#[interrupt] // delcare the following function as a interrupt handler for GPIO
fn GPIO() {
  // stuff
}

Xtensa

I have a good idea of how to handle this in Xtensa, as it's something we did in the original esp32-hal. The idea is that internally the hal will claim all interrupt levels from xtensa_lx_rt (here) at which point any interrupt goes to a single handler where the status is checked. From there it's easy to get the interrupt number and call the corresponding peripheral handler.

RISCV

I am less sure of the best approach on RISCV, something similar to the above could work. Perhaps we reserve 15 CPU interrupts, one for each of the priority levels and do the same approach. This then leaves the other 17 interrupts available for direct use which will result in slightly lower interrupt latency.

I am more than happy to take a stab at implementing this but I would appreciate some feedback on the design.

@MabezDev MabezDev self-assigned this Jun 10, 2022
@bjoernQ
Copy link
Contributor

bjoernQ commented Jun 11, 2022

Originally, I thought of something like this as a separate crate - in any case I'd love to have this opt-in or opt-out since e.g. for esp-wifi I'm not sure if we can do it this way ... but maybe we can ... anyway, would be good to have a choice.

Additionally (not now probably) it would be nice to have some convenience to pass resources into the handlers - not as invasive as RTIC in best case. But haven't thought that through.

Other than that sounds like a good idea

@MabezDev
Copy link
Member Author

Originally, I thought of something like this as a separate crate - in any case I'd love to have this opt-in or opt-out since e.g. for esp-wifi I'm not sure if we can do it this way ... but maybe we can ... anyway, would be good to have a choice.

Because there a predetermined interrupts for WiFi, bluetooth etc? Provided they set a flag in the interrupt status I think we should be okay, as we can add WIFI & BLUETOOTH handlers in our PAC crates, then esp-wifi can use them.

Even if that doesn't work, for Xtensa, there are also predetermined interrupts for CPU timers etc, so we will need to solve this issue anyway. It should be simple enough to have a per chip interrupt mask that will give us the available "free" cpu interrupts for the user to use.

@MabezDev
Copy link
Member Author

By the way, if you enable all interrupts on Xtensa, does your board hang (presumably in the default interrupt handler for some interrupt)? I'd like to track down what these interrupts are, and if we can stop them.

@bjoernQ
Copy link
Contributor

bjoernQ commented Jun 11, 2022

Yes it hangs for me, too. I think it's the Xtensa timer interrupts which fire

@MabezDev
Copy link
Member Author

I think you're right, from my testing I believe Interrupt6Timer0Priority1, Interrupt15Timer1Priority3 & Interrupt16Timer2Priority5 are always pending. This is odd, because it should be being disabled at boot: https://github.com/esp-rs/xtensa-lx-rt/blob/455d2b77b8a5724d36db0ea66c88467a4515f376/src/lib.rs#L72

@MabezDev
Copy link
Member Author

Ah! reset_timers function needs be updated, it is only resetting the timers on the esp32...

@bjoernQ
Copy link
Contributor

bjoernQ commented Jul 4, 2022

One thing that came to my mind ... not sure if it's a good idea or a terrible one:

What if we make the user's interrupt handlers actual callbacks?

Then users could just take (mutable) reference into a closure without all the static frills.

I played around with that in the ESP32-C3 timer example and it looks quite promising to me. But maybe I overlooked something

Unfortunately, with no-alloc that works fine for Fn and FnMut but not so good for FnOnce

@MabezDev
Copy link
Member Author

We could do this for sure! I think we'll get the pac interrupt version in first, then we can add a separate feature to use closure callbacks.

Then users could just take (mutable) reference into a closure without all the static frills.

Maybe I'm missing something, but wouldn't it have to be an immutable borrow? With some interior mutability via a mutex, because once the call back is set anything borrowed by the closure will be borrowed that whole time?

@bjoernQ
Copy link
Contributor

bjoernQ commented Jul 22, 2022

Maybe I'm missing something, but wouldn't it have to be an immutable borrow? With some interior mutability via a mutex, because once the call back is set anything borrowed by the closure will be borrowed that whole time?

Yes, you are right if both sides want to access it - only if no access is needed at the call-site anymore it could be mutable - like in the multicore example:

cpu1_task(&mut timer1, &counter);

Moving it would be better but then it would need to be FnOnce

@MabezDev
Copy link
Member Author

I think we can close this now, with #118 & #103 being merged. We can explore a closure-based approach in another issue.

Repository owner moved this from Todo to Done in esp-rs Jul 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

No branches or pull requests

2 participants