PCI/ACPI: Add stacked IRQ domain support to PCI Interrupt Link - #2119
Open
wangchenlu2236 wants to merge 1 commit into
Open
PCI/ACPI: Add stacked IRQ domain support to PCI Interrupt Link#2119wangchenlu2236 wants to merge 1 commit into
wangchenlu2236 wants to merge 1 commit into
Conversation
The ResourceSource field of an Extended Interrupt Descriptor was ignored
when the driver is parsing _PRS method of PNP0C0F PCI Interrupt Link
devices, which means PCI INTx would be always registered under the GSI
domain. This patch introduces stacked IRQ domain support to PCI Interrupt
Link devices for ACPI.
With this support, we can populate the ResourceSource field in _PRS method
of PCI Interrupt Link devices to refer to a device object that describes
an interrupt controller as the following examples:
Device (IXIU) {
...
}
Device(LINKA) {
Name(_HID, EISAID("PNP0C0F"))
Name(_PRS, ResourceTemplate(){
Interrupt(ResourceProducer, Level, ActiveHigh, Exclusive,
0, "\\SB.IXIU") { 60 }
})
...
}
Signed-off-by: Chen Baozi <chenbaozi@phytium.com.cn>
Signed-off-by: Wang Yinfeng <wangyinfeng@phytium.com.cn>
Signed-off-by: Wang Chenlu <wangchenlu2236@phytium.com.cn>
Reviewer's GuideThe PR enables PCI Interrupt Link INTx interrupts to be mapped through the interrupt controller named by an Extended IRQ ResourceSource, while preserving GSI-domain behavior as a compatibility fallback through new fwnode-aware ACPI IRQ registration APIs. Sequence diagram for PCI INTx ResourceSource IRQ mappingsequenceDiagram
participant PCI as PCI device
participant PIRQ as ACPI PCI IRQ
participant PciLink as PCI Interrupt Link
participant ACPI as ACPI ResourceSource
participant IRQ as IRQ domain
PCI->>PIRQ: acpi_pci_irq_enable()
PIRQ->>PciLink: acpi_pci_link_allocate_irq()
PciLink->>ACPI: acpi_get_irq_source_fwhandle(resource_source, 0)
ACPI-->>PciLink: interrupt-controller fwnode
PciLink-->>PIRQ: GSI, trigger, polarity, rs_fwnode
PIRQ->>IRQ: acpi_register_irq(dev, GSI, trigger, polarity, rs_fwnode)
IRQ-->>PCI: Linux IRQ mapping
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Hi @wangchenlu2236. Thanks for your PR. I'm waiting for a deepin-community member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
🟡 Changes recommended
IRQ-domain selection, failure handling, teardown, and resource ownership contain correctness issues.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds hierarchical ACPI IRQ-domain support for PCI Interrupt Link devices.
Changes:
- Preserves
_PRSinterrupt resource sources. - Registers PCI INTx through referenced IRQ domains.
- Refactors GSI registration around explicit firmware nodes.
File summaries
| File | Description |
|---|---|
include/linux/acpi.h |
Declares resource-source lookup. |
include/acpi/acpi_drivers.h |
Extends link allocation API. |
drivers/acpi/pci_link.c |
Preserves and resolves resource sources. |
drivers/acpi/pci_irq.c |
Uses domain-aware IRQ registration. |
drivers/acpi/irq.c |
Adds explicit-fwnode registration. |
drivers/acpi/internal.h |
Declares the internal registration helper. |
Review details
Suppressed comments (2)
drivers/acpi/pci_irq.c:457
- This can create the mapping in the
ResourceSourcedomain, butacpi_pci_irq_disable()still tears it down withacpi_unregister_gsi(gsi), which searches the GSI domain. A child-domain hwirq is not necessarily a GSI, so disable can leave the new mapping allocated or dispose a different GSI mapping; retain enough mapping information to dispose the IRQ in the same domain in which it was created.
rc = acpi_register_irq(&dev->dev, gsi, triggering, polarity, rs_fwnode);
drivers/acpi/pci_link.c:151
- The duplicated source string is owned by
link, but both the attach-error path andacpi_pci_link_remove()free onlylink, leaking this allocation. Also handle allocation failure before leaving a nonzerostring_lengthpaired with a NULL pointer; add matching cleanup on every link destruction path or use an appropriate managed allocation.
rs->string_ptr = kstrdup(p->resource_source.string_ptr,
GFP_KERNEL);
- Files reviewed: 6/6 changed files
- Comments generated: 4
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| fwspec.param[1] = acpi_dev_get_irq_type(trigger, polarity); | ||
| fwspec.param_count = 2; | ||
|
|
||
| return irq_create_fwspec_mapping(&fwspec); |
| } | ||
|
|
||
| rc = acpi_register_gsi(&dev->dev, gsi, triggering, polarity); | ||
| rc = acpi_register_irq(&dev->dev, gsi, triggering, polarity, rs_fwnode); |
Comment on lines
+147
to
+151
| if (p->resource_source.string_length) { | ||
| rs->index = p->resource_source.index; | ||
| rs->string_length = p->resource_source.string_length; | ||
| rs->string_ptr = kstrdup(p->resource_source.string_ptr, | ||
| GFP_KERNEL); |
| * are only for LoongArch. Therefore, passing any value makes no differences for us. | ||
| * Need some reconstruction for this function to meet the real semantic. | ||
| */ | ||
| *rs_fwnode = acpi_get_irq_source_fwhandle(&link->irq.resource_source, 0); |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
The ResourceSource field of an Extended Interrupt Descriptor was ignored when the driver is parsing _PRS method of PNP0C0F PCI Interrupt Link devices, which means PCI INTx would be always registered under the GSI domain. This patch introduces stacked IRQ domain support to PCI Interrupt Link devices for ACPI.
With this support, we can populate the ResourceSource field in _PRS method of PCI Interrupt Link devices to refer to a device object that describes an interrupt controller as the following examples:
Device (IXIU) {
...
}
Device(LINKA) {
Name(_HID, EISAID("PNP0C0F"))
Name(_PRS, ResourceTemplate(){
Interrupt(ResourceProducer, Level, ActiveHigh, Exclusive,
0, "\SB.IXIU") { 60 }
})
...
}
Summary by Sourcery
Enable PCI Interrupt Link devices to resolve and register PCI INTx interrupts through hierarchical ACPI IRQ domains.
New Features:
Bug Fixes:
Enhancements: