Skip to content

Commit 8eafade

Browse files
dgarskedgarske
committed
Peer review fixes
1 parent 2f8ce14 commit 8eafade

3 files changed

Lines changed: 85 additions & 70 deletions

File tree

‎hal/mpfs250.c‎

Lines changed: 61 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -66,10 +66,13 @@ void hal_init(void)
6666
#endif
6767

6868
#ifdef EXT_FLASH
69-
qspi_init();
70-
69+
if (qspi_init() != 0) {
70+
wolfBoot_printf("QSPI: Init failed\n");
71+
}
7172
#if defined(TEST_EXT_FLASH) && defined(__WOLFBOOT)
72-
test_ext_flash();
73+
else {
74+
test_ext_flash();
75+
}
7376
#endif
7477
#endif /* EXT_FLASH */
7578
}
@@ -82,24 +85,16 @@ void hal_init(void)
8285
* and responses are read from the mailbox RAM.
8386
* ============================================================================ */
8487

85-
static int mpfs_scb_is_busy(void)
86-
{
87-
return (SCBCTRL_REG(SERVICES_SR_OFFSET) & SERVICES_SR_BUSY_MASK);
88-
}
89-
90-
static int mpfs_scb_wait_ready(uint32_t timeout)
88+
/* Wait for SCB register bits to clear, with timeout */
89+
static int mpfs_scb_wait_clear(uint32_t reg_offset, uint32_t mask,
90+
uint32_t timeout)
9191
{
92-
while (mpfs_scb_is_busy() && timeout > 0) {
93-
timeout--;
94-
}
95-
96-
if (timeout == 0) {
97-
return -1;
98-
}
99-
return 0;
92+
while ((SCBCTRL_REG(reg_offset) & mask) && --timeout)
93+
;
94+
return (timeout == 0) ? -1 : 0;
10095
}
10196

102-
static int mpfs_scb_read_mailbox(uint8_t *out, uint32_t len)
97+
int mpfs_scb_read_mailbox(uint8_t *out, uint32_t len)
10398
{
10499
uint32_t i;
105100

@@ -139,27 +134,13 @@ static void mpfs_scb_write_mailbox(const uint8_t *data, uint32_t len)
139134
}
140135
}
141136

