electro-smith / libDaisy

Hardware Library for the Daisy Audio Platform
https://www.electro-smith.com/daisy
MIT License
312 stars 131 forks source link

Fix QSPI memory cache invalidation for PersistentStorage #572

Closed ndonald2 closed 1 year ago

ndonald2 commented 1 year ago

Summary

Band-aid fix for #570

Details

If the program is running outside internal flash, we need to invalidate the DCache for the memory mapped to QSPI before comparing settings data, otherwise it will always be "dirty" until the next device reboot.

See linked issue #570 for more details.

ndonald2 commented 1 year ago

Noticed the change broke unit tests so I did the simplest thing I could think of and skipped the cache invalidation code when building for unit tests 🤷 LMK if you think there's a better solution. I wasn't sure how best to stub/mock the dependencies on System and the cache invalidate function.

stephenhensley commented 1 year ago

@ndonald2 as ugly as ifndefs can be, I think this a suitable fix for now. It's very clear why it would be getting skipped.

Thanks so much for the fix!

thealienthing commented 1 year ago

Hey all! I'm a student doing a project on daisy seed and I've been having this very issue but I have the latest changes in libdaisy with this PR included. I'm not certain as to what part of memory my firmware is being stored since I'm still very new to stm32 development, but if it helps understand, I'm pretty much using my seed in the most default way I know of by pressing the boot and reset buttons and flashing the firmware with make program-dfu. So I believe my program is in fact running within internal flash.

Either way, I'm having the same behavior of not seeing changes to my flash data between qspi erase/write actions without performing a reboot. Any ideas where I should look to fix this? I appreciate any help I can get.

stephenhensley commented 1 year ago

Hi @thealienthing

So the fix here applied only when not running from internal flash. It's possible that there's a similar/related issue when running from internal flash.

To see if the same solution resolves the problem, you can do either of the following:

  1. Comment out lines 129-130 of the src/util/PersistentStorage.h and recompile both libDaisy, and your project.

or

  1. manually invalidate the cache before attempting to read back the data from the reference provided by PersistentStorage<>::GetSettings():
SCB_InvalidateDCache();

Let me know if either of those work, and we can make a patch to get it fixed.

Thanks!

thealienthing commented 1 year ago

Excellent! I appreciate your quick response. I'll give this a try tonight and let you know if that solves the problem.