Rewrite flashmem driver to cope with misaligned RAM buffers (#1490)

Sming/Services/SpifFS/spiffs_config.h

@@ -11,7 +11,7 @@
 // ----------- 8< ------------
 #ifdef __ets__
 	#include <user_config.h>
-	#include "../system/flashmem.h"
+	#include "flashmem.h"
 #else
 	#include "spiffy_host.h"
 #endif /* __ets__ */

Sming/system/flashmem.c

@@ -1,102 +1,179 @@
-#include "flashmem.h"
-#include <stdlib.h>
+/****
+ * Sming Framework Project - Open Source framework for high efficiency native ESP8266 development.
+ * Created 2015 by Skurydin Alexey
+ * http://github.com/SmingHub/Sming
+ * All files of the Sming Core are provided under the LGPL v3 license.
+ *
+ * Based on NodeMCU platform_flash
+ * https://github.com/nodemcu/nodemcu-firmware
+ *
+ * @author: 27/8/2018 - Mikee47 <[email protected]>
+ *
+ * Rewritten to deal with misaligned RAM buffers.
+ *
+ ****/
 
-// Based on NodeMCU platform_flash
-// https://github.com/nodemcu/nodemcu-firmware
+#include "flashmem.h"
+#include "espinc/peri.h"
 
 extern char _flash_code_end[];
 
-uint32_t flashmem_write( const void *from, uint32_t toaddr, uint32_t size )
+/*
+ * To ensure memory alignment a temporary buffer is used by flashmem_read and flashmem_write functions.
+ *
+ * The buffer must be an integer multiple of INTERNAL_FLASH_WRITE_UNIT_SIZE.
+ */
+#define FLASH_BUFFERS 32
+
+/** @brief determines if the given value is aligned to a word (4-byte) boundary */
+#undef IS_ALIGNED
+#define IS_ALIGNED(x) (((uint32_t)(x)&0x00000003) == 0)
+
+// Buffers need to be word aligned for flash access
+#define __aligned __attribute__((aligned(4)))
+
+// If this module were in C++ we could use std::min
+static inline uint32_t min(uint32_t a, uint32_t b)
 {
-  uint32_t temp, rest, ssize = size;
-  unsigned i;
-  char tmpdata[ INTERNAL_FLASH_WRITE_UNIT_SIZE ];
-  const uint8_t *pfrom = ( const uint8_t* )from;
-  const uint32_t blksize = INTERNAL_FLASH_WRITE_UNIT_SIZE;
-  const uint32_t blkmask = INTERNAL_FLASH_WRITE_UNIT_SIZE - 1;
-
-  // Align the start
-  if( toaddr & blkmask )
-  {
-    rest = toaddr & blkmask;
-    temp = toaddr & ~blkmask; // this is the actual aligned address
-    flashmem_read_internal( tmpdata, temp, blksize );
-    for( i = rest; size && ( i < blksize ); i ++, size --, pfrom ++ )
-      tmpdata[ i ] = *pfrom;
-    flashmem_write_internal( tmpdata, temp, blksize );
-    if( size == 0 )
-      return ssize;
-    toaddr = temp + blksize;
-  }
-  // The start address is now a multiple of blksize
-  // Compute how many bytes we can write as multiples of blksize
-  rest = size & blkmask;
-  temp = size & ~blkmask;
-  // Program the blocks now
-  if( temp )
-  {
-	flashmem_write_internal( pfrom, toaddr, temp );
-    toaddr += temp;
-    pfrom += temp;
-  }
-  // And the final part of a block if needed
-  if( rest )
-  {
-    flashmem_read_internal( tmpdata, toaddr, blksize );
-    for( i = 0; size && ( i < rest ); i ++, size --, pfrom ++ )
-      tmpdata[ i ] = *pfrom;
-    flashmem_write_internal( tmpdata, toaddr, blksize );
-  }
-  return ssize;
+	return (a < b) ? a : b;
 }
 
-uint32_t flashmem_read( void *to, uint32_t fromaddr, uint32_t size )
+uint32_t flashmem_write(const void* from, uint32_t toaddr, uint32_t size)
 {
-  uint32_t temp, rest, ssize = size;
-  unsigned i;
-  char tmpdata[ INTERNAL_FLASH_READ_UNIT_SIZE ];
-  uint8_t *pto = ( uint8_t* )to;
-  const uint32_t blksize = INTERNAL_FLASH_READ_UNIT_SIZE;
-  const uint32_t blkmask = INTERNAL_FLASH_READ_UNIT_SIZE - 1;
-
-  // Align the start
-  if( fromaddr & blkmask )
-  {
-    rest = fromaddr & blkmask;
-    temp = fromaddr & ~blkmask; // this is the actual aligned address
-    flashmem_read_internal( tmpdata, temp, blksize );
-    for( i = rest; size && ( i < blksize ); i ++, size --, pto ++ )
-      *pto = tmpdata[ i ];
-
-    if( size == 0 )
-      return ssize;
-    fromaddr = temp + blksize;
-  }
-  // The start address is now a multiple of blksize
-  // Compute how many bytes we can read as multiples of blksize
-  rest = size & blkmask;
-  temp = size & ~blkmask;
-  // Program the blocks now
-  if( temp )
-  {
-	flashmem_read_internal( pto, fromaddr, temp );
-    fromaddr += temp;
-    pto += temp;
-  }
-  // And the final part of a block if needed
-  if( rest )
-  {
-	flashmem_read_internal( tmpdata, fromaddr, blksize );
-    for( i = 0; size && ( i < rest ); i ++, size --, pto ++ )
-      *pto = tmpdata[ i ];
-  }
-  return ssize;
+	if(IS_ALIGNED(from) && IS_ALIGNED(toaddr) && IS_ALIGNED(size))
+		return flashmem_write_internal(from, toaddr, size);
+
+	const uint32_t blksize = INTERNAL_FLASH_WRITE_UNIT_SIZE;
+	const uint32_t blkmask = INTERNAL_FLASH_WRITE_UNIT_SIZE - 1;
+
+	__aligned char tmpdata[FLASH_BUFFERS * INTERNAL_FLASH_WRITE_UNIT_SIZE];
+	const uint8_t* pfrom = (const uint8_t*)from;
+	uint32_t remain = size;
+
+	// Align the start
+	if(toaddr & blkmask)
+	{
+		uint32_t rest = toaddr & blkmask;
+		uint32_t addr_aligned = toaddr & ~blkmask; // this is the actual aligned address
+
+		// Read existing unit and overlay with new data
+		if(flashmem_read_internal(tmpdata, addr_aligned, blksize) != blksize)
+			return 0;
+		while(remain && rest < blksize)
+		{
+			tmpdata[rest++] = *pfrom++;
+			--remain;
+		}
+
+		// Write the unit
+		uint32_t written = flashmem_write_internal(tmpdata, addr_aligned, blksize);
+		if(written != blksize)
+			return written;
+
+		if (remain == 0)
+			return size;
+
+		toaddr = addr_aligned + blksize;
+	}
+
+	// The start address is now a multiple of blksize
+	// Compute how many bytes we can write as multiples of blksize
+	uint32_t rest = remain & blkmask;
+	remain &= ~blkmask;
+	// Program the blocks now
+	while(remain)
+	{
+		unsigned count = min(remain, sizeof(tmpdata));
+		memcpy(tmpdata, pfrom, count);
+		uint32_t written = flashmem_write_internal(tmpdata, toaddr, count);
+		remain -= written;
+		if(written != count)
+			return size - remain;
+		toaddr += count;
+		pfrom += count;
+	}
+
+	// And the final part of a block if needed
+	if(rest)
+	{
+		if(flashmem_read_internal(tmpdata, toaddr, blksize) != blksize)
+			return size - remain;
+		unsigned i;
+		for(i = 0; i < rest; ++i)
+			tmpdata[i] = *pfrom++;
+		uint32_t written = flashmem_write_internal(tmpdata, toaddr, blksize);
+		remain -= written;
+		if(written != blksize)
+			return size - remain;
+	}
+
+	return size;
+}
+
+uint32_t flashmem_read(void* to, uint32_t fromaddr, uint32_t size)
+{
+	if(IS_ALIGNED(to) && IS_ALIGNED(fromaddr) && IS_ALIGNED(size))
+		return flashmem_read_internal(to, fromaddr, size);
+
+	const uint32_t blksize = INTERNAL_FLASH_READ_UNIT_SIZE;
+	const uint32_t blkmask = INTERNAL_FLASH_READ_UNIT_SIZE - 1;
+
+	__aligned char tmpdata[FLASH_BUFFERS * INTERNAL_FLASH_READ_UNIT_SIZE];
+	uint8_t* pto = (uint8_t*)to;
+	uint32_t remain = size;
+
+	// Align the start
+	if(fromaddr & blkmask)
+	{
+		uint32_t rest = fromaddr & blkmask;
+		uint32_t addr_aligned = fromaddr & ~blkmask; // this is the actual aligned address
+		if (flashmem_read_internal(tmpdata, addr_aligned, blksize) != blksize)
+			return 0;
+		// memcpy(pto, &tmpdata[rest], std::min(blksize - rest, remain))
+		while(remain && rest < blksize)
+		{
+			*pto++ = tmpdata[rest++];
+			--remain;
+		}
+		if (remain == 0)
+			return size;
+		fromaddr = addr_aligned + blksize;
+	}
+
+	// The start address is now a multiple of blksize
+	// Compute how many bytes we can read as multiples of blksize
+	uint32_t rest = remain & blkmask;
+	remain &= ~blkmask;
+	// Read the blocks now
+	while(remain)
+	{
+		unsigned count = min(remain, sizeof(tmpdata));
+		uint32_t read = flashmem_read_internal(tmpdata, fromaddr, count);
+		memcpy(pto, tmpdata, read);
+		remain -= read;
+		if(read != count)
+			return size - remain;
+		fromaddr += count;
+		pto += count;
+	}
+
+	// And the final part of a block if needed
+	if(rest)
+	{
+		if(flashmem_read_internal(tmpdata, fromaddr, blksize) != blksize)
+			return size - remain;
+		unsigned i;
+		for(i = 0; i < rest; ++i)
+			*pto++ = tmpdata[i];
+	}
+
+	return size;
 }
 
-bool flashmem_erase_sector( uint32_t sector_id )
+bool flashmem_erase_sector(uint32_t sector_id)
 {
-  WRITE_PERI_REG(0x60000914, 0x73);
-  return spi_flash_erase_sector( sector_id ) == SPI_FLASH_RESULT_OK;
+	WDT_FEED();
+	return spi_flash_erase_sector(sector_id) == SPI_FLASH_RESULT_OK;
 }
 
 SPIFlashInfo flashmem_get_info()
@@ -149,9 +226,7 @@ uint16_t flashmem_get_size_sectors()
     return flashmem_get_size_bytes() / SPI_FLASH_SEC_SIZE;
 }
 
