micropython / stm32lib

STM32 Cube library - CMSIS and HAL for F4, F7 and L4 microcontrollers
63 stars 71 forks source link

F4_HAL/uart: Fix data loss when using DMA Rx with hardware flow control. #12

Closed dlech closed 2 years ago

dlech commented 3 years ago

Data loss can occur when using DMA Rx + hardware flow control.

Consider the following situation where a device is receiving UART data that consists of one byte that gives the size of the data followed by "size" bytes:

This adds a check to see if flow control is enabled and only calls __HAL_UART_CLEAR_OREFLAG() if flow control is disabled. This should prevent breaking users who aren't using flow control and may already depend on this behavior.

dlech commented 3 years ago

FYI, this is a variation of part of the fixes from https://github.com/bluekitchen/btstack/commit/bef4d37c6d80c3f9083517fcb10ee119384a5614. Disabling IRQs is probably also needed, but I'm not sure the way it is done in the linked commit won't cause problems for existing users of MicroPython (i.e. interrupts may already be disabled).

dpgeorge commented 3 years ago

This looks OK to me. I assume you have tested it and it fixed the issue with lost chars on DMA RX?

Disabling IRQs is probably also needed

All F4 MCUs have a bit-band region (every bit is assigned a 32-bit word alias), and this can be used to make atomic changes to bits in peripheral registers. I think this would be a good way to solve that issue of SET_BIT not being atomic.

Eg:

#define PERIPH_BB_REG_ADDR(reg, bit_pos) (__IO uint32_t *)(PERIPH_BB_BASE + ((uintptr_t)&(reg) - PERIPH_BASE) * 32 + (bit_pos) * 4)
#define SET_BIT_PERIPH_BB(reg, bit_pos) (*(PERIPH_BB_REG_ADDR((reg), (bit_pos)) = 1)

{
    ...
    SET_BIT_PERIPH_BB(huart->Instance->CR1, USART_CR1_PEIE_Pos);
    ...
}

Note: the above macros are untested!

dpgeorge commented 3 years ago

Also, can you please retarget this to the new working branch work-F0-1.9.0+F4-1.16.0+F7-1.7.0+H7-1.6.0+L0-1.11.2+L4-1.8.1+WB-1.10.0 ?

dlech commented 3 years ago

Hi @dpgeorge. I have rebased and added the suggested bit-band macros. This has been tested and confirmed to fix the DMA issue.

dpgeorge commented 2 years ago

Sorry this got lost, now merged.