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/esp32_spi: Add check to see if the TX_FIFO is empty #15904

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

nuttxs
Copy link
Contributor

@nuttxs nuttxs commented Feb 25, 2025

Summary

arch/esp32_spi: Add check to see if the TX_FIFO is empty to avoid sending empty data

Impact

New Feature/Change: Issue fix (no new feature).
User Impact: No
Build Impact:No new Kconfig options or build system changes.
Hardware Impact: Only ESP32; other architectures unaffected.
Security: No
Compatibility: Backward-compatible; no breaking changes.

Testing

ESP32-DevKitC, NuttX with SPI configurations, ESP32 as SPI master device send data.

to avoid sending empty data

Signed-off-by: nuttxs <[email protected]>
@github-actions github-actions bot added Arch: xtensa Issues related to the Xtensa architecture Size: S The size of the change in this PR is small labels Feb 25, 2025
@nuttxpr
Copy link

nuttxpr commented Feb 25, 2025

[Experimental Bot, please feedback here]

Yes, this PR appears to meet the NuttX requirements, although the Testing section could be improved. While it specifies the target, it lacks specific build host information (OS, compiler, etc.) and, critically, it's missing the actual content of the testing logs before and after the change. Simply stating "ESP32 as SPI master device send data" isn't sufficient evidence of testing. The logs should demonstrate the erroneous behavior before the fix and the correct behavior afterward.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Arch: xtensa Issues related to the Xtensa 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.

2 participants