Skip to content
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

Flash area functions for listing active and specified type flash memory areas/partitions #33144

Closed

Conversation

de-nordic
Copy link
Collaborator

@de-nordic de-nordic commented Mar 8, 2021

The PR adds:

  • dts bindings for describing type of flash partitions (compatibles)
  • adds FLASH_AREA_TYPE definitions that correspond to new compatibles
  • adds functions to get information on active partition end enumerate partitions by type
  • modifies default flash map to add type of partition

TODO

  • Tests for macros
  • Tests for functions
  • Documentation
  • Sample overlays
  • Modify code that needs to figure out DFU or active partition to use the new API

Resolves #32257

@github-actions github-actions bot added area: API Changes to public APIs area: Boards area: Devicetree area: Devicetree Binding PR modifies or adds a Device Tree binding area: Storage Storage subsystem labels Mar 8, 2021
@de-nordic de-nordic force-pushed the flash-map-area-type branch 3 times, most recently from c8467c9 to 4b4aca1 Compare March 8, 2021 19:19
Copy link
Collaborator

@galak galak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor tweak on the compats, use - instead of , seperator:

zephyr,application-image
zephyr,mcuboot-image
zephyr,mcuboot-scratch

The commit adds bindings for following compatibles:
 zephyr,mcuboot-image
 zephyr,mcuboot-scratch
 zephyr,application-image

These compatobles may be used to specify partitions that are
capable to be used for mcuboot image, mcuboot scratch and application
image respectively.

Signed-off-by: Dominik Ermel <[email protected]>
The flash_area structure has been extended with flash area type filed,
fa_type, that is supposed to have one of FLASH_AREA_TYPE_* predefined
values which represent the purpose of the area.

Signed-off-by: Dominik Ermel <[email protected]>
The commit adds macros that allows to extract flash area type and
translate it from C specific "compatible" identifier, for example
zephyr_application_image, to FLASH_AREA_TYPE_* constants.
The commit modifies defaullt flash_map generator with new macros
to assign the area type to fa_type for each partition.

Signed-off-by: Dominik Ermel <[email protected]>
The is example of zephyr,mcuboot-image, zephyr,mcuboot-scratch and
zaphyr,applicaiton-image usage.
[TODO] Other boards.

Signed-off-by: Dominik Ermel <[email protected]>
The commit adds macros that allow to get information on currently
active flash partition and perform operations on partitions of
selected type.
Following macros have been added in this commit:
 FLASH_AREA_ACTIVE_ID -- returns ID of flash area an active application
  is running from;
 FLASH_AREA_FOR_EACH_COMPATIBLE -- allows to invoke macro on each
  instance of compatible type partition/flash area.

Signed-off-by: Dominik Ermel <[email protected]>
@de-nordic de-nordic force-pushed the flash-map-area-type branch 2 times, most recently from 530016f to 0b4f365 Compare March 10, 2021 13:11
The commit adds:
 flash_area_active_app -- that returns pointer to flash_area occupied
   by active application
 flash_area_enum_type -- that enumerates flash_area for specified type

[TODO] Tests

Signed-off-by: Dominik Ermel <[email protected]>
The flash_area_type sample has been added that presents how user
may utilise flash_area_api, to get information on current active
partition and enumerate list of partitions to access partitions of
specified type.
This sample presents usage of functions:
 flash_area_active
 flash_area_enum_type
and macros
 FLASH_AREA_FOR_EACH_COMPATIBLE

Signed-off-by: Dominik Ermel <[email protected]>
description: |
Partition or other storage area for mcuboot bootloader image.

compatible: "zephyr,mcuboot-image"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason not to use generic identifiers, e.g. just bl-image/bl-scratch, for bootloader areas?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is zephyr/mcuboot specific. Probably other bootloader could be used, or be chained to/in front of the mcuboot, and that booloader would have different compatibility to recognize its partitions.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does not explain why generic partition identifiers cannot be used. MCUboot is also only a Zephyr application.

#define FLASH_AREA_NODE_COMPAT_TYPE(n, c, t) COND_CODE_1(DT_NODE_HAS_COMPAT(n, c), (t), ())

#define FLASH_AREA_NODE_TYPE(n) \
FLASH_AREA_NODE_COMPAT_TYPE(n, zephyr_mcuboot_image, FLASH_AREA_TYPE_MCUBOOT) \
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this is MCUboot specific, should not it be in flash_map_mcuboot.h?

/**
* @brief Return pointer to flash_area occupied by running application.
*
* @return non-NULL pointer to @p flash_area structure holding information on area an application is
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All this long lines here can be reshaped.

description: |
Partition or other storage area for mcuboot bootloader image.

compatible: "zephyr,mcuboot-image"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does not explain why generic partition identifiers cannot be used. MCUboot is also only a Zephyr application.

@@ -67,6 +68,12 @@ struct flash_area {
const char *fa_dev_name;
};

#define FLASH_AREA_TYPE_UNSPECIFIED 0x00 /* No type */
#define FLASH_AREA_TYPE_MCUBOOT 0x01 /* zephyr,mcuboot,image */
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does not look right. Should every bootloader place its ID here?
Disk subsystem offers possibility to find a block device without such hard-coded IDs.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does not look right. Should every bootloader place its ID here?

No, the ID should be added when support for bootloader within Zephyr is added.

Disk subsystem offers possibility to find a block device without such hard-coded IDs.

Yes, but the disk subsystem may use for example names of mount points.
I am trying to avoid bringing the "compatible" identifiers into flash; the space on mcuboot partition becomes tight.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but the disk subsystem may use for example names of mount points.
I am trying to avoid bringing the "compatible" identifiers into flash; the space on mcuboot partition becomes tight.

How the disk subsystem identifiers are formed is debatable and not set in stone. This approach here does not scale and is not generic, like so much around flash.

@de-nordic
Copy link
Collaborator Author

Unfortunately I have lost confidence in the feature or it being required at all: the mcuboot is the only supported bootloader and its support for compile-time resolved image placement is very much hardcoded in common code and would require a lot of rework in mcuboot.
Therefore it I no sense to pursue the PR any further.

@de-nordic de-nordic closed this Mar 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: API Changes to public APIs area: Boards area: Devicetree Binding PR modifies or adds a Device Tree binding area: Devicetree area: Documentation area: Samples Samples area: Storage Storage subsystem
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Common DFU partition enumeration API
3 participants