[Deepin-Kernel-SIG] [linux 6.6-y] [Upstream] [Alibaba] Backport bugfixes for Root Port PMU for Yitian710

[Avenger-285714]

Fixes: #831

Suggested-by: Wentao Guan guanwentao@uniontech.com

Summary by Sourcery

Backport fixes for the root port PMU driver to correctly identify devices by full SBDF and simplify initialization logic.

Bug Fixes:

• Register and identify root port PMUs by full SBDF (domain:bus:dev:fn) instead of just BDF.
• Use the platform device’s id for PMU naming and error messages to ensure consistency.
• Remove redundant ‘found’ flag and unreachable -ENODEV return to prevent init from failing when no device is found.

Enhancements:

• Rename internal variable from bdf to sbdf for clarity.

[sourcery-ai]

Reviewer's Guide

This backport adjusts the PCI identifier logic to use a 32-bit SBDF (including domain) instead of the legacy 16-bit BDF throughout the driver, and cleans up a dead initialization code path.

Sequence Diagram: PCI Device Registration with SBDF

sequenceDiagram
    participant init as dwc_pcie_pmu_init
    participant reg_dev as dwc_pcie_register_dev
    participant pdev_obj as "pci_dev (data)"
    participant plat_reg as platform_device_register_data

    init->>reg_dev: Call dwc_pcie_register_dev(pdev)
    activate reg_dev
    reg_dev->>pdev_obj: Read bus->number, devfn
    reg_dev->>pdev_obj: Read pci_domain_nr(pdev->bus)
    reg_dev-->>reg_dev: Calculate sbdf = (domain << 16) | PCI_DEVID(bus_num, devfn)
    reg_dev->>plat_reg: platform_device_register_data(NULL, "dwc_pcie_pmu", sbdf, pdev, sizeof(*pdev))
    activate plat_reg
    plat_reg-->>reg_dev: Return plat_dev or ERR_PTR
    deactivate plat_reg
    reg_dev-->>init: Return status
    deactivate reg_dev

Loading

Sequence Diagram: PMU Probe using SBDF

sequenceDiagram
    participant plat_core as Platform Core
    participant pmu_probe as dwc_pcie_pmu_probe
    participant plat_dev_obj as "platform_device (data)"
    participant kasprintf as devm_kasprintf
    participant perf_reg as perf_pmu_register

    plat_core->>pmu_probe: Call dwc_pcie_pmu_probe(plat_dev)
    activate pmu_probe
    pmu_probe->>plat_dev_obj: Get pci_dev from plat_dev->dev.platform_data
    pmu_probe->>plat_dev_obj: Get sbdf from plat_dev->id
    pmu_probe-->>pmu_probe: Use sbdf for operations
    pmu_probe->>kasprintf: devm_kasprintf(&plat_dev->dev, GFP_KERNEL, "dwc_rootport_%x", sbdf)
    activate kasprintf
    kasprintf-->>pmu_probe: Return name
    deactivate kasprintf
    pmu_probe->>perf_reg: perf_pmu_register(&pcie_pmu->pmu, name, -1)
    activate perf_reg
    alt Registration Succeeded
        perf_reg-->>pmu_probe: Return 0
    else Registration Failed
        perf_reg-->>pmu_probe: Return error_code (ret)
        pmu_probe-->>pmu_probe: Log error: pci_err(pdev, "Error %d registering PMU @%x", ret, sbdf)
    end
    deactivate perf_reg
    pmu_probe-->>plat_core: Return status
    deactivate pmu_probe

Loading

Class Diagram: Changes in dwc_pcie_pmu.c Functions

classDiagram
    class `dwc_pcie_register_dev()` {
        <<C Function in dwc_pcie_pmu.c>>
        + Calculates `sbdf` (32-bit Segment/Bus/Device/Function ID)
        + sbdf = (pci_domain_nr(pdev->bus) << 16) | PCI_DEVID(pdev->bus->number, pdev->devfn)
        + Calls `platform_device_register_data()` with `sbdf`
        - Replaces logic using 16-bit `bdf`
    }

    class `dwc_pcie_pmu_probe()` {
        <<C Function in dwc_pcie_pmu.c>>
        + Retrieves `sbdf` from `plat_dev->id`
        + Uses `sbdf` for device naming: `devm_kasprintf(..., "dwc_rootport_%x", sbdf)`
        + Uses `sbdf` in error logging: `pci_err(..., sbdf)`
        - Replaces logic using 16-bit `bdf`
    }

    class `dwc_pcie_pmu_init()` {
        <<C Function in dwc_pcie_pmu.c>>
        - Removed `bool found` variable and related logic (dead code cleanup)
        + Iterates `for_each_pci_dev(pdev)`
        + Calls `dwc_pcie_register_dev(pdev)` in loop
    }

    `dwc_pcie_pmu_init()` --> `dwc_pcie_register_dev()` : calls

Loading

File-Level Changes

Change	Details	Files
Switch root port identifier from 16-bit BDF to 32-bit SBDF	Rename bdf to sbdf Compute sbdf using pci_domain_nr and PCI_DEVID Pass sbdf to platform_device_register_data Use plat_dev->id as sbdf in devm_kasprintf and pci_err messages	drivers/perf/dwc_pcie_pmu.c
Remove unused initialization flag and redundant code path	Delete the 'found' variable declaration Remove assignments to 'found' inside the PCI loop Eliminate final !found check and ENODEV return	drivers/perf/dwc_pcie_pmu.c

---

Tips and commands

Interacting with Sourcery

