bd: Added block randomizations and some more helpful checks to testing

Author: geky

features/filesystem/spif/SPIFBlockDevice.cpp

@@ -24,7 +24,7 @@
 #define SPIF_TIMEOUT    10000
 
 // Debug available
-#define SPIF_DEBUG      true
+#define SPIF_DEBUG      0
 
 // MX25R Series Register Command Table. 
 enum ops {
@@ -51,20 +51,22 @@ SPIFBlockDevice::SPIFBlockDevice(
     PinName mosi, PinName miso, PinName sclk, PinName cs, int freq)
     : _spi(mosi, miso, sclk), _cs(cs), _size(0)
 {
+    _cs = 1;
     _spi.frequency(freq);
 }
 
 bd_error_t SPIFBlockDevice::init()
 {
     // Check for vendor specific hacks, these should move into more general
     // handling when possible. RDID is not used to verify a device is attached.
-    uint8_t id;
-    _cmdread(SPIF_RDID, 0, 1, 0x0, &id);
+    uint8_t id[3];
+    _cmdread(SPIF_RDID, 0, 3, 0x0, id);
 
-    switch (id) {
+    switch (id[0]) {
         case 0xbf:
             // SST devices come preset with block protection
             // enabled for some regions, issue gbpu instruction to clear
+            _wren();
             _cmdwrite(0x98, 0, 0, 0x0, NULL);
             break;
     }
@@ -82,13 +84,13 @@ bd_error_t SPIFBlockDevice::init()
 
     // Verify SFDP signature for sanity
     // Also check that major/minor version is acceptable
-    if (!(memcmp(&header[0], "SFDP", 4) == 0 && header[4+1] == 1 && header[4+0] >= 6)) {
+    if (!(memcmp(&header[0], "SFDP", 4) == 0 && header[5] == 1)) {
         return BD_ERROR_DEVICE_ERROR;
     }
 
     // The SFDP spec indicates the standard table is always at offset 0
     // in the parameter headers, we check just to be safe
-    if (!(header[8] == 0 && header[9+1] == 1 && header[9+0] >= 6)) {
+    if (!(header[8] == 0 && header[10] == 1)) {
         return BD_ERROR_DEVICE_ERROR;
     }
 
@@ -152,17 +154,15 @@ void SPIFBlockDevice::_cmdread(
     _cs = 1;
 
     if (SPIF_DEBUG) {
-        printf("spif %02x", op);
-        for (uint32_t i = 0; i < 4; i++) {
+        printf("spif <- %02x", op);
+        for (uint32_t i = 0; i < addrc; i++) {
             if (i < addrc) {
                 printf("%02lx", 0xff & (addr >> 8*(addrc-1 - i)));
             } else {
                 printf("  ");
             }
         }
-        if (retc > 0) {
-            printf(" <- ");
-        }
+        printf(" ");
         for (uint32_t i = 0; i < 16 && i < retc; i++) {
             printf("%02x", rets[i]);
         }
@@ -190,17 +190,15 @@ void SPIFBlockDevice::_cmdwrite(
     _cs = 1;
 
     if (SPIF_DEBUG) {
-        printf("spif %02x", op);
-        for (uint32_t i = 0; i < 4 || i < addrc; i++) {
+        printf("spif -> %02x", op);
+        for (uint32_t i = 0; i < addrc; i++) {
             if (i < addrc) {
                 printf("%02lx", 0xff & (addr >> 8*(addrc-1 - i)));
             } else {
                 printf("  ");
             }
         }
-        if (argc > 0) {
-            printf(" -> ");
-        }
+        printf(" ");
         for (uint32_t i = 0; i < 16 && i < argc; i++) {
             printf("%02x", args[i]);
         }
@@ -274,12 +272,16 @@ bd_error_t SPIFBlockDevice::program(const void *buffer, bd_addr_t addr, bd_size_
         }
 
         // Write up to 256 bytes a page
-        uint32_t chunk = (size < 256) ? size : 256;
+        // TODO handle unaligned programs
+        uint32_t off = addr % 256;
+        uint32_t chunk = (off + size < 256) ? size : (256-off);
         _cmdwrite(SPIF_PROG, 3, chunk, addr, static_cast<const uint8_t *>(buffer));
         buffer = static_cast<const uint8_t*>(buffer) + chunk;
         addr += chunk;
         size -= chunk;
 
+        wait_ms(1);
+
         err = _sync();
         if (err) {
             return err;
@@ -318,17 +320,17 @@ bd_error_t SPIFBlockDevice::erase(bd_addr_t addr, bd_size_t size)
     return 0;
 }
 
-bd_size_t SPIFBlockDevice::read_size()
+bd_size_t SPIFBlockDevice::get_read_size()
 {
     return SPIF_READ_SIZE;
 }
 
-bd_size_t SPIFBlockDevice::program_size()
+bd_size_t SPIFBlockDevice::get_program_size()
 {
     return SPIF_PROG_SIZE;
 }
 
-bd_size_t SPIFBlockDevice::erase_size()
+bd_size_t SPIFBlockDevice::get_erase_size()
 {
     return SPIF_SE_SIZE;
 }

features/filesystem/spif/SPIFBlockDevice.h

@@ -23,7 +23,8 @@
 #include "BlockDevice.h"
 
 
-/** BlockDevice for the SPIF series of spi-based flash chips
+/** BlockDevice for SPI based flash devices
+ *  such as the MX25R or SST26F016B
  *
  *  @code
  *  #include "mbed.h"
@@ -36,15 +37,15 @@
  *      printf("mx25r test\n");
  *      mx52r.init();
  *      printf("mx25r size: %llu\n", mx25r.size());
- *      printf("mx25r read size: %llu\n", mx25r.read_size());
- *      printf("mx25r program size: %llu\n", mx25r.program_size());
- *      printf("mx25r erase size: %llu\n", mx25r.erase_size());
+ *      printf("mx25r read size: %llu\n", mx25r.get_read_size());
+ *      printf("mx25r program size: %llu\n", mx25r.get_program_size());
+ *      printf("mx25r erase size: %llu\n", mx25r.get_erase_size());
  *
- *      uint8_t *buffer = malloc(mx25r.erase_size());
+ *      uint8_t *buffer = malloc(mx25r.get_erase_size());
  *      sprintf(buffer, "Hello World!\n");
- *      mx25r.erase(0, mx25r.erase_size());
- *      mx25r.write(buffer, 0, mx25r.erase_size());
- *      mx25r.read(buffer, 0, mx25r.erase_size());
+ *      mx25r.erase(0, mx25r.get_erase_size());
+ *      mx25r.write(buffer, 0, mx25r.get_erase_size());
+ *      mx25r.read(buffer, 0, mx25r.get_erase_size());
  *      printf("%s", buffer);
  *
  *      mx25r.deinit();
@@ -58,9 +59,9 @@ class SPIFBlockDevice : public BlockDevice {
      *  @param miso     SPI master in, slave out pin
      *  @param sclk     SPI clock pin
      *  @param csel     SPI chip select pin
-     *  @param freq     Clock speed of the SPI bus (defaults to 50MHz)
+     *  @param freq     Clock speed of the SPI bus (defaults to 40MHz)
      */
-    SPIFBlockDevice(PinName mosi, PinName miso, PinName sclk, PinName csel, int freq=50000000);
+    SPIFBlockDevice(PinName mosi, PinName miso, PinName sclk, PinName csel, int freq=4000000);
 
     /** Initialize a block device
      *
@@ -108,21 +109,21 @@ class SPIFBlockDevice : public BlockDevice {
      *
      *  @return         Size of a readable block in bytes
      */
-    virtual bd_size_t read_size();
+    virtual bd_size_t get_read_size();
 
     /** Get the size of a programable block
      *
      *  @return         Size of a programable block in bytes
      *  @note Must be a multiple of the read size
      */
-    virtual bd_size_t program_size();
+    virtual bd_size_t get_program_size();
 
     /** Get the size of a eraseable block
      *
      *  @return         Size of a eraseable block in bytes
      *  @note Must be a multiple of the program size
      */
-    virtual bd_size_t erase_size();
+    virtual bd_size_t get_erase_size();
 
     /** Get the total size of the underlying device
      *

features/filesystem/spif/TESTS/block_device/spif/main.cpp

@@ -16,46 +16,111 @@ using namespace utest::v1;
 //#error [NOT_SUPPORTED] SPIF Required
 //#endif
 
-#define BLOCK_SIZE 4096
-uint8_t write_block[BLOCK_SIZE];
-uint8_t read_block[BLOCK_SIZE];
+#if defined(TARGET_K82F)
+#define TEST_PINS PTE2, PTE4, PTE1, PTE5
+#define TEST_FREQ 40000000
+#else
+#define TEST_PINS D11, D12, D13, D10
+#define TEST_FREQ 1000000
+#endif
+
+#define TEST_BLOCK_COUNT 10
+#define TEST_ERROR_MASK 16
+
+const struct {
+    const char *name;
+    bd_size_t (BlockDevice::*method)();
+} ATTRS[] = {
+    {"read size",    &BlockDevice::get_read_size},
+    {"program size", &BlockDevice::get_program_size},
+    {"erase size",   &BlockDevice::get_erase_size},
+    {"total size",   &BlockDevice::size},
+};
 
 
 void test_read_write() {
-    //SPIFBlockDevice bd(PTE2, PTE4, PTE1, PTE5);
-    SPIFBlockDevice bd(D11, D12, D13, D10);
+    SPIFBlockDevice bd(TEST_PINS, TEST_FREQ);
 
     int err = bd.init();
     TEST_ASSERT_EQUAL(0, err);
 
-    printf("read size: %llu\r\n", bd.read_size());
-    printf("program size: %llu\r\n", bd.program_size());
-    printf("erase size: %llu\r\n", bd.erase_size());
-
-    // Fill with random sequence
-    srand(1);
-    for (int i = 0; i < BLOCK_SIZE; i++) {
-        write_block[i] = 0xff & rand();
+    for (unsigned a = 0; a < sizeof(ATTRS)/sizeof(ATTRS[0]); a++) {
+        static const char *prefixes[] = {"", "k", "M", "G"};
+        for (int i = 3; i >= 0; i--) {
+            bd_size_t size = (bd.*ATTRS[a].method)();
+            if (size >= (1ULL << 10*i)) {
+                printf("%s: %llu%sbytes (%llubytes)\n",
+                    ATTRS[a].name, size >> 10*i, prefixes[i], size);
+                break;
+            }
+        }
     }
 
-    // Write, sync, and read the block
-    err = bd.write(write_block, 0, BLOCK_SIZE);
-    TEST_ASSERT_EQUAL(0, err);
-
-    err = bd.read(read_block, 0, BLOCK_SIZE);
-    TEST_ASSERT_EQUAL(0, err);
-
-    // Check that the data was unmodified
-    printf("w (0x%02x%02x%02x%02x)\n",
-        write_block[0], write_block[1], write_block[2], write_block[3],
-        write_block[4], write_block[5], write_block[6], write_block[7]);
-    printf("r (0x%02x%02x%02x%02x)\n",
-        read_block[0], read_block[1], read_block[2], read_block[3],
-        read_block[4], read_block[5], read_block[6], read_block[7]);
-
-    srand(1);
-    for (int i = 0; i < BLOCK_SIZE; i++) {
-        TEST_ASSERT_EQUAL(0xff & rand(), read_block[i]);
+    bd_size_t block_size = bd.get_erase_size();
+    uint8_t *write_block = new uint8_t[block_size];
+    uint8_t *read_block = new uint8_t[block_size];
+    uint8_t *error_mask = new uint8_t[TEST_ERROR_MASK];
+    unsigned addrwidth = ceil(log(bd.size()-1) / log(16))+1;
+
+    for (int b = 0; b < TEST_BLOCK_COUNT; b++) {
+        // Find a random block
+        bd_addr_t block = (rand()*block_size) % bd.size();
+
+        // Use next random number as temporary seed to keep
+        // the address progressing in the pseudorandom sequence
+        unsigned seed = rand();
+
+        // Fill with random sequence
+        srand(seed);
+        for (bd_size_t i = 0; i < block_size; i++) {
+            write_block[i] = 0xff & rand();
+        }
+
+        // Write, sync, and read the block
+        printf("test  %0*llx:%llu...\n", addrwidth, block, block_size);
+
+        err = bd.write(write_block, block, block_size);
+        TEST_ASSERT_EQUAL(0, err);
+
+        printf("write %0*llx:%llu ", addrwidth, block, block_size);
+        for (int i = 0; i < 16; i++) {
+            printf("%02x", write_block[i]);
+        }
+        printf("...\n");
+
+        err = bd.read(read_block, block, block_size);
+        TEST_ASSERT_EQUAL(0, err);
+
+        printf("read  %0*llx:%llu ", addrwidth, block, block_size);
+        for (int i = 0; i < 16; i++) {
+            printf("%02x", read_block[i]);
+        }
+        printf("...\n");
+
+        // Find error mask for debugging
+        memset(error_mask, 0, TEST_ERROR_MASK);
+        bd_size_t error_scale = block_size / (TEST_ERROR_MASK*8);
+
+        srand(seed);
+        for (bd_size_t i = 0; i < TEST_ERROR_MASK*8; i++) {
+            for (bd_size_t j = 0; j < error_scale; j++) {
+                if ((0xff & rand()) != read_block[i*error_scale + j]) {
+                    error_mask[i/8] |= 1 << (i%8);
+                }
+            }
+        }
+
+        printf("error %0*llx:%llu ", addrwidth, block, block_size);
+        for (int i = 0; i < 16; i++) {
+            printf("%02x", error_mask[i]);
+        }
+        printf("\n");
+
+        // Check that the data was unmodified
+        srand(seed);
+        for (bd_size_t i = 0; i < block_size; i++) {
+            TEST_ASSERT_EQUAL(0xff & rand(), read_block[i]);
+        }
     }
 
     err = bd.deinit();
@@ -65,12 +130,12 @@ void test_read_write() {
 
 // Test setup
 utest::v1::status_t test_setup(const size_t number_of_cases) {
-    GREENTEA_SETUP(10*1000*1000, "default_auto");
+    GREENTEA_SETUP(30, "default_auto");
     return verbose_test_setup_handler(number_of_cases);
 }
 
 Case cases[] = {
-    Case("Testing read write of a block", test_read_write),
+    Case("Testing read write random blocks", test_read_write),
 };
 
 Specification specification(test_setup, cases);