diff --git a/lib/pbio/drv/block_device/block_device_ev3.c b/lib/pbio/drv/block_device/block_device_ev3.c index 635a06104..72c429361 100644 --- a/lib/pbio/drv/block_device/block_device_ev3.c +++ b/lib/pbio/drv/block_device/block_device_ev3.c @@ -20,13 +20,6 @@ #include #include -#include "../core.h" - -#include -#include -#include -#include - #include #include #include @@ -36,9 +29,15 @@ #include #include -#include "block_device_ev3.h" +#include "../core.h" #include "../drv/gpio/gpio_ev3.h" +#include "../sys/storage_data.h" +#include "block_device_ev3.h" +#include +#include +#include +#include #include #include #include @@ -831,7 +830,11 @@ static struct { * portion of this, up to pbdrv_block_device_get_writable_size() bytes, * gets saved to flash at shutdown. */ - uint8_t data[PBDRV_CONFIG_BLOCK_DEVICE_RAM_SIZE]; + union { + // ensure that data is properly aligned for pbsys_storage_data_map_t + pbsys_storage_data_map_t data_map; + uint8_t data[PBDRV_CONFIG_BLOCK_DEVICE_RAM_SIZE]; + }; } ramdisk __attribute__((section(".noinit"), used)); uint32_t pbdrv_block_device_get_writable_size(void) { @@ -840,8 +843,8 @@ uint32_t pbdrv_block_device_get_writable_size(void) { static pbio_error_t pbdrv_block_device_load_err = PBIO_ERROR_FAILED; -pbio_error_t pbdrv_block_device_get_data(uint8_t **data) { - *data = ramdisk.data; +pbio_error_t pbdrv_block_device_get_data(pbsys_storage_data_map_t **data) { + *data = &ramdisk.data_map; // Higher level code can use the ramdisk data if initialization completed // successfully. Otherwise it should reset to factory default data. diff --git a/lib/pbio/drv/block_device/block_device_flash_stm32.c b/lib/pbio/drv/block_device/block_device_flash_stm32.c index 20e3b6220..696bbb9f6 100644 --- a/lib/pbio/drv/block_device/block_device_flash_stm32.c +++ b/lib/pbio/drv/block_device/block_device_flash_stm32.c @@ -10,9 +10,8 @@ #include #include -#include - #include "../core.h" +#include "../sys/storage_data.h" #include @@ -41,7 +40,11 @@ static struct { * portion of this, up to pbdrv_block_device_get_writable_size() bytes, * gets saved to flash at shutdown. */ - uint8_t data[PBDRV_CONFIG_BLOCK_DEVICE_RAM_SIZE]; + union { + // ensure that data is properly aligned for pbsys_storage_data_map_t + pbsys_storage_data_map_t data_map; + uint8_t data[PBDRV_CONFIG_BLOCK_DEVICE_RAM_SIZE]; + }; } ramdisk __attribute__((section(".noinit"), used)); const uint32_t header_size = sizeof(ramdisk.saved_size) + sizeof(ramdisk.checksum_complement); @@ -70,8 +73,8 @@ void pbdrv_block_device_init(void) { memcpy(&ramdisk, _pbdrv_block_device_storage_start, size); } -pbio_error_t pbdrv_block_device_get_data(uint8_t **data) { - *data = ramdisk.data; +pbio_error_t pbdrv_block_device_get_data(pbsys_storage_data_map_t **data) { + *data = &ramdisk.data_map; return init_err; } diff --git a/lib/pbio/drv/block_device/block_device_test.c b/lib/pbio/drv/block_device/block_device_test.c index 496aa49e8..572c8eb20 100644 --- a/lib/pbio/drv/block_device/block_device_test.c +++ b/lib/pbio/drv/block_device/block_device_test.c @@ -10,8 +10,9 @@ #include #include -#include +#include "../sys/storage_data.h" +#include #include /** @@ -59,23 +60,20 @@ static const uint8_t _program_data[] = { // version at the right place. FIXME: Move the git version to pybricks build // system, since it isn't actually the micropython git version. #include "genhdr/mpversion.h" -#include + static struct { - uint8_t user_data[PBSYS_CONFIG_STORAGE_USER_DATA_SIZE]; - char stored_firmware_hash[8]; - pbsys_storage_settings_t settings; - uint32_t program_offset; - uint32_t program_size; - uint8_t program_data[sizeof(_program_data)]; -} ramdisk __attribute__((section(".noinit"), used)) = { 0 }; + // ensure that data is properly aligned for pbsys_storage_data_map_t + pbsys_storage_data_map_t data_map; + uint8_t data[PBDRV_CONFIG_BLOCK_DEVICE_RAM_SIZE]; +} ramdisk; uint32_t pbdrv_block_device_get_writable_size(void) { return 0; } -pbio_error_t pbdrv_block_device_get_data(uint8_t **data) { - *data = (void *)&ramdisk; +pbio_error_t pbdrv_block_device_get_data(pbsys_storage_data_map_t **data) { + *data = &ramdisk.data_map; // Higher level code can use the ramdisk data if initialization completed // successfully. Otherwise it should reset to factory default data. @@ -83,9 +81,9 @@ pbio_error_t pbdrv_block_device_get_data(uint8_t **data) { } void pbdrv_block_device_init(void) { - ramdisk.program_size = sizeof(_program_data); - memcpy(&ramdisk.stored_firmware_hash[0], MICROPY_GIT_HASH, sizeof(ramdisk.stored_firmware_hash)); - memcpy(&ramdisk.program_data[0], _program_data, sizeof(_program_data)); + ramdisk.data_map.slot_info[0].size = sizeof(_program_data); + memcpy(ramdisk.data_map.stored_firmware_hash, MICROPY_GIT_HASH, sizeof(ramdisk.data_map.stored_firmware_hash)); + memcpy(ramdisk.data_map.program_data, _program_data, sizeof(_program_data)); } // Don't store any data in this implementation. diff --git a/lib/pbio/drv/block_device/block_device_w25qxx_stm32.c b/lib/pbio/drv/block_device/block_device_w25qxx_stm32.c index 73f250c97..666140f25 100644 --- a/lib/pbio/drv/block_device/block_device_w25qxx_stm32.c +++ b/lib/pbio/drv/block_device/block_device_w25qxx_stm32.c @@ -13,9 +13,8 @@ #include STM32_HAL_H -#include - #include "../core.h" +#include "../sys/storage_data.h" #include "block_device_w25qxx_stm32.h" #include @@ -36,15 +35,19 @@ static struct { * portion of this, up to pbdrv_block_device_get_writable_size() bytes, * gets saved to flash at shutdown. */ - uint8_t data[PBDRV_CONFIG_BLOCK_DEVICE_RAM_SIZE]; + union { + // ensure that data is properly aligned for pbsys_storage_data_map_t + pbsys_storage_data_map_t data_map; + uint8_t data[PBDRV_CONFIG_BLOCK_DEVICE_RAM_SIZE]; + }; } ramdisk __attribute__((section(".noinit"), used)); uint32_t pbdrv_block_device_get_writable_size(void) { return PBDRV_CONFIG_BLOCK_DEVICE_W25QXX_STM32_SIZE - sizeof(ramdisk.saved_size); } -pbio_error_t pbdrv_block_device_get_data(uint8_t **data) { - *data = ramdisk.data; +pbio_error_t pbdrv_block_device_get_data(pbsys_storage_data_map_t **data) { + *data = &ramdisk.data_map; // Higher level code can use the ramdisk data if initialization completed // successfully. Otherwise it should reset to factory default data. diff --git a/lib/pbio/include/pbdrv/block_device.h b/lib/pbio/include/pbdrv/block_device.h index a29e9261e..875dfd871 100644 --- a/lib/pbio/include/pbdrv/block_device.h +++ b/lib/pbio/include/pbdrv/block_device.h @@ -11,9 +11,10 @@ #include +#include "../sys/storage_data.h" + #include #include - #include #if PBDRV_CONFIG_BLOCK_DEVICE @@ -28,7 +29,7 @@ * ::PBIO_ERROR_IO (driver-specific error), though boot * does not proceed if this happens. */ -pbio_error_t pbdrv_block_device_get_data(uint8_t **data); +pbio_error_t pbdrv_block_device_get_data(pbsys_storage_data_map_t **data); /** * Writes the "RAM Disk" to storage. May erase entire disk prior to writing. @@ -57,7 +58,7 @@ uint32_t pbdrv_block_device_get_writable_size(void); #else -static inline pbio_error_t pbdrv_block_device_get_data(uint8_t **data) { +static inline pbio_error_t pbdrv_block_device_get_data(pbsys_storage_data_map_t **data) { return PBIO_ERROR_NOT_SUPPORTED; } diff --git a/lib/pbio/platform/nxt/pbdrvconfig.h b/lib/pbio/platform/nxt/pbdrvconfig.h index 72c635b53..3b8c32877 100644 --- a/lib/pbio/platform/nxt/pbdrvconfig.h +++ b/lib/pbio/platform/nxt/pbdrvconfig.h @@ -11,8 +11,6 @@ #define PBDRV_CONFIG_BLOCK_DEVICE (1) #define PBDRV_CONFIG_BLOCK_DEVICE_RAM_SIZE (10 * 1024) #define PBDRV_CONFIG_BLOCK_DEVICE_TEST (1) -#define PBDRV_CONFIG_BLOCK_DEVICE_TEST_SIZE (8 * 1024) -#define PBDRV_CONFIG_BLOCK_DEVICE_TEST_SIZE_USER (512) #define PBDRV_CONFIG_BUTTON (1) #define PBDRV_CONFIG_BUTTON_NXT (1) diff --git a/lib/pbio/platform/nxt/pbsysconfig.h b/lib/pbio/platform/nxt/pbsysconfig.h index 65b9060c9..7f5399e01 100644 --- a/lib/pbio/platform/nxt/pbsysconfig.h +++ b/lib/pbio/platform/nxt/pbsysconfig.h @@ -11,7 +11,7 @@ #define PBSYS_CONFIG_MAIN (1) #define PBSYS_CONFIG_STORAGE (1) #define PBSYS_CONFIG_STORAGE_NUM_SLOTS (1) -#define PBSYS_CONFIG_STORAGE_USER_DATA_SIZE (PBDRV_CONFIG_BLOCK_DEVICE_TEST_SIZE_USER) +#define PBSYS_CONFIG_STORAGE_USER_DATA_SIZE (512) #define PBSYS_CONFIG_STATUS_LIGHT (0) #define PBSYS_CONFIG_STATUS_LIGHT_BATTERY (0) #define PBSYS_CONFIG_STATUS_LIGHT_BLUETOOTH (0) diff --git a/lib/pbio/sys/storage.c b/lib/pbio/sys/storage.c index daa13e57b..c1a283008 100644 --- a/lib/pbio/sys/storage.c +++ b/lib/pbio/sys/storage.c @@ -22,62 +22,11 @@ #include "core.h" #include "hmi.h" -/** - * Information about one code slot. - * - * A size of 0 means that this slot is not used. The offset indicates where - * the program is stored in user storage. - * - * Code slots are *not* stored chronologically by slot id. Instead they are - * stored consecutively as they are received, with the newest program last. - * - * If a slot is already in use and a new program should be loaded into it, - * it is deleted by mem-moving any subsequent programs into its place, and - * appending the new program to be last again. Since a user is typically only - * iterating code in one slot, this is therefore usually the last stored - * program. This means memmoves happen very little, only when needed. - * - */ -typedef struct { - uint32_t offset; - uint32_t size; -} pbsys_storage_slot_info_t; - /** * Slot at which incoming program data is currently being received. */ static uint8_t incoming_slot = 0; -/** - * Map of loaded data. All data types are little-endian. - */ -typedef struct { - /** - * End-user read-write accessible data. Everything after this is also - * user-readable but not writable. - */ - uint8_t user_data[PBSYS_CONFIG_STORAGE_USER_DATA_SIZE]; - /** - * First 8 symbols of the git hash of the firmware version used to create - * this data map. If this does not match the version of the running - * firmware, user data will be reset to 0. - */ - char stored_firmware_hash[8]; - /** - * System settings. Settings will be reset to defaults when the firmware - * version changes due to an update. - */ - pbsys_storage_settings_t settings; - /** - * Size and offset info for each slot. - */ - pbsys_storage_slot_info_t slot_info[PBSYS_CONFIG_STORAGE_NUM_SLOTS]; - /** - * Data of the application program (code + heap). - */ - uint8_t program_data[] __attribute__((aligned(sizeof(void *)))); -} pbsys_storage_data_map_t; - /** * Map of loaded data. This is kept in memory between successive runs of the * end user application (like MicroPython). @@ -399,7 +348,7 @@ static pbio_error_t pbsys_storage_deinit_process_thread(pbio_os_state_t *state, */ void pbsys_storage_init(void) { - pbio_error_t err = pbdrv_block_device_get_data((uint8_t **)&map); + pbio_error_t err = pbdrv_block_device_get_data(&map); // Test that storage successfully loaded and matches current firmware, // otherwise reset storage. diff --git a/lib/pbio/sys/storage_data.h b/lib/pbio/sys/storage_data.h new file mode 100644 index 000000000..a51331656 --- /dev/null +++ b/lib/pbio/sys/storage_data.h @@ -0,0 +1,69 @@ +// SPDX-License-Identifier: MIT +// Copyright (c) 2025 The Pybricks Authors + +// Data structures for non-volatile storage. + +// NB: Normally, pbsys code isn't referenced by drivers, however this is a +// special case where the block device driver needs to have the actual struct +// definition in order to ensure correct memory layout and alignment. + +#ifndef _PBSYS_SYS_STORAGE_DATA_H_ +#define _PBSYS_SYS_STORAGE_DATA_H_ + +#include + +#include +#include + +/** + * Information about one code slot. + * + * A size of 0 means that this slot is not used. The offset indicates where + * the program is stored in user storage. + * + * Code slots are *not* stored chronologically by slot id. Instead they are + * stored consecutively as they are received, with the newest program last. + * + * If a slot is already in use and a new program should be loaded into it, + * it is deleted by mem-moving any subsequent programs into its place, and + * appending the new program to be last again. Since a user is typically only + * iterating code in one slot, this is therefore usually the last stored + * program. This means memmoves happen very little, only when needed. + * + */ +typedef struct { + uint32_t offset; + uint32_t size; +} pbsys_storage_slot_info_t; + +/** + * Map of loaded data. All data types are little-endian. + */ +typedef struct { + /** + * End-user read-write accessible data. Everything after this is also + * user-readable but not writable. + */ + uint8_t user_data[PBSYS_CONFIG_STORAGE_USER_DATA_SIZE]; + /** + * First 8 symbols of the git hash of the firmware version used to create + * this data map. If this does not match the version of the running + * firmware, user data will be reset to 0. + */ + char stored_firmware_hash[8]; + /** + * System settings. Settings will be reset to defaults when the firmware + * version changes due to an update. + */ + pbsys_storage_settings_t settings; + /** + * Size and offset info for each slot. + */ + pbsys_storage_slot_info_t slot_info[PBSYS_CONFIG_STORAGE_NUM_SLOTS]; + /** + * Data of the application program (code + heap). + */ + uint8_t program_data[] __attribute__((aligned(sizeof(void *)))); +} pbsys_storage_data_map_t; + +#endif // _PBSYS_SYS_STORAGE_DATA_H_