nuttxpr commented on PR #15624: URL: https://github.com/apache/nuttx/pull/15624#issuecomment-2603782475
[**\[Experimental Bot, please feedback here\]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13552&type=issues) No, this PR description does not fully meet the NuttX requirements. Here's why and how to fix it: **Missing Information in Summary:** * **Why is this change necessary?** What problem does unifying the boot flow solve? Is it for code clarity, performance improvement, bug fix, or something else? This is the most crucial piece of missing information. * **What functional part of the code is being changed?** Be specific. Mention the files/modules affected (e.g., `arch/arm/src/armv7-a/boot.c`, `arch/arm/src/armv7-r/boot.S`). * **How does the change exactly work?** Provide a more detailed explanation of the mechanism used to unify the boot flow. What was the old flow, and what is the new flow? **Missing Information in Impact:** * **Is new feature added? Is existing feature changed?** This needs a YES/NO answer. It sounds like an existing feature is being changed. * **Impact on user:** This should be NO/YES, followed by a description if YES. Even if the answer is NO, briefly explain why the user won't be affected. * **Impact on build, hardware, documentation, security, compatibility:** All of these require a NO/YES answer and a description if YES. Even if the impact is NO, provide a brief justification (e.g., "Impact on build: NO, no build system changes are required."). **Missing Information in Testing:** * **Build Host(s):** Be specific about your development environment. For example: "Linux (Ubuntu 20.04), x86_64, GCC 9.4.0". * **Target(s):** Provide more detail. Instead of "qemu-armv7a:nsh," specify things like: "QEMU ARMv7-A (Cortex-A15), nsh configuration" or similar. * **Testing logs:** The placeholder logs need to be replaced with *actual* logs demonstrating the functionality before and after the change. Show that the change works and doesn't introduce regressions. Even a simple "nsh>" prompt before and after can be helpful. If the logs are too extensive, consider providing a link to a CI run or a separate log file. **Example of an improved description:** ``` ## Summary This change unifies the boot flow of ARMv7-A and ARMv7-R architectures to improve code maintainability and reduce redundancy. Previously, the ARMv7-A and ARMv7-R had different low-level boot sequences before reaching `nx_start()`. This PR refactors the `arch/arm/src/armv7-a/boot.c` and `arch/arm/src/armv7-r/boot.S` files to use a common C-level boot function that directly calls `nx_start()`, eliminating assembly language differences in the early boot stage. This simplifies the codebase and makes it easier to add support for new ARMv7 variants in the future. Relates to [NuttX Issue #XXX](if applicable). ## Impact * Is new feature added? NO * Is existing feature changed? YES, the low-level boot process for ARMv7-A is changed to match the ARMv7-R approach. * Impact on user: NO, the user experience should be identical. The change is internal to the boot process. * Impact on build: NO, no build system modifications are necessary. * Impact on hardware: YES, this change affects ARMv7-A architectures. ARMv7-R architectures are also affected, but the change aligns them with the new unified approach. No changes are expected for other architectures. * Impact on documentation: NO, no documentation updates are required. * Impact on security: NO, this change does not have any security implications. * Impact on compatibility: NO, this change maintains backward compatibility. No changes to user applications or drivers are expected. * Anything else to consider: None. ## Testing I confirm that changes are verified on local setup and work as intended: * Build Host(s): Linux (Ubuntu 22.04), x86_64, GCC 11.2.0 * Target(s): QEMU ARMv7-A (Cortex-A15), nsh configuration; QEMU ARMv7-A (Cortex-A9) SMP configuration; QEMU ARMv7-A (Cortex-A15) knsh configuration; QEMU ARMv7-R, nsh configuration Testing logs before change (qemu-armv7a:nsh): ``` NuttShell (NSH) nsh> ``` Testing logs after change (qemu-armv7a:nsh): ``` NuttShell (NSH) nsh> ``` (Repeat logs for other tested targets) ``` This improved example provides more context, explains the "why," and includes more specific details about the testing environment and results, making it much more likely to be accepted. Remember to replace the placeholders with your actual information. -- 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: commits-unsubscr...@nuttx.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
