SFDP: Add unit tests for Sector Map Parameter Table parsing #14983
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When a flash chip's SFDP table has no sector map, we treat the whole flash as a single region. As an optimization, this should only be done if there is indeed no sector map.
ciarmcom
added
the
release-type: patch
|
@LDong-Arm, thank you for your changes. |
Patater
previously requested changes
Aug 6, 2021
A flash device with no sector map table has uniform sectors, and we treat the whole flash as one region. In this case the function `sfdp_parse_sector_map_table()` sets the single region's size and high boundary accordingly, but it lacks setting of region count to 1, so users of the SFDP parser may not get the correct number of regions. This commit fixes the issue.
The test data `struct mbed::sfdp_smptbl_info smptbl` is only relevant to `TestEraseTypeAlgorithm` and not shared with other test cases.
LDong-Arm
force-pushed
the
sfdp_sector_map
branch
from
3da94f8 to
ab46817
Compare
|
@Patater Thanks for reviewing, I've addressed your comments, please have a look. |
|
CI Started |
Patater
previously requested changes
Aug 6, 2021
Add tests for `sfdp_parse_sector_map_table()` which currently (at the time of this commit) supports flash devices with * no Sector Map Parameter Table (i.e. the whole flash is uniform and non-configurable) * a single descriptor in the Sector Map Parameter Table (i.e. the flash layout is non-configurable and has multiple regions) Support and unit tests for flashes with multiple configurable layouts will be added in the future. Note: The implementation of `sfdp_parse_sector_map_table()` assumes the table to be valid if read succeeds, so the SFDP reader callback needs to ensure it reads data correctly or return an error.
LDong-Arm
force-pushed
the
sfdp_sector_map
branch
from
ab46817 to
a16c2bf
Compare
Patater
approved these changes
Aug 6, 2021
Jenkins CI Test : ❌ FAILEDBuild Number: 1 | 🔒 Jenkins CI Job | 🌐 Logs & ArtifactsCLICK for Detailed Summary
|
Jenkins CI Test : ✔️ SUCCESSBuild Number: 2 | 🔒 Jenkins CI Job | 🌐 Logs & ArtifactsCLICK for Detailed Summary
|
mbedmain
added
release-version: 6.14.0
Release-pending
and removed
release-type: patch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary of changes
Add tests for
sfdp_parse_sector_map_table()which currently (at the time of this PR) supports flash devices withSupport and unit tests for flashes with multiple configurable layouts (issue #12574) will be added in a subsequent PR.
Note: The implementation of
sfdp_parse_sector_map_table()assumes the table to be valid if read succeeds, so the SFDP reader callback needs to ensure it reads data correctly or return an error.When we cover #12574 in a upcoming PR, we'll add more checks on SFDP data consistency when we rework this function.
Impact of changes
Migration actions required
Documentation
None.
Pull request type
Test results
Reviewers
@ARMmbed/mbed-os-core