nuttxpr commented on PR #14902: URL: https://github.com/apache/nuttx/pull/14902#issuecomment-2493030078
[**\[Experimental Bot, please feedback here\]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13552&type=issues) This PR does **not** fully meet the NuttX requirements yet. While it provides a basic outline of the changes, it lacks the crucial details requested in each section. Here's a breakdown of what's missing: * **Summary:** The summary is a placeholder. It needs to explain *why* fetching the FSPI header and FCB generation script is necessary. What problem does this solve? It also needs to describe *how* `mkimage` is used to generate the NOR bootable image and the purpose of compiling the bootloader with `-mstrict-align`. * **Impact:** All impact sections are placeholders. This section requires specific answers (YES/NO) and descriptions for each impact area. Even if the answer is NO, briefly explain why (e.g., "Impact on user: NO - This change is internal to the build process and does not affect user functionality."). * **Testing:** The testing section needs actual build hosts and target information, along with before/after logs. Simply stating that the changes are verified is insufficient. Provide specific details about your test setup and the results you observed. Include commands used, any output, and clear evidence that the change works as intended. **Example of how to improve the PR:** ``` ## Summary This change addresses the issue where [link to NuttX issue if applicable] the bootloader was unable to correctly boot from NOR flash due to inconsistencies in the FSPI header and FCB. This PR automates the process of fetching the correct FSPI header and FCB generation script, and then uses `mkimage` to create a valid NOR bootable image. The `-mstrict-align` flag is used during bootloader compilation to ensure data alignment compatibility with the NOR flash. ## Impact * Is new feature added? NO * Is existing feature changed? YES - The bootloader build process is modified to include automated header/FCB retrieval and image generation. * Impact on user: YES - Users will now be able to reliably boot from NOR flash. Previously, they may have experienced boot failures due to the aforementioned inconsistencies. * Impact on build: YES - The build process now includes steps to fetch the FSPI header, generate the FCB, and use `mkimage`. This will require users to have `mkimage` in their build environment. * Impact on hardware: NO - This change only affects the software build process and does not directly impact hardware. * Impact on documentation: YES - The bootloader build instructions need to be updated to reflect the new process. [Link to updated documentation or indicate where it will be updated]. * Impact on security: NO - This change does not have any security implications. * Impact on compatibility: NO - This change maintains backward compatibility. * Anything else to consider: N/A ## Testing I confirm that changes are verified on local setup and works as intended: * Build Host(s): Linux (Ubuntu 20.04), x86_64, GCC 9.4.0 * Target(s): sim:qemu-rv32imac Testing logs before change: ``` [Previous build logs showing boot failure] Booting... [Boot failure message] ``` Testing logs after change: ``` [New build logs showing successful image generation] Booting... [Boot success message] Login: ``` ``` By providing concrete details, the reviewers can more easily understand the change, assess its impact, and verify its correctness, leading to a smoother and faster review process. Remember to fill in the bracketed placeholders with 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
