zephyr: Add support for automatically calculcating max sectors #2019
Conversation
|
How it looks on a device with large (different) sector sizes (stm32f411e_disco): It builds and default remains at 128 |
|
@butok interestingly it looks like the nxp ones were a bit optimistic...
This means 0xc000 is needed to store the update data but you have actually reserved 0x62000... |
Adds a feature that will calculate the maximum number of sectors that are needed for a build. Can be disabled to revert back to the old behaviour by disabling CONFIG_BOOT_MAX_IMG_SECTORS_AUTO Signed-off-by: Jamie McCrae <jamie.mccrae@nordicsemi.no>
Adds a note about the new feature that has been added Signed-off-by: Jamie McCrae <jamie.mccrae@nordicsemi.no>
nordicjm
force-pushed
the
sectorcountguess
branch
from
fbe13ca
to
fc6ca4b
Compare
Is it bad or good? |
Updated the comment just as you posted:
That's 344KiB of wasted space |
The board has 64MB, so the configuration value was set to have a bigger coverage. |
| dt_prop(slot0_size PATH "${slot0_flash}" PROPERTY "reg" INDEX 1) | ||
| dt_get_parent(slot0_flash) | ||
| dt_get_parent(slot0_flash) | ||
| dt_prop(erase_size_slot0 PATH "${slot0_flash}" PROPERTY "erase-block-size") |
There was a problem hiding this comment.
I kinda do not like that we get to the Flash device layer to get this information as I think that we start to pick information that we should not access directly from scripts, but unless the DTS parsing script does not start definitions for info like number of pages per partitions this is the only way. Still this makes changes to dts harder in the future.
There was a problem hiding this comment.
The other problem is that I don't believe the code cares what this property is even set to. The flash api directly asks the driver about the erase block size. And that API is needlessly complicated to allow for devices with variable sized sectors, even though the only reasonable thing to do with them in mcuboot is treat the device as if all of the sectors were the size of the largest one.
There was a problem hiding this comment.
What this means is that if this property is at a mismatch with the underlying device, the calculation will be wrong. For example, people have reported trying to increase the erase size in the DT, to discover it didn't work, as the code still asked the device what the real sector size is.
I think we should look into changing things so that mcuboot uses this property only to determine the erase-block-size. This will make it easier to support multiple devices by just setting the value to the largest size.
There was a problem hiding this comment.
For variably sized sectors, like some stm32 parts, that property is not set and this will give the warning then use the existing default of 128 instead, I did give it a try
No description provided.