From 179a4ea267afee2012cb33eed19ce48b36174371 Mon Sep 17 00:00:00 2001 From: "Ben V. Brown" Date: Tue, 9 May 2023 07:39:25 +1000 Subject: [PATCH 1/6] Extract error codes --- include/blisp.h | 14 +------------- include/error_codes.h | 21 +++++++++++++++++++++ 2 files changed, 22 insertions(+), 13 deletions(-) create mode 100644 include/error_codes.h diff --git a/include/blisp.h b/include/blisp.h index ce85fb1..16cd2e2 100644 --- a/include/blisp.h +++ b/include/blisp.h @@ -3,20 +3,8 @@ #define _LIBBLISP_H #include - #include "blisp_chip.h" - -enum blisp_return { - BLISP_OK = 0, - BLISP_ERR_UNKNOWN = -1, - BLISP_ERR_NO_RESPONSE = -2, - BLISP_ERR_DEVICE_NOT_FOUND = -3, - BLISP_ERR_CANT_OPEN_DEVICE = -4, - // Can't auto-find device due it doesn't have native USB - BLISP_ERR_NO_AUTO_FIND_AVAILABLE = -5, - BLISP_ERR_PENDING = -6, - BLISP_ERR_CHIP_ERR = -7 -}; +#include "error_codes.h" struct blisp_segment_header { uint32_t dest_addr; diff --git a/include/error_codes.h b/include/error_codes.h new file mode 100644 index 0000000..b190dd8 --- /dev/null +++ b/include/error_codes.h @@ -0,0 +1,21 @@ +#ifndef BLISP_S_RC_ERROR_CODES_H_ +#define BLISP_S_RC_ERROR_CODES_H_ + +typedef enum { + BLISP_OK = 0, + // All error states must be <0. + // Generic error return; for when we are unsure what failed + BLISP_ERR_UNKNOWN = -1, + // Device did not respond, if serial link, could be that its not in bootloader + BLISP_ERR_NO_RESPONSE = -2, + // Failed to open a device, likely libusb or permissions + BLISP_ERR_DEVICE_NOT_FOUND = -3, + BLISP_ERR_CANT_OPEN_DEVICE = -4, + // Can't auto-find device due it doesn't have native USB + BLISP_ERR_NO_AUTO_FIND_AVAILABLE = -5, + BLISP_ERR_PENDING = -6, + BLISP_ERR_CHIP_ERR = -7, + BLISP_ERR_INVALID_CHIP_TYPE = -8, + +} blisp_return_t; +#endif \ No newline at end of file From fced48570ac31388f0995fea7fcfed006561f80b Mon Sep 17 00:00:00 2001 From: "Ben V. Brown" Date: Tue, 9 May 2023 07:39:34 +1000 Subject: [PATCH 2/6] Start migration to error codes enum --- lib/blisp.c | 110 ++++++++++++++++++------------------ tools/blisp/src/cmd/write.c | 15 ++--- tools/blisp/src/common.c | 36 +++++++----- 3 files changed, 86 insertions(+), 75 deletions(-) diff --git a/lib/blisp.c b/lib/blisp.c index 0d5882c..479fc9d 100644 --- a/lib/blisp.c +++ b/lib/blisp.c @@ -15,19 +15,20 @@ static void drain(struct sp_port* port) { #if defined(__APPLE__) || defined(__FreeBSD__) - sp_drain(port); + sp_drain(port); #endif } -int32_t blisp_device_init(struct blisp_device* device, - struct blisp_chip* chip) { +blisp_return_t blisp_device_init(struct blisp_device* device, + struct blisp_chip* chip) { device->chip = chip; device->is_usb = false; - return 0; + return BLISP_OK; } -int32_t blisp_device_open(struct blisp_device* device, const char* port_name) { - int ret; +blisp_return_t blisp_device_open(struct blisp_device* device, + const char* port_name) { + blisp_return_t ret; struct sp_port* serial_port = NULL; if (port_name != NULL) { @@ -69,8 +70,7 @@ int32_t blisp_device_open(struct blisp_device* device, const char* port_name) { ret = sp_open(serial_port, SP_MODE_READ_WRITE); if (ret != SP_OK) { blisp_dlog("SP open failed: %d", ret); - return BLISP_ERR_UNKNOWN; // TODO: Maybe this should be that it can't open - // device? + return BLISP_ERR_CANT_OPEN_DEVICE; } // TODO: Handle errors in following functions, although, none of them *should* // fail @@ -107,11 +107,11 @@ int32_t blisp_device_open(struct blisp_device* device, const char* port_name) { return BLISP_OK; } -int32_t blisp_send_command(struct blisp_device* device, - uint8_t command, - void* payload, - uint16_t payload_size, - bool add_checksum) { +blisp_return_t blisp_send_command(struct blisp_device* device, + uint8_t command, + void* payload, + uint16_t payload_size, + bool add_checksum) { int ret; struct sp_port* serial_port = device->serial_port; @@ -141,8 +141,8 @@ int32_t blisp_send_command(struct blisp_device* device, return BLISP_OK; } -int32_t blisp_receive_response(struct blisp_device* device, - bool expect_payload) { +blisp_return_t blisp_receive_response(struct blisp_device* device, + bool expect_payload) { // TODO: Check checksum int ret; struct sp_port* serial_port = device->serial_port; @@ -174,7 +174,8 @@ int32_t blisp_receive_response(struct blisp_device* device, return BLISP_ERR_UNKNOWN; } -int32_t blisp_device_handshake(struct blisp_device* device, bool in_ef_loader) { +blisp_return_t blisp_device_handshake(struct blisp_device* device, + bool in_ef_loader) { int ret; uint8_t handshake_buffer[600]; struct sp_port* serial_port = device->serial_port; @@ -203,8 +204,8 @@ int32_t blisp_device_handshake(struct blisp_device* device, bool in_ef_loader) { } } ret = sp_blocking_write(serial_port, handshake_buffer, bytes_count, 500); - // not sure about Apple part, but FreeBSD needs it - drain(serial_port); + // not sure about Apple part, but FreeBSD needs it + drain(serial_port); if (ret < 0) { blisp_dlog("Handshake write failed, ret %d", ret); return BLISP_ERR_UNKNOWN; @@ -221,15 +222,14 @@ int32_t blisp_device_handshake(struct blisp_device* device, bool in_ef_loader) { return BLISP_OK; } } - } blisp_dlog("Received no response from chip."); return BLISP_ERR_NO_RESPONSE; } -int32_t blisp_device_get_boot_info(struct blisp_device* device, - struct blisp_boot_info* boot_info) { - int ret; +blisp_return_t blisp_device_get_boot_info(struct blisp_device* device, + struct blisp_boot_info* boot_info) { + blisp_return_t ret; ret = blisp_send_command(device, 0x10, NULL, 0, false); if (ret < 0) @@ -240,7 +240,8 @@ int32_t blisp_device_get_boot_info(struct blisp_device* device, return ret; memcpy(boot_info->boot_rom_version, &device->rx_buffer[0], - 4); // TODO: Endianess + 4); // TODO: Endianess; this may break on big endian machines + if (device->chip->type == BLISP_CHIP_BL70X) { memcpy(boot_info->chip_id, &device->rx_buffer[16], 8); } @@ -249,9 +250,9 @@ int32_t blisp_device_get_boot_info(struct blisp_device* device, } // TODO: Use struct instead of uint8_t* -int32_t blisp_device_load_boot_header(struct blisp_device* device, - uint8_t* boot_header) { - int ret; +blisp_return_t blisp_device_load_boot_header(struct blisp_device* device, + uint8_t* boot_header) { + blisp_return_t ret; ret = blisp_send_command(device, 0x11, boot_header, 176, false); if (ret < 0) return ret; @@ -262,10 +263,10 @@ int32_t blisp_device_load_boot_header(struct blisp_device* device, return BLISP_OK; } -int32_t blisp_device_load_segment_header( +blisp_return_t blisp_device_load_segment_header( struct blisp_device* device, struct blisp_segment_header* segment_header) { - int ret; + blisp_return_t ret; ret = blisp_send_command(device, 0x17, segment_header, 16, false); if (ret < 0) return ret; @@ -276,10 +277,10 @@ int32_t blisp_device_load_segment_header( return BLISP_OK; } -int32_t blisp_device_load_segment_data(struct blisp_device* device, - uint8_t* segment_data, - uint32_t segment_data_length) { - int ret; +blisp_return_t blisp_device_load_segment_data(struct blisp_device* device, + uint8_t* segment_data, + uint32_t segment_data_length) { + blisp_return_t ret; ret = blisp_send_command(device, 0x18, segment_data, segment_data_length, false); if (ret < 0) @@ -291,8 +292,8 @@ int32_t blisp_device_load_segment_data(struct blisp_device* device, return BLISP_OK; } -int32_t blisp_device_check_image(struct blisp_device* device) { - int ret; +blisp_return_t blisp_device_check_image(struct blisp_device* device) { + blisp_return_t ret; ret = blisp_send_command(device, 0x19, NULL, 0, false); if (ret < 0) return ret; @@ -303,11 +304,11 @@ int32_t blisp_device_check_image(struct blisp_device* device) { return BLISP_OK; } -int32_t blisp_device_write_memory(struct blisp_device* device, - uint32_t address, - uint32_t value, - bool wait_for_res) { - int ret; +blisp_return_t blisp_device_write_memory(struct blisp_device* device, + uint32_t address, + uint32_t value, + bool wait_for_res) { + blisp_return_t ret; uint8_t payload[8]; *(uint32_t*)(payload) = address; *(uint32_t*)(payload + 4) = value; // TODO: Endianness @@ -323,8 +324,8 @@ int32_t blisp_device_write_memory(struct blisp_device* device, return BLISP_OK; } -int32_t blisp_device_run_image(struct blisp_device* device) { - int ret; +blisp_return_t blisp_device_run_image(struct blisp_device* device) { + blisp_return_t ret; if (device->chip->type == BLISP_CHIP_BL70X) { // ERRATA ret = blisp_device_write_memory(device, 0x4000F100, 0x4E424845, true); @@ -351,14 +352,14 @@ int32_t blisp_device_run_image(struct blisp_device* device) { return BLISP_OK; } -int32_t blisp_device_flash_erase(struct blisp_device* device, - uint32_t start_address, - uint32_t end_address) { +blisp_return_t blisp_device_flash_erase(struct blisp_device* device, + uint32_t start_address, + uint32_t end_address) { uint8_t payload[8]; *(uint32_t*)(payload + 0) = start_address; *(uint32_t*)(payload + 4) = end_address; - int ret = blisp_send_command(device, 0x30, payload, 8, true); + blisp_return_t ret = blisp_send_command(device, 0x30, payload, 8, true); if (ret < 0) return ret; do { @@ -368,17 +369,18 @@ int32_t blisp_device_flash_erase(struct blisp_device* device, return 0; } -int32_t blisp_device_flash_write(struct blisp_device* device, - uint32_t start_address, - uint8_t* payload, - uint32_t payload_size) { +blisp_return_t blisp_device_flash_write(struct blisp_device* device, + uint32_t start_address, + uint8_t* payload, + uint32_t payload_size) { // TODO: Add max payload size (8184?) // TODO: Don't use malloc + add check uint8_t* buffer = malloc(4 + payload_size); *((uint32_t*)(buffer)) = start_address; memcpy(buffer + 4, payload, payload_size); - int ret = blisp_send_command(device, 0x31, buffer, payload_size + 4, true); + blisp_return_t ret = + blisp_send_command(device, 0x31, buffer, payload_size + 4, true); if (ret < 0) goto exit1; ret = blisp_receive_response(device, false); @@ -387,7 +389,7 @@ exit1: return ret; } -int32_t blisp_device_program_check(struct blisp_device* device) { +blisp_return_t blisp_device_program_check(struct blisp_device* device) { int ret = blisp_send_command(device, 0x3A, NULL, 0, true); if (ret < 0) return ret; @@ -395,11 +397,11 @@ int32_t blisp_device_program_check(struct blisp_device* device) { if (ret < 0) return ret; - return 0; + return BLISP_OK; } -int32_t blisp_device_reset(struct blisp_device* device) { - int ret = blisp_send_command(device, 0x21, NULL, 0, true); +blisp_return_t blisp_device_reset(struct blisp_device* device) { + blisp_return_t ret = blisp_send_command(device, 0x21, NULL, 0, true); if (ret < 0) return ret; ret = blisp_receive_response(device, false); diff --git a/tools/blisp/src/cmd/write.c b/tools/blisp/src/cmd/write.c index a35e921..c3b3178 100644 --- a/tools/blisp/src/cmd/write.c +++ b/tools/blisp/src/cmd/write.c @@ -1,14 +1,14 @@ // SPDX-License-Identifier: MIT +#include #include +#include +#include #include #include #include #include "../cmd.h" #include "../common.h" #include "../util.h" -#include -#include -#include #define REG_EXTENDED 1 #define REG_ICASE (REG_EXTENDED << 1) @@ -164,12 +164,13 @@ void fill_up_boot_header(struct bfl_boot_header* boot_header) { boot_header->crc32 = 0xDEADBEEF; } -void blisp_flash_firmware() { +blisp_return_t blisp_flash_firmware() { struct blisp_device device; - int32_t ret; + blisp_return_t ret; + ret = blisp_common_init_device(&device, port_name, chip_type); - if (blisp_common_init_device(&device, port_name, chip_type) != 0) { - return; + if (ret != 0) { + return ret; } if (blisp_common_prepare_flash(&device) != 0) { diff --git a/tools/blisp/src/common.c b/tools/blisp/src/common.c index b3c4725..34d70f4 100644 --- a/tools/blisp/src/common.c +++ b/tools/blisp/src/common.c @@ -3,23 +3,26 @@ #include #include #include -#include #include #include +#include #include #include "blisp_easy.h" +#include "error_codes.h" #include "util.h" -void blisp_common_progress_callback(uint32_t current_value, uint32_t max_value) { +void blisp_common_progress_callback(uint32_t current_value, + uint32_t max_value) { printf("%" PRIu32 "b / %u (%.2f%%)\n", current_value, max_value, (((float)current_value / (float)max_value) * 100.0f)); } -int32_t blisp_common_init_device(struct blisp_device* device, struct arg_str* port_name, struct arg_str* chip_type) -{ +blisp_return_t blisp_common_init_device(struct blisp_device* device, + struct arg_str* port_name, + struct arg_str* chip_type) { if (chip_type->count == 0) { fprintf(stderr, "Chip type is invalid.\n"); - return -1; + return BLISP_ERR_INVALID_CHIP_TYPE; } struct blisp_chip* chip = NULL; @@ -30,23 +33,25 @@ int32_t blisp_common_init_device(struct blisp_device* device, struct arg_str* po chip = &blisp_chip_bl60x; } else { fprintf(stderr, "Chip type is invalid.\n"); - return -1; + return BLISP_ERR_INVALID_CHIP_TYPE; } - int32_t ret; + blisp_return_t ret; ret = blisp_device_init(device, chip); if (ret != BLISP_OK) { fprintf(stderr, "Failed to init device.\n"); - return -1; + return ret; } ret = blisp_device_open(device, port_name->count == 1 ? port_name->sval[0] : NULL); if (ret != BLISP_OK) { - fprintf(stderr, ret == BLISP_ERR_DEVICE_NOT_FOUND ? "Device not found\n" : "Failed to open device.\n"); - return -1; + fprintf(stderr, ret == BLISP_ERR_DEVICE_NOT_FOUND + ? "Device not found\n" + : "Failed to open device.\n"); + return ret; } - return 0; + return BLISP_OK; } /** @@ -93,12 +98,15 @@ int32_t blisp_common_prepare_flash(struct blisp_device* device) { uint8_t* eflash_loader_buffer = NULL; // TODO: Error check - int64_t eflash_loader_buffer_length = device->chip->load_eflash_loader(0, &eflash_loader_buffer); + int64_t eflash_loader_buffer_length = + device->chip->load_eflash_loader(0, &eflash_loader_buffer); struct blisp_easy_transport eflash_loader_transport = - blisp_easy_transport_new_from_memory(eflash_loader_buffer, eflash_loader_buffer_length); + blisp_easy_transport_new_from_memory(eflash_loader_buffer, + eflash_loader_buffer_length); - ret = blisp_easy_load_ram_app(device, &eflash_loader_transport, blisp_common_progress_callback); + ret = blisp_easy_load_ram_app(device, &eflash_loader_transport, + blisp_common_progress_callback); if (ret != BLISP_OK) { fprintf(stderr, "Failed to load eflash_loader, ret: %d\n", ret); From 6e2d40b9c44ea835b9701d14216d95ae3accb404 Mon Sep 17 00:00:00 2001 From: "Ben V. Brown" Date: Tue, 9 May 2023 07:46:45 +1000 Subject: [PATCH 3/6] Stitch more error codes through tool --- include/error_codes.h | 3 ++ tools/blisp/src/cmd.h | 6 ++-- tools/blisp/src/cmd/iot.c | 63 ++++++++++++++++++++----------------- tools/blisp/src/cmd/write.c | 16 +++++----- 4 files changed, 49 insertions(+), 39 deletions(-) diff --git a/include/error_codes.h b/include/error_codes.h index b190dd8..0c94b95 100644 --- a/include/error_codes.h +++ b/include/error_codes.h @@ -16,6 +16,9 @@ typedef enum { BLISP_ERR_PENDING = -6, BLISP_ERR_CHIP_ERR = -7, BLISP_ERR_INVALID_CHIP_TYPE = -8, + BLISP_ERR_OUT_OF_MEMORY = -9, + BLISP_ERR_INVALID_COMMAND = -10, + BLISP_ERR_CANT_OPEN_FILE=-11, } blisp_return_t; #endif \ No newline at end of file diff --git a/tools/blisp/src/cmd.h b/tools/blisp/src/cmd.h index 451b7b6..6d4dbb0 100644 --- a/tools/blisp/src/cmd.h +++ b/tools/blisp/src/cmd.h @@ -3,11 +3,11 @@ #define BLISP_CMD_H #include - +#include "error_codes.h" struct cmd { const char* name; - int8_t (*args_init)(); - uint8_t (*args_parse_exec)(int argc, char** argv); + blisp_return_t (*args_init)(); + blisp_return_t (*args_parse_exec)(int argc, char** argv); void (*args_print_syntax)(); void (*args_free)(); }; diff --git a/tools/blisp/src/cmd/iot.c b/tools/blisp/src/cmd/iot.c index 4f441e1..9137765 100644 --- a/tools/blisp/src/cmd/iot.c +++ b/tools/blisp/src/cmd/iot.c @@ -1,7 +1,7 @@ +#include #include #include "../cmd.h" #include "../common.h" -#include #define REG_EXTENDED 1 #define REG_ICASE (REG_EXTENDED << 1) @@ -9,21 +9,21 @@ static struct arg_rex* cmd; static struct arg_file* single_download; static struct arg_int* single_download_location; -static struct arg_str *port_name, *chip_type; // TODO: Make this common +static struct arg_str *port_name, *chip_type; // TODO: Make this common static struct arg_lit* reset; static struct arg_end* end; static void* cmd_iot_argtable[7]; -void blisp_single_download() -{ +blisp_return_t blisp_single_download() { struct blisp_device device; - int32_t ret; + blisp_return_t ret; - if (blisp_common_init_device(&device, port_name, chip_type) != 0) { - return; + ret = blisp_common_init_device(&device, port_name, chip_type); + if (ret != BLISP_OK) { + return ret; } - - if (blisp_common_prepare_flash(&device) != 0) { + ret = blisp_common_prepare_flash(&device); + if (ret != BLISP_OK) { // TODO: Error handling goto exit1; } @@ -32,6 +32,7 @@ void blisp_single_download() if (data_file == NULL) { fprintf(stderr, "Failed to open data file \"%s\".\n", single_download->filename[0]); + ret = BLISP_ERR_CANT_OPEN_FILE; goto exit1; } fseek(data_file, 0, SEEK_END); @@ -39,9 +40,9 @@ void blisp_single_download() rewind(data_file); printf("Erasing the area, this might take a while...\n"); - ret = - blisp_device_flash_erase(&device, *single_download_location->ival, - *single_download_location->ival + data_file_size + 1); + ret = blisp_device_flash_erase( + &device, *single_download_location->ival, + *single_download_location->ival + data_file_size + 1); if (ret != BLISP_OK) { fprintf(stderr, "Failed to erase.\n"); goto exit2; @@ -51,8 +52,8 @@ void blisp_single_download() struct blisp_easy_transport data_transport = blisp_easy_transport_new_from_file(data_file); - ret = blisp_easy_flash_write(&device, &data_transport, *single_download_location->ival, - data_file_size, + ret = blisp_easy_flash_write(&device, &data_transport, + *single_download_location->ival, data_file_size, blisp_common_progress_callback); if (ret < BLISP_OK) { fprintf(stderr, "Failed to write data to flash.\n"); @@ -67,21 +68,28 @@ void blisp_single_download() } printf("Program OK!\n"); - if (reset->count > 0) { // TODO: could be common - blisp_device_reset(&device); + if (reset->count > 0) { // TODO: could be common printf("Resetting the chip.\n"); + ret = blisp_device_reset(&device); + if (ret != BLISP_OK) { + fprintf(stderr, "Failed to reset chip.\n"); + goto exit2; + } + } + if (ret == BLISP_OK) { + printf("Download complete!\n"); } - - printf("Download complete!\n"); exit2: if (data_file != NULL) fclose(data_file); exit1: blisp_device_close(&device); + + return ret; } -int8_t cmd_iot_args_init() { +blisp_return_t cmd_iot_args_init() { cmd_iot_argtable[0] = cmd = arg_rex1(NULL, NULL, "iot", NULL, REG_ICASE, NULL); cmd_iot_argtable[1] = chip_type = @@ -99,9 +107,9 @@ int8_t cmd_iot_args_init() { if (arg_nullcheck(cmd_iot_argtable) != 0) { fprintf(stderr, "insufficient memory\n"); - return -1; + return BLISP_ERR_OUT_OF_MEMORY; } - return 0; + return BLISP_OK; } void cmd_iot_args_print_glossary() { @@ -111,20 +119,19 @@ void cmd_iot_args_print_glossary() { arg_print_glossary(stdout, cmd_iot_argtable, " %-25s %s\n"); } -uint8_t cmd_iot_parse_exec(int argc, char** argv) { +blisp_return_t cmd_iot_parse_exec(int argc, char** argv) { int errors = arg_parse(argc, argv, cmd_iot_argtable); if (errors == 0) { if (single_download->count == 1 && single_download_location->count == 1) { - blisp_single_download(); - return 1; + return blisp_single_download(); } else { - return 0; + return BLISP_ERR_INVALID_COMMAND; } } else if (cmd->count == 1) { cmd_iot_args_print_glossary(); - return 1; + return BLISP_OK; } - return 0; + return BLISP_ERR_INVALID_COMMAND; } void cmd_iot_args_print_syntax() { @@ -137,4 +144,4 @@ void cmd_iot_free() { } struct cmd cmd_iot = {"iot", cmd_iot_args_init, cmd_iot_parse_exec, - cmd_iot_args_print_syntax, cmd_iot_free}; + cmd_iot_args_print_syntax, cmd_iot_free}; diff --git a/tools/blisp/src/cmd/write.c b/tools/blisp/src/cmd/write.c index c3b3178..5eed5a8 100644 --- a/tools/blisp/src/cmd/write.c +++ b/tools/blisp/src/cmd/write.c @@ -249,7 +249,7 @@ exit1: blisp_device_close(&device); } -int8_t cmd_write_args_init() { +blisp_return_t cmd_write_args_init() { cmd_write_argtable[0] = cmd = arg_rex1(NULL, NULL, "write", NULL, REG_ICASE, NULL); cmd_write_argtable[1] = chip_type = @@ -265,9 +265,9 @@ int8_t cmd_write_args_init() { if (arg_nullcheck(cmd_write_argtable) != 0) { fprintf(stderr, "insufficient memory\n"); - return -1; + return BLISP_ERR_OUT_OF_MEMORY; } - return 0; + return BLISP_OK; } void cmd_write_args_print_glossary() { @@ -277,16 +277,16 @@ void cmd_write_args_print_glossary() { arg_print_glossary(stdout, cmd_write_argtable, " %-25s %s\n"); } -uint8_t cmd_write_parse_exec(int argc, char** argv) { +blisp_return_t cmd_write_parse_exec(int argc, char** argv) { int errors = arg_parse(argc, argv, cmd_write_argtable); if (errors == 0) { - blisp_flash_firmware(); // TODO: Error code? - return 1; + return blisp_flash_firmware(); // TODO: Error code? + } else if (cmd->count == 1) { cmd_write_args_print_glossary(); - return 1; + return BLISP_OK; } - return 0; + return BLISP_ERR_INVALID_COMMAND; } void cmd_write_args_print_syntax() { From 6ec0e9e862f74de31eadfe60bf33f529af22734a Mon Sep 17 00:00:00 2001 From: "Ben V. Brown" Date: Tue, 9 May 2023 07:58:06 +1000 Subject: [PATCH 4/6] Expand blisp_easy to use error codes --- include/error_codes.h | 1 + lib/blisp_easy.c | 43 +++++++++++++++++++++++-------------------- 2 files changed, 24 insertions(+), 20 deletions(-) diff --git a/include/error_codes.h b/include/error_codes.h index 0c94b95..dca65d3 100644 --- a/include/error_codes.h +++ b/include/error_codes.h @@ -19,6 +19,7 @@ typedef enum { BLISP_ERR_OUT_OF_MEMORY = -9, BLISP_ERR_INVALID_COMMAND = -10, BLISP_ERR_CANT_OPEN_FILE=-11, + BLISP_ERR_NOT_IMPLEMENTED=-12, } blisp_return_t; #endif \ No newline at end of file diff --git a/lib/blisp_easy.c b/lib/blisp_easy.c index eef6073..6b40ff8 100644 --- a/lib/blisp_easy.c +++ b/lib/blisp_easy.c @@ -6,12 +6,16 @@ #include #include -static int64_t blisp_easy_transport_read(struct blisp_easy_transport* transport, - void* buffer, - uint32_t size) { +static blisp_return_t blisp_easy_transport_read( + struct blisp_easy_transport* transport, + void* buffer, + uint32_t size) { if (transport->type == 0) { // TODO: Implement reading more than available - memcpy(buffer, (uint8_t*)transport->data.memory.data_location + transport->data.memory.current_position, size); + memcpy(buffer, + (uint8_t*)transport->data.memory.data_location + + transport->data.memory.current_position, + size); transport->data.memory.current_position += size; return size; } else { @@ -19,13 +23,14 @@ static int64_t blisp_easy_transport_read(struct blisp_easy_transport* transport, } } -static int64_t blisp_easy_transport_size(struct blisp_easy_transport* transport) { +static blisp_return_t blisp_easy_transport_size( + struct blisp_easy_transport* transport) { if (transport->type == 0) { return transport->data.memory.data_size; } else { // TODO: Implement - printf("%s() Warning: calling non-implemented function\n", __func__); - return -1; + printf("%s() Warning: calling non-implemented function\n", __func__); + return BLISP_ERR_NOT_IMPLEMENTED; } } @@ -65,7 +70,6 @@ int32_t blisp_easy_load_segment_data( const uint16_t buffer_max_size = 4092; #endif - uint32_t sent_data = 0; uint32_t buffer_size = 0; #ifdef _WIN32 @@ -92,7 +96,7 @@ int32_t blisp_easy_load_segment_data( sent_data += buffer_size; blisp_easy_report_progress(progress_callback, sent_data, segment_size); } - return 0; + return BLISP_OK; } int32_t blisp_easy_load_ram_image( @@ -143,10 +147,10 @@ int32_t blisp_easy_load_ram_image( return BLISP_OK; } -int32_t blisp_easy_load_ram_app(struct blisp_device* device, - struct blisp_easy_transport* app_transport, - blisp_easy_progress_callback progress_callback) -{ +int32_t blisp_easy_load_ram_app( + struct blisp_device* device, + struct blisp_easy_transport* app_transport, + blisp_easy_progress_callback progress_callback) { int32_t ret; // TODO: Rework // region boot header fill @@ -293,7 +297,6 @@ int32_t blisp_easy_load_ram_app(struct blisp_device* device, boot_header.crc32 = 0xDEADBEEF; // endregion - ret = blisp_device_load_boot_header(device, (uint8_t*)&boot_header); if (ret != BLISP_OK) { blisp_dlog("Failed to load boot header, ret: %d.", ret); @@ -304,9 +307,9 @@ int32_t blisp_easy_load_ram_app(struct blisp_device* device, .dest_addr = device->chip->tcm_address, .length = blisp_easy_transport_size(app_transport), .reserved = 0, - .crc32 = 0 - }; - segment_header.crc32 = crc32_calculate(&segment_header, 3 * sizeof(uint32_t)); // TODO: Make function + .crc32 = 0}; + segment_header.crc32 = crc32_calculate( + &segment_header, 3 * sizeof(uint32_t)); // TODO: Make function ret = blisp_device_load_segment_header(device, &segment_header); if (ret != 0) { @@ -314,14 +317,14 @@ int32_t blisp_easy_load_ram_app(struct blisp_device* device, return ret; } - ret = blisp_easy_load_segment_data(device, blisp_easy_transport_size(app_transport), + ret = blisp_easy_load_segment_data(device, + blisp_easy_transport_size(app_transport), app_transport, progress_callback); if (ret != 0) { // TODO: Error printing return ret; } - return BLISP_OK; } @@ -331,7 +334,7 @@ int32_t blisp_easy_flash_write(struct blisp_device* device, uint32_t data_size, blisp_easy_progress_callback progress_callback) { int32_t ret; -#if defined (__APPLE__) || defined (__FreeBSD__) +#if defined(__APPLE__) || defined(__FreeBSD__) const uint16_t buffer_max_size = 372 * 1; #else const uint16_t buffer_max_size = 2052; From bac4f4b49fca7ef70b3f833cd73ca8e62f394f7e Mon Sep 17 00:00:00 2001 From: "Ben V. Brown" Date: Tue, 9 May 2023 07:58:18 +1000 Subject: [PATCH 5/6] Cleanup blisp tool to use error codes --- tools/blisp/src/common.c | 18 ++++++++--------- tools/blisp/src/main.c | 42 +++++++++++++++++++++++----------------- 2 files changed, 32 insertions(+), 28 deletions(-) diff --git a/tools/blisp/src/common.c b/tools/blisp/src/common.c index 34d70f4..b486daa 100644 --- a/tools/blisp/src/common.c +++ b/tools/blisp/src/common.c @@ -58,21 +58,21 @@ blisp_return_t blisp_common_init_device(struct blisp_device* device, * Prepares chip to access flash * this means performing handshake, and loading eflash_loader if needed. */ -int32_t blisp_common_prepare_flash(struct blisp_device* device) { - int32_t ret = 0; +blisp_return_t blisp_common_prepare_flash(struct blisp_device* device) { + blisp_return_t ret = 0; printf("Sending a handshake...\n"); ret = blisp_device_handshake(device, false); if (ret != BLISP_OK) { fprintf(stderr, "Failed to handshake with device.\n"); - return -1; + return ret; } printf("Handshake successful!\nGetting chip info...\n"); struct blisp_boot_info boot_info; ret = blisp_device_get_boot_info(device, &boot_info); if (ret != BLISP_OK) { fprintf(stderr, "Failed to get boot info.\n"); - return -1; + return ret; } printf( @@ -85,7 +85,7 @@ int32_t blisp_common_prepare_flash(struct blisp_device* device) { boot_info.chip_id[6], boot_info.chip_id[7]); if (device->chip->load_eflash_loader == NULL) { - return 0; + return BLISP_OK; } if (boot_info.boot_rom_version[0] == 255 && @@ -93,7 +93,7 @@ int32_t blisp_common_prepare_flash(struct blisp_device* device) { boot_info.boot_rom_version[2] == 255 && boot_info.boot_rom_version[3] == 255) { printf("Device already in eflash_loader.\n"); - return 0; + return BLISP_OK; } uint8_t* eflash_loader_buffer = NULL; @@ -110,7 +110,7 @@ int32_t blisp_common_prepare_flash(struct blisp_device* device) { if (ret != BLISP_OK) { fprintf(stderr, "Failed to load eflash_loader, ret: %d\n", ret); - ret = -1; + goto exit1; } @@ -120,14 +120,12 @@ int32_t blisp_common_prepare_flash(struct blisp_device* device) { ret = blisp_device_check_image(device); if (ret != 0) { fprintf(stderr, "Failed to check image.\n"); - ret = -1; goto exit1; } ret = blisp_device_run_image(device); if (ret != BLISP_OK) { fprintf(stderr, "Failed to run image.\n"); - ret = -1; goto exit1; } @@ -135,12 +133,12 @@ int32_t blisp_common_prepare_flash(struct blisp_device* device) { ret = blisp_device_handshake(device, true); if (ret != BLISP_OK) { fprintf(stderr, "Failed to handshake with device.\n"); - ret = -1; goto exit1; } printf("Handshake with eflash_loader successful.\n"); exit1: if (eflash_loader_buffer != NULL) free(eflash_loader_buffer); + return ret; } \ No newline at end of file diff --git a/tools/blisp/src/main.c b/tools/blisp/src/main.c index 86aaa7a..566b518 100644 --- a/tools/blisp/src/main.c +++ b/tools/blisp/src/main.c @@ -14,7 +14,7 @@ static struct arg_lit* version; static struct arg_end* end; static void* argtable[3]; -int8_t args_init() { +blisp_return_t args_init() { argtable[0] = help = arg_lit0(NULL, "help", "print this help and exit"); argtable[1] = version = arg_lit0(NULL, "version", "print version information and exit"); @@ -22,10 +22,10 @@ int8_t args_init() { if (arg_nullcheck(argtable) != 0) { fprintf(stderr, "insufficient memory\n"); - return -1; + return BLISP_ERR_OUT_OF_MEMORY; } - return 0; + return BLISP_OK; } void print_help() { @@ -43,14 +43,14 @@ int8_t args_parse_exec(int argc, char** argv) { if (error == 0) { if (help->count) { print_help(); - return 1; + return BLISP_OK; } else if (version->count) { printf("blisp v0.0.3\n"); printf("Copyright (C) 2023 Marek Kraus and PINE64 Community\n"); - return 1; + return BLISP_OK; } } - return 0; + return BLISP_ERR_INVALID_COMMAND; } void args_free() { @@ -58,27 +58,29 @@ void args_free() { } int main(int argc, char** argv) { - int exit_code = 0; - - if (args_init() != 0) { - exit_code = -1; + blisp_return_t ret = args_init(); + if (ret != 0) { goto exit; } for (uint8_t i = 0; i < cmds_count; i++) { - if (cmds[i]->args_init() != 0) { - exit_code = -1; + ret = cmds[i]->args_init(); + if (ret != BLISP_OK) { + goto exit; + } + } + // Try and parse as a help request + { + ret = args_parse_exec(argc, argv); + if (ret == BLISP_OK) { goto exit; } } - if (args_parse_exec(argc, argv)) { - goto exit; - } - uint8_t command_found = false; for (uint8_t i = 0; i < cmds_count; i++) { - if (cmds[i]->args_parse_exec(argc, argv)) { + ret = cmds[i]->args_parse_exec(argc, argv); + if (ret != BLISP_ERR_INVALID_COMMAND) { command_found = true; break; } @@ -93,5 +95,9 @@ exit: cmds[i]->args_free(); } args_free(); - return exit_code; + // Make error codes more intuitive, but converting to +ve mirror + if (ret < 0) { + ret = -ret; + } + return ret; } From 5306720e5da051425532702d9f2769e201b8d8ee Mon Sep 17 00:00:00 2001 From: "Ben V. Brown" Date: Tue, 9 May 2023 08:05:38 +1000 Subject: [PATCH 6/6] More annotations on errors --- include/error_codes.h | 25 +++++++++++++++---------- lib/blisp.c | 14 +++++++------- 2 files changed, 22 insertions(+), 17 deletions(-) diff --git a/include/error_codes.h b/include/error_codes.h index dca65d3..c30d283 100644 --- a/include/error_codes.h +++ b/include/error_codes.h @@ -6,20 +6,25 @@ typedef enum { // All error states must be <0. // Generic error return; for when we are unsure what failed BLISP_ERR_UNKNOWN = -1, - // Device did not respond, if serial link, could be that its not in bootloader + // Device did not respond, if serial link, could be that its not in boot + // loader BLISP_ERR_NO_RESPONSE = -2, // Failed to open a device, likely libusb or permissions - BLISP_ERR_DEVICE_NOT_FOUND = -3, - BLISP_ERR_CANT_OPEN_DEVICE = -4, + BLISP_ERR_DEVICE_NOT_FOUND = -3, // We could not find a device + BLISP_ERR_CANT_OPEN_DEVICE = + -4, // Couldn't open device; could it be permissions or its in use? // Can't auto-find device due it doesn't have native USB BLISP_ERR_NO_AUTO_FIND_AVAILABLE = -5, - BLISP_ERR_PENDING = -6, - BLISP_ERR_CHIP_ERR = -7, - BLISP_ERR_INVALID_CHIP_TYPE = -8, - BLISP_ERR_OUT_OF_MEMORY = -9, - BLISP_ERR_INVALID_COMMAND = -10, - BLISP_ERR_CANT_OPEN_FILE=-11, - BLISP_ERR_NOT_IMPLEMENTED=-12, + BLISP_ERR_PENDING = -6, // Internal error for device is busy and to come back + BLISP_ERR_CHIP_ERR = -7, // Chip returned an error to us + BLISP_ERR_INVALID_CHIP_TYPE = -8, // unsupported chip type provided + BLISP_ERR_OUT_OF_MEMORY = + -9, // System could not allocate enough ram (highly unlikely) + BLISP_ERR_INVALID_COMMAND = -10, // Invalid user command provided + BLISP_ERR_CANT_OPEN_FILE = -11, // Cant open the firmware file to flash + BLISP_ERR_NOT_IMPLEMENTED = -12, // Non implemented function called + BLISP_ERR_API_ERROR = -13, // Errors outside our control from api's we + // integrate (Generally serial port/OS related) } blisp_return_t; #endif \ No newline at end of file diff --git a/lib/blisp.c b/lib/blisp.c index 479fc9d..b3d8f42 100644 --- a/lib/blisp.c +++ b/lib/blisp.c @@ -45,7 +45,7 @@ blisp_return_t blisp_device_open(struct blisp_device* device, ret = sp_list_ports(&port_list); if (ret != SP_OK) { blisp_dlog("Couldn't list ports, err: %d", ret); - return BLISP_ERR_UNKNOWN; + return BLISP_ERR_DEVICE_NOT_FOUND; } for (int i = 0; port_list[i] != NULL; i++) { struct sp_port* port = port_list[i]; @@ -99,8 +99,8 @@ blisp_return_t blisp_device_open(struct blisp_device* device, #endif ret = sp_set_baudrate(serial_port, device->current_baud_rate); if (ret != SP_OK) { - blisp_dlog("Set baud rate failed: %d... Also hello macOS user :)", ret); - return BLISP_ERR_UNKNOWN; + blisp_dlog("Set baud rate failed: %d... Also hello MacOS user :)", ret); + return BLISP_ERR_API_ERROR; } device->serial_port = serial_port; @@ -134,7 +134,7 @@ blisp_return_t blisp_send_command(struct blisp_device* device, sp_blocking_write(serial_port, device->tx_buffer, 4 + payload_size, 1000); if (ret != (4 + payload_size)) { blisp_dlog("Received error or not written all data: %d", ret); - return BLISP_ERR_UNKNOWN; + return BLISP_ERR_API_ERROR; } drain(serial_port); @@ -149,7 +149,7 @@ blisp_return_t blisp_receive_response(struct blisp_device* device, ret = sp_blocking_read(serial_port, &device->rx_buffer[0], 2, 1000); if (ret < 2) { blisp_dlog("Failed to receive response, ret: %d", ret); - return BLISP_ERR_UNKNOWN; // TODO: Terrible + return BLISP_ERR_NO_RESPONSE; } else if (device->rx_buffer[0] == 'O' && device->rx_buffer[1] == 'K') { if (expect_payload) { sp_blocking_read(serial_port, &device->rx_buffer[2], 2, @@ -171,7 +171,7 @@ blisp_return_t blisp_receive_response(struct blisp_device* device, } blisp_dlog("Failed to receive any response (err: %d, %d - %d)", ret, device->rx_buffer[0], device->rx_buffer[1]); - return BLISP_ERR_UNKNOWN; + return BLISP_ERR_NO_RESPONSE; } blisp_return_t blisp_device_handshake(struct blisp_device* device, @@ -208,7 +208,7 @@ blisp_return_t blisp_device_handshake(struct blisp_device* device, drain(serial_port); if (ret < 0) { blisp_dlog("Handshake write failed, ret %d", ret); - return BLISP_ERR_UNKNOWN; + return BLISP_ERR_API_ERROR; } if (!in_ef_loader && !device->is_usb) {