-
Notifications
You must be signed in to change notification settings - Fork 709
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
Multi flash #359
Multi flash #359
Conversation
@d3zd3z If you can do some review of the rust code here I'd be glad (you can skip the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a comment about the map type. The rust part looks fine.
sim/mcuboot-sys/src/api.rs
Outdated
|
||
lazy_static! { | ||
/// Maintain a table of [device_id -> (align, erased_val)] | ||
static ref FLASH_PARAMS: Mutex<HashMap<u8, (u8, u8)>> = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm kind of on the fence as to whether this map should map to a struct with two fields rather than a general tuple. It does seem to currently only be used below, so isn't that great of an issue. The compiler will generate the same code with a struct defined.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh yeah, it definitely improves a lot the legibility; I squashed the fix into the original commit. Thanks!
Copying some of last commit's message:
I am fairly confident (or hopeful?) that this is 100% functional now and not breaking the swap ugprades. I have not tested at all on Zephyr (but expect no problems there). @d3zd3z @carlescufi @nvlsianpu Feel free to add more reviewers if you feel it might be helpful! @d3zd3z Thanks for the early review! |
3da566a
to
f7d848a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm in progress with the review.
@hakonfam @oyvindronningstad can you please take a look at this PR? |
gcc create has become deprecated: https://crates.io/crates/gcc Signed-off-by: Fabio Utzig <utzig@apache.org>
Update `flash_area_*()` functions to call `sim_flash_*()` directly instead of using `hal_flash_*()` functions that were not part of the main bootloader anymore. Signed-off-by: Fabio Utzig <utzig@apache.org>
A new align() function was added to SimFlash, and most functions that were using/receiving align or erased_val parameters that had access to a Flash trait were cleaned up so that they get the parameters directly from the Flash device. This will make it easier to extend for multiple Flash devices since parameters should depend on the device in use. Signed-off-by: Fabio Utzig <utzig@apache.org>
The previous c/rust ffi functions were hardcoding the values of align and erased_val before each run through static globals. This adds new sim flash functions that get the align/erased_val from the sim flash device that is being run on, allowing that later multiple flash devices can each use its own params. Signed-off-by: Fabio Utzig <utzig@apache.org>
AreaDesc was modified to not receive a flash device on its constructor, and instead a new function `add_flash_sectors` was added that allows it to receive a flash device and id. The `add_image` function that populates the areas also now receives a dev_id that is used internally as fa_device_id. Signed-off-by: Fabio Utzig <utzig@apache.org>
A new type `FlashMap` that stores a HashMap of [device_id -> Flash trait] was added, which enables multi-flash devices to be passed around. The previously existing static FLASH value that was used to simulate the raw device, was updated to using a FlashMap which enables bootutil to interface with more than one flash device. Signed-off-by: Fabio Utzig <utzig@apache.org>
This adds an initial device with multiple flash (nrf52840 + SPI flash) and updates all test routines to use a HashMap of flash devices (added as type SimFlashMap). Signed-off-by: Fabio Utzig <utzig@apache.org>
This adds an external SPI flash that uses a larger sector size than the internal flash. Currently this breaks the tests but it's being added here to trigger a CI fail that will be fixed by adding support for this feature in a subsequent commit. Signed-off-by: Fabio Utzig <utzig@apache.org>
Signed-off-by: Fabio Utzig <utzig@apache.org>
This adds bootutil support for slots on different flash devices the happen to have different sector sizes. It consists basically in relaxing the `boot_slots_compatible` to allow swaps as long as the sectors that are required to fit both images are able to fit inside scratch and both slot's sectors have sizes that are multiple of each other. This is now tested on the simulator and was tested in a Nordic's pca10056 using slot0 in internal flash, and slot1 in the external QSPI flash, configured with 4K, 8K and 16K sized sectors (the HW is 4KB but Mynewt allows emulating multiples of that!) Signed-off-by: Fabio Utzig <utzig@apache.org>
@utzig is there an example nrf52840_pca10056.dts with slot1 and scratch on the external QSPI flash? or was this checked on mynewt only? |
@markus-becker-tridonic-com Unfortunately I never tried using the external flash on Zephyr, and I am not sure it is actually supported. For Mynewt it is available with a just few |
@utzig there is zephyrproject-rtos/zephyr#9873 and zephyrproject-rtos/zephyr#7561 which might add support for that chip. |
No description provided.