Fixed serial_device IRQ infinite loop bug due to uint8_t overflowing in STM devices

[caoyuan96421]

Description

See
#11031 (comment)

Fixed now by changing the variable type from uint8_t to size_t (which is supposedly 32-bit unsigned).

Applies to STM32F0, F1, F2, F3, F4, F7, H7, L0, L1, L4, WB series.

Pull request type

[X] Fix
[ ] Refactor
[ ] Target update
[ ] Functionality change
[ ] Docs update
[ ] Test update
[ ] Breaking change

Reviewers

@jeromecoutant

Release Notes

[ciarmcom]

@caoyuan96421, thank you for your changes.
@ARMmbed/mbed-os-maintainers please review.

[kjbracey]

Fix seems correct, but I'll note a couple of other things in passing about the code being touched:

• That i really should be being declared in the for statement itself. Was a long search through the diff to find out what it was for
• The volatile on return_event is pointless, and could well be making the compiler produce very silly unoptimised code. Wouldn't be at all surprised if taking it off made the image nearly 100 bytes smaller.
• The register reading itself is also very inefficient - the flag register is read multiple times, and the IT source register potentially. All those __HAL_UART operations are real hardware reads - they really should be read once into a local variable in routines like this. A good idea both for efficiency, and to help atomicity.

First two are easy tweaks, at least - the last is a general pattern throughout the code.

[jeromecoutant]

UART FPGA test OK:

target	platform_name	test suite	result	elapsed_time (sec)	copy_method
NUCLEO_F429ZI-ARMC6	NUCLEO_F429ZI	tests-mbed_hal_fpga_ci_test_shield-uart	OK	21.09	default
NUCLEO_L4R5ZI_P-ARMC6	NUCLEO_L4R5ZI_P	tests-mbed_hal_fpga_ci_test_shield-uart	OK	21.25	default

[artokin]

CI started.

[mbed-ci]

Test run: SUCCESS

Summary: 11 of 11 test jobs passed
Build number : 1
Build artifacts

[caoyuan96421]

Fix seems correct, but I'll note a couple of other things in passing about the code being touched:

• That i really should be being declared in the for statement itself. Was a long search through the diff to find out what it was for
• The volatile on return_event is pointless, and could well be making the compiler produce very silly unoptimised code. Wouldn't be at all surprised if taking it off made the image nearly 100 bytes smaller.
• The register reading itself is also very inefficient - the flag register is read multiple times, and the IT source register potentially. All those __HAL_UART operations are real hardware reads - they really should be read once into a local variable in routines like this. A good idea both for efficiency, and to help atomicity.

First two are easy tweaks, at least - the last is a general pattern throughout the code.

About the third point, ideally all those flag checks should only happen in the call to HAL_UART_IRQHandler in the STM32Cube. My understanding is that those extra checks are necessary to set the event flags of the serial driver before they are cleared by HAL_UART_IRQHandler. If the status registers are to read really only once, we'll have to realize a good portion of the logic in HAL_UART_IRQHandler into serial_device.c .

[kjbracey]

I've not studied this serial code in detail to draw any conclusion on the broader structure, but I'm aware that this is an area where there is certainly scope for problems if the accesses are done non-atomically, or in the wrong sequence. (Eg clearing an ISR latch after calling a IRQ handler).

I'm not saying there's a functional problem here, it's just a note that the STM HAL is very "bloated" in size compared to some other platforms', and part of the cause is this idiom of using macros that wrap up hardware reads - registers are often read or written multiple times, once for each flag bit or status field. Reducing adjacent reads is worthwhile just on code size grounds.