Clean up Agilex 5 shared platform helpers

Author: aidangarske

.github/workflows/test-build-agilex5.yml

@@ -176,6 +176,9 @@ ifeq ($(ARCH),AARCH64)
     CFLAGS+=-DWOLFBOOT_DUALBOOT -DWOLFBOOT_UBOOT_LEGACY
     # BL31 owns GICv3 and secondary-core/PSCI setup. Do not touch RVBAR.
     CFLAGS+=-DSKIP_RVBAR=1 -DSKIP_GIC_INIT=1
+    ifeq ($(DEBUG_UART),1)
+      OBJS+=hal/uart/ns16550.o
+    endif
     # Full SDHCI host reset: SPL leaves the Cadence host in UHS-I 1.8V/SDR104,
     # which mutes CMD8 against the 3.3V re-powered card (hardware-verified).
   endif

arch.mk

@@ -52,7 +52,7 @@ CFLAGS_EXTRA+=-DLINUX_BOOTARGS_ROOT=\"/dev/mmcblk0p4\"
 # Board settings, see docs/Agilex5.md. Defaults match the 013B dev kit.
 #CFLAGS_EXTRA+=-DDEBUG_UART_NUM=1
 #CFLAGS_EXTRA+=-DAGILEX5_DDR_SIZE=0x70000000
-CFLAGS_EXTRA+=-DAGILEX5_FDT_DISABLE_PATHS=\"/leds\"
+CFLAGS_EXTRA+=-DWOLFBOOT_FDT_DISABLE_PATHS=\"/leds\"
 
 # SD6HC uses SDMA for cache-coherent multi-block transfers.
 CFLAGS_EXTRA+=-DSDHCI_FORCE_CARD_DETECT

config/examples/agilex5_013b_sdcard.config

@@ -22,11 +22,11 @@ make clean
 make -j"$(nproc)"
 ```
 
-The hosted build is also covered by the `Agilex 5 wolfBoot build` GitHub
-Actions workflow. It installs the same freestanding AArch64 toolchain used by
-the other Cortex-A ports and builds this configuration on every relevant
-pull request; hardware is required only for the SD-card and Linux handoff
-steps below.
+The hosted build is covered by the Agilex 5 job in the `Test Example Configs`
+GitHub Actions workflow. It installs the same freestanding AArch64 toolchain
+used by the other Cortex-A ports and builds this configuration on every pull
+request; hardware is required only for the SD-card and Linux handoff steps
+below.
 
 The default RSA-4096/SHA3-384 key is for development only. Production builds
 must supply controlled signing keys and keep them separate from both the SDM
@@ -43,10 +43,10 @@ their config:
 
 | Setting | Default | Purpose |
 |---|---|---|
-| `CFLAGS_EXTRA+=-DDEBUG_UART_NUM=1` | UART0 (`0x10C02000`) | console UART for wolfBoot and Linux `earlycon` |
+| `CFLAGS_EXTRA+=-DDEBUG_UART_NUM=<0\|1>` | UART0 (`0x10C02000`) | select UART0 or UART1 (`0x10C02100`) for wolfBoot and Linux `earlycon` |
 | `CFLAGS_EXTRA+=-DDEBUG_UART_BASE=<addr>` | unset | any other console UART; write the address without a `UL` suffix |
 | `CFLAGS_EXTRA+=-DAGILEX5_DDR_SIZE=<bytes>` | `0x70000000` (1792 MiB) | DDR size written to the DTB memory node |
-| `CFLAGS_EXTRA+=-DAGILEX5_FDT_DISABLE_PATHS=\"/leds\"` | unset | comma-separated DTB node paths to mark `disabled`, for hardware behind the unprogrammed FPGA |
+| `CFLAGS_EXTRA+=-DWOLFBOOT_FDT_DISABLE_PATHS=\"/leds\"` | unset | comma-separated DTB node paths to mark `disabled`, for hardware behind the unprogrammed FPGA |
 
 wolfBoot does not read the DDR size from the DTB, because without U-Boot the
 memory node holds whatever the DTB author wrote rather than the fitted size.

docs/Agilex5.md

@@ -27,6 +27,7 @@
 #include "fdt.h"
 #include "hal.h"
 #include "hal/agilex5.h"
+#include "aarch64_arch.h"
 #include "image.h"
 #include "printf.h"
 
@@ -46,70 +47,36 @@
 #endif
 
 #ifdef DEBUG_UART
