Skip to content

Free serial resources if not needed anymore #10924

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 14 commits into from
Oct 30, 2019
Merged

Free serial resources if not needed anymore #10924

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
e218b5d
Free serial resources if not needed anymore
Jul 1, 2019
0a395b6
Fix astyle
Jul 15, 2019
b3b7f98
Fix stub constructor
Jul 22, 2019
aa2969f
Public SerialBase::enable_input
Jul 23, 2019
37b54ca
Add serial_free stub
Jul 23, 2019
31babca
Retain flow control state
Aug 26, 2019
9e97037
Fix astyle
Aug 28, 2019
c31a360
Fix spelling
Aug 28, 2019
e72468c
Handle undefined DEVICE_SERIAL_FC
Aug 28, 2019
a310706
Call correct method
Oct 23, 2019
1c907e6
Update deep sleep locks according to enabled state
Oct 24, 2019
a77a49d
Add serial_free definition
Oct 24, 2019
2e81dad
Fix build
Oct 24, 2019
df2cfae
Maxim: fix serial free
0xc0170 Oct 29, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion UNITTESTS/stubs/SerialBase_stub.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@

namespace mbed {

SerialBase::SerialBase(PinName tx, PinName rx, int baud)
SerialBase::SerialBase(PinName tx, PinName rx, int baud) :
_tx_pin(tx), _rx_pin(rx)
{
}

Expand Down
43 changes: 43 additions & 0 deletions drivers/SerialBase.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,30 @@ class SerialBase : private NonCopyable<SerialBase> {
*/
void send_break();

/** Enable serial input
*
* If both serial input and serial output are disabled, the
* peripheral is freed. If either serial input or serial
* output is re-enabled, the peripheral is reinitialized.
*
* On re-initialization rx interrupts will be enabled if a
* rx handler is attached. The rx handler is called once
* during re-initialization.
*/
void enable_input(bool enable = true);

/** Enable serial output
*
* If both serial input and serial output are disabled, the
* peripheral is freed. If either serial input or serial
* output is re-enabled, the peripheral is reinitialized.
*
* On re-initialization tx interrupts will be enabled if a
* tx handler is attached. The tx handler is called once
* during re-initialization.
*/
void enable_output(bool enable = true);

#if !defined(DOXYGEN_ONLY)
protected:

Expand Down Expand Up @@ -295,6 +319,14 @@ class SerialBase : private NonCopyable<SerialBase> {

int _base_putc(int c);

/** Initialize serial port
*/
void _init();

/** Deinitialize serial port
*/
void _deinit();

#if DEVICE_SERIAL_ASYNCH
CThunk<SerialBase> _thunk_irq;
DMAUsage _tx_usage;
Expand All @@ -308,6 +340,17 @@ class SerialBase : private NonCopyable<SerialBase> {
serial_t _serial;
Callback<void()> _irq[IrqCnt];
int _baud;
bool _rx_enabled;
bool _tx_enabled;
const PinName _tx_pin;
const PinName _rx_pin;

#if DEVICE_SERIAL_FC
Flow _flow_type;
PinName _flow1;
PinName _flow2;
#endif

#endif
};

Expand Down
144 changes: 126 additions & 18 deletions drivers/source/SerialBase.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,17 +30,25 @@ 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),
#if DEVICE_SERIAL_FC
_flow_type(Disabled),
_flow1(NC),
_flow2(NC),
#endif
_rx_enabled(true),
_tx_enabled(true),
_tx_pin(tx),
_rx_pin(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)
Expand Down Expand Up @@ -78,24 +86,30 @@ int SerialBase::writeable()
void SerialBase::attach(Callback<void()> func, IrqType type)
{
lock();
// Disable interrupts when attaching interrupt handler
core_util_critical_section_enter();
if (func) {
// lock deep sleep only the first time
if (!_irq[type]) {
sleep_manager_lock_deep_sleep();
}
const bool enabled { (_rx_enabled &&(type == RxIrq)) || (_tx_enabled &&(type == TxIrq)) };
// If corresponding direction is not enabled only update the handler
if (!enabled) {
_irq[type] = func;
serial_irq_set(&_serial, (SerialIrq)type, 1);
} else {
// unlock deep sleep only the first time
if (_irq[type]) {
sleep_manager_unlock_deep_sleep();
// Disable interrupts when attaching interrupt handler
core_util_critical_section_enter();
if (func) {
// lock deep sleep only the first time
if (!_irq[type]) {
sleep_manager_lock_deep_sleep();
}
_irq[type] = func;
serial_irq_set(&_serial, (SerialIrq)type, 1);
} else {
// unlock deep sleep only the first time
if (_irq[type]) {
sleep_manager_unlock_deep_sleep();
}
_irq[type] = NULL;
serial_irq_set(&_serial, (SerialIrq)type, 0);
}
_irq[type] = NULL;
serial_irq_set(&_serial, (SerialIrq)type, 0);
core_util_critical_section_exit();
}
core_util_critical_section_exit();
unlock();
}

Expand All @@ -120,6 +134,95 @@ int SerialBase::_base_putc(int c)
return c;
}

void SerialBase::_init()
{
serial_init(&_serial, _tx_pin, _rx_pin);
#if DEVICE_SERIAL_FC
set_flow_control(_flow_type, _flow1, _flow2);
#endif
serial_baud(&_serial, _baud);
serial_irq_handler(&_serial, SerialBase::_irq_handler, (uint32_t)this);
}

void SerialBase::_deinit()
{
serial_free(&_serial);
}

void SerialBase::enable_input(bool enable)
{
lock();
if (_rx_enabled != enable) {
if (enable && !_tx_enabled) {
_init();
}

core_util_critical_section_enter();
if (enable) {
// Enable rx IRQ and lock deep sleep if a rx handler is attached
// (indicated by rx IRQ callback not NULL)
if (_irq[RxIrq]) {
_irq[RxIrq].call();
sleep_manager_lock_deep_sleep();
serial_irq_set(&_serial, (SerialIrq)RxIrq, 1);
}
} else {
// Disable rx IRQ
serial_irq_set(&_serial, (SerialIrq)RxIrq, 0);
// Unlock deep sleep if a rx handler is attached
// (indicated by rx IRQ callback not NULL)
if (_irq[RxIrq]) {
sleep_manager_unlock_deep_sleep();
}
}
core_util_critical_section_exit();

_rx_enabled = enable;

if (!enable && !_tx_enabled) {
_deinit();
}
}
unlock();
}

void SerialBase::enable_output(bool enable)
{
lock();
if (_tx_enabled != enable) {
if (enable && !_rx_enabled) {
_init();
}

core_util_critical_section_enter();
if (enable) {
// Enable tx IRQ and lock deep sleep if a tx handler is attached
// (indicated by tx IRQ callback not NULL)
if (_irq[TxIrq]) {
_irq[TxIrq].call();
sleep_manager_lock_deep_sleep();
serial_irq_set(&_serial, (SerialIrq)TxIrq, 1);
}
} else {
// Disable tx IRQ
serial_irq_set(&_serial, (SerialIrq)TxIrq, 0);
// Unlock deep sleep if a tx handler is attached
// (indicated by tx IRQ callback not NULL)
if (_irq[TxIrq]) {
sleep_manager_unlock_deep_sleep();
}
}
core_util_critical_section_exit();

_tx_enabled = enable;

if (!enable && !_rx_enabled) {
_deinit();
}
}
unlock();
}

void SerialBase::set_break()
{
lock();
Expand Down Expand Up @@ -175,6 +278,11 @@ SerialBase::~SerialBase()
void SerialBase::set_flow_control(Flow type, PinName flow1, PinName flow2)
{
lock();

_flow_type = type;
_flow1 = flow1;
_flow2 = flow2;

FlowControl flow_type = (FlowControl)type;
switch (type) {
case RTS:
Expand Down
32 changes: 6 additions & 26 deletions drivers/source/UARTSerial.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -366,38 +366,18 @@ void UARTSerial::disable_tx_irq()

int UARTSerial::enable_input(bool enabled)
{
core_util_critical_section_enter();
if (_rx_enabled != enabled) {
if (enabled) {
UARTSerial::rx_irq();
if (!_rxbuf.full()) {
enable_rx_irq();
}
} else {
disable_rx_irq();
}
_rx_enabled = enabled;
}
core_util_critical_section_exit();
api_lock();
SerialBase::enable_input(enabled);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where is the call to enable_rx_irq to attach the rx Irq?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The interrupt handler remains attached. It activated by enabling the rx irq in SerialBase::enable_input.

But i think there is a real problem with this version, as the device is not allowed to go to deep sleep, because deep sleep is unblocked by passing nullptr as func to SerialBase::attach (which is not called).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch; the SerialBase::enable_input/output functions will have to manipulate the locks. Not sure what the most efficient way of handling that is - ideally you'd be able to have some common "update HAL IRQ + lock state" call that was used by both attach and enable.

api_unlock();

return 0;
}

int UARTSerial::enable_output(bool enabled)
{
core_util_critical_section_enter();
if (_tx_enabled != enabled) {
if (enabled) {
UARTSerial::tx_irq();
if (!_txbuf.empty()) {
enable_tx_irq();
}
} else {
disable_tx_irq();
}
_tx_enabled = enabled;
}
core_util_critical_section_exit();
api_lock();
SerialBase::enable_output(enabled);
api_unlock();

return 0;
}
Expand Down
6 changes: 6 additions & 0 deletions targets/TARGET_Maxim/TARGET_MAX32600/serial_api.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,12 @@ void serial_init(serial_t *obj, PinName tx, PinName rx)
}
}

//******************************************************************************
void serial_free(serial_t *obj)
{
serial_irq_ids[obj->index];
}

//******************************************************************************
void serial_baud(serial_t *obj, int baudrate)
{
Expand Down
5 changes: 5 additions & 0 deletions targets/TARGET_Maxim/TARGET_MAX32620C/serial_api.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -418,3 +418,8 @@ const PinMap *serial_rts_pinmap()
{
return PinMap_UART_RTS;
}

void serial_free(serial_t *obj)
{
// Not implemented
}
7 changes: 7 additions & 0 deletions targets/TARGET_Maxim/TARGET_MAX32625/serial_api.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,13 @@ void serial_init(serial_t *obj, PinName tx, PinName rx)
MBED_ASSERT(retval == E_NO_ERROR);
}

//******************************************************************************
void serial_free(serial_t *obj)
{
UART_Shutdown(obj->uart);
objs[obj->index] = 0;
}

//******************************************************************************
void serial_baud(serial_t *obj, int baudrate)
{
Expand Down
7 changes: 7 additions & 0 deletions targets/TARGET_Maxim/TARGET_MAX32630/serial_api.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,13 @@ void serial_init(serial_t *obj, PinName tx, PinName rx)
MBED_ASSERT(retval == E_NO_ERROR);
}

//******************************************************************************
void serial_free(serial_t *obj)
{
UART_Shutdown(obj->uart);
objs[obj->index] = 0;
}

//******************************************************************************
void serial_baud(serial_t *obj, int baudrate)
{
Expand Down