Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DataFlash: Change erase size to pages to reduce memory usage. #10478

Merged
merged 1 commit into from May 21, 2019

Conversation

Projects
None yet
6 participants
@chrissnow
Copy link
Contributor

commented Apr 25, 2019

Description

DataFlash supports page erases, this means that FATFS can use smaller sector sizes and therefore less memory for buffers since read-modify-write can be done in smaller chunks (there are 8 pages to a block)

It would be possible to optimise erases by erasing blocks or sectors where possible but this is not implemented.

This also means that when using non binary pages sizes that you do not exceed FF_MAX_SS of 4096 with 4224 on 16 and 32Mb devices. (DataFlash actually has 264b/528b per page)

Pull request type

[ ] Fix
[ ] Refactor
[ ] Target update
[X] Functionality change
[ ] Docs update
[ ] Test update
[ ] Breaking change

Release Notes

Previously the DataFlash driver exposed the minimum erase size as blocks which are typically 4K, DataFlash however supports page erases as small as 256B.

This change implements page erases which will lower RAM requirements for buffers.

Since this has no relation to any file system and lowers the minimum erase size it should not cause issues with devices with existing data stored on them, however a format of FatFs in future may end up with smaller sectors by default, which will further reduce RAM requirements.

@chrissnow chrissnow changed the title Change DataFlash erase size to pages to reduce memory usage. DataFlash: Change erase size to pages to reduce memory usage. Apr 25, 2019

@ciarmcom ciarmcom requested review from ARMmbed/mbed-os-maintainers Apr 25, 2019

@ciarmcom

This comment has been minimized.

Copy link
Member

commented Apr 25, 2019

@chrissnow, thank you for your changes.
@ARMmbed/mbed-os-storage @ARMmbed/mbed-os-maintainers please review.

@davidsaada
Copy link
Contributor

left a comment

This change is surprising, as I wasn't aware of the fact you could erase a page. Is this supported on all dataflash components?

@chrissnow

This comment has been minimized.

Copy link
Contributor Author

commented Apr 25, 2019

Yes. All devices support page erases.

@davidsaada

This comment has been minimized.

Copy link
Contributor

commented Apr 25, 2019

Yes. All devices support page erases.

All right then, so this is definitely a good change. Approving.

@0xc0170

This comment has been minimized.

Copy link
Member

commented Apr 29, 2019

Yes. All devices support page erases.

@chrissnow Where can I verify this detail?

@chrissnow

This comment has been minimized.

Copy link
Contributor Author

commented Apr 29, 2019

@0xc0170
I went through all of these for a start
https://www.adestotech.com/products/data-flash/

Also https://www.adestotech.com/wp-content/uploads/Adesto-Memory-Products-Brochure.pdf

The Adesto AT45DBxxx DataFlash devices offer uniform page erase size as small as 256 bytes

It seems the docs need an update too, it states it's I2C for a start!
https://os.mbed.com/docs/mbed-os/v5.11/apis/dataflash-block-device.html
it also mentions that there a page erases of 512b, it should really state as low as 256b

Though I can't go through every revision of every device, I'm pretty confident that any AT45 will be fine, small page erases are the main reason to choose DataFlash !

@0xc0170

0xc0170 approved these changes May 2, 2019

@0xc0170 0xc0170 added needs: CI and removed needs: review labels May 2, 2019

@adbridge

This comment has been minimized.

Copy link
Contributor

commented May 7, 2019

CI started

@mbed-ci

This comment has been minimized.

Copy link

commented May 7, 2019

Test run: FAILED

Summary: 2 of 11 test jobs failed
Build number : 1
Build artifacts

Failed test jobs:

  • jenkins-ci/mbed-os-ci_dynamic-memory-usage
  • jenkins-ci/mbed-os-ci_greentea-test
@chrissnow

This comment has been minimized.

Copy link
Contributor Author

commented May 8, 2019

Are those CI issues related to this PR?

@0xc0170

This comment has been minimized.

Copy link
Member

commented May 12, 2019

Are those CI issues related to this PR?

I don't think so. CI job restarted

@0xc0170

This comment has been minimized.

Copy link
Member

commented May 15, 2019

@chrissnow can you rebase this PR? I am seeing unrelated error in the dynamic memory usage (new to me), I would restart testing

@chrissnow chrissnow force-pushed the chrissnow:Dataflash-Erase_Size branch 2 times, most recently from aa93f0f to b09c007 May 15, 2019

@chrissnow chrissnow force-pushed the chrissnow:Dataflash-Erase_Size branch from b09c007 to 157debf May 15, 2019

@chrissnow

This comment has been minimized.

Copy link
Contributor Author

commented May 15, 2019

@0xc0170 rebased, made a bit of a mess with git but should be ok now.

@adbridge

This comment has been minimized.

Copy link
Contributor

commented May 16, 2019

CI restarted

@0xc0170

This comment has been minimized.

Copy link
Member

commented May 20, 2019

Internal CI fault, restarted

@0xc0170

This comment has been minimized.

Copy link
Member

commented May 21, 2019

It again failed to checkout the branch, I restarted

@mbed-ci

This comment has been minimized.

Copy link

commented May 21, 2019

Test run: SUCCESS

Summary: 11 of 11 test jobs passed
Build number : 5
Build artifacts

@0xc0170 0xc0170 removed the needs: CI label May 21, 2019

@0xc0170

This comment has been minimized.

Copy link
Member

commented May 21, 2019

@chrissnow Can you add release notes for this functionality change? (section in your first comment here).

@chrissnow

This comment has been minimized.

Copy link
Contributor Author

commented May 21, 2019

@0xc0170 Release notes added, Is this roughly the sort of thing you were after?

@0xc0170

This comment has been minimized.

Copy link
Member

commented May 21, 2019

Thanks , yes

@0xc0170 0xc0170 merged commit 0560ecc into ARMmbed:master May 21, 2019

26 checks passed

continuous-integration/jenkins/pr-head This commit looks good
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
jenkins-ci/build-ARM Success
Details
jenkins-ci/build-GCC_ARM Success
Details
jenkins-ci/build-IAR Success
Details
jenkins-ci/cloud-client-test Success
Details
jenkins-ci/dynamic-memory-usage Success
Details
jenkins-ci/exporter Success
Details
jenkins-ci/greentea-test Success
Details
jenkins-ci/mbed2-build-ARM Success
Details
jenkins-ci/mbed2-build-GCC_ARM Success
Details
jenkins-ci/mbed2-build-IAR Success
Details
jenkins-ci/unittests Success
Details
travis-ci/astyle Success!
Details
travis-ci/docs Success!
Details
travis-ci/doxy-spellcheck Success!
Details
travis-ci/events Success! Runtime is 8637 cycles.
Details
travis-ci/gitattributestest Success!
Details
travis-ci/include_check Success!
Details
travis-ci/licence_check Success!
Details
travis-ci/littlefs Success! Code size is 8448B.
Details
travis-ci/psa-autogen Success!
Details
travis-ci/tools-py2.7 Success!
Details
travis-ci/tools-py3.5 Success!
Details
travis-ci/tools-py3.6 Success!
Details
travis-ci/tools-py3.7 Success!
Details

@chrissnow chrissnow deleted the chrissnow:Dataflash-Erase_Size branch May 21, 2019

@chrissnow chrissnow restored the chrissnow:Dataflash-Erase_Size branch May 21, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.