STM32F: skip LittleFileSystem default instance and TDBStore tests #13593

LDong-Arm · 2020-09-10T15:43:05Z

Summary of changes

STM32F2/4/7 internal flashes have non-uniform sector layouts – a few smaller sectors (16/32KB), followed by some large ones (e.g. up to 128/256KB) that form most of the total flash. This leads to alignment issues of LittleFileSystem which only supports one constant erase size, whereas we might get a mixture of smaller and larger sectors. This is largely reproducible on F7 targets, though F2/F4's small sectors have less combined capacity so only large ones get used by luck.

This PR skips LittleFileSystem creation on STM32F targets. If a user needs LittleFileSystem on STM32F, they can specify a subset of the flash (where sectors are of the same size), by setting flashiap-block-device.base-address and flashiap-block-device.size in target_overrides of their mbed_app.json. Alternatively, they can manually create a LittleFileSystem instance on top of SlicingBlockDevice to limit the range of FlashIAPBlockDevice to use at runtime.

The alignment issue also affects TDBStore sometimes, which requires two blocks of the same size. Thus we also skip TDBStore in features-storage-tests-kvstore-general_tests_phase_1&2

Impact of changes

On STM32F targets: no more default instance of LittleFileSystem on STM32F provided; TDBStore test cases skipped.
(To reenable both, a user can specify a region of uniform sectors as described above.)

Migration actions required

To use LittleFileSystem on STM32F, users need to specify a region of uniform sectors as described above.

Documentation

We need to update https://os.mbed.com/docs/mbed-os/latest/apis/littlefilesystem.html to instruct users to specify a region of uniform sectors in those scenarios.

Pull request type

[x] Patch update (Bug fix / Target update / Docs update / Test update / Refactor)
[] 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)
[x] Covered by existing mbed-os tests (Greentea or Unittest)
[] Tests / results supplied as part of this PR

Reviewers

@jeromecoutant @LMESTM @evedon @MarceloSalazar
@ARMmbed/mbed-os-core

storage/kvstore/tests/TESTS/kvstore/general_tests_phase_1/main.cpp

ciarmcom · 2020-09-10T16:00:30Z

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

LDong-Arm · 2020-09-10T17:28:12Z

storage/kvstore/tests/TESTS/kvstore/general_tests_phase_1/main.cpp

