Cerberus Firmware : StopBits argument ignored in usart initialization function

Checking the native code for Cerb-family, in STM32F4_usart_functions.cpp,
The configuration function for USART, StopBits argument is completely ignored and always configures the usart for one stop bit. Unless I’m missing something huge elsewhere in the code, I fail to see how we can configure usart with two stop bits. Is it a bug or a feature ? :slight_smile:


BOOL CPU_USART_Initialize( int ComPortNum, int BaudRate, int Parity, int DataBits, int StopBits, int FlowValue )
{
    if (ComPortNum >= TOTAL_USART_PORT) return FALSE;
    if (Parity >= USART_PARITY_MARK) return FALSE;
    
    GLOBAL_LOCK(irq);
    
    ptr_USART_TypeDef uart = g_STM32F4_Uart_Ports[ComPortNum];
    UINT32 clk;
    
    // enable UART clock
    if (ComPortNum == 5) { // COM6 on APB2
        RCC->APB2ENR |= RCC_APB2ENR_USART6EN;
        clk = SYSTEM_APB2_CLOCK_HZ;
    } else if (ComPortNum) { // COM2-5 on APB1
        RCC->APB1ENR |= RCC_APB1ENR_USART2EN >> 1 << ComPortNum;
        clk = SYSTEM_APB1_CLOCK_HZ;
    } else { // COM1 on APB2
        // USART1 is not used on Cerberus but USART6 is used on Cerbuino.
        // This is just for Cerbuino. 
        //RCC->APB2ENR |= RCC_APB2ENR_USART1EN;
        //clk = SYSTEM_APB2_CLOCK_HZ;
        RCC->APB2ENR |= RCC_APB2ENR_USART6EN;
        clk = SYSTEM_APB2_CLOCK_HZ;
    }
    
    //  baudrate
    UINT16 div = (UINT16)((clk + (BaudRate >> 1)) / BaudRate); // rounded
    uart->BRR = div;
    
    // control
    UINT16 ctrl = USART_CR1_TE | USART_CR1_RE;
    if (DataBits == 9) ctrl |= USART_CR1_M;
    if (Parity) ctrl |= USART_CR1_PCE;
    if (Parity == USART_PARITY_ODD) ctrl |= USART_CR1_PS;
    uart->CR1 = ctrl;
    
    if (DataBits == USART_STOP_BITS_ONE) DataBits = 0;
    uart->CR2 = (UINT16)(DataBits << 12);
    
    ctrl = 0;
    if (FlowValue & USART_FLOW_HW_OUT_EN) ctrl |= USART_CR3_CTSE;
    if (FlowValue & USART_FLOW_HW_IN_EN)  ctrl |= USART_CR3_RTSE;
    uart->CR3 = ctrl;

    GPIO_PIN rxPin, txPin, ctsPin, rtsPin;
    CPU_USART_GetPins(ComPortNum, rxPin, txPin, ctsPin, rtsPin);
    UINT32 alternate = 0x72; // AF7 = USART1-3
    //if (ComPortNum >= 3) alternate = 0x82; // AF8 = UART4-6
    if (ComPortNum == 0) alternate = 0x82; // AF8 = UART4-6 -->> Only because USART1 is really USART6
    CPU_GPIO_DisablePin(rxPin, RESISTOR_PULLUP, 0, (GPIO_ALT_MODE)alternate);
    CPU_GPIO_DisablePin(txPin, RESISTOR_DISABLED, 1, (GPIO_ALT_MODE)alternate);
    if (FlowValue & USART_FLOW_HW_OUT_EN)
        if (ctsPin == GPIO_PIN_NONE) return FALSE;
        CPU_GPIO_DisablePin(ctsPin, RESISTOR_DISABLED, 0, (GPIO_ALT_MODE)alternate);
    if (FlowValue & USART_FLOW_HW_IN_EN)
        if (rtsPin == GPIO_PIN_NONE) return FALSE;
        CPU_GPIO_DisablePin(rtsPin, RESISTOR_DISABLED, 1, (GPIO_ALT_MODE)alternate);

    CPU_USART_ProtectPins(ComPortNum, FALSE);
    
    switch (ComPortNum) {
    //case 0: CPU_INTC_ActivateInterrupt(USART1_IRQn, STM32F4_USART_Interrupt0, 0); break;
    case 0: CPU_INTC_ActivateInterrupt(USART6_IRQn, STM32F4_USART_Interrupt0, 0); break;
    case 1: CPU_INTC_ActivateInterrupt(USART2_IRQn, STM32F4_USART_Interrupt1, 0); break;
    case 2: CPU_INTC_ActivateInterrupt(USART3_IRQn, STM32F4_USART_Interrupt2, 0); break;
    case 3: CPU_INTC_ActivateInterrupt(UART4_IRQn, STM32F4_USART_Interrupt3, 0); break;
    case 4: CPU_INTC_ActivateInterrupt(UART5_IRQn, STM32F4_USART_Interrupt4, 0); break;
    case 5: CPU_INTC_ActivateInterrupt(USART6_IRQn, STM32F4_USART_Interrupt5, 0);
    }

    uart->CR1 |= USART_CR1_UE; // start uart

    return TRUE;
}

This

 if (DataBits == USART_STOP_BITS_ONE) DataBits = 0;
    uart->CR2 = (UINT16)(DataBits << 12);

should be replaced by :

if (StopBits == USART_STOP_BITS_ONE) StopBits = 0;
    uart->CR2 = (UINT16)(StopBits << 12);

To allow different stopbits than one.
Where to report this ?

1 Like

You just did! :slight_smile:

GHI will see it and will fix it if it is an error.

the 9 bits DataBits configuration is wrong too. Someone ought to be carefully reviewing what the function does, because it breaks several things for “unusual” usart setups.
I’m still waiting for my logic analyzer to prove all of this.

1 Like

We will look into this, thanks

fixed in latest firmware thanks