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

async gpio fixes #537

Merged
merged 2 commits into from
May 15, 2023
Merged

async gpio fixes #537

merged 2 commits into from
May 15, 2023

Conversation

MabezDev
Copy link
Member

@MabezDev MabezDev commented May 15, 2023

  • Fix pin number calculation for bank1
  • Clear interrupt status after disabling interrupt to avoid hardware pending another interrupt
  • Clear interrupt on InputFuture creation.

Close #535

@bugadani
Copy link
Contributor

No more misfires, thanks!

@bugadani
Copy link
Contributor

bugadani commented May 15, 2023

One thing, though, regarding the implementation: what do you think about clearing status after the loop, instead of every iteration? It's unlikely that multiple GPIOs interrupt at the same time, but it's still probably enough to clear the status registers once at the end - we have the bit patterns to do this.

@MabezDev MabezDev force-pushed the async/gpio-fixups branch 2 times, most recently from 28ac7a4 to 39c373b Compare May 15, 2023 12:59
Copy link
Member

@jessebraham jessebraham left a comment

Choose a reason for hiding this comment

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

LGTM

@MabezDev MabezDev marked this pull request as draft May 15, 2023 13:21
@MabezDev
Copy link
Member Author

Don't merge yet, sorry I've realised this might not be correct.

- Fix pin number calculation for bank1
- Clear interrupt status after disabling interrupt to avoid hardware
  pending another interrupt
- Clear interrupt status per pin when we create the input future
@MabezDev MabezDev requested a review from jessebraham May 15, 2023 13:45
@MabezDev MabezDev marked this pull request as ready for review May 15, 2023 13:45
@MabezDev
Copy link
Member Author

I had a last-second wobble as @bugadani pointed out a possible logical error. However, in practice the hardware (at least on C3/S3) will clear the gpio interrupt status bit when int_ena is cleared. For the sake of an extra write (or two) I've added back the explicit clear just in case future chips (or older chips) don't work this way.

@MabezDev MabezDev merged commit eb3a449 into esp-rs:main May 15, 2023
SergioGasquez pushed a commit to SergioGasquez/esp-hal that referenced this pull request Jun 9, 2023
* async gpio fixes

- Fix pin number calculation for bank1
- Clear interrupt status after disabling interrupt to avoid hardware
  pending another interrupt
- Clear interrupt status per pin when we create the input future

* add changelog item
i404788 pushed a commit to i404788/esp-hal that referenced this pull request Jul 22, 2023
* async gpio fixes

- Fix pin number calculation for bank1
- Clear interrupt status after disabling interrupt to avoid hardware
  pending another interrupt
- Clear interrupt status per pin when we create the input future

* add changelog item
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.

Level-triggered GPIO interrupts are firing multiple times
3 participants