@@ -101,6 +101,11 @@ static void kvstore_init()
    TEST_ASSERT_EQUAL_ERROR_CODE(0, res);

    if (kv_setup == TDBStoreSet) {
+#if COMPONENT_FLASHIAP && !COMPONENT_SPIF && !COMPONENT_QSPIF && !COMPONENT_DATAFLASH && !COMPONENT_SD


This is cumbersome, but we don't know if the default instance of BlockDevice is FlashIAP or others.

LDong-Arm · 2020-09-10T17:39:24Z

@ARMmbed/mbed-os-storage @MarceloSalazar
For now we want minimal change, so I just added a config.
But I personally think something is lacking from HAL flash_api. It allows us to query the sector/erase size at any address we give. However, we don't have a high-level view of how many regions (of different sector sizes) there are. In the future, something like

void flash_get_region_map_array(uint32_t **array_ptr, size_t *size_ptr);

might help?

LMESTM

LGTM

targets/targets.json

jeromecoutant · 2020-09-11T08:12:10Z

Storage tests with NUCLEO_F767ZI-ARMC6:

master branch: 266 OK / 13 FAIL
with PR: 279 OK

Note that greentea doesn't make the difference between "real OK" and "skipped"

LDong-Arm · 2020-09-11T08:41:41Z

Note that greentea doesn't make the difference between "real OK" and "skipped"

The "runtime" skip prints a skip message in the log, but eventually reported as a pass,. It's a limitation of the test runner I think.

…for STM32L2/4/7 Some internal flashes have non-uniform sectors, and for those ones we want to skip the initialization of default LittleFileSystem on FlashIAPBlockDevice (unless the user specifies an address range that's uniform). This commit adds a config to indicate if sectors are uniform.

…form sectors

LDong-Arm · 2020-09-11T09:07:53Z

@jeromecoutant @LMESTM I've changed the config name and set it for STM32F2/F4 too, could you please have another look?

…or size is non-uniform By default TDBStore requires blocks to have the same size.

evedon

Excellent work @LDong-Arm

LDong-Arm · 2020-09-14T10:11:02Z

@adbridge @0xc0170 Shall we run CI as we've got approvals from both ST and our side?

0xc0170 · 2020-09-15T07:42:34Z

CI restarted

mbed-ci · 2020-09-15T08:44:15Z

Jenkins CI Test : ❌ FAILED

Build Number: 1 | 🔒 Jenkins CI Job | 🌐 Logs & Artifacts

CLICK for Detailed Summary

jobs	Status
jenkins-ci/mbed-os-ci_unittests	✔️
jenkins-ci/mbed-os-ci_build-ARM	✔️
jenkins-ci/mbed-os-ci_build-GCC_ARM	✔️
jenkins-ci/mbed-os-ci_dynamic-memory-usage	✔️
jenkins-ci/mbed-os-ci_cloud-client-pytest	❌
jenkins-ci/mbed-os-ci_greentea-test	✔️

LDong-Arm · 2020-09-15T08:49:37Z

cloud-client-test failed with

FileNotFoundError: [Errno 2] No such file or directory: 'manifest-dev-tool': 'manifest-dev-tool'

0xc0170 · 2020-09-15T12:40:23Z

I found the same issues in LTS CI, it looks like it's on master as well. I'll try to restart if all queues are empty to make sure it is valid issue.

LDong-Arm · 2020-09-17T08:57:22Z

Hi @0xc0170 can we try CI on this again?

mbed-ci · 2020-09-17T10:08:48Z

Jenkins CI Test : ✔️ SUCCESS

Build Number: 2 | 🔒 Jenkins CI Job | 🌐 Logs & Artifacts

CLICK for Detailed Summary

jobs	Status
jenkins-ci/mbed-os-ci_unittests	✔️
jenkins-ci/mbed-os-ci_build-ARM	✔️
jenkins-ci/mbed-os-ci_build-GCC_ARM	✔️
jenkins-ci/mbed-os-ci_dynamic-memory-usage	✔️
jenkins-ci/mbed-os-ci_cloud-client-pytest	✔️
jenkins-ci/mbed-os-ci_greentea-test	✔️

LDong-Arm commented Sep 10, 2020

View reviewed changes

storage/kvstore/tests/TESTS/kvstore/general_tests_phase_1/main.cpp Outdated Show resolved Hide resolved

ciarmcom added the release-type: patch Indentifies a PR as containing just a patch label Sep 10, 2020

ciarmcom requested a review from a team September 10, 2020 16:00

ciarmcom added the needs: review label Sep 10, 2020

LDong-Arm force-pushed the platform_storage_default_requirements branch from c44526f to 6c956eb Compare September 10, 2020 17:27

LDong-Arm commented Sep 10, 2020

View reviewed changes

mergify bot added needs: work and removed needs: review labels Sep 10, 2020

LMESTM approved these changes Sep 11, 2020

View reviewed changes

jeromecoutant approved these changes Sep 11, 2020

View reviewed changes

targets/targets.json Outdated Show resolved Hide resolved

targets/targets.json Outdated Show resolved Hide resolved

LDong-Arm added 2 commits September 11, 2020 09:49

PlatformStorage: skip LittleFileSystem on internal flash with non-uni…

577d450

…form sectors

LDong-Arm force-pushed the platform_storage_default_requirements branch 2 times, most recently from ec091ac to 8ea6fcb Compare September 11, 2020 09:05

LDong-Arm changed the title ~~STM32F7: skip LittleFileSystem default instance and TDBStore tests~~ STM32F: skip LittleFileSystem default instance and TDBStore tests Sep 11, 2020

kvstore general_tests_phase_1/2 tests: Skip TDBStore if FlashIAP sect…

8910ec6

…or size is non-uniform By default TDBStore requires blocks to have the same size.

LDong-Arm force-pushed the platform_storage_default_requirements branch from 8ea6fcb to 8910ec6 Compare September 11, 2020 09:13

jeromecoutant approved these changes Sep 11, 2020

View reviewed changes

evedon approved these changes Sep 11, 2020

View reviewed changes

mergify bot added needs: CI and removed needs: work labels Sep 11, 2020

LDong-Arm mentioned this pull request Sep 14, 2020

NUCLEO_F767ZI failed with features-storage-tests-kvstore-general_tests_phase_1&2 #12806

Closed

mergify bot added the needs: work label Sep 15, 2020

mergify bot removed the needs: CI label Sep 15, 2020

0xc0170 added ready for merge and removed needs: work labels Sep 17, 2020

0xc0170 approved these changes Sep 17, 2020

View reviewed changes

0xc0170 merged commit 33a538a into ARMmbed:master Sep 17, 2020

mergify bot removed the ready for merge label Sep 17, 2020

mbedmain added release-version: 6.4.0 Release-pending and removed release-type: patch Indentifies a PR as containing just a patch Release-pending labels Oct 20, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

STM32F: skip LittleFileSystem default instance and TDBStore tests #13593

STM32F: skip LittleFileSystem default instance and TDBStore tests #13593

LDong-Arm commented Sep 10, 2020 •

edited

ciarmcom commented Sep 10, 2020

LDong-Arm Sep 10, 2020

LDong-Arm commented Sep 10, 2020 •

edited

LMESTM left a comment

jeromecoutant commented Sep 11, 2020

LDong-Arm commented Sep 11, 2020

LDong-Arm commented Sep 11, 2020

evedon left a comment

LDong-Arm commented Sep 14, 2020

0xc0170 commented Sep 15, 2020

mbed-ci commented Sep 15, 2020

LDong-Arm commented Sep 15, 2020

0xc0170 commented Sep 15, 2020

LDong-Arm commented Sep 17, 2020

mbed-ci commented Sep 17, 2020

STM32F: skip LittleFileSystem default instance and TDBStore tests #13593

STM32F: skip LittleFileSystem default instance and TDBStore tests #13593

Conversation

LDong-Arm commented Sep 10, 2020 • edited

Summary of changes

Impact of changes

Migration actions required

Documentation

Pull request type

Test results

Reviewers

ciarmcom commented Sep 10, 2020

LDong-Arm Sep 10, 2020

Choose a reason for hiding this comment

LDong-Arm commented Sep 10, 2020 • edited

LMESTM left a comment

Choose a reason for hiding this comment

jeromecoutant commented Sep 11, 2020

LDong-Arm commented Sep 11, 2020

LDong-Arm commented Sep 11, 2020

evedon left a comment

Choose a reason for hiding this comment

LDong-Arm commented Sep 14, 2020

0xc0170 commented Sep 15, 2020

mbed-ci commented Sep 15, 2020

Jenkins CI Test : ❌ FAILED

Build Number: 1 | 🔒 Jenkins CI Job | 🌐 Logs & Artifacts

LDong-Arm commented Sep 15, 2020

0xc0170 commented Sep 15, 2020

LDong-Arm commented Sep 17, 2020

mbed-ci commented Sep 17, 2020

Jenkins CI Test : ✔️ SUCCESS

Build Number: 2 | 🔒 Jenkins CI Job | 🌐 Logs & Artifacts

LDong-Arm commented Sep 10, 2020 •

edited

LDong-Arm commented Sep 10, 2020 •

edited