Skip to content
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

Pci ep framework #13553

Merged
merged 5 commits into from
Sep 23, 2024
Merged

Pci ep framework #13553

merged 5 commits into from
Sep 23, 2024

Conversation

lipengfei28
Copy link
Contributor

Summary

nuttx add pci ep framework

Impact

Testing

@github-actions github-actions bot added the Size: XL The size of the change in this PR is very large. Consider breaking down the PR into smaller pieces. label Sep 20, 2024
@nuttxpr
Copy link

nuttxpr commented Sep 20, 2024

[Experimental Bot, please feedback here]

Squash The Commits: This PR contains 6 Commits. Please Squash the Multiple Commits into a Single Commit.

No, this PR does not meet the NuttX requirements.

Summary:

  • Insufficient information: The summary is too brief and lacks essential details. It doesn't explain:
    • Necessity: Why is the PCI EP framework needed? What problem does it solve, or what improvement does it bring?
    • Functionality: Which part of the codebase is affected? What specific PCI EP capabilities are being added?
    • Implementation: How does the framework work? What changes have been made to the code?
  • Missing references: Are there related NuttX issues or NuttX Apps issues/pull requests?

Impact:

  • Entirely missing: The impact section is empty. You must address all the points to demonstrate a thorough understanding of your changes.

Testing:

  • No details provided:
    • You need to list the specific build hosts (OS, CPU, compiler) and target devices (architecture, board, configuration) where you tested the changes.
    • Provide actual testing logs from before and after the change to show the difference in behavior.

To improve this PR:

  1. Expand the Summary: Provide a clear and detailed explanation of the changes, their purpose, and how they are implemented.
  2. Complete the Impact section: Analyze and describe the impact of your changes on all the listed aspects (user, build, hardware, documentation, security, compatibility).
  3. Provide Testing Details: Specify your build hosts and target devices, and include relevant testing logs from before and after applying your changes.

By addressing these points, your PR will be far more likely to be reviewed and merged.

@lipengfei28 lipengfei28 force-pushed the pci_ep_framework branch 2 times, most recently from 7c39116 to 7d96fc5 Compare September 20, 2024 09:55
Copy link
Contributor

@jerpelea jerpelea left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please fill commit messages that explain the change and the reason why it is needed

lipengfei28 and others added 5 commits September 23, 2024 10:36
This used for add pci ep drver framework

Signed-off-by: lipengfei28 <[email protected]>
The pci ep framework use bitmap manage free bar

Signed-off-by: lipengfei28 <[email protected]>
and move common math funtions to math32.h:
div_round_up
div_round_closest
is_power_of_2
roundup_pow_of_two
rounddown_pow_of_two

Signed-off-by: lipengfei28 <[email protected]>
The pci ep framewoek use bitmap manage the pci outbond memory

Signed-off-by: lipengfei28 <[email protected]>
Signed-off-by: yangshuyong <[email protected]>
Signed-off-by: lipengfei28 <[email protected]>
@lipengfei28
Copy link
Contributor Author

@jerpelea I add the comment and explain the change

@jerpelea jerpelea merged commit a537ecd into apache:master Sep 23, 2024
29 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Graphics Area: OS Components OS Components issues Area: PCI Size: XL The size of the change in this PR is very large. Consider breaking down the PR into smaller pieces.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants