Skip to content

Commit 10df0b6

Browse files
committed
Peer review fixes
1 parent 2ae347e commit 10df0b6

3 files changed

Lines changed: 103 additions & 50 deletions

File tree

arch.mk

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -995,7 +995,7 @@ ifeq ($(TARGET),nxp_t2080)
995995
ARCH_FLAGS=-mhard-float -mcpu=e6500
996996
CFLAGS+=$(ARCH_FLAGS)
997997
BIG_ENDIAN=1
998-
CFLAGS+=-DMMU -DWOLFBOOT_FDT -DWOLFBOOT_DUALBOOT -DDEBUG_UART
998+
CFLAGS+=-DMMU -DWOLFBOOT_FDT -DWOLFBOOT_DUALBOOT
999999
CFLAGS+=-pipe # use pipes instead of temp files
10001000
CFLAGS+=-feliminate-unused-debug-types
10011001
LDFLAGS+=$(ARCH_FLAGS)

hal/nxp_t2080.c

Lines changed: 53 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -763,10 +763,15 @@ static int RAMFUNCTION hal_flash_ppb_unlock(uint32_t sector)
763763
FLASH_IO8_WRITE(0, FLASH_UNLOCK_ADDR2, AMD_CMD_UNLOCK_ACK);
764764
FLASH_IO8_WRITE(0, FLASH_UNLOCK_ADDR1, AMD_CMD_SET_PPB_ENTRY);
765765

766-
/* Read PPB status for target sector: DQ0=0 means protected */
767-
ppb_status = FLASH_IO8_READ(sector, 0);
768-
766+
/* Read PPB status for target sector: DQ0=0 means protected.
767+
* On 16-bit bus, must read both chip lanes to check both devices. */
768+
#if FLASH_CFI_WIDTH == 16
769+
ppb_status = FLASH_IO16_READ(sector, 0);
769770
if ((ppb_status & 0x0101) == 0x0101) {
771+
#else
772+
ppb_status = FLASH_IO8_READ(sector, 0);
773+
if ((ppb_status & 0x01) == 0x01) {
774+
#endif
770775
/* Both chips report unprotected — exit PPB mode and return */
771776
FLASH_IO8_WRITE(0, 0, AMD_CMD_SET_PPB_EXIT_BC1);
772777
FLASH_IO8_WRITE(0, 0, AMD_CMD_SET_PPB_EXIT_BC2);
@@ -792,11 +797,17 @@ static int RAMFUNCTION hal_flash_ppb_unlock(uint32_t sector)
792797
FLASH_IO8_WRITE(0, 0, AMD_CMD_PPB_UNLOCK_BC1); /* 0x80 */
793798
FLASH_IO8_WRITE(0, 0, AMD_CMD_PPB_UNLOCK_BC2); /* 0x30 */
794799

795-
/* Wait for PPB erase completion — poll for toggle stop */
800+
/* Wait for PPB erase completion — poll for toggle stop.
801+
* On 16-bit bus, read both chip lanes to ensure both complete. */
796802
timeout = 0;
797803
do {
804+
#if FLASH_CFI_WIDTH == 16
805+
read1 = FLASH_IO16_READ(0, 0);
806+
read2 = FLASH_IO16_READ(0, 0);
807+
#else
798808
read1 = FLASH_IO8_READ(0, 0);
799809
read2 = FLASH_IO8_READ(0, 0);
810+
#endif
800811
if (read1 == read2)
801812
break;
802813
ram_udelay(10);
@@ -828,19 +839,37 @@ static int RAMFUNCTION hal_flash_status_wait(uint32_t sector, uint16_t mask,
828839
uint32_t timeout = 0;
829840
uint16_t read1, read2;
830841

842+
/* Replicate 8-bit AMD status mask to both bytes for parallel chips */
843+
#if FLASH_CFI_WIDTH == 16
844+
uint16_t mask16 = (mask << 8) | mask;
845+
uint16_t toggle16 = (AMD_STATUS_TOGGLE << 8) | AMD_STATUS_TOGGLE;
846+
#else
847+
uint16_t mask16 = mask;
848+
uint16_t toggle16 = AMD_STATUS_TOGGLE;
849+
#endif
850+
831851
do {
832852
/* detection of completion happens when reading status bits
833853
* DQ6 and DQ2 stop toggling (0x44) */
854+
#if FLASH_CFI_WIDTH == 16
855+
read1 = FLASH_IO16_READ(sector, 0);
856+
if ((read1 & toggle16) == 0)
857+
read1 = FLASH_IO16_READ(sector, 0);
858+
read2 = FLASH_IO16_READ(sector, 0);
859+
if ((read2 & toggle16) == 0)
860+
read2 = FLASH_IO16_READ(sector, 0);
861+
#else
834862
read1 = FLASH_IO8_READ(sector, 0);
835-
if ((read1 & AMD_STATUS_TOGGLE) == 0)
863+
if ((read1 & toggle16) == 0)
836864
read1 = FLASH_IO8_READ(sector, 0);
837865
read2 = FLASH_IO8_READ(sector, 0);
838-
if ((read2 & AMD_STATUS_TOGGLE) == 0)
866+
if ((read2 & toggle16) == 0)
839867
read2 = FLASH_IO8_READ(sector, 0);
868+
#endif
840869
#ifdef DEBUG_FLASH
841870
wolfBoot_printf("Wait toggle %x -> %x\n", read1, read2);
842871
#endif
843-
if (read1 == read2 && ((read1 & mask) == mask))
872+
if (read1 == read2 && ((read1 & mask16) == mask16))
844873
break;
845874
ram_udelay(1);
846875
} while (timeout++ < timeout_us);
@@ -858,6 +887,14 @@ int RAMFUNCTION hal_flash_write(uint32_t address, const uint8_t *data, int len)
858887
{
859888
int ret = 0;
860889
uint32_t i, pos, sector, offset, xfer, nwords;
890+
const uint32_t width_bytes = FLASH_CFI_WIDTH / 8;
891+
892+
/* Enforce alignment to flash bus width */
893+
if ((address % width_bytes) != 0 || (len % width_bytes) != 0) {
894+
wolfBoot_printf("Flash Write: unaligned addr 0x%x or len %d "
895+
"(need %d-byte alignment)\n", address, len, width_bytes);
896+
return -1;
897+
}
861898

862899
/* adjust for flash base */
863900
if (address >= FLASH_BASE_ADDR)
@@ -1191,21 +1228,19 @@ int hal_dts_fixup(void* dts_addr)
11911228

11921229
/* fixup the memory region - single bank */
11931230
off = fdt_find_devtype(fdt, -1, "memory");
1194-
if (off != -FDT_ERR_NOTFOUND) {
1195-
/* build addr/size as 64-bit */
1196-
uint8_t ranges[sizeof(uint64_t) * 2], *p = ranges;
1197-
*(uint64_t*)p = cpu_to_fdt64(DDR_ADDRESS);
1198-
p += sizeof(uint64_t);
1199-
*(uint64_t*)p = cpu_to_fdt64(DDR_SIZE);
1200-
p += sizeof(uint64_t);
1231+
if (off >= 0) {
1232+
/* build addr/size as aligned 64-bit values */
1233+
uint64_t ranges[2];
1234+
ranges[0] = cpu_to_fdt64(DDR_ADDRESS);
1235+
ranges[1] = cpu_to_fdt64(DDR_SIZE);
12011236
wolfBoot_printf("FDT: Set memory, start=0x%x, size=0x%x\n",
12021237
DDR_ADDRESS, (uint32_t)DDR_SIZE);
1203-
fdt_setprop(fdt, off, "reg", ranges, (int)(p - ranges));
1238+
fdt_setprop(fdt, off, "reg", ranges, sizeof(ranges));
12041239
}
12051240

12061241
/* fixup CPU status and release address and enable method */
12071242
off = fdt_find_devtype(fdt, -1, "cpu");
1208-
while (off != -FDT_ERR_NOTFOUND) {
1243+
while (off >= 0) {
12091244
int core;
12101245
#ifdef ENABLE_MP
12111246
uint64_t core_spin_table;
@@ -1237,13 +1272,13 @@ int hal_dts_fixup(void* dts_addr)
12371272

12381273
/* fixup the soc clock */
12391274
off = fdt_find_devtype(fdt, -1, "soc");
1240-
if (off != -FDT_ERR_NOTFOUND) {
1275+
if (off >= 0) {
12411276
fdt_fixup_val(fdt, off, "soc", "bus-frequency", hal_get_plat_clk());
12421277
}
12431278

12441279
/* fixup the serial clocks */
12451280
off = fdt_find_devtype(fdt, -1, "serial");
1246-
while (off != -FDT_ERR_NOTFOUND) {
1281+
while (off >= 0) {
12471282
fdt_fixup_val(fdt, off, "serial", "clock-frequency", hal_get_bus_clk());
12481283
off = fdt_find_devtype(fdt, off, "serial");
12491284
}

src/boot_ppc_start.S

Lines changed: 49 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -297,14 +297,15 @@ find_pc:
297297
oris r3, r3, MAS1_IPROT@h
298298
mtspr MAS1, r3
299299

300-
/* Align PC (R1) to TLB page size boundary */
301-
lis r3, TLB1_EPN_MASK@h
302-
ori r3, r3, TLB1_EPN_MASK@l
300+
/* Align PC (R1) to TLB page size boundary.
301+
* Use LOAD_ADDR32: TLB1_EPN_MASK has bit 31 set (e.g. 0xFFFC0000),
302+
* so lis would sign-extend to 0xFFFFFFFF_FFFC0000 on e6500. */
303+
LOAD_ADDR32(r3, TLB1_EPN_MASK)
303304
and r1, r1, r3
304305

305-
/* Set the real and virtual page for this TLB */
306-
lis r3, MAS2_EPN@h
307-
ori r3, r3, MAS2_EPN@l
306+
/* Set the real and virtual page for this TLB.
307+
* Use LOAD_ADDR32: MAS2_EPN (0xFFFFF000) has bit 31 set. */
308+
LOAD_ADDR32(r3, MAS2_EPN)
308309
mfspr r2, MAS2
309310
andc r2, r2, r3
310311
or r2, r2, r1
@@ -939,8 +940,8 @@ setup_stack:
939940
stwu r0, -4(r1)
940941
stwu r0, -4(r1) /* Terminate Back chain */
941942
stwu r1, -8(r1) /* Save back chain and move SP */
942-
lis r0, RESET_VECTOR@h /* Address of reset vector */
943-
ori r0, r0, RESET_VECTOR@l
943+
/* RESET_VECTOR (0xEFFFFFFC) has bit 31 set; use LOAD_ADDR32 on e6500 */
944+
LOAD_ADDR32(r0, RESET_VECTOR)
944945
stwu r1, -8(r1) /* Save back chain and move SP */
945946
stw r0, +12(r1) /* Save return addr (underflow vect) */
946947

@@ -962,6 +963,7 @@ setup_stack:
962963
* 0xFFFFFFFF_EFFExxxx), causing an instruction TLB miss on blr. */
963964
LOAD_ADDR32(r3, boot_entry_C)
964965
mtlr r3
966+
uart_putc_debug 'Z' /* checkpoint Z: about to blr to boot_entry_C */
965967
blr
966968
#else
967969
/* jump to wolfboot */
@@ -1091,8 +1093,8 @@ ddr_call_with_stack:
10911093
stwu r0, -4(r1) /* Terminate back chain */
10921094
stwu r0, -4(r1)
10931095
stwu r1, -8(r1) /* Save back chain and move SP */
1094-
lis r0, RESET_VECTOR@h
1095-
ori r0, r0, RESET_VECTOR@l
1096+
/* RESET_VECTOR (0xEFFFFFFC) has bit 31 set; use LOAD_ADDR32 on e6500 */
1097+
LOAD_ADDR32(r0, RESET_VECTOR)
10961098
stwu r1, -8(r1) /* Save back chain and move SP */
10971099
stw r0, +12(r1) /* Save return addr (underflow vector) */
10981100
/* Call the function */
@@ -1196,8 +1198,8 @@ in_ram:
11961198
stwu r0, -4(r1)
11971199
stwu r0, -4(r1) /* Terminate Back chain */
11981200
stwu r1, -8(r1) /* Save back chain and move SP */
1199-
lis r0, RESET_VECTOR@h /* Address of reset vector */
1200-
ori r0, r0, RESET_VECTOR@l
1201+
/* RESET_VECTOR (0xEFFFFFFC) has bit 31 set; use LOAD_ADDR32 on e6500 */
1202+
LOAD_ADDR32(r0, RESET_VECTOR)
12011203
stwu r1, -8(r1) /* Save back chain and move SP */
12021204
stw r0, +12(r1) /* Save return addr (underflow vect) */
12031205

@@ -1210,27 +1212,43 @@ in_ram:
12101212
.section .isr_vector
12111213
.align 8
12121214
isr_empty:
1213-
/* Minimal fault dump for early bring-up */
1215+
/* Minimal fault dump for early bring-up.
1216+
* IMPORTANT: Do NOT use r0 as base register for addi/stw!
1217+
* PowerPC treats RA=0 specially: addi with RA=0 uses literal 0
1218+
* (not GPR0), and stw with RA=0 uses EA=0+D (not GPR0+D).
1219+
* This caused all stores to go to address 0x0000 (DDR, not
1220+
* initialized on cold boot) -> nested machine check -> checkstop.
1221+
* Use r3 as base, r4 as scratch. */
1222+
#if defined(DEBUG_UART) && defined(TARGET_nxp_t2080)
1223+
/* Print '!' to UART to signal exception occurred */
1224+
LOAD_ADDR32(r3, 0xFE11C500)
1225+
.L_isr_wait:
1226+
lbz r4, 5(r3)
1227+
andi. r4, r4, 0x20
1228+
beq .L_isr_wait
1229+
li r4, '!'
1230+
stb r4, 0(r3)
1231+
eieio
1232+
#endif
12141233
#ifdef L2SRAM_ADDR
1215-
LOAD_ADDR32(r0, L2SRAM_ADDR)
1216-
addi r0, r0, 0x200
1217-
mfspr r1, SRR0
1218-
stw r1, 0x00(r0)
1219-
mfspr r1, SRR1
1220-
stw r1, 0x04(r0)
1221-
mfspr r1, SPRN_ESR
1222-
stw r1, 0x08(r0)
1223-
mfspr r1, SPRN_DEAR
1224-
stw r1, 0x0C(r0)
1225-
mfspr r1, SPRN_MCSR
1226-
stw r1, 0x10(r0)
1227-
mfspr r1, SPRN_PIR
1228-
stw r1, 0x14(r0)
1234+
LOAD_ADDR32(r3, L2SRAM_ADDR + 0x200)
1235+
mfspr r4, SRR0
1236+
stw r4, 0x00(r3)
1237+
mfspr r4, SRR1
1238+
stw r4, 0x04(r3)
1239+
mfspr r4, SPRN_ESR
1240+
stw r4, 0x08(r3)
1241+
mfspr r4, SPRN_DEAR
1242+
stw r4, 0x0C(r3)
1243+
mfspr r4, SPRN_MCSR
1244+
stw r4, 0x10(r3)
1245+
mfspr r4, SPRN_PIR
1246+
stw r4, 0x14(r3)
12291247
/* Machine check exceptions use MCSRR0/MCSRR1 (not SRR0/SRR1) */
1230-
mfspr r1, SPRN_MCSRR0
1231-
stw r1, 0x18(r0)
1232-
mfspr r1, SPRN_MCSRR1
1233-
stw r1, 0x1C(r0)
1248+
mfspr r4, SPRN_MCSRR0
1249+
stw r4, 0x18(r3)
1250+
mfspr r4, SPRN_MCSRR1
1251+
stw r4, 0x1C(r3)
12341252
#endif
12351253
1: b 1b
12361254
#endif

0 commit comments

Comments
 (0)