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

IRQ Improvements #18

Draft
wants to merge 6 commits into
base: master
Choose a base branch
from
Draft

Conversation

1Revenger1
Copy link

@1Revenger1 1Revenger1 commented Nov 22, 2024

I'm not entirely sure what to name this PR honestly, since it's all a bunch of small changes. I was helping out @theroadw with a touchpad that keeps the IRQ line asserted and wrote these changes in an attempt to figure help troubleshoot it.

Notes on each of the commits:

  • Remove unused isCommunityActive
  • Only set IRQ type on resume/register
    • IOInterruptEventSource calls disableInterrupt/enableInterrupt automatically on a level interrupt, so setting the pin type was being called every time a touchpad received an interrupt.
    • VoodooGPIO should save off all the pad configuration registers on sleep so I don't believe this needs to be called on wakeup.
  • Always clear interrupt status on IRQ
    • I think 99% of I2C devices uses level interrupts so this was never an issue, but the linux driver does always clear this bit. Datasheets also suggest that this isn't specific to level interrupts.
  • Mux pin into GPIO controller logic
    • Copied from the linux implementation, makes sure that the pin wasn't routed to another block of hardware.
  • Fix pmode being removed twice from pad config
    • I meant to split this into two commits, as it hides a pretty big change to how interrupts are handled.
    • This leaves interrupts always enabled so that the IOInterruptEventSource disabling an interrupt temporarily doesn't prevent other devices from receiving IRQs. Disabling still masks the interrupt so I think this should be fine.
  • Return interrupt type for GPIO pin rather than overall GPIO controller
    • IOInterruptEventSource does test the interrupt type to check if it should be disabling/enabling interrupts. Was a pretty quick thing to add but since everything uses level interrupts, the behavior doesn't really change in the end.

@1Revenger1 1Revenger1 changed the title Improvements IRQ Improvements Nov 25, 2024
Copy link
Collaborator

@kprinssu kprinssu left a comment

Choose a reason for hiding this comment

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

Thanks for cleaning up some of the tech debt. This LGTM.

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.

2 participants