Skip to content

boot/nxboot: Enhancements to add progress messages and copy-to-RAM #3068

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

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 53 additions & 0 deletions boot/nxboot/Kconfig
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,36 @@ config NXBOOT_BOOTLOADER

if NXBOOT_BOOTLOADER

config NXBOOT_COPY_TO_RAM
bool "Copy bootable image to RAM before calling board boot-image function"
default n
Copy link
Contributor

Choose a reason for hiding this comment

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

Please include a ---help--- explaining when and why to use it, etc

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added

---help---
The is a board and/or arch specific option that may be used when running
directly from flash, especially if external flash, even in XIP mode, is too
slow.
Running from RAM usually results in faster execution but the board must, of
course, have sufficient RAM available for the application image, stack
and heap.

config NXBOOT_RAMSTART
hex "Start address in RAM that the application is to be loaded"
default 0x0
depends on NXBOOT_COPY_TO_RAM
---help---
This will be board specific. A check of the board's linker script
may be informative. For example the SAMA5D2-XULT eval board's uboot
linker script - boards/arm/sama5/sama5d2-xult/scripts/uboot.ld - has:

sdram (W!RX) : ORIGIN = 0x20008000, LENGTH = 256M - 32K

This shows the load address to be 0x20008000 and would be the address
to use here if the same linker script is to be be used for NXboot.

Typically the address is the base address of the RAM to be used, plus the
size of the NXboot image itself. The example above has reserved
32KiB (0x8000) for this from the 256MiB available on the board at
address 0x20000000.

config NXBOOT_SWRESET_ONLY
bool "Perform update/revert only on SW reset"
default n
Expand Down Expand Up @@ -92,6 +122,29 @@ config NXBOOT_PREVENT_DOWNGRADE
WARNING: NXboot currently implements preferences only for
MAJOR.MINOR.PATCH and ignores prerelease.

config NXBOOT_PRINTF_PROGRESS
bool "Enable progress messages to be sent to STDOUT"
default y
---help---
This will display progress during typically lengthy operations:
- Calculating checksums
- copying images between slots

Note: the NXboot binary will be approximately 2KiB larger with this enabled.

choice
prompt "Choose preferred progress indication type"
depends on NXBOOT_PRINTF_PROGRESS
default NXBOOT_PRINTF_PROGRESS_PERCENT

config NXBOOT_PRINTF_PROGRESS_DOTS
bool "Display progress using sequential dots"

config NXBOOT_PRINTF_PROGRESS_PERCENT
bool "Display progress using percentage remaining"

endchoice

endif # NXBOOT_BOOTLOADER

endif # BOOT_NXBOOT
66 changes: 60 additions & 6 deletions boot/nxboot/include/nxboot.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@
* Pre-processor Definitions
****************************************************************************/

#define ERROR -1

#define NXBOOT_PRIMARY_SLOT_NUM (0)
#define NXBOOT_SECONDARY_SLOT_NUM (1)
#define NXBOOT_TERTIARY_SLOT_NUM (2)
Expand Down Expand Up @@ -117,6 +119,7 @@ struct nxboot_img_header

struct nxboot_img_version img_version; /* Image version */
};

static_assert(CONFIG_NXBOOT_HEADER_SIZE > sizeof(struct nxboot_img_header),
"CONFIG_NXBOOT_HEADER_SIZE has to be larger than"
"sizeof(struct nxboot_img_header)");
Expand All @@ -131,10 +134,42 @@ struct nxboot_state
enum nxboot_update_type next_boot; /* nxboot_update_type with next operation */
};

enum progress_type_e
{
nxboot_info = 0, /* Prefixes arg. string with "INFO:" */
nxboot_error, /* Prefixes arg. string with "ERR:" */
nxboot_progress_start, /* Prints arg. string with no newline to allow ..... sequence to follow */
nxboot_progress_dot, /* Prints of a "." to the ..... progress sequence */
nxboot_progress_percent, /* Displays progress as % remaining */
nxboot_progress_end, /* Flags end of a "..." progrees sequence and prints newline */
};

enum progress_msg_e
{
startup_msg = 0,
power_reset,
soft_reset,
found_bootable_image,
no_bootable_image,
boardioc_image_boot_fail,
ramcopy_started,
recovery_revert,
recovery_create,
update_from_update,
validate_primary,
validate_recovery,
validate_update,
recovery_created,
recovery_invalid,
update_failed,
};

/****************************************************************************
* Public Function Prototypes
****************************************************************************/

void nxboot_progress(enum progress_type_e type, ...);
Copy link
Contributor

Choose a reason for hiding this comment

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

Please document this function and its arguments. Primarily, what is expected to be passed in variadic.

And I am not sure if variadic is worth it here. As far as I can see, it is used only to allow progress to be passed or not in case of progress end, and a dot. I would instead define integer and define it for that, for example, where end and dot would have some invalid value such as -1. Secondly, for dot ,it might be interesting to hint also on remaining progress if implementations would prefer more in-depth info over just dots, but that is just an idea.


/****************************************************************************
* Name: nxboot_get_state
*
Expand All @@ -148,7 +183,7 @@ struct nxboot_state
* state: The pointer to nxboot_state structure. The state is stored here.
*
* Returned Value:
* 0 on success, -1 and sets errno on failure.
* OK (0) on success, ERROR (-1) and sets errno on failure.
*
****************************************************************************/

Expand All @@ -164,7 +199,7 @@ int nxboot_get_state(struct nxboot_state *state);
* if afterwards.
*
* Returned Value:
* Valid file descriptor on success, -1 and sets errno on failure.
* Valid file descriptor on success, ERROR (-1) and sets errno on failure.
*
****************************************************************************/

Expand All @@ -180,7 +215,7 @@ int nxboot_open_update_partition(void);
* state of the bootloader.
*
* Returned Value:
* 1 means confirmed, 0 not confirmed, -1 and sets errno on failure.
* 1 if confirmed, OK (0) on success, ERROR (-1) and sets errno on failure.
*
****************************************************************************/

Expand All @@ -194,14 +229,14 @@ int nxboot_get_confirm(void);
* its copy in update partition as a recovery.
*
* Returned Value:
* 0 on success, -1 and sets errno on failure.
* OK (0) on success, ERROR (-1) and sets errno on failure.
*
****************************************************************************/

int nxboot_confirm(void);

/****************************************************************************
* Name: nxboot_perform_swap
* Name: nxboot_perform_update
*
* Description:
* Checks for the possible firmware update and performs it by copying
Expand All @@ -216,10 +251,29 @@ int nxboot_confirm(void);
* check_only: Only repairs corrupted update, but do not start another one
*
* Returned Value:
* 0 on success, -1 and sets errno on failure.
* OK (0) on success, ERROR (-1) and sets errno on failure.
*
****************************************************************************/

int nxboot_perform_update(bool check_only);

/****************************************************************************
* Name: nxboot_ramcopy
*
* Description:
* Copies the (already) validate bootable image to RAM memory
*
* NOTE - no checking that the RAM location is correct, nor that the
* image size is appropriate for that RAM address!
*
* Input parameters:
* none
*
* Returned Value:
* OK (0) on success, ERROR (-1) on fail
*
****************************************************************************/

int nxboot_ramcopy(void);

#endif /* __BOOT_NXBOOT_INCLUDE_NXBOOT_H */
Loading
Loading