From c04796c322eff9184b382aeee20c7249bf2a8922 Mon Sep 17 00:00:00 2001 From: Daniele Lacamera Date: Tue, 18 Aug 2026 10:03:25 +0200 Subject: [PATCH] p1021: report an FCM command that never completes The wait loop increments timeout past FLASH_TIMEOUT_TRIES on its way out, so "timeout == FLASH_TIMEOUT_TRIES" never matched and a hung command returned success. Test the completion flag instead. That made the failure path reachable for the first time, and it did not exit: the break left the inner block loop only, so the outer loop respun the same failing command forever with pos unchanged. --- hal/nxp_p1021.c | 15 ++++++++++++--- tools/unit-tests/unit-p1021-fcm-bytes.c | 20 ++++++++++++++++++++ 2 files changed, 32 insertions(+), 3 deletions(-) diff --git a/hal/nxp_p1021.c b/hal/nxp_p1021.c index 90d5f95b..fe29fd8a 100644 --- a/hal/nxp_p1021.c +++ b/hal/nxp_p1021.c @@ -664,6 +664,7 @@ static int hal_flash_command(uint8_t iswrite) { int ret = 0; int timeout = 0; + uint32_t ltesr; uint32_t fmr = ELBC_FMR_CWTO(15) | /* max timeout */ ELBC_FMR_AL(2) | /* 4 byte address */ @@ -681,12 +682,19 @@ static int hal_flash_command(uint8_t iswrite) timeout++ < FLASH_TIMEOUT_TRIES) { /* NOP */ }; - if (timeout == FLASH_TIMEOUT_TRIES) { + + ltesr = get32(ELBC_LTESR); + + /* Test the completion flag rather than the loop counter: on a + * timeout exit "timeout" has already been incremented past + * FLASH_TIMEOUT_TRIES, so the old "timeout == FLASH_TIMEOUT_TRIES" + * never matched and a command that never completed returned 0. */ + if (!(ltesr & ELBC_LTESR_CC)) { ret = -1; } /* clear interrupt */ - set32(ELBC_LTESR, get32(ELBC_LTESR) & ELBC_NAND_MASK); + set32(ELBC_LTESR, ltesr & ELBC_NAND_MASK); set32(ELBC_LTEATR, 0); return ret; @@ -1760,7 +1768,7 @@ int ext_flash_read(uintptr_t address, uint8_t *data, int len) ret = hal_flash_command(0); if (ret != 0) - break; + goto read_done; /* check for bad page. if either of the first two pages are bad then * skip to next block */ @@ -1780,6 +1788,7 @@ int ext_flash_read(uintptr_t address, uint8_t *data, int len) } while ((address & (block_size - 1)) && (pos < len)); }; +read_done: /* on success return size read */ if (ret == 0) { ret = len; diff --git a/tools/unit-tests/unit-p1021-fcm-bytes.c b/tools/unit-tests/unit-p1021-fcm-bytes.c index b0d3a15d..d286e23a 100644 --- a/tools/unit-tests/unit-p1021-fcm-bytes.c +++ b/tools/unit-tests/unit-p1021-fcm-bytes.c @@ -558,6 +558,25 @@ START_TEST(test_p1021_read_multipart) } END_TEST +/* An FCM command that never raises CC must fail, and must end the + * read. The completion test used to be "timeout == + * FLASH_TIMEOUT_TRIES", but the loop increments timeout past that + * value on the way out, so the comparison never matched and a hung + * command reported success. Making it fail exposed the exit path: the + * old "break" left the inner block loop only, and the outer loop + * respun the same failing command forever. */ +START_TEST(test_p1021_command_never_completes) +{ + uint8_t data[4]; + + /* CC clear for the whole run: the controller never completes. */ + set32(ELBC_LTESR, 0); + memset(data, 0xEE, sizeof(data)); + + ck_assert_int_lt(ext_flash_read(0, data, 4), 0); +} +END_TEST + Suite *p1021_fcm_suite(void) { Suite *s = suite_create("p1021-fcm-bytes"); @@ -578,6 +597,7 @@ Suite *p1021_fcm_suite(void) tcase_add_test(tc, test_p1021_read_full_page); tcase_add_test(tc, test_p1021_read_short_spare_loaded); tcase_add_test(tc, test_p1021_read_multipart); + tcase_add_test(tc, test_p1021_command_never_completes); suite_add_tcase(s, tc); return s;