ARMmbed / mbed-os

Arm Mbed OS is a platform operating system designed for the internet of things
https://mbed.com
Other
4.67k stars 2.98k forks source link

SDBlockDevice does not implement erase command #9465

Closed stevew817 closed 5 years ago

stevew817 commented 5 years ago

Description

Following up on https://github.com/ARMmbed/simple-mbed-cloud-client/pull/67, it seems that SDBlockDevice does not implement the erase function. Why is that? According to the SD command specifications, SD cards are more than capable of doing block erases when asked to (CMD32, CMD33, CMD38 with the appropriate delays). It's even in the header: https://github.com/ARMmbed/mbed-os/blob/16e544d9b4d8790c5a9ce3f7bc7480634539c8ac/components/storage/blockdevice/COMPONENT_SD/SDBlockDevice.h#L180 Alternatively, the erase command could be implemented by writing a full block with an empty buffer.

This would at least allow people to actually write storage-independent code for block-based storage. Not this ostrich-policy of pretending like the erase was successful, but reading the same block back will return the old data.

CC @screamerbg

Issue request type

[ ] Question
[ ] Enhancement
[X] Bug
screamerbg commented 5 years ago

Thanks for raising this @stevew817.

Agreed with @stevew817's observation above that sdblockdevice behaves differently compared to other blockdevices. For example, if I need to wipe the contents of a sector regardless of the underlying blockdevice type, do I need to call program() or erase()?

cmonr commented 5 years ago

@ARMmbed/mbed-os-storage Fyi

ciarmcom commented 5 years ago

Internal Jira reference: https://jira.arm.com/browse/MBOCUSTRIA-786

deepikabhavnani commented 5 years ago

it seems that SDBlockDevice does not implement the erase function. Why is that?

SD cards do not need explicit erase before program like raw NAND/NOR flash memories

For example, if I need to wipe the contents of a sector regardless of the underlying blockdevice type, do I need to call program() or erase()?

In case of flash device, explicit erase is needed before program. But in case of SD erase is not needed (internal flash translation layer manages that). If we add erase implementation writes will be slow for unnecessary erase. FAT explicitly erases block before program.

Alternatively, SD has trim functionality which users can use to perform explicit disk / partition / block erase.

stevew817 commented 5 years ago

I get that there are certain optimisations that SD is providing and you want to make use of. But that's not the point of this ticket.

The point is that all of the storage volumes implementing the BlockDevice interface should behave similarly, regardless of whether that's the most optimal way to go about something for that specific device or not. Otherwise you can't very well call it an interface...

deepikabhavnani commented 5 years ago

This would at least allow people to actually write storage-independent code for block-based storage. Not this ostrich-policy of pretending like the erase was successful, but reading the same block back will return the old data.

Storage independent code can be written now as well (example is FAT / LittleFs). Please explain what use case is failing here? it seems block device is used independent of filesystem and some issues are faced. See if https://github.com/ARMmbed/mbed-os/blob/master/components/storage/blockdevice/COMPONENT_SD/SDBlockDevice.h#L114 serves the purpose

deepikabhavnani commented 5 years ago

@stevew817 - We need some way to have performance plus functionality, all block devices are not same and do not behave similarly.

Erase + Trim functionality is provided in block device layer https://github.com/ARMmbed/mbed-os/blob/master/features/storage/blockdevice/BlockDevice.h#L136

If application is writing storage-independent code on Mbed OS with block device directly, every erase call will be combination of erase and trim in sequence. No block device will implement both.

deepikabhavnani commented 5 years ago

Another option is to perform write with zeros to do erase from the application https://github.com/ARMmbed/mbed-os/blob/993c897b55b27e90035bef784d39fb68d9adec64/features/storage/filesystem/fat/ChaN/ff.cpp#L1644 https://github.com/ARMmbed/mbed-os/blob/993c897b55b27e90035bef784d39fb68d9adec64/features/storage/filesystem/fat/ChaN/ff.cpp#L1664 Please note every disk_write operation is erase and program - Erase (if implemented will erase for flash devices) - Program (with zeros - can be heap / SD card )

davidsaada commented 5 years ago

Agree with @deepikabhavnani. Would just like to elaborate: SD driver indeed doesn't implement erase, and so are other block devices (such as HeapBlockDevice). We have added the get_erase_value API to all block devices, which either returns the read value after erase or -1 if the device doesn't support erase (like SD). Now, we have also added FlashSimBlockDevice - this is a virtual block device, getting a physical block device as an argument when constructed. This virtual block device implements erase on top of the underlying block device, by filling the value of choice when calling this API. This will give you the effect you want. For example:

SDBlockDevice sd(...);
FlashSimBlockDevice flash_sd(&sd, 0xFF). 

Any API you call on flash_sd will be equivalent to the one you call on sd, except for erase, which will program the value 0xFF on the range you choose.