Bugfix: LPC1768 IAP could not copy flash to flash (#156)

* Bugfix: LPC1768 IAP could not copy flash to flash

* Add flash to flash test for flash API

* Fix style

* Style again

* Augh again

hal/include/hal/flash_api.h

@@ -79,7 +79,14 @@ int32_t flash_read(flash_t *obj, uint32_t address, uint8_t *data, uint32_t size)
 /** Program pages starting at defined address
  *
  * The pages should not cross multiple sectors.
- * This function does not do any check for address alignments or if size is aligned to a page size.
+ *
+ * \note The upper level FlashIAP.cpp code guarantees:
+ * <ul><li>\c data is 32-bit aligned</li>
+ * <li>\c size is a multiple of the page size</li>
+ * <li>\c address is inside a flash sector</li>
+ * <li>\c address is aligned to the page size (but not the sector size)</li>
+ * </ul>So, implementations of this function do not need to check these things.
+ *
  * @param obj The flash object
  * @param address The sector starting address
  * @param data The data buffer to be programmed

hal/tests/TESTS/mbed_hal/flash/functional_tests/main.cpp

@@ -218,6 +218,53 @@ void flash_program_page_test()
     delete[] data_flashed;
 }
 
+// Tests that one flash page can be copied to another
+void flash_copy_flash_to_flash()
+{
+    flash_t test_flash;
+    int32_t ret = flash_init(&test_flash);
+    TEST_ASSERT_EQUAL_INT32(0, ret);
+
+    uint32_t last_page_address = flash_get_start_address(&test_flash) + flash_get_size(&test_flash) - flash_get_page_size(&test_flash);
+    uint32_t *last_page_pointer = reinterpret_cast<uint32_t *>(last_page_address);
+    uint32_t second_to_last_page_address = last_page_address - flash_get_page_size(&test_flash);
+    uint32_t *second_to_last_page_pointer = reinterpret_cast<uint32_t *>(second_to_last_page_address);
+
+    // Erase the sector(s) which contain the last two pages
+    uint32_t last_page_sector = ALIGN_DOWN(last_page_address, flash_get_sector_size(&test_flash, last_page_address));
+    uint32_t second_to_last_page_sector = ALIGN_DOWN(second_to_last_page_address, flash_get_sector_size(&test_flash, second_to_last_page_address));
+
+    ret = flash_erase_sector(&test_flash, last_page_sector);
+    TEST_ASSERT_EQUAL_INT32(0, ret);
+
+    if (last_page_sector != second_to_last_page_sector) {
+        ret = flash_erase_sector(&test_flash, second_to_last_page_sector);
+        TEST_ASSERT_EQUAL_INT32(0, ret);
+    }
+
+    // Fill second to last page with test data
+    size_t const numDataWords = flash_get_page_size(&test_flash) / sizeof(uint32_t);
+    uint32_t *data = new uint32_t[numDataWords];
+    for (size_t wordIdx = 0; wordIdx < numDataWords; ++wordIdx) {
+        data[wordIdx] = wordIdx;
+    }
+
+    ret = flash_program_page(&test_flash, second_to_last_page_address, reinterpret_cast<const uint8_t *>(data), flash_get_page_size(&test_flash));
+    TEST_ASSERT_EQUAL_INT32(0, ret);
+
+    // Make sure data was written
+    TEST_ASSERT_EQUAL_UINT32_ARRAY(data, second_to_last_page_pointer, numDataWords);
+
+    // Now, program last page from the second to last page
+    ret = flash_program_page(&test_flash, last_page_address, reinterpret_cast<const uint8_t *>(second_to_last_page_pointer), flash_get_page_size(&test_flash));
+    TEST_ASSERT_EQUAL_INT32(0, ret);
+
+    // Make sure data was written
+    TEST_ASSERT_EQUAL_UINT32_ARRAY(data, last_page_pointer, numDataWords);
+
+    delete[] data;
+}
+
 // check the execution speed at the start and end of the test to make sure
 // cache settings weren't changed
 void flash_clock_and_cache_test()
@@ -232,6 +279,7 @@ Case cases[] = {
     Case("Flash - mapping alignment", flash_mapping_alignment_test),
     Case("Flash - erase sector", flash_erase_sector_test),
     Case("Flash - program page", flash_program_page_test),
+    Case("Flash - copy flash to flash", flash_copy_flash_to_flash),
     Case("Flash - clock and cache test", flash_clock_and_cache_test),
 };
 

targets/TARGET_NXP/TARGET_LPC176X/device/flash_api.c

@@ -113,18 +113,23 @@ int32_t flash_program_page(flash_t *obj, uint32_t address,
         const uint8_t *data, uint32_t size)
 {
     unsigned long n;
-    const uint32_t copySize = 1024; // should be 256|512|1024|4096
-    uint8_t *alignedData, *source;
+    const uint32_t pageSize = flash_get_page_size(obj);
+    uint8_t *tempBuffer, *source;
 
-    alignedData = 0;
+    tempBuffer = 0;
     source = (uint8_t *)data;
+    
+    // On LPC1768, the first RAM bank starts at 0x1000000, so anywhere below that has to be flash.
+    // The IAP firmware does not support flash to flash copies, so if the source data is in flash
+    // it must be buffered in RAM.
+    bool isFlashToFlashCopy = (ptrdiff_t)(data) < 0x10000000;
 
     // check word boundary
-    if (((uint32_t)data % 4) != 0) {
+    if (isFlashToFlashCopy) {
         // always malloc outside critical section
-        alignedData = malloc(copySize);
-        if (alignedData == 0) {
-            return (1);
+        tempBuffer = malloc(pageSize);
+        if (tempBuffer == 0) {
+            return -1;
         }
     }
 
@@ -132,10 +137,12 @@ int32_t flash_program_page(flash_t *obj, uint32_t address,
 
     core_util_critical_section_enter();
 
-    while (size) {
-        if (((uint32_t)data % 4) != 0) {
-            memcpy(alignedData, source, copySize);
-            source = alignedData;
+    for(size_t pageIdx = 0; pageIdx < (size / pageSize); ++pageIdx)
+    {
+        uint8_t * pageSourceAddr = source + (pageIdx * pageSize);
+        if (isFlashToFlashCopy) {
+            memcpy(tempBuffer, pageSourceAddr, pageSize);
+            pageSourceAddr = tempBuffer;
         }
 
         /*
@@ -147,28 +154,28 @@ int32_t flash_program_page(flash_t *obj, uint32_t address,
         IAP.par[1] = n;  // End Sector
         IAP_Call (&IAP.cmd, &IAP.stat); // Call IAP Command
         if (IAP.stat) {
-            return (1); // Command Failed
+            core_util_critical_section_exit();
+            return -1; // Command Failed
         }
 
         IAP.cmd = 51; // Copy RAM to Flash
         IAP.par[0] = address; // Destination Flash Address
-        IAP.par[1] = (unsigned long)source; // Source RAM Address        
-        IAP.par[2] = copySize; // number of bytes to be written
+        IAP.par[1] = (unsigned long)pageSourceAddr; // Source RAM Address
+        IAP.par[2] = pageSize; // number of bytes to be written
         IAP.par[3] = CCLK; // CCLK in kHz
         IAP_Call (&IAP.cmd, &IAP.stat); // Call IAP Command
         if (IAP.stat) {
-            return (1); // Command Failed
+            core_util_critical_section_exit();
+            return -1; // Command Failed
         }
 
-        source += copySize;
-        size -= copySize;
-        address += copySize;
+        address += pageSize;
     }
 
     core_util_critical_section_exit();
 
-    if(alignedData != 0) { // We allocated our own memory
-        free(alignedData);
+    if(tempBuffer != 0) { // We allocated our own memory
+        free(tempBuffer);
     }
 
     return (0); // Finished without Errors