Skip to content

Commit

Permalink
mimxrt/mboot: Fixes code review findings.
Browse files Browse the repository at this point in the history
Fixes code formatting with curly brackets on `for` loops and adds
`void` argument to empty function declarations and definitions.

Signed-off-by: Philipp Ebensberger
  • Loading branch information
alphaFred committed Jul 18, 2022
1 parent c665595 commit 8d8c7de
Show file tree
Hide file tree
Showing 6 changed files with 23 additions and 28 deletions.
1 change: 1 addition & 0 deletions ports/mimxrt/mboot.mk
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ CFLAGS_BL += \
-nostdlib \
-std=c99 \
-Wall \
-Werror \
-Wdouble-promotion \
-Werror \
-Wfloat-conversion \
Expand Down
16 changes: 7 additions & 9 deletions ports/mimxrt/mboot/main.c
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ extern uint32_t __firmware_start; // Linker symbol
// --------------------------------------------------------------------+
// External Function Declarations
// --------------------------------------------------------------------+
extern void SystemInit();
extern void SystemInit(void);

// --------------------------------------------------------------------+
// Local Function Declarations
Expand All @@ -94,8 +94,7 @@ int main(void) {
}

// Fatal error occured - wait for power cycle
for (;;)
{
for (;;) {
__asm__ ("nop");
}

Expand All @@ -105,7 +104,7 @@ int main(void) {
// --------------------------------------------------------------------+
// Local Function Definitions
// --------------------------------------------------------------------+
static bool reprog_request() {
static bool reprog_request(void) {
bool reprogramming_requested = false;

if ((bl_command.magic == BOOT_COMMAND_MAGIC) && (bl_command.command == BOOT_COMMAND_RESET_DFU)) {
Expand All @@ -121,7 +120,7 @@ static bool reprog_request() {
return reprogramming_requested;
}

static bool valid_fw_available() {
static bool valid_fw_available(void) {
fw_header_t *fw_header = (fw_header_t *)MEM_GET_SYMBOL_VALUE(__firmware_start);
uintptr_t start_addr = ((uintptr_t)MEM_GET_SYMBOL_VALUE(__firmware_start)) + sizeof(fw_header_t);
mboot_validate_status_t fw_status = mboot_validate_firmware(fw_header, start_addr);
Expand All @@ -135,18 +134,17 @@ static inline firmware_entry_func get_entry_func() {
return entry_func;
}

static inline void fw_start() {
static inline void fw_start(void) {
firmware_entry_func entry_func = get_entry_func();

entry_func();

for (;;)
{
for (;;) {
__asm__ ("nop");
}
}

static void board_init() {
static void board_init(void) {
// Init clock
BOARD_BootClockRUN();
SystemCoreClockUpdate();
Expand Down
2 changes: 1 addition & 1 deletion ports/mimxrt/mboot/mboot_buffer.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@
// --------------------------------------------------------------------+
// Global Function Declarations
// --------------------------------------------------------------------+
void mboot_buffer_init();
void mboot_buffer_init(void);
size_t mboot_buffer_append(const uint8_t *data, size_t length);
void mboot_buffer_pad(void);
void mboot_buffer_reset(void);
Expand Down
12 changes: 6 additions & 6 deletions ports/mimxrt/mboot/mboot_upgrade.c
Original file line number Diff line number Diff line change
Expand Up @@ -74,16 +74,16 @@ extern uintptr_t __flash_start;
// --------------------------------------------------------------------+
// Local Function Declarations
// --------------------------------------------------------------------+
static mboot_upgrade_status_t write_initial_block_handler();
static mboot_upgrade_status_t write_block_handler();
static mboot_upgrade_status_t write_initial_block_handler(void);
static mboot_upgrade_status_t write_block_handler(void);
static mboot_upgrade_status_t write_block(uint8_t *data, uint32_t length, uintptr_t *eaddr, uintptr_t *waddr);

// --------------------------------------------------------------------+
// Global Function Definitions
// --------------------------------------------------------------------+

// Todo: Add configuration structure
mboot_upgrade_status_t mboot_upgrade_init() {
mboot_upgrade_status_t mboot_upgrade_init(void) {
flash_init();

// Initialize firmware upgrade session
Expand All @@ -109,7 +109,7 @@ mboot_upgrade_status_t mboot_upgrade_init() {
return BU_STATUS_OK;
}

mboot_upgrade_status_t mboot_upgrade_deinit() {
mboot_upgrade_status_t mboot_upgrade_deinit(void) {
// De-initialize firmware upgrade session
session.n_blocks = 0UL;
session.n_bytes_written = 0UL;
Expand Down Expand Up @@ -213,7 +213,7 @@ mboot_upgrade_status_t mboot_upgrade_validate() {
// --------------------------------------------------------------------+
// Local Function Definitions
// --------------------------------------------------------------------+
static mboot_upgrade_status_t write_initial_block_handler() {
static mboot_upgrade_status_t write_initial_block_handler(void) {
mboot_upgrade_status_t ret_status = BU_STATUS_UNKNOWN_ERROR;
uint8_t *data = mboot_buffer_data_ptr();
uint32_t length = mboot_buffer_len();
Expand Down Expand Up @@ -253,7 +253,7 @@ static mboot_upgrade_status_t write_initial_block_handler() {
return ret_status;
}

static mboot_upgrade_status_t write_block_handler() {
static mboot_upgrade_status_t write_block_handler(void) {
mboot_upgrade_status_t ret_status = BU_STATUS_UNKNOWN_ERROR;
uint8_t *data = mboot_buffer_data_ptr();
uint32_t length = mboot_buffer_len();
Expand Down
8 changes: 4 additions & 4 deletions ports/mimxrt/mboot/mboot_upgrade.h
Original file line number Diff line number Diff line change
Expand Up @@ -45,12 +45,12 @@ typedef enum _mboot_upgrade_status_t {
/**
* Initializes firmware upgrade session base don selected configuration.
*/
mboot_upgrade_status_t mboot_upgrade_init();
mboot_upgrade_status_t mboot_upgrade_init(void);

/**
* De-initializes firmware upgrade session.
*/
mboot_upgrade_status_t mboot_upgrade_deinit();
mboot_upgrade_status_t mboot_upgrade_deinit(void);

/**
* Write block of data to memory.
Expand All @@ -63,13 +63,13 @@ mboot_upgrade_status_t mboot_upgrade_write_data(uint16_t block_num, const uint8_
* Finalize firmware upgrade by:
* - flushing remaining data from buffer and write to memory
**/
mboot_upgrade_status_t mboot_upgrade_finalize();
mboot_upgrade_status_t mboot_upgrade_finalize(void);

/**
* Check and validate firmware upgrade:
* - check written data
* - valdiate firmware image if check ok
**/
mboot_upgrade_status_t mboot_upgrade_validate();
mboot_upgrade_status_t mboot_upgrade_validate(void);

#endif // MICROPY_INCLUDED_MIMXRT_MBOOT_UPGRADE_H
12 changes: 4 additions & 8 deletions ports/mimxrt/mboot/mboot_validate.c
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,7 @@ mboot_validate_status_t mboot_validate_header(fw_header_t *img_header) {
static uint8_t reflect8(uint8_t val) {
uint8_t resVal = 0;

for (int i = 0; i < 8; i++)
{
for (int i = 0; i < 8; i++) {
if ((val & (1 << i)) != 0) {
resVal |= (uint8_t)(1 << (7 - i));
}
Expand All @@ -97,8 +96,7 @@ static uint8_t reflect8(uint8_t val) {
static uint32_t reflect32(uint32_t val) {
uint32_t resVal = 0;

for (int i = 0; i < 32; i++)
{
for (int i = 0; i < 32; i++) {
if ((val & (1 << i)) != 0) {
resVal |= (uint32_t)(1 << (31 - i));
}
Expand All @@ -110,14 +108,12 @@ static uint32_t reflect32(uint32_t val) {
uint32_t crc32(const uint8_t *data, size_t len) {
uint32_t crc = crc_initial;

for (int i = 0; i < len; ++i)
{
for (int i = 0; i < len; ++i) {
uint8_t curByte = reflect8(data[i]);

crc ^= (uint32_t)(curByte << 24); /* move byte into MSB of 32bit CRC */

for (int i = 0; i < 8; i++)
{
for (int i = 0; i < 8; i++) {
if ((crc & 0x80000000) != 0) { /* test for MSB = bit 31 */
crc = (uint32_t)((crc << 1) ^ crc_polynomial);
} else {
Expand Down

0 comments on commit 8d8c7de

Please sign in to comment.