boards/feather-nrf52840*: add external flash configuration#21324
boards/feather-nrf52840*: add external flash configuration#21324mguetschow merged 3 commits intoRIOT-OS:masterfrom
Conversation
mguetschow
added
the
CI: ready for build
crasbe
left a comment
crasbe
left a comment
There was a problem hiding this comment.
I don't have a Feather for testing unfortunately, but in general the changes are straight forward. Even though the mtd.c files are mostly guarded by the #ifdef MODULE_MTD, it would be nice to have documentation for the structs IMO.
I know that the other mtd.c files don't have that either, but it would still be nice. Perhaps even allow Doxygen to look into the file?
crasbe
added
the
Type: enhancement
boards/feather-nrf52840-sense/mtd.c
Outdated
| .driver = &mtd_spi_nor_driver, | ||
| .page_size = 256, | ||
| .pages_per_sector = 16, | ||
| .sector_count = 512, |
There was a problem hiding this comment.
Do you need to specify the sector_count? If not set it should be detected automatically.
There was a problem hiding this comment.
No, I just noticed that some mtd params definitions do, so I added it. Will recheck without the explicit sector count.
There was a problem hiding this comment.
It's also not documented to be an optional information: https://doc.riot-os.org/structmtd__dev__t.html
There was a problem hiding this comment.
Yep, still works:
$ make -C tests/drivers/mtd_raw flash test
...
test 0
-=[ MTD_0 ]=-
sectors: 512
pages per sector: 16
page size: 256
total: 2 MiB
Would you mind posting a follow-up PR that updates the documentation in that regard? Or tell me which other fields are optional, too, and I can do it?
|
Thanks for having a look already!
The structs themselves are documented in https://doc.riot-os.org/group__drivers__mtd__spi__nor.html and the concrete values picked don't add much to be worth documenting IMO. After all, they are taken more or less straight from the datasheet.
Doxygen currently doesn't parse source files at all: https://github.com/RIOT-OS/RIOT/blob/master/doc/doxygen/riot.doxyfile#L929. Changing that would be a major task, I guess, especially because it will start complaining about an infinite amount of undocumented symbols. |
Right, I missed that it's a source file. You can pretty much ignore that comment then 😅 |
|
You can squash the fixups. Unfortunately this needs to be rebased after #21327. |
pins derived from https://learn.adafruit.com/assets/68545, flash device is GD25Q16C as on adafruit-pybadge with datasheet at https://cdn-shop.adafruit.com/product-files/4763/4763_GD25Q16CTIGR.pdf
pins derived from https://learn.adafruit.com/assets/127209, flash device is GD25Q16C as on feather-nrf52840 with datasheet at https://cdn-shop.adafruit.com/product-files/4763/4763_GD25Q16CTIGR.pdf
mguetschow
force-pushed
the
feather-nrf52840-flash
branch
from
862aef2 to
d1084e0
Compare
Done :) |
|
Thank you! |
4 participants
Contribution description
Both boards
feather-nrf52840andfeather-nrf52840-sensehave a GD25Q16 external flash according to the schematic, I've assumed that it is actually a GD25Q16C as onadafruit-pybadge.Schematics can be found here and here, the datasheet is here, but I've copied the flash-specific values from
adafruit-pybadge.Testing procedure
make -C tests/drivers/mtd_raw flash testsucceeds on both devices.Playing around with
make -C examples/basic/filesystem flash termis successful on both devices, too.Issues/PRs references
Followed the example from #20980