Skip to content

NXP T2080 Port Refresh#680

Draft
dgarske wants to merge 14 commits intowolfSSL:masterfrom
dgarske:nxp_t2080_refresh
Draft

NXP T2080 Port Refresh#680
dgarske wants to merge 14 commits intowolfSSL:masterfrom
dgarske:nxp_t2080_refresh

Conversation

@dgarske
Copy link
Contributor

@dgarske dgarske commented Feb 4, 2026

No description provided.

@dgarske dgarske self-assigned this Feb 4, 2026
Copilot AI review requested due to automatic review settings February 26, 2026 00:23
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Refreshes the NXP T2080 (e6500) port by updating early boot/MMU bring-up, moving RAMFUNCTION code into CPC SRAM before DDR init, adding DDR stack relocation, and expanding platform/flash support.

Changes:

  • Add e6500-safe address loading, CPC SRAM init/release path, early UART checkpoints, and DDR stack switch.
  • Rework T2080 HAL: split SoC definitions into nxp_t2080.h, add DDR init improvements, flash erase/program support, MP spin-table boot, and DTS fixups.
  • Update linker script/config/docs and build flags to match the new memory layout and tooling.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 8 comments.

Show a summary per file
File Description
src/boot_ppc_start.S Adds e6500-safe address loads, early UART macros, CPC SRAM sequence changes, DDR stack trampoline, and ISR dump behavior.
src/boot_ppc_mp.S Fixes linear core ID calc and adds temporary CCSR TLB for secondary cores on e6500.
src/boot_ppc.c Copies .ramcode early (pre-DDR), flushes caches, and switches stack to DDR before main().
include/wolfboot/wolfboot.h Extends RAMFUNCTION attribute behavior for PPC.
hal/nxp_t2080.ld Changes RAM to 1MB CPC SRAM and introduces .ramcode placement/copy symbols.
hal/nxp_t2080.h New SoC/board header consolidating T2080 register and DDR/IFC/UART definitions.
hal/nxp_t2080.c Major HAL rewrite: DDR init, CPC release-to-cache, flash ops, MP bring-up, DTS fixups.
hal/nxp_ppc.h Updates target config, adds early UART helper, corrects SPR defs for e6500, and adjusts asm set_tlb for 64-bit MAS.
docs/Targets.md Updates T2080 documentation/config guidance and sample output.
config/examples/nxp-t2080.config Updates example config for CPC SRAM + RAM_CODE and new flash/partition settings.
config/examples/nxp-t2080-68ppc2.config Removes legacy per-board config file.
arch.mk Adds PPC freestanding flags, target CPU flags, forces DEBUG_UART for T2080, and includes MP object.
.github/workflows/test-configs.yml Removes CI build job for the deleted 68ppc2 config.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +301 to +302
lis r3, TLB1_EPN_MASK@h
ori r3, r3, TLB1_EPN_MASK@l
Copy link

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

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

TLB1_EPN_MASK will often have bit 31 set (e.g., 0xF0000000). On e6500, lis sign-extends into the upper 32 bits, producing 0xFFFFFFFF_F0000000, which can corrupt the aligned address in r1 and later MAS programming. Load this mask with the same zero-extended sequence used elsewhere (e.g., via LOAD_ADDR32) so upper bits remain 0 on e6500.

Suggested change
lis r3, TLB1_EPN_MASK@h
ori r3, r3, TLB1_EPN_MASK@l
LOAD_ADDR32(r3, TLB1_EPN_MASK)

Copilot uses AI. Check for mistakes.
Comment on lines +1088 to +1101
mr r1, r4 /* Set stack pointer to DDR */
/* Create minimal PPC ABI stack frame with terminated back chain */
li r0, 0
stwu r0, -4(r1) /* Terminate back chain */
stwu r0, -4(r1)
stwu r1, -8(r1) /* Save back chain and move SP */
lis r0, RESET_VECTOR@h
ori r0, r0, RESET_VECTOR@l
stwu r1, -8(r1) /* Save back chain and move SP */
stw r0, +12(r1) /* Save return addr (underflow vector) */
/* Call the function */
mtctr r3
bctrl
/* Should never reach here */
Copy link

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

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

The stack frame construction here is inconsistent (multiple stwu back-to-back, stwu r1,-8(r1) twice, and positive offsets after multiple updates). This is very likely to corrupt the new stack pointer and violate PPC ABI alignment expectations (typically 16-byte aligned SP). Consider rewriting this to a single, ABI-consistent minimal frame (terminated back chain once, reserve the correct frame size once, then store any needed values with stable offsets). Also, on e6500, RESET_VECTOR loads should avoid lis sign-extension if the constant/symbol can set bit 31.

