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.

Sign up for GitHub

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

Jump to bottom

Add ondie and software ECC code to SPI NAND block device driver #15468

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add ondie and software ECC code to SPI NAND block device driver #15468

wants to merge 1 commit into from
+1,085 −40

Conversation

daniel-0723
Copy link

Summary of changes

Add ondie and software ECC code to SPI NAND block device driver for using ondie ECC function of SPI NAND Flash like Macronix Flash MX31LF4GE4BC or external ECC.

Impact of changes

Migration actions required

Documentation

Pull request type 

[] Patch update (Bug fix / Target update / Docs update / Test update / Refactor)
[x] Feature update (New feature / Functionality change / New API)
[] Major update (Breaking change E.g. Return code change / API behaviour change)

Test results 

[] No Tests required for this change (E.g docs only update)
[] Covered by existing mbed-os tests (Greentea or Unittest)
[x] Tests / results supplied as part of this PR

This driver is tested on DISCO_L4R9I. The flash on this board is MX31LF4GE4BC. You need to replace it with MX31LF4GE4BC.

mbedgt: test case summary: 19 passes, 0 failures
mbedgt: all tests finished!
mbedgt: shuffle seed: 0.2715403632
mbedgt: test suite report:

his driver is tested on DISCO_L4R9I. The flash on this board is MX31LF4GE4BC. You need to replace it with MX31LF4GE4BC.

mbedgt: test case summary: 19 passes, 0 failures
mbedgt: all tests finished!
mbedgt: shuffle seed: 0.2715403632
mbedgt: test suite report:


targetplatform_nametest suiteresultelapsed_time (sec)copy_method
DISCO_L4R9I-GCC_ARMDISCO_L4R9Istorage-blockdevice-tests-tests-blockdevice-general_block_deviceOK39.01default
mbedgt: test suite results: 1 OK
mbedgt: test case report:
targetplatform_nametest suitetest casepassedfailed
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------
DISCO_L4R9I-GCC_ARMDISCO_L4R9Istorage-blockdevice-tests-tests-blockdevice-general_block_deviceDEFAULT Testing get type functionality10
DISCO_L4R9I-GCC_ARMDISCO_L4R9Istorage-blockdevice-tests-tests-blockdevice-general_block_deviceFLASHIAP Testing BlockDevice erase functionality10
DISCO_L4R9I-GCC_ARMDISCO_L4R9Istorage-blockdevice-tests-tests-blockdevice-general_block_deviceFLASHIAP Testing Deinit block device10
DISCO_L4R9I-GCC_ARMDISCO_L4R9Istorage-blockdevice-tests-tests-blockdevice-general_block_deviceFLASHIAP Testing Init block device10
DISCO_L4R9I-GCC_ARMDISCO_L4R9Istorage-blockdevice-tests-tests-blockdevice-general_block_deviceFLASHIAP Testing contiguous erase, write and read10
DISCO_L4R9I-GCC_ARMDISCO_L4R9Istorage-blockdevice-tests-tests-blockdevice-general_block_deviceFLASHIAP Testing multi threads erase program read10
DISCO_L4R9I-GCC_ARMDISCO_L4R9Istorage-blockdevice-tests-tests-blockdevice-general_block_deviceFLASHIAP Testing program read small data sizes10
DISCO_L4R9I-GCC_ARMDISCO_L4R9Istorage-blockdevice-tests-tests-blockdevice-general_block_deviceFLASHIAP Testing read write random blocks10
DISCO_L4R9I-GCC_ARMDISCO_L4R9Istorage-blockdevice-tests-tests-blockdevice-general_block_deviceFLASHIAP Testing unaligned erase blocks10
DISCO_L4R9I-GCC_ARMDISCO_L4R9Istorage-blockdevice-tests-tests-blockdevice-general_block_deviceFLASHIAP Testing write -> deinit -> init -> read10
DISCO_L4R9I-GCC_ARMDISCO_L4R9Istorage-blockdevice-tests-tests-blockdevice-general_block_deviceSPINAND Testing BlockDevice erase functionality10
DISCO_L4R9I-GCC_ARMDISCO_L4R9Istorage-blockdevice-tests-tests-blockdevice-general_block_deviceSPINAND Testing Deinit block device10
DISCO_L4R9I-GCC_ARMDISCO_L4R9Istorage-blockdevice-tests-tests-blockdevice-general_block_deviceSPINAND Testing Init block device10
DISCO_L4R9I-GCC_ARMDISCO_L4R9Istorage-blockdevice-tests-tests-blockdevice-general_block_deviceSPINAND Testing contiguous erase, write and read10
DISCO_L4R9I-GCC_ARMDISCO_L4R9Istorage-blockdevice-tests-tests-blockdevice-general_block_deviceSPINAND Testing multi threads erase program read10
DISCO_L4R9I-GCC_ARMDISCO_L4R9Istorage-blockdevice-tests-tests-blockdevice-general_block_deviceSPINAND Testing program read small data sizes10
DISCO_L4R9I-GCC_ARMDISCO_L4R9Istorage-blockdevice-tests-tests-blockdevice-general_block_deviceSPINAND Testing read write random blocks10
DISCO_L4R9I-GCC_ARMDISCO_L4R9Istorage-blockdevice-tests-tests-blockdevice-general_block_deviceSPINAND Testing unaligned erase blocks10
DISCO_L4R9I-GCC_ARMDISCO_L4R9Istorage-blockdevice-tests-tests-blockdevice-general_block_deviceSPINAND Testing write -> deinit -> init -> read10
mbedgt: test case results: 19 OK
mbedgt: completed in 41.02 sec

