From 07b0b3d8998a48e9bf9727640899a3a05f92de49 Mon Sep 17 00:00:00 2001 From: Sebastian Stockhammer Date: Mon, 17 Jun 2019 10:51:52 +0200 Subject: [PATCH 1/4] Free and reinitialize serial resources based on input and output enable state --- drivers/UARTSerial.cpp | 23 +++++++++++++++++++++++ drivers/UARTSerial.h | 2 ++ 2 files changed, 25 insertions(+) diff --git a/drivers/UARTSerial.cpp b/drivers/UARTSerial.cpp index e9213555d76..7742dd26a29 100644 --- a/drivers/UARTSerial.cpp +++ b/drivers/UARTSerial.cpp @@ -374,12 +374,18 @@ int UARTSerial::enable_input(bool enabled) core_util_critical_section_enter(); if (_rx_enabled != enabled) { if (enabled) { + if (!_tx_enabled) { + init(); + } UARTSerial::rx_irq(); if (!_rxbuf.full()) { enable_rx_irq(); } } else { disable_rx_irq(); + if (!_tx_enabled) { + serial_free(&_serial); + } } _rx_enabled = enabled; } @@ -393,12 +399,18 @@ int UARTSerial::enable_output(bool enabled) core_util_critical_section_enter(); if (_tx_enabled != enabled) { if (enabled) { + if (!_rx_enabled) { + init(); + } UARTSerial::tx_irq(); if (!_txbuf.empty()) { enable_tx_irq(); } } else { disable_tx_irq(); + if (!_rx_enabled) { + serial_free(&_serial); + } } _tx_enabled = enabled; } @@ -407,6 +419,17 @@ int UARTSerial::enable_output(bool enabled) return 0; } +void UARTSerial::init() { + for (size_t i = 0; i < sizeof _irq / sizeof _irq[0]; i++) { + _irq[i] = NULL; + } + + serial_init(&_serial, _serial.pin_tx, _serial.pin_rx); + serial_baud(&_serial, _baud); + serial_irq_handler(&_serial, SerialBase::_irq_handler, (uint32_t)this); + enable_rx_irq(); +} + void UARTSerial::wait_ms(uint32_t millisec) { /* wait_ms implementation for RTOS spins until exact microseconds - we diff --git a/drivers/UARTSerial.h b/drivers/UARTSerial.h index 2959ce77e2c..6deb55fc141 100644 --- a/drivers/UARTSerial.h +++ b/drivers/UARTSerial.h @@ -254,6 +254,8 @@ class UARTSerial : private SerialBase, public FileHandle, private NonCopyable Date: Mon, 17 Jun 2019 14:53:24 +0200 Subject: [PATCH 2/4] Reduce critical sections --- drivers/SerialBase.cpp | 34 ++++++++++++++++++-------- drivers/SerialBase.h | 10 ++++++++ drivers/UARTSerial.cpp | 55 ++++++++++++++++++++---------------------- drivers/UARTSerial.h | 2 -- 4 files changed, 60 insertions(+), 41 deletions(-) diff --git a/drivers/SerialBase.cpp b/drivers/SerialBase.cpp index 856944fd347..abf9765d0c8 100644 --- a/drivers/SerialBase.cpp +++ b/drivers/SerialBase.cpp @@ -30,17 +30,9 @@ SerialBase::SerialBase(PinName tx, PinName rx, int baud) : _rx_callback(NULL), _tx_asynch_set(false), _rx_asynch_set(false), #endif - _serial(), _baud(baud) + _serial(), _baud(baud), _tx(tx), _rx(rx) { - // No lock needed in the constructor - - for (size_t i = 0; i < sizeof _irq / sizeof _irq[0]; i++) { - _irq[i] = NULL; - } - - serial_init(&_serial, tx, rx); - serial_baud(&_serial, _baud); - serial_irq_handler(&_serial, SerialBase::_irq_handler, (uint32_t)this); + init(); } void SerialBase::baud(int baudrate) @@ -161,6 +153,26 @@ void SerialBase:: unlock() // Stub } +void SerialBase::init(void) +{ + lock(); + for (size_t i = 0; i < sizeof _irq / sizeof _irq[0]; i++) { + _irq[i] = NULL; + } + + serial_init(&_serial, _tx, _rx); + serial_baud(&_serial, _baud); + serial_irq_handler(&_serial, SerialBase::_irq_handler, (uint32_t)this); + unlock(); +} + +void SerialBase::deinit(void) +{ + lock(); + serial_free(&_serial); + unlock(); +} + SerialBase::~SerialBase() { // No lock needed in destructor @@ -169,6 +181,8 @@ SerialBase::~SerialBase() for (int irq = 0; irq < IrqCnt; irq++) { attach(NULL, (IrqType)irq); } + + deinit(); } #if DEVICE_SERIAL_FC diff --git a/drivers/SerialBase.h b/drivers/SerialBase.h index 99da7a9b45d..095c9f38887 100644 --- a/drivers/SerialBase.h +++ b/drivers/SerialBase.h @@ -162,6 +162,14 @@ class SerialBase : private NonCopyable { /** Release exclusive access to this serial port */ virtual void unlock(void); + + /** Initialize serial port + */ + void init(void); + + /** Deinitialize serial port + */ + void deinit(void); #endif public: @@ -304,6 +312,8 @@ class SerialBase : private NonCopyable { serial_t _serial; Callback _irq[IrqCnt]; int _baud; + PinName _rx; + PinName _tx; #endif }; diff --git a/drivers/UARTSerial.cpp b/drivers/UARTSerial.cpp index 7742dd26a29..dcd345e1d64 100644 --- a/drivers/UARTSerial.cpp +++ b/drivers/UARTSerial.cpp @@ -371,63 +371,60 @@ void UARTSerial::disable_tx_irq() int UARTSerial::enable_input(bool enabled) { - core_util_critical_section_enter(); + api_lock(); if (_rx_enabled != enabled) { + if (enabled && !_tx_enabled) { + init(); + } + + core_util_critical_section_enter(); if (enabled) { - if (!_tx_enabled) { - init(); - } UARTSerial::rx_irq(); if (!_rxbuf.full()) { enable_rx_irq(); } } else { disable_rx_irq(); - if (!_tx_enabled) { - serial_free(&_serial); - } } + core_util_critical_section_exit(); + _rx_enabled = enabled; - } - core_util_critical_section_exit(); + if (!enabled && !_tx_enabled) { + deinit(); + } + } + api_unlock(); return 0; } int UARTSerial::enable_output(bool enabled) { - core_util_critical_section_enter(); + api_lock(); if (_tx_enabled != enabled) { + if (enabled && !_rx_enabled) { + init(); + } + + core_util_critical_section_enter(); if (enabled) { - if (!_rx_enabled) { - init(); - } UARTSerial::tx_irq(); if (!_txbuf.empty()) { enable_tx_irq(); } } else { disable_tx_irq(); - if (!_rx_enabled) { - serial_free(&_serial); - } } - _tx_enabled = enabled; - } - core_util_critical_section_exit(); + core_util_critical_section_exit(); - return 0; -} + _tx_enabled = enabled; -void UARTSerial::init() { - for (size_t i = 0; i < sizeof _irq / sizeof _irq[0]; i++) { - _irq[i] = NULL; + if (!enabled && !_rx_enabled) { + deinit(); + } } - - serial_init(&_serial, _serial.pin_tx, _serial.pin_rx); - serial_baud(&_serial, _baud); - serial_irq_handler(&_serial, SerialBase::_irq_handler, (uint32_t)this); - enable_rx_irq(); + api_unlock(); + return 0; } void UARTSerial::wait_ms(uint32_t millisec) diff --git a/drivers/UARTSerial.h b/drivers/UARTSerial.h index 6deb55fc141..2959ce77e2c 100644 --- a/drivers/UARTSerial.h +++ b/drivers/UARTSerial.h @@ -254,8 +254,6 @@ class UARTSerial : private SerialBase, public FileHandle, private NonCopyable Date: Tue, 18 Jun 2019 14:55:28 +0200 Subject: [PATCH 3/4] Prefix init/deinit with underscore --- drivers/SerialBase.cpp | 8 ++++---- drivers/SerialBase.h | 4 ++-- drivers/UARTSerial.cpp | 8 ++++---- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/drivers/SerialBase.cpp b/drivers/SerialBase.cpp index abf9765d0c8..91d37395ce0 100644 --- a/drivers/SerialBase.cpp +++ b/drivers/SerialBase.cpp @@ -32,7 +32,7 @@ SerialBase::SerialBase(PinName tx, PinName rx, int baud) : #endif _serial(), _baud(baud), _tx(tx), _rx(rx) { - init(); + _init(); } void SerialBase::baud(int baudrate) @@ -153,7 +153,7 @@ void SerialBase:: unlock() // Stub } -void SerialBase::init(void) +void SerialBase::_init(void) { lock(); for (size_t i = 0; i < sizeof _irq / sizeof _irq[0]; i++) { @@ -166,7 +166,7 @@ void SerialBase::init(void) unlock(); } -void SerialBase::deinit(void) +void SerialBase::_deinit(void) { lock(); serial_free(&_serial); @@ -182,7 +182,7 @@ SerialBase::~SerialBase() attach(NULL, (IrqType)irq); } - deinit(); + _deinit(); } #if DEVICE_SERIAL_FC diff --git a/drivers/SerialBase.h b/drivers/SerialBase.h index 095c9f38887..99b9e59f416 100644 --- a/drivers/SerialBase.h +++ b/drivers/SerialBase.h @@ -165,11 +165,11 @@ class SerialBase : private NonCopyable { /** Initialize serial port */ - void init(void); + void _init(void); /** Deinitialize serial port */ - void deinit(void); + void _deinit(void); #endif public: diff --git a/drivers/UARTSerial.cpp b/drivers/UARTSerial.cpp index dcd345e1d64..9d534f9cd06 100644 --- a/drivers/UARTSerial.cpp +++ b/drivers/UARTSerial.cpp @@ -374,7 +374,7 @@ int UARTSerial::enable_input(bool enabled) api_lock(); if (_rx_enabled != enabled) { if (enabled && !_tx_enabled) { - init(); + _init(); } core_util_critical_section_enter(); @@ -391,7 +391,7 @@ int UARTSerial::enable_input(bool enabled) _rx_enabled = enabled; if (!enabled && !_tx_enabled) { - deinit(); + _deinit(); } } api_unlock(); @@ -403,7 +403,7 @@ int UARTSerial::enable_output(bool enabled) api_lock(); if (_tx_enabled != enabled) { if (enabled && !_rx_enabled) { - init(); + _init(); } core_util_critical_section_enter(); @@ -420,7 +420,7 @@ int UARTSerial::enable_output(bool enabled) _tx_enabled = enabled; if (!enabled && !_rx_enabled) { - deinit(); + _deinit(); } } api_unlock(); From d1250fccfe2a327f889bba5bfb2dbd6e2e6eb651 Mon Sep 17 00:00:00 2001 From: Sebastian Stockhammer Date: Tue, 18 Jun 2019 14:56:16 +0200 Subject: [PATCH 4/4] Remove obsolete locks --- drivers/SerialBase.cpp | 4 ---- 1 file changed, 4 deletions(-) diff --git a/drivers/SerialBase.cpp b/drivers/SerialBase.cpp index 91d37395ce0..e62c53788d4 100644 --- a/drivers/SerialBase.cpp +++ b/drivers/SerialBase.cpp @@ -155,7 +155,6 @@ void SerialBase:: unlock() void SerialBase::_init(void) { - lock(); for (size_t i = 0; i < sizeof _irq / sizeof _irq[0]; i++) { _irq[i] = NULL; } @@ -163,14 +162,11 @@ void SerialBase::_init(void) serial_init(&_serial, _tx, _rx); serial_baud(&_serial, _baud); serial_irq_handler(&_serial, SerialBase::_irq_handler, (uint32_t)this); - unlock(); } void SerialBase::_deinit(void) { - lock(); serial_free(&_serial); - unlock(); } SerialBase::~SerialBase()