Implement various interfaces for trace configuration #342

tmplt · 2021-04-28T17:23:29Z

I'm working on tracing support and aim to implement functions that abstract the configuration of relevant peripherals. Of chief interest is DWT, ITM and TPIU. Some propored abstractions will go against what is established in the crate; I will ask for comments on these.

rust-highfive · 2021-04-28T17:23:32Z

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @thalesfragoso (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

src/peripheral/dwt.rs

src/peripheral/tpiu.rs

src/peripheral/dwt.rs

tmplt · 2021-05-05T11:36:41Z

API changes and implementations are ready for review. Some TODOs about armv6m remain. I'll resolve them after review.

adamgreig · 2021-07-06T11:59:25Z

Hi, just a quick note to say I just saw this via the reference in probe-rs and will try and review it for you soon, it would be great to have more support for trace management in here.

tmplt · 2021-07-06T17:15:33Z

We probably want to offer sane defaults for the added configuration structs.

src/peripheral/dwt.rs

src/peripheral/itm.rs

adamgreig

Thanks, sorry this sat waiting for so long!

On the whole this looks good to me. I've made a couple of comments. Besides that, could you fix up the clippy lints, which you can see in the PR view here? Mostly just needs some #[allow] on the capitalised names and a few other minor things.

A DWT unit can do much more than just comparing addresses. PC comparison, for example, which is only possible with the first DWT unit.

src/peripheral/tpiu.rs

tmplt · 2021-11-21T01:25:18Z

Ready for a second review, @newAM. (Github's re-request functionality doesn't work on my end.)

newAM

Looks good to me, thanks for all your work on this, I'm looking forward to using it 👍

As per Rust API guidelines: <https://rust-lang.github.io/api-guidelines/interoperability.html#c-common-traits>.

jonathanpallant · 2021-11-25T10:41:04Z

This looks OK to me. @newAM are you happy with the changes that arrived after your last review?

newAM

Looks good to me!

adamgreig · 2021-11-27T12:25:04Z

Thanks! I think finally the MSRV needs to be bumped to support non_exhaustive, from 1.38 to 1.40 at least.

adamgreig · 2021-11-27T12:37:30Z

Thanks! I meant to say, it needs updating:

in .github/workflows/ci.yml as you've done
in .github/bors.toml so bors knows to wait for the new MSRV job
in README.md and src/lib.rs for user-facing documentation

Required to support the #[non_exhaustive] attribute.

adamgreig

Perfect, thanks!

bors merge

bors · 2021-11-27T12:45:57Z

Build succeeded:

rust-embedded/cortex-m#363 and rust-embedded/cortex-m#342 have been merged.

375: Prepare v0.7.4 r=thejpster a=adamgreig I've created a new branch, `v0.7.x`, which is currently at the latest non-breaking commit (so includes #346 #349 #347 #351 #339 #352 #348 #363 #362 #361 but does not include #342), to track the 0.7 series since master now contains breaking changes for v0.8. This PR (which targets the new branch) cherry-picks #372 #369 #374 and bumps the version to v0.7.4 (and updates CHANGELOG) ready for a new v0.7.4 release. Once complete I'll also backport the changelog entries and bump the version in master to 0.7.4. I think this is everything that should be in 0.7 -- the only excluded PRs from master are #342 and #367 I believe, and I don't think we have any open PRs targeting 0.7 either. Any other thoughts on items for inclusion in 0.7.4 (or other changelog entries I missed)? Co-authored-by: bors[bot] <26634292+bors[bot]@users.noreply.github.com> Co-authored-by: Adam Greig <adam@adamgreig.com>

tmplt added 2 commits April 28, 2021 18:41

tpiu: impl functions related to trace data output

720282f

dwt: impl functions related to trace generation

f75f17f

rust-highfive assigned thalesfragoso Apr 28, 2021

rust-highfive added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-cortex-m labels Apr 28, 2021

tmplt added 3 commits April 30, 2021 00:35

tpiu: fix always-zero field-comparison

8c53e60

dwt: configure address comparison using struct

be983a3

dwt: add missing #[inline]

9305580

tmplt commented Apr 30, 2021

View reviewed changes

src/peripheral/dwt.rs Outdated Show resolved Hide resolved

src/peripheral/tpiu.rs Show resolved Hide resolved

tmplt commented Apr 30, 2021

View reviewed changes

src/peripheral/dwt.rs Outdated Show resolved Hide resolved

tmplt added 5 commits April 30, 2021 03:00

dcb: add note about vendor-specific trace options

9285dcc

dwt: reimplement with bitfield

859f9d8

itm: impl functions related to trace generation

b533eb6

tpiu: fix flipped SWOSupports field, reimplement with bitfield

fb604a7

itm, dwt: limit some bitfields to u8

085e738

tmplt marked this pull request as ready for review May 5, 2021 11:31

tmplt requested a review from a team as a code owner May 5, 2021 11:31

tmplt mentioned this pull request Jul 6, 2021

Refactor: exploit cortex-m structs instead of rolling own? probe-rs/probe-rs#727

Open

adamgreig reviewed Sep 23, 2021

View reviewed changes

src/peripheral/dwt.rs Outdated Show resolved Hide resolved

adamgreig reviewed Sep 23, 2021

View reviewed changes

src/peripheral/dwt.rs Outdated Show resolved Hide resolved

adamgreig reviewed Sep 23, 2021

View reviewed changes

src/peripheral/itm.rs Outdated Show resolved Hide resolved

adamgreig requested changes Sep 23, 2021

View reviewed changes

tmplt added 4 commits September 24, 2021 05:41

dwt: feature gate trace and PC samples out of armv6m

669c872

itm: fix field spelling

362ad2d

itm: remove useless conversion

02853a4

allow clippy::upper_case_acronyms

aa17958

tmplt added 5 commits November 21, 2021 01:34

dwt: refactor out unnecessary explicit panic

0e64774

tpiu: remove get_ prefix, as per Rust API guidelines

d45bad7

dwt: DWTError -> DwtError for in-crate consistency

633a631

dwt: mark ComparatorFunction, DwtError as non-exhaustive

09929b1

A DWT unit can do much more than just comparing addresses. PC comparison, for example, which is only possible with the first DWT unit.

itm: properly document ITMSettings

c37f80b

tmplt force-pushed the feat/tracing branch from 70abf40 to d62ee1a Compare November 21, 2021 01:11

tmplt commented Nov 21, 2021

View reviewed changes

src/peripheral/tpiu.rs Outdated Show resolved Hide resolved

newAM previously approved these changes Nov 21, 2021

View reviewed changes

tmplt added 3 commits November 21, 2021 02:51

tpiu: use bitfield for SPPR

1efe319

tpiu: improve TYPE field documentation

5a92298

dwt, itm, tpiu: derive common traits for structs/enums

92c15ed

As per Rust API guidelines: <https://rust-lang.github.io/api-guidelines/interoperability.html#c-common-traits>.

tmplt dismissed newAM’s stale review via 92c15ed November 21, 2021 01:51

tmplt force-pushed the feat/tracing branch from 5bac9f5 to 92c15ed Compare November 21, 2021 01:51

newAM previously approved these changes Nov 25, 2021

View reviewed changes

dwt: refactor enable_exception_tracing into enable/disable funs

360fb33

tmplt dismissed newAM’s stale review via 360fb33 November 27, 2021 09:34

tmplt force-pushed the feat/tracing branch from 9bd220d to 9a87a55 Compare November 27, 2021 12:39

bump MSRV

c1d434a

Required to support the #[non_exhaustive] attribute.

tmplt force-pushed the feat/tracing branch from 9a87a55 to c1d434a Compare November 27, 2021 12:39

adamgreig approved these changes Nov 27, 2021

View reviewed changes

bors bot merged commit ae1d2a6 into rust-embedded:master Nov 27, 2021

tmplt added a commit to rtic-scope/itm that referenced this pull request Nov 27, 2021

cargo: pin cortex-m from rust-embedded instead

12f482a

rust-embedded/cortex-m#363 and rust-embedded/cortex-m#342 have been merged.

adamgreig mentioned this pull request Dec 31, 2021

Prepare v0.7.4 #375

Merged

TDHolmes mentioned this pull request Jan 6, 2022

Differentiate comparator 0 as the only one capable of cycle compare #377

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement various interfaces for trace configuration #342

Implement various interfaces for trace configuration #342

tmplt commented Apr 28, 2021

rust-highfive commented Apr 28, 2021

tmplt commented May 5, 2021

adamgreig commented Jul 6, 2021

tmplt commented Jul 6, 2021

adamgreig left a comment

tmplt commented Nov 21, 2021

newAM left a comment

jonathanpallant commented Nov 25, 2021

newAM left a comment

adamgreig commented Nov 27, 2021

adamgreig commented Nov 27, 2021

adamgreig left a comment

bors bot commented Nov 27, 2021

Implement various interfaces for trace configuration #342

Implement various interfaces for trace configuration #342

Conversation

tmplt commented Apr 28, 2021

rust-highfive commented Apr 28, 2021

tmplt commented May 5, 2021

adamgreig commented Jul 6, 2021

tmplt commented Jul 6, 2021

adamgreig left a comment

Choose a reason for hiding this comment

tmplt commented Nov 21, 2021

newAM left a comment

Choose a reason for hiding this comment

jonathanpallant commented Nov 25, 2021

newAM left a comment

Choose a reason for hiding this comment

adamgreig commented Nov 27, 2021

adamgreig commented Nov 27, 2021

adamgreig left a comment

Choose a reason for hiding this comment

bors bot commented Nov 27, 2021