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

No way to access w after writing #827

Open
BigPeteB opened this issue Mar 19, 2024 · 1 comment
Open

No way to access w after writing #827

BigPeteB opened this issue Mar 19, 2024 · 1 comment

Comments

@BigPeteB
Copy link
Contributor

I have two scenarios that I'm unable to solve with svd2rust. I'm lumping them into one ticket because I think the solutions may be related.

Problem 1

I'm porting some driver code where the original in C does this:

void write_special_reg(int value) {
    // A write to this register will be ignored if the device is busy,
    // but we can't simply poll for the device to be not-busy
    // because it could become busy after we check its status. 🤦🏻‍♂️
    // The workaround is to write the register
    // and then check that the register has the desired value,
    // repeating until it succeeds.
    while (tries--) {
        write_reg(SPECIAL_REG, value);
        if (value == read_reg(SPECIAL_REG))
            break;
    }
}

Unfortunately, I don't see any way to implement this using svd2rust. Not only is w inaccessible outside of the closure, there's no way to read from w, so you can't compare w to r in any way!

for _ in 0..tries {
    let w = device.special_reg().write(|w| w.parity().none()); // ⚠️ Not possible: no way to save `w` from the closure
    if w.parity() == device.special_reg().read().parity() { // ⚠️ Not possible: no way to read fields from `w`
        break;
    }
}

Nor can you do

for _ in 0..tries {
    let mut parity;
    device.special_reg().write(|w| {
        w.parity().none();
        parity = w.read().parity().bits(); // ⚠️ Not possible: no way to read fields from `w`
        w
    });
    if parity == device.special_reg().read().parity().bits() {
        break;
    }
}

Problem 2

The inability to save w and use it later means you can't generate an efficient sequence of writes. Suppose you need to write the same register multiple times, e.g. turning on power to sub-blocks sequentially. Ideally, you would do this using write(); it's wasteful to use modify() when you know the contents of the register didn't change since the last time you wrote it. However, svd2rust doesn't have a way to do this. You either have to use modify():

device.power_reg().write(|w| w.periph1().on());
delay_usec(100);
device.power_reg().modify(|r, w| w.periph2().on()); // ⚠️ Wasteful: has to read the register
                                                    // even though we know its contents haven't changed

Or you have to use write() and repeat yourself:

device.power_reg().write(|w| w.periph1().on());
delay_usec(100);
device.power_reg().write(|w| w.periph1().on().periph2().on()); // ⚠️ Repetitive: you have to say `periph1().on()` again,
                                                               // which leads to code maintenance problems

// It also makes it very annoying to have a dynamic value (for example, an optional peripheral);
// you would have to do this every time:
device.power_reg().write(|w| {
    // Set the fields that were previously set
    w.periph1().on();
    if use_periph2 {
        w.periph2().on();
    }
    // Now set the field we wanted to change this time
    w.periph3().on()
});

It would be better if there were a way to "preserve" the value of w across writes so that you could do something like this, using a write_again() function:

let mut last_w = device.power_reg().write(|w| w.periph1().on());
delay_usec(100);
last_w = device.power_reg().write_again(last_w, |w| w.periph2().on());
delay_usec(100);
// Even an optional peripheral is easy:
if use_periph3 {
    last_w = device.power_reg().write_again(last_w, |w| w.periph3().on());
}
device.power_reg().write_again(last_w, |w| w.periph4().on()); // value will be correct whether use_periph3 was true or false

Or a more functional programming solution might look like this, using w.with(previous_w) overwriting all of w with the value from the previous call:

let mut last_w = device.power_reg().write(|w| w.periph1().on());
delay_usec(100);
last_w = device.power_reg().write(|w| w.with(last_w).periph2().on());
delay_usec(100);
// Even an optional peripheral is easy:
if use_periph3 {
    last_w = device.power_reg().write(|w| w.with(last_w).periph3().on());
}
device.power_reg().write(|w| w.with(last_w).periph4().on()); // value will be correct whether use_periph3 was true or false

(Although at that point, you could probably ignore w and just use last_w directly, unless this causes a problem with lifetimes:

last_w = device.power_reg().write(|_| last_w.periph2().on());

)

To be sure, this problem is really just a microoptimization, but would be nice to have in order to further svd2rust's goal of generating "the same efficient code as you would write in C" (however that goal is phrased).

Solutions

The solution for problem 2 seems somewhat straightforward. write() and modify() should return w, and there should be a way to use the saved w later for a subsequent write().

The solution for problem 1 may overlap. The first issue is that there's no way to read from w. That might be solved be implementing w.read() or w.as_reader(), or on a per-field basis by implementing w.<field>_read(). Depending on how that capability is implemented, it might be used in a couple of ways. One would be to return w from write() and modify(), as in the solution for problem 2, so that it can be used freely outside of the write() or modify():

let w = device.special_reg().write(|w| w.parity().none());
if w.as_reader() == device.special_reg().read() {
    break;
}

Another might be that you have to save the value from the closure, as in:

let mut w_reader;
device.special_reg().write(|w| {
    w.parity().none();
    w_reader = w.as_reader();
    w
});
if w_reader == device.special_reg().read() {
    break;
}
@burrbull
Copy link
Member

related to #708 & #738

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