-
Notifications
You must be signed in to change notification settings - Fork 300
Added API to raise and reset software interrupts #426
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a great start, thank you!
esp-hal-common/src/system.rs
Outdated
@@ -56,7 +63,50 @@ pub enum Peripheral { | |||
#[cfg(esp32c6)] | |||
Twai1, | |||
} | |||
#[cfg(not(any(esp32, esp32c6)))] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Software CPU interrupts are supported on all chips, so I don't think we need the feature gates? It might the case that they are not included in the PACs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think they are in the PACs but maybe named differently
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, i could only find them in system for the chips i included.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At least we use SW_INTR3 in esp-wifi
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's in the INTPRI peripheral in the C6 PAC. I don't have one for testing however, so i don't feel confident implementing it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the esp32 it's in the DPORT, so i guess support there is just a matter of enabling it
Awesome! Thank you! It would be great if you could add a very simple example (at least for the C3 since you cannot verify it on the other chips). Also, implementation for ESP32 and ESP32-C6 will be added by someone else later |
I'd rather include it in this PR honestly, though I won't block merging on that. These things have a tendency of being forgotten. If there are issues with the PAC please let me know exactly what they are and we can get them resolved. |
I tested it on the supported targes (i.e. not ESP32, not ESP32-C6) and it works fine For ESP32 we need the interrupts in the SVD/PAC first I'd suggest we can merge this as is, wait for the SVD/PAC changes (there are already changes planned so it makes sense to do it all together) and then add ESP32 and ESP32-C6 |
I will be reviewing some SVD changes made by a colleague today. I have additionally done some work in aligning the peripheral and interrupt names, but I will verify the ones mentioned by @bjoernQ before publishing new SVDs. I'm hoping to have new PACs published either this afternoon or tomorrow, however there's always a possibility this could slip to next week. |
Sorry, I messed up with the last SVDs and the ESP32's CPU interrupts are still missing in the latest PACs 🙈 I will make sure that they're present in the next release. |
With #447 merged the required interrupts should now be available. |
@onsdagens Do you plan to continue on this or would you like someone to take over (especially since you said you don't have all the hardware to verify it on all targets). No rush, however |
i can give it a go, but as you said i can't test so would appreciate some help with that |
Awesome! Thanks for your effort. If there is something you need to get tested just ping me |
oopsie didn't mean to do that, just finished some other things so i'll be working on getting this up and running next. |
Merged with latest version, added support for all of the MCUs and added some examples. Keep in mind that the examples are not tested for anything but the C3, i know they compile for the others but that's as far as it goes, could use some help with testing @bjoernQ . |
Not sure what the issue is with CI here. |
Problem with CI is that the latest nightlies refuse to compile embedded-hal-async - I will create a PR to pin the nightly compiler version in CI for now (until embedded-hal-async got fixed) which should make CI happy again |
Rebasing should fix the CI problem (hopefully) |
Thanks! Now there seems to be a dependency issue, embassy-executor does not want to build. I'm unsure why it's pinned to that specific revision, so i don't know how to proceed here without breaking something else. |
Seems it was just a temporary connectivity problem - rerunning the failed jobs cured it |
Tested all the examples on all targets! 👍 Works fine! Only thing I don't agree with is the top-level comment in the examples 😆 |
Haha, oops! Fixed the comments now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Oh rustfmt isn't happy, yet |
Sorry, didn't expect comments to be misformatted... live and learn. |
Implemented an API for raising and resetting software interrupts as mentioned in #425 on the ESP32C2, ESP32C3, ESP32S2 and ESP32S3. Builds and works as expected on the C3 at least, not tested on any of the other chips since i don't own them.