Conversation
There was a problem hiding this comment.
- some errors must be propagated better
- UART drivers inclusion in bootloader should depend on whether UART is enabled in the config (default: off)
- No tests: at least one build test is needed, but at this stage it would also be useful to have a comparison of the footprint for the extra added layer. Please check footprint test, build on the same target/compiler with wolfHAL and compare footprints
- wrong submodule URL: it should be
https://github.com/wolfssl/wolfhal, notgit@github.com ...
config/wolfHAL/stm32wb55_nucleo.c
Outdated
| @@ -0,0 +1,101 @@ | |||
| #include <wolfHAL/platform/st/stm32wb55xx.h> | |||
|
|
|||
| whal_StRcc_PeriphClk periphClkEn[] = | |||
There was a problem hiding this comment.
These can't be static since they are externed in both hal/wolfhal.c and the test app
config/wolfHAL/stm32wb55_nucleo.c
Outdated
| WHAL_ST_RCC_PERIPH_FLASH, | ||
| }; | ||
|
|
||
| whal_Clock wbClock = { |
There was a problem hiding this comment.
These can't be static since they are externed in both hal/wolfhal.c and the test app
config/wolfHAL/stm32wb55_nucleo.c
Outdated
| }, | ||
| }; | ||
|
|
||
| whal_Gpio wbGpio = { |
There was a problem hiding this comment.
These can't be static since they are externed in both hal/wolfhal.c and the test app
config/wolfHAL/stm32wb55_nucleo.c
Outdated
| .pinCount = 3, | ||
| }; | ||
|
|
||
| whal_Uart wbUart = { |
There was a problem hiding this comment.
These can't be static since they are externed in both hal/wolfhal.c and the test app
config/wolfHAL/stm32wb55_nucleo.c
Outdated
| }, | ||
| }; | ||
|
|
||
| whal_Flash wbFlash = { |
There was a problem hiding this comment.
These can't be static since they are externed in both hal/wolfhal.c and the test app
config/wolfHAL/stm32wb55_nucleo.mk
Outdated
There was a problem hiding this comment.
inclusion of st_uart.o should be conditional to config (default is no uart enabled in bootloader)
There was a problem hiding this comment.
inclusion of this in the build should be conditional (default = off)
There was a problem hiding this comment.
This file should only be compiled in when DEBUG_UART is defined as er arch.mk or if UART_FLASH is defined as per options.mk
hal/uart/uart_drv_wolfhal.c
Outdated
| int uart_rx(uint8_t *c) | ||
| { | ||
| whal_Uart_Recv(&wbUart, c, 1); | ||
| /* ALEX NOTE: this function also returns zero if no data is available... */ |
There was a problem hiding this comment.
a better handling of whal_ return values is required
There was a problem hiding this comment.
All return values from the underlying whal_ interfaces are discarded. Please propagate failures/errors
ab7ccb4 to
3e80589
Compare
0743918 to
54cdcf8
Compare
I hacked together this quick wolfHAL integration using the current implementation of wolfHAL that I already worked on.
This PR is more-so meant for discussion/buy-in on how wolfHAL should be used within wolfBoot rather than getting this PR merged in.
Here's an overview of what I did:
wolfhalTARGETstm32wb-wolfhal.config./config/wolfHAL/hal_*api functions using wolfHALuart_*api functions using wolfHALI've tested this on the stm32wb nucleo board and I'm able to boot fully into the target app
Here is the wolfHAL repo I used to build this https://github.com/AlexLanzano/wolfHAL/tree/wolfboot-integration
As you can see the design is unchanged from what I presented in the meeting. SInce I had the stm32wb drivers already done in this design I figured it would be best to just give the integration a shot and have you guys point out the specific parts you dont like