On 9/15/26 06:58, Peter Maydell wrote:
> On Mon, 17 Aug 2026 at 06:03, Jacob Whitaker Abrams
> <[email protected]> wrote:
>> Official STM32CubeL4 drivers use and require support for 16-bit writes to
>> UART registers for proper function.
> This is a breach of the datasheet (assuming I have the right one:
> RM0351 "STM32L47xxx, STM32L48xxx, STM32L49xxx and STM32L4Axxx
> advanced Arm®-based 32-bit MCUs"), which is pretty clear:
> "The peripheral registers have to be accessed by words (32 bits)".
>
> Still, if the official drivers are doing it then presumably the
> hardware actually does allow smaller accesses.
>
> Do the drivers do small accesses to the top parts of registers,
> or do they always use 4-aligned addresses but just sometimes
> do smaller width accesses ? If only the latter is needed, we
> can make the code simpler (reads return the full register value
> and the QEMU core code chops off the unneeded high parts, writes
> are "write as if zero-extended").
Yes it is a documentation error, we discussed this in 2024, Nicolas Fillon at
STM wrote back to me "I see we are making 16 bit access read and write to these
16 bit registers in our library for both HAL and LL so this should be a
documentation issue."
It appears specifically to affect the USART peripheral inside the chip, here is
the official C struct they use in their source code, it contains uint16_t sized
registers resulting in 16-bit memory assembly operations:
typedef struct
{
__IO uint32_t CR1; /*!< USART Control register 1,
Address offset: 0x00 */
__IO uint32_t CR2; /*!< USART Control register 2,
Address offset: 0x04 */
__IO uint32_t CR3; /*!< USART Control register 3,
Address offset: 0x08 */
__IO uint32_t BRR; /*!< USART Baud rate register,
Address offset: 0x0C */
__IO uint16_t GTPR; /*!< USART Guard time and prescaler register,
Address offset: 0x10 */
uint16_t RESERVED2; /*!< Reserved, 0x12
*/
__IO uint32_t RTOR; /*!< USART Receiver Time Out register,
Address offset: 0x14 */
__IO uint16_t RQR; /*!< USART Request register,
Address offset: 0x18 */
uint16_t RESERVED3; /*!< Reserved, 0x1A
*/
__IO uint32_t ISR; /*!< USART Interrupt and status register,
Address offset: 0x1C */
__IO uint32_t ICR; /*!< USART Interrupt flag Clear register,
Address offset: 0x20 */
__IO uint16_t RDR; /*!< USART Receive Data register,
Address offset: 0x24 */
uint16_t RESERVED4; /*!< Reserved, 0x26
*/
__IO uint16_t TDR; /*!< USART Transmit Data register,
Address offset: 0x28 */
uint16_t RESERVED5; /*!< Reserved, 0x2A
*/
__IO uint32_t PRESC; /*!< USART Prescaler register,
Address offset: 0x2C */
} USART_TypeDef;
>> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2540
>> Signed-off-by: Jacob Whitaker Abrams <[email protected]>
>> ---
>> hw/char/stm32l4x5_usart.c | 93 +++++++++++++++++++++++-------
>> tests/qtest/stm32l4x5_usart-test.c | 50 +++++++++++++++-
>> 2 files changed, 120 insertions(+), 23 deletions(-)
>>
>> diff --git a/hw/char/stm32l4x5_usart.c b/hw/char/stm32l4x5_usart.c
>> index dd1b099195..88c4a3b5a7 100644
>> --- a/hw/char/stm32l4x5_usart.c
>> +++ b/hw/char/stm32l4x5_usart.c
>> @@ -154,8 +154,15 @@ REG32(RDR, 0x24)
>> REG32(TDR, 0x28)
>> FIELD(TDR, TDR, 0, 9)
>>
>> +#define ISR_RESET_VALUE (0x020000C0)
>> +
>> static void stm32l4x5_update_isr(Stm32l4x5UsartBaseState *s)
>> {
>> + if (!(s->cr1 & R_CR1_UE_MASK)) {
>> + s->isr = ISR_RESET_VALUE;
>> + return;
>> + }
> This seems to be an unrelated change to adding 16-bit access handling ?
Fair point, I will remove that from the upcoming v2 patch.
>> +
>> if (s->cr1 & R_CR1_TE_MASK) {
>> s->isr |= R_ISR_TEACK_MASK;
>> } else {
>> @@ -404,9 +411,11 @@ static uint64_t stm32l4x5_usart_base_read(void *opaque,
>> hwaddr addr,
>> unsigned int size)
>> {
>> Stm32l4x5UsartBaseState *s = opaque;
>> + hwaddr base = addr & ~0x3ULL;
>> + unsigned int offset = addr & 0x3;
>> uint64_t retvalue = 0;
>>
>> - switch (addr) {
>> + switch (base) {
>> case A_CR1:
>> retvalue = s->cr1;
>> break;
>> @@ -451,6 +460,13 @@ static uint64_t stm32l4x5_usart_base_read(void *opaque,
>> hwaddr addr,
>> break;
>> }
>>
>> + /* Adjust for partial access */
>> + if (size == 1) {
>> + retvalue = (retvalue >> (offset * 8)) & 0xFF;
>> + } else if (size == 2) {
>> + retvalue = (retvalue >> (offset * 8)) & 0xFFFF;
>> + }
> You can avoid the if():
>
> retvalue = extract32(retvalue, offset * 8, size * 8);
OK I will do so.
>> +
>> trace_stm32l4x5_usart_read(addr, retvalue);
>>
>> return retvalue;
>> @@ -460,55 +476,88 @@ static void stm32l4x5_usart_base_write(void *opaque,
>> hwaddr addr,
>> uint64_t val64, unsigned int size)
>> {
>> Stm32l4x5UsartBaseState *s = opaque;
>> - const uint32_t value = val64;
>> + hwaddr base = addr & ~0x3ULL;
>> + unsigned int offset = addr & 0x3;
>> + uint32_t value = (uint32_t)val64;
>> +
>> + /* Build mask for partial access */
>> + uint32_t mask;
>> + if (size == 4) {
>> + mask = 0xFFFFFFFF;
>> + } else if (size == 2) {
>> + mask = 0xFFFF << (offset * 8);
>> + } else if (size == 1) {
>> + mask = 0xFF << (offset * 8);
>> + } else {
>> + qemu_log_mask(LOG_GUEST_ERROR, "%s: Unsupported access size %u\n",
>> __func__, size);
>> + return;
>> + }
>> + value = (value << (offset * 8)) & mask;
>>
>> trace_stm32l4x5_usart_write(addr, value);
>>
>> - switch (addr) {
>> - case A_CR1:
>> - s->cr1 = value;
>> + switch (base) {
>> + case A_CR1: {
>> + uint32_t old = s->cr1;
>> + s->cr1 = (old & ~mask) | value;
> If you write these as
> s->cr1 = deposit32(s->cr1, value, offset * 8, size * 8);
>
> then you don't need to calculate a mask or shift-and-mask
> the value by hand.
OK I will.
Regards,
Jacob Abrams