142-
static int mpfs_scb_wait_req_clear(uint32_t timeout)
143-
{
144-
while ((SCBCTRL_REG(SERVICES_CR_OFFSET) & SERVICES_CR_REQ_MASK) &&
145-
timeout > 0) {
146-
timeout--;
147-
}
148-
149-
if (timeout == 0) {
150-
return -1;
151-
}
152-
return 0;
153-
}
154-
155-
static int mpfs_scb_service_call_timeout(uint8_t opcode, const uint8_t *mb_data,
156-
uint32_t mb_len, uint32_t req_timeout,
157-
uint32_t busy_timeout)
137+
int mpfs_scb_service_call(uint8_t opcode, const uint8_t *mb_data,
138+
uint32_t mb_len, uint32_t timeout)
158139
{
159140
uint32_t cmd;
160141
uint32_t status;
161142

162-
if (mpfs_scb_is_busy()) {
143+
if (mpfs_scb_wait_clear(SERVICES_SR_OFFSET, SERVICES_SR_BUSY_MASK, 1)) {
163144
return -1;
164145
}
165146

@@ -171,26 +152,25 @@ static int mpfs_scb_service_call_timeout(uint8_t opcode, const uint8_t *mb_data,
171152
SERVICES_CR_REQ_MASK;
172153
SCBCTRL_REG(SERVICES_CR_OFFSET) = cmd;
173154

174-
if (mpfs_scb_wait_req_clear(req_timeout) < 0) {
155+
if (mpfs_scb_wait_clear(SERVICES_CR_OFFSET, SERVICES_CR_REQ_MASK,
156+
timeout) < 0) {
175157
return -2;
176158
}
177159

178-
if (mpfs_scb_wait_ready(busy_timeout) < 0) {
160+
if (mpfs_scb_wait_clear(SERVICES_SR_OFFSET, SERVICES_SR_BUSY_MASK,
161+
timeout) < 0) {
179162
return -3;
180163
}
181164

182-
status = (SCBCTRL_REG(SERVICES_SR_OFFSET) >> SERVICES_SR_STATUS_SHIFT) & 0xFFFF;
165+
status = (SCBCTRL_REG(SERVICES_SR_OFFSET) >> SERVICES_SR_STATUS_SHIFT)
166+
& 0xFFFF;
183167
if (status != 0) {
184168
return -4;
185169
}
186170

187171
return 0;
188172
}
189173

190-
static int mpfs_scb_service_call(uint8_t opcode, const uint8_t *mb_data, uint32_t mb_len)
191-
{
192-
return mpfs_scb_service_call_timeout(opcode, mb_data, mb_len, 10000, 10000);
193-
}
194174
/**
195175
* mpfs_read_serial_number - Read the device serial number via system services
196176
* @serial: Buffer to store the 16-byte device serial number
@@ -200,15 +180,16 @@ static int mpfs_scb_service_call(uint8_t opcode, const uint8_t *mb_data, uint32_
200180
*
201181
* Returns: 0 on success, negative error code on failure
202182
*/
203-
static int mpfs_read_serial_number(uint8_t *serial)
183+
int mpfs_read_serial_number(uint8_t *serial)
204184
{
205185
int ret;
206186

207187
if (serial == NULL) {
208188
return -1;
209189
}
210190

211-
ret = mpfs_scb_service_call(SYS_SERV_CMD_SERIAL_NUMBER, NULL, 0);
191+
ret = mpfs_scb_service_call(SYS_SERV_CMD_SERIAL_NUMBER, NULL, 0,
192+
MPFS_SCB_TIMEOUT);
212193
if (ret != 0) {
213194
wolfBoot_printf("SCB mailbox error: %d\n", ret);
214195
return ret;
@@ -335,10 +316,9 @@ int hal_dts_fixup(void* dts_addr)
335316

336317
return 0;
337318
}
319+
338320
void hal_prepare_boot(void)
339321
{
340-
/* reset the eMMC/SD card? */
341-
342322

343323
}
344324

@@ -400,16 +380,18 @@ static void qspi_flash_wakeup(void)
400380
udelay(10);
401381
}
402382

403-
void qspi_init(void)
383+
int qspi_init(void)
404384
{
405385
uint8_t id[3];
386+
uint32_t timeout;
406387

407388
#ifdef MPFS_SC_SPI
408389
wolfBoot_printf("QSPI: Using SC QSPI Controller (0x%x)\n", QSPI_BASE);
409390

410391
/* Wait for system controller to finish any pending operations before
411392
* taking direct control of the SC QSPI peripheral */
412-
mpfs_scb_wait_ready(100000);
393+
mpfs_scb_wait_clear(SERVICES_SR_OFFSET, SERVICES_SR_BUSY_MASK,
394+
QSPI_TIMEOUT_TRIES);
413395

414396
#ifdef DEBUG_QSPI
415397
wolfBoot_printf("QSPI: Initial CTRL=0x%x, STATUS=0x%x, DIRECT=0x%x\n",
@@ -449,7 +431,12 @@ void qspi_init(void)
449431
QSPI_CTRL_EN;
450432

451433
/* Wait for controller to be ready */
452-
while (!(QSPI_STATUS & QSPI_STATUS_READY));
434+
timeout = QSPI_TIMEOUT_TRIES;
435+
while (!(QSPI_STATUS & QSPI_STATUS_READY) && --timeout);
436+
if (timeout == 0) {
437+
wolfBoot_printf("QSPI: Controller not ready\n");
438+
return -1;
439+
}
453440

454441
/* Wake up flash from deep power-down (if applicable) */
455442
qspi_flash_wakeup();
@@ -462,6 +449,8 @@ void qspi_init(void)
462449

463450
/* Enter 4-byte addressing mode for >16MB flash */
464451
qspi_enter_4byte_mode();
452+
453+
return 0;
465454
}
466455

467456
/* QSPI Block Transfer Function
@@ -484,7 +473,7 @@ static int qspi_transfer_block(uint8_t read_mode, const uint8_t *cmd,
484473
uint32_t timeout;
485474

486475
/* Wait for controller to be ready before starting */
487-
timeout = 100000;
476+
timeout = QSPI_TIMEOUT_TRIES;
488477
while (!(QSPI_STATUS & QSPI_STATUS_READY) && --timeout);
489478
if (timeout == 0) {
490479
wolfBoot_printf("QSPI: Timeout waiting for READY\n");
@@ -521,24 +510,24 @@ static int qspi_transfer_block(uint8_t read_mode, const uint8_t *cmd,
521510
/* Send command bytes (opcode + address).
522511
* Use TXAVAIL (bit 3) to check for FIFO space -- CoreQSPI v2 does NOT
523512
* have a TXFULL status bit (bit 5 is reserved/always 0).
524-
* A fence after each TX write ensures the store reaches the peripheral
525-
* before we read STATUS again (RISC-V RVWMO allows posted stores that
526-
* could cause stale TXAVAIL reads and FIFO overflow). */
513+
* A fence (iorw, iorw) after each TX write ensures the store reaches the
514+
* peripheral before we read STATUS again (RISC-V RVWMO allows posted
515+
* stores that could cause stale TXAVAIL reads and FIFO overflow). */
527516
for (i = 0; i < cmd_len; i++) {
528-
timeout = 100000;
517+
timeout = QSPI_TIMEOUT_TRIES;
529518
while (!(QSPI_STATUS & QSPI_STATUS_TXAVAIL) && --timeout);
530519
if (timeout == 0) {
531520
wolfBoot_printf("QSPI: TX FIFO full timeout\n");
532521
return -2;
533522
}
534523
QSPI_TX_DATA = cmd[i];
535-
__asm__ __volatile__("fence o,i" ::: "memory");
524+
__asm__ __volatile__("fence iorw, iorw" ::: "memory");
536525
}
537526

538527
if (read_mode) {
539528
/* Read mode: poll RXAVAIL for each data byte. */
540529
for (i = 0; i < data_len; i++) {
541-
timeout = 1000000;
530+
timeout = QSPI_RX_TIMEOUT_TRIES;
542531
while (!(QSPI_STATUS & QSPI_STATUS_RXAVAIL) && --timeout);
543532
if (timeout == 0) {
544533
wolfBoot_printf("QSPI: RX timeout at byte %d, status=0x%x\n",
@@ -548,29 +537,39 @@ static int qspi_transfer_block(uint8_t read_mode, const uint8_t *cmd,
548537
data[i] = QSPI_RX_DATA;
549538
}
550539
/* Wait for receive complete */
551-
timeout = 1000000;
540+
timeout = QSPI_RX_TIMEOUT_TRIES;
552541
while (!(QSPI_STATUS & QSPI_STATUS_RXDONE) && --timeout);
542+
if (timeout == 0) {
543+
wolfBoot_printf("QSPI: RXDONE timeout\n");
544+
return -5;
545+
}
553546
} else {
554547
/* Write mode: send data bytes.
555548
* Must push bytes without delay -- any gap causes FIFO underflow
556549
* since CoreQSPI continues clocking with empty FIFO.
557-
* Fence after each write ensures the store reaches the FIFO before
558-
* we re-read STATUS (prevents FIFO overflow from posted stores). */
550+
* Fence (iorw, iorw) after each write ensures the store reaches the
551+
* FIFO before we re-read STATUS (prevents FIFO overflow from posted
552+
* stores). */
559553
if (data && data_len > 0) {
560554
for (i = 0; i < data_len; i++) {
561-
timeout = 100000;
555+
timeout = QSPI_TIMEOUT_TRIES;
562556
while (!(QSPI_STATUS & QSPI_STATUS_TXAVAIL) && --timeout);
563557
if (timeout == 0) {
564558
wolfBoot_printf("QSPI: TX data timeout\n");
565559
return -4;
566560
}
567561
QSPI_TX_DATA = data[i];
568-
__asm__ __volatile__("fence o,i" ::: "memory");
562+
__asm__ __volatile__("fence iorw, iorw" ::: "memory");
569563
}
570564
}
571565
/* Wait for transmit complete */
572-
timeout = 100000;
566+
timeout = QSPI_TIMEOUT_TRIES;
573567
while (!(QSPI_STATUS & QSPI_STATUS_TXDONE) && --timeout);
568+
if (timeout == 0) {
569+
wolfBoot_printf("QSPI: TXDONE timeout, status=0x%x\n",
570+
QSPI_STATUS);
571+
return -5;
572+
}
574573
}
575574

576575
#ifdef DEBUG_QSPI

‎hal/mpfs250.h‎

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -161,16 +161,35 @@
161161

162162
/* System Service command opcodes */
163163
#define SYS_SERV_CMD_SERIAL_NUMBER 0x00u
164-
#define SYS_SERV_CMD_SPI_COPY 0x50u
164+
#define SYS_SERV_CMD_SPI_COPY 0x50u /* SCB mailbox SPI copy service */
165165

166166
/* Device serial number size in bytes */
167167
#define DEVICE_SERIAL_NUMBER_SIZE 16
168168

169+
/* Timeout loop iteration counts (override at build time via CFLAGS) */
170+
#ifndef MPFS_SCB_TIMEOUT
171+
#define MPFS_SCB_TIMEOUT 10000 /* SCB mailbox polling */
172+
#endif
173+
#ifndef QSPI_TIMEOUT_TRIES
174+
#define QSPI_TIMEOUT_TRIES 100000 /* QSPI controller/TX polling */
175+
#endif
176+
#ifndef QSPI_RX_TIMEOUT_TRIES
177+
#define QSPI_RX_TIMEOUT_TRIES 1000000 /* QSPI RX polling (longer) */
178+
#endif
179+
169180
/* System Controller register access */
170181
#define SCBCTRL_REG(off) (*((volatile uint32_t*)(SCBCTRL_BASE + (off))))
171182
#define SCBMBOX_REG(off) (*((volatile uint32_t*)(SCBMBOX_BASE + (off))))
172183
#define SCBMBOX_BYTE(off) (*((volatile uint8_t*)(SCBMBOX_BASE + (off))))
173184

185+
/* System Controller Mailbox API */
186+
#ifndef __ASSEMBLER__
187+
int mpfs_scb_service_call(uint8_t opcode, const uint8_t *mb_data,
188+
uint32_t mb_len, uint32_t timeout);
189+
int mpfs_scb_read_mailbox(uint8_t *out, uint32_t len);
190+
int mpfs_read_serial_number(uint8_t *serial);
191+
#endif /* __ASSEMBLER__ */
192+
174193
/* Crypto Engine: Athena F5200 TeraFire Crypto Processor (1x), 200 MHz */
175194
#define ATHENA_BASE (SYSREG_BASE + 0x125000)
176195

@@ -358,7 +377,7 @@
358377

359378
/* Function declarations for QSPI (when EXT_FLASH enabled) */
360379
#ifndef __ASSEMBLER__
361-
void qspi_init(void);
380+
int qspi_init(void);
362381
#endif /* __ASSEMBLER__ */
363382

364383
#endif /* EXT_FLASH */

‎test-app/RISCV64-mpfs250.ld‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ MEMORY
1515
{
1616
/* For RAM boot (NO_XIP), use WOLFBOOT_LOAD_ADDRESS for execution
1717
* For XIP boot, use WOLFBOOT_TEST_APP_ADDRESS (partition + header) */
18-
IRAM (rx) :ORIGIN = @WOLFBOOT_LOAD_ADDRESS@, LENGTH = @WOLFBOOT_TEST_APP_SIZE@
18+
IRAM (rwx) :ORIGIN = @WOLFBOOT_LOAD_ADDRESS@, LENGTH = @WOLFBOOT_TEST_APP_SIZE@
1919
DDR (rwx) :ORIGIN = 0x80000000, LENGTH = 1M
2020
LSRAM (rwx) :ORIGIN = 0x08000000, LENGTH = 128K
2121
}
@@ -50,10 +50,10 @@ SECTIONS
5050
} >IRAM
5151

5252
/* used by the startup to initialize data */
53-
_stored_data = LOADADDR(.data);
53+
_stored_data = .;
5454

5555
/* Initialized data sections - keep in same region as code for RAM boot */
56-
.data :
56+
.data : AT (_stored_data)
5757
{
5858
. = ALIGN(8);
5959
_start_data = .; /* create a global symbol at data start */
@@ -66,9 +66,6 @@ SECTIONS
6666
_end_data = .; /* define a global symbol at data end */
6767
} >IRAM
6868

69-
/* use from libwolfboot.c */
70-
_stored_data = _start_data;
71-
7269
/* Uninitialized data section */
7370
/* Mark as NOLOAD so it's not included in binary (zero-initialized at runtime) */
7471
. = ALIGN(8);

0 commit comments

Comments
 (0)