-#define UART_RBR 0U
-#define UART_THR 0U
-#define UART_LSR 5U
-#define UART_LSR_THRE (1U << 5)
-#define UART_LSR_TEMT (1U << 6)
-#define UART_TIMEOUT 1000000U
-#define UART_REG(_n) \
-    (*(volatile uint32_t *)((uintptr_t)DEBUG_UART_BASE + \
-        ((uintptr_t)(_n) << 2)))
+#include "ns16550.h"
+
+static struct ns16550_dev uart_console;
 
 void uart_init(void)
 {
-    /* SPL configured the DesignWare APB UART, its clock and pinmux. */
+    uart_console.base = (uintptr_t)DEBUG_UART_BASE;
+    uart_console.reg_shift = 2;
+    uart_console.reg_off = 0;
+    uart_console.clk_hz = 0;
+    uart_console.io_width = 4;
+    uart_console.crlf = 1;
+    /* Keep the baud, clock and pinmux settings inherited from SPL. */
 }
 
 void uart_write(const char *buf, uint32_t len)
 {
-    uint32_t i;
-
-    for (i = 0; i < len; i++) {
-        uint32_t timeout = UART_TIMEOUT;
-        if (buf[i] == '\n') {
-            while (((UART_REG(UART_LSR) & UART_LSR_THRE) == 0U) &&
-                    (--timeout != 0U)) {}
-            if (timeout != 0U)
-                UART_REG(UART_THR) = '\r';
-            timeout = UART_TIMEOUT;
-        }
-        while (((UART_REG(UART_LSR) & UART_LSR_THRE) == 0U) &&
-                (--timeout != 0U)) {}
-        if (timeout != 0U)
-            UART_REG(UART_THR) = (uint32_t)(uint8_t)buf[i];
-    }
+    (void)ns16550_write(&uart_console, buf, len);
 }
 #else
 #define uart_init() do {} while (0)
 #endif
 
