electro-smith / libDaisy

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

QSPI erase bounds error correction #510

Closed CorvusPrudens closed 2 years ago

CorvusPrudens commented 2 years ago

Currently, the QSPI erase routine will erase one 4kB sector beyond the given end address, instead of the expected behavior of stopping there. I created the following routine to verify the error exists and that the fix works:

#include "daisy_seed.h"

using namespace daisy;

DaisySeed       hw;

#define TEST_ADDR 0x40000
#define TEST_LEN 8192
uint8_t test_ram[TEST_LEN];

void blink(uint32_t interval)
{
    uint32_t shifted = 1 << interval;
    while (1)
    {
        hw.SetLed(System::GetNow() & shifted);
    }
}

int main(void)
{
    hw.Init();

    for(uint32_t i = 0; i < TEST_LEN; i++)
    {
        test_ram[i] = i & 255;
    }

    uint8_t* qspi_buffer = (uint8_t*) (0x90000000 + TEST_ADDR);

    // First write full sequence
    hw.qspi.Erase(TEST_ADDR, TEST_ADDR + TEST_LEN);
    hw.qspi.Write(TEST_ADDR, TEST_LEN, test_ram);

    for (uint32_t i = 0; i < TEST_LEN; i++)
    {
        if (test_ram[i] != qspi_buffer[i])
        {
            hw.SetLed(true);
            while (1);
        }
    }

    // Now erase just first sector and test second
    hw.qspi.Erase(TEST_ADDR, TEST_ADDR + (TEST_LEN / 2));

    for (uint32_t i = TEST_LEN / 2; i < TEST_LEN; i++)
    {
        if (test_ram[i] != qspi_buffer[i])
        {
            blink(6);
        }
    }

    // If the LED toggles state every half-second, the test passed
    blink(9);
}

Before applying the fix to libDaisy, a rev4 Seed will produce the fast blinking pattern that we expect if both 4kB sectors are erased instead of only the first. After the fix, the seed passed the test and blinked slowly.

github-actions[bot] commented 2 years ago

Unit Test Results

    1 files  ±0    16 suites  ±0   0s :stopwatch: ±0s 147 tests ±0  147 :heavy_check_mark: ±0  0 :zzz: ±0  0 :x: ±0 

Results for commit fd98256d. ± Comparison against base commit c8a34a7c.

:recycle: This comment has been updated with latest results.

stephenhensley commented 2 years ago

nice fix, and well documented verification.

Can you add a note to the changelog?

Otherwise looks good!