• Trigger a new review: Comment @sourcery-ai review on the pull request.
• Continue discussions: Reply directly to Sourcery's review comments.
• Generate a GitHub issue from a review comment: Ask Sourcery to create an
  issue from a review comment by replying to it. You can also reply to a
  review comment with @sourcery-ai issue to create an issue from it.
• Generate a pull request title: Write @sourcery-ai anywhere in the pull
  request title to generate a title at any time. You can also comment
  @sourcery-ai title on the pull request to (re-)generate the title at any time.
• Generate a pull request summary: Write @sourcery-ai summary anywhere in
  the pull request body to generate a PR summary at any time exactly where you
  want it. You can also comment @sourcery-ai summary on the pull request to
  (re-)generate the summary at any time.
• Generate reviewer's guide: Comment @sourcery-ai guide on the pull
  request to (re-)generate the reviewer's guide at any time.
• Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
  pull request to resolve all Sourcery comments. Useful if you've already
  addressed all the comments and don't want to see them anymore.
• Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
  request to dismiss all existing Sourcery reviews. Especially useful if you
  want to start fresh with a new review - don't forget to comment
  @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

• Enable or disable review features such as the Sourcery-generated pull request
  summary, the reviewer's guide, and others.
• Change the review language.
• Add, remove or edit custom review instructions.
• Adjust other review settings.

Getting Help

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[deepin-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please ask for approval from avenger-285714. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• deepin/OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[deepin-ci-robot]

deepin pr auto review

代码审查意见：

1. 变量命名：

   • bdf 变量被重命名为 sbdf，但 sbdf 的命名不够直观，建议使用更具描述性的名称，如 bus_dev_fn 或 pci_bus_device_function。

2. 代码重复：

   • 在 dwc_pcie_pmu_probe 函数中，sbdf 被赋值为 plat_dev->id，但在其他地方仍然使用 PCI_DEVID(pdev->bus->number, pdev->devfn)。建议统一使用 sbdf，以减少代码重复。

3. 错误处理：

   • 在 dwc_pcie_pmu_probe 函数中，当 devm_kasprintf 失败时，应该释放之前分配的资源，以避免内存泄漏。可以在返回 -ENOMEM 之前添加资源释放的代码。

4. 代码可读性：

   • 在 dwc_pcie_pmu_init 函数中，found 变量被引入，但在循环结束后没有使用。如果这个变量是多余的，应该移除它。如果它有其他用途，应该添加相应的注释说明其用途。

5. 性能优化：

   • 在 dwc_pcie_pmu_init 函数中，for_each_pci_dev(pdev) 循环中，每次循环都会调用 pci_dev_put(pdev)，这可能会导致性能问题。如果 pdev 在循环结束后不再需要，应该提前释放它。

6. 错误信息：

   • 在 dwc_pcie_pmu_probe 函数中，错误信息中使用了 bdf，但在 dwc_pcie_pmu_init 函数中使用了 sbdf。建议统一错误信息的格式，以便于维护和理解。

7. 代码注释：

   • 在代码中添加适当的注释，特别是在复杂的逻辑和变量命名不直观的地方，以提高代码的可读性和可维护性。

8. 安全性：

   • 检查是否有对 pdev 和 plat_dev 的空指针检查，以防止潜在的空指针解引用问题。

综上所述，代码的改进主要集中在变量命名、代码重复、错误处理、代码可读性、性能优化、错误信息和安全性方面。通过这些改进，可以提高代码的质量和可维护性。

[Copilot]

Pull Request Overview

This PR backports bugfixes for the Root Port PMU to include PCI domain information in the BDF and streamlines device initialization.

• Incorporate PCI domain into the BDF value passed to the platform device.
• Rename internal BDF variable to sbdf and update related registration/print calls.
• Remove the found flag and its -ENODEV check in dwc_pcie_pmu_init.

Comments suppressed due to low confidence (3)

drivers/perf/dwc_pcie_pmu.c:559

• [nitpick] The name sbdf is terse and may be unclear. Consider renaming to something more descriptive like segment_bdf or domain_bdf to clarify that it includes the PCI domain.

u32 sbdf;

drivers/perf/dwc_pcie_pmu.c:561

• This new logic computing a segment-aware BDF should be covered by unit or integration tests (e.g., verifying correct encoding of domain, bus, device, and function) to prevent regressions.

sbdf = (pci_domain_nr(pdev->bus) << 16) | PCI_DEVID(pdev->bus->number, pdev->devfn);

drivers/perf/dwc_pcie_pmu.c:731

• Removing the !found check and its -ENODEV return means the CPHP state will be set up even if no matching PCI root port is found. Consider retaining a guard or adding a fallback to avoid registering hotplug callbacks with no target devices.

bool found = false;

[Copilot]

The shift by 16 is a magic number here. Introduce a named constant or helper macro (e.g., PCI_DOMAIN_SHIFT) to document its purpose and avoid repeated literals.

Suggested change

	sbdf = (pci_domain_nr(pdev->bus) << 16) | PCI_DEVID(pdev->bus->number, pdev->devfn);
	sbdf = (pci_domain_nr(pdev->bus) << PCI_DOMAIN_SHIFT) | PCI_DEVID(pdev->bus->number, pdev->devfn);

[sourcery-ai]

Hey @Avenger-285714 - I've reviewed your changes and they look great!

Here's what I looked at during the review

• 🟢 General issues: all looks good
• 🟢 Security: all looks good
• 🟢 Testing: all looks good
• 🟢 Complexity: all looks good
• 🟢 Documentation: all looks good

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}