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

Simplify PCI driver configuration #14822

Merged
merged 4 commits into from
Nov 17, 2024
Merged

Conversation

xiaoxiang781216
Copy link
Contributor

Note: Please adhere to Contributing Guidelines.

Summary

  • pci_ivshmem: return back to polling mode when interrupt mode invalid
  • pci e1000 net delete produce config
  • pci: Remove igc produt config
  • serial/pci: Remove the product specific config

Impact

Kconfig

Testing

ci

Improve the pci ivshmem device compatibility, do not return error when
irq mode init failed, fallback to the polling mode as mush as possible.

Signed-off-by: Bowen Wang <[email protected]>
@github-actions github-actions bot added Area: Networking Effects networking subsystem Area: Drivers Drivers issues Area: PCI Board: x86_64 Size: S The size of the change in this PR is small labels Nov 16, 2024
@nuttxpr
Copy link

nuttxpr commented Nov 16, 2024

[Experimental Bot, please feedback here]

No, this PR does not fully meet the NuttX requirements as described. While it provides a summary of the changes, it lacks crucial details and context. Here's a breakdown of the missing information:

  • Summary: While it lists the changes made, it doesn't explain why these changes are necessary. What problem are they solving? What's the motivation behind reverting to polling mode, removing product configs, etc.? How exactly do these changes work? What is the mechanism by which the code reverts to polling? This section needs significant expansion. Issue/PR references are also missing.

  • Impact: Simply stating "Kconfig" is insufficient. It needs to explicitly answer the questions about user impact, build impact, hardware impact, documentation impact, security impact, and compatibility impact. For example, if the Kconfig is changed, does this mean the user needs to reconfigure their build? Are any boards no longer supported? Does removing product-specific configs simplify the build process? Be explicit with "YES" or "NO" for each impact item and then provide a description if the answer is "YES."

  • Testing: "ci" is not adequate testing information. The PR needs to specify the build host OS, CPU architecture, and compiler used. It also needs to specify the target architecture and board configuration used for testing. Critically, it's missing the "before" and "after" testing logs that demonstrate the change's effectiveness. Just saying "ci" doesn't provide any verifiable evidence of the changes working as intended. The CI results themselves should be linked in the PR, not just mentioned.

In short, the PR needs to be significantly more detailed and explicit in all sections to meet the NuttX requirements. It needs to provide the "why," "how," and verifiable "proof" for each change.

yezhonghui2024 and others added 3 commits November 17, 2024 00:20
to simplify the configuration since the driver could detect the device correctly

Signed-off-by: Xiang Xiao <[email protected]>
since the driver could detect them automatically

Signed-off-by: Xiang Xiao <[email protected]>
@raiden00pl raiden00pl merged commit 2c9d412 into apache:master Nov 17, 2024
27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Drivers Drivers issues Area: Networking Effects networking subsystem Area: PCI Board: x86_64 Size: S The size of the change in this PR is small
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants