nuttxpr commented on PR #14434: URL: https://github.com/apache/nuttx/pull/14434#issuecomment-2427499627
[**\[Experimental Bot, please feedback here\]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13552&type=issues) The PR summary is missing some key information, and the testing section needs more detail to meet the NuttX requirements fully. Here's a breakdown: **What's Missing/Needs Improvement:** * **Summary:** * **Why is the change necessary?** The summary mentions a data abort but doesn't explain why adding a DDR memory map fixes it. Provide a technical explanation; for instance, is the DDR region not being recognized by the MMU, causing an access violation? * **What functional part is changed?** Be specific. Mention the files modified (e.g., memory map configuration, `arm_addrenv_utils.c`). * **How does the change work?** Explain the mechanism: * How does adding the DDR memory map resolve the data abort? * How is the `memset` issue in `arm_addrenv_utils.c` connected to the missing memory map? * **Impact:** * **Impact on hardware:** State which architectures and boards are affected by this change (specifically those using the DDR region). * **Impact on documentation:** If the DDR region is new, documentation will likely need updates to inform users about its configuration and usage. * **Testing:** * **Build Host(s):** Specify the OS, CPU architecture, and compiler version used for testing. * **Target(s):** "CI test" isn't sufficient. Mention the specific QEMU configurations used (e.g., `qemu-armv7a:knsh`). * **Testing logs:** The provided logs only show the error before the fix. Include logs after the change demonstrating that the data abort no longer occurs. **How to Improve the PR:** 1. **Expand the Summary:** Clearly explain the technical reason for the data abort and how your changes address the root cause. Be specific about the files and code sections modified. 2. **Complete the Impact Section:** Provide details about affected hardware and indicate whether documentation updates are necessary. 3. **Enhance the Testing Section:** Include details about your build host environment, the exact QEMU targets tested, and logs showing successful operation after applying your changes. By providing this additional information, you'll make it much easier for reviewers to understand your changes and assess if the PR meets the NuttX requirements. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
