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 MCAN shutdown by writing to CSR #45

Merged
merged 5 commits into from
Mar 4, 2024

Conversation

Ironedde
Copy link
Contributor

@Ironedde Ironedde commented Feb 21, 2024

MCAN can safely be shut down by writing to the CCCR.CSR register
(Clock Stop Request) and then waiting for CCCR.CSA (Clock Stop
Acknowledge) to become high.

This will not interrupt any ongoing transmissions/receptions and is
therefore deemed the safe way to shut down the bus.

Note: It is not stated directly, but testing has shown that issuing a
stop request will only wait for the currently ongoing transmissions
and not what is queued. These messages will be sent once we enter
operational mode again.

Thank you!

Thank you for your contribution.
Please make sure that your submission includes the following:

Must

  • The code compiles without errors or warnings.
  • All tests pass and in the best case you also added new tests.
  • cargo +stable fmt was run.
  • cargo +stable clippy yields no warnings.
  • Your changes were added to the CHANGELOG.md in the proper section.
  • You add a description of your work to this PR.
  • You added proper docs (in code, rustdoc and README.md) for your
    newly added features and code.

@Ironedde Ironedde requested a review from a team as a code owner February 21, 2024 12:41
mcan/src/bus.rs Outdated Show resolved Hide resolved
mcan/src/bus.rs Outdated Show resolved Hide resolved
mcan/src/reg.rs Outdated Show resolved Hide resolved
mcan/src/bus.rs Outdated Show resolved Hide resolved
mcan/src/bus.rs Outdated Show resolved Hide resolved
@epontan
Copy link
Collaborator

epontan commented Feb 21, 2024

Looks great @Ironedde! Just some nits about the documentation and terminology, but otherwise it looks good to me.

@Ironedde Ironedde requested a review from vcrn February 22, 2024 10:34
Copy link

@vcrn vcrn left a comment

Choose a reason for hiding this comment

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

Looks good! Agree with all comments from @epontan, like changing the name of "shutdown" to "power down mode", or possibly "sleep mode", to be consistent with the official documentation.

I'll try to come up with some suggestion for a high level description for ready_for_shutdown.

vcrn
vcrn previously approved these changes Feb 23, 2024
Copy link

@vcrn vcrn left a comment

Choose a reason for hiding this comment

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

Looks good to me!

Copy link
Collaborator

@epontan epontan left a comment

Choose a reason for hiding this comment

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

Looks great, @Ironedde! Just a few remaining nits, otherwise LGTM 👍

mcan/src/bus.rs Outdated Show resolved Hide resolved
mcan/src/bus.rs Outdated Show resolved Hide resolved
mcan/src/bus.rs Show resolved Hide resolved
mcan/src/reg.rs Outdated Show resolved Hide resolved
mcan/CHANGELOG.md Outdated Show resolved Hide resolved
@epontan
Copy link
Collaborator

epontan commented Feb 23, 2024

We should perhaps create a commit before this which allows missing_safety_doc in the reg module so the clippy action passes?

@Ironedde
Copy link
Contributor Author

We should perhaps create a commit before this which allows missing_safety_doc in the reg module so the clippy action passes?

@epontan I totally agree, I'm on it.

I also found out why we haven't gotten any annotations on the code when clippy failed.
Acording to the README in the actions-rs/clippy-check repo (which is archived btw), you cannot have a fork with the same name as the original repo and have it work out.

I'll rename my fork after this PR chain.

Ironedde added 5 commits March 4, 2024 13:45
MCAN can safely be shut down by writing to the `CCCR.CSR` register
(Clock Stop Request) and then waiting for `CCCR.CSA` (Clock Stop
Acknowledge) to become high.

This will not interrupt any ongoing transmissions/receptions and is
therefore deemed the safe way to shut down the bus.

Note: It is not stated directly, but testing has shown that issuing a
stop request will only wait for the currently ongoing transmissions
and not what is queued. These messages will be sent once we enter
operational mode again.
@Ironedde Ironedde force-pushed the implement-shutdown branch from 2c96ce4 to 794b91b Compare March 4, 2024 12:59
@Ironedde
Copy link
Contributor Author

Ironedde commented Mar 4, 2024

@epontan Sorry For all the notifications, I didn't see that you had resolved all the conversations above.

I have now rebased on top of #46 and all checks are going through 😄

Copy link
Collaborator

@epontan epontan left a comment

Choose a reason for hiding this comment

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

Nice! LGTM 👍

@epontan epontan merged commit a4c8e75 into GrepitAB:master Mar 4, 2024
5 checks passed
Ironedde added a commit to Ironedde/mcan-clone that referenced this pull request Mar 4, 2024
- Add safe way to shutdown the bus when actively transmitting/receiving
(GrepitAB#45)
- Add method to finalize configuration into initialization mode (GrepitAB#47)

- *Breaking* Update the register mappings with svd2rust 0.30.2 and
form 0.10.0 (GrepitAB#46)
@Ironedde Ironedde mentioned this pull request Mar 4, 2024
7 tasks
Ironedde added a commit to Ironedde/mcan-clone that referenced this pull request Mar 4, 2024
- Add safe way to shutdown the bus when actively transmitting/receiving
(GrepitAB#45)
- Add method to finalize configuration into initialization mode (GrepitAB#47)

- *Breaking* Update the register mappings with svd2rust 0.30.2 and
form 0.10.0 (GrepitAB#46)
Ironedde added a commit that referenced this pull request Mar 4, 2024
### Added
- Add safe way to shutdown the bus when actively transmitting/receiving
(#45)
- Add method to finalize configuration into initialization mode (#47)

### Changed
- *Breaking* Update the register mappings with svd2rust 0.30.2 and form
0.10.0 (#46)
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.

3 participants