-
Notifications
You must be signed in to change notification settings - Fork 234
Fully revert ACPI SCI Handler to Bookworm Behavior #618
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
Open
croos12
wants to merge
1
commit into
sonic-net:master
Choose a base branch
from
croos12:croos-acpi-sci-followup-reverts-public
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
45 changes: 45 additions & 0 deletions
45
patches-sonic/revert-acpi-ec-use-threaded-handler-for-dedicated-irq.patch
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,45 @@ | ||
| From: Connor Roos <croos@nvidia.com> | ||
| Date: 2026-08-05 | ||
| Subject: Revert "ACPI: EC: Use a threaded handler for dedicated IRQ" | ||
|
|
||
| This reverts commit 655a6e7c0d83d47c36218525708c9fcfdd7f4b43. | ||
|
|
||
| That commit moved the ACPI EC dedicated-IRQ handler to a threaded | ||
| handler, on the grounds that commit 7a36b901a6eb ("ACPI: OSL: Use a | ||
| threaded interrupt handler for SCI") had already made all EC code run in | ||
| thread context on GPE-signalled systems, so the dedicated-IRQ path may as | ||
| well match. | ||
|
|
||
| We revert 7a36b901a6eb in | ||
| revert-acpi-osl-use-threaded-irq-for-sci.patch, restoring hardirq | ||
| context for the SCI. Revert this commit too so the EC dedicated-IRQ | ||
| path goes back to the non-threaded request_irq() handler and the EC | ||
| interrupt handling is consistent with the pre-7a36b901a6eb behaviour. | ||
|
|
||
| This patch and | ||
| revert-acpi-osl-use-spin-locks-without-disabling-interrupts.patch are a | ||
| pair: with the EC dedicated-IRQ handler back in hardirq context, ACPICA | ||
| can once again be entered from a hardirq, so the irqsave/irqrestore | ||
| variants that patch restores are what keep the EC path safe. Do not | ||
| apply one without the other, and keep both ordered after | ||
| revert-acpi-osl-use-threaded-irq-for-sci.patch in the series. | ||
|
|
||
| Signed-off-by: Connor Roos <croos@nvidia.com> | ||
| --- | ||
| drivers/acpi/ec.c | 4 ++-- | ||
| 1 file changed, 2 insertions(+), 2 deletions(-) | ||
|
|
||
| diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c | ||
| --- a/drivers/acpi/ec.c | ||
| +++ b/drivers/acpi/ec.c | ||
| @@ -1506,8 +1506,8 @@ | ||
|
|
||
| static bool install_gpio_irq_event_handler(struct acpi_ec *ec) | ||
| { | ||
| - return request_threaded_irq(ec->irq, NULL, acpi_ec_irq_handler, | ||
| - IRQF_SHARED | IRQF_ONESHOT, "ACPI EC", ec) >= 0; | ||
| + return request_irq(ec->irq, acpi_ec_irq_handler, IRQF_SHARED, | ||
| + "ACPI EC", ec) >= 0; | ||
| } | ||
|
|
||
| /** | ||
59 changes: 59 additions & 0 deletions
59
patches-sonic/revert-acpi-osl-use-spin-locks-without-disabling-interrupts.patch
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,59 @@ | ||
| From: Connor Roos <croos@nvidia.com> | ||
| Date: 2026-08-05 | ||
| Subject: Revert "ACPI: OSL: Use spin locks without disabling interrupts" | ||
|
|
||
| This reverts commit 8e57de43076477c5cce113f2579bef02ce3e8b27. | ||
|
|
||
| That commit dropped the local interrupt disable from the ACPICA spinlock | ||
| primitives, arguing it was no longer needed because commit 7a36b901a6eb | ||
| ("ACPI: OSL: Use a threaded interrupt handler for SCI") had moved all | ||
| ACPICA code out of hardirq context. | ||
|
|
||
| We revert 7a36b901a6eb in | ||
| revert-acpi-osl-use-threaded-irq-for-sci.patch, which puts the SCI | ||
| handler back in hardirq context. With that revert in place the premise | ||
| of 8e57de4307 no longer holds: ACPICA can once again run from a hardirq | ||
| handler, so acpi_os_acquire_lock()/acpi_os_release_lock() must disable | ||
| and restore local interrupts again. Restore the irqsave/irqrestore | ||
| variants. | ||
|
|
||
| The same applies to | ||
| revert-acpi-ec-use-threaded-handler-for-dedicated-irq.patch, which | ||
| puts the EC dedicated-IRQ handler back in hardirq context: this patch is | ||
| what makes that one safe. Do not apply one without the other, and keep | ||
| both ordered after revert-acpi-osl-use-threaded-irq-for-sci.patch in the | ||
| series. | ||
|
|
||
| Signed-off-by: Connor Roos <croos@nvidia.com> | ||
| --- | ||
| drivers/acpi/osl.c | 10 ++++++---- | ||
| 1 file changed, 6 insertions(+), 4 deletions(-) | ||
|
|
||
| diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c | ||
| --- a/drivers/acpi/osl.c | ||
| +++ b/drivers/acpi/osl.c | ||
| @@ -1515,18 +1515,20 @@ | ||
| acpi_cpu_flags acpi_os_acquire_lock(acpi_spinlock lockp) | ||
| __acquires(lockp) | ||
| { | ||
| - spin_lock(lockp); | ||
| - return 0; | ||
| + acpi_cpu_flags flags; | ||
| + | ||
| + spin_lock_irqsave(lockp, flags); | ||
| + return flags; | ||
| } | ||
|
|
||
| /* | ||
| * Release a spinlock. See above. | ||
| */ | ||
|
|
||
| -void acpi_os_release_lock(acpi_spinlock lockp, acpi_cpu_flags not_used) | ||
| +void acpi_os_release_lock(acpi_spinlock lockp, acpi_cpu_flags flags) | ||
| __releases(lockp) | ||
| { | ||
| - spin_unlock(lockp); | ||
| + spin_unlock_irqrestore(lockp, flags); | ||
| } | ||
|
|
||
| #ifndef ACPI_USE_LOCAL_CACHE |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use
git format-patchformatted patches.