bsp: Synchronize serial driver access (#29)

The serial driver is used by multiple tasks and not all tasks use
the Log APIs to send data to the serial driver.
This was causing serial data buffer corruption.

Synchronise the access of the serial driver using mutual exclusion
to make it thread-safe.

This also removes code duplication and fixes return values for
the write system call:
* Arm GNU Toolchain: On failure, `-1`.
                     On success, total chars written.
* Arm Compiler: On failure, a positive number representing the number of
                chars NOT written or a negative number indicating an error.
                On success, `0`.

Signed-off-by: Devaraj Ranganna <[email protected]>
Signed-off-by: Hugues Kamba-Mpiana <[email protected]>
Co-authored-by: Devaraj Ranganna <[email protected]>

bsp/CMakeLists.txt

@@ -65,4 +65,5 @@ target_include_directories(fri-bsp
 target_link_libraries(fri-bsp
     PUBLIC
         arm-corstone-platform-bsp
+        freertos_kernel
 )

bsp/common/bsp_serial.c

@@ -9,12 +9,47 @@
 #include "Driver_USART.h"
 #include "bsp_serial.h"
 
+#include "FreeRTOS.h"
+#include "semphr.h"
+
+#define STDIN_FILENO     0
+#define STDOUT_FILENO    1
+#define STDERR_FILENO    2
+
+typedef enum
+{
+    WRITE_ERROR_SEND_FAIL = -3,
+    WRITE_ERROR_SYNC_FAILED = -2,
+    WRITE_ERROR_INVALID_ARGS = -1,
+    WRITE_ERROR_NONE = 0
+} WriteError_t;
+
+typedef struct
+{
+    WriteError_t error;
+    unsigned int charsWritten;
+} WriteResult_t;
+
 extern ARM_DRIVER_USART Driver_USART0;
 
+static SemaphoreHandle_t xLoggingMutex = NULL;
+
+static bool prvValidFdHandle( int fd );
+static void prvWriteChars( int fd,
+                           const unsigned char * str,
+                           unsigned int len,
+                           WriteResult_t * result );
+
 void bsp_serial_init( void )
 {
     Driver_USART0.Initialize( NULL );
     Driver_USART0.Control( ARM_USART_MODE_ASYNCHRONOUS, DEFAULT_UART_BAUDRATE );
+
+    if( xLoggingMutex == NULL )
+    {
+        xLoggingMutex = xSemaphoreCreateMutex();
+        configASSERT( xLoggingMutex );
+    }
 }
 
 void bsp_serial_print( char * str )
@@ -28,10 +63,6 @@ void bsp_serial_print( char * str )
 
     #include <rt_sys.h>
 
-    #define STDIN_FILENO     0
-    #define STDOUT_FILENO    1
-    #define STDERR_FILENO    2
-
     FILEHANDLE _sys_open( const char * name,
                           int openmode )
     {
@@ -73,20 +104,24 @@ void bsp_serial_print( char * str )
                     unsigned int len,
                     int mode )
     {
-        /* From <rt_sys.h>: `mode' exists for historical reasons and must be ignored. */
+        /* From <rt_sys.h>: `mode` exists for historical reasons and must be ignored. */
         ( void ) mode;
 
-        if( ( fd != STDOUT_FILENO ) && ( fd != STDERR_FILENO ) )
+        WriteResult_t result = { .error = WRITE_ERROR_NONE, .charsWritten = 0 };
+        prvWriteChars( ( int ) fd, str, len, &result );
+
+        if( ( result.error == WRITE_ERROR_NONE ) && ( result.charsWritten == len ) )
         {
-            return -1;
+            return 0;
         }
-
-        if( Driver_USART0.Send( str, len ) != ARM_DRIVER_OK )
+        else if( result.error == WRITE_ERROR_SEND_FAIL )
         {
-            return -1;
+            return len - result.charsWritten;
+        }
+        else
+        {
+            return ( int ) result.error;
         }
-
-        return 0;
     }
 
     int _sys_read( FILEHANDLE fd,
@@ -133,12 +168,50 @@ void bsp_serial_print( char * str )
                 char * str,
                 int len )
     {
-        if( Driver_USART0.Send( str, len ) == ARM_DRIVER_OK )
-        {
-            return len;
-        }
+        WriteResult_t result = { .error = WRITE_ERROR_NONE, .charsWritten = 0 };
 
-        return 0;
+        prvWriteChars( fd, str, len, &result );
+
+        return ( ( result.error == WRITE_ERROR_NONE ) && ( result.charsWritten == len ) ) ? result.charsWritten : -1;
     }
 
 #endif /* if defined( __ARMCOMPILER_VERSION ) */
+
+static bool prvValidFdHandle( int fd )
+{
+    return ( bool ) ( ( fd == STDOUT_FILENO ) || ( fd == STDERR_FILENO ) );
+}
+
+static void prvWriteChars( int fd,
+                           const unsigned char * str,
+                           unsigned int len,
+                           WriteResult_t * result )
+{
+    result->charsWritten = 0;
+
+    if( prvValidFdHandle( fd ) == false )
+    {
+        result->error = WRITE_ERROR_INVALID_ARGS;
+        return;
+    }
+
+    if( xSemaphoreTake( xLoggingMutex, portMAX_DELAY ) != pdTRUE )
+    {
+        result->error = WRITE_ERROR_SYNC_FAILED;
+        return;
+    }
+
+    bool allCharsWritten = ( bool ) ( Driver_USART0.Send( str, len ) == ARM_DRIVER_OK );
+
+    ( void ) xSemaphoreGive( xLoggingMutex );
+
+    if( allCharsWritten == true )
+    {
+        result->charsWritten = len;
+        result->error = WRITE_ERROR_NONE;
+    }
+    else
+    {
+        result->error = WRITE_ERROR_SEND_FAIL;
+    }
+}

release_changes/202401041514.change

@@ -0,0 +1 @@
+serial: Fix multithread synchronisation