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

src/port/amd/boards/zcu102/Makefile

@@ -155,6 +155,21 @@ $(LAYOUT_STAMP): FORCE
 	fi
 $(OBJS): $(LAYOUT_STAMP)
 
+# make cannot see CFLAGS_EXTRA as a dependency, so changing it - the GEM
+# selection above all - would silently relink objects built with the previous
+# value and look like a dead PHY. Same stamp trick as LAYOUT. The stamp must
+# also be created when the value is empty, which is the default build, or the
+# rule has no output file and every make rebuilds everything.
+CFLAGS_EXTRA_STAMP := .cflags_extra_stamp
+$(CFLAGS_EXTRA_STAMP): FORCE
+	@if [ ! -f $@ ]; then \
+	    echo "$(CFLAGS_EXTRA)" > $@; \
+	elif [ "`cat $@`" != "$(CFLAGS_EXTRA)" ]; then \
+	    echo "CFLAGS_EXTRA changed; forcing rebuild"; \
+	    echo "$(CFLAGS_EXTRA)" > $@; \
+	fi
+$(OBJS): $(CFLAGS_EXTRA_STAMP)
+
 all: app.elf
 	@echo "Built: app.elf"
 	@$(SIZE) app.elf
@@ -198,7 +213,7 @@ bootbin:
 	FSBL_ELF=$$FSBL_ELF APP_ELF=$$PWD/app.elf bootgen/build_bootbin.sh
 
 clean:
-	rm -f $(OBJS) app.elf BOOT.BIN $(LAYOUT_STAMP)
+	rm -f $(OBJS) app.elf $(CFLAGS_EXTRA_STAMP) BOOT.BIN $(LAYOUT_STAMP)
 
 .PHONY: all clean bootbin help
 

src/port/amd/boards/zcu102/board.h

@@ -55,9 +55,44 @@
 #define GEM2_BASE               0xFF0D0000UL
 #define GEM3_BASE               0xFF0E0000UL
 
-/* On-board RJ45 is GEM3 on the ZCU102. */
-#define GEM_BASE                GEM3_BASE
-#define IRQ_GEM                 IRQ_GEM3
+/* Which GEM drives the Ethernet port (ZCU102 on-board RJ45 is GEM3). The
+ * base address, interrupt, reset bit and clock register all follow this
+ * index, deliberately as one knob: set individually they drift, and you end
+ * up driving one GEM while clocking another. */
+#ifndef ZYNQMP_GEM_INDEX
+#define ZYNQMP_GEM_INDEX        3
+#endif
+#if (ZYNQMP_GEM_INDEX < 0) || (ZYNQMP_GEM_INDEX > 3)
+#error "ZYNQMP_GEM_INDEX must be 0, 1, 2 or 3"
+#endif
+
+#define GEM_BASE                (GEM0_BASE + ((ZYNQMP_GEM_INDEX) * 0x10000UL))
+#define IRQ_GEM                 (32 + 57 + ((ZYNQMP_GEM_INDEX) * 2))
+
+/* ZynqMP designs often wire one MDIO bus and hang every PHY off it, so the
+ * controller that reaches the PHY need not be the one carrying the data.
+ * Defaults to the data GEM; set ZYNQMP_GEM_MDIO_INDEX when they differ. */
+#ifndef ZYNQMP_GEM_MDIO_INDEX
+#define ZYNQMP_GEM_MDIO_INDEX   ZYNQMP_GEM_INDEX
+#endif
+#if (ZYNQMP_GEM_MDIO_INDEX < 0) || (ZYNQMP_GEM_MDIO_INDEX > 3)
+#error "ZYNQMP_GEM_MDIO_INDEX must be 0, 1, 2 or 3"
+#endif
+#define GEM_MDIO_BASE           (GEM0_BASE + ((ZYNQMP_GEM_MDIO_INDEX) * 0x10000UL))
+/* When it differs from the data GEM, that controller must already be out of
+ * reset with its APB clock running - normally true, since platform firmware
+ * brings up the GEM it wired the MDIO pins to. We deliberately do not reset
+ * or reclock it: another driver may own it. If it is not up, MDIO simply
+ * finds no PHY and init fails with a message rather than misbehaving. */
+
+/* Optional: pin the PHY's MDIO address instead of scanning for it. Needed
+ * when several PHYs share the bus, since the scan takes whichever answers
+ * first. On ZynqMP this should match wolfBoot's ZYNQMP_PHY_ADDR.
+ *   -DGEM_PHY_ADDR=0x0F
+ */
+#if defined(GEM_PHY_ADDR) && ((GEM_PHY_ADDR) < 0 || (GEM_PHY_ADDR) > 31)
+#error "GEM_PHY_ADDR must be an MDIO address in the range 0-31"
+#endif
 
 #define CRL_APB_BASE            0xFF5E0000UL
 #define IOU_SLCR_BASE           0xFF180000UL
