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

Panic when DefaultHandler gets invoked #1005

Merged
merged 4 commits into from
Dec 11, 2023

Conversation

bjoernQ
Copy link
Contributor

@bjoernQ bjoernQ commented Dec 6, 2023

The DefaultHandler can never do something much more useful than panicking

Fixes #1001

Easy check: Use the gpio_interrupt.rs example and remove the interrupt handler. Pressing the boot button will now panic instead of silently do nothing anymore

@MabezDev
Copy link
Member

MabezDev commented Dec 6, 2023

What's the difference between EspDefaultHandler and DefaultHandler? Shouldn't we get rid of DefaultHandler and push everything through EspDefaultHandler or am I missing something?

@bjoernQ
Copy link
Contributor Author

bjoernQ commented Dec 6, 2023

Yeah EspDefaultHandler is what I wanted to use in the first place but it didn't work. Now it does

@MabezDev
Copy link
Member

MabezDev commented Dec 6, 2023

I think I got it wrong, I think DefaultHandler is for CPU level interrupts i.e 0-31 on either RISCV or Xtensa, and EspDefaultHandler is the default handler for when a #[interrupt] handler is missing. So I think we need both, but I have no idea why DefaultHandler was Xtensa only before 🤔.

@bjoernQ
Copy link
Contributor Author

bjoernQ commented Dec 6, 2023

It's even a bit more complicated because DefaultHandler is used in the device.x in PACs - I don't know if we can change that.
Now with this we would get something very ugly for unhandled CPU level interrupts... but at least not nothing

@bjoernQ
Copy link
Contributor Author

bjoernQ commented Dec 6, 2023

But I think we could only end up there from a CPU level interrupt on Xtensa for something > level 3 ... which would be a problem on its own. For RISC-V we should always end up in start_trap_rust_hal

So maybe it's not that bad

Copy link
Member

@MabezDev MabezDev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I actually just used this PR to find out which interrupt was putting me in the DefaultHandler :D. LGTM! Just needs a rebase I think :)

@bjoernQ bjoernQ added this pull request to the merge queue Dec 11, 2023
Merged via the queue into esp-rs:main with commit dfad09d Dec 11, 2023
17 checks passed
@bjoernQ bjoernQ deleted the panic-in-default-handler branch December 11, 2023 08:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

RV: Hitting the default interrupt handler silently hangs
2 participants