rust-embedded
ITM: exhaustively check feature support during `configure`; improve standard correctness, documentation
As discussed in #382, this PR checks that the requested features are ensured before continuing in ITM::configure. Because some register fields in ITM_TCR are RAZ/WI and RAZ or RAO (and no dedicated RO flags exists that we can otherwise check) there are cases when we end up with partially applied configurations. These cases are documented on the relevant errors.
No ITM::configure_unchecked in this PR because of the code duplication it would require. But I do not see a common use-case for configuring the ITM outside of an init function which (in my current case at least) allows for a few more registers reads.
This PR also ensures that the ITM is unlocked (if a software lock is implemented) and disabled before modifying any registers. When the configuration has been correctly applied, the ITM is re-locked, as per CoreSight recommendation. The ITM is not re-locked if the configuration fails to apply.
r? @thalesfragoso
(rust-highfive has picked a reviewer for you, use r? to override)
dc4642512761f2b6d9fe65d7878cd12a2131aeb8 can be fixup'd into 622a9b7297f47a1d6a0d40894e2d2994ad46db27.
Wait with eventual merge for me to test these checks. It should be done by next week.
https://github.com/rust-embedded/cortex-m/issues/392 must be handled in this PR.
ARM DDI 0403E.d, p. C1-716:
If multi-source trace is in use, the debugger must write a non-zero value to this field.
C-718:
If a system supports multiple trace streams, the debugger must write a nonzero value to the ITM_TCR.TraceBusID field. For information about permitted values for this field in a CoreSight-compliant implementation, see the ARM IHI 0029E [...] An example of a system with multiple trace streams is an ARMv7-M core with ETM. In this case, the debugger must program the ETM TraceBusID register with a different nonzero identifier for the ETM trace stream.
ARM IHI 0029E does not contain the term "TraceBusID".
It is not very clear whether non-zero should be written if multi-source trace is wanted, or if the target implements it. If the latter, we'll need to write non-zero even if we only want a single trace source.
This refactored implementation of ITM::configure works as expected on the atsame51n. I've sent an email to ARM in hopes of some clarification on TraceBusID; eventual fixes for which can be postponed to another PR.
I consider this PR ready for review.
lock and unlock has been streamlined a bit and in the process has_software_lock and locked have been hidden. lock and unlock could potentially be hidden later when #393 and similar close so that configure is exhaustive.
On TraceBusID: after a brief back-and-forth with ARM it seems to be valid to write non-zero. If only ITM is supported on the system, TraceBusID is potentially RAZ (in ARMv8-M, at least). Some rules do apply when multi-source trace is in use, but that's for another day.
The re-locking sequence currently incorrectly always assumes that a software lock is implemented. Amendmend will follow.
Hi all,
What happened with this PR? Looking over it, it seems ready to go.
Ping @rust-embedded/cortex-m
IIRC
The re-locking sequence currently incorrectly always assumes that a software lock is implemented
is still relevant, and should be fixed before merge.
I do not have access to relevant hardware, and my knowledge of the area has faded somewhat. Is anyone else interested in taking up the torch?
@tmplt I could probably give it a look. What HW is needed? I can get some ordered for testing.
I've tested the code on SAME51 and SAMV71 boards. IIRC one of these lack the software lock, resulting in a busy wait that never exits. I don't think the ITM coverage is exhaustively detailed in the data sheets, and so must be detected at run-time.