Suggested change
mr r1, r4 /* Set stack pointer to DDR */
/* Create minimal PPC ABI stack frame with terminated back chain */
li r0, 0
stwu r0, -4(r1) /* Terminate back chain */
stwu r0, -4(r1)
stwu r1, -8(r1) /* Save back chain and move SP */
lis r0, RESET_VECTOR@h
ori r0, r0, RESET_VECTOR@l
stwu r1, -8(r1) /* Save back chain and move SP */
stw r0, +12(r1) /* Save return addr (underflow vector) */
/* Call the function */
mtctr r3
bctrl
/* Should never reach here */
mr r1, r4 /* Set stack pointer to DDR */
/* Create minimal 16-byte PPC ABI stack frame with terminated back chain */
li r0, 0
stwu r0, -16(r1) /* SP -= 16, store 0 as back chain at 0(r1) */
/* Load reset/underflow vector address safely (handles e6500 sign-extension) */
LOAD_ADDR32(r0, RESET_VECTOR)
mtlr r0 /* Set link register to RESET_VECTOR */
stw r0, 4(r1) /* Save LR in standard ABI slot */
/* Call the function */
mtctr r3
bctrl
/* Should never reach here; if it does, spin */

Copilot uses AI. Check for mistakes.
Comment on lines +1234 to 1236
#endif
1: b 1b
#endif
Copy link

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

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

isr_empty no longer executes rfi (it now loops forever). This is a behavioral change that can hang the system on any unexpected interrupt/exception that hits this vector outside of early bring-up. If this “halt + dump” behavior is intended only for early debug, it should be guarded behind a dedicated debug/build flag; otherwise, preserve the prior rfi behavior (or at least allow returning when appropriate).

Suggested change
#endif
1: b 1b
#endif
#endif
#ifdef EARLY_ISR_HALT
1: b 1b
#else
rfi
#endif
#endif

Copilot uses AI. Check for mistakes.
Comment on lines +766 to +774
/* Read PPB status for target sector: DQ0=0 means protected */
ppb_status = FLASH_IO8_READ(sector, 0);

if ((ppb_status & 0x0101) == 0x0101) {
/* Both chips report unprotected — exit PPB mode and return */
FLASH_IO8_WRITE(0, 0, AMD_CMD_SET_PPB_EXIT_BC1);
FLASH_IO8_WRITE(0, 0, AMD_CMD_SET_PPB_EXIT_BC2);
return 0;
}
Copy link

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

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

In 16-bit mode, FLASH_IO8_READ is defined to return only an 8-bit value (it casts a 16-bit read down to uint8_t). That means ppb_status cannot reliably contain “both chips” status, and the 0x0101 mask check is incorrect (upper byte is lost). Use a 16-bit read in 16-bit mode (or redefine the read helper to return uint16_t) and validate both bytes/lanes explicitly.

Copilot uses AI. Check for mistakes.
Comment on lines +830 to +843

