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

Implement coprocessor access instructions #531

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

spcan
Copy link

@spcan spcan commented May 17, 2024

Implements coprocessor basic access instructions (MCR, MRC, MCRR, MRRC) that compile to a single instruction under --release profiles

@spcan spcan requested a review from a team as a code owner May 17, 2024 17:42
@spcan
Copy link
Author

spcan commented May 17, 2024

CI tests:

  • rustfmt fails but functions are formatted the same as the rest of the assembly functions
  • stm32 target tests fail due to 3 probes being present: Need to repeat test

Copy link
Member

@adamgreig adamgreig left a comment

Choose a reason for hiding this comment

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

Thanks for the RP! What's the longer term plan, since these methods are all pub(crate)? Do you imagine adding further coprocessor-related functionality in cortex-m? Should the methods be publicly available?

cortex-m/src/coprocessor.rs Outdated Show resolved Hide resolved
cortex-m/Cargo.toml Outdated Show resolved Hide resolved
@spcan
Copy link
Author

spcan commented May 17, 2024

The aim of these is to expose a common method to interact through the ARM coprocessor interface. Separate libs can then implement the coprocessor driver's using these functions.

For example, I'm using it currently to interact with the LPC55S69 math coprocessor

@adamgreig
Copy link
Member

If you want separate libs to later use these functions, don't they need to be pub instead of pub(crate)?

@spcan
Copy link
Author

spcan commented May 17, 2024

Yeah, good catch, that was there on the test library, forgot to remove it

@spcan
Copy link
Author

spcan commented May 17, 2024

Ok, it's now failing to compile because const ASM operands are unstable, needing #![feature(asm_const)], can this be enabled in the crate, or should this PR wait until ASM const is stabilized??

This is getting stabilized soon as per #93332

@jannic
Copy link
Member

jannic commented Aug 28, 2024

This is getting stabilized soon as per #93332

Likely in rust 1.82.0.

@spcan
Copy link
Author

spcan commented Oct 21, 2024

Hello

With the stabilization of Rust 1.82 asm_const has been stabilized. This should be enough to merge these instructions into cortex-m without breaking, although there is the caveat of a new minimum Rust version now (1.82).

Additionally, with the release of the new RP2350 chip, there is now another mainstream processor with coprocessors, which adds some necessity to the access to coprocessor instructions.

I would like some input from somebody in the team of how to proceed at this point, if I should close this PR and open a new one, keep this PR or maybe spin this functionality into a different crate.

Many thanks

@thejpster
Copy link
Contributor

which adds some necessity to the access to coprocessor instructions

The rp2350-hal uses inline assembler to make use of the instructions. Nicer APIs would be better though.

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.

4 participants