Skip to content

fix(spi_nand_flash): spi_nand_program_load fails if buffer is not DMA aligned#700

Merged
RathiSonika merged 1 commit intoespressif:masterfrom
RathiSonika:fix/nand_prog_load_buf_align_issue
Mar 13, 2026
Merged

fix(spi_nand_flash): spi_nand_program_load fails if buffer is not DMA aligned#700
RathiSonika merged 1 commit intoespressif:masterfrom
RathiSonika:fix/nand_prog_load_buf_align_issue

Conversation

@RathiSonika
Copy link
Collaborator

Change description

Fix failure in spi_nand_program_load when the input buffer is not DMA-aligned.

Mainly it handles-

  • Page-sized writes : always length-aligned on all chips → uses temp_buffer bounce only when buffer isn't DMA-ok, avoids SPI driver malloc
  • Small marker writes (infrequent, 4 bytes): unaligned on P4 → delegated to SPI driver, which is safe for such tiny allocations. On non-P4 chips, 4 bytes is aligned to 4, so even these use the direct/temp_buffer path

Closes #684

@RathiSonika RathiSonika force-pushed the fix/nand_prog_load_buf_align_issue branch from 3ab4a5c to ec6d455 Compare March 9, 2026 12:00
@pacucha42
Copy link
Collaborator

Left 2 minor comment, PTAL

@RathiSonika RathiSonika force-pushed the fix/nand_prog_load_buf_align_issue branch from 369008a to e3b5e57 Compare March 11, 2026 12:34
@RathiSonika
Copy link
Collaborator Author

Fixed. PTAL. Thank you!

@RathiSonika RathiSonika force-pushed the fix/nand_prog_load_buf_align_issue branch 2 times, most recently from b2cee4b to 54e0e68 Compare March 11, 2026 18:09
@pacucha42
Copy link
Collaborator

pacucha42 commented Mar 12, 2026

AI generated review, PTAL:

1. Linux build: esp_memory_utils.h and SOC_DMA_* (spi_nand_oper.c)

Error: The PR adds unconditional #include "esp_memory_utils.h" and uses esp_ptr_dma_capable(data) inside the #if ESP_IDF_VERSION >= ESP_IDF_VERSION_VAL(5, 2, 0) block. That block is compiled for the Linux target as well. esp_memory_utils.h uses SOC_DMA_LOW and SOC_DMA_HIGH from soc/soc.h. The Linux target’s components/soc/linux/include/soc/soc.h does not define SOC_DMA_LOW or SOC_DMA_HIGH. Building spi_nand_flash for the Linux target can therefore fail with undefined macro errors.

Fix: Either:

  • Guard the include and all uses of esp_ptr_dma_capable / DMA-alignment logic with #if !CONFIG_IDF_TARGET_LINUX (and use a single path for Linux that does not depend on DMA capability), or
  • Ensure the Linux soc provides stub definitions for SOC_DMA_LOW / SOC_DMA_HIGH so that esp_memory_utils.h compiles on Linux (and document that esp_ptr_dma_capable is then meaningless on host).

2. CHANGELOG.md formatting

Error: The new changelog entry splits the issue reference onto a new line after the hyphen, e.g.:

- fix: spi_nand_program_load fails in case buffer is not DMA aligned
 (https://github.com/...)

This can render as a broken list or odd line break. Prefer a single line, e.g. - fix: ... (issue #684) or the URL on the same line.

Minor: There are two blank lines before ## [0.18.0]; use a single blank line between version blocks for consistency.


3. Header spi_nand_oper.h – C++ linkage

Error: The new declaration size_t spi_nand_get_dma_alignment(void); is not wrapped in extern "C". If this header is included from C++ (e.g. from another component or a C++ app), the symbol may be mangled and cause link errors.

Fix: Wrap the declaration (and any other C API in that header) in:

#ifdef __cplusplus
extern "C" {
#endif
...
#ifdef __cplusplus
}
#endif

4. nand.c – CONFIG_IDF_TARGET_LINUX availability

Check: The #ifndef CONFIG_IDF_TARGET_LINUX / #else branches rely on CONFIG_IDF_TARGET_LINUX being defined when building for the Linux target. Ensure sdkconfig.h (or the build system’s equivalent) is included before this preprocessor use so that the correct branch is selected for Linux vs non-Linux builds.


Summary

# Severity Item
1 Critical Linux build can fail: esp_memory_utils.h needs SOC_DMA_*; guard include/use or add Linux soc stubs.
2 Minor CHANGELOG: single-line entry and single blank line between version blocks.
3 Medium spi_nand_oper.h: add extern "C" for C++ inclusion.
4 Verify nand.c: ensure CONFIG_IDF_TARGET_LINUX is defined when expected.

@RathiSonika RathiSonika force-pushed the fix/nand_prog_load_buf_align_issue branch 3 times, most recently from eb9d580 to 8f22bc2 Compare March 12, 2026 18:18
@RathiSonika
Copy link
Collaborator Author

  1. esp_memory_utils.h / SOC_DMA_* on Linux - Not applicable. Per CMakeLists.txt, spi_nand_oper.c is only compiled for non-Linux targets, so there's no Linux build risk here.

  2. CHANGELOG : Combined the split entry into a single line.

  3. spi_nand_oper.h - extern "C" guards - Already present. The header already wraps declarations in extern "C" (lines 16-18 and 73-74).

  4. CONFIG_IDF_TARGET_LINUX availability - Already handled. nand.c includes esp_check.h which transitively includes sdkconfig.h, so the macro is properly defined before the preprocessor uses it.

Additionally, for host_tests, there is no need for a fatfs dependency. In fact, the spi_nand_flash component itself does not require a fatfs dependency. I will remove this dependency in a following MR. For now, it has only been updated for host-specific builds.

@RathiSonika RathiSonika force-pushed the fix/nand_prog_load_buf_align_issue branch from 8f22bc2 to 698dd74 Compare March 13, 2026 10:16
@RathiSonika RathiSonika force-pushed the fix/nand_prog_load_buf_align_issue branch from 698dd74 to 9cda83d Compare March 13, 2026 10:26
Copy link
Collaborator

@pacucha42 pacucha42 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for addressing my and AI comments

@RathiSonika RathiSonika merged commit cee96a2 into espressif:master Mar 13, 2026
85 checks passed
@RathiSonika RathiSonika deleted the fix/nand_prog_load_buf_align_issue branch March 13, 2026 12:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SPI NAND program load may fail with non-DMA-capable buffer (IEC-488)

3 participants