Trunk Build 670

Author: quizic

Diff for: core/driver/blockio.c

@@ -28,6 +28,12 @@
     The OS block device implementations operate on sectors.  The core does I/O
     in terms of logical blocks: this module translates from logical blocks to
     sectors.
+
+    If bBlockIoRetries is greater than 0 for the current volume, then this
+    module will retry block device calls on failure up to the configured number
+    of times.  This behavior caters to the type of unreliable hardware and
+    drivers that are sometimes found in the IoT world, where one operation may
+    fail but the next may still succeed.
 */
 #include <redfs.h>
 #include <redcore.h>
@@ -52,7 +58,7 @@ REDSTATUS RedIoRead(
     uint32_t    ulBlockCount,
     void       *pBuffer)
 {
-    REDSTATUS   ret;
+    REDSTATUS   ret = 0;
 
     if(    (bVolNum >= REDCONF_VOLUME_COUNT)
         || (ulBlockStart >= gaRedVolume[bVolNum].ulBlockCount)
@@ -68,11 +74,20 @@ REDSTATUS RedIoRead(
         uint8_t  bSectorShift = gaRedVolume[bVolNum].bBlockSectorShift;
         uint64_t ullSectorStart = (uint64_t)ulBlockStart << bSectorShift;
         uint32_t ulSectorCount = ulBlockCount << bSectorShift;
+        uint8_t  bRetryIdx;
 
         REDASSERT(bSectorShift < 32U);
         REDASSERT((ulSectorCount >> bSectorShift) == ulBlockCount);
 
-        ret = RedOsBDevRead(bVolNum, ullSectorStart, ulSectorCount, pBuffer);
+        for(bRetryIdx = 0U; bRetryIdx <= gpRedVolConf->bBlockIoRetries; bRetryIdx++)
+        {
+            ret = RedOsBDevRead(bVolNum, ullSectorStart, ulSectorCount, pBuffer);
+
+            if(ret == 0)
+            {
+                break;
+            }
+        }
     }
 
     CRITICAL_ASSERT(ret == 0);
@@ -101,7 +116,7 @@ REDSTATUS RedIoWrite(
     uint32_t    ulBlockCount,
     const void *pBuffer)
 {
-    REDSTATUS   ret;
+    REDSTATUS   ret = 0;
 
     if(    (bVolNum >= REDCONF_VOLUME_COUNT)
         || (ulBlockStart >= gaRedVolume[bVolNum].ulBlockCount)
@@ -117,11 +132,20 @@ REDSTATUS RedIoWrite(
         uint8_t  bSectorShift = gaRedVolume[bVolNum].bBlockSectorShift;
         uint64_t ullSectorStart = (uint64_t)ulBlockStart << bSectorShift;
         uint32_t ulSectorCount = ulBlockCount << bSectorShift;
+        uint8_t  bRetryIdx;
 
         REDASSERT(bSectorShift < 32U);
         REDASSERT((ulSectorCount >> bSectorShift) == ulBlockCount);
 
-        ret = RedOsBDevWrite(bVolNum, ullSectorStart, ulSectorCount, pBuffer);
+        for(bRetryIdx = 0U; bRetryIdx <= gpRedVolConf->bBlockIoRetries; bRetryIdx++)
+        {
+            ret = RedOsBDevWrite(bVolNum, ullSectorStart, ulSectorCount, pBuffer);
+
+            if(ret == 0)
+            {
+                break;
+            }
+        }
     }
 
     CRITICAL_ASSERT(ret == 0);
@@ -144,7 +168,7 @@ REDSTATUS RedIoWrite(
 REDSTATUS RedIoFlush(
     uint8_t     bVolNum)
 {
-    REDSTATUS   ret;
+    REDSTATUS   ret = 0;
 
     if(bVolNum >= REDCONF_VOLUME_COUNT)
     {
@@ -153,7 +177,17 @@ REDSTATUS RedIoFlush(
     }
     else
     {
-        ret = RedOsBDevFlush(bVolNum);
+        uint8_t  bRetryIdx;
+
+        for(bRetryIdx = 0U; bRetryIdx <= gpRedVolConf->bBlockIoRetries; bRetryIdx++)
+        {
+            ret = RedOsBDevFlush(bVolNum);
+
+            if(ret == 0)
+            {
+                break;
+            }
+        }
     }
 
     CRITICAL_ASSERT(ret == 0);

Diff for: core/driver/buffer.c

@@ -311,7 +311,7 @@ REDSTATUS RedBufferGet(
                                 mounted; that condition is expected and should
                                 not result in an assertion.
                             */
-                            CRITICAL_ASSERT((uFlags & BFLAG_META_MASTER) != 0U);
+                            CRITICAL_ASSERT((uFlags & BFLAG_META_MASTER) == BFLAG_META_MASTER);
                             ret = -RED_EIO;
                         }
                     }

Diff for: doc/release_notes.md

@@ -5,6 +5,30 @@ recent releases and a list of known issues.
 
 ## Release History and Changes
 
+### Reliance Edge v1.0.2, February 2016
+
+#### Common Code Changes
+- A new per-volume configuration option has been added: users can specify a
+  number of times to retry a block device read, write or flush operation before
+  returning a failure.  The configuration tool has been updated to version 1.0.2
+  with this change.
+  - This added a new field to the volume configuration in to redconf.c: existing
+    redconf.c files from v1.0.1 and earlier must be updated to work with v1.0.2.
+    Open redconf.h and redconf.c with the configuration tool, enable
+    "Retry block device I/O on failure" for any volumes if desired, and save the
+    redconf files.
+
+#### FreeRTOS Port Changes
+- Added support for the STM32 HAL SD card driver in the FreeRTOS block device
+  interface.  Two boards are supported out-of-the-box: the STM324xG-EVAL and the
+  STM32F746NG-Discovery.  A sample project is included for the STM324xG-EVAL.
+
+#### MQX Port Changes
+- Fixed a bug which prevented Reliance Edge from compiling if the File System
+  Essentials API was selected in the configuration.
+- Fixed a bug which would have returned an uninitialized value from
+  `RedOsBDevFlush()` for block devices that support flushing.
+  
 ### Reliance Edge v1.0.1, October 2015
 
 - Added MQX RTOS support in the commercial kit, with example projects for

Diff for: doc/release_notes.txt

@@ -9,6 +9,34 @@ course of recent releases and a list of known issues.
 
 Release History and Changes
 
+Reliance Edge v1.0.2, February 2016
+
+Common Code Changes
+
+-   A new per-volume configuration option has been added: users can
+    specify a number of times to retry a block device read, write or
+    flush operation before returning a failure. The configuration tool
+    has been updated to version 1.0.2 with this change.
+-   This added a new field to the volume configuration in to redconf.c:
+    existing redconf.c files from v1.0.1 and earlier must be updated to
+    work with v1.0.2. Open redconf.h and redconf.c with the
+    configuration tool, enable "Retry block device I/O on failure" for
+    any volumes if desired, and save the redconf files.
+
+FreeRTOS Port Changes
+
+-   Added support for the STM32 HAL SD card driver in the FreeRTOS block
+    device interface. Two boards are supported out-of-the-box: the
+    STM324xG-EVAL and the STM32F746NG-Discovery. A sample project is
+    included for the STM324xG-EVAL.
+
+MQX Port Changes
+
+-   Fixed a bug which prevented Reliance Edge from compiling if the File
+    System Essentials API was selected in the configuration.
+-   Fixed a bug which would have returned an uninitialized value from
+    RedOsBDevFlush() for block devices that support flushing.
+
 Reliance Edge v1.0.1, October 2015
 
 -   Added MQX RTOS support in the commercial kit, with example projects

Diff for: include/redver.h

@@ -33,7 +33,7 @@
 
     <!-- This macro is updated automatically: do not edit! -->
 */
-#define RED_BUILD_NUMBER "668"
+#define RED_BUILD_NUMBER "670"
 
 #define RED_KIT_GPL         0U  /* Open source GPL kit. */
 #define RED_KIT_COMMERCIAL  1U  /* Commercially-licensed kit. */
@@ -48,7 +48,7 @@
 
 /** @brief Version number to display in output.
 */
-#define RED_VERSION "v1.0.1"
+#define RED_VERSION "v1.0.2"
 
 
 /** @brief On-disk version number.

Diff for: include/redvolume.h

@@ -62,6 +62,13 @@ typedef struct
     */
     uint32_t    ulInodeCount;
 
+    /** This is the maximum number of times a block device I/O operation will
+        be retried.  If a block device read, write, or flush fails, Reliance
+        Edge will try again up to this number of times until the operation is
+        successful.  Set this to 0 to disable retries.
+    */
+    uint8_t     bBlockIoRetries;
+
   #if REDCONF_API_POSIX == 1
     /** The path prefix for the volume; for example, "VOL1:", "FlashDisk", etc.
     */

Diff for: os/freertos/include/redosdeviations.h

@@ -36,8 +36,7 @@
 #endif
 
 
-#if REDCONF_ASSERTS == 1
-#if REDCONF_OUTPUT == 1
+#if (REDCONF_ASSERTS == 1) && (REDCONF_OUTPUT == 1)
 /** Print a formatted message for an assertion.
 
     Usages of this macro deviate from MISRA C:2012 Rule 21.6 (required).  Using
@@ -50,11 +49,8 @@
     As Rule 21.6 is required, a separate deviation record is required.
 */
 #define PRINT_ASSERT(file, line) \
-    (void)printf("Assertion failed in \"%s\" at line %u\n\r", ((file) == NULL) ? "" : (file), (unsigned)(line))
-#else
-#define PRINT_ASSERT(file, line) do { (void)(file); (void)(line); } while(false)
-#endif /* REDCONF_OUTPUT == 1 */
-#endif /* REDCONF_ASSERTS == 1 */
+    printf("Assertion failed in \"%s\" at line %u\n\r", ((file) == NULL) ? "" : (file), (unsigned)(line))
+#endif
 
 
 /** Cast a value to unsigned long.
@@ -161,5 +157,88 @@
 #endif
 
 
+/** Ignore the return value of a function (cast to void)
+
+    Usages of this macro deviate from MISRA C:2012 Directive 4.7, which states
+    that error information must be checked immediately after a function returns
+    potential error information.
+
+    If asserts and output are enabled, then this macro is used to document that
+    the return value of printf() is ignored.  A failure of printf() does not
+    impact the filesystem core, nor is there anything the filesystem can do to
+    respond to such an error (especially since it occurs within an assert).
+    Thus, the most reasonable action is to ignore the error.
+
+    In the STM32 SDIO block device implementation, errors are also ignored in an
+    IRQ interrupt handler.  This is the most reasonable action to take for two
+    reasons: (a) it would be dangerous to spend processor time responding to the
+    error inside the IRQ handler; (b) it has been verified that the same error
+    is propegated to the DiskRead/Write method, which does return the error to
+    the core.
+
+    In the Atmel SD/MMC block device implementation, error information from
+    sd_mmc_read_capacity() is ignored.  This is a reasonable action because all
+    of the possible error conditions were eliminated by a previous check.
+    sd_mmc_read_capacity() fails under the same conditions as
+    sd_mmc_test_unit_ready(), which was checked ealier in the same function.
+
+    In the mutex module, error information returned from the mutex release
+    function is ignored when asserts are disabled.  This is a reasonable action
+    because the mutex release function (xSemaphoreGive) is documented only to
+    fail if the mutex was not obtained correctly, which can be demonstrably
+    avoided.
+
+    As Directive 4.7 is required, a separate deviation record is required.
+*/
+#define IGNORE_ERRORS(fn) ((void) (fn))
+
+
+/** @brief Determine whether a pointer is aligned on a 32-bit boundary.
+
+    This is used to determine whether a data buffer meets the requirements of
+    the underlying block device implementation.  When transferring data via
+    DMA (Direct Memory Access) on an STM32 device, the data buffer must be cast
+    as a uint32 pointer, and unexpected behavior may occur if the buffer is not
+    aligned correctly.
+
+    There is no way to perform this check without deviating from MISRA C rules
+    against casting pointers to integer types.  Usage of this macro deviates
+    from MISRA C:2012 Rule 11.4 (advisory).  The main rationale the rule cites
+    against converting pointers to integers is that the chosen integer type may
+    not be able to represent the pointer; this is a non-issue here since we use
+    uintptr_t.  The text says the rule still applies when using uintptr_t due to
+    concern about unaligned pointers, but that is not an issue here since the
+    integer value of the pointer is not saved and not converted back into a
+    pointer and dereferenced.  The result of casting a pointer to a sufficiently
+    large integer is implementation-defined, but macros similar to this one have
+    been used by Datalight for a long time in a wide variety of environments and
+    they have always worked as expected.
+
+    This deviation only occurs when using the STM32 SDIO block device
+    implementation.
+
+    As Rule 11.4 is advisory, a deviation record is not required.  This notice
+    is the only record of deviation.
+*/
+#define IS_UINT32_ALIGNED_PTR(ptr) (((uintptr_t)(ptr) & (sizeof(uint32_t) - 1U)) == 0U)
+
+
+/** @brief Cast a 32-bit aligned void pointer to a uint32 pointer.
+
+    Usages of this macro deviate from MISRA C:2012 Rule 11.5 (advisory).  A
+    cast from a void pointer to an object pointer is discouraged because of
+    potential alignment issues.  However, this macro is only used to cast
+    pointers that have already been tested to be 32-bit aligned, so the
+    operation will be safe.
+
+    This deviation only occurs when using the STM32 SDIO block device
+    implementation.
+
+    As rule 11.5 is advisory, a deviation record is not required.  This notice
+    is the only record of the deviation.
+*/
+#define CAST_UINT32_PTR(ptr) ((uint32_t *) (ptr))
+
+
 #endif
 

Diff for: os/freertos/services/osassert.c

@@ -43,7 +43,9 @@ void RedOsAssertFail(
     const char *pszFileName,
     uint32_t    ulLineNum)
 {
-    PRINT_ASSERT(pszFileName, ulLineNum);
+  #if REDCONF_OUTPUT == 1
+    IGNORE_ERRORS(PRINT_ASSERT(pszFileName, ulLineNum));
+  #endif
 
     while(true)
     {