Skip to content

Commit e7b737d

Browse files
committed
[NUC472/M453] Fix serial error with sync/async calls interlaced
Serial implementation uses different vector handlers for sync/async calls respectively. The issue can be reproduced with the following flow: 1. Register sync mode callback with Serial.attach(). 2. Sync call with Serial.putc()/getc(). 3. Change to async call with Serial.write()/read(). 4. Change back to sync call with Serial.putc()/getc(). Now, vector handller is still for async mode, not for sync mode. To fix it: 1. Introduce internal function serial_enable_interrupt() for both sync/async vector handler enable/disable. Original HAL function serial_irq_set() is reduced to call it for sync mode vector handler enable/disable. 2. Introduce internal function serial_rollback_interrupt() to roll back sync mode vector handler at end of async transfer.
1 parent 32a7e6b commit e7b737d

File tree

4 files changed

+190
-122
lines changed

4 files changed

+190
-122
lines changed

targets/TARGET_NUVOTON/TARGET_M451/TARGET_NUMAKER_PFM_M453/objects.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ struct serial_s {
6161
void (*vec)(void);
6262
uint32_t irq_handler;
6363
uint32_t irq_id;
64+
uint32_t irq_en;
6465
uint32_t inten_msk;
6566

6667
// Async transfer related fields

targets/TARGET_NUVOTON/TARGET_M451/serial_api.c

Lines changed: 94 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
#include "PeripheralPins.h"
2525
#include "nu_modutil.h"
2626
#include "nu_bitutil.h"
27+
#include <string.h>
2728

2829
#if DEVICE_SERIAL_ASYNCH
2930
#include "dma_api.h"
@@ -61,6 +62,8 @@ static void uart_dma_handler_rx(uint32_t id, uint32_t event);
6162

6263
static void serial_tx_enable_interrupt(serial_t *obj, uint32_t address, uint8_t enable);
6364
static void serial_rx_enable_interrupt(serial_t *obj, uint32_t address, uint8_t enable);
65+
static void serial_enable_interrupt(serial_t *obj, SerialIrq irq, uint32_t enable);
66+
static void serial_rollback_interrupt(serial_t *obj, SerialIrq irq);
6467
static int serial_write_async(serial_t *obj);
6568
static int serial_read_async(serial_t *obj);
6669

@@ -159,7 +162,7 @@ void serial_init(serial_t *obj, PinName tx, PinName rx)
159162

160163
const struct nu_modinit_s *modinit = get_modinit(obj->serial.uart, uart_modinit_tab);
161164
MBED_ASSERT(modinit != NULL);
162-
MBED_ASSERT(modinit->modname == obj->serial.uart);
165+
MBED_ASSERT(modinit->modname == (int) obj->serial.uart);
163166

164167
struct nu_uart_var *var = (struct nu_uart_var *) modinit->var;
165168

@@ -186,6 +189,7 @@ void serial_init(serial_t *obj, PinName tx, PinName rx)
186189
serial_format(obj, 8, ParityNone, 1);
187190

188191
obj->serial.vec = var->vec;
192+
obj->serial.irq_en = 0;
189193

190194
#if DEVICE_SERIAL_ASYNCH
191195
obj->serial.dma_usage_tx = DMA_USAGE_NEVER;
@@ -212,7 +216,7 @@ void serial_free(serial_t *obj)
212216
{
213217
const struct nu_modinit_s *modinit = get_modinit(obj->serial.uart, uart_modinit_tab);
214218
MBED_ASSERT(modinit != NULL);
215-
MBED_ASSERT(modinit->modname == obj->serial.uart);
219+
MBED_ASSERT(modinit->modname == (int) obj->serial.uart);
216220

217221
struct nu_uart_var *var = (struct nu_uart_var *) modinit->var;
218222

@@ -329,7 +333,7 @@ void serial_irq_handler(serial_t *obj, uart_irq_handler handler, uint32_t id)
329333

330334
const struct nu_modinit_s *modinit = get_modinit(obj->serial.uart, uart_modinit_tab);
331335
MBED_ASSERT(modinit != NULL);
332-
MBED_ASSERT(modinit->modname == obj->serial.uart);
336+
MBED_ASSERT(modinit->modname == (int) obj->serial.uart);
333337

334338
obj->serial.irq_handler = (uint32_t) handler;
335339
obj->serial.irq_id = id;
@@ -340,51 +344,18 @@ void serial_irq_handler(serial_t *obj, uart_irq_handler handler, uint32_t id)
340344

341345
void serial_irq_set(serial_t *obj, SerialIrq irq, uint32_t enable)
342346
{
343-
if (enable) {
344-
const struct nu_modinit_s *modinit = get_modinit(obj->serial.uart, uart_modinit_tab);
345-
MBED_ASSERT(modinit != NULL);
346-
MBED_ASSERT(modinit->modname == obj->serial.uart);
347-
348-
NVIC_SetVector(modinit->irq_n, (uint32_t) obj->serial.vec);
349-
NVIC_EnableIRQ(modinit->irq_n);
350-
351-
struct nu_uart_var *var = (struct nu_uart_var *) modinit->var;
352-
// Multiple serial S/W objects for single UART H/W module possibly.
353-
// Bind serial S/W object to UART H/W module as interrupt is enabled.
354-
var->obj = obj;
355-
356-
switch (irq) {
357-
// NOTE: Setting inten_msk first to avoid race condition
358-
case RxIrq:
359-
obj->serial.inten_msk = obj->serial.inten_msk | (UART_INTEN_RDAIEN_Msk | UART_INTEN_RXTOIEN_Msk);
360-
UART_ENABLE_INT(((UART_T *) NU_MODBASE(obj->serial.uart)), (UART_INTEN_RDAIEN_Msk | UART_INTEN_RXTOIEN_Msk));
361-
break;
362-
case TxIrq:
363-
obj->serial.inten_msk = obj->serial.inten_msk | UART_INTEN_THREIEN_Msk;
364-
UART_ENABLE_INT(((UART_T *) NU_MODBASE(obj->serial.uart)), UART_INTEN_THREIEN_Msk);
365-
break;
366-
}
367-
} else { // disable
368-
switch (irq) {
369-
case RxIrq:
370-
UART_DISABLE_INT(((UART_T *) NU_MODBASE(obj->serial.uart)), (UART_INTEN_RDAIEN_Msk | UART_INTEN_RXTOIEN_Msk));
371-
obj->serial.inten_msk = obj->serial.inten_msk & ~(UART_INTEN_RDAIEN_Msk | UART_INTEN_RXTOIEN_Msk);
372-
break;
373-
case TxIrq:
374-
UART_DISABLE_INT(((UART_T *) NU_MODBASE(obj->serial.uart)), UART_INTEN_THREIEN_Msk);
375-
obj->serial.inten_msk = obj->serial.inten_msk & ~UART_INTEN_THREIEN_Msk;
376-
break;
377-
}
378-
}
347+
obj->serial.irq_en = enable;
348+
serial_enable_interrupt(obj, irq, enable);
379349
}
380350

381351
int serial_getc(serial_t *obj)
382352
{
383-
// TODO: Fix every byte access requires accompaniness of one interrupt. This degrades performance much.
353+
// TODO: Fix every byte access requires accompaniment of one interrupt. This has side effect of performance degradation.
384354
while (! serial_readable(obj));
385355
int c = UART_READ(((UART_T *) NU_MODBASE(obj->serial.uart)));
386356

387-
// Simulate clear of the interrupt flag
357+
// NOTE: On Nuvoton targets, no H/W IRQ to match TxIrq/RxIrq.
358+
// Simulation of TxIrq/RxIrq requires the call to Serial::putc()/Serial::getc() respectively.
388359
if (obj->serial.inten_msk & (UART_INTEN_RDAIEN_Msk | UART_INTEN_RXTOIEN_Msk)) {
389360
UART_ENABLE_INT(((UART_T *) NU_MODBASE(obj->serial.uart)), (UART_INTEN_RDAIEN_Msk | UART_INTEN_RXTOIEN_Msk));
390361
}
@@ -394,11 +365,12 @@ int serial_getc(serial_t *obj)
394365

395366
void serial_putc(serial_t *obj, int c)
396367
{
397-
// TODO: Fix every byte access requires accompaniness of one interrupt. This degrades performance much.
368+
// TODO: Fix every byte access requires accompaniment of one interrupt. This has side effect of performance degradation.
398369
while (! serial_writable(obj));
399370
UART_WRITE(((UART_T *) NU_MODBASE(obj->serial.uart)), c);
400371

401-
// Simulate clear of the interrupt flag
372+
// NOTE: On Nuvoton targets, no H/W IRQ to match TxIrq/RxIrq.
373+
// Simulation of TxIrq/RxIrq requires the call to Serial::putc()/Serial::getc() respectively.
402374
if (obj->serial.inten_msk & UART_INTEN_THREIEN_Msk) {
403375
UART_ENABLE_INT(((UART_T *) NU_MODBASE(obj->serial.uart)), UART_INTEN_THREIEN_Msk);
404376
}
@@ -500,7 +472,7 @@ int serial_tx_asynch(serial_t *obj, const void *tx, size_t tx_length, uint8_t tx
500472
// DMA way
501473
const struct nu_modinit_s *modinit = get_modinit(obj->serial.uart, uart_modinit_tab);
502474
MBED_ASSERT(modinit != NULL);
503-
MBED_ASSERT(modinit->modname == obj->serial.uart);
475+
MBED_ASSERT(modinit->modname == (int) obj->serial.uart);
504476

505477
PDMA_T *pdma_base = dma_modbase();
506478

@@ -563,7 +535,7 @@ void serial_rx_asynch(serial_t *obj, void *rx, size_t rx_length, uint8_t rx_widt
563535
// DMA way
564536
const struct nu_modinit_s *modinit = get_modinit(obj->serial.uart, uart_modinit_tab);
565537
MBED_ASSERT(modinit != NULL);
566-
MBED_ASSERT(modinit->modname == obj->serial.uart);
538+
MBED_ASSERT(modinit->modname == (int) obj->serial.uart);
567539

568540
PDMA_T *pdma_base = dma_modbase();
569541

@@ -612,10 +584,8 @@ void serial_tx_abort_asynch(serial_t *obj)
612584
}
613585

614586
// Necessary for both interrupt way and DMA way
615-
serial_irq_set(obj, TxIrq, 0);
616-
// FIXME: more complete abort operation
617-
//UART_HAL_DisableTransmitter(obj->serial.serial.address);
618-
//UART_HAL_FlushTxFifo(obj->serial.serial.address);
587+
serial_enable_interrupt(obj, TxIrq, 0);
588+
serial_rollback_interrupt(obj, TxIrq);
619589
}
620590

621591
void serial_rx_abort_asynch(serial_t *obj)
@@ -633,20 +603,30 @@ void serial_rx_abort_asynch(serial_t *obj)
633603
}
634604

635605
// Necessary for both interrupt way and DMA way
636-
serial_irq_set(obj, RxIrq, 0);
637-
// FIXME: more complete abort operation
638-
//UART_HAL_DisableReceiver(obj->serial.serial.address);
639-
//UART_HAL_FlushRxFifo(obj->serial.serial.address);
606+
serial_enable_interrupt(obj, RxIrq, 0);
607+
serial_rollback_interrupt(obj, RxIrq);
640608
}
641609

642610
uint8_t serial_tx_active(serial_t *obj)
643611
{
644-
return serial_is_irq_en(obj, TxIrq);
612+
// NOTE: Judge by serial_is_irq_en(obj, TxIrq) doesn't work with sync/async modes interleaved. Change with TX FIFO empty flag.
613+
const struct nu_modinit_s *modinit = get_modinit(obj->serial.uart, uart_modinit_tab);
614+
MBED_ASSERT(modinit != NULL);
615+
MBED_ASSERT(modinit->modname == (int) obj->serial.uart);
616+
617+
struct nu_uart_var *var = (struct nu_uart_var *) modinit->var;
618+
return (obj->serial.vec == var->vec_async);
645619
}
646620

647621
uint8_t serial_rx_active(serial_t *obj)
648622
{
649-
return serial_is_irq_en(obj, RxIrq);
623+
// NOTE: Judge by serial_is_irq_en(obj, RxIrq) doesn't work with sync/async modes interleaved. Change with RX FIFO empty flag.
624+
const struct nu_modinit_s *modinit = get_modinit(obj->serial.uart, uart_modinit_tab);
625+
MBED_ASSERT(modinit != NULL);
626+
MBED_ASSERT(modinit->modname == (int) obj->serial.uart);
627+
628+
struct nu_uart_var *var = (struct nu_uart_var *) modinit->var;
629+
return (obj->serial.vec == var->vec_async);
650630
}
651631

652632
int serial_irq_handler_asynch(serial_t *obj)
@@ -886,7 +866,7 @@ static int serial_write_async(serial_t *obj)
886866
{
887867
const struct nu_modinit_s *modinit = get_modinit(obj->serial.uart, uart_modinit_tab);
888868
MBED_ASSERT(modinit != NULL);
889-
MBED_ASSERT(modinit->modname == obj->serial.uart);
869+
MBED_ASSERT(modinit->modname == (int) obj->serial.uart);
890870

891871
UART_T *uart_base = (UART_T *) NU_MODBASE(obj->serial.uart);
892872

@@ -938,7 +918,7 @@ static int serial_read_async(serial_t *obj)
938918
{
939919
const struct nu_modinit_s *modinit = get_modinit(obj->serial.uart, uart_modinit_tab);
940920
MBED_ASSERT(modinit != NULL);
941-
MBED_ASSERT(modinit->modname == obj->serial.uart);
921+
MBED_ASSERT(modinit->modname == (int) obj->serial.uart);
942922

943923
uint32_t rx_fifo_busy = (((UART_T *) NU_MODBASE(obj->serial.uart))->FIFOSTS & UART_FIFOSTS_RXPTR_Msk) >> UART_FIFOSTS_RXPTR_Pos;
944924
//uint32_t rx_fifo_free = ((struct nu_uart_var *) modinit->var)->fifo_size_rx - rx_fifo_busy;
@@ -1013,28 +993,81 @@ static void serial_tx_enable_interrupt(serial_t *obj, uint32_t handler, uint8_t
1013993
{
1014994
const struct nu_modinit_s *modinit = get_modinit(obj->serial.uart, uart_modinit_tab);
1015995
MBED_ASSERT(modinit != NULL);
1016-
MBED_ASSERT(modinit->modname == obj->serial.uart);
996+
MBED_ASSERT(modinit->modname == (int) obj->serial.uart);
1017997

1018998
// Necessary for both interrupt way and DMA way
1019999
struct nu_uart_var *var = (struct nu_uart_var *) modinit->var;
10201000
// With our own async vector, tx/rx handlers can be different.
10211001
obj->serial.vec = var->vec_async;
10221002
obj->serial.irq_handler_tx_async = (void (*)(void)) handler;
1023-
serial_irq_set(obj, TxIrq, enable);
1003+
serial_enable_interrupt(obj, TxIrq, enable);
10241004
}
10251005

10261006
static void serial_rx_enable_interrupt(serial_t *obj, uint32_t handler, uint8_t enable)
10271007
{
10281008
const struct nu_modinit_s *modinit = get_modinit(obj->serial.uart, uart_modinit_tab);
10291009
MBED_ASSERT(modinit != NULL);
1030-
MBED_ASSERT(modinit->modname == obj->serial.uart);
1010+
MBED_ASSERT(modinit->modname == (int) obj->serial.uart);
10311011

10321012
// Necessary for both interrupt way and DMA way
10331013
struct nu_uart_var *var = (struct nu_uart_var *) modinit->var;
10341014
// With our own async vector, tx/rx handlers can be different.
10351015
obj->serial.vec = var->vec_async;
10361016
obj->serial.irq_handler_rx_async = (void (*) (void)) handler;
1037-
serial_irq_set(obj, RxIrq, enable);
1017+
serial_enable_interrupt(obj, RxIrq, enable);
1018+
}
1019+
1020+
static void serial_enable_interrupt(serial_t *obj, SerialIrq irq, uint32_t enable)
1021+
{
1022+
if (enable) {
1023+
const struct nu_modinit_s *modinit = get_modinit(obj->serial.uart, uart_modinit_tab);
1024+
MBED_ASSERT(modinit != NULL);
1025+
MBED_ASSERT(modinit->modname == (int) obj->serial.uart);
1026+
1027+
NVIC_SetVector(modinit->irq_n, (uint32_t) obj->serial.vec);
1028+
NVIC_EnableIRQ(modinit->irq_n);
1029+
1030+
struct nu_uart_var *var = (struct nu_uart_var *) modinit->var;
1031+
// Multiple serial S/W objects for single UART H/W module possibly.
1032+
// Bind serial S/W object to UART H/W module as interrupt is enabled.
1033+
var->obj = obj;
1034+
1035+
switch (irq) {
1036+
// NOTE: Setting inten_msk first to avoid race condition
1037+
case RxIrq:
1038+
obj->serial.inten_msk = obj->serial.inten_msk | (UART_INTEN_RDAIEN_Msk | UART_INTEN_RXTOIEN_Msk);
1039+
UART_ENABLE_INT(((UART_T *) NU_MODBASE(obj->serial.uart)), (UART_INTEN_RDAIEN_Msk | UART_INTEN_RXTOIEN_Msk));
1040+
break;
1041+
case TxIrq:
1042+
obj->serial.inten_msk = obj->serial.inten_msk | UART_INTEN_THREIEN_Msk;
1043+
UART_ENABLE_INT(((UART_T *) NU_MODBASE(obj->serial.uart)), UART_INTEN_THREIEN_Msk);
1044+
break;
1045+
}
1046+
}
1047+
else { // disable
1048+
switch (irq) {
1049+
case RxIrq:
1050+
UART_DISABLE_INT(((UART_T *) NU_MODBASE(obj->serial.uart)), (UART_INTEN_RDAIEN_Msk | UART_INTEN_RXTOIEN_Msk));
1051+
obj->serial.inten_msk = obj->serial.inten_msk & ~(UART_INTEN_RDAIEN_Msk | UART_INTEN_RXTOIEN_Msk);
1052+
break;
1053+
case TxIrq:
1054+
UART_DISABLE_INT(((UART_T *) NU_MODBASE(obj->serial.uart)), UART_INTEN_THREIEN_Msk);
1055+
obj->serial.inten_msk = obj->serial.inten_msk & ~UART_INTEN_THREIEN_Msk;
1056+
break;
1057+
}
1058+
}
1059+
}
1060+
1061+
static void serial_rollback_interrupt(serial_t *obj, SerialIrq irq)
1062+
{
1063+
const struct nu_modinit_s *modinit = get_modinit(obj->serial.uart, uart_modinit_tab);
1064+
MBED_ASSERT(modinit != NULL);
1065+
MBED_ASSERT(modinit->modname == (int) obj->serial.uart);
1066+
1067+
struct nu_uart_var *var = (struct nu_uart_var *) modinit->var;
1068+
1069+
obj->serial.vec = var->vec;
1070+
serial_enable_interrupt(obj, irq, obj->serial.irq_en);
10381071
}
10391072

10401073
static void serial_check_dma_usage(DMAUsage *dma_usage, int *dma_ch)

targets/TARGET_NUVOTON/TARGET_NUC472/TARGET_NUMAKER_PFM_NUC472/objects.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ struct serial_s {
6161
void (*vec)(void);
6262
uint32_t irq_handler;
6363
uint32_t irq_id;
64+
uint32_t irq_en;
6465
uint32_t inten_msk;
6566

6667
// Async transfer related fields

0 commit comments

Comments
 (0)