8 bit and 16 bit register writes

[dlaw]

There are a handful of STM32 peripheral registers which have different behavior depending on the byte width of the load or store instruction. The ones I've encountered are the SPI DR and CRC DR on the STM32G4.

The write_reg macro always generates a 32-bit write, causing the CRC or SPI peripheral to process 4 bytes of data. In order to write a single byte, I must cast the register address to *mut u8 and work with the resulting raw pointer. It would be nice if there was some way to perform this operation without losing the good ergonomics of write_reg. The same issue exists for read_reg when reading these registers.

// This needs to be an 8-bit write to the data register (to transmit 1 byte only).
// We can't use `write_reg` because it always generates 32-bit writes.
*(core::ptr::addr_of!((*spi::SPI2).DR) as *mut u8) = 0x52;

By the way, I'm using stm32ral on a production project because I find the ergonomics to be so much better than the svd2rust crates. Thanks for making it :slightly_smiling_face:

[adamgreig]

I'm glad you like it!

Yes, the DR access width is definitely a nuisance. It's very uncommon, so I don't want to make the normal experience worse. I think the best option long-term is to change away from generating a struct that directly memory-maps the peripheral and instead having methods on the struct which return pointers to the registers, which means you no longer have references into MMIO memory (helps avoid some questions around LLVM inserting dereferences) and can easily alias an 8, 16, and 32 bit register on top of each other. Then, you'd just use write_reg!(crc, dr_8, 123) or something like that.

It's a longer term plan though, so I'm open to a short term idea too. The best I can think of is adding an optional argument to the macro that specifies the write type, but I don't love the idea (the register size is already present in the peripheral struct after all) and the macros are already fairly complicated.

[dlaw]

My thinking for a short-term solution would be totally separate macros (write8_reg and read8_reg, or something like that) for generating byte-wide operations. I'm not sure how nicely the macro innards would work out (if you want to avoid blatant code duplication), but from a user perspective, that's what would make sense to me.

[jannic]

(core::ptr::addr_of!((spi::SPI2).DR) as *mut u8) = 0x52;

This is technically UB: While DR contains an UnsafeCell, and an UnsafeCell is writable (even through a shared reference), it's only allowed to write to it via .get(). Additionally, RWRegister<T> (the type of DR) is not even declared as repr(transparent), so there's no guarantee that RWRegister has the same memory layout as T.

[jannic]

so there's no guarantee that RWRegister has the same memory layout as T

Thinking about it, this is already an issue for the RegisterBlock definitions, isn't it? If the compiler for some reason added padding to RWRegister, which it is technically allowed to do, the layout of RegisterBlocks would change, even though the RegisterBlocks themselves are declared #[repr(C)]. That would obviously have catastrophic results.

Now, of course, there's little reason for the compiler to do such a thing. But it would still be better to add #[repr(C)] or #[repr(transparent)] to the register definitions in src/register.rs.