cpu/samd5x/periph_can: fix RX [backport 2025.01] #21184
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
maribu
added
Area: cpu
MrKevinWeiss
approved these changes
Feb 3, 2025
|
I will also manually add it to the release notes |
kfessel
approved these changes
Feb 3, 2025
|
I removed this from the merge queue for now. The issue is that Github merged an old state of the PR backported here. But this backport did indeed correctly pick the commit that should have been merged to |
CAN required CLK_CANx_APB and CLK_CANx_APB to be running and will not request any clock by itself. We can ensure both clocks to be running by preventing the MCU from entering IDLE state. The SAMD5x/SAME5x Family Data Sheet says in Section "39.6.9 Sleep Mode Operation" says: > The CAN can be configured to operate in any idle sleep mode. The CAN > cannot operate in Standby sleep mode. > > [...] > > To leave low power mode, CLK_CANx_APB and GCLK_CANx must be active > before writing CCCR.CSR to '0'. The CAN will acknowledge this by > resetting CCCR.CSA = 0. Afterwards, the application can restart CAN > communication by resetting bit CCCR.INIT. tl;dr: At most SAM0_PM_IDLE is allowed while not shutting down the CAN controller, but even that will pause communication (including RX). Apparently, the CAN controller was never tested without also using the USB peripheral, which kept the clocks running as side effect.
maribu
force-pushed
the
backport/2025.01/cpu/samd5x/can-fix-rx
branch
from
1ea14e8 to
97006a8
Compare
|
I updated this to the commit that actually got merged into |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Backport of #21181
Contribution description
CAN required CLK_CANx_APB and CLK_CANx_APB to be running and will not request any clock by itself. We can ensure both clocks to be running by preventing the MCU from entering IDLE state.
The SAMD5x/SAME5x Family Data Sheet says in Section "39.6.9 Sleep Mode Operation" says:
tl;dr: At most SAM0_PM_IDLE is allowed while not shutting down the CAN controller, but even that will pause communication (including RX).
Apparently, the CAN controller was never tested without also using the USB peripheral, which kept the clocks running as side effect.
Testing procedure
Set up USB CAN stick
Sending to the SAME54-XPRO from Linux
Output of
candump anyRIOT output with
masterRIOT output with this PR
Conclusion
In both
masterand this PR sending from RIOT was picked up on the Linux side. But sending from Linux to RIOT was not picked up by RIOT inmaster, but with this PR.Issues/PRs references
#21181