Merge pull request #14498 from ghseb/uartserial-shadowing

UARTSerial writes even if tx is disabled
ARMmbed · Jun 9, 2021 · a0b02aa · a0b02aa
2 parents 2bec3ee + c51822c
commit a0b02aa
Show file tree

Hide file tree

Showing 2 changed files with 30 additions and 33 deletions.
diff --git a/drivers/UARTSerial.h b/drivers/UARTSerial.h
@@ -278,9 +278,9 @@ class UARTSerial : private SerialBase, public FileHandle, private NonCopyable<UA
     /** Unbuffered write - invoked when write called from critical section */
     ssize_t write_unbuffered(const char *buf_ptr, size_t length);
 
-    void enable_rx_irq();
+    void update_rx_irq();
     void disable_rx_irq();
-    void enable_tx_irq();
+    void update_tx_irq();
     void disable_tx_irq();
 
     /** Software serial buffers
@@ -296,8 +296,6 @@ class UARTSerial : private SerialBase, public FileHandle, private NonCopyable<UA
     bool _blocking;
     bool _tx_irq_enabled;
     bool _rx_irq_enabled;
-    bool _tx_enabled;
-    bool _rx_enabled;
     InterruptIn *_dcd_irq;
 
     /** Device Hanged up

diff --git a/drivers/source/UARTSerial.cpp b/drivers/source/UARTSerial.cpp
@@ -28,25 +28,21 @@ UARTSerial::UARTSerial(PinName tx, PinName rx, int baud) :
     _blocking(true),
     _tx_irq_enabled(false),
     _rx_irq_enabled(false),
-    _tx_enabled(true),
-    _rx_enabled(true),
     _dcd_irq(NULL)
 {
     /* Attatch IRQ routines to the serial device. */
-    enable_rx_irq();
+    update_rx_irq();
 }
 
 UARTSerial::UARTSerial(const serial_pinmap_t &static_pinmap, int baud) :
     SerialBase(static_pinmap, baud),
     _blocking(true),
     _tx_irq_enabled(false),
     _rx_irq_enabled(false),
-    _tx_enabled(true),
-    _rx_enabled(true),
     _dcd_irq(NULL)
 {
     /* Attatch IRQ routines to the serial device. */
-    enable_rx_irq();
+    update_rx_irq();
 }
 
 UARTSerial::~UARTSerial()
@@ -196,14 +192,7 @@ ssize_t UARTSerial::write(const void *buffer, size_t length)
             data_written++;
         }
 
-        core_util_critical_section_enter();
-        if (_tx_enabled && !_tx_irq_enabled) {
-            UARTSerial::tx_irq();                // only write to hardware in one place
-            if (!_txbuf.empty()) {
-                enable_tx_irq();
-            }
-        }
-        core_util_critical_section_exit();
+        update_tx_irq();
     }
 
     api_unlock();
@@ -238,14 +227,7 @@ ssize_t UARTSerial::read(void *buffer, size_t length)
         data_read++;
     }
 
-    core_util_critical_section_enter();
-    if (_rx_enabled && !_rx_irq_enabled) {
-        UARTSerial::rx_irq();               // only read from hardware in one place
-        if (!_rxbuf.full()) {
-            enable_rx_irq();
-        }
-    }
-    core_util_critical_section_exit();
+    update_rx_irq();
 
     api_unlock();
 
@@ -352,25 +334,40 @@ void UARTSerial::tx_irq(void)
     }
 }
 
-/* These are all called from critical section */
-void UARTSerial::enable_rx_irq()
+void UARTSerial::update_rx_irq()
 {
-    SerialBase::attach(callback(this, &UARTSerial::rx_irq), RxIrq);
-    _rx_irq_enabled = true;
+    core_util_critical_section_enter();
+    if (_rx_enabled && !_rx_irq_enabled) {
+        UARTSerial::rx_irq();
+        if (!_rxbuf.full()) {
+            SerialBase::attach(callback(this, &UARTSerial::rx_irq), RxIrq);
+            _rx_irq_enabled = true;
+        }
+    }
+    core_util_critical_section_exit();
 }
 
+/* This is called called from critical section or interrupt context */
 void UARTSerial::disable_rx_irq()
 {
     SerialBase::attach(NULL, RxIrq);
     _rx_irq_enabled = false;
 }
 
-void UARTSerial::enable_tx_irq()
+void UARTSerial::update_tx_irq()
 {
-    SerialBase::attach(callback(this, &UARTSerial::tx_irq), TxIrq);
-    _tx_irq_enabled = true;
+    core_util_critical_section_enter();
+    if (_tx_enabled && !_tx_irq_enabled) {
+        UARTSerial::tx_irq();
+        if (!_txbuf.empty()) {
+            SerialBase::attach(callback(this, &UARTSerial::tx_irq), TxIrq);
+            _tx_irq_enabled = true;
+        }
+    }
+    core_util_critical_section_exit();
 }
 
+/* This is called called from critical section or interrupt context */
 void UARTSerial::disable_tx_irq()
 {
     SerialBase::attach(NULL, TxIrq);
@@ -381,6 +378,7 @@ int UARTSerial::enable_input(bool enabled)
 {
     api_lock();
     SerialBase::enable_input(enabled);
+    update_rx_irq(); // Eventually enable rx-interrupt to handle incoming data
     api_unlock();
 
     return 0;
@@ -390,6 +388,7 @@ int UARTSerial::enable_output(bool enabled)
 {
     api_lock();
     SerialBase::enable_output(enabled);
+    update_tx_irq(); // Eventually enable tx-interrupt to flush buffered data
     api_unlock();
 
     return 0;