do {
/* detection of completion happens when reading status bits
* DQ6 and DQ2 stop toggling (0x44) */
read1 = FLASH_IO8_READ(sector, 0);
if ((read1 & AMD_STATUS_TOGGLE) == 0)
read1 = FLASH_IO8_READ(sector, 0);
read2 = FLASH_IO8_READ(sector, 0);
if ((read2 & AMD_STATUS_TOGGLE) == 0)
read2 = FLASH_IO8_READ(sector, 0);
#ifdef DEBUG_FLASH
wolfBoot_printf("Wait toggle %x -> %x\n", read1, read2);
#endif
if (read1 == read2 && ((read1 & mask) == mask))
Copy link

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

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

read1/read2 are uint16_t, but FLASH_IO8_READ returns an 8-bit value in 16-bit mode, and the status masks (0x44, AMD_STATUS_TOGGLE = 0x40) are 8-bit. On a 16-bit (two-chip) flash bus, status polling generally needs to validate both bytes (e.g., 0x4040/0x4444) and ensure both devices have completed. As written, polling can report “done” too early (only one lane) or miss errors on the other chip.

Suggested change
do {
/* detection of completion happens when reading status bits
* DQ6 and DQ2 stop toggling (0x44) */
read1 = FLASH_IO8_READ(sector, 0);
if ((read1 & AMD_STATUS_TOGGLE) == 0)
read1 = FLASH_IO8_READ(sector, 0);
read2 = FLASH_IO8_READ(sector, 0);
if ((read2 & AMD_STATUS_TOGGLE) == 0)
read2 = FLASH_IO8_READ(sector, 0);
#ifdef DEBUG_FLASH
wolfBoot_printf("Wait toggle %x -> %x\n", read1, read2);
#endif
if (read1 == read2 && ((read1 & mask) == mask))
#if FLASH_CFI_WIDTH == 16
/* On a 16-bit bus, replicate 8-bit status bits to both bytes (two chips). */
uint16_t toggle_mask = (uint16_t)(AMD_STATUS_TOGGLE |
(AMD_STATUS_TOGGLE << 8));
uint16_t eff_mask = (uint16_t)((mask & 0x00FF) |
((mask & 0x00FF) << 8));
#else
uint16_t toggle_mask = AMD_STATUS_TOGGLE;
uint16_t eff_mask = mask;
#endif
do {
/* detection of completion happens when reading status bits
* DQ6 and DQ2 stop toggling (0x44) */
#if FLASH_CFI_WIDTH == 16
read1 = FLASH_IO16_READ(sector, 0);
if ((read1 & toggle_mask) == 0)
read1 = FLASH_IO16_READ(sector, 0);
read2 = FLASH_IO16_READ(sector, 0);
if ((read2 & toggle_mask) == 0)
read2 = FLASH_IO16_READ(sector, 0);
#else
read1 = FLASH_IO8_READ(sector, 0);
if ((read1 & toggle_mask) == 0)
read1 = FLASH_IO8_READ(sector, 0);
read2 = FLASH_IO8_READ(sector, 0);
if ((read2 & toggle_mask) == 0)
read2 = FLASH_IO8_READ(sector, 0);
#endif
#ifdef DEBUG_FLASH
wolfBoot_printf("Wait toggle %x -> %x\n", read1, read2);
#endif
if (read1 == read2 && ((read1 & eff_mask) == eff_mask))

Copilot uses AI. Check for mistakes.
Comment on lines +884 to +894
offset = address - (sector * FLASH_SECTOR_SIZE);
offset /= (FLASH_CFI_WIDTH/8);
xfer = len;
if (xfer > FLASH_PAGE_SIZE)
xfer = FLASH_PAGE_SIZE;
nwords = xfer / (FLASH_CFI_WIDTH/8);

#ifdef DEBUG_FLASH
wolfBoot_printf("Flash Write: Sector %d, Offset %d, Len %d, Pos %d\n",
sector, offset, xfer, pos);
#endif
Copy link

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

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

nwords truncates when xfer isn’t a multiple of the bus width (1 or 2 bytes), and the write loop advances pos by FLASH_CFI_WIDTH/8. Any tail byte(s) in len that don’t fill a full word will be silently skipped. Either enforce alignment (return an error if address/len aren’t width-aligned) or add handling for partial final writes.

Copilot uses AI. Check for mistakes.
Comment on lines +1188 to +1204
/* expand total size */
fdt->totalsize += 2048; /* expand by 2KB */
wolfBoot_printf("FDT: Expanded (2KB) to %d bytes\n", fdt->totalsize);

/* fixup the memory region - single bank */
off = fdt_find_devtype(fdt, -1, "memory");
if (off != -FDT_ERR_NOTFOUND) {
/* build addr/size as 64-bit */
uint8_t ranges[sizeof(uint64_t) * 2], *p = ranges;
*(uint64_t*)p = cpu_to_fdt64(DDR_ADDRESS);
p += sizeof(uint64_t);
*(uint64_t*)p = cpu_to_fdt64(DDR_SIZE);
p += sizeof(uint64_t);
wolfBoot_printf("FDT: Set memory, start=0x%x, size=0x%x\n",
DDR_ADDRESS, (uint32_t)DDR_SIZE);
fdt_setprop(fdt, off, "reg", ranges, (int)(p - ranges));
}
Copy link

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

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

There are multiple correctness issues in this block: (1) struct fdt_header fields are big-endian; mutating fdt->totalsize directly will corrupt the header on big-endian systems—use libfdt helpers or cpu_to_fdt32(fdt_totalsize()+extra) and store it back. (2) Writing uint64_t through a uint8_t* (*(uint64_t*)p = ...) can fault on PPC due to 8-byte alignment requirements—use an aligned fdt64_t array or memcpy. (3) fdt_find_devtype error handling should check off >= 0; off != -FDT_ERR_NOTFOUND will treat other negative errors as valid offsets.

Copilot uses AI. Check for mistakes.
Comment on lines 995 to +1004
ARCH_FLAGS=-mhard-float -mcpu=e6500
CFLAGS+=$(ARCH_FLAGS)
BIG_ENDIAN=1
CFLAGS+=-DMMU -DWOLFBOOT_FDT -DWOLFBOOT_DUALBOOT
CFLAGS+=-DMMU -DWOLFBOOT_FDT -DWOLFBOOT_DUALBOOT -DDEBUG_UART
CFLAGS+=-pipe # use pipes instead of temp files
CFLAGS+=-feliminate-unused-debug-types
LDFLAGS+=$(ARCH_FLAGS)
LDFLAGS+=-Wl,--hash-style=both # generate both sysv and gnu symbol hash table
LDFLAGS+=-Wl,--as-needed # remove weak functions not used
OBJS+=src/boot_ppc_mp.o # support for spin table
Copy link

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

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

-DDEBUG_UART is being forced for TARGET=nxp_t2080 regardless of the DEBUG_UART config setting. This can unintentionally enable debug-only UART behavior in builds that explicitly request DEBUG_UART=0 (and may conflict with earlier ifeq ($(DEBUG_UART),0) handling). Recommend making this conditional on $(DEBUG_UART) (or removing it here and letting the config control the define).

Copilot uses AI. Check for mistakes.
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.

2 participants