Skip to content

Conversation

@KushalMeghani1644
Copy link
Contributor

This PR reintroduces the mtopi CSR addition from my previous PRs (#347 and #359),
but from a clean branch as suggested by the reviewers.
The diff is identical to the intended changes; commit history has been cleaned up.

@KushalMeghani1644 KushalMeghani1644 requested a review from a team as a code owner October 31, 2025 11:50
Copy link
Contributor

@romancardenas romancardenas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Just a few changes: add the field check in the new macro to check the field bit range and use hex representation in a test.

#[cfg(test)]
#[macro_export]
macro_rules! test_ro_csr_field {
($reg:ident, $field:ident: [$start:expr, $end:expr], $expected:expr) => {{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are not using start nor end. I the previous PR, this macro did a couple of extra checks (ensuring that the returned value matches the expected bit range)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll definitely do this, I just need a bit of time... :)

Co-authored-by: Román Cárdenas Rodríguez <[email protected]>
@KushalMeghani1644
Copy link
Contributor Author

KushalMeghani1644 commented Nov 3, 2025

I feel like this CI failing again, isn't the fault of the code changes made in this PR, the spellings were definitely not changed by me, I can & will fix this, but it's quiet late in my timezone... and as for the lints, same condition there, I'll fix them :)

@romancardenas
Copy link
Contributor

Hi @KushalMeghani1644

Don't worry about CI now, I will try to have it working ASAP (Nightly clippy triggers new errors from time to time). The checks that fail are not blockers for merging.

Please, address the comment about the tests and it is good to go.

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

Successfully merging this pull request may close these issues.

2 participants