Add VORAGO VA416xx Ethernet port (PEB1 EVK)#27
Add VORAGO VA416xx Ethernet port (PEB1 EVK)#27dgarske wants to merge 1 commit intowolfSSL:masterfrom
Conversation
f417afb to
acfb50c
Compare
There was a problem hiding this comment.
Pull request overview
This pull request adds a bare-metal port of wolfIP for the VORAGO VA416xx Cortex-M4 microcontroller, targeting the PEB1 EVK board with a KSZ8041TL Ethernet PHY over MII. The port includes a complete Ethernet driver implementation using the Synopsys DesignWare GMAC with normal (legacy) descriptor format, along with supporting infrastructure for bare-metal operation.
Changes:
- Added Ethernet MAC/DMA driver for VA416xx using normal (legacy) DWC GMAC descriptor format with chain mode for TX and ring mode for RX
- Implemented complete bare-metal environment including linker script with DMA buffer placement validation, startup code, syscalls, and interrupt vector table
- Created TCP echo server application with DHCP support, LED heartbeat, and optional throughput testing
Reviewed changes
Copilot reviewed 14 out of 15 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/port/va416xx/va416xx_eth.h | Public API for Ethernet driver (initialization, statistics, diagnostics) |
| src/port/va416xx/va416xx_eth.c | Complete Ethernet driver implementation with normal descriptor format, PHY initialization, and comprehensive hardware workarounds |
| src/port/va416xx/target.ld | Linker script defining 256KB flash and 64KB RAM layout with .dma_bss section enforcement for system bus accessible memory |
| src/port/va416xx/syscalls.c | Newlib stubs routing printf to UART0, implementing heap management and minimal time functions |
| src/port/va416xx/startup.c | Reset handler performing .data copy, .bss/.dma_bss clearing, and C runtime initialization |
| src/port/va416xx/main.c | Application code with HAL initialization, GPIO/clock configuration, DHCP client, TCP echo/speed test server, and extensive TX self-test diagnostics |
| src/port/va416xx/ivt.c | Cortex-M4 interrupt vector table with 16 system + 64 external IRQ handlers |
| src/port/va416xx/hal_config.h | SDK HAL configuration defining SysTick parameters |
| src/port/va416xx/flash.jlink | J-Link script for flashing application binary via JTAG/SWD |
| src/port/va416xx/config.h | wolfIP stack configuration optimized for 64KB SRAM with 2 TCP sockets, 1 UDP socket, DHCP enabled |
| src/port/va416xx/board.h | Board-specific header including PEB1 EVK definitions |
| src/port/va416xx/README.md | Comprehensive documentation covering hardware specs, build instructions, architecture details, and testing procedures |
| src/port/va416xx/Makefile | Build system with SDK integration, relaxed warnings for external code, and workarounds for SDK issues |
| Makefile | Added cppcheck suppressions for va416xx startup and syscalls |
| .gitignore | Added va416xx/app.elf to ignore list |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| HAL_Iocfg_PinMux(PORTB, pin, 0); | ||
| PORTB->DIR &= ~(1U << pin); | ||
| { volatile uint32_t _s; for (_s = 0; _s < 20U; _s++) {} } | ||
| ones = 0; zeros = 0; | ||
| for (i = 0; i < 64U; i++) { | ||
| if (PORTB->DATAIN & (1U << pin)) ones++; else zeros++; | ||
| } | ||
| HAL_Iocfg_PinMux(PORTB, pin, 1); |
There was a problem hiding this comment.
HAL_Iocfg_PinMux should be called with VOR_PORTB instead of PORTB. All other calls to HAL_Iocfg_PinMux in this file (lines 126, 127, 149, 154) use the VOR_ prefix. Using PORTB instead of VOR_PORTB will likely cause a compilation error or incorrect behavior.
| HAL_Iocfg_PinMux(PORTB, pin, 0); | |
| PORTB->DIR &= ~(1U << pin); | |
| { volatile uint32_t _s; for (_s = 0; _s < 20U; _s++) {} } | |
| ones = 0; zeros = 0; | |
| for (i = 0; i < 64U; i++) { | |
| if (PORTB->DATAIN & (1U << pin)) ones++; else zeros++; | |
| } | |
| HAL_Iocfg_PinMux(PORTB, pin, 1); | |
| HAL_Iocfg_PinMux(VOR_PORTB, pin, 0); | |
| PORTB->DIR &= ~(1U << pin); | |
| { volatile uint32_t _s; for (_s = 0; _s < 20U; _s++) {} } | |
| ones = 0; zeros = 0; | |
| for (i = 0; i < 64U; i++) { | |
| if (PORTB->DATAIN & (1U << pin)) ones++; else zeros++; | |
| } | |
| HAL_Iocfg_PinMux(VOR_PORTB, pin, 1); |
f968a99 to
f5abb11
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 15 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /* Frame filter: promiscuous for initial bring-up */ | ||
| VOR_ETH->MAC_FRAME_FLTR = ETH_MAC_FRAME_FLTR_PR_Msk; |
There was a problem hiding this comment.
MAC frame filter is set to promiscuous mode (PR=1) unconditionally. This will accept all Ethernet traffic, increasing CPU load and potentially exposing traffic the stack shouldn't see. Consider configuring the filter to accept only unicast to our MAC + broadcast (and optionally required multicasts), and keep promiscuous mode behind a DEBUG/bring-up build flag.
| /* Frame filter: promiscuous for initial bring-up */ | |
| VOR_ETH->MAC_FRAME_FLTR = ETH_MAC_FRAME_FLTR_PR_Msk; | |
| /* Frame filter: | |
| * - In normal builds, use perfect DA filtering (unicast to our MAC + | |
| * broadcast) by leaving PR=0. | |
| * - In DEBUG_ETH builds, enable promiscuous mode to aid bring-up. */ | |
| #ifdef DEBUG_ETH | |
| VOR_ETH->MAC_FRAME_FLTR = ETH_MAC_FRAME_FLTR_PR_Msk; | |
| #else | |
| VOR_ETH->MAC_FRAME_FLTR = 0U; | |
| #endif |
| void *_sbrk(ptrdiff_t incr) | ||
| { | ||
| char *prev; | ||
| if (heap_end == 0) { | ||
| heap_end = (char *)&_ebss; | ||
| } | ||
| prev = heap_end; | ||
| if ((heap_end + incr) >= (char *)&_estack) { | ||
| errno = ENOMEM; | ||
| return (void *)-1; | ||
| } | ||
| heap_end += incr; | ||
| return prev; |
There was a problem hiding this comment.
_sbrk() grows the heap from _ebss up to _estack, but this port introduces a dedicated .dma_bss region in RAM1 starting at _sdma_bss. If malloc/newlib heap is used, it can grow into and corrupt the Ethernet DMA descriptors/buffers. Consider limiting the heap to _sdma_bss (or introducing an explicit _heap_end symbol in the linker script) so the heap cannot overlap .dma_bss.
| /* HAL_time_ms: millisecond tick counter maintained by SysTick ISR (10ms | ||
| * resolution by default). Used as the wolfIP `now` parameter so that all | ||
| * stack timers (DHCP, ARP, TCP retransmit, etc.) run in real wall-clock | ||
| * time rather than depending on CPU loop speed. */ |
There was a problem hiding this comment.
Comment states HAL_time_ms has 10ms resolution by default, but this port's hal_config.h sets SYSTICK_INTERVAL_MS to 1ms. Update this comment (and any dependent assumptions) to match the actual SysTick interval used by the HAL.
| /* HAL_time_ms: millisecond tick counter maintained by SysTick ISR (10ms | |
| * resolution by default). Used as the wolfIP `now` parameter so that all | |
| * stack timers (DHCP, ARP, TCP retransmit, etc.) run in real wall-clock | |
| * time rather than depending on CPU loop speed. */ | |
| /* HAL_time_ms: millisecond tick counter maintained by SysTick ISR (1ms | |
| * resolution on this port, matching SYSTICK_INTERVAL_MS). Used as the | |
| * wolfIP `now` parameter so that all stack timers (DHCP, ARP, TCP | |
| * retransmit, etc.) run in real wall-clock time rather than depending on | |
| * CPU loop speed. */ |
| /* 10. Main loop — use HAL_time_ms (SysTick-based, 10ms resolution) | ||
| * so wolfIP timers (TCP, ARP, etc.) run in real wall-clock time. */ | ||
| { |
There was a problem hiding this comment.
This comment says HAL_time_ms is SysTick-based with 10ms resolution, but hal_config.h configures SysTick for a 1ms interval. Align the comment (and any timing constants) with the configured tick interval.
| static uint32_t lfsr = 0x1A2B3C4DU; | ||
| lfsr ^= lfsr << 13; | ||
| lfsr ^= lfsr >> 17; | ||
| lfsr ^= lfsr << 5; |
There was a problem hiding this comment.
wolfIP_getrandom() uses an LFSR with a fixed, constant seed, which makes ephemeral ports (and potentially TCP ISNs) predictable across reboots. Consider seeding the PRNG from a per-device unique value and some runtime entropy (e.g., unique ID registers + timer jitter), or using a hardware RNG if available.
| static uint32_t lfsr = 0x1A2B3C4DU; | |
| lfsr ^= lfsr << 13; | |
| lfsr ^= lfsr >> 17; | |
| lfsr ^= lfsr << 5; | |
| /* LFSR-based PRNG with runtime seeding to avoid fixed, predictable | |
| * sequence across reboots. Seeded on first use from HAL_time_ms and | |
| * variable addresses to introduce per-boot and per-device variation. */ | |
| static uint32_t lfsr; | |
| static int seeded = 0; | |
| if (!seeded) { | |
| uint32_t seed = (uint32_t)HAL_time_ms; | |
| seed ^= (uint32_t)(uintptr_t)&lfsr; | |
| seed ^= (uint32_t)(uintptr_t)&seeded; | |
| if (seed == 0U) { | |
| /* Fallback to non-zero default only if entropy sources are all 0 */ | |
| seed = 0x1A2B3C4DU; | |
| } | |
| lfsr = seed; | |
| seeded = 1; | |
| } | |
| /* Stir in current time each call to add runtime variability. */ | |
| lfsr ^= (uint32_t)HAL_time_ms; | |
| lfsr ^= lfsr << 13; | |
| lfsr ^= lfsr >> 17; | |
| lfsr ^= lfsr << 5; |
| | `ivt.c` | Interrupt vector table (16 system + 64 external IRQs) | | ||
| | `syscalls.c` | Newlib stubs (`_write` routes to UART0) | | ||
| | `target.ld` | Linker script (Flash 256 KB, RAM 64 KB, `.dma_bss` in RAM1) | | ||
| | `hal_config.h` | SDK HAL configuration (SysTick 10 ms tick) | |
There was a problem hiding this comment.
README lists hal_config.h as providing a “SysTick 10 ms tick”, but hal_config.h in this port defines SYSTICK_INTERVAL_MS as 1ms. Update the README to match the actual tick interval so timing expectations (DHCP retry timing, diagnostics cadence, etc.) are correct.
| | `hal_config.h` | SDK HAL configuration (SysTick 10 ms tick) | | |
| | `hal_config.h` | SDK HAL configuration (SysTick 1 ms tick) | |
Summary
Features
va416xx_eth.c, 885 lines): DWC GMAC using normal (legacy) descriptor format with chain-mode TX and ring-mode RX, 3+3 DMA descriptors, polling (no interrupts)-DSPEED_TEST): Combined RX discard + TX chargen for performance measurement-DDEBUG_ETHfor per-frame DMA traces,-DTX_SELFTESTfor startup self-test with MAC loopback and PHY register dumpMeasured Throughput (10 Mbps MII)
Hardware-specific findings
The VA416xx DWC GMAC has several silicon quirks requiring workarounds:
Files added (15 files, +2,553 lines)
src/port/va416xx/va416xx_eth.csrc/port/va416xx/va416xx_eth.hsrc/port/va416xx/main.csrc/port/va416xx/config.hsrc/port/va416xx/Makefilesrc/port/va416xx/target.ld.dma_bss)src/port/va416xx/startup.c.data/.bssinit, SysTicksrc/port/va416xx/ivt.csrc/port/va416xx/syscalls.c_write)src/port/va416xx/hal_config.hsrc/port/va416xx/board.hsrc/port/va416xx/flash.jlinksrc/port/va416xx/README.mdRoot files modified:
.gitignore(addapp.elf),Makefile(add cppcheck suppressions for VA416xx port)Test plan
arm-none-eabi-gcc,-Werror, 48 KB)DEBUG_ETH,TX_SELFTEST,SPEED_TEST)