-// Helper function: find the flash sector in which an address resides
-// Return the sector number, as well as the start and end address of the sector
-uint32_t flashmem_find_sector( uint32_t address, uint32_t *pstart, uint32_t *pend )
+uint32_t flashmem_find_sector(uint32_t address, uint32_t *pstart, uint32_t *pend)
 {
   // All the sectors in the flash have the same size, so just align the address
   uint32_t sect_id = address / INTERNAL_FLASH_SECTOR_SIZE;
@@ -172,19 +247,11 @@ uint32_t flashmem_get_sector_of_address( uint32_t addr )
 
 uint32_t flashmem_write_internal( const void *from, uint32_t toaddr, uint32_t size )
 {
-  SpiFlashOpResult r;
-  const uint32_t blkmask = INTERNAL_FLASH_WRITE_UNIT_SIZE - 1;
-  uint32_t *apbuf = NULL;
-  if( ((uint32_t)from) & blkmask ){
-    apbuf = (uint32_t *)malloc(size);
-    if(!apbuf)
-      return 0;
-    memcpy(apbuf, from, size);
-  }
-  WRITE_PERI_REG(0x60000914, 0x73);
-  r = spi_flash_write(toaddr, apbuf?(uint32 *)apbuf:(uint32 *)from, size);
-  if(apbuf)
-    free(apbuf);
+  assert(IS_ALIGNED(from) && IS_ALIGNED(toaddr) && IS_ALIGNED(size));
+
+  WDT_FEED();
+
+  SpiFlashOpResult r = spi_flash_write(toaddr, (uint32*)from, size);
   if(SPI_FLASH_RESULT_OK == r)
     return size;
   else{
@@ -195,9 +262,11 @@ uint32_t flashmem_write_internal( const void *from, uint32_t toaddr, uint32_t si
 
 uint32_t flashmem_read_internal( void *to, uint32_t fromaddr, uint32_t size )
 {
-  SpiFlashOpResult r;
-  WRITE_PERI_REG(0x60000914, 0x73);
-  r = spi_flash_read(fromaddr, (uint32 *)to, size);
+  assert(IS_ALIGNED(to) && IS_ALIGNED(fromaddr) && IS_ALIGNED(size));
+
+  WDT_FEED();
+
+  SpiFlashOpResult r = spi_flash_read(fromaddr, (uint32*)to, size);
   if(SPI_FLASH_RESULT_OK == r)
     return size;
   else{
@@ -208,9 +277,9 @@ uint32_t flashmem_read_internal( void *to, uint32_t fromaddr, uint32_t size )
 
 uint32_t flashmem_get_first_free_block_address()
 {
-  if (_flash_code_end == NULL)
+  if(_flash_code_end == NULL)
   {
-	  debugf("_flash_code_end:%08x\n", (uint32_t)_flash_code_end);
+	  debugf("_flash_code_end is null");
 	  return 0;
   }