search

espressif / idf-extra-components

Additional components for ESP-IDF, maintained by Espressif
153 stars 93 forks source link

feat(spi_nand_flash): expose lower-level API and make it usable without Dhara #447

Open RathiSonika opened 2 weeks ago

RathiSonika commented 2 weeks ago

spi_nand_flash └── src ├── nand.c : Defines all public interfaces; includes definitions for spi_nand_flash.h ├── nand_gigadevice.c : Initializes chip-specific settings for GigaDevice ├── nand_micron.c : Initializes chip-specific settings for Micron ├── nand_winbond.c : Initializes chip-specific settings for Winbond ├── nand_alliance.c : Initializes chip-specific settings for Alliance ├── spi_nand_oper.c : Contains lower-level SPI communication APIs ├── nand_impl.c : Implements lower-level APIs such as nand_erase, nand_mark_bad, nand_prog, etc. ├── nand_impl_wrap.c : Implements wrapper functions for lower-level APIs implemented in nand_impl.c └── dhara_glue.c : Initializes and defines Dhara-related APIs and registers function pointers in nand.c. Only dhara_glue.c depends on the Dhara library, making it easy to replace with another library if needed, allowing the code to function independently of Dhara.

Refactoring nand.c and the chip-specific settings is possible; however, this PR primarily focuses on exposing lower-level APIs and enabling easy replacement of the Dhara library in the future, if needed.

Checklist

Change description

  1. Expose lower level API such as nand_erase, nand_mark_bad, nand_prog etc. and make it usable without Dhara
  2. Fixed an issue in the nand_mark_bad() API. After erasing a block, it now correctly waits for the erase_block_delay time. Previously, the function was failing to mark the block as bad due to this missing delay.
pacucha42 commented 5 days ago

Generally LGTM, but why do you place spi_nand_api.h into private includes? Isn't this PR about exposing the APIs?

RathiSonika commented 5 days ago

Generally LGTM, but why do you place spi_nand_api.h into private includes? Isn't this PR about exposing the APIs?

This is actually a private header for this component. The APIs defined here are used internally within the component and are not public APIs. I understand the name might be confusing, so I’ll rename it.

RathiSonika commented 1 day ago

@igrr PTAL. Thank you!