Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

arch: cxd56xx: Fix i2c bus freeze at i2c initialization #15870

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

SPRESENSE
Copy link
Contributor

Summary

During I2C communication between non-Spresense devices is performed,
the I2C bus may freeze after initialization of the Spresense I2C.

As a workaround,

  • switch the pin mode to I2C at the end of the i2c initialization function.
  • add clock gating process and disable GPIO input to the I2C reset function.

Impact

Only for spresense board

Testing

@github-actions github-actions bot added Arch: arm Issues related to ARM (32-bit) architecture Size: S The size of the change in this PR is small labels Feb 19, 2025
@nuttxpr
Copy link

nuttxpr commented Feb 19, 2025

[Experimental Bot, please feedback here]

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.

@lupyuen
Copy link
Member

lupyuen commented Feb 19, 2025

Please remember to sign-off the 2 commits: git commit -s. Thanks :-)

During I2C communication between non-Spresense devices is performed,
the I2C bus may freeze after initialization of the Spresense I2C.
As a workaround, add clock gating process and disable GPIO input
to the I2C reset function.

Signed-off-by: SPRESENSE <[email protected]>
During I2C communication between non-Spresense devices is performed,
the I2C bus may freeze after initialization of the Spresense I2C.
As a workaround, switch the pin mode to I2C at the end of the i2c
initialization function.

Signed-off-by: SPRESENSE <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Arch: arm Issues related to ARM (32-bit) architecture Size: S The size of the change in this PR is small
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants