nuttxpr commented on PR #15870: URL: https://github.com/apache/nuttx/pull/15870#issuecomment-2667860548
[**\[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:** * **Summary:** * **Why is the change *necessary*?** You stated the problem, but not the root cause. Explain *why* switching pin mode and adding clock gating fixes the issue. What's the underlying problem that these changes address? * **What functional part of the code is being changed?** Be specific. Mention the filenames and functions modified (e.g., `i2c_initialize()` in `arch/arm/src/spresense/spresense_i2c.c`). * **NuttX Issue/PR references:** Are there any related issues or PRs in the NuttX or NuttX Apps repositories? If not, consider creating one to track the original bug. * **Impact:** * **Is this a bug fix or a new feature?** It seems like a bug fix. Clarify this. * **Impact on user:** Even though it's Spresense-specific, how will this change affect users of the Spresense I2C? Will their existing code work without modification? * **Impact on hardware:** Be specific about the Spresense hardware. Which Spresense boards are affected? Which I2C peripherals? * **Impact on documentation:** Does any documentation need to be updated to reflect this change? If not, explicitly state "NO." * **Impact on security, compatibility:** Even if "NO," explicitly state it for all categories. * **Testing:** * **Build Host(s):** Provide details about your build environment. * **Target(s):** Specify the Spresense board and configuration you tested on. * **Testing logs:** The logs are completely missing. Include *relevant* logs demonstrating the issue before the change and the correct behavior after the change. Don't just dump massive logs; show the key parts that prove the fix works. **Example of an Improved PR Description:** ``` ## Summary The Spresense I2C driver was initializing the I2C pins too early, causing bus contention and freezes when other I2C devices on the bus were already active. This is due to [explain the root cause - e.g., incorrect pin configuration during initialization, premature enabling of the I2C peripheral]. This PR fixes this issue by switching the pin mode to I2C at the end of the `i2c_initialize()` function in `arch/arm/src/spresense/spresense_i2c.c` and adding clock gating and disabling GPIO input in the `i2c_reset()` function. This ensures that the Spresense I2C peripheral doesn't interfere with other devices during initialization. Related NuttX Issue: #[If applicable, link to NuttX issue] ## Impact * **Is new feature added? Is existing feature changed?** Bug fix for existing I2C driver. * **Impact on user:** NO. Users should not need to adapt to this change. The fix addresses a background issue that caused unpredictable behavior. * **Impact on build:** NO. * **Impact on hardware:** YES. This change specifically affects the I2C peripheral on Spresense boards [list specific board models]. * **Impact on documentation:** NO. * **Impact on security:** NO. * **Impact on compatibility:** NO. * **Anything else to consider:** NO. ## 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): Spresense/nuttx_spresense Testing logs before change: ``` [relevant log snippet showing I2C bus freeze, e.g., I2C timeout errors] ``` Testing logs after change: ``` [relevant log snippet showing successful I2C communication with no errors] ``` ``` By providing the missing details and clear explanations, your PR will be much easier to review and merge. Remember to keep the information concise and focused. -- 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
