-
Notifications
You must be signed in to change notification settings - Fork 138
[RFC] Integrate wolfHAL #674
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- 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 ...
| @@ -0,0 +1,101 @@ | |||
| #include <wolfHAL/platform/st/stm32wb55xx.h> | |||
|
|
|||
| whal_StRcc_PeriphClk periphClkEn[] = | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
static
| WHAL_ST_RCC_PERIPH_FLASH, | ||
| }; | ||
|
|
||
| whal_Clock wbClock = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
static
| }, | ||
| }; | ||
|
|
||
| whal_Gpio wbGpio = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
static
| .pinCount = 3, | ||
| }; | ||
|
|
||
| whal_Uart wbUart = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
static
| }, | ||
| }; | ||
|
|
||
| whal_Flash wbFlash = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
static
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
inclusion of st_uart.o should be conditional to config (default is no uart enabled in bootloader)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
inclusion of this in the build should be conditional (default = off)
| 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a better handling of whal_ return values is required
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All return values from the underlying whal_ interfaces are discarded. Please propagate failures/errors
ab7ccb4 to
3e80589
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