@mergify Mergify
Copy link

mergify bot commented Nov 24, 2023

⚠️ The sha of the head commit of this PR conflicts with #15242. Mergify cannot evaluate rules on this PR. ⚠️

@mergify Mergify
Copy link

mergify bot commented Dec 19, 2023

⚠️ The sha of the head commit of this PR conflicts with #15242. Mergify cannot evaluate rules on this PR. ⚠️

0xc0170
0xc0170 reviewed Dec 19, 2023
View reviewed changes
Copy link
Contributor

@0xc0170 0xc0170 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We haven't accepted any feature PR for a while. This is adding new functionality to the component only. have to think this through.

storage/blockdevice/COMPONENT_SPINAND/include/SPINAND/SPINANDBlockDevice.h
@@ -237,6 +238,10 @@ class SPINANDBlockDevice : public mbed::BlockDevice {
*/
virtual const char *get_type() const;

virtual bool is_bad_block(uint16_t blk_idx);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing doxygen docs (line 243 as well)

@multiplemonomials
Copy link
Contributor

@daniel-0723 We'd be interesting in accepting this PR over at https://github.com/mbed-ce/mbed-os ! It seems like a really useful thing to have. However, there are some questions and comments I have about it.

  • Do you have any test data to show that it works? It would be really great if you could run some sort of stress test on your block device to prove that it works. You might be able to adapt the I2C EEPROM block device test suite for this
  • Where did the code in bch.c come from? Could you leave a code comment indicating the original source and any docs that it has?
  • Would you be amenable to splitting this code up into 3 classes? I was thinking, we could keep the existing SPINANDBlockDevice mostly as is, but add new subclasses with your functionality -- maybe OnDieECCNANDBlockDevice (for the on-die ECC) and SwECCNANDBlockDevice (for the software ECC one). That way, compatibility of the original class would be guaranteed for anyone still using it.

@mergify Mergify
Copy link

mergify bot commented Apr 12, 2024

⚠️ The sha of the head commit of this PR conflicts with #15242. Mergify cannot evaluate rules on this PR. ⚠️

1 similar comment
@mergify Mergify
Copy link

mergify bot commented Apr 23, 2024

⚠️ The sha of the head commit of this PR conflicts with #15242. Mergify cannot evaluate rules on this PR. ⚠️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@0xc0170 0xc0170 0xc0170 left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
needs: review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@daniel-0723 @multiplemonomials @0xc0170 @daniel-723