Nickez/bb03 binaries#1971
Conversation
4da9335 to
7d3d4e5
Compare
4dcbf07 to
671dc8e
Compare
82484f8 to
a102a55
Compare
56fade0 to
2314078
Compare
19bd701 to
f048234
Compare
9862fc6 to
c6341c4
Compare
c6341c4 to
9bd2aab
Compare
| raise ValueError("board name '{}' is reserved".format(board)) | ||
|
|
||
|
|
||
| def hal_driver_directories(source): |
There was a problem hiding this comment.
for HAL_CONF we have an exact path and name, but for the HAL_DRIVER folder we go for a loose/generic way. If possible, I would prefer consistency here. Are we really expecting multiple HAL drivers when we just expect a single stm32u5xx_hal_conf.h?
| Path("USBX/Target"), | ||
| ) | ||
| OPTIONAL_MIDDLEWARE_DIRECTORIES = (Path("Middlewares/ST/usbx"),) | ||
| RESERVED_BOARD_NAMES = frozenset(("Core", "Common", "Drivers")) |
There was a problem hiding this comment.
shouldn't "USBX" and "Middlewares" be part of that set too?
| if directory.parts[0] == "Core": | ||
| dst = board_dir / directory.relative_to("Core") | ||
| else: | ||
| dst = board_dir / directory |
There was a problem hiding this comment.
Does the else statement make sense here? How can it not have "Core" as the parts[0] but still be valid?
| Path("Drivers/CMSIS"), | ||
| ) | ||
| REQUIRED_FILES = (HAL_CONF,) | ||
| OPTIONAL_SHARED_DIRECTORIES = ( |
There was a problem hiding this comment.
Regarding optional directories: If a later CubeMX project does not have e.g. USBX (for whatever reason), then the USBX folder can become stale. For USBX that might not be a problem but what's your opinion on potentially stale code?
| use std::path::{Path, PathBuf}; | ||
| use std::process::Command; | ||
|
|
||
| const ST_SOURCES: &[&str] = &[ |
There was a problem hiding this comment.
These sources are not minimal. The build succeeds, even without:
- stm32u5xx_hal_dma.c
- stm32u5xx_hal_dma_ex.c
- stm32u5xx_hal_rcc_ex.c
- stm32u5xx_hal_gpio.c
- stm32u5xx_hal_exti.c
- stm32u5xx_hal_pwr.c
- stm32u5xx_hal_icache.c
Is this intentionally chosen as a base for upcoming work? Or should we keep this minimal for now?
| ); | ||
| } | ||
|
|
||
| println!("cargo::rustc-env=BITBOX03_IMAGE_HEADER_LEN={IMAGE_HEADER_LEN}"); |
There was a problem hiding this comment.
Is BITBOX03_IMAGE_HEADER_LEN intended for later build binaries? I do not see it used in this commit.
| bitbox-boot-utils = { path = "../../bitbox-boot-utils" } | ||
| bitbox-board-stm32u5a9j-dk = { path = "../../bitbox-board-stm32u5a9j-dk", optional = true } | ||
|
|
||
| [target.'cfg(all(target_arch = "arm", target_os = "none"))'.dependencies] |
There was a problem hiding this comment.
Boot0 had these deps unconditionally. I think this (how it's done for Boot1) is an improvement, but should we then do it for Boot0 as well (also for consistency)?
| println!("cargo::rustc-link-arg={}", header_object.display()); | ||
| } | ||
|
|
||
| fn main() { |
There was a problem hiding this comment.
Boot0/Boot1 fail if no board feature is selected. Why don't we have the same behavior here?
| use core::panic::PanicInfo; | ||
| use cortex_m_rt::entry; | ||
|
|
||
| const BOOT0_ADDR: u32 = 0x0800_2000; |
There was a problem hiding this comment.
Why not use board::memory::BOOT0_ADDR?
| @@ -0,0 +1,6 @@ | |||
| MEMORY | |||
There was a problem hiding this comment.
This duplicates memory assumptions. Should we make sure they don't accidentally drift?
Some terminology
Platform - Everything that is common to the stm32u5 platform (basically the stm32u5 HAL)
Board - Everything that is specific to a single board and MCU, for example the STM32U5A9J-DK dev-kit, the bb03 "test board" or the bb03 "production board". Every board has at least a unique pinout and memory layout.