Implement the ability to work with `W` outside of `write`

[AndreySmirnov81]

I want for example instead of this

use stm32f1::stm32f103 as pac;
let periph = pac::Peripherals::take().unwrap();
let usart1 = periph.USART1;
// ...
usart1.cr1.write(|w| {
    w.ue().enabled();
    w.m().m9();
    w.pce().enabled();
    w.ps().even();
    w.te().enabled();
    w.tcie().enabled();
    w.re().disabled(); // <- First write
    w
});
// ...
usart1.cr1.write(|w| {
    w.ue().enabled();
    w.m().m9();
    w.pce().enabled();
    w.ps().even();
    w.te().enabled();
    w.tcie().enabled();
    w.re().enabled(); // <- Modified
    w
});
// ...
usart1.cr1.write(|w| {
    w.ue().enabled();
    w.m().m9();
    w.pce().enabled();
    w.ps().even();
    w.te().enabled();
    w.tcie().enabled();
    w.re().disabled(); // <- Modified
    w
});

to do so

use stm32f1::stm32f103 as pac;
let periph = pac::Peripherals::take().unwrap();
let usart1 = periph.USART1;
// ...
const USART_BASE_CFG: pac::usart1::cr1::W = pac::usart1::cr1::INITIAL_VALUE
    .ue().enabled()
    .m().m9()
    .pce().enabled()
    .ps().even()
    .te().enabled()
    .tcie().enabled()
    .re().disabled();
// ...
usart1.cr1.write(USART_BASE_CFG);
// ...
usart1.cr1.write(USART_BASE_CFG.re().enabled());
// ...
usart1.cr1.write(USART_BASE_CFG.re().disabled());

Or for example like this

let mut w = pac::usart1::cr1::INITIAL_VALUE;
w = w
   .ue().enabled()
   .m().m9()
   .pce().enabled()
   .ps().even()
   .te().enabled()
   .tcie().enabled()
   .re().disabled();
usart1.cr1.write(w);

Is it possible to change svd2rust to allow this?

[adamgreig]

Thanks for suggesting this!

We discussed it at the end of today's meeting (here) though didn't really come to any conclusions yet.

I think one problem is that even if we added a const W like your INITIAL_VALUE (which is fairly easy to do, though may affect compile times a bit), it's not clear how you could pass it in. The write() method takes a closure that returns a reference to the W, so for example something like this doesn't work:

let mut w: stm32f405::tim2::cr1::W  = unsafe { core::mem::transmute(0) };
w.opm().enabled();
tim2.cr1.write(|_| w.cen().enabled());

error[E0597]: `w` does not live long enough
  --> src/main.rs:33:24
   |
33 |     tim2.cr1.write(|_| w.cen().enabled());
   |                    --- ^----------------
   |                    |   |
   |                    |   borrowed value does not live long enough
   |                    |   returning this value requires that `w` is borrowed for `'static`
   |                    value captured here
...
43 | }
   | - `w` dropped here while still borrowed

Making a whole new function that takes W directly isn't a great option as it seems very likely impact compile times for something that's not likely to see much use. There are some workarounds, like you could use bits() on the W to load in the common fields and then just set the extra fields you want, or write your own wrapper around write() that also accepts a closure, but first sets your common fields before calling the closure. None of that's ideal though; maybe someone else will have a better idea of a way to include this.

It's worth noting that chiptool, an svd2rust fork, does already support this feature by only having one type for register values, so you can easily modify it and then pass it to write().

[AndreySmirnov81]

Thank you for considering the proposal!

Let me clarify a little how I imagine it:

1. make W copyable
2. add method write_value(w: W)
3. add constants INITIAL_VALUE: W
4. make methods for getting field writers and their method as const fn (I'm not sure it's possible right now)

After that, instead of

write(|w| w.f1().a1().f2().a2());

you can write

write_value(*pac::peripheral::register::INITIAL_VALUE.f1().a1().f2().a2());

or for example

let mut val = pac::peripheral::register::INITIAL_VALUE;
val.f1().a1();
val.f2().a2();
write_value(val);

const VAL: pac::peripheral::register::W = *pac::peripheral::register::INITIAL_VALUE.f1().a1().f2().a2();
write_value(VAL);

Instead of

reset();

you can write

write_value(pac::peripheral::register::INITIAL_VALUE);

[Dominaezzz]

I have a different use case for this.

The SPI peripheral of the ESP32 chips support changing its registers via DMA.
This allows one to setup multiple SPI transfers/transactions, with different settings (like CS pin, frequency, etc) in a single DMA transfer.

To use the feature, I basically need to write the intended value of the registers into a [u32; 8] buffer for the 8 registers I can change.
I could do this manually with offsets and sizes but doing this for 6 different esp32 chips with different offsets is crazy and why do that when a PAC exists.
To my knowledge there's no trivial way to this in svd2rust.

[burrbull]

To use the feature, I basically need to write the intended value of the registers into a [u32; 8] buffer for the 8 registers I can change. I could do this manually with offsets and sizes but doing this for 6 different esp32 chips with different offsets is crazy and why do that when a PAC exists.

Could you show example of code you blocked on?

[Dominaezzz]

The (pseudo) code would look something like this. (The PAC for reference)

let mut registers = [0u32; 8];

registers[0] = spi2::USR2::create(|w| {
    w.usr_command_bitlen().bits(4);
    w.usr_command_value().bits(0x20))
});
register[1] = spi2::USER1::create(|w| {
    unsafe { w.usr_dummy_cyclelen().bits(dummy - 1) };
    w.usr_addr_bitlen().bits((address.width() - 1) as u8)
});
// .... register[n] = ...;

let dma_buffer = DmaBuffer::new(descriptors, &registers);
let transfer = spi_dma.segmented_transfer(dma_buffer);

[burrbull]

So you just need a create which works like write but does not write anything, just returns a value.

[Dominaezzz]

Pretty much! Couldn't have said it better myself.

[burrbull]

I think we could add such method. The only moment. I'm not a fan of the name. "create" does not describe its purposes.
Do you have ideas?

[Dominaezzz]

Here's what I'm come up with so far.

• raw
• stack
• create_bits
• fake_write
• mock_write
• from
• value (I think I like this the most)
• create_value (this comes in 2nd)
• mock

[Dominaezzz]

Ah, I'm going on a tangent here but maybe we could have a prepare that works like so.

let reg_value: <some new type> = spi2::USR2::prepare(|w| {
    w.usr_command_bitlen().bits(4);
    w.usr_command_value().bits(0x20))
});

// For me
registers[0] = reg_value.bits();

// For the author of this issue.
spi_reg_block.usr2().write(|w| {
    w.safe_bits(reg_value);
    w.usr_command_value().bits(5)
})

[burrbull]

Ah, I'm going on a tangent here but maybe we could have a prepare that works like so.

There is no sense in new type as you can always pass bits in writer with bits.
Also write and modify now return written value. So you can pass it in next write (was not possible when issue created).

"create_value" is good for me. "prepare" is also good name/part of name.

Could you open a PR and we discuss it on next meeting?

[Dominaezzz]

Yeah that's fair enough I didn't know there was already a solution for that.

I can open a PR but not any time soon, definitely not in time for Tuesday 😄