From 77f794e94a35763fc2f399f5e8392c67d4228c8b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Hru=C5=A1ka?= Date: Mon, 15 Jan 2018 02:24:46 +0100 Subject: [PATCH] some debuggiong. Tx is not reliable and sometimes duplicates or loses bytes --- gex.mk | 2 +- units/usart/_dmas.c | 175 ++++++++++++++++++++++++++++++++++----- units/usart/_internal.h | 51 +++++++++++- units/usart/unit_usart.c | 60 +++++++++----- 4 files changed, 244 insertions(+), 44 deletions(-) diff --git a/gex.mk b/gex.mk index 84ff98e..0b7e2fe 100644 --- a/gex.mk +++ b/gex.mk @@ -88,7 +88,7 @@ GEX_CDEFS = $(GEX_CDEFS_BASE) \ -DDEBUG_VFS=0 \ -DDEBUG_FLASH_WRITE=0 \ -DVERBOSE_HARDFAULT=1 \ - -DUSE_STACK_MONITOR=1 \ + -DUSE_STACK_MONITOR=0 \ -DUSE_DEBUG_UART=1 \ -DDEBUG_MALLOC=0 \ -DDEBUG_RSC=0 diff --git a/units/usart/_dmas.c b/units/usart/_dmas.c index c0622de..0b9659a 100644 --- a/units/usart/_dmas.c +++ b/units/usart/_dmas.c @@ -110,7 +110,7 @@ error_t UUSART_ClaimDMAs(Unit *unit) trap("Missing DMA mapping for USART%d", (int)priv->periph_num); } - dbg("USART %d - selected DMA ch Tx(%d), Rx(%d)", priv->periph_num, priv->dma_tx_chnum, priv->dma_rx_chnum); +// dbg("USART %d - selected DMA ch Tx(%d), Rx(%d)", priv->periph_num, priv->dma_tx_chnum, priv->dma_rx_chnum); return E_SUCCESS; } @@ -135,7 +135,7 @@ error_t UUSART_SetupDMAs(Unit *unit) { LL_DMA_StructInit(&init); init.Direction = LL_DMA_DIRECTION_MEMORY_TO_PERIPH; - init.Mode = LL_DMA_MODE_CIRCULAR; + init.Mode = LL_DMA_MODE_NORMAL; init.PeriphOrM2MSrcAddress = (uint32_t) &priv->periph->TDR; init.PeriphOrM2MSrcDataSize = LL_DMA_PDATAALIGN_BYTE; @@ -184,6 +184,10 @@ error_t UUSART_SetupDMAs(Unit *unit) return E_SUCCESS; } +/** + * Handler for the Rx DMA half or full interrupt + * @param arg - unit instance + */ static void UUSART_DMA_RxHandler(void *arg) { Unit *unit = arg; @@ -197,29 +201,147 @@ static void UUSART_DMA_RxHandler(void *arg) bool tc = LL_DMA_IsActiveFlag_TC(isrsnapshot, priv->dma_rx_chnum); bool ht = LL_DMA_IsActiveFlag_HT(isrsnapshot, priv->dma_rx_chnum); + // Here we have to either copy it somewhere else, or notify another thread (queue?) + // that the data is ready for reading + + if (ht) { + uint16_t end = (uint16_t) UUSART_RXBUF_LEN / 2; + UUSART_DMA_HandleRxFromIRQ(unit, end); + LL_DMA_ClearFlag_HT(priv->dma, priv->dma_rx_chnum); + } + + if (tc) { + uint16_t end = (uint16_t) UUSART_RXBUF_LEN; + UUSART_DMA_HandleRxFromIRQ(unit, end); + LL_DMA_ClearFlag_TC(priv->dma, priv->dma_rx_chnum); + } - uint16_t end = (uint16_t) (ht ? UUSART_RXBUF_LEN / 2 : UUSART_RXBUF_LEN); + if (LL_DMA_IsActiveFlag_TE(isrsnapshot, priv->dma_rx_chnum)) { + // this shouldn't happen + LL_DMA_ClearFlag_TE(priv->dma, priv->dma_rx_chnum); + } + } +} - uint16_t toRead = (uint16_t) (end - priv->rx_buf_readpos); +/** + * Start sending a chunk of data. + * This must be called when the DMA is completed. + * + * @param priv + */ +static void UUSART_DMA_TxStart(struct priv *priv) +{ + dbg("DMA_TxStart (nr %d, nw %d)", (int)priv->tx_buf_nr, (int)priv->tx_buf_nw); + + assert_param(priv->dma_tx->CNDTR == 0); + uint16_t nr = priv->tx_buf_nr; + uint16_t nw = priv->tx_buf_nw; + +// if (nr == nw-1 || nr==0&&nw==UUSART_TXBUF_LEN-1) { +// dbg("FULL buf, cant start") +// } + + if (nr == nw) { + dbg("remain=0,do nothing"); + return; + } // do nothing if we're done + + uint16_t chunk = 0; + if (nr < nw) { + // linear forward + chunk = nw - nr; + } else { + // wrapped + chunk = (uint16_t) (UUSART_TXBUF_LEN - nr); + } + dbg("chunk %d", (int)chunk); - PUTS(">"); - PUTSN((char *) priv->rx_buffer+priv->rx_buf_readpos, toRead); - PUTS("<\r\n"); + priv->tx_buf_chunk = chunk; - // Prepare position for next read - if (ht) priv->rx_buf_readpos = (uint16_t) (end); - else priv->rx_buf_readpos = 0; + LL_DMA_DisableChannel(priv->dma, priv->dma_tx_chnum); + { + LL_DMA_SetMemoryAddress(priv->dma, priv->dma_tx_chnum, (uint32_t) (priv->tx_buffer + nr)); + LL_DMA_SetDataLength(priv->dma, priv->dma_tx_chnum, chunk); + LL_USART_ClearFlag_TC(priv->periph); + } + LL_DMA_EnableChannel(priv->dma, priv->dma_tx_chnum); +} +/** + * Put data on the queue. Only a part may be sent due to a buffer size limit. + * + * @param priv + * @param buffer - buffer to send + * @param len - buffer size + * @return number of bytes that were really written (from the beginning) + */ +uint16_t UUSART_DMA_TxQueue(struct priv *priv, const uint8_t *buffer, uint16_t len) +{ + const uint16_t nr = priv->tx_buf_nr; + uint16_t nw = priv->tx_buf_nw; - if (ht) LL_DMA_ClearFlag_HT(priv->dma, priv->dma_rx_chnum); - if (tc) LL_DMA_ClearFlag_TC(priv->dma, priv->dma_rx_chnum); - if (LL_DMA_IsActiveFlag_TE(isrsnapshot, priv->dma_rx_chnum)) { - dbg("Rx TE"); // this shouldn't happen - LL_DMA_ClearFlag_TE(priv->dma, priv->dma_rx_chnum); + if (nw == nr-1 || (nr==0&&nw==UUSART_TXBUF_LEN-1)) { + dbg("Buffer full, cant queue"); + return 0; + } + + dbg("\r\nQueue.."); + + uint16_t used = 0; + if (nr == nw) { + used = 0; + } + else if (nw > nr) { + // simple linear + used = (uint16_t) (nw - nr); + } + else if (nw < nr) { + // wrapped + used = (uint16_t) ((UUSART_TXBUF_LEN - nr) + nw); + } + + dbg("Trying to send buffer of len %d", (int)len); + uint16_t avail = (const uint16_t) (UUSART_TXBUF_LEN - 1 - used); + dbg("nr %d, nw %d, used %d, avail %d", (int)nr, (int)nw, (int)used, (int)avail); + + uint16_t towrite = MIN(avail, len); + const uint16_t towrite_orig = towrite; + + uint32_t cnt = 0; + while (towrite > 0) { + // this should run max 2x + assert_param(cnt < 2); + cnt++; + + uint16_t chunk = (uint16_t) MIN(towrite, UUSART_TXBUF_LEN - nw); + memcpy((uint8_t *) (priv->tx_buffer + nw), buffer, chunk); + dbg("- memcpy %d bytes at %d", (int)chunk, (int)nw); + nw += chunk; + towrite -= chunk; + + if (nw == UUSART_TXBUF_LEN) { + nw = 0; } } + priv->tx_buf_nw = nw; + + dbg("Written. -> nr %d, nw %d, used %d, avail %d", (int)nr, (int)nw, (int)used, (int)avail); + + // start the DMA if it's idle + if (priv->dma_tx->CNDTR == 0) { + dbg("Write done, requesting DMA."); + UUSART_DMA_TxStart(priv); + } else { + dbg("DMA in progress, not requesting"); + } + + return towrite_orig; } +/** + * Handler for the Tx DMA - completion interrupt + * @param arg - unit instance + */ static void UUSART_DMA_TxHandler(void *arg) { Unit *unit = arg; @@ -228,12 +350,25 @@ static void UUSART_DMA_TxHandler(void *arg) assert_param(priv); uint32_t isrsnapshot = priv->dma->ISR; - if (LL_DMA_IsActiveFlag_G(isrsnapshot, priv->dma_tx_chnum)) { - if (LL_DMA_IsActiveFlag_TC(isrsnapshot, priv->dma_tx_chnum)) { - dbg("Tx TC"); - } + if (LL_DMA_IsActiveFlag_TC(isrsnapshot, priv->dma_tx_chnum)) { + // chunk Tx is finished + dbg("DMA_TxHandler, lr %d, nw %d, chunk %d", (int)priv->tx_buf_nr, (int)priv->tx_buf_nw, (int)priv->tx_buf_chunk); - LL_DMA_ClearFlags(priv->dma, priv->dma_tx_chnum); +// dbg("StartPos advance..."); + priv->tx_buf_nr += priv->tx_buf_chunk; + if (UUSART_TXBUF_LEN == priv->tx_buf_nr) priv->tx_buf_nr = 0; + priv->tx_buf_chunk = 0; + + LL_DMA_ClearFlag_TC(priv->dma, priv->dma_tx_chnum); + + // Wait for TC + while (!LL_USART_IsActiveFlag_TC(priv->periph)); + + // start the next chunk + if (priv->tx_buf_nr != priv->tx_buf_nw) { + dbg("Flag cleared ... asking for more. lr %d, nw %d", (int)priv->tx_buf_nr, (int)priv->tx_buf_nw); + UUSART_DMA_TxStart(priv); + } } } diff --git a/units/usart/_internal.h b/units/usart/_internal.h index 39dfd02..b9132fc 100644 --- a/units/usart/_internal.h +++ b/units/usart/_internal.h @@ -11,7 +11,7 @@ #include "platform.h" -#define UUSART_RXBUF_LEN 128 +#define UUSART_RXBUF_LEN 64 #define UUSART_TXBUF_LEN 128 /** Private data structure */ @@ -48,9 +48,52 @@ struct priv { uint8_t dma_rx_chnum; uint8_t dma_tx_chnum; - uint8_t *rx_buffer; - uint8_t *tx_buffer; - uint16_t rx_buf_readpos; + // DMA stuff + volatile uint8_t *rx_buffer; + volatile uint8_t *tx_buffer; + volatile uint16_t rx_buf_readpos; + volatile uint16_t tx_buf_nr; + volatile uint16_t tx_buf_nw; + volatile uint16_t tx_buf_chunk; }; +/** Allocate data structure and set defaults */ +error_t UUSART_preInit(Unit *unit); +// ------------------------------------------------------------------------ +/** Load from a binary buffer stored in Flash */ +void UUSART_loadBinary(Unit *unit, PayloadParser *pp); +/** Write to a binary buffer for storing in Flash */ +void UUSART_writeBinary(Unit *unit, PayloadBuilder *pb); +// ------------------------------------------------------------------------ +/** Parse a key-value pair from the INI file */ +error_t UUSART_loadIni(Unit *unit, const char *key, const char *value); +/** Generate INI file section for the unit */ +void UUSART_writeIni(Unit *unit, IniWriter *iw); +// ------------------------------------------------------------------------ +/** Tear down the unit */ +void UUSART_deInit(Unit *unit); +/** Finalize unit set-up */ +error_t UUSART_init(Unit *unit); + +/** + * Handle received data (we're inside the IRQ) + * + * @param unit - handled unit + * @param endpos - end position in the buffer + */ +void UUSART_DMA_HandleRxFromIRQ(Unit *unit, uint16_t endpos); + +/** + * Put data on the queue. Only a part may be sent due to a buffer size limit. + * + * @param priv + * @param buffer - buffer to send + * @param len - buffer size + * @return number of bytes that were really written (from the beginning) + */ +uint16_t UUSART_DMA_TxQueue(struct priv *priv, const uint8_t *buffer, uint16_t len); + +// ------------------------------------------------------------------------ + + #endif //GEX_F072_UUSART_INTERNAL_H diff --git a/units/usart/unit_usart.c b/units/usart/unit_usart.c index a1c4e5c..88a3420 100644 --- a/units/usart/unit_usart.c +++ b/units/usart/unit_usart.c @@ -10,25 +10,36 @@ #define UUSART_INTERNAL #include "_internal.h" -/** Allocate data structure and set defaults */ -extern error_t UUSART_preInit(Unit *unit); -// ------------------------------------------------------------------------ -/** Load from a binary buffer stored in Flash */ -extern void UUSART_loadBinary(Unit *unit, PayloadParser *pp); -/** Write to a binary buffer for storing in Flash */ -extern void UUSART_writeBinary(Unit *unit, PayloadBuilder *pb); -// ------------------------------------------------------------------------ -/** Parse a key-value pair from the INI file */ -extern error_t UUSART_loadIni(Unit *unit, const char *key, const char *value); -/** Generate INI file section for the unit */ -extern void UUSART_writeIni(Unit *unit, IniWriter *iw); -// ------------------------------------------------------------------------ -/** Tear down the unit */ -extern void UUSART_deInit(Unit *unit); -/** Finalize unit set-up */ -extern error_t UUSART_init(Unit *unit); -// ------------------------------------------------------------------------ +/** + * Handle received data (we're inside the IRQ) + * + * @param unit - handled unit + * @param endpos - end position in the buffer + */ +void UUSART_DMA_HandleRxFromIRQ(Unit *unit, uint16_t endpos) +{ + assert_param(unit); + struct priv *priv = unit->data; + assert_param(priv); + + uint16_t readpos = priv->rx_buf_readpos; + + assert_param(endpos > readpos); + uint16_t count = (endpos - readpos); + uint8_t *start = (uint8_t *) (priv->rx_buffer + readpos); + + // Do something with the data... + PUTSN((char *) start, count); + PUTNL(); + + // Move the read cursor, wrap around if needed + if (endpos == UUSART_RXBUF_LEN) endpos = 0; + priv->rx_buf_readpos = endpos; +} + + +#if 0 static error_t usart_wait_until_flag(struct priv *priv, uint32_t flag, bool stop_state) { uint32_t t_start = HAL_GetTick(); @@ -50,6 +61,7 @@ static error_t sync_send(struct priv *priv, const uint8_t *buf, uint32_t len) TRY(usart_wait_until_flag(priv, USART_ISR_TC, 1)); return E_SUCCESS; } +#endif enum PinCmd_ { @@ -66,7 +78,17 @@ static error_t UUSART_handleRequest(Unit *unit, TF_ID frame_id, uint8_t command, uint32_t len; const uint8_t *pld = pp_tail(pp, &len); - TRY(sync_send(priv, pld, len)); + while (len > 0) { + uint16_t chunk = UUSART_DMA_TxQueue(priv, pld, (uint16_t) len); + pld += chunk; + len -= chunk; + + // We give up control if there's another thread waiting + if (len > 0) { + osThreadYield(); + } + } + return E_SUCCESS; //return E_NOT_IMPLEMENTED;