-static uint64_t timer_count(void)
-{
-    uint64_t count;
-    __asm__ volatile("mrs %0, cntpct_el0" : "=r"(count));
-    return count;
-}
-
-static uint64_t timer_frequency(void)
-{
-    uint64_t frequency;
-    __asm__ volatile("mrs %0, cntfrq_el0" : "=r"(frequency));
-    return frequency;
-}
-
 uint64_t hal_get_timer_us(void)
 {
-    uint64_t frequency = timer_frequency();
-    uint64_t count = timer_count();
+    uint64_t frequency = timer_get_freq();
+    uint64_t count = timer_get_count();
     uint64_t whole;
     uint64_t rem;
 
-    /* BL31 normally programs CNTFRQ_EL0. Keep delay loops live even when a
-     * non-conforming handoff leaves it clear.
-     */
-    if (frequency == 0U)
-        frequency = 100000000U;
     /* Split the conversion so it stays in 64-bit math: a __uint128_t divide
      * pulls in libgcc's __udivti3, which the bare-metal link has no runtime
      * for. The whole and remainder parts are exact and cannot overflow. */
@@ -129,11 +96,6 @@ void hal_init(void)
 
 void hal_prepare_boot(void)
 {
-#ifdef DEBUG_UART
-    uint32_t timeout = UART_TIMEOUT;
-    while (((UART_REG(UART_LSR) & UART_LSR_TEMT) == 0U) &&
-            (--timeout != 0U)) {}
-#endif
     __asm__ volatile("dsb sy" : : : "memory");
     __asm__ volatile("ic iallu" : : : "memory");
     __asm__ volatile("dsb sy" : : : "memory");
@@ -156,16 +118,16 @@ void *hal_get_dts_update_address(void)
 }
 
 #ifdef __WOLFBOOT
-#ifdef AGILEX5_FDT_DISABLE_PATHS
-static const char* const agilex5_fdt_disable[] = { AGILEX5_FDT_DISABLE_PATHS };
+#ifdef WOLFBOOT_FDT_DISABLE_PATHS
+static const char* const fdt_disable_paths[] = { WOLFBOOT_FDT_DISABLE_PATHS };
 #endif
 
 int hal_dts_fixup(void *dts_addr, uint32_t capacity)
 {
     fdt_ctx ctx;
     int off;
     int ret;
-#ifdef AGILEX5_FDT_DISABLE_PATHS
+#ifdef WOLFBOOT_FDT_DISABLE_PATHS
     unsigned int i;
 #endif
 
@@ -203,17 +165,17 @@ int hal_dts_fixup(void *dts_addr, uint32_t capacity)
 
     /* wolfBoot does not program the fabric, so the board config lists the
      * nodes whose hardware sits behind the unprogrammed FPGA. */
-#ifdef AGILEX5_FDT_DISABLE_PATHS
-    for (i = 0; i < sizeof(agilex5_fdt_disable) /
-                    sizeof(agilex5_fdt_disable[0]); i++) {
-        off = fdt_path_offset(&ctx, agilex5_fdt_disable[i]);
+#ifdef WOLFBOOT_FDT_DISABLE_PATHS
+    for (i = 0; i < sizeof(fdt_disable_paths) /
+                    sizeof(fdt_disable_paths[0]); i++) {
+        off = fdt_path_offset(&ctx, fdt_disable_paths[i]);
         if (off < 0)
             continue;
-        ret = fdt_fixup_str(&ctx, off, agilex5_fdt_disable[i], "status",
+        ret = fdt_fixup_str(&ctx, off, fdt_disable_paths[i], "status",
                             "disabled");
         if (ret != 0) {
             wolfBoot_printf("FDT: failed to disable %s (%d)\n",
-                agilex5_fdt_disable[i], ret);
+                fdt_disable_paths[i], ret);
             return ret;
         }
     }
@@ -251,22 +213,6 @@ int RAMFUNCTION hal_flash_erase(uintptr_t address, int len)
 #if defined(DISK_SDCARD) || defined(DISK_EMMC)
 #include "sdhci.h"
 
-#define AGILEX5_SDHCI_HRS05                  0x014U
-#define AGILEX5_SDHCI_HRS07                  0x01CU
-#define AGILEX5_SDHCI_HRS09                  0x024U
-#define AGILEX5_SDHCI_HRS10                  0x028U
-#define AGILEX5_SDHCI_HRS16                  0x040U
-
-#define AGILEX5_SDHCI_PHY_DQ_TIMING          0x2000U
-#define AGILEX5_SDHCI_PHY_DQS_TIMING         0x2004U
-#define AGILEX5_SDHCI_PHY_GATE_LPBK_CTRL     0x2008U
-#define AGILEX5_SDHCI_PHY_DLL_MASTER_CTRL    0x200CU
-#define AGILEX5_SDHCI_PHY_DLL_SLAVE_CTRL     0x2010U
-
-#define AGILEX5_SDHCI_PHY_INIT_COMPLETE      (1U << 1)
-#define AGILEX5_SDHCI_PHY_SW_RESET           (1U << 0)
-#define AGILEX5_SDHCI_PHY_INIT_TIMEOUT       1000000U
-
 static void agilex5_sdhci_write_phy(uint32_t address, uint32_t value)
 {
     sdhci_reg_write(SDHCI_HRS04, address);

hal/agilex5.c

@@ -51,28 +51,54 @@
 #endif
 
 #define AGILEX5_DDR_BASE       0x80000000UL
-/* U-Boot normally writes the measured DDR size into the DTB; wolfBoot hands
- * the DTB straight to Linux, so the board config supplies it. Default is the
- * DK-A5E013BM16AEA's 1792 MiB. */
+/* The GSRD U-Boot device tree starts HPS DDR at 0x80000000. U-Boot normally
+ * writes the measured size into the DTB; wolfBoot supplies the 013B's
+ * 1792 MiB because it hands the DTB directly to Linux. */
 #ifndef AGILEX5_DDR_SIZE
 #define AGILEX5_DDR_SIZE       0x70000000UL
 #endif
 
-/* No UL suffix: the base is also stringified into the earlycon argument. */
+/* Agilex 5 HPS UART0 and UART1 MMIO bases. The GSRD device tree maps these as
+ * serial@10c02000 and serial@10c02100. The selected value is also emitted in
+ * Linux's earlycon argument, so keep the address macros free of a C suffix. */
+#define AGILEX5_UART0_BASE     0x10C02000
+#define AGILEX5_UART1_BASE     0x10C02100
 #ifndef DEBUG_UART_BASE
     #if defined(DEBUG_UART_NUM) && DEBUG_UART_NUM == 1
-        #define DEBUG_UART_BASE 0x10C02100
+        #define DEBUG_UART_BASE AGILEX5_UART1_BASE
     #else
-        #define DEBUG_UART_BASE 0x10C02000
+        #define DEBUG_UART_BASE AGILEX5_UART0_BASE
     #endif
 #endif
+
+/* HPS SDMMC controller base from the Agilex 5 HPS memory map. */
 #define AGILEX5_SDHCI_BASE     0x10808000UL
+/* The Cortex-A55 data cache uses 64-byte lines. */
 #define CACHE_LINE_SIZE        64UL
 
-#ifndef __ASSEMBLER__
-unsigned int current_el(void);
+/* TF-A programs the architectural counter to 100 MHz on the GSRD boot path. */
+#ifndef TIMER_CLK_FREQ
+#define TIMER_CLK_FREQ         100000000UL
 #endif
 
+/* Cadence SD6HC host and PHY register offsets from the GSRD U-Boot driver. */
+#define AGILEX5_SDHCI_HRS05                  0x014U
+#define AGILEX5_SDHCI_HRS07                  0x01CU
+#define AGILEX5_SDHCI_HRS09                  0x024U
+#define AGILEX5_SDHCI_HRS10                  0x028U
+#define AGILEX5_SDHCI_HRS16                  0x040U
+#define AGILEX5_SDHCI_PHY_DQ_TIMING          0x2000U
+#define AGILEX5_SDHCI_PHY_DQS_TIMING         0x2004U
+#define AGILEX5_SDHCI_PHY_GATE_LPBK_CTRL     0x2008U
+#define AGILEX5_SDHCI_PHY_DLL_MASTER_CTRL    0x200CU
+#define AGILEX5_SDHCI_PHY_DLL_SLAVE_CTRL     0x2010U
+#define AGILEX5_SDHCI_PHY_INIT_COMPLETE      (1U << 1)
+#define AGILEX5_SDHCI_PHY_SW_RESET           (1U << 0)
+#define AGILEX5_SDHCI_PHY_INIT_TIMEOUT       1000000U
+
+/* wolfBoot occupies the GSRD BL33 region at 0x80200000. The DTB is staged at
+ * 0x8f000000, matching agilex5.its, and the signed image uses the next
+ * 256 MiB at 0x90000000 so neither buffer overlaps wolfBoot or the DTB. */
 #ifndef WOLFBOOT_LOAD_ADDRESS
 #define WOLFBOOT_LOAD_ADDRESS  0x90000000UL
 #endif

hal/agilex5.h

@@ -598,14 +598,6 @@
  */
 #ifndef __ASSEMBLER__
 
-/* Get current exception level */
-static inline unsigned int current_el(void)
-{
-    unsigned long el;
-    __asm__ volatile("mrs %0, CurrentEL" : "=r" (el));
-    return (unsigned int)((el >> 2) & 0x3);
-}
-
 /* Memory barrier */
 static inline void dmb(void)
 {

hal/versal.h

@@ -34,6 +34,7 @@
 #endif
 
 #include <target.h>
+#include "aarch64_arch.h"
 #include "image.h"
 #include "printf.h"
 #include "hal_fpga.h"
@@ -113,7 +114,6 @@ static int test_ext_flash(QspiDev_t* dev);
 
 /* asm function */
 extern void flush_dcache_range(unsigned long start, unsigned long stop);
-extern unsigned int current_el(void);
 
 void hal_delay_ms(uint64_t ms);
 uint64_t hal_timer_ms(void);

hal/zynq.c

@@ -19,23 +19,28 @@
  * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1335, USA
  */
 
-/* Architectural AArch64 helpers that carry no SoC-specific state: the generic
- * timer and set/way data-cache maintenance. Several HALs each grew their own
- * copy of these; this is the shared home for them. Inline, so including it
- * changes nothing for a HAL that does not call them.
+/* Architectural AArch64 helpers that carry no SoC-specific state: exception
+ * level access, the generic timer and set/way data-cache maintenance. Several
+ * HALs each grew their own copy of these; this is the shared home for them.
+ * Inline, so including it changes nothing for a HAL that does not call them.
  *
- * Define TIMER_CLK_FREQ before including for the rate to assume when firmware
- * left CNTFRQ_EL0 unprogrammed. */
+ * Define TIMER_CLK_FREQ before including to expose the timer helpers and set
+ * the rate to assume when firmware left CNTFRQ_EL0 unprogrammed. */
 
 #ifndef _AARCH64_ARCH_H_
 #define _AARCH64_ARCH_H_
 
 #include <stdint.h>
 
-#ifndef TIMER_CLK_FREQ
-#error "TIMER_CLK_FREQ must be defined before including aarch64_arch.h"
-#endif
+static inline unsigned int current_el(void)
+{
+    unsigned long el;
 
+    __asm__ volatile("mrs %0, CurrentEL" : "=r"(el) : : "cc");
+    return (unsigned int)((el >> 2) & 0x3U);
+}
+
+#ifdef TIMER_CLK_FREQ
 /* CNTPCT_EL0, the architectural counter. */
 static inline uint64_t timer_get_count(void)
 {
@@ -68,6 +73,7 @@ static inline int timer_expired(uint64_t deadline)
 {
     return timer_get_count() > deadline;
 }
+#endif
 
 /* Data-cache maintenance by set/way over every level to the point of
  * coherency. clean != 0 -> clean+invalidate, else invalidate only. */

include/aarch64_arch.h

@@ -483,9 +483,6 @@ void sdhci_platform_init(void);
  * to the card clock and return that, keeping the internal divider ~1. */
 uint32_t sdhci_platform_set_clock(uint32_t clock_khz, uint32_t base_clk_khz);
 
-/* Optional controller settle time in microseconds after each init command.
- * Opt in with -DSDHCI_WAIT_AFTER_CMD_US=<us>. */
-
 /* Platform interrupt setup (PLIC/NVIC/GIC/etc.) */
 void sdhci_platform_irq_init(void);