Skip to content

Commit 17ae9b9

Browse files
Merge pull request #4538 from hasnainvirk/avoid_mtx_lock
Avoid lock collision b/w SerialBase & UARTSerial
2 parents 86f7cf4 + 64a92df commit 17ae9b9

File tree

2 files changed

+47
-28
lines changed

2 files changed

+47
-28
lines changed

drivers/UARTSerial.cpp

Lines changed: 30 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
* limitations under the License.
1515
*/
1616

17-
#if DEVICE_SERIAL
17+
#if (DEVICE_SERIAL && DEVICE_INTERRUPTIN)
1818

1919
#include <errno.h>
2020
#include "UARTSerial.h"
@@ -80,16 +80,16 @@ off_t UARTSerial::seek(off_t offset, int whence)
8080

8181
int UARTSerial::sync()
8282
{
83-
lock();
83+
api_lock();
8484

8585
while (!_txbuf.empty()) {
86-
unlock();
86+
api_unlock();
8787
// Doing better than wait would require TxIRQ to also do wake() when becoming empty. Worth it?
8888
wait_ms(1);
89-
lock();
89+
api_lock();
9090
}
9191

92-
unlock();
92+
api_unlock();
9393

9494
return 0;
9595
}
@@ -111,16 +111,16 @@ ssize_t UARTSerial::write(const void* buffer, size_t length)
111111
size_t data_written = 0;
112112
const char *buf_ptr = static_cast<const char *>(buffer);
113113

114-
lock();
114+
api_lock();
115115

116116
while (_txbuf.full()) {
117117
if (!_blocking) {
118-
unlock();
118+
api_unlock();
119119
return -EAGAIN;
120120
}
121-
unlock();
121+
api_unlock();
122122
wait_ms(1); // XXX todo - proper wait, WFE for non-rtos ?
123-
lock();
123+
api_lock();
124124
}
125125

126126
while (data_written < length && !_txbuf.full()) {
@@ -138,7 +138,7 @@ ssize_t UARTSerial::write(const void* buffer, size_t length)
138138
}
139139
core_util_critical_section_exit();
140140

141-
unlock();
141+
api_unlock();
142142

143143
return data_written;
144144
}
@@ -149,24 +149,24 @@ ssize_t UARTSerial::read(void* buffer, size_t length)
149149

150150
char *ptr = static_cast<char *>(buffer);
151151

152-
lock();
152+
api_lock();
153153

154154
while (_rxbuf.empty()) {
155155
if (!_blocking) {
156-
unlock();
156+
api_unlock();
157157
return -EAGAIN;
158158
}
159-
unlock();
159+
api_unlock();
160160
wait_ms(1); // XXX todo - proper wait, WFE for non-rtos ?
161-
lock();
161+
api_lock();
162162
}
163163

164164
while (data_read < length && !_rxbuf.empty()) {
165165
_rxbuf.pop(*ptr++);
166166
data_read++;
167167
}
168168

169-
unlock();
169+
api_unlock();
170170

171171
return data_read;
172172
}
@@ -205,12 +205,24 @@ short UARTSerial::poll(short events) const {
205205
return revents;
206206
}
207207

208-
void UARTSerial::lock(void)
208+
void UARTSerial::lock()
209+
{
210+
// This is the override for SerialBase.
211+
// No lock required as we only use SerialBase from interrupt or from
212+
// inside our own critical section.
213+
}
214+
215+
void UARTSerial::unlock()
216+
{
217+
// This is the override for SerialBase.
218+
}
219+
220+
void UARTSerial::api_lock(void)
209221
{
210222
_mutex.lock();
211223
}
212224

213-
void UARTSerial::unlock(void)
225+
void UARTSerial::api_unlock(void)
214226
{
215227
_mutex.unlock();
216228
}
@@ -262,4 +274,4 @@ void UARTSerial::tx_irq(void)
262274

263275
} //namespace mbed
264276

265-
#endif //DEVICE_SERIAL
277+
#endif //(DEVICE_SERIAL && DEVICE_INTERRUPTIN)

drivers/UARTSerial.h

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919

2020
#include "platform/platform.h"
2121

22-
#if DEVICE_SERIAL
22+
#if (DEVICE_SERIAL && DEVICE_INTERRUPTIN) || defined(DOXYGEN_ONLY)
2323

2424
#include "FileHandle.h"
2525
#include "SerialBase.h"
@@ -58,7 +58,7 @@ class UARTSerial : private SerialBase, public FileHandle {
5858
/** Write the contents of a buffer to a file
5959
*
6060
* @param buffer The buffer to write from
61-
* @param size The number of bytes to write
61+
* @param length The number of bytes to write
6262
* @return The number of bytes written, negative error on failure
6363
*/
6464
virtual ssize_t write(const void* buffer, size_t length);
@@ -72,17 +72,11 @@ class UARTSerial : private SerialBase, public FileHandle {
7272
* * If any data is available, call returns immediately
7373
*
7474
* @param buffer The buffer to read in to
75-
* @param size The number of bytes to read
75+
* @param length The number of bytes to read
7676
* @return The number of bytes read, 0 at end of file, negative error on failure
7777
*/
7878
virtual ssize_t read(void* buffer, size_t length);
7979

80-
/** Acquire mutex */
81-
virtual void lock(void);
82-
83-
/** Release mutex */
84-
virtual void unlock(void);
85-
8680
/** Close a file
8781
*
8882
* @return 0 on success, negative error code on failure
@@ -159,6 +153,18 @@ class UARTSerial : private SerialBase, public FileHandle {
159153

160154
private:
161155

156+
/** SerialBase lock override */
157+
virtual void lock(void);
158+
159+
/** SerialBase unlock override */
160+
virtual void unlock(void);
161+
162+
/** Acquire mutex */
163+
virtual void api_lock(void);
164+
165+
/** Release mutex */
166+
virtual void api_unlock(void);
167+
162168
/** Software serial buffers
163169
* By default buffer size is 256 for TX and 256 for RX. Configurable through mbed_app.json
164170
*/
@@ -191,8 +197,9 @@ class UARTSerial : private SerialBase, public FileHandle {
191197
void wake(void);
192198

193199
void dcd_irq(void);
200+
194201
};
195202
} //namespace mbed
196203

197-
#endif //DEVICE_SERIAL
204+
#endif //(DEVICE_SERIAL && DEVICE_INTERRUPTIN) || defined(DOXYGEN_ONLY)
198205
#endif //MBED_UARTSERIAL_H

0 commit comments

Comments
 (0)