Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor flash setup for MIMXRT105x devices, add flash API support for MIMXRT1062 EVK #187

Merged
merged 4 commits into from
Oct 1, 2023

Conversation

multiplemonomials
Copy link
Collaborator

Summary of changes

I will say that, this PR was a journey. I started off with a simple-ish goal: adding Flash API support for the MIMRT1060 EVK. Since the MIMXRT1050, which is basically the same silicon, already supports Flash, this should be easy... right?

Well, not so much. I started by doing a bit of a refactor of how flash information is tracked for MIMXRT105x devices. Instead of putting some of it it as defines in targets.json, and some of it in device.h, it all now lives in a header which is specific to each target, mimxrt_memory_info.h. This way we can collect all of these constants in one place that's easy to modify when adding a new MIMXRT105x family target (and hopefully other MIMXRT targets later on as well!). This basically worked, and meant that I could easily adapt the MIMXRT1050_EVK flash_api.c to work for MIMXRT1060_EVK as well.

But, when I tried to use this code, bad stuff happened. You see, the 1050 EVK board uses a different type of flash chip (OSPI) by default. And, while NXP did implement flash stuff for QSPI, they apparently didn't test it very well, because it did not work when I tried to use it. Or, more specifically, it mostly worked, BUT whenever I initialized a flash_t, the application would sometimes randomly crash at a later point in the execution. From what I could tell, it would crash because it didn't execute the correct instructions from the program image, e.g. it would go right over a branch instruction without branching. But, it was quite difficult to nail down the issue, because it would happen repeatably with the same compiled program but would vanish and come back if you changed unrelated parts of the code.

After about 6 hours of debugging and A-B testing, I was able to narrow this down to the FLEXSPI_Init() call in flash_init(). Removing this call caused the issue to not happen anymore. My suspicion is that this call was resetting the FLEXSPI peripheral while background accesses to it were still happening, causing invalid data to get transferred to the processor instruction cache, where it would later be executed. I tested two mitigations: waiting to reset the peripheral until it was no longer busy, and clearing the I-cache after doing the initialization. Both of these fixed the issue on their own, so I added both (only way to be sure!).

I also made another largeish change to improve reliability: until now, the MIMXRT would use the boot rom's configuration for the flash peripheral until you called flash_init() for the first time in code. This means that applications which use the flash API are potentially operating using a different flash configuration than those that don't. This feels like a source of heisenbugs, since only 1 test uses the flash API and all the rest don't. So, I moved the flash config code into a new function mimxrt_flash_setup() which is always called at boot. So, now every application will always use the full-featured flash config with write and erase support.

That did seem to fix the issues with the flash API, but I then started getting a bunch of new failures in the test suite for basically any code that uses deep sleep. This made me do a little more digging through the deep sleep code, which I have a little more insight into now that I found AN12085, which seems to be the "implementation guide" for doing deep sleep on MIMXRT. I discovered that the deep sleep code also resets the FlexSPI peripheral when changing its clock, so it could have the same type of issue as with the flash API. Adding the "wait until not busy" loop didn't seem to do anything, but clearing the I-cache helped a lot.

Now, both the tests that used to be flaky (hal-sleep, rtos-mutex, etc) and the other deep sleep tests that newly started failing seem to work reliably. The only two exceptions are hal-lp-ticker and hal-rtc. Both of these seem to suffer some sort of stack corruption after coming out of deep sleep. I think that a future effort will be needed to dive deeper into MIMXRT deep sleep and debug these issues.

Ultimately, while this MR doesn't fix every test issue on the MIMXRT, I think it still makes a significant improvement. It trades some flaky, nebulous issues for ones which occur more repeatably, which seems like a win in my book. And it also lays the groundwork for adding flash support in the future for other MIMXRT targets such as Teensy!

Impact of changes

MIMXRT1060_EVK now supports Flash IAP API. Some deep sleep weirdness on MIMXRT105x family devices fixed.

Migration actions required

Documentation


Pull request type

[] Patch update (Bug fix / Target update / Docs update / Test update / Refactor)
[X] Feature update (New feature / Functionality change / New API)
[] Major update (Breaking change E.g. Return code change / API behaviour change)

Test results

[] No Tests required for this change (E.g docs only update)
[] Covered by existing mbed-os tests (Greentea or Unittest)
[X] Tests / results supplied as part of this PR
96% tests passed, 3 tests failed out of 81

Total Test time (real) = 1487.79 sec

The following tests did not run:
          8 - mbed-hal-crc (Skipped)
         13 - mbed-hal-mpu (Skipped)
         14 - mbed-hal-ospi (Skipped)
         16 - mbed-hal-qspi (Skipped)
         17 - mbed-hal-reset-reason (Skipped)
         19 - mbed-hal-rtc-reset (Skipped)
         43 - mbed-platform-crash-reporting (Skipped)
         53 - mbed-platform-stats-cpu (Skipped)
         54 - mbed-platform-stats-heap (Skipped)
         55 - mbed-platform-stats-sys (Skipped)
         56 - mbed-platform-stats-thread (Skipped)
         72 - mbed-connectivity-mbedtls-sanity (Skipped)
         74 - mbed-connectivity-nfc-eeprom (Skipped)
         76 - mbed-connectivity-netsocket-nidd (Skipped)

The following tests FAILED:
         11 - mbed-hal-lp-ticker (Failed)
         18 - mbed-hal-rtc (Failed)
         27 - mbed-hal-trng (Failed)
Errors while running CTest

Reviewers


@multiplemonomials multiplemonomials merged commit 9f4987d into master Oct 1, 2023
9 checks passed
@multiplemonomials multiplemonomials deleted the dev/mimxrt1060-flash-support branch October 1, 2023 16:25
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