diff options
| author | iabdalkader <i.abdalkader@gmail.com> | 2021-09-30 09:13:15 +1000 |
|---|---|---|
| committer | Damien George <damien@micropython.org> | 2021-10-15 17:59:31 +1100 |
| commit | eea6cd85b37351ceda1b4c80a804912605977997 (patch) | |
| tree | 9ab18763f39f01ff0b9cbb3016fc9563576d7df6 | |
| parent | 4c9e17e0a16266523e5d89640f756a1a0ad2d8e3 (diff) | |
stm32/sdram: Enforce gcc opt, and use volatile and DSB in sdram_test.
Ensures consistent behaviour and resolves the D-Cache bug (the "exhaustive"
argument being lost due to cache being turned off) when O0 is used.
The changes in this commit are:
- Change -O0 to -Os because "gcc is considered broken at -O0" according to
https://github.com/ARM-software/CMSIS_5/issues/620#issuecomment-550235656
- Use volatile for mem_base so the compiler doesn't optimise away reads or
writes to the SDRAM, which is being tested.
- Use DSB to prevent any other compiler optimisations that would change the
testing logic.
- Use alternating pattern/antipattern in exhaustive test to catch more
hardware/configuration errors.
Implementation adapted by @andrewleech, taken directly from investigation
by @iabdalkader and @dpgeorge.
See #7841 and #7869 for further discussion.
| -rw-r--r-- | ports/stm32/sdram.c | 19 |
1 files changed, 11 insertions, 8 deletions
diff --git a/ports/stm32/sdram.c b/ports/stm32/sdram.c index e0e350083..4615c4fe1 100644 --- a/ports/stm32/sdram.c +++ b/ports/stm32/sdram.c @@ -283,10 +283,10 @@ void sdram_leave_low_power(void) { #pragma GCC diagnostic ignored "-Wstringop-overflow" #endif -bool __attribute__((optimize("O0"))) sdram_test(bool exhaustive) { +bool __attribute__((optimize("Os"))) sdram_test(bool exhaustive) { uint8_t const pattern = 0xaa; uint8_t const antipattern = 0x55; - uint8_t *const mem_base = (uint8_t *)sdram_start(); + volatile uint8_t *const mem_base = (uint8_t *)sdram_start(); #if MICROPY_HW_SDRAM_TEST_FAIL_ON_ERROR char error_buffer[1024]; @@ -310,12 +310,13 @@ bool __attribute__((optimize("O0"))) sdram_test(bool exhaustive) { // Test data bus for (uint32_t i = 0; i < MICROPY_HW_SDRAM_MEM_BUS_WIDTH; i++) { - *((uint32_t *)mem_base) = (1 << i); - if (*((uint32_t *)mem_base) != (1 << i)) { + *((volatile uint32_t *)mem_base) = (1 << i); + __DSB(); + if (*((volatile uint32_t *)mem_base) != (1 << i)) { #if MICROPY_HW_SDRAM_TEST_FAIL_ON_ERROR snprintf(error_buffer, sizeof(error_buffer), "Data bus test failed at 0x%p expected 0x%x found 0x%lx", - &mem_base[0], (1 << i), ((uint32_t *)mem_base)[0]); + &mem_base[0], (1 << i), ((volatile uint32_t *)mem_base)[0]); __fatal_error(error_buffer); #endif return false; @@ -325,6 +326,7 @@ bool __attribute__((optimize("O0"))) sdram_test(bool exhaustive) { // Test address bus for (uint32_t i = 1; i < MICROPY_HW_SDRAM_SIZE; i <<= 1) { mem_base[i] = pattern; + __DSB(); if (mem_base[i] != pattern) { #if MICROPY_HW_SDRAM_TEST_FAIL_ON_ERROR snprintf(error_buffer, sizeof(error_buffer), @@ -338,6 +340,7 @@ bool __attribute__((optimize("O0"))) sdram_test(bool exhaustive) { // Check for aliasing (overlaping addresses) mem_base[0] = antipattern; + __DSB(); for (uint32_t i = 1; i < MICROPY_HW_SDRAM_SIZE; i <<= 1) { if (mem_base[i] != pattern) { #if MICROPY_HW_SDRAM_TEST_FAIL_ON_ERROR @@ -356,15 +359,15 @@ bool __attribute__((optimize("O0"))) sdram_test(bool exhaustive) { // is enabled, it's not just writing and reading from cache. // Note: This test should also detect refresh rate issues. for (uint32_t i = 0; i < MICROPY_HW_SDRAM_SIZE; i++) { - mem_base[i] = pattern; + mem_base[i] = ((i % 2) ? pattern : antipattern); } for (uint32_t i = 0; i < MICROPY_HW_SDRAM_SIZE; i++) { - if (mem_base[i] != pattern) { + if (mem_base[i] != ((i % 2) ? pattern : antipattern)) { #if MICROPY_HW_SDRAM_TEST_FAIL_ON_ERROR snprintf(error_buffer, sizeof(error_buffer), "Address bus slow test failed at 0x%p expected 0x%x found 0x%x", - &mem_base[i], pattern, mem_base[i]); + &mem_base[i], ((i % 2) ? pattern : antipattern), mem_base[i]); __fatal_error(error_buffer); #endif return false; |
