nuttxpr commented on PR #15329: URL: https://github.com/apache/nuttx/pull/15329#issuecomment-2561102695
[**\[Experimental Bot, please feedback here\]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13552&type=issues) __Fill In The Commit Message:__ This PR contains a Commit with an Empty Commit Message. Please fill in the Commit Message with the PR Summary. The PR summary is okay, but could be more specific about *why* reinitializing the peripheral was problematic and how making prefetch configurable addresses that. For example, "Reinitializing the peripheral during driver operation caused X (problem). Making prefetch configurable allows developers to prevent conflicting AHB transfers during data transfer, resolving the issue." The impact section is severely lacking. While the changes might be behind a Kconfig variable, the impact needs to be described *if that variable is enabled*. Each "NO" should be justified or changed to "YES" with a description. Just saying "minimal" isn't helpful. For example: * **Impact on user:** YES. Users who enable the new Kconfig option will need to configure the prefetch setting appropriately to avoid performance issues or errors. * **Impact on build:** Potentially YES if enabling the Kconfig option changes compile-time dependencies. Specify if any new dependencies are introduced. * **Impact on hardware:** YES. Affects IMXRT106X devices with FlexSPI2, fixing the clock configuration. Also impacts any hardware using the driver with the new Kconfig option enabled, as it changes how the FlexSPI peripheral is managed. * **Impact on documentation:** YES. Documentation needs to be updated to explain the new Kconfig option and its effects, including how to configure the prefetch setting. * **Impact on security:** Possibly YES/NO. Consider if the changes to peripheral initialization or prefetch have any security implications. If not, justify why. * **Impact on compatibility:** Potentially YES. Does this change break compatibility with any existing drivers or applications? If not, explain why. The testing section is wholly insufficient. "MIMXRT1064 based platform implementing LittleFS on the same flash where XIP occurs" describes the *test setup*, not the *tests themselves*. You need to provide actual test results and logs. What were you testing? What were the expected outcomes? Did the changes produce the expected outcomes? Show *specific* commands used and the output before and after the change. Examples: **Before Change:** ``` # Command to reproduce the issue ls /mnt/lfs # Output showing the issue ls: /mnt/lfs: Input/output error ``` **After Change:** ``` # Same command after the change ls /mnt/lfs # Output showing the issue is resolved file1.txt file2.txt ``` **In short, this PR does *not* yet meet the NuttX requirements as described.** It needs significantly more detail in the Impact and Testing sections. The Summary could also benefit from being more explicit about the problems solved and the mechanisms of the solution. -- 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
