Add generic wolfHAL Ethernet port with STM32H563ZI Nucleo board support - #97
Add generic wolfHAL Ethernet port with STM32H563ZI Nucleo board support#97AlexLanzano wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a new src/port/wolfHAL/ port intended to let wolfIP run on any target that provides wolfHAL whal_Eth / whal_EthPhy, with STM32H563ZI Nucleo-144 as the first reference board and a standalone Makefile-based bare-metal build.
Changes:
- Introduces a generic wolfHAL-based link-layer adapter (
wolfhal_eth.c/.h) that bridges wolfIP ll_dev poll/send towhal_Eth_Recv/whal_Eth_Sendand starts the MAC after PHY negotiation. - Adds a generic bare-metal
main.cfor bringing up wolfIP, configuring a static IPv4 address, and starting a TCP echo server. - Adds a complete STM32H563ZI Nucleo board package (startup/IVT/linker/board init/syscalls + board.mk) and a port-local build system +
.gitignore.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/port/wolfHAL/wolfhal_eth.h | Public API/docs for the generic wolfHAL Ethernet bridge. |
| src/port/wolfHAL/wolfhal_eth.c | Implements wolfIP ll_dev poll/send via wolfHAL and link negotiation + MAC start. |
| src/port/wolfHAL/main.c | Bare-metal reference app wiring board init + wolfIP + echo server. |
| src/port/wolfHAL/Makefile | New Makefile entrypoint for BOARD-based builds. |
| src/port/wolfHAL/.gitignore | Ignores build artifacts. |
| src/port/wolfHAL/boards/stm32h563zi_nucleo/board.mk | Toolchain flags and board + wolfHAL driver source list. |
| src/port/wolfHAL/boards/stm32h563zi_nucleo/board.h | Board API and exported wolfHAL device instances. |
| src/port/wolfHAL/boards/stm32h563zi_nucleo/board.c | STM32H563ZI init: flash latency, clocks, GPIO, ETH/PHY, SysTick timer, UART. |
| src/port/wolfHAL/boards/stm32h563zi_nucleo/syscalls.c | Newlib stubs + wolfIP hooks (incl. RNG). |
| src/port/wolfHAL/boards/stm32h563zi_nucleo/startup.c | Minimal reset handler (data/bss init + libc init + main). |
| src/port/wolfHAL/boards/stm32h563zi_nucleo/ivt.c | Cortex-M33 vector table. |
| src/port/wolfHAL/boards/stm32h563zi_nucleo/linker.ld | Memory map and sections for STM32H563ZI. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
b5999a2 to
015ca06
Compare
015ca06 to
70ac819
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 15 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
70ac819 to
6e50685
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 15 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
6e50685 to
fc3675e
Compare
fc3675e to
80a210f
Compare
9fbb31e to
878c293
Compare
dgarske
left a comment
There was a problem hiding this comment.
Can wolfHAL be less intrusive? Or show what an update of the existing STM32H5 wolfIP port look for the H5? Have you compare code size between the two (existing and this)? I'd almost like to see instead a proper update to a wolfHAL version of the drivers. Not src/port/wolfHAL but src/port/stm32h5 optionally using the wolfHAL to compare.
@dgarske The nice thing about having this in a separate src/port/wolfHAL is that we can maintain a generic application and layer on things like FreeRTOS, MQTT, TLS client/servers without having to re-implement this in each port. Having this generic wolfHAL port structure makes it much easier to port existing platforms in wolfHAL to wolfip. You just add a new entry in boards/ that contains all the platform specific config instead of spinning up a new src/port/ dir and then having to re-implement all the same application level code. I can move all of this out of src/port/wolfHAL and into src/port/stm32h5 but this would lose a lot of the nice things that wolfHAL provides IMO Also, I'll get this size comparison between the existing H5 port and the wolfHAL port soon |
878c293 to
d1530a9
Compare
9009c88 to
3974801
Compare
3974801 to
2e9c1f4
Compare
2e9c1f4 to
8fbd34f
Compare
aidangarske
left a comment
There was a problem hiding this comment.
Skoll Code Review
Scan type: securityOverall recommendation: COMMENT
Findings: 2 total — 2 posted, 0 skipped
Posted findings
- [Medium] Unpinned wolfHAL checkout makes the CI firmware build depend on mutable upstream HEAD —
.github/workflows/wolfhal-stm32h563zi-nucleo.yml:1-38 - [Medium] wolfHAL Ethernet init failure is logged but networking still starts —
src/port/stm32h563/main.c:849-1453
Review generated by Skoll
aidangarske
left a comment
There was a problem hiding this comment.
Skoll Code Review
Scan type: review-securityOverall recommendation: COMMENT
Findings: 6 total — 6 posted, 0 skipped
6 finding(s) posted as inline comments (see file-level comments below)
Posted findings
- [Medium] wolfHAL syscalls omit wall-clock hooks required by TLS —
src/port/stm32h563/boards/stm32h563zi_nucleo/syscalls.c:23-121 - [Medium] 32-bit tick wraps while wolfIP expects a monotonic 64-bit clock —
src/port/stm32h563/main.c:1095-1098 - [Medium] wolfHAL backend conflicts with the existing DOT1X SysTick handler —
src/port/stm32h563/boards/stm32h563zi_nucleo/board.c:34-37 - [Medium] CI builds against mutable wolfHAL HEAD —
.github/workflows/wolfhal-stm32h563zi-nucleo.yml:23-26 - [Low] DEBUG_H5_ETH still references the removed STM32 Ethernet driver —
src/port/stm32h563/main.c:1062-1128 - [Low] New wolfHAL bridge has no automated behavioral coverage —
src/port/wolfHAL/wolfhal_eth.c:27-96
Review generated by Skoll
| * along with this program; if not, write to the Free Software | ||
| * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1335, USA | ||
| */ | ||
| #include <sys/stat.h> |
There was a problem hiding this comment.
🟠 [Medium] wolfHAL syscalls omit wall-clock hooks required by TLS · Logic
The wolfHAL Makefile replaces the existing STM32H563 syscalls with this new file, but the new implementation omits time() and _gettimeofday(). The previous syscalls provided both specifically for wolfSSL certificate validity checks and the 802.1X EAP-TLS demo. TLS/DOT1X builds may therefore fail to link or receive unusable time from libnosys.
Fix: Share or copy the existing wall-clock implementations into the board syscalls and test ENABLE_TLS and ENABLE_DOT1X builds.
| } | ||
| #endif | ||
| while (!dhcp_bound(IPStack) && dhcp_client_is_running(IPStack)) { | ||
| #ifdef ENABLE_WOLFHAL |
There was a problem hiding this comment.
🟠 [Medium] 32-bit tick wraps while wolfIP expects a monotonic 64-bit clock · Logic
Before this PR, the port passed a continuously incremented uint64_t tick to wolfIP_poll. The wolfHAL path repeatedly assigns the 32-bit board_get_tick() value to that variable. After roughly 49.7 days, wolfIP's 64-bit timer clock moves backward to zero; existing TCP, DHCP, ARP, and other timer expirations can then be delayed for up to another 49 days.
Fix: Expose a monotonic uint64_t tick or extend the 32-bit SysTick counter across wrap before passing it to wolfIP.
| /* SysTick timing */ | ||
| volatile uint32_t g_tick = 0; | ||
|
|
||
| void SysTick_Handler(void) |
There was a problem hiding this comment.
🟠 [Medium] wolfHAL backend conflicts with the existing DOT1X SysTick handler · Logic
The new board adds a strong SysTick_Handler, while the existing dot1x_client.c also defines a strong handler. The new Makefile rejects only TZEN and FreeRTOS, and still adds dot1x_client.c when ENABLE_DOT1X=1, so ENABLE_WOLFHAL=1 ENABLE_DOT1X=1 fails with a multiple-definition link error. Additionally, DOT1X programs a 32 MHz SysTick reload after wolfHAL has configured the CPU for 168 MHz.
Fix: Use one shared handler/timebase for wolfHAL and DOT1X, or explicitly reject ENABLE_DOT1X with ENABLE_WOLFHAL until the clock and handler integration is implemented.
| sudo apt-get update | ||
| sudo apt-get install -y gcc-arm-none-eabi | ||
|
|
||
| - name: Clone wolfHAL |
There was a problem hiding this comment.
🟠 [Medium] CI builds against mutable wolfHAL HEAD · Security
The new workflow clones the wolfHAL default branch without pinning or verifying a commit. An upstream force-push or repository compromise can silently change the code compiled by CI and make firmware build results non-reproducible without any wolfIP PR change.
Fix: Pin wolfHAL to a reviewed commit or tag and verify its SHA; pin third-party Actions to immutable SHAs as well.
| #else | ||
| dhcp_start_tick = tick; | ||
| #endif | ||
| #ifdef DEBUG_H5_ETH |
There was a problem hiding this comment.
🔵 [Low] DEBUG_H5_ETH still references the removed STM32 Ethernet driver · Logic
The wolfHAL branch no longer includes stm32_eth.h or builds stm32_eth.c, but both DEBUG_H5_ETH blocks still call stm32_eth_get_* functions. Building the new backend with EXTRA_CFLAGS=-DDEBUG_H5_ETH therefore produces missing declarations and linker failures.
Fix: Guard these diagnostics with #ifndef ENABLE_WOLFHAL or replace them with wolfHAL-compatible statistics.
| #include "wolfhal_eth.h" | ||
| #include "board.h" | ||
| #include <string.h> | ||
|
|
There was a problem hiding this comment.
🔵 [Low] New wolfHAL bridge has no automated behavioral coverage · Missing Tests
The new bridge has no tests for null arguments, PHY query failures, link timeout, MAC start errors, receive buffer lengths, or WHAL_ENOTREADY translation. The new CI job only verifies that an image and size output exist, so these paths are not exercised.
Fix: Add host-side tests with mocked wolfHAL devices, plus a target test covering link negotiation and frame polling.
There was a problem hiding this comment.
Not sure if it's worth adding a mock host-side test suite for CI. The bridge will be tested on target.
8fbd34f to
268a01e
Compare
268a01e to
26851f3
Compare
|
Need to wait for 64bit tick support to land in wolfHAL (PR is open here wolfSSL/wolfHAL#53) for the wolfHAL CI build checks to pass |
Summary
whal_Eth_Recv/whal_Eth_Send
Structure
src/port/wolfHAL/
├── Makefile, main.c, wolfhal_eth.c/h # Generic (board-independent)
└── boards/stm32h563zi_nucleo/ # Board-specific: clock, GPIO, ETH, UART, linker
Adding a new board requires only a
boards/<name>/directory with board.c/h, board.mk, and bare-metal startup files. The generic main.c and wolfhal_eth.c are shared across all boards.Build
Requires arm-none-eabi-gcc and wolfHAL cloned as a sibling directory (overridable via WOLFHAL_ROOT).
Testing
I tested this by doing a
ping 192.168.1.100on my host PC. I successfully got a responses back.