doc: add Doxygen comments to PCI driver subsystem#11484
Open
GorrayLi wants to merge 3 commits into
Open
Conversation
Signed-off-by: ligr <liguorui1213@163.com>
Signed-off-by: ligr <liguorui1213@163.com>
|
👋 感谢您对 RT-Thread 的贡献！Thank you for your contribution to RT-Thread! 为确保代码符合 RT-Thread 的编码规范，请在你的仓库中执行以下步骤运行代码格式化工作流（如果格式化CI运行失败）。 🛠 操作步骤 | Steps
完成后，提交将自动更新至 如有问题欢迎联系我们，再次感谢您的贡献！💐 |
📌 Code Review Assignment🏷️ Tag: componentsReviewers: @Maihuanyi Changed Files (Click to expand)
📊 Current Review Status (Last Updated: 2026-06-15 08:48 CST)
📝 Review Instructions
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR aims to improve maintainability of RT-Thread’s PCI subsystem by adding Doxygen documentation blocks across the PCI core, host/ECAM/DW drivers, MSI/MSI-X, procfs, and endpoint framework.
Changes:
- Add Doxygen
@file/API comments across PCI core, host, MSI/MSI-X, procfs, and endpoint code. - Minor code formatting adjustments (indentation/brace style) mixed into multiple PCI files.
- Includes at least one non-doc functional change (so the PR is not doc-only as titled/described).
Notes (review checklist related / 审查清单相关提示):
- PR title format: repository guideline prefers lowercase prefix like
[module][subsystem] ...; current title uses"doc: ..."rather than[doc].../[components][drivers].... - PR description “BSP / .config / action” fields are left empty.
Reviewed changes
Copilot reviewed 26 out of 26 changed files in this pull request and generated 19 comments.
Show a summary per file
| File | Description |
|---|---|
| components/drivers/pci/procfs.h | Adds procfs interface Doxygen comments. |
| components/drivers/pci/procfs.c | Adds procfs implementation Doxygen comments and some formatting changes. |
| components/drivers/pci/probe.c | Adds detailed PCI enumeration/probing Doxygen comments. |
| components/drivers/pci/pme.c | Adds PME capability Doxygen comments and replaces legacy block comment. |
| components/drivers/pci/pci.c | Adds core PCI API Doxygen comments; includes some formatting/readability changes. |
| components/drivers/pci/ofw.c | Adds device-tree integration Doxygen comments and adjusts log alignment. |
| components/drivers/pci/msi/msi.c | Adds MSI/MSI-X core Doxygen comments and helper doc. |
| components/drivers/pci/msi/irq.c | Adds file/function Doxygen comments; includes a logic change in a touched hunk. |
| components/drivers/pci/msi/device.c | Adds MSI/MSI-X capability init Doxygen comments. |
| components/drivers/pci/irq.c | Adds INTx routing/assignment Doxygen comments. |
| components/drivers/pci/host/pci-host-generic.c | Adds generic host driver Doxygen comments; changes initializer formatting. |
| components/drivers/pci/host/pci-host-common.c | Adds common probe/remove Doxygen comments. |
| components/drivers/pci/host/dw/pcie-dw.c | Adds DesignWare core library Doxygen comments and reformats some control flow. |
| components/drivers/pci/host/dw/pcie-dw_platfrom.c | Adds DesignWare platform driver Doxygen comments. |
| components/drivers/pci/host/dw/pcie-dw_host.c | Adds RC-mode Doxygen comments and some formatting changes. |
| components/drivers/pci/host/dw/pcie-dw_ep.c | Adds EP-mode Doxygen comments and reformats ops table. |
| components/drivers/pci/host-bridge.c | Adds host-bridge driver Doxygen comments and restructures comments/initializers. |
| components/drivers/pci/endpoint/mem.c | Adds endpoint memory allocator Doxygen comments; minor formatting. |
| components/drivers/pci/endpoint/endpoint.c | Adds endpoint framework Doxygen comments across public APIs. |
| components/drivers/pci/ecam.h | Adds ECAM definition Doxygen comments and restructures macro docs. |
| components/drivers/pci/ecam.c | Adds ECAM implementation Doxygen comments; changes initializer brace style. |
| components/drivers/pci/access.c | Adds config access layer Doxygen comments, including macro documentation blocks. |
| components/drivers/include/drivers/pci_msi.h | Adds MSI/MSI-X structure and API Doxygen comments. |
| components/drivers/include/drivers/pci_endpoint.h | Adds comprehensive endpoint public API Doxygen documentation. |
Comments suppressed due to low confidence (2)
components/drivers/pci/procfs.c:290
- [spelling/拼写]:
pci_lseek()parameter name and Doxygen tag usewherece, which is a typo forwhence.
English: Using the conventional whence name (and matching documentation) improves readability and avoids confusion with lseek(2) semantics.
中文：建议将参数名统一为常见的 whence（并同步更新注释），避免与 lseek 语义产生混淆。
* @param[in] wherece SEEK_SET, SEEK_CUR, or SEEK_END (config space size)
* @return New position, or negative error
*/
static off_t pci_lseek(struct dfs_file *file, off_t offset, int wherece)
{
struct proc_dentry *dentry = file->vnode->data;
struct rt_pci_device *pdev = dentry->data;
switch (wherece)
components/drivers/pci/ecam.c:107
- [codebase_conventions/代码库规范]: This initializer switches to K&R-style braces (
= {) which conflicts with RT-Thread's documented Allman brace style.
English: documentation/7.contribution/coding_style_en.md recommends braces on their own lines and suggests formatting with astyle --style=allman. Please keep braces on separate lines for these initializers (including nested initializers).
中文：RT-Thread 编码规范建议使用 Allman 风格（大括号独占一行，并推荐 astyle --style=allman）。这里的初始化（包括嵌套初始化）建议保持大括号换行，以保持代码风格一致。
/** @brief Standard ECAM operations using direct MMIO read/write */
const struct pci_ecam_ops pci_generic_ecam_ops = {
.pci_ops = {
.map = pci_ecam_map,
.read = rt_pci_bus_read_config_uxx,
.write = rt_pci_bus_write_config_uxx,
}
};
Comment on lines
+15
to
+16
| * Provides /proc/pci/<BDF> per-device configuration space files | ||
| * and /proc/pci/devices enumeration for userspace access. |
Comment on lines
+29
to
+40
| /** | ||
| * @brief Create /proc/pci/<BDF> entry for a PCI device | ||
| * | ||
| * @param[in] pdev PCI device to attach | ||
| */ | ||
| void pci_procfs_attach(struct rt_pci_device *pdev); | ||
|
|
||
| /** | ||
| * @brief Remove /proc/pci/<BDF> entry for a PCI device | ||
| * | ||
| * @param[in] pdev PCI device to detach | ||
| */ |
Comment on lines
+43
to
+56
| /** | ||
| * @brief No-op: create /proc/pci/<BDF> entry (procfs disabled) | ||
| * | ||
| * @param[in] pdev PCI device | ||
| */ | ||
| rt_inline void pci_procfs_attach(struct rt_pci_device *pdev) | ||
| { | ||
| } | ||
|
|
||
| /** | ||
| * @brief No-op: remove /proc/pci/<BDF> entry (procfs disabled) | ||
| * | ||
| * @param[in] pdev PCI device | ||
| */ |
Comment on lines
+16
to
+18
| * - /proc/pci/devices: A list of all PCI devices with their BARs and drivers | ||
| * - /proc/pci/<BDF>: Per-device config space read/write (like Linux's sysfs config) | ||
| * |
Comment on lines
+98
to
+102
| * @brief Read PCI config space from userspace (/proc/pci/<BDF>) | ||
| * | ||
| * Handles arbitrary offsets and sizes with proper alignment. | ||
| * Data is converted to little-endian for userspace consumption. | ||
| * |
Comment on lines
625
to
+626
| LOG_E("%s: MSI pic MUST implemented %s", | ||
| rt_ofw_node_full_name(msi_ic_np), "irq_free_msi"); | ||
| rt_ofw_node_full_name(msi_ic_np), "irq_free_msi"); |
Comment on lines
864
to
865
| LOG_E("Invail num-lanes = %d", pci->num_lanes); | ||
| return; |
Comment on lines
81
to
86
| if (last_irq >= 0 && last_irq + 1 != irq) | ||
| { | ||
| for (int idx = 0; idx < i; ++i, --last_irq) | ||
| for (int idx = 0; idx < i; ++idx, --last_irq) | ||
| { | ||
| rt_bitmap_set_bit(msi_irq_map, last_irq); | ||
| } |
Comment on lines
+32
to
40
| /** @brief CAM-mode ECAM ops with 16-bit bus shift for legacy PCI */ | ||
| static const struct pci_ecam_ops gen_pci_cfg_cam_bus_ops = { | ||
| .bus_shift = 16, | ||
| .pci_ops = | ||
| { | ||
| .pci_ops = { | ||
| .map = pci_ecam_map, | ||
| .read = rt_pci_bus_read_config_uxx, | ||
| .write = rt_pci_bus_write_config_uxx, | ||
| } | ||
| }; |
Comment on lines
+1340
to
1347
| /** @brief PCI bus type descriptor */ | ||
| static struct rt_bus pci_bus = { | ||
| .name = "pci", | ||
| .match = pci_match, | ||
| .probe = pci_probe, | ||
| .remove = pci_remove, | ||
| .shutdown = pci_shutdown, | ||
| }; |
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.
拉取/合并请求描述：(PR description)
[
为什么提交这份PR (why to submit this PR)
doc: add Doxygen comments to PCI driver subsystem
你的解决方案是什么 (what is your solution)
doc: add Doxygen comments to PCI driver subsystem
请提供验证的bsp和config (provide the config and bsp)
BSP:
.config:
action:
]
当前拉取/合并请求的状态 Intent for your PR
必须选择一项 Choose one (Mandatory):
代码质量 Code Quality：
我在这个拉取/合并请求中已经考虑了 As part of this pull request, I've considered the following:
#if 0代码，不包含已经被注释了的代码 All redundant code is removed and cleaned up