idf-extra-components
idf-extra-components copied to clipboard
added support for Micron MT29F nand flash (IEC-110)
Checklist
- [x] Component contains License
- [x] Component contains README.md
- [x] Component contains idf_component.yml file with
urlfield defined - [x] Component was added to upload job
- [x] Component was added to build job
- [x] Optional: Component contains unit tests
- [x] CI passing
Change description
Add support for the Micron MT29F nand chip, tested specifically on MT29F4G01ABAFDWB
Hi, I don't really understand what the CI is failing at
@UnTraDe the failing clang-tidy job was fixed on the master branch, if your rebase your branch it should get fixed here as well.
Pre-commit check has noticed a minor formatting issue. If you install pre-commit hooks (pip install pre-commit && pre-commit install) then next time you make a commit in this repo, it will be automatically formatted. See https://pre-commit.com/ for more information about this tool.
(Alternatively, you can check the failing pre-commit job log and manually apply formatting changes as shown there.)
Just a formatting issue left
diff --git a/spi_nand_flash/src/nand.c b/spi_nand_flash/src/nand.c
index 61975a4..4791980 100644
--- a/spi_nand_flash/src/nand.c
+++ b/spi_nand_flash/src/nand.c
@@ -136,13 +136,13 @@ static esp_err_t spi_nand_micron_init(spi_nand_flash_device_t *dev)
.command = CMD_READ_ID,
.dummy_bits = 16,
.miso_len = 1,
- .miso_data = &device_id};
+ .miso_data = &device_id
+ };
spi_nand_execute_transaction(dev->config.device_handle, &t);
dev->read_page_delay_us = 25;
dev->erase_block_delay_us = 10000;
dev->program_page_delay_us = 600;
- switch (device_id)
- {
+ switch (device_id) {
case MICRON_DI_34:
dev->dhara_nand.num_blocks = 2048;
dev->dhara_nand.log2_ppb = 6; // 64 pages per block
I've fixed the formatting :)
@UnTraDe I'm sorry, I forgot one thing: please increase the version number in https://github.com/espressif/idf-extra-components/blob/d9b31d9cc535c750c517bd700a86a1268072b69c/spi_nand_flash/idf_component.yml#L1 to 0.2.0. Then your changes will be automatically released once the PR is merged.
P.S
This flash is relatively big (4Gb or 512 MB) with the following configuration:
So basically it has
2048*64 = 131072 addressable logical sectors, but the spi_nand_flash API accepts sector ID as uint16_t, so the entire flash cannot be utilized fully:
https://github.com/espressif/idf-extra-components/blob/d9b31d9cc535c750c517bd700a86a1268072b69c/spi_nand_flash/include/spi_nand_flash.h#L50
It seems that the dhara library API uses dhara_sector_t which is defined as:
typedef uint32_t dhara_sector_t;
So it might be an easy change to just use uint32_t instead?
So it might be an easy change to just use uint32_t instead?
I see no downside to it... @RathiSonika WDYT?
Yes, there shouldn't be an issue. In fact, it should be uint32_t. Thank you for pointing that out.
Should I make it part of this PR or open a separate one?
Is there anything else blocking this PR? 🤔
@UnTraDe Sorry for the late reply, I was on leave. You can either add the uint32_t change here or in a separate PR, up to you. If the current PR without uint32_t change is already useable on this flash chip, I guess we can release it first.
If you could clean up the commit history (git rebase -i to combine commits) before we merge the PR, it would be much appreciated.
@igrr I think it should be in a separate PR, the current version is usable, you just can't refer to the upper sectors. I tried to rebase on master, but since I have merge in this branch it kind of messed up the PR, adding commits from other people in-line. What should I do? (I've reverted the rebase for now)
@igrr Sorry for bumping again, currently our project relies on cloning the fork locally, and specifying it's path relative to the Cargo.toml which causes a bit of a mess. Should I open a new branch and redo these changes to make the commit history a bit more straightforwad?
After that I'll open a new PR for the uint32_t change
@UnTraDe I've pushed one additional fix for CI to pass and cleaned up commit history. I'll merge it as soon as the pipeline passes.