@@ -81,8 +116,10 @@
 /* ---------------------------------------------------------------------
  * CRL_APB clock and reset registers
  * ------------------------------------------------------------------- */
-#define CRL_APB_GEM3_REF_CTRL   (CRL_APB_BASE + 0x5C)
-#define CRL_APB_RST_LPD_IOU0    (CRL_APB_BASE + 0x230)  /* GEM3 reset bit 3 */
+/* GEM0_REF_CTRL at 0x50, four consecutive words. RST_LPD_IOU0 bit N = GEMn. */
+#define CRL_APB_GEM_REF_CTRL    (CRL_APB_BASE + 0x50 + ((ZYNQMP_GEM_INDEX) * 4))
+#define CRL_APB_RST_LPD_IOU0    (CRL_APB_BASE + 0x230)
+#define CRL_RST_GEM             (1u << (ZYNQMP_GEM_INDEX))
 
 /* ---------------------------------------------------------------------
  * PS UART0 (Cadence) - on-board USB-UART on ZCU102 via U104 FT4232

src/port/amd/boards/zcu102/board_gem.c

@@ -4,30 +4,35 @@
  *
  * This file is part of wolfIP TCP/IP stack.
  *
- * ZCU102 (ZynqMP) GEM clock/reset hooks for the shared GEM core. The GEM3
- * reference clock and reset live in CRL_APB, which bare-metal may poke on
- * ZynqMP.
+ * ZCU102 (ZynqMP) GEM clock/reset hooks for the shared GEM core. The selected
+ * GEM's clock and reset live in CRL_APB. See ZYNQMP_GEM_INDEX in board.h.
  */
 #include <stdint.h>
 #include "board.h"
 #include "gem_port.h"
 #include "timer.h"   /* delay_us / delay_ms - deterministic, counter-backed */
 
-#define CRL_RST_GEM3        (1u << 3)   /* GEM3 reset bit in RST_LPD_IOU0 */
 
 void gem_soc_pre_init(void)
 {
     /* No SoC quirk needed before MAC config on ZynqMP. */
 }
 
-/* Configure CRL_APB.GEM3_REF_CTRL for the negotiated link speed. The MAC
- * sources TX_CLK to the PHY at this rate (RGMII): 125/25/2.5 MHz for
- * 1G/100M/10M. IOPLL = 1500 MHz, /12 base. Register layout (TRM):
- * CLKACT bit26, CLKACT_RX bit25, DIVISOR1 [21:16], DIVISOR0 [13:8],
- * SRCSEL [2:0]. */
+/* Drive TX_CLK at 125/25/2.5 MHz for 1G/100M/10M from IOPLL 1500 MHz, /12
+ * base. Register layout (TRM): CLKACT bit26, CLKACT_RX bit25, DIVISOR1
+ * [21:16], DIVISOR0 [13:8], SRCSEL [2:0]. RGMII and GMII both need this.
+ *
+ * SGMII does not: the reference comes from the PS-GTR serdes and platform
+ * firmware owns it, so writing here would undo a working setup. SGMII boards
+ * build with -DZYNQMP_GEM_EXT_REF_CLK; nothing else should. */
 void gem_set_ref_clk(int speed_mbps)
 {
-    volatile uint32_t *gem3_ref = (volatile uint32_t *)CRL_APB_GEM3_REF_CTRL;
+/* SGMII implies it: the reference comes from the PS-GTR serdes, so a
+ * downshift must not reprogram CRL_APB and take the link down. */
+#if defined(ZYNQMP_GEM_EXT_REF_CLK) || defined(ZYNQMP_GEM_SGMII)
+    (void)speed_mbps;
+#else
+    volatile uint32_t *gem_ref = (volatile uint32_t *)CRL_APB_GEM_REF_CTRL;
     uint32_t div1;
     uint32_t val;
 
@@ -42,20 +47,27 @@ void gem_set_ref_clk(int speed_mbps)
         | ((div1 & 0x3Fu) << 16)   /* DIVISOR1 */
         | ((12u  & 0x3Fu) << 8)    /* DIVISOR0 */
         | (0u);                    /* SRCSEL = IOPLL */
-    *gem3_ref = val;
+    *gem_ref = val;
+#endif
 }
 
-/* Pulse the GEM3 reset bit so the MAC starts from a known state, then
- * force the 125 MHz reference (amd_eth_init downshifts later if the PHY
- * negotiates 100/10). */
+/* Pulse this GEM's reset so the MAC starts clean, then force 125 MHz
+ * (amd_eth_init downshifts later if the PHY negotiates 100/10).
+ *
+ * Build with -DZYNQMP_GEM_NO_RESET where platform firmware has already set
+ * the controller up and the reset would discard that, for instance when the
+ * link runs off an externally supplied reference. Pair it with
+ * -DZYNQMP_GEM_EXT_REF_CLK to leave the clock alone as well. */
 void gem_clk_reset(void)
 {
+#ifndef ZYNQMP_GEM_NO_RESET
     volatile uint32_t *rst = (volatile uint32_t *)CRL_APB_RST_LPD_IOU0;
 
-    *rst |= CRL_RST_GEM3;
+    *rst |= CRL_RST_GEM;
     delay_us(10);                /* hold the reset asserted */
-    *rst &= ~CRL_RST_GEM3;
+    *rst &= ~CRL_RST_GEM;
     delay_ms(10);                /* settle after deassert (counter-backed) */
+#endif
 
     gem_set_ref_clk(1000);
 }

src/port/amd/common/gem_core.c

@@ -78,7 +78,7 @@ static int mdio_wait_idle(void)
 {
     int spin;
     for (spin = 0; spin < 100000; spin++) {
-        if (GEM_NWSR & NWSR_PHY_IDLE)
+        if (GEM_MDIO_NWSR & NWSR_PHY_IDLE)
             return 0;
     }
     return -1;
@@ -262,7 +262,9 @@ void gem_dump_state(void)
  * ------------------------------------------------------------------- */
 int amd_eth_init(struct wolfIP_ll_dev *ll)
 {
-    uint8_t addr;
+#ifndef GEM_PHY_ADDR
+    uint8_t addr;   /* scan cursor; unused when the address is pinned */
+#endif
     uint16_t id1;
     int found_phy;
     int speed;
@@ -295,13 +297,31 @@ int amd_eth_init(struct wolfIP_ll_dev *ll)
               | NWCFG_1536RXEN
               | NWCFG_MCASTHASHEN
               | (5u << NWCFG_MDCDIV_SHIFT);
-#ifdef XILINX_AARCH64
+#if defined(XILINX_AARCH64) || defined(__aarch64__)
     /* 64-bit AMBA data width: appropriate on the AArch64 SoCs (ZynqMP /
      * Versal). The Zynq-7000 GEM is fed by a 32-bit AXI master, where this
-     * bit is inert, so it is left clear there. */
+     * bit is inert, so it is left clear there.
+     *
+     * Keyed off the compiler as well as XILINX_AARCH64, which only the board
+     * Makefiles set: a consumer building these sources its own way loses the
+     * bit, and a 64-bit master with a 32-bit datapath never transmits
+     * (TSR.TXGO stuck, zero octets) while receive keeps working. */
     GEM_NWCFG |= NWCFG_DWIDTH_64;
 #endif
 
+#ifdef ZYNQMP_GEM_SGMII
+    /* SGMII: the MAC talks to the PHY through the internal PCS rather than a
+     * parallel RGMII/GMII interface, so the PCS has to be selected and its
+     * own clause-37 negotiation run. This is separate from, and in addition
+     * to, the PHY's copper negotiation with the link partner.
+     *
+     * UNTESTED. There is no SGMII board here to exercise it on, so it is
+     * opt-in and off by default. Treat it as a starting point: the PS-GTR
+     * serdes must already be up (platform firmware's job), and a board may
+     * need its own lane or PCS setup beyond this. */
+    GEM_NWCFG |= NWCFG_PCSSEL | NWCFG_SGMIIEN;
+#endif
+
     /* DMACR: AHB fixed burst 16 beats, RX buffer 1536/64=24, TX/RX packet
      * buffer memory at max. Do NOT set bit 30 (DMA_ADDR_BUS_WIDTH 64-bit):
      * that selects 16-byte BD format with addr_hi and would break the
@@ -354,13 +374,60 @@ int amd_eth_init(struct wolfIP_ll_dev *ll)
     /* Enable MDIO so we can talk to the PHY. */
     GEM_NWCTRL |= NWCTRL_MDEN;
 
+#if (GEM_MDIO_BASE != GEM_BASE)
+    /* The PHY answers on another controller's management bus, so that one
+     * needs its own MDC divisor and management enable; the block we just
+     * configured is only carrying data. Everything else about it is left
+     * alone, since another driver may own it. */
+    GEM_MDIO_NWCFG = (GEM_MDIO_NWCFG & ~(7u << NWCFG_MDCDIV_SHIFT))
+                   | (5u << NWCFG_MDCDIV_SHIFT);
+    GEM_MDIO_NWCTRL |= NWCTRL_MDEN;
+#endif
+
     /* Scan all 32 MDIO addresses, reporting each responsive PHY's ID and
      * link status (BMSR reg 1, bit 2). A board may present more than one
      * PHY on the bus; prefer one that already has copper link so we
      * configure the PHY wired to the on-board RJ45 rather than the first
      * responder. */
     found_phy = 0;
     gem_phy_addr = 0;
+#ifdef GEM_PHY_ADDR
+    /* The board pins the address. Scanning cannot be trusted where several
+     * PHYs share one MDIO bus: it takes the first that answers, which may
+     * belong to a different GEM than the one carrying our data. */
+    gem_phy_addr = (uint8_t)(GEM_PHY_ADDR);
+#ifdef DEBUG_PHY
+    /* Still report every responder: which addresses answer, and how their
+     * link state compares, is what tells you whether the pinned one is the
+     * port you meant. */
+    {
+        uint8_t a;
+        uint16_t sid, sbmsr;
+        for (a = 0; a < 32; a++) {
+            sid = 0;
+            if (gem_mdio_read(a, 0x02, &sid) != 0 || sid == 0xFFFFu || sid == 0)
+                continue;
+            sbmsr = 0;
+            (void)gem_mdio_read(a, 0x01, &sbmsr);
+            uart_puts("MDIO scan: addr="); uart_puthex(a);
+            uart_puts(" id1=");            uart_puthex(sid);
+            uart_puts(" bmsr=");           uart_puthex(sbmsr);
+            uart_puts((sbmsr & 0x0004u) ? " LINK" : "");
+            uart_puts((a == gem_phy_addr) ? " <- pinned\n" : "\n");
+        }
+    }
+#endif
+    if (gem_mdio_read(gem_phy_addr, 0x02, &id1) == 0
+            && id1 != 0xFFFFu && id1 != 0) {
+        found_phy = 1;
+    }
+    if (!found_phy) {
+        uart_puts("GEM: no PHY at the configured MDIO address ");
+        uart_puthex(gem_phy_addr);
+        uart_puts("\n");
+        return -10;
+    }
+#else
     {
         uint16_t bmsr;
         for (addr = 0; addr < 32; addr++) {
@@ -384,6 +451,7 @@ int amd_eth_init(struct wolfIP_ll_dev *ll)
             return -10;
         }
     }
+#endif
     /* Re-read id1 for the selected PHY so the vendor dispatch is correct
      * even when the scan broke early on a linked PHY. */
     (void)gem_mdio_read(gem_phy_addr, 0x02, &id1);
@@ -412,6 +480,32 @@ int amd_eth_init(struct wolfIP_ll_dev *ll)
         gem_set_ref_clk(speed);
     }
 
+#ifdef ZYNQMP_GEM_SGMII
+    /* Run the PCS side once the copper link is up. Failure is reported but
+     * not fatal: the MAC is still usable if a board's PCS is brought up
+     * elsewhere, and returning an error here would take down a link that
+     * may be working. UNTESTED - see the note at the NWCFG bits above. */
+    {
+        uint32_t pcs;
+        int spin;
+
+        GEM_PCS_AN_ADV = 0x0020u;   /* full duplex, no pause */
+        pcs = GEM_PCS_CTRL;
+        GEM_PCS_CTRL = pcs | PCS_CTRL_ANEN | PCS_CTRL_ANRESTART;
+        for (spin = 0; spin < 2000000; spin++) {
+            if (GEM_PCS_STATUS & PCS_STATUS_ANDONE)
+                break;
+        }
+        pcs = GEM_PCS_STATUS;
+        uart_puts("GEM: PCS");
+        uart_puts((pcs & PCS_STATUS_ANDONE) ? " autoneg done" : " autoneg TIMEOUT");
+        uart_puts((pcs & PCS_STATUS_LINK) ? " link up" : " link down");
+        uart_puts(" lp=");
+        uart_puthex(GEM_PCS_AN_LP_BASE);
+        uart_puts("\n");
+    }
+#endif
+
     /* Arm the RX delivery model (install IRQ handler, or leave masked for
      * poll-only ports) and enable RX/TX. */
     gem_rx_install();

src/port/amd/common/gem_regs.h

@@ -30,7 +30,17 @@
 #define GEM_IER             (*(volatile uint32_t *)(GEM_BASE + 0x028))
 #define GEM_IDR             (*(volatile uint32_t *)(GEM_BASE + 0x02C))
 #define GEM_IMR             (*(volatile uint32_t *)(GEM_BASE + 0x030))
-#define GEM_PHYMNTNC        (*(volatile uint32_t *)(GEM_BASE + 0x034))
+/* MDIO lives on GEM_MDIO_BASE, which is GEM_BASE unless the board says the
+ * PHY is reached through a different controller. */
+#ifndef GEM_MDIO_BASE
+#define GEM_MDIO_BASE       GEM_BASE
+#endif
+#define GEM_PHYMNTNC        (*(volatile uint32_t *)(GEM_MDIO_BASE + 0x034))
+#define GEM_MDIO_NWCTRL     (*(volatile uint32_t *)(GEM_MDIO_BASE + 0x000))
+#define GEM_MDIO_NWCFG      (*(volatile uint32_t *)(GEM_MDIO_BASE + 0x004))
+/* PHY_IDLE is per controller: it tracks the block that issued the transaction,
+ * so it has to be read from the one owning PHYMNTNC above. */
+#define GEM_MDIO_NWSR       (*(volatile uint32_t *)(GEM_MDIO_BASE + 0x008))
 #define GEM_HASHL           (*(volatile uint32_t *)(GEM_BASE + 0x080))
 #define GEM_HASHH           (*(volatile uint32_t *)(GEM_BASE + 0x084))
 #define GEM_LADDR1L         (*(volatile uint32_t *)(GEM_BASE + 0x088))
@@ -76,6 +86,21 @@
 #define NWCFG_MDCDIV_SHIFT  18u
 #define NWCFG_MDCDIV_MASK   (7u << 18)
 #define NWCFG_DWIDTH_64     (1u << 21)  /* Data bus width = 64 bit (AArch64) */
+#define NWCFG_PCSSEL        (1u << 11)  /* Use the internal PCS (SGMII) */
+#define NWCFG_SGMIIEN       (1u << 27)  /* SGMII mode, with PCSSEL */
+
+/* PCS block, used only in SGMII mode. The PCS carries its own clause-37
+ * autonegotiation with the PHY, separate from the PHY's copper-side
+ * negotiation with the link partner. */
+#define GEM_PCS_CTRL        (*(volatile uint32_t *)(GEM_BASE + 0x200))
+#define GEM_PCS_STATUS      (*(volatile uint32_t *)(GEM_BASE + 0x204))
+#define GEM_PCS_AN_ADV      (*(volatile uint32_t *)(GEM_BASE + 0x210))
+#define GEM_PCS_AN_LP_BASE  (*(volatile uint32_t *)(GEM_BASE + 0x214))
+
+#define PCS_CTRL_ANEN       (1u << 12)  /* autonegotiation enable */
+#define PCS_CTRL_ANRESTART  (1u << 9)   /* restart autonegotiation */
+#define PCS_STATUS_ANDONE   (1u << 5)   /* autonegotiation complete */
+#define PCS_STATUS_LINK     (1u << 2)   /* PCS link up */
 
 #define NWSR_PHY_IDLE       (1u << 2)