Skip to content

WIP mtd_spi_nor: MTD interface driver for SPI NOR flash chips#5668

Closed
jnohlgard wants to merge 38 commits intoRIOT-OS:masterfrom
jnohlgard:pr/mtd-spi-nor
Closed

WIP mtd_spi_nor: MTD interface driver for SPI NOR flash chips#5668
jnohlgard wants to merge 38 commits intoRIOT-OS:masterfrom
jnohlgard:pr/mtd-spi-nor

Conversation

@jnohlgard
Copy link
Member

This is a generic SPI NOR flash driver which can be used with many different flash chips.

Depends on #5616, #5624

I wrote this before #5660 was posted, but I found that there are some useful bits in this PR that might come in handy.

I have posted this for comparison against #5660. I believe the two PRs should be unified before merging either.

Tested on mulle with #5625 applied on top.

Joakim Nohlgård and others added 30 commits July 20, 2016 12:26
The VFS layer provides file system abstractions to allow using a unified
interface to access files from mounted file systems.
usage: vfs r /path/to/file nbytes offset

Only reading is currently supported
mtdi stands for memory technology device interface. It's a generic interface
which can be used to implement flash drivers.
@jnohlgard jnohlgard added Type: new feature The issue requests / The PR implemements a new feature for RIOT State: waiting for other PR State: The PR requires another PR to be merged first Area: drivers Area: Device drivers labels Jul 20, 2016
@jnohlgard
Copy link
Member Author

@vincent-d we should try to unify your #5660 driver and this one. I've tested this driver and it works on the Micron M25P16 onboard on Mulle, but this driver does not provide support for write protect, chip and block level erase. On the other hand, this driver supports opcode tables for supporting chips with different command sets.

@vincent-d
Copy link
Member

@gebart I'm off until Monday. I'll take a look at this next week. It looks interesting, I believe we can have a nice driver if we unify our both PR.

@vincent-d
Copy link
Member

vincent-d commented Jul 25, 2016

I like the genericity and the simplicity of your driver.

I think we should work on a way to ease the initialization of the different layers, maybe with some macros to avoid ugly struct init with default parameters in the board files.

Last comment, your current driver seems to use 64K sectors, and the sector erase op code correspond to the block erase in the mx25v, the 4K sector erase is another op code. But it could be easily managed by using different op codes arrays.

@jnohlgard
Copy link
Member Author

Yes, the chip I was testing against (Micron M25P16) only supports one kind of sector erase (64 KB).
I guess we should add more erase opcodes and let the implementation choose between them based on the size of the erase requested.
In particular, doing sector by sector erase to format the file system is extremely slow compared to performing a chip erase. Chip erase takes 10 seconds while sector by sector takes 30-60 seconds on the M25P16.

I took some inspiration from Linux' mtd/spi-nor driver which is more dynamic than this PR, but it's targeting much more powerful systems than RIOT is.

@jnohlgard
Copy link
Member Author

@vincent-d I won't have the opportunity to work any more on this in the near future, would you like to take point on merging the best of the two implementations?
I'll try to finish the unit tests on VFS to make it ready for mainline so that we can merge #5616 once all this release craziness is over.

@vincent-d
Copy link
Member

@gebart sure, I'll try to work on that within the next days

@vincent-d
Copy link
Member

@gebart I have an updated version of this PR locally which fixes erase and port to new spi interface, how could we proceed to update that one ?

@jnohlgard
Copy link
Member Author

@vincent-d open it as a new PR and we can do the review collaboratively. I mean we both review the code, since I wrote some parts and you wrote some parts.

@vincent-d
Copy link
Member

Closed in favor of #6762

@vincent-d vincent-d closed this Mar 27, 2017
@jnohlgard jnohlgard deleted the pr/mtd-spi-nor branch April 6, 2017 08:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: drivers Area: Device drivers State: waiting for other PR State: The PR requires another PR to be merged first Type: new feature The issue requests / The PR implemements a new feature for RIOT

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants