[Deepin-Kernel-SIG] [linux 6.6-y] [Deepin] PCI: Add ARCH_PHYTIUM check for phytium root and switch ports#1550
Conversation
deepin inclusion category: bugfix To prevent affect other platform or use a runtime check, use ARCH_PHYTIUM to limit the quirk in our arm64 platform kernel. Fixes: 66d2694 ("PCI: Add ACS quirk for phytium root and switch ports") Signed-off-by: Wentao Guan <guanwentao@uniontech.com>
Reviewer's guide (collapsed on small PRs)Reviewer's GuideScopes existing PCI ACS quirks for PLX and Cadence devices to Phytium architectures only, avoiding unintended impact on non-Phytium platforms while preserving the Phytium-specific behavior. Class diagram for pci_dev_acs_enabled entries and quirk functionclassDiagram
class pci_dev_acs_enabled {
int vendor_id
int device_id
void (*quirk_fn)(struct pci_dev *dev)
}
class pci_quirk_xgene_acs {
+void pci_quirk_xgene_acs(struct pci_dev *dev)
}
class PCI_VENDOR_ID_PLX {
<<constant>>
+int value
}
class PCI_VENDOR_ID_CDNS {
<<constant>>
+int value
}
class PCI_VENDOR_ID_PHYTIUM {
<<constant>>
+int value
}
pci_dev_acs_enabled "*" --> "1" pci_quirk_xgene_acs : uses_quirk_fn
pci_dev_acs_enabled --> PCI_VENDOR_ID_PLX : matches_vendor
pci_dev_acs_enabled --> PCI_VENDOR_ID_CDNS : matches_vendor
pci_dev_acs_enabled --> PCI_VENDOR_ID_PHYTIUM : matches_vendor
class CONFIG_ARCH_PHYTIUM {
<<config_option>>
+bool enabled
}
CONFIG_ARCH_PHYTIUM --> pci_dev_acs_enabled : controls_presence_of_PLX_and_CDNS_entries
Flow diagram for PCI ACS quirk usage under ARCH_PHYTIUMflowchart TD
A[Kernel_config] --> B{CONFIG_ARCH_PHYTIUM}
B -- enabled --> C[Compile Phytium_specific_ACS_quirks]
B -- disabled --> D[Exclude Phytium_specific_ACS_quirks]
C --> E[acs_dev_table]
E --> F[Entry: vendor_id=PCI_VENDOR_ID_PLX\n device_id=PCI_ANY_ID\n quirk_fn=pci_quirk_xgene_acs]
E --> G[Entry: vendor_id=PCI_VENDOR_ID_CDNS\n device_id=PCI_ANY_ID\n quirk_fn=pci_quirk_xgene_acs]
E --> H[Entry: vendor_id=PCI_VENDOR_ID_PHYTIUM\n device_id=PCI_ANY_ID\n quirk_fn=pci_quirk_xgene_acs]
D --> I[acs_dev_table]
I --> J[Entry: vendor_id=PCI_VENDOR_ID_PHYTIUM\n device_id=PCI_ANY_ID\n quirk_fn=pci_quirk_xgene_acs]
K[PCI_enumeration_runtime] --> L[Check acs_dev_table for matching vendor/device]
L --> M[Apply pci_quirk_xgene_acs when matching entry found]
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 |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- Instead of using CONFIG_ARCH_PHYTIUM directly in generic PCI quirk code, consider introducing a dedicated Kconfig option (e.g., CONFIG_PCI_PHYTIUM_ACS_QUIRK) selected by ARCH_PHYTIUM so the quirk remains decoupled from a specific architecture symbol and easier to control or reuse.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Instead of using CONFIG_ARCH_PHYTIUM directly in generic PCI quirk code, consider introducing a dedicated Kconfig option (e.g., CONFIG_PCI_PHYTIUM_ACS_QUIRK) selected by ARCH_PHYTIUM so the quirk remains decoupled from a specific architecture symbol and easier to control or reuse.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Pull request overview
Guards overly broad PCI ACS quirk entries for PLX and Cadence vendor IDs behind CONFIG_ARCH_PHYTIUM to prevent them from affecting non-Phytium platforms, while keeping the Phytium-specific vendor ID entry unconditionally active.
Changes:
- Wrapped PLX and CDNS ACS quirk entries in
#ifdef CONFIG_ARCH_PHYTIUMto limit their scope to Phytium platform kernels.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| { PCI_VENDOR_ID_MUCSE, 0x1061, pci_quirk_mf_endpoint_acs }, | ||
| { PCI_VENDOR_ID_MUCSE, 0x1c61, pci_quirk_mf_endpoint_acs }, | ||
| /* Phytium Technology */ | ||
| #ifdef CONFIG_ARCH_PHYTIUM |
There was a problem hiding this comment.
According to the original patch submission, the incompatibility here is caused by Phytium bridge chips (for example, the x100).
Is simply isolating it with CONFIG_ARCH_PHYTIUM sufficient to cover all cases?
For example, bridge chips like the x100 might also be used on RISC‑V devices, in which case this workaround would not take effect.
There was a problem hiding this comment.
According to the original patch submission, the incompatibility here is caused by Phytium bridge chips (for example, the x100).
Is simply isolating it with CONFIG_ARCH_PHYTIUM sufficient to cover all cases?
For example, bridge chips like the x100 might also be used on RISC‑V devices, in which case this workaround would not take effect.
这个是故意的 因为原始quirk依赖SMMU，如果其他架构需要应该重新添加
https://lore.kernel.org/all/20170720161905.24e300eb@ul30vt.home/
https://lore.kernel.org/all/1519183167-17716-1-git-send-email-fkan@apm.com/
https://lore.kernel.org/all/1501267843-8095-1-git-send-email-fkan@apm.com/
There was a problem hiding this comment.
这个是故意的 因为原始quirk依赖SMMU，如果其他架构需要应该重新添加 https://lore.kernel.org/all/20170720161905.24e300eb@ul30vt.home/ https://lore.kernel.org/all/1519183167-17716-1-git-send-email-fkan@apm.com/ https://lore.kernel.org/all/1501267843-8095-1-git-send-email-fkan@apm.com/
Understood.
Please add the relevant explanation to the commit message — that will save future readers from asking the same question and make tracing the change easier.
| #ifdef CONFIG_ARCH_PHYTIUM | ||
| { PCI_VENDOR_ID_PLX, PCI_ANY_ID, pci_quirk_xgene_acs }, | ||
| { PCI_VENDOR_ID_CDNS, PCI_ANY_ID, pci_quirk_xgene_acs }, | ||
| #endif |
There was a problem hiding this comment.
#endif /* CONFIG_ARCH_PHYTIUM */
deepin inclusion
category: bugfix
To prevent affect other platform or use a runtime check, use ARCH_PHYTIUM to limit the quirk in our arm64 platform kernel.
Fixes: 66d2694 ("PCI: Add ACS quirk for phytium root and switch ports")
Summary by Sourcery
Bug Fixes: