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

Wifi: switch tasks at interrupt level 1 #3164

Merged
merged 8 commits into from
Feb 28, 2025
Merged

Conversation

bugadani
Copy link
Contributor

@bugadani bugadani commented Feb 21, 2025

Closes #3160

Previously, we used both Software1 and the timer interrupt to switch tasks. Software1 runs at priority level 3, while the timer ran at level 2. Trying to run the timer at level 1 meant esp-wifi crashed because the Interrupt Option and the High Priority interrupt option work a bit differently.

The root cause of the issue is that level 1 interrupt handling restores PS directly from the stack, while higher interrupt levels restore it into EPSn, and then that gets moved into PS by the rfi instruction.

This PR fixes this problem by using Software0, which is a level 1 priority interrupt.

@bugadani bugadani added the skip-changelog No changelog modification needed label Feb 21, 2025
@bugadani bugadani changed the title Wifi: always switch tasks at interrupt level 3 Wifi: switch tasks at interrupt level 1 Feb 21, 2025
@MabezDev
Copy link
Member

I think I'd prefer this fix, but based on the description of #3165, this fix doesn't work for esp32?

@bugadani
Copy link
Contributor Author

Yes, the ESP32 doesn't have a free software interrupt at priority level 1, and other CPU interrupts like Timer can't be triggered from software.

@bugadani
Copy link
Contributor Author

I guess we can pick this PR for chips except ESP32, and the other one for ESP32?

@MabezDev
Copy link
Member

I think that makes sense. It's a bit annoying to have a esp32 workaround, but I don't see the point in making all the chips suffer the perf hit of the other work around. What do you think @bjoernQ?

@bugadani bugadani added skip-changelog No changelog modification needed and removed skip-changelog No changelog modification needed labels Feb 28, 2025
@bjoernQ
Copy link
Contributor

bjoernQ commented Feb 28, 2025

I think that makes sense. It's a bit annoying to have a esp32 workaround, but I don't see the point in making all the chips suffer the perf hit of the other work around. What do you think @bjoernQ?

Good idea (and yes having different code for ESP32 is very annoying but it's not the first time ESP32 needs something special)! Would be a shame to let the S3 suffer from this

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.

LGTM, thanks!

Copy link
Contributor

@bjoernQ bjoernQ left a comment

Choose a reason for hiding this comment

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

Thanks

@bugadani bugadani added this pull request to the merge queue Feb 28, 2025
Merged via the queue into esp-rs:main with commit fc2a656 Feb 28, 2025
27 of 28 checks passed
@bugadani bugadani deleted the level1 branch February 28, 2025 11:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
skip-changelog No changelog modification needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

esp-wifi: Scheduler not working at priority level 1 on Xtensa
3 participants