Support registers without a defined reset value

[lambda]

In case no reset value is defined for a register, we suppress generation
of the reset, and reset_value functions. For writable registers which
don't have a reset value, we pass the only possible value we could into the
closure, a zeroed bitfield.

This is a rebase of pull request #175 from @ecstatic-morse, along with an implementation of write which doesn't depend on the reset value as suggested by @sjroe in #174, and enabling of some tests that cover this case as had been requested in #175. It looks like #175 hasn't been updated in a while, so I figured I'd send a new pull request.

Fixes: #174

[lambda]

@japaric @Emilgardis Ping, looking for a review or at least comment on whether this approach looks reasonable.

[Emilgardis]

LGTM, gonna kick the CI

[Emilgardis]

bors try

[bors]

try

Build succeeded

• continuous-integration/travis-ci/push

[ecstatic-morse]

I've started working on other things, but using 0 as a default reset value seems like a really bad idea. This means that calling write on a register with a nonzero reset value can cause some bits in the register being silently flipped. This would not be a fun problem to debug.

Is this no longer the case?

[Disasm]

As for me, using some non-zero default value for write() is terribly wrong: you can't rely on write() behavior if your svd file can be changed. If you want to write something based on the reset value of the register, you should explicitly use reset_value(). From the other side, write has meaning like "overwrite the register with a new value", so 0 as the default write value seems ok. Documentation for the write function says "Writes to the register", nothing about the reset value.
So my proposition is to generate write() for any register that could be written, with the default value of 0. The reset_value() function should be generated only if we have such value defined in svd (directly or indirectly). Maybe @japaric can say something about that.

[japaric]

The documentation says (with an example):

The write method takes a closure with signature (&mut W) -> &mut W. If the "identity closure", |w| w, is passed then the write method will set the CR2 register to its reset value. Otherwise, the closure specifies how the reset value will be modified before it's written to CR2.

Using zero as the initial value if the <resetValue> information is missing doesn't seem like a good idea to me. If someone adds the missing info to the SVD later on that would change the semantics of write, which is a breaking change. This PR goes against svd2rust design principle of "adding new information to the SVD only extends the API (see enumeratedValues)".

As stated above this PR is a breaking change (changes semantics) and requires an RFC / approval from the team to land. cc @rust-embedded/tools

(changing label to S-waiting-on-team)

[therealprof]

I'd agree with the reasoning above and suggest to not accept this PR.

@Emilgardis You approved this change, what is your opinion?

[Disasm]

@therealprof We discussed this during the Oxidize and came to a decision of having deprecated write method and two new methods like write_with_reset and write_with_zero.

[therealprof]

@Disasm Do we really need to deprecate write()? That's literally used everywhere and would cause major breakage or at least annoyance.

I'd like to take the opportunity of a working CI to move svd2rust forward a bit in the next time so now would be the right time to propose breaking changes.

[Disasm]

@therealprof I can imagine the following plan:

1. Documenting write properly (mentioning that is uses reset value as its initial write value)
2. Adding write_with_zero and write_with_reset, hiding write from docs
3. Deprecating write in favor of new methods

[burrbull]

Each register has write_with_zero method since 0.16.0

[nickray]

I learned about this write_with_zero method only today :) It should be advertised more maybe :)

Using the reset value for write does make sense "kinda" (although there are quite a few registers I've seen where you can't write the reset value, so just because it's in the SVD doesn't imply it's a more natural choice than zeros), but it can definitely be confusing, especially for someone coming from C.