ARMmbed / mbed-os

Arm Mbed OS is a platform operating system designed for the internet of things
https://mbed.com
Other
4.65k stars 2.97k forks source link

STM32L0 UART IRQ infinitely retriggered #3452

Closed bisqwit closed 7 years ago

bisqwit commented 7 years ago

I am having trouble where uart2_irq gets triggered, but never acknowledged, causing the IRQ being immediately triggered again when it exits, with user code never getting chance to progress.

I single-step the IRQ handler, and I see it tests and sees that UART_FLAG_ORE is set. Then it tests whether UART_IT_ORE is set, and it’s not, so it never clears the ORE condition. And the IRQ gets never acknowledged, and it therefore gets triggered again, infinitely.

I made a comparison on how uart_irq() is written in all the different STM32 targets, and I see they are quite different on each CPU. Should they be? Should it perchance be testing USART_IT_ERR instead of UART_IT_ORE, on L0, like it is doing on F4, and like it is actually doing in serial_irq_handler_asynch() on L0?

--- TARGET_STM32L0 --- 
static void uart_irq(int id)
{
    UART_HandleTypeDef * huart = &uart_handlers[id];

    if (serial_irq_ids[id] != 0) {
        if (__HAL_UART_GET_FLAG(huart, UART_FLAG_TC) != RESET) {
            if (__HAL_UART_GET_IT_SOURCE(huart, UART_IT_TC) != RESET) {
                irq_handler(serial_irq_ids[id], TxIrq);
                __HAL_UART_CLEAR_FLAG(huart, UART_CLEAR_TCF);
            }
        }
        if (__HAL_UART_GET_FLAG(huart, UART_FLAG_RXNE) != RESET) {
            if (__HAL_UART_GET_IT_SOURCE(huart, UART_IT_RXNE) != RESET) {
                irq_handler(serial_irq_ids[id], RxIrq);
                volatile uint32_t tmpval = huart->Instance->RDR; // Clear RXNE flag
            }
        }
        if (__HAL_UART_GET_FLAG(huart, UART_FLAG_ORE) != RESET) {
            if (__HAL_UART_GET_IT_SOURCE(huart, UART_IT_ORE) != RESET) {
                __HAL_UART_CLEAR_FLAG(huart, UART_CLEAR_OREF);
            }
        }
    }
}
--- TARGET_STM32L1 --- 
static void uart_irq(int id)
{
    UART_HandleTypeDef * huart = &uart_handlers[id];

    if (serial_irq_ids[id] != 0) {
        if (__HAL_UART_GET_FLAG(huart, UART_FLAG_TC) != RESET) {
            if (__HAL_UART_GET_IT_SOURCE(huart, UART_IT_TC) != RESET) {
                irq_handler(serial_irq_ids[id], TxIrq);
                __HAL_UART_CLEAR_FLAG(huart, UART_FLAG_TC);
            }
        }
        if (__HAL_UART_GET_FLAG(huart, UART_FLAG_RXNE) != RESET) {
            if (__HAL_UART_GET_IT_SOURCE(huart, UART_IT_RXNE) != RESET) {
                irq_handler(serial_irq_ids[id], RxIrq);
                __HAL_UART_CLEAR_FLAG(huart, UART_FLAG_RXNE);
            }
        }
        if (__HAL_UART_GET_FLAG(huart, UART_FLAG_ORE) != RESET) {
            if (__HAL_UART_GET_IT_SOURCE(huart, UART_IT_ERR) != RESET) {
                volatile uint32_t tmpval = huart->Instance->DR; // Clear ORE flag
            }
        }
    }
}
--- TARGET_STM32L4 --- 
static void uart_irq(int id)
{
    UART_HandleTypeDef * huart = &uart_handlers[id];

    if (serial_irq_ids[id] != 0) {
        if (__HAL_UART_GET_FLAG(huart, UART_FLAG_TC) != RESET) {
            if (__HAL_UART_GET_IT_SOURCE(huart, UART_IT_TC) != RESET) {
            irq_handler(serial_irq_ids[id], TxIrq);
                __HAL_UART_CLEAR_FLAG(huart, UART_FLAG_TC);
            }
        }
        if (__HAL_UART_GET_FLAG(huart, UART_FLAG_RXNE) != RESET) {
            if (__HAL_UART_GET_IT_SOURCE(huart, UART_IT_RXNE) != RESET) {
            irq_handler(serial_irq_ids[id], RxIrq);
                __HAL_UART_CLEAR_FLAG(huart, UART_FLAG_RXNE);
            }
        }
        if (__HAL_UART_GET_FLAG(huart, UART_FLAG_ORE) != RESET) {
            if (__HAL_UART_GET_IT_SOURCE(huart, UART_IT_ERR) != RESET) {
                volatile uint32_t tmpval = huart->Instance->RDR; // Clear ORE flag
            }
        }
    }
}
--- TARGET_STM32F0 --- 
static void uart_irq(int id)
{
    UART_HandleTypeDef * huart = &uart_handlers[id];

    if (serial_irq_ids[id] != 0) {
        if (__HAL_UART_GET_FLAG(huart, UART_FLAG_TC) != RESET) {
            if (__HAL_UART_GET_IT_SOURCE(huart, UART_IT_TC) != RESET) {
                irq_handler(serial_irq_ids[id], TxIrq);
                __HAL_UART_CLEAR_FLAG(huart, UART_CLEAR_TCF);
            }
        }
        if (__HAL_UART_GET_FLAG(huart, UART_FLAG_RXNE) != RESET) {
            if (__HAL_UART_GET_IT_SOURCE(huart, UART_IT_RXNE) != RESET) {
                irq_handler(serial_irq_ids[id], RxIrq);
                volatile uint32_t tmpval = huart->Instance->RDR; // Clear RXNE flag
            }
        }
        if (__HAL_UART_GET_FLAG(huart, UART_FLAG_ORE) != RESET) {
            if (__HAL_UART_GET_IT_SOURCE(huart, UART_IT_ORE) != RESET) {
                __HAL_UART_CLEAR_FLAG(huart, UART_CLEAR_OREF);
            }
        }
    }
}
--- TARGET_STM32F1 --- 
static void uart_irq(int id)
{
    UART_HandleTypeDef * huart = &uart_handlers[id];

    if (serial_irq_ids[id] != 0) {
        if (__HAL_UART_GET_FLAG(huart, UART_FLAG_TC) != RESET) {
            if (__HAL_UART_GET_IT_SOURCE(huart, UART_IT_TC) != RESET) {
                irq_handler(serial_irq_ids[id], TxIrq);
                __HAL_UART_CLEAR_FLAG(huart, UART_FLAG_TC);
            }
        }
        if (__HAL_UART_GET_FLAG(huart, UART_FLAG_RXNE) != RESET) {
            if (__HAL_UART_GET_IT_SOURCE(huart, UART_IT_RXNE) != RESET) {
                irq_handler(serial_irq_ids[id], RxIrq);
                __HAL_UART_CLEAR_FLAG(huart, UART_FLAG_RXNE);
            }
        }
        if (__HAL_UART_GET_FLAG(huart, UART_FLAG_ORE) != RESET) {
            if (__HAL_UART_GET_IT_SOURCE(huart, UART_IT_ERR) != RESET) {
                volatile uint32_t tmpval = huart->Instance->DR; // Clear ORE flag
            }
        }
    }
}
--- TARGET_STM32F2 --- 
static void uart_irq(int id)
{
    UART_HandleTypeDef * huart = &uart_handlers[id];

    if (serial_irq_ids[id] != 0) {
        if (__HAL_UART_GET_FLAG(huart, UART_FLAG_TC) != RESET) {
            if (__HAL_UART_GET_IT_SOURCE(huart, UART_IT_TC) != RESET) {
                irq_handler(serial_irq_ids[id], TxIrq);
                __HAL_UART_CLEAR_FLAG(huart, UART_FLAG_TC);
            }
        }
        if (__HAL_UART_GET_FLAG(huart, UART_FLAG_RXNE) != RESET) {
            if (__HAL_UART_GET_IT_SOURCE(huart, UART_IT_RXNE) != RESET) {
                irq_handler(serial_irq_ids[id], RxIrq);
                __HAL_UART_CLEAR_FLAG(huart, UART_FLAG_RXNE);
            }
        }
        if (__HAL_UART_GET_FLAG(huart, UART_FLAG_ORE) != RESET) {
            if (__HAL_UART_GET_IT_SOURCE(huart, USART_IT_ERR) != RESET) {
                volatile uint32_t tmpval = huart->Instance->DR; // Clear ORE flag
            }
        }
    }
}
--- TARGET_STM32F3 --- 
static void uart_irq(int id)
{
    UART_HandleTypeDef * huart = &uart_handlers[id];

    if (serial_irq_ids[id] != 0) {
        if (__HAL_UART_GET_FLAG(huart, UART_FLAG_TC) != RESET) {
            if (__HAL_UART_GET_IT_SOURCE(huart, UART_IT_TC) != RESET) {
            irq_handler(serial_irq_ids[id], TxIrq);
                __HAL_UART_CLEAR_FLAG(huart, UART_CLEAR_TCF);
        }
        }
        if (__HAL_UART_GET_FLAG(huart, UART_FLAG_RXNE) != RESET) {
            if (__HAL_UART_GET_IT_SOURCE(huart, UART_IT_RXNE) != RESET) {
            irq_handler(serial_irq_ids[id], RxIrq);
                volatile uint32_t tmpval = huart->Instance->RDR; // Clear RXNE flag
                UNUSED(tmpval);
            }
        }
        if (__HAL_UART_GET_FLAG(huart, UART_FLAG_ORE) != RESET) {
            if (__HAL_UART_GET_IT_SOURCE(huart, UART_IT_ORE) != RESET) {
                __HAL_UART_CLEAR_FLAG(huart, UART_CLEAR_OREF);
            }
        }
    }
}
--- TARGET_STM32F4 --- 
static void uart_irq(int id)
{
    UART_HandleTypeDef * huart = &uart_handlers[id];

    if (serial_irq_ids[id] != 0) {
        if (__HAL_UART_GET_FLAG(huart, UART_FLAG_TC) != RESET) {
            if (__HAL_UART_GET_IT_SOURCE(huart, UART_IT_TC) != RESET) {
                irq_handler(serial_irq_ids[id], TxIrq);
                __HAL_UART_CLEAR_FLAG(huart, UART_FLAG_TC);
            }
        }
        if (__HAL_UART_GET_FLAG(huart, UART_FLAG_RXNE) != RESET) {
            if (__HAL_UART_GET_IT_SOURCE(huart, UART_IT_RXNE) != RESET) {
                irq_handler(serial_irq_ids[id], RxIrq);
                __HAL_UART_CLEAR_FLAG(huart, UART_FLAG_RXNE);
            }
        }
        if (__HAL_UART_GET_FLAG(huart, UART_FLAG_ORE) != RESET) {
            if (__HAL_UART_GET_IT_SOURCE(huart, USART_IT_ERR) != RESET) {
                volatile uint32_t tmpval = huart->Instance->DR; // Clear ORE flag
            }
        }
    }
}
--- TARGET_STM32F7 --- 
static void uart_irq(int id)
{
    UART_HandleTypeDef * huart = &uart_handlers[id];

    if (serial_irq_ids[id] != 0) {
        if (__HAL_UART_GET_FLAG(huart, UART_FLAG_TC) != RESET) {
            if (__HAL_UART_GET_IT_SOURCE(huart, UART_IT_TC) != RESET) {
                irq_handler(serial_irq_ids[id], TxIrq);
                __HAL_UART_CLEAR_IT(huart, UART_CLEAR_TCF);
            }
        }
        if (__HAL_UART_GET_FLAG(huart, UART_FLAG_RXNE) != RESET) {
            if (__HAL_UART_GET_IT_SOURCE(huart, UART_IT_RXNE) != RESET) {
                irq_handler(serial_irq_ids[id], RxIrq);
                volatile uint32_t tmpval = huart->Instance->RDR; // Clear RXNE
            }
        }
        if (__HAL_UART_GET_FLAG(huart, UART_FLAG_ORE) != RESET) {
            if (__HAL_UART_GET_IT_SOURCE(huart, UART_IT_ORE) != RESET) {
                __HAL_UART_CLEAR_IT(huart, UART_CLEAR_OREF);
            }
        }
    }
}
0xc0170 commented 7 years ago

cc @bcostm @adustm @LMESTM @jeromecoutant

LMESTM commented 7 years ago

@bisqwit thanks for reporting the issue and sorry for not coming back to you earlier.

After a quick check, I noticed that there is a misalignment between some of MACROs definition of HAL_UART_GET_IT_SOURCE and HAL_UART_GET_IT. In case of L0 HAL_UART_GET_IT_SOURCE Checks whether the specified UART interrupt source is enabled HAL_UART_GET_IT Checks whether the specified UART interrupt has occurred or not so it seems that we may not be using the proper MACRO

Before proposing a fix, I'd like to reproduce your issue and make sure I will solve it. May you please provide come sample code and describe the use case where you tested so that I can easily reproduce the issue ?

LMESTM commented 7 years ago

@bisqwit I could actually build a simple example where I think that I can reproduce the same issue as you were describing. I unblocked the situation by changing :

$ git diff targets/TARGET_STM/TARGET_STM32L0/serial_device.c
--- a/targets/TARGET_STM/TARGET_STM32L0/serial_device.c
+++ b/targets/TARGET_STM/TARGET_STM32L0/serial_device.c
@@ -214,7 +214,7 @@ static void uart_irq(int id)
             }
         }
         if (__HAL_UART_GET_FLAG(huart, UART_FLAG_ORE) != RESET) {
-            if (__HAL_UART_GET_IT_SOURCE(huart, UART_IT_ORE) != RESET) {
+            if (__HAL_UART_GET_IT(huart, UART_IT_ORE) != RESET) {
                 __HAL_UART_CLEAR_FLAG(huart, UART_CLEAR_OREF);
             }
         }

Would be great if you could confirm it helps on your side - I will anyway post the patch accordingly.

LMESTM commented 7 years ago

Fix pushed for review and test : #4707

LMESTM commented 7 years ago

4707 has been merged and should fix the issue. Please close or share updates if any issue.

ST_TO_BE_CLOSED