From 1ef7d093e1e89b2b4a0e38705cd76a5cfcfb6252 Mon Sep 17 00:00:00 2001 From: Konstantin Kondrashov Date: Wed, 13 Feb 2019 20:10:53 +0800 Subject: [PATCH 1/2] freemodbus: Fix remove critical_sections Closes: https://github.com/espressif/esp-idf/issues/3009 --- components/freemodbus/modbus/ascii/mbascii.c | 6 ------ components/freemodbus/modbus/rtu/mbrtu.c | 6 ------ 2 files changed, 12 deletions(-) diff --git a/components/freemodbus/modbus/ascii/mbascii.c b/components/freemodbus/modbus/ascii/mbascii.c index c513457eae..55490903ca 100644 --- a/components/freemodbus/modbus/ascii/mbascii.c +++ b/components/freemodbus/modbus/ascii/mbascii.c @@ -110,7 +110,6 @@ eMBASCIIInit( UCHAR ucSlaveAddress, UCHAR ucPort, ULONG ulBaudRate, eMBParity eP eMBErrorCode eStatus = MB_ENOERR; ( void )ucSlaveAddress; - ENTER_CRITICAL_SECTION( ); ucMBLFCharacter = MB_ASCII_DEFAULT_LF; if( xMBPortSerialInit( ucPort, ulBaudRate, 7, eParity ) != TRUE ) @@ -122,7 +121,6 @@ eMBASCIIInit( UCHAR ucSlaveAddress, UCHAR ucPort, ULONG ulBaudRate, eMBParity eP eStatus = MB_EPORTERR; } - EXIT_CRITICAL_SECTION( ); return eStatus; } @@ -130,10 +128,8 @@ eMBASCIIInit( UCHAR ucSlaveAddress, UCHAR ucPort, ULONG ulBaudRate, eMBParity eP void eMBASCIIStart( void ) { - ENTER_CRITICAL_SECTION( ); vMBPortSerialEnable( TRUE, FALSE ); eRcvState = STATE_RX_IDLE; - EXIT_CRITICAL_SECTION( ); /* No special startup required for ASCII. */ ( void )xMBPortEventPost( EV_READY ); @@ -142,10 +138,8 @@ eMBASCIIStart( void ) void eMBASCIIStop( void ) { - ENTER_CRITICAL_SECTION( ); vMBPortSerialEnable( FALSE, FALSE ); vMBPortTimersDisable( ); - EXIT_CRITICAL_SECTION( ); } eMBErrorCode diff --git a/components/freemodbus/modbus/rtu/mbrtu.c b/components/freemodbus/modbus/rtu/mbrtu.c index 28bd296224..b0b84deb03 100644 --- a/components/freemodbus/modbus/rtu/mbrtu.c +++ b/components/freemodbus/modbus/rtu/mbrtu.c @@ -84,7 +84,6 @@ eMBRTUInit( UCHAR ucSlaveAddress, UCHAR ucPort, ULONG ulBaudRate, eMBParity ePar ULONG usTimerT35_50us; ( void )ucSlaveAddress; - ENTER_CRITICAL_SECTION( ); /* Modbus RTU uses 8 Databits. */ if( xMBPortSerialInit( ucPort, ulBaudRate, 8, eParity ) != TRUE ) @@ -117,7 +116,6 @@ eMBRTUInit( UCHAR ucSlaveAddress, UCHAR ucPort, ULONG ulBaudRate, eMBParity ePar eStatus = MB_EPORTERR; } } - EXIT_CRITICAL_SECTION( ); return eStatus; } @@ -125,7 +123,6 @@ eMBRTUInit( UCHAR ucSlaveAddress, UCHAR ucPort, ULONG ulBaudRate, eMBParity ePar void eMBRTUStart( void ) { - ENTER_CRITICAL_SECTION( ); /* Initially the receiver is in the state STATE_RX_INIT. we start * the timer and if no character is received within t3.5 we change * to STATE_RX_IDLE. This makes sure that we delay startup of the @@ -135,16 +132,13 @@ eMBRTUStart( void ) vMBPortSerialEnable( TRUE, FALSE ); vMBPortTimersEnable( ); - EXIT_CRITICAL_SECTION( ); } void eMBRTUStop( void ) { - ENTER_CRITICAL_SECTION( ); vMBPortSerialEnable( FALSE, FALSE ); vMBPortTimersDisable( ); - EXIT_CRITICAL_SECTION( ); } // The lines below are required to suppress GCC warnings about unused but set variable 'xFrameReceived' From aaa1cb6eec636d2f634e91e6512a9c0a733d5162 Mon Sep 17 00:00:00 2001 From: aleks Date: Thu, 7 Mar 2019 09:51:25 +0100 Subject: [PATCH 2/2] freemodbus: change critical sections to semaphore mutex revert changes made in mbrtu.c, mbascii.c change critical section type to semaphore mutex instead of spin lock Closes: https://github.com/espressif/esp-idf/issues/3009 --- components/freemodbus/modbus/ascii/mbascii.c | 6 ++++++ components/freemodbus/modbus/rtu/mbrtu.c | 6 ++++++ components/freemodbus/port/port.h | 7 +++++-- components/freemodbus/port/portother.c | 12 ++++++------ components/freemodbus/port/portserial.c | 2 -- 5 files changed, 23 insertions(+), 10 deletions(-) diff --git a/components/freemodbus/modbus/ascii/mbascii.c b/components/freemodbus/modbus/ascii/mbascii.c index 55490903ca..c513457eae 100644 --- a/components/freemodbus/modbus/ascii/mbascii.c +++ b/components/freemodbus/modbus/ascii/mbascii.c @@ -110,6 +110,7 @@ eMBASCIIInit( UCHAR ucSlaveAddress, UCHAR ucPort, ULONG ulBaudRate, eMBParity eP eMBErrorCode eStatus = MB_ENOERR; ( void )ucSlaveAddress; + ENTER_CRITICAL_SECTION( ); ucMBLFCharacter = MB_ASCII_DEFAULT_LF; if( xMBPortSerialInit( ucPort, ulBaudRate, 7, eParity ) != TRUE ) @@ -121,6 +122,7 @@ eMBASCIIInit( UCHAR ucSlaveAddress, UCHAR ucPort, ULONG ulBaudRate, eMBParity eP eStatus = MB_EPORTERR; } + EXIT_CRITICAL_SECTION( ); return eStatus; } @@ -128,8 +130,10 @@ eMBASCIIInit( UCHAR ucSlaveAddress, UCHAR ucPort, ULONG ulBaudRate, eMBParity eP void eMBASCIIStart( void ) { + ENTER_CRITICAL_SECTION( ); vMBPortSerialEnable( TRUE, FALSE ); eRcvState = STATE_RX_IDLE; + EXIT_CRITICAL_SECTION( ); /* No special startup required for ASCII. */ ( void )xMBPortEventPost( EV_READY ); @@ -138,8 +142,10 @@ eMBASCIIStart( void ) void eMBASCIIStop( void ) { + ENTER_CRITICAL_SECTION( ); vMBPortSerialEnable( FALSE, FALSE ); vMBPortTimersDisable( ); + EXIT_CRITICAL_SECTION( ); } eMBErrorCode diff --git a/components/freemodbus/modbus/rtu/mbrtu.c b/components/freemodbus/modbus/rtu/mbrtu.c index b0b84deb03..28bd296224 100644 --- a/components/freemodbus/modbus/rtu/mbrtu.c +++ b/components/freemodbus/modbus/rtu/mbrtu.c @@ -84,6 +84,7 @@ eMBRTUInit( UCHAR ucSlaveAddress, UCHAR ucPort, ULONG ulBaudRate, eMBParity ePar ULONG usTimerT35_50us; ( void )ucSlaveAddress; + ENTER_CRITICAL_SECTION( ); /* Modbus RTU uses 8 Databits. */ if( xMBPortSerialInit( ucPort, ulBaudRate, 8, eParity ) != TRUE ) @@ -116,6 +117,7 @@ eMBRTUInit( UCHAR ucSlaveAddress, UCHAR ucPort, ULONG ulBaudRate, eMBParity ePar eStatus = MB_EPORTERR; } } + EXIT_CRITICAL_SECTION( ); return eStatus; } @@ -123,6 +125,7 @@ eMBRTUInit( UCHAR ucSlaveAddress, UCHAR ucPort, ULONG ulBaudRate, eMBParity ePar void eMBRTUStart( void ) { + ENTER_CRITICAL_SECTION( ); /* Initially the receiver is in the state STATE_RX_INIT. we start * the timer and if no character is received within t3.5 we change * to STATE_RX_IDLE. This makes sure that we delay startup of the @@ -132,13 +135,16 @@ eMBRTUStart( void ) vMBPortSerialEnable( TRUE, FALSE ); vMBPortTimersEnable( ); + EXIT_CRITICAL_SECTION( ); } void eMBRTUStop( void ) { + ENTER_CRITICAL_SECTION( ); vMBPortSerialEnable( FALSE, FALSE ); vMBPortTimersDisable( ); + EXIT_CRITICAL_SECTION( ); } // The lines below are required to suppress GCC warnings about unused but set variable 'xFrameReceived' diff --git a/components/freemodbus/port/port.h b/components/freemodbus/port/port.h index c7965a1f23..0d8173a654 100644 --- a/components/freemodbus/port/port.h +++ b/components/freemodbus/port/port.h @@ -60,8 +60,11 @@ #define MB_ENTER_CRITICAL(mux) portENTER_CRITICAL(mux) #define MB_EXIT_CRITICAL(mux) portEXIT_CRITICAL(mux) -#define ENTER_CRITICAL_SECTION( ) ( vMBPortEnterCritical() ) -#define EXIT_CRITICAL_SECTION( ) ( vMBPortExitCritical() ) +#define ENTER_CRITICAL_SECTION( ) { ESP_LOGD(MB_PORT_TAG,"%s: Port enter critical.", __func__); \ + vMBPortEnterCritical(); } + +#define EXIT_CRITICAL_SECTION( ) { vMBPortExitCritical(); \ + ESP_LOGD(MB_PORT_TAG,"%s: Port exit critical", __func__); } typedef char BOOL; diff --git a/components/freemodbus/port/portother.c b/components/freemodbus/port/portother.c index e4b90e64c0..a789545598 100644 --- a/components/freemodbus/port/portother.c +++ b/components/freemodbus/port/portother.c @@ -32,16 +32,16 @@ #include #include #include -#include /* ----------------------- Modbus includes ----------------------------------*/ #include "mb.h" #include "mbport.h" +#include "sys/lock.h" /* ----------------------- Modbus includes ----------------------------------*/ /* ----------------------- Variables ----------------------------------------*/ -static portMUX_TYPE mb_mutex = portMUX_INITIALIZER_UNLOCKED; +static _lock_t s_port_lock; /* ----------------------- Start implementation -----------------------------*/ @@ -52,16 +52,16 @@ bMBPortIsWithinException( void ) return bIsWithinException; } -void +inline void vMBPortEnterCritical( void ) { - portENTER_CRITICAL(&mb_mutex); + _lock_acquire(&s_port_lock); } -void +inline void vMBPortExitCritical( void ) { - portEXIT_CRITICAL(&mb_mutex); + _lock_release(&s_port_lock); } void diff --git a/components/freemodbus/port/portserial.c b/components/freemodbus/port/portserial.c index 3543135ee3..dee5fdd365 100644 --- a/components/freemodbus/port/portserial.c +++ b/components/freemodbus/port/portserial.c @@ -74,7 +74,6 @@ static USHORT uiRxBufferPos = 0; // position in the receiver buffer void vMBPortSerialEnable(BOOL bRxEnable, BOOL bTxEnable) { // This function can be called from xMBRTUTransmitFSM() of different task - ENTER_CRITICAL_SECTION(); if (bRxEnable) { //uart_enable_rx_intr(ucUartNumber); bRxStateEnabled = TRUE; @@ -88,7 +87,6 @@ void vMBPortSerialEnable(BOOL bRxEnable, BOOL bTxEnable) } else { bTxStateEnabled = FALSE; } - EXIT_CRITICAL_SECTION(); } static void vMBPortSerialRxPoll(size_t xEventSize)