Can't use non-absolute offset for Bluetooth TLV storage area in PICO_FLASH_BANK #1278

New issue

Closed

opened 2023-02-23 16:32:10 -06:00 by earlephilhower · 3 comments

earlephilhower commented

2023-02-23 16:32:10 -06:00

(Migrated from github.com)

By default the 2 Bluetooth TLV stores are placed at the last 2 sectors of flash.

Unfortunately, in my own framework many people already have data in those last two sectors, so I need to move them elsewhere. Dynamically would be best. So I'm attempting to do something like creating a "const" variable that can be overwritten at the proper alignment in app code

const uint8_t __bluetooth_tlv[8192] __attribute__ ((aligned(4096))) = { 0 };
extern const uint8_t __flash_binary_start;

#define PICO_FLASH_BANK_TOTAL_SIZE sizeof(__bluetooth_tlv)
#define PICO_FLASH_BANK_STORAGE_OFFSET ((unsigned int)(__bluetooth_tlv - &__flash_binary_start))

(the undef/redef in the real app is handled in the Cmakefile and btstack_config.h, this is just a self-contained snippet for simplicity)

The problem is that the following static asserts fail due to the "non-constness" of the address-of operator. The compiler can't know the offset of the variable, only the linker knows that, so the asserts fail.
github.com/raspberrypi/pico-sdk@777cd52e21/src/rp2_common/pico_btstack/btstack_flash_bank.c (L13-L15)

I would ask to either remove those static_asserts or bracket them with, say, a new marker #if !defined(PICO_FLASH_BANK_DYNAMIC). There doesn't seem to be any further dependency on the values being compile-time constants, only these assertions.

-- edit --
One additional runtime assert needs to be bracketed or removed as well (it's checking we don't overwrite the "app" with the flash TLV, but in this use case we actually expect to):
github.com/raspberrypi/pico-sdk@777cd52e21/src/rp2_common/pico_btstack/btstack_flash_bank.c (L140)

By default the 2 Bluetooth TLV stores are placed at the last 2 sectors of flash. Unfortunately, in my own framework many people already have data in those last two sectors, so I need to move them elsewhere. Dynamically would be best. So I'm attempting to do something like creating a "const" variable that can be overwritten at the proper alignment in app code ```` const uint8_t __bluetooth_tlv[8192] __attribute__ ((aligned(4096))) = { 0 }; extern const uint8_t __flash_binary_start; #define PICO_FLASH_BANK_TOTAL_SIZE sizeof(__bluetooth_tlv) #define PICO_FLASH_BANK_STORAGE_OFFSET ((unsigned int)(__bluetooth_tlv - &__flash_binary_start)) ```` (the undef/redef in the real app is handled in the `Cmakefile` and `btstack_config.h`, this is just a self-contained snippet for simplicity) The problem is that the following static asserts fail due to the "non-constness" of the address-of operator. The compiler can't know the offset of the variable, only the linker knows that, so the asserts fail. https://github.com/raspberrypi/pico-sdk/blob/777cd52e21daf7c9a764e28611fceb3004ace488/src/rp2_common/pico_btstack/btstack_flash_bank.c#L13-L15 I would ask to either remove those `static_assert`s or bracket them with, say, a new marker `#if !defined(PICO_FLASH_BANK_DYNAMIC)`. There doesn't seem to be any further dependency on the values being compile-time constants, only these assertions. -- edit -- One additional runtime assert needs to be bracketed or removed as well (it's checking we don't overwrite the "app" with the flash TLV, but in this use case we actually expect to): https://github.com/raspberrypi/pico-sdk/blob/777cd52e21daf7c9a764e28611fceb3004ace488/src/rp2_common/pico_btstack/btstack_flash_bank.c#L140

peterharperuk commented

2023-02-23 18:31:04 -06:00

(Migrated from github.com)

It sounds like we should just make it runtime rather than build time configurable?
Edit. Actually ignore that. I see what you're doing now. I'm fine with changing the library to use your method if it works. I'd rather not add more ifdef complications.

It sounds like we should just make it runtime rather than build time configurable? Edit. Actually ignore that. I see what you're doing now. I'm fine with changing the library to use your method if it works. I'd rather not add more ifdef complications.

kilograham commented

2023-06-06 12:48:33 -05:00

(Migrated from github.com)