Skip to content

amd: fix the 64-bit AMBA data width and make the ZynqMP GEM instance selectable - #176

Open
dgarske wants to merge 8 commits into
wolfSSL:masterfrom
dgarske:zynqmp_gem_select
Open

dgarske wants to merge 8 commits into
wolfSSL:masterfrom
dgarske:zynqmp_gem_select

Conversation

@dgarske

@dgarske dgarske commented Sep 23, 2026 •

Copy link
Copy Markdown
Member

Fixes for the AMD Xilinx ZynqMP GEM port, all of which bite once it is used outside the ZCU102's default arrangement.

64-bit AMBA data width. NWCFG_DWIDTH_64 was set under #ifdef XILINX_AARCH64, a define only the zcu102 and versal Makefiles add. Built any other way the MAC runs a 32-bit datapath on a 64-bit master and never transmits: descriptors are armed, TSR.TXGO asserts, txcnt stays zero, while receive works normally. Now keyed off __aarch64__ as well. ARMv7 is unchanged, where the bit is inert.

Selecting the controller. GEM3 was hardcoded across four values that must agree: base address, interrupt, RST_LPD_IOU0 reset bit and CRL_APB clock register. -DZYNQMP_GEM_INDEX=1 moves all four together; the default stays 3, and an out-of-range index fails the build. Boards that share one MDIO bus across controllers can keep management where the pins are with -DZYNQMP_GEM_MDIO_INDEX, and pin the address with -DGEM_PHY_ADDR rather than relying on a scan that takes whichever PHY answers first. Where platform firmware has already set a controller up, -DZYNQMP_GEM_EXT_REF_CLK and -DZYNQMP_GEM_NO_RESET stop the driver reclocking or resetting it. A stamp file forces a rebuild when any of this changes, since make cannot see CFLAGS_EXTRA as a dependency.

Foreign PHYs. dp83867_init() soft-reset the PHY and wrote TI registers whatever it found, then read speed and duplex from TI's PHYSTS. On another vendor's part the reset discards what the platform configured, the indirect writes land in its MMD space, and the speed decode is meaningless: one board came up with the MAC at 10 Mbps half duplex against a gigabit link and passed no traffic. A PHY that is not a DP83867 is now left as its own init left it, only autonegotiation is restarted, and speed is resolved from the clause-22 registers every 802.3 PHY implements.

Diagnostics. -DDEBUG_PHY now reports the PHY's state on entry, before the driver writes anything: BMCR with PDOWN and ISOLATE decoded, the advertisement and partner registers, and every responding MDIO address with the pinned one marked. The autonegotiation and link waits are settable with -DGEM_PHY_ANEG_TIMEOUT_MS and -DGEM_PHY_LINK_TIMEOUT_MS.

SGMII, opt-in and untested. -DZYNQMP_GEM_SGMII selects the internal PCS (NWCFG.PCSSEL and the SGMII mode bit) and runs its clause-37 negotiation with the PHY, which is separate from the PHY's copper negotiation with the link partner. It implies -DZYNQMP_GEM_EXT_REF_CLK, since on SGMII the reference comes from the PS-GTR serdes and a speed downshift must not reprogram CRL_APB. There is no SGMII board here to exercise it on, so it is off by default and should be treated as a starting point rather than a working configuration: the serdes must already be up, and a board may need lane or PCS setup beyond this.

Test status

boards/zcu102 builds for the default, each GEM index, the MDIO split, a pinned PHY address, both clock and reset options, SGMII, DEBUG_PHY, and extended timeouts; out-of-range index, PHY address and timeout values all fail the build as intended. boards/zynq7000 is unchanged and still clean for ARMv7.

Hardware, on a ZCU102 with a DP83867 on GEM3: the data-width fix took the board from transmitting nothing to a peer answering in 1 ms, and after the PHY changes the default path still detects the DP83867 and negotiates 1000 Mbps full duplex. Forcing a GEM with no PHY reproduces no PHY responding on MDIO as expected. The foreign-PHY, MDIO-split and SGMII paths have no hardware here and are not exercised.

Copilot AI lite review requested due to automatic review settings September 23, 2026 18:46
@dgarske dgarske self-assigned this Sep 23, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🔵 Needs a closer look

Incremental builds may reuse GEM3 objects when switching the GEM selector without a clean rebuild.

Review effort: Lite
Findings: None

What changed in this PR

Updates ZynqMP GEM support for correct AArch64 data width and selectable GEM instances.

Changes:

  • Enables 64-bit AMBA mode for AArch64 builds.
  • Selects GEM address, IRQ, reset, and clock settings by index.
  • Supports externally managed SGMII reference clocks.
File Description
src/​port/​amd/​common/​gem_core.c Fixes AArch64 GEM data-width configuration.
src/​port/​amd/​boards/​zcu102/​board.h Adds selectable GEM instance mappings.
src/​port/​amd/​boards/​zcu102/​board_gem.c Applies selected clock/reset settings and external-clock handling.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@dgarske dgarske assigned danielinux and unassigned danielinux Sep 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants