nuttxpr commented on PR #15831: URL: https://github.com/apache/nuttx/pull/15831#issuecomment-2656199614
[**\[Experimental Bot, please feedback here\]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13552&type=issues) The PR summary is good, but lacks some details. It states *what* the change fixes, but not precisely *how* it fixes it. For example, does it add a new configuration option? Modify existing driver code? A little more detail here would be helpful. The Impact section is too brief. While it mentions IMX95 only, it should explicitly answer all the questions with YES/NO and provide descriptions where necessary. Even if the answer is NO, stating so explicitly clarifies things for the reviewer. The Testing section is insufficient. It needs to provide details about the build host and target as requested. Critically, it's missing the **testing logs** before and after the change. These logs are crucial for verifying the fix and understanding its impact. **Here's how the PR information could be improved:** ## Summary This fixes crashes caused by receiving interrupts from unused/unassigned GPIO banks on the i.MX95. The i.MX95 allows mapping GPIO banks between the A55, M7, and M33 cores. This change introduces a new Kconfig option, `CONFIG_IMX95_GPIO_BANK_MASK`, to specify which GPIO banks the M7 core is allowed to access. The GPIO driver is modified to only handle interrupts from the configured banks, preventing spurious interrupts and crashes. Fixes #[NuttX Issue Number (if applicable)] ## Impact * Is new feature added? NO * Is existing feature changed? YES. The GPIO driver now respects a configuration option to mask out interrupts from unused GPIO banks. * Impact on user (will user need to adapt to change)? YES. Users working with the i.MX95 will need to configure the `CONFIG_IMX95_GPIO_BANK_MASK` option to specify the allowed GPIO banks for the M7. * Impact on build (will build process change)? NO * Impact on hardware (will arch(s) / board(s) / driver(s) change)? YES. This change affects the i.MX95 architecture and the GPIO driver. * Impact on documentation (is update required / provided)? YES. The documentation should be updated to describe the new `CONFIG_IMX95_GPIO_BANK_MASK` configuration option and its usage. * Impact on security (any sort of implications)? NO * Impact on compatibility (backward/forward/interoperability)? Potentially YES. Existing configurations that implicitly relied on all GPIO banks being accessible might need to be adjusted. * Anything else to consider? None. ## Testing I confirm that changes are verified on local setup and works as intended: * Build Host(s): Linux (Ubuntu 22.04), x86_64, GCC 11.3.0 * Target(s): i.MX95 (IMX95-EVK board), `nsh` configuration Testing logs before change: ``` [before change logs showing the crash/issue] e.g., nsh> gpio_read 100 // Accessing unconfigured GPIO ... system crash log ... ``` Testing logs after change: ``` [after change logs showing the correct behavior] e.g., nsh> make menuconfig (set CONFIG_IMX95_GPIO_BANK_MASK to allow access to GPIO 100) nsh> gpio_read 100 // Accessing configured GPIO 1 ``` -- 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]
