USART: support reading and writing 9-bit words, change to 32-bit data register access

[samcrow]

Motivation

In issue 280, Maldus512 asked how to use 9-bit words with the serial driver. I noticed that the current code did not allow that use, so I decided to improve the code.

Part 1: 16-bit reads and writes for 9-bit words

I added a WORD type parameter to Serial, Tx, and Rx. It defaults to u8, so that existing code will not break. I also added functions to convert between Serial<USART, PINS, u8> and Serial<USART, PINS, u16>.

I added some trait implementations that use u16 as the word type instead of u8:

• impl<USART> serial::Read<u16> for Rx<USART, u16>
• impl<USART> serial::Write<u16> for Tx<USART, u16>
• impl<USART> blocking::serial::Write<u16> for Tx<USART, u16>
• impl<USART, PINS> serial::Read<u16> for Serial<USART, PINS, u16>
• impl<USART, PINS> serial::Write<u16> for Serial<USART, PINS, u16>
• impl<USART, PINS> blocking::serial::Write<u16> for Serial<USART, PINS, u16>

I also added a simple example that demonstrates that the hardware can send and receive 9 bits.

Probably not a breaking change

Because the WORD type parameter has a default value of u8, existing code that does not specify WORD should continue to compile and work correctly. When WORD is not specified, Serial, Tx, and Rx implement the same traits as before.

Limitations

It is possible to configure a USART with 8-bit word length and then send/receive u16s. We could prevent this error, but it would require a major breaking change to the configuration code that is probably not worth it.

DMA with 16-bit words is not yet supported.

Part 2: 32-bit read and write operations on USART_DR

While working on part 1, I noticed that the current code goes out of its way to read or write only 8 bits when accessing the USART data register. Based on the reference manuals, this might be incorrect.

In most STM32F4 reference manuals, the beginning of the "USART registers" section says that "The peripheral registers have to be accessed by half-words (16 bits) or words (32 bits)."
The STM32F446 reference manual is more restrictive: "The peripheral registers have to be accessed by words (32 bits)."
The C drivers provided by ST appear to use 32-bit reads and writes on the data register.

Therefore, I have replaced the custom 8-bit reads and writes with 32-bit reads and writes that use the normal svd2rust API.

With this change applied, the serial example still works.

[burrbull]

I'm not sure what way is better. This one or that is used in:
stm32-rs/stm32f1xx-hal#319

[samcrow]

Thanks for bringing that up. Inspired by that version, I made some changes so that u8/u16 is a type parameter (u8 by default) and the serial traits are no longer implemented in an ambiguous way for the same type.

[therealprof]

With this change applied, the serial example still works.

On which chip did you test that? NB: There are STM peripherals which do weird things when you're trying to send 8bit with a 16bit or even 32bit write; I'm not sure that is the case for any F4 but it is something we should be careful about. Also F446 is one of the later added oddballs in the F4 family; though it seems to have the same serial peripheral version as older versions from the family according to: https://github.com/STMicroelectronics/STM32_open_pin_data/

[samcrow]

I have tested the serial example with these driver changes applied on an STM32F411, STM32F412, and STM32F469 (those are all the evaluation boards I have). I'm willing to set up and run some more tests, including tests for receiving.

I'm glad that we are being cautious about the methods used to access the data register. We shouldn't change something that works without a good reason. However, if the current approach is not fully correct and only works because of hardware implementation details, that is a good reason to change it.

As I will explain, the current approach of using 8-bit accesses to the USART data register is inconsistent with all applicable reference manuals and all non-Rust USART drivers that I have been able to find. This approach is risky and may not always work.

History

The history of 8-bit reads and writes on the data register goes back at least to May 2017, with the first commit to stm32f103xx-hal. Note that RM0008, the reference manual for the STM32F103 and related models, says that "the [USART] peripheral registers can be accessed by half-words (16-bit) or words (32-bit)."

This approach was clearly intentional, and there may have been a good reason for it. I would be happy to learn what the reason is, but I have not been able to find it yet.

Other peripherals

Some peripheral registers can be accessed using 8-bit reads and writes. For example, RM0090, the reference manual for several STM32F4 models, allows some 8-bit accesses:

• GPIO
• ADC (reads only; writes must be 32 bits)
• Timers (reads from 16-bit registers only; writes must be 16 or 32 bits; 32-bit registers must use 32-bit reads and writes)
• Ethernet

Other peripherals allow only 32-bit register access:

• CRC
• DCMI
• DMA
• DAC
• RNG
• RTC
• SDIO
• CAN
• OTG_FS
• OTG_HS
• Some FSMC registers

These restrictions tell us that 8-bit accesses to peripheral registers are not universally allowed. When in doubt, we should assume that a register should only be accessed with an instruction that matches its size (for example, 16-bit reads and writes on a 16-bit register).

In the USART case, there is no doubt: every STM32F4 reference manual except the STM32F446 clearly states that "The [USART] peripheral registers have to be accessed by half-words (16 bits) or words (32 bits)." The STM32F446 manual is even more restrictive, allowing 32-bit access only.

Other drivers

STMicroelectronics C LL

The "LL" C drivers provided by STMicroelectronics define the USART registers:

typedef struct
{
  __IO uint32_t SR;         /*!< USART Status register,                   Address offset: 0x00 */
  __IO uint32_t DR;         /*!< USART Data register,                     Address offset: 0x04 */
  __IO uint32_t BRR;        /*!< USART Baud rate register,                Address offset: 0x08 */
  __IO uint32_t CR1;        /*!< USART Control register 1,                Address offset: 0x0C */
  __IO uint32_t CR2;        /*!< USART Control register 2,                Address offset: 0x10 */
  __IO uint32_t CR3;        /*!< USART Control register 3,                Address offset: 0x14 */
  __IO uint32_t GTPR;       /*!< USART Guard time and prescaler register, Address offset: 0x18 */
} USART_TypeDef;

Here are its receive and transmit functions:

__STATIC_INLINE uint8_t LL_USART_ReceiveData8(USART_TypeDef *USARTx)
{
  return (uint8_t)(READ_BIT(USARTx->DR, USART_DR_DR));
}
__STATIC_INLINE void LL_USART_TransmitData8(USART_TypeDef *USARTx, uint8_t Value)
{
  USARTx->DR = Value;
}

The data register DR is a 32-bit field. The code accesses it directly, never using a pointer to a smaller integer. The READ_BIT(USARTx->DR, USART_DR_DR) macro expands to USARTx->DR & 0x1ff.

STMicroelectronics C HAL

The "HAL" C drivers provided by STMicroelectronics use the same USART_TypeDef. Its receive and transmit functions are too long to copy here, but they access the data register like this:

• huart->Instance->DR = (*tmp & (uint16_t)0x01FF);
• huart->Instance->DR = (*pData++ & (uint8_t)0xFF);
• *tmp = (uint16_t)(huart->Instance->DR & (uint16_t)0x01FF);
• *tmp = (uint16_t)(huart->Instance->DR & (uint16_t)0x00FF);
• *pData++ = (uint8_t)(huart->Instance->DR & (uint8_t)0x00FF);
• *pData++ = (uint8_t)(huart->Instance->DR & (uint8_t)0x007F);

Again, they use 32-bit accesses to the data register without any pointer casts.

RT-Thread

This slightly different USART driver uses the same USART_TypeDef and accesses the data register like this:

• uart->uart_device->DR = c;
• ch = uart->uart_device->DR & 0xff;

libopencm3

The libopencm3 USART drivers use the USART_DR macro to access the data register, defined as:

#define USART_DR(usart_base)		MMIO32((usart_base) + 0x04)

MMIO32 is:

#define MMIO32(addr)		(*(volatile uint32_t *)(addr))

This clearly compiles down to 32-bit reads and writes.

The current approach works (sometimes)

It is clear that 8-bit accesses to the USART data registers do work in the cases that people have tested so far. The problem is that this approach is not guaranteed to work. Any new STM32F4 model, or a new revision of a current model, might not work correctly. There may even be a USART peripheral on some revision of some current STM32F4 model where our existing approach corrupts data or causes a bus error. We do not know about that case because we have not exhaustively tested every possible configuration.

We are essentially relying on undefined behavior of hardware, and placing ourselves at the mercy of STMicroelectronics and the tools they use to design their products.

Conclusion

The current approach of 8-bit accesses to the USART data registers has a long history but no clear explanation for how it came to be. The reference manuals clearly state that this approach is not supported. No other major driver code accesses the data register this way.

The people who use this library are counting on us to provide code that works. Many of us are contributing in our free time with limited access to hardware, so we don't always get everything right. However, we should at least follow the example of other working drivers and work within the restrictions laid out in the reference manuals.

What this change means in the compiled code

loop {
    nb::block!(tx.write(0x99)).unwrap();
}

The above code (following the normal USART initialization) compiles to:

Previous

 8000356:       2199            movs    r1, #153        ; 0x99
 8000358:       6802            ldr     r2, [r0, #0]
 800035a:       0612            lsls    r2, r2, #24
 800035c:       d5fc            bpl.n   8000358 <serial_simple::__cortex_m_rt_main+0x116>
 800035e:       7101            strb    r1, [r0, #4] ; Write to data register
 8000360:       e7fa            b.n     8000358 <serial_simple::__cortex_m_rt_main+0x116>

Changed

 8000356:       2199            movs    r1, #153        ; 0x99
 8000358:       6802            ldr     r2, [r0, #0]
 800035a:       0612            lsls    r2, r2, #24
 800035c:       d5fc            bpl.n   8000358 <serial_simple::__cortex_m_rt_main+0x116>
 800035e:       6041            str     r1, [r0, #4] ; Write to data register
 8000360:       e7fa            b.n     8000358 <serial_simple::__cortex_m_rt_main+0x116>

The only difference is that the data register write instruction is a str (32 bits) instead of an strb (8 bits).

[burrbull]

Looks good to me.

Therefore, I have replaced the custom 8-bit reads and writes with 32-bit reads and writes that use the normal svd2rust API.

Add this to changelog

[burrbull]

bors r+

[bors]

👎 Rejected by too few approved reviews

[burrbull]

bors retry

[bors]

Build succeeded:

• ci (stm32f401, stable)
• ci (stm32f405, stable)
• ci (stm32f407, stable)
• ci (stm32f410, stable)
• ci (stm32f411, stable)
• ci (stm32f412, stable)
• ci (stm32f413, stable)
• ci (stm32f415, stable)
• ci (stm32f417, stable)
• ci (stm32f423, stable)
• ci (stm32f427, stable)
• ci (stm32f429, stable)
• ci (stm32f437, stable)
• ci (stm32f439, stable)
• ci (stm32f446, stable)
• ci (stm32f469, stable)
• ci (stm32f479, stable)
• Rustfmt

[therealprof]

@samcrow No worries! Thanks for the elaborate description and research; for me a simple "I've tested this with a few different boards and a few others like ... are doing it the same way" would have sufficed. 😅

[samcrow]

Thanks; my comment was a little more argumentative than necessary. Now I know more about what kind of justification we expect for changes.