Skip to content

Conversation

@bugadani
Copy link
Contributor

@bugadani bugadani commented Jul 21, 2023

Thank you!

Thank you for your contribution.
Please make sure that your submission includes the following:

Must

  • The code compiles without errors or warnings.
  • All examples work.
  • cargo fmt was run.
  • Your changes were added to the CHANGELOG.md in the proper section.
  • You updated existing examples or added examples (if applicable).
  • Added examples are checked in CI

Nice to have

  • You add a description of your work to this PR.
  • You added proper docs for your newly added features and code.

This PR fixes an issue where write_interrupt_status_clear was always called with 0 as its argument, because the processing loop cleared the status bits. We also clear the bits by subtracting them instead of creating a mask first to and with - hopefully this optimizes better, too.

@bugadani
Copy link
Contributor Author

cc @MabezDev because of the write_interrupt_status_clear thing.

@bugadani bugadani changed the title Split up gpio handling into two loops GPIO interrupt handling tweaks Jul 21, 2023
@MabezDev
Copy link
Member

MabezDev commented Jul 22, 2023

Oh wow, I'm actually dumb 🤦. How did I not think about the previous loop clearing all the bits in intrs 🙈. Thanks for pointing that out!

Regarding the split, could we do that in a separate PR? I'd like to see the performance difference once the cztz/czlz codegen improvements have been merged.

@bugadani
Copy link
Contributor Author

Regarding the split, could we do that in a separate PR?

Sure thing, we can do whatever :) Any opinions about the bit clearing change?

@MabezDev
Copy link
Member

Regarding the split, could we do that in a separate PR?

Sure thing, we can do whatever :) Any opinions about the bit clearing change?

Looks good to me :)

@bugadani bugadani force-pushed the ctlz branch 2 times, most recently from ee4320d to d4f3215 Compare July 22, 2023 16:36
@bugadani bugadani changed the title GPIO interrupt handling tweaks Fix GPIO interrupt status not being cleared Jul 22, 2023
@bugadani bugadani marked this pull request as ready for review July 22, 2023 16:40
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!

@anriha could you see if this resolves #657?

@anriha
Copy link

anriha commented Jul 22, 2023

@MabezDev Looks like it works with this.

Spoke too soon. It still doesn't work properly.

@MabezDev MabezDev merged commit b012624 into esp-rs:main Jul 22, 2023
@bugadani bugadani deleted the ctlz branch July 22, 2023 19:04
bugadani added a commit to bugadani/esp-hal that referenced this pull request Jul 24, 2023
* Fix clearing interrupt status

* Use simpler way to clear bits

* Add note about xtensa

* Add to changelog
playfulFence pushed a commit to playfulFence/esp-hal that referenced this pull request Sep 26, 2023
* Fix clearing interrupt status

* Use simpler way to clear bits

* Add note about xtensa

* Add to changelog
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.

3 participants