Flash bank customisation #1293
Flash bank customisation #1293
Conversation
|
Frankly, i'd just be tempted to use |
|
Nice. I literally didn't know that existed |
peterharperuk
force-pushed
the
bt_flash_bank_dynamic
branch
from
ad7d3fb
to
dbb5bd9
Compare
|
I've used __builtin_constant_p() |
uh yeah, duh, good point ;-) too little sleep! |
peterharperuk
changed the title
|
Unless I'm mistaken this still isn't going to work without modifying files in the sdk? They want to define the flash bank storage offset with pointer arithmetic. Which means the variable has to be defined in the btstack_flash_bank.h file. It can't go in pico_w.h because that's included in assembler. Maybe we just need to make it possible to use an alternate version of pico_flash_bank_instance maybe via a #define specifying the function name or a cmake variable specifying a different source file. @kilograham are there any other examples for enabling customisation like this? |
peterharperuk
force-pushed
the
bt_flash_bank_dynamic
branch
2 times, most recently
from
ae7694a
to
70ea92f
Compare
|
I've fixed this differently. Now if you define pico_flash_bank_get_offset_func you can change the function used to get the offset. |
|
@earlephilhower you better tell me if you're happy with this. |
|
@peterharperuk, first thanks a bunch for your support. That's a neat solution I'm also thinking, maybe this and #1278 can also be dropped. The reason is that the current TVL flash bank implementation isn't multicore safe, and I think lots of folks are going to use that 2nd core. The SDK can't really cater to all use cases, but it can be a simple example for folks to build off of for their own TLV implementations, maybe? |
|
I don't think you should be using sizeof to set the size of the flash area. I can't see the need. Just set PICO_FLASH_BANK_TOTAL_SIZE to the number of sectors you want to reserve. Are you saying you don't need this change? If the flash bank implementation isn't multicore safe (you're probably right) we can just fix that. |
Agreed, I edited afterwards which probably wasn't obvious once I realized the same thing. Sorry, it was pre-coffee in the morning for me!
Well, this is necessary but not sufficient for my use case. The multicore safety needs to be there, too, but then I think I could use the stock version, yes. But, my comment was more that you're moving into the application-side of things here, while most of the SDK is low-level tools. TLV could be in a fixed flash sector, it could be in a dynamic one like this patch, it could be a file (if you have a FS you could spread out flash wear), etc. So you've shown how to do it already, and since BT is really an app-level thing it's up to the end user to implement the right TLV store for their app. |
|
Understood. I agree we're straying into the app side of things. We just want to provide the minimum necessary for the users who just want their code to work. And we don't want to get in the way of "power users" like yourself. I'll look into the multicore safety issue as that was an open issue that needs to be resolved. |
|
just marking this as a draft so i don't merge it yet... will likely add some help for multicore in 1.5.1 too |
|
now dependent on #1412 (with some merge fun) |
|
Needs to be rebased on top of #1412 |
peterharperuk
force-pushed
the
bt_flash_bank_dynamic
branch
from
01b2a48
to
23f307a
Compare
|
Seems to be working okay |
Allow the pico_flash_bank_get_offset function to be changed by defining pico_flash_bank_get_offset_func Fixes raspberrypi#1278
peterharperuk
force-pushed
the
bt_flash_bank_dynamic
branch
from
23f307a
to
a382df4
Compare
|
Rebase done! |
Defaults to true. Can be set to false if the flash bank offset is dynamically generated, e.g. to be inside the binary itself.
Fixes #1278