Erase size

[chrissnow]

@marcuschangarm I thought I would switch to you driver rather than mine and noticed a few issues, not all are directly related to your driver though.

You are using block erase rather than page erase, is the improvement in erase time worth the overhead of a larger read modify write? I'm thinking that with mbed people will be storing small files or making small changes to files?, fatfs will also need a 8x larger buffer which wont be good on smaller targets.

This also means that 16mbit and above in 528b the sector size for fatfs is greater than 4096 which is the largest that is supported.

Another odd problem is that if you use 512\528 with a 16mbit device and try and format without specifying the AU it fails.
https://github.com/ARMmbed/mbed-os/blob/20d93bf78d5172b6a0f1c1065058d641e4e0f2e1/features/filesystem/fat/ChaN/ff.cpp#L4172-L4174
results in FAT16 as the result since nclst = 0x1000
yet this then fails here as there is not enough sectors for FAT16
https://github.com/ARMmbed/mbed-os/blob/20d93bf78d5172b6a0f1c1065058d641e4e0f2e1/features/filesystem/fat/ChaN/ff.cpp#L4204-L4207
and returns

FR_MKFS_ABORTED

if you format with an AU of 2

FATFileSystem::format(&flash,2)

It will succeed with FAT12

Also with the default SPI speed of 40M the lowest bit is missed on the LPC1768 resulting in a shift right on all SPI reads. 8M works fine though. (I didn't try higher).

cc @geky

[marcuschangarm]

Hi @chrissnow, the sector erase was mainly chosen to give it the same timing behavior as other older flash chips. If most chips have page erase I see no problem in changing it here as well.

Regarding the LPC1768, could you open an issue on the main mbed OS site, please? Thanks!

[chrissnow]

Looks like the LPC1768 can go up to 33Mhz, I'm not sure how mbed deals with excessive requests.
Perhaps a safer default could be used?

[marcuschangarm]

@geky thoughts? 😄

[geky]

I'm not sure I understand the concern about timing behavior. Is this just a performance optimization? Have we ran into issues with writes?

The read/prog/erase sizes should all be the storage device's physical limitations, so the smaller the erase size the better. It's up to the filesystem to provide a "logical block size" that the user can configure for better performance. (FAT has cluster size/AU, littlefs has block size, and spiffs has page and block sizes).

The bugs are troublesome but better to find them now than later 😄

• The spi frequency is an interesting issue. We've been using 40MHz all over the place as a default. I wonder if we could get a sort of "max spi freq" target option into mbed OS.

  In the meantime I'm not opposed to switching to a safer value.

• The FAT16 bug is definitely a problem that needs fixing. If you can, opening an issue on mbed OS would be helpful to track it.

[marcuschangarm]

My timing concerns are (based on large erase sizes used to be the norm):

• Customers swapping one flash type to another might be surprised by the different behavior.
• Customers with code optimized for large erase sizes would take a performance hit.

However, if we know that that is no longer the case and/or we choose to favor small erase sizes over speed, then we should change the erase size in the driver.

I'm fine with either. 😄

[chrissnow]

I don't think people would expect that different flash types act the same, Having smaller erase sizes certainly allows more flexibility and is exactly the reason I picked dataflash for my projects, low RAM overhead and working with small files doesn't cause large areas to be erased and rewritten for no reason.

If you weren't interested in the small erase sizes of dataflash you would probably be better off with something else. it's a nice compromise between eeprom and traditional flash, 4k is a fair overhead for a little micro.

The nicest solution to this is actually to set the erase size to the page size but to do a block erase if possible, this might mean page erases, a block erase and more page erases depending on alignment and requested size.

That way the user could set their logical block size to 4096 and the driver would end up doing page erases.

[geky]

Users can always choose a larger block size at the filesystem level. They can't exactly go backwards, so I would lean towards smaller erase sizes.

And @chrissnow has a good point, if we chose the larger erase size when the erase region fits we could remove any performance hit. Though this optimization is probably low priority at the moment.

[marcuschangarm]

Sounds good. @geky can you put this on Jira so it gets properly processed please? 😄

[geky]

Sidenote, found an old implementation of a greedy erase that shows how choosing the fastest erase size could be implemented:
https://github.com/geky/mbed/blob/f23fc167a0cf67611a4355a4b53ce5ce2110c4a7/features/filesystem/mx25r/MX25RBlockDevice.cpp#L161-L219

[geky]

@marcuschangarm here you go IOTMORF-1632, IOTMORF-1633.

@chrissnow don't feel detered from trying to fix these if you have the time (wink, wink), the jira tickets are just for internal tracking+prioritization.