Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement the ability to work with W outside of write #708

Open
AndreySmirnov81 opened this issue Jan 27, 2023 · 2 comments
Open

Implement the ability to work with W outside of write #708

AndreySmirnov81 opened this issue Jan 27, 2023 · 2 comments

Comments

@AndreySmirnov81
Copy link

AndreySmirnov81 commented Jan 27, 2023

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?

@AndreySmirnov81 AndreySmirnov81 changed the title Implement the ability to work with W outside of `write' Implement the ability to work with W outside of write Jan 27, 2023
@adamgreig
Copy link
Member

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
Copy link
Author

AndreySmirnov81 commented Feb 1, 2023

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);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants