STM32F4 - Add STORAGE driver #2556
Conversation
|
@0xc0170 @screamerbg Do you know any people who can be interested in this PR ? |
|
Any news ? |
| "detect_code": ["0796"], | ||
| "features": ["IPV4"], | ||
| "features": ["IPV4", "STORAGE"], |
The storage is in the features folder. If I remove |
|
CFStore and its use on internal storage/journal is under discussion. For now its best to get this working not worrying about those. The hal API may also change so please beware. |
|
I'm very interested in seeing this PR go into mbed-os. @svastm do you have all the information you need to proceed? |
|
Thanks for submitting Storage implementations.
It is not mandatory to support a large cf-store. cf-store isn't meant to be a filesystem; it is meant as a small persistent dictionary of key-value pairs. It is up to the application to choose a size for its config-store; failing which, the system designer/porter should provide a reasonable default.
It is OK to split the test specification in order to allow time for the tests to pass. Perhaps there's too much bundled into basicAPI to fit within 60 seconds on all platforms. But there is some value in bundling API testing together, and I'd rather raise the 60-second limit to something which allows tests to run to completion on the ST platforms: update the following within GREENTEA_SETUP(60, "default_auto");Will 120 seconds suffice?
The Storage driver works under the assumption that only one command can be active at a time. All data-oriented commands (i.e. commands with result in hardware interaction) may return if (controllerCurrentlyBusy()) {
/* The user cannot initiate any further FTFE commands until notified that the
* current command has completed.*/
return (int32_t)ARM_DRIVER_ERROR_BUSY;
}Such a check (resulting in a return) is made very early during command validation to avoid affecting existing operations. Higher layers may also introduce synchronization to serialize access to Storage. |
There was a problem hiding this comment.
Please implement the storage driver to cover all of internal flash; not just a space that would be expected to hold the config-store.
Please combine sectors into ARM_STORAGE_BLOCKs.
Please consider implementing optimizations for double-word access.
My quick reading uncovered some minor bugs; please address that.
| #endif | ||
|
|
||
| #include <stdint.h> | ||
| #include "flash-journal-strategy-sequential/config.h" |
There was a problem hiding this comment.
there is no need to depend on the flash-journal. Furthermore, this particular file doesn't exist any longer (or is in the process of getting obsoleted by one of the pull requests).
| #include <stdint.h> | ||
| #include "flash-journal-strategy-sequential/config.h" | ||
|
|
||
| #define ERASE_UNIT (128 * 1024) /* Max size of a sector (in bytes) */ |
There was a problem hiding this comment.
In the case of ST micros, it doesn't make sense to have a single ERASE_UNIT (since this varies over the storage)
| #define OPTIMAL_PROGRAM_UNIT (0) /* Not used, we are flashing bytes */ | ||
|
|
||
| /* Size of the flash block */ | ||
| #define BLOCK_SIZE (SEQUENTIAL_FLASH_JOURNAL_MAX_LOGGED_BLOBS * ERASE_UNIT) |
There was a problem hiding this comment.
likewise, there is no one single BLOCK_SIZE. Particularly, this has nothing to do with the flash-journal.
| #define BLOCK_SIZE (SEQUENTIAL_FLASH_JOURNAL_MAX_LOGGED_BLOBS * ERASE_UNIT) | ||
|
|
||
| /* Address of the flash block */ | ||
| #define BLOCK_ADDR (FLASH_TOP_ADDR - BLOCK_SIZE) |
There was a problem hiding this comment.
this value doesn't make sense. It seems like you're setting aside one config-store partition equivalent to SEQUENTIAL_FLASH_JOURNAL_MAX_LOGGED_BLOBS units of 128K at the end of flash. This sort of thing shouldn't contaminate a generic Storage implementation.
|
|
||
| #include "storage_driver_info.h" | ||
|
|
||
| const flash_sector_info_t flash_sector_info_table[] = { |
There was a problem hiding this comment.
It may be better to model this as an array of ARM_STORAGE_BLOCK; multiple 'sectors' with identical attributes can be combined into a single ARM_STORAGE_BLOCK. So instead of having 4 entries of 16K sectors, it may be better to have a single ARM_STORAGE_BLOCK with a 16k erase-unit.
| EraseInitStruct.NbSectors = nb_sectors; | ||
|
|
||
| if (capabilities.asynchronous_ops == 0) { | ||
|
|
There was a problem hiding this comment.
remove un-necessary blank line
|
|
||
| context->command = ARM_STORAGE_OPERATION_ERASE; | ||
|
|
||
| int8_t base_sector = -1; |
There was a problem hiding this comment.
this paragraph of code needs re-implementation based on ARM_STORAGE_BLOCK, not sectors.
| /* Synchronous programming */ | ||
| HAL_FLASH_Unlock(); | ||
| for (uint64_t i = 0; i < size; i++) { | ||
| if (HAL_FLASH_Program(FLASH_TYPEPROGRAM_BYTE, (uint32_t)(i + addr), ((const uint8_t *)data)[i]) != HAL_OK) { |
There was a problem hiding this comment.
doing I/O one byte at a time is going to be inefficient.
| static const ARM_STORAGE_INFO informations = { | ||
| .total_storage = BLOCK_SIZE, | ||
| .program_unit = PROGRAM_UNIT, | ||
| .optimal_program_unit = OPTIMAL_PROGRAM_UNIT, |
There was a problem hiding this comment.
I believe the optimal program unit for this family of ST micros would be a double-word--i.e. 8 bytes. It seems you don't want to offer that optimization at this stage.
| #define ERASE_UNIT (128 * 1024) /* Max size of a sector (in bytes) */ | ||
| #define FLASH_TOP_ADDR (0x8200000UL) /* Top of the flash address */ | ||
| #define PROGRAM_UNIT (8UL) /* Program alignement (8 bytes) */ | ||
| #define OPTIMAL_PROGRAM_UNIT (0) /* Not used, we are flashing bytes */ |
There was a problem hiding this comment.
this could be as large as a double-word. That would be good for performance and energy-consumption.
svastm
force-pushed
the
storage_f4
branch
2 times, most recently
from
5a77ab8
to
b2b440c
Compare
|
Thank you for the complete answers. Also I focus now this PR on storage driver and forget about cfstorage.
Done. My sector_table was a copy of this struct.
This can be done but the
The voltages of the NUCLEO boards allow only a word size programming but I did the change. Tests
|
| #define FLASH_INFO_H | ||
|
|
||
| /* Total size reserved for the storage */ | ||
| #define FLASH_INFO_TOTAL_STORAGE (0x00100000) |
There was a problem hiding this comment.
1MB is the total storage, isn't it. You've correctly pointed out that if the entire internal flash is covered by Storage then the tests might inadvertently overwrite/erase portions of the test-application.
Perhaps this 'TOTAL_STORAGE' (together with start address) could be configuration variable(s).
|
|
||
| static const ARM_STORAGE_INFO informations = { | ||
| .total_storage = FLASH_INFO_TOTAL_STORAGE, | ||
| .program_unit = 4, /* word (4 bytes) programming */ |
There was a problem hiding this comment.
program_unit can still be 1 byte; that allows finer granularity for updates.
Your correct in pointing this out. Perhaps you could have the Storage driver configurable in its start-addr and total-size parameters. By default , these parameters could be set to span the second half of the internal flash space.
Thanks for making this effort. Could you take a small additional step of allowing program_unit as low as 1 byte? Word access should still be possible and indicated by setting The program() operation should internally switch to word sized updates for write sequences larger than a word. |
Is there something in the API to do it ? For now, the interface give information about the size but to know where the start address of the storage is we have to use
it would be a good optimization. Unfortunately the test of // line 381 of storage_abstraction/basicAPI.cpp
size_t sizeofData = info.program_unit;
TEST_ASSERT(BUFFER_SIZE >= sizeofData);
TEST_ASSERT((sizeofData % sizeof(uint32_t)) == 0);
for (size_t index = 0; index < sizeofData / sizeof(uint32_t); index++) {
((uint32_t *)buffer)[index] = BYTE_PATTERN;
} |
Here's an example of how the start addr is set for the K64F storage driver. This is a configuration variable which can be tweaked at compile time. You could arbitrarily start midway from the flash, and range until the end. This approach works with the K64F because all sectors come in the same size. It may not work as nicely with STM32, where sector sizes increase exponentially. What you could do is to have a smart
Yes, the test (as you pointed out) places an arbitrary requirement upon (line 378) with and subsequently replace |
svastm
force-pushed
the
storage_f4
branch
2 times, most recently
from
c5d3e3e
to
1777440
Compare
The new |
| using namespace utest::v1; | ||
|
|
||
| #if defined(TARGET_K64F) | ||
| extern ARM_DRIVER_STORAGE ARM_Driver_Storage_MTD_K64F; |
There was a problem hiding this comment.
@simonqhughes Is this renamed by accident ?
Hi @svastm, I'm curious how you would put the cfstorage in sectors 0 to 3? I thought your bootloader expects the firmware to start in either sector 1 or 2? Or am I reading application note AN2606 wrong? Also, I see the PR passes all checks now. Do you feel it is feature complete or should I wait a bit before using it? Thanks! |
|
@marcuschangarm it was just an idea but you are right; it wouldn't be as easy as I expected and we wouldn't be able to store data in sector 0. As @sg- pointed out, the API may change so I see this PR more like a first implementation. You should maybe wait for the deployment over all the ST families to use it. |
|
@svastm @simonqhughes @rgrover @0xc0170 @sg- @simonfordarm How close is this PR to be good enough for the three boards in question? Can we make a push for getting this into the 5.2 release, please? |
There was a problem hiding this comment.
@simonqhughes, @marcuschangarm : This pull request tries to introduce a config-variable STORAGE_START_SECTOR to define the boundary of the storage space to be managed by the underlying Storage driver. There are two things to consider here:
- Is this a reasonable config variable that we'll support? Does this fit with our general design of Storage/Config-store?
- Is sector-count the correct unit for this variable? An alternative could be start-address.
Please comment.
| const ARM_STORAGE_BLOCK *iter = &block_table[0]; | ||
| uint8_t sector = 0; | ||
| for (size_t index = 0; index < block_table_size; ++index, ++iter) { | ||
| if (sector < STORAGE_START_SECTOR) { |
There was a problem hiding this comment.
I believe the expression in this check is incorrect. It should be:
if ((sector + (iter->size / iter->attributes.erase_unit) <= STORAGE_START_SECTOR) {Please reconsider.
There was a problem hiding this comment.
I don't understand. Does it change the behavior of the loop ?
There was a problem hiding this comment.
I believe the loop has a bug as you've coded it. Imagine that the first block has 2 sectors and STORAGE_START_SECTOR is set to 1; in this case, the loop should terminate at the first block. And in this case, the start_addr of the returned BLOCK should also be adjusted to start at 1 * sector_size; that's missing too.
There was a problem hiding this comment.
Thanks for the example. I forgot the mention that the STORAGE_START_SECTOR have to correspond to the start of a block. It's also required by the erase function. Should we allow STORAGE_START_SECTOR to be non block aligned ? If not, I will add some docs in the storage_driver_info.h
There was a problem hiding this comment.
I'm not sure if STORAGE_START_SECTOR is the correct abstraction to use in this case. I've posted a question to solicit input on this. It may be that STORAGE_START_SECTOR should remain as 0, and therefore implicit, and we use the storage-volume-manager to arbitrate access to partitions within a Storage driver.
I'm hoping @simonqhughes, @marcuschangarm and @sg- would be able to provide input and drive a consensus.
| break; | ||
| } | ||
| } | ||
| return ARM_DRIVER_OK; |
There was a problem hiding this comment.
please note that it is valid to call GetNextBlock(NULL, NULL), if the caller is only interested in establishing the existence of a storage Block. In your code, calling GetNextBlock(NULL, NULL) will return ARM_DRIVER_OK without checking (block_table_size > 1).
There was a problem hiding this comment.
You are right. I'm changing this.
| extern ARM_DRIVER_STORAGE ARM_Driver_Storage_MTD_K64F; | ||
| ARM_DRIVER_STORAGE *drv = &ARM_Driver_Storage_MTD_K64F; | ||
| #else | ||
| extern ARM_DRIVER_STORAGE ARM_Driver_Storage_(0); |
There was a problem hiding this comment.
We should not use ARM_Driver_Storage_(0); it is too generic, and could lead to name-clashes when supporting more than one Storage devices concurrently.
There was a problem hiding this comment.
Ok so I rename the ST one in ARM_Driver_Storage_MTD_STM32
| @@ -371,17 +377,16 @@ control_t test_programDataUsingProgramUnit(const size_t call_count) | |||
| TEST_ASSERT(rc >= 0); | |||
| if (rc == ARM_DRIVER_OK) { | |||
| TEST_ASSERT_EQUAL(1, capabilities.asynchronous_ops); | |||
| return (call_count < REPEAT_INSTANCES) ? CaseTimeout(200) + CaseRepeatAll: CaseTimeout(200); | |||
| return (call_count < REPEAT_INSTANCES) ? CaseTimeout(ERASE_CASE_TIMEOUT) + CaseRepeatAll: CaseTimeout(ERASE_CASE_TIMEOUT); | |||
There was a problem hiding this comment.
You've combined two different things into a single commit. It is better to separate conceptually different changes into distinct commits.
What is the justification for changing the erase Timeouts? Were you hitting this timeout on the ST NVM?
There was a problem hiding this comment.
Yes, i changed the erase timeouts because they were hitten. Typical erase time for a 128kB sector on STM32F429ZI is 1s and for a 16kB sector, 0.25 s.
| @@ -911,7 +916,7 @@ control_t test_programDataWithMultipleProgramUnits(const size_t call_count) | |||
| TEST_ASSERT(rc >= 0); | |||
| if (rc == ARM_DRIVER_OK) { | |||
| TEST_ASSERT_EQUAL(1, capabilities.asynchronous_ops); | |||
| return CaseTimeout(500); | |||
| return CaseTimeout(1000); | |||
There was a problem hiding this comment.
why has this timeout been changed?
There was a problem hiding this comment.
This one was hitten too. I don't remember the N value but with this timeout the test pass.
|
I don't think the functionality provided by Setting the |
|
@marcuschangarm : I agree that the volume manager is better suited for arbitrating access to regions of Storage. In some cases, the Storage driver might want to limit the exposed address-space even further using a config-option like Perhaps @simonqhughes and others would like to comment on what strategy would be suitable for mbedOS. |
svastm
force-pushed
the
storage_f4
branch
2 times, most recently
from
f6bc620
to
14695d2
Compare
|
@rgrover I don't think the RWW problem should be handled by limiting the storage drivers access, but by choosing sensible locations for your configuration store and other libraries that uses NVM. |
Enable storage driver for the following targets: - NUCLEO_F429ZI - DISCO_F429ZI - DISCO_F469NI
- Increase some timeouts - Allow program_unit smaller than 4 bytes
|
Needs a lot of work as to how drivers are composed and used. This is mainly a problem with not following the mbed C hal rather bringing in a new hal. Not sure the solution ATM but this will not fly and goes against all that we're trying to do. |
|
Hello @sg- |
|
@svastm I think this one can be closed? the new replacement is in place. |
|
Hello @0xc0170 |
…..91f3ff6 91f3ff6 Merge branch 'release_internal' into release_external 3999b6e Iotthd 4495 (ARMmbed#2556) 90c3263 RPL Prefix handling update: f761409 Close Nanostack v12.8.0 ChangeLog (ARMmbed#2549) git-subtree-dir: features/nanostack/sal-stack-nanostack git-subtree-split: 91f3ff6
…..0903b81 0903b81 Merge remote-tracking branch 'origin/release_internal' into release_external 51429c9 FHSS WS: api function to set TX allowance level (ARMmbed#2612) 01b1188 Fix Child NUD with long ARO registrations 20b49ce Optimize out of memory handler to remove more memory in EF mode f1b03bc FHSS WS: Allow transmitting unicast frames on broadcast channel for 1st hop node in EF mode (ARMmbed#2609) 2f5e5e2 Generic forwarding callback and EF state enabler for Wi-SUN BBR. 007dfa2 Allow transmitting on RX slot for 1st hop device in expedited forwarding mode (ARMmbed#2607) 6524872 Implemented FHSS expedited forwarding mode (ARMmbed#2606) 91e0b4c QoS traffic class documentation update. 3acd3a4 Fix warnings found by cppcheck (ARMmbed#2605) d2f5347 MPX and MAC API update 7310cc0 MAC: "CCA fail on RX" event for TX done callback (ARMmbed#2602) cd109c3 Clear Ack tx and tx process in MAC reset (ARMmbed#2601) 45504fd Optimize stagger based on uptime and startup type ed5209e Iotthd 4584 (ARMmbed#2599) 60726dc Fixed blacklisting overflow (ARMmbed#2597) 23334b7 Added support for High Priority forward state 3ec2a2c Corrected freed memory access on incoming EAPOL handling aecadc4 Fixed delayed interrupt (ARMmbed#2596) 1fca2c1 CCA backoffs max to 9 (ARMmbed#2595) f3d2fa1 Added API function to get neighbor table information from Wi-SUN 3bb089b Validate randomized fixed channel (ARMmbed#2592) 70743a1 MAC stabilisation fixes (ARMmbed#2591) e936a26 Reduce periodic DNS traces a45fe3f Improved CSMA-CA logic for Wi-SUN (ARMmbed#2585) 56b7735 improved Wi-SUN stack statistics added e656190 Wi-SUN neighbour allocate update 799f837 Added address check for Whiteboard address ADD 0b6caa3 Wi-SUN network timing parameter tuning 4921465 Supress warnings f5cecd7 Enable external connection for routers e129a0a Added LLC EAPOL temporary neighbor remove when authentication completes (ARMmbed#2583) fa20fb9 Added calculation of LLC and LLC EAPOL message queue average (ARMmbed#2582) 7f7c01a Added retry traces to authenticator EAP-TLS, 4WH, and GKH a87646d On startup deletes NVM GTKs if EUI-64 has been changed (ARMmbed#2576) 509a6f9 Add CI commands to PR template (ARMmbed#2579) eb6a4f7 Change stagger calculation to give more bandwidth to application 82f1d54 Wi-SUN bpptstrap clear destination cache at discovery phase. 71b0588 Destination cache update: f92c385 Enabled PMTU timeout to destination cache after used. 957b358 DHCP server and Agent relay update 25b9124 Merge remote-tracking branch 'origin/release_internal' into release_external c825b04 Corrected covery warning on delay_us multiplication be63bbb Updated changelog 77a76c7 Corrected nw size set on automatic mode 65e6c2d Updated unit tests 16e3402 Added waiting queue to EAPOL authentications to Border Router b9c0b7d Wi-SUN border router starts faster in static configuration 2f427e1 Local repair start and stop clear advertised_dodag_membership_since_last_repair when state is updated. 0a01ab1 RPL dio send update dd39963 Wi-SUN Border router uses Static address as DODAGID 7a3c833 Additional check to detect parent connection problem ffe48c9 WS management: domain configuration functions implemented (ARMmbed#2567) 5e9ac4e Added new Callback to RPL indicate Multicast DIS received from RPL Parent 85b949e Bootstrap and EAPOL treats all MAC TX failure causes similarly b57d9bc Add support for anonymous addressing in Wi-SUN border router (ARMmbed#2565) 7400c8b CFG: API for PHY mode id and channel plan id get & validate (ARMmbed#2564) 2832fe8 Added Socket reference limitter 890aad1 Wi-SUN MTU size update and IPv6 minium MTU routing skip 3ad28c1 Added throttling of number of simultaneous EAPOL authentications 0b84299 Source route handler call Wi-Sun border router alive function. c8343b1 Added support for dynamic RPL default lifetime d258068 Iotthd 4478 (ARMmbed#2560) 7ca6c24 Enable and modify memory limits for packet receiving e2b028d Close CHANGELOG.md for v12.8.1 (ARMmbed#2557) 91f3ff6 Merge branch 'release_internal' into release_external 3999b6e Iotthd 4495 (ARMmbed#2556) 90c3263 RPL Prefix handling update: f761409 Close Nanostack v12.8.0 ChangeLog (ARMmbed#2549) f8ae0e9 Merge remote-tracking branch 'origin/release_internal' into release_external 3275f83 Added support for handle RPL hop by Hop sender rank 0. d62c589 Activated RPL force tunnel for wi-sun. 3e1064a RPL tunnel force functionality update 3207e5c RPL parent select timer random update from 1.0-1.2 to 1.0-1.5. bc09cba MAC Ack wait fixed for OFDM (ARMmbed#2552) 5106b1d Fixed unused variable and function warnings. 4096c1a Wi-SUN bootstrap support RPL poison from Connected state to Discovery 66378d1 RPL Poison update git-subtree-dir: features/nanostack/sal-stack-nanostack git-subtree-split: 0903b81
…183d87..0903b81 0903b81 Merge remote-tracking branch 'origin/release_internal' into release_external 51429c9 FHSS WS: api function to set TX allowance level (ARMmbed#2612) 01b1188 Fix Child NUD with long ARO registrations 20b49ce Optimize out of memory handler to remove more memory in EF mode f1b03bc FHSS WS: Allow transmitting unicast frames on broadcast channel for 1st hop node in EF mode (ARMmbed#2609) 2f5e5e2 Generic forwarding callback and EF state enabler for Wi-SUN BBR. 007dfa2 Allow transmitting on RX slot for 1st hop device in expedited forwarding mode (ARMmbed#2607) 6524872 Implemented FHSS expedited forwarding mode (ARMmbed#2606) 91e0b4c QoS traffic class documentation update. 3acd3a4 Fix warnings found by cppcheck (ARMmbed#2605) d2f5347 MPX and MAC API update 7310cc0 MAC: "CCA fail on RX" event for TX done callback (ARMmbed#2602) cd109c3 Clear Ack tx and tx process in MAC reset (ARMmbed#2601) 45504fd Optimize stagger based on uptime and startup type ed5209e Iotthd 4584 (ARMmbed#2599) 60726dc Fixed blacklisting overflow (ARMmbed#2597) 23334b7 Added support for High Priority forward state 3ec2a2c Corrected freed memory access on incoming EAPOL handling aecadc4 Fixed delayed interrupt (ARMmbed#2596) 1fca2c1 CCA backoffs max to 9 (ARMmbed#2595) f3d2fa1 Added API function to get neighbor table information from Wi-SUN 3bb089b Validate randomized fixed channel (ARMmbed#2592) 70743a1 MAC stabilisation fixes (ARMmbed#2591) e936a26 Reduce periodic DNS traces a45fe3f Improved CSMA-CA logic for Wi-SUN (ARMmbed#2585) 56b7735 improved Wi-SUN stack statistics added e656190 Wi-SUN neighbour allocate update 799f837 Added address check for Whiteboard address ADD 0b6caa3 Wi-SUN network timing parameter tuning 4921465 Supress warnings f5cecd7 Enable external connection for routers e129a0a Added LLC EAPOL temporary neighbor remove when authentication completes (ARMmbed#2583) fa20fb9 Added calculation of LLC and LLC EAPOL message queue average (ARMmbed#2582) 7f7c01a Added retry traces to authenticator EAP-TLS, 4WH, and GKH a87646d On startup deletes NVM GTKs if EUI-64 has been changed (ARMmbed#2576) 509a6f9 Add CI commands to PR template (ARMmbed#2579) eb6a4f7 Change stagger calculation to give more bandwidth to application 82f1d54 Wi-SUN bpptstrap clear destination cache at discovery phase. 71b0588 Destination cache update: f92c385 Enabled PMTU timeout to destination cache after used. 957b358 DHCP server and Agent relay update 25b9124 Merge remote-tracking branch 'origin/release_internal' into release_external c825b04 Corrected covery warning on delay_us multiplication be63bbb Updated changelog 77a76c7 Corrected nw size set on automatic mode 65e6c2d Updated unit tests 16e3402 Added waiting queue to EAPOL authentications to Border Router b9c0b7d Wi-SUN border router starts faster in static configuration 2f427e1 Local repair start and stop clear advertised_dodag_membership_since_last_repair when state is updated. 0a01ab1 RPL dio send update dd39963 Wi-SUN Border router uses Static address as DODAGID 7a3c833 Additional check to detect parent connection problem ffe48c9 WS management: domain configuration functions implemented (ARMmbed#2567) 5e9ac4e Added new Callback to RPL indicate Multicast DIS received from RPL Parent 85b949e Bootstrap and EAPOL treats all MAC TX failure causes similarly b57d9bc Add support for anonymous addressing in Wi-SUN border router (ARMmbed#2565) 7400c8b CFG: API for PHY mode id and channel plan id get & validate (ARMmbed#2564) 2832fe8 Added Socket reference limitter 890aad1 Wi-SUN MTU size update and IPv6 minium MTU routing skip 3ad28c1 Added throttling of number of simultaneous EAPOL authentications 0b84299 Source route handler call Wi-Sun border router alive function. c8343b1 Added support for dynamic RPL default lifetime d258068 Iotthd 4478 (ARMmbed#2560) 7ca6c24 Enable and modify memory limits for packet receiving e2b028d Close CHANGELOG.md for v12.8.1 (ARMmbed#2557) 91f3ff6 Merge branch 'release_internal' into release_external 3999b6e Iotthd 4495 (ARMmbed#2556) 90c3263 RPL Prefix handling update: f761409 Close Nanostack v12.8.0 ChangeLog (ARMmbed#2549) f8ae0e9 Merge remote-tracking branch 'origin/release_internal' into release_external 3275f83 Added support for handle RPL hop by Hop sender rank 0. d62c589 Activated RPL force tunnel for wi-sun. 3e1064a RPL tunnel force functionality update 3207e5c RPL parent select timer random update from 1.0-1.2 to 1.0-1.5. bc09cba MAC Ack wait fixed for OFDM (ARMmbed#2552) 5106b1d Fixed unused variable and function warnings. 4096c1a Wi-SUN bootstrap support RPL poison from Connected state to Discovery 66378d1 RPL Poison update git-subtree-dir: connectivity/nanostack/sal-stack-nanostack git-subtree-split: 0903b81
This is a work in progress, please do not merge.
This PR add a first implementation of
STORAGEfeature for STM32F4 targets.For now, only the synchronous version pass the tests.
Concern for now :
TODO:
There is still some points not very clear to me.
Flash Size
Compare to the K64F, the flash on STM32F4 targets is divided in big sectors. example for a
1MBflash :Usually the firmware is stored at the beginning of the flash and the storage at the end. That mean the cfstorage use sectors of
128kB(=ERASE_UNIT). If the space needed by cfstorage does not exceed64kB(no FOTA ?) we could put the cfstorage in the sectors 0 to 3 and the firmware after that ? It would reduceERASE_UNITto16kB.So my question is, is it mandatory to support a big cfstorage size ? Is it meant to be a file system or only to be used to store configurations ? What size should we usually keep for firmware ?
Test timeout
I tried
features-storage-feature_storage-tests-flash_journal-basicapibut it looks like greentea put a60stimeout on test suite. Also the test pass if it is split in two parts. I did not find a way to change timeout so maybe we should consider to split the test ?Asynchronous
If the asynch mode is used, who is responsible for checking that no command is ongoing before starting a new one ? I have added a wait before
read(),program_data()anderase()but it should be handle higher right ? For now, this is not handle in the tests so i think i missed something.Tests
Synchronous
Results
The
features-storage-feature_storage-tests-cfstore-flushfail only on GCC_ARM because the test suite is specific to this toolchain ? I don't know why. The test failed is namedcfstore_flush_test_02_k64f.