Uh oh!
There was an error while loading. Please reload this page.
zynq: add optional ZynqMP GEM Ethernet datapath bring-up - #845
Conversation
There was a problem hiding this comment.
Pull request overview
Adds an opt-in ZynqMP-specific helper to initialize Cadence GEM Ethernet MAC datapaths for fixed-link SGMII ports, allowing wolfBoot to leave Ethernet hardware in the state the OS expects when wolfBoot replaces U-Boot.
Changes:
- Introduces
WOLFBOOT_ZYNQMP_GEM_INITwith a board-providedZYNQMP_GEM_INIT_LISTfor{gem_base, speed_mbps}initialization entries. - Implements
zynqmp_gem_init()to program GEM NWCFG/PCS/NWCTRL for fixed-link bring-up duringhal_init(). - Updates ZynqMP target documentation and example configs to describe and enable the new optional bring-up path.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| hal/zynq.h | Adds GEM bring-up configuration, register offsets, and bit definitions for fixed-link SGMII initialization. |
| hal/zynq.c | Adds the zynqmp_gem_init() routine and wires it into hal_init() behind a build-time option. |
| docs/Targets.md | Documents when to use PHY init vs GEM datapath bring-up and how to configure each. |
| config/examples/zynqmp.config | Adds example configuration snippets for enabling and defining GEM datapath bring-up. |
| config/examples/zynqmp_sdcard.config | Adds example configuration snippets for enabling and defining GEM datapath bring-up for the sdcard target. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.
Suppressed comments (3)
hal/zynq.c:2210
- The GEM base address is
uintptr_tbut is logged via(unsigned int)and%08x, which truncates addresses on 64-bit targets (AArch64 ZynqMP) and can make diagnostics misleading. Prefer printing addresses with a pointer/uintptr_t-safe format (e.g.,%pwith(void*)gems[i].base, orPRIxPTR/PRIuPTRwith an appropriate cast) so logs remain correct across 32/64-bit builds.
wolfBoot_printf("GEM init @0x%08x: bad speed %d, skipped\n",
(unsigned int)gems[i].base, (int)gems[i].speed);
hal/zynq.c:2225
- The GEM base address is
uintptr_tbut is logged via(unsigned int)and%08x, which truncates addresses on 64-bit targets (AArch64 ZynqMP) and can make diagnostics misleading. Prefer printing addresses with a pointer/uintptr_t-safe format (e.g.,%pwith(void*)gems[i].base, orPRIxPTR/PRIuPTRwith an appropriate cast) so logs remain correct across 32/64-bit builds.
wolfBoot_printf("GEM init @0x%08x NWCFG 0x%08x != 0x%08x "
"(gated/in reset?)\n", (unsigned int)gems[i].base,
(unsigned int)rb, (unsigned int)nwcfg);
}
else {
wolfBoot_printf("GEM init @0x%08x %dMbps\n",
(unsigned int)gems[i].base, (int)gems[i].speed);
docs/Targets.md:3813
- The inline example for defining
ZYNQMP_GEM_INIT_LISTviaCFLAGS_EXTRA+=-D...is easy to copy into a shell or CI environment where{...,...}can trigger brace expansion if not quoted. Consider updating the docs to recommend quoting the-Dvalue (or preferring the header-based approach) so users don’t hit hard-to-diagnose build issues when enabling this from a command line.
Some ZynqMP ports are on a fixed link (a backplane, or an SGMII connection with no PHY that the OS driver auto-configures) and rely on the bootloader having brought the GEM datapath up. U-Boot's `zynq_gem` driver does this the first time it uses an interface (for example a `tftpboot`); when wolfBoot replaces U-Boot that step is lost and those ports stay down even though the OS enumerates them. Enable `CFLAGS_EXTRA+=-DWOLFBOOT_ZYNQMP_GEM_INIT` and list the affected GEMs as `{gem_base, speed_mbps}` rows in `ZYNQMP_GEM_INIT_LIST`, either directly (`CFLAGS_EXTRA+=-DZYNQMP_GEM_INIT_LIST={0xFF0B0000UL,1000},{0xFF0C0000UL,1000}`, no spaces) or from a header (`CFLAGS_EXTRA+=-DZYNQMP_GEM_INIT_HEADER='"myboard_gem.h"'` with `#define ZYNQMP_GEM_INIT_LIST { 0xFF0B0000UL, 1000 }, { 0xFF0C0000UL, 1000 }`) - GEM0 `0xFF0B0000` .. GEM3 `0xFF0E0000`, speed 1000/100/10. For each GEM wolfBoot writes `NWCFG` (SGMII + PCS select + speed) and the PCS control register (fixed link, auto-negotiation disabled) - the same registers U-Boot's driver sets. RX/TX are deliberately left off: the OS driver enables them after it programs its DMA descriptor rings (wolfBoot has none), so enabling RX here could DMA a received frame to an arbitrary address during image verify. The GEM must already be clocked and out of reset by the FSBL `psu_init`. This is distinct from the PHY init above (which configures an external MDIO PHY); the two helpers must target different GEMs, and if both name the same base this full `NWCFG` write wins - override `GEMI_CFG_MDCDIV` to reconcile the MDC divisor.
No description provided.