Skip to content

cpu/samd5x: improve can-initialization#21173

Merged
kfessel merged 1 commit intoRIOT-OS:masterfrom
kfessel:p-sam-caninit
Apr 5, 2025
Merged

cpu/samd5x: improve can-initialization#21173
kfessel merged 1 commit intoRIOT-OS:masterfrom
kfessel:p-sam-caninit

Conversation

@kfessel
Copy link
Copy Markdown
Contributor

@kfessel kfessel commented Jan 29, 2025

Contribution description

This makes the can initialization for the samd5x more stable

  • init: just write values not change them from an unknown state
  • setup clock: follow the Peripheral Channel Control setup method described in 14.6.3.3 of the SAM D5x/E5x Family datasheet.

Testing procedure

find a Board that uses this cpu but has sight problems of getting the can bus up reliably
find its more reliable with this Patch

Issues/PRs references

SAM D5x/E5x Family datasheet page 144 (DS60001507M) 14.6.3.3

@github-actions github-actions bot added Platform: ARM Platform: This PR/issue effects ARM-based platforms Area: cpu Area: CPU/MCU ports labels Jan 29, 2025
@kfessel kfessel requested a review from maribu January 29, 2025 23:39
Copy link
Copy Markdown
Contributor

@mguetschow mguetschow left a comment

Choose a reason for hiding this comment

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

I don't have a device to test this, but looks sensible from a quick glance. Do you have a pointer to the datasheet?


/* setup */
pchctrl = GCLK_PCHCTRL_GEN(dev->conf->gclk_src);
GCLK->PCHCTRL[pchid].reg = pchctrl;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

why no busy wait loop here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

at this point the register is changeable (since the channel is deactivated)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think @mguetschow suggested to add a comment here to explain why not waiting here, in contrast to the other writes.

@kfessel kfessel force-pushed the p-sam-caninit branch 2 times, most recently from 93a057b to 98ce32d Compare January 30, 2025 12:55
@maribu
Copy link
Copy Markdown
Member

maribu commented Jan 30, 2025

I tested this with my SAME54-XPRO.

In master: TX works, RX does not, unless make debug.
With this PR: TX works, RX does not, unless make debug.

But having the clock init better matching the process in the datasheet does have merit.

Please squash!

@kfessel kfessel added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Jan 31, 2025
@riot-ci
Copy link
Copy Markdown

riot-ci commented Jan 31, 2025

Murdock results

✔️ PASSED

1fa6e09 cpu/samd5x: improve can-initialization

Success Failures Total Runtime
10271 0 10271 10m:42s

Artifacts

@kfessel kfessel enabled auto-merge January 31, 2025 10:59
@kfessel kfessel added this pull request to the merge queue Apr 5, 2025
Merged via the queue into RIOT-OS:master with commit c44f01a Apr 5, 2025
1 check passed
@mguetschow mguetschow added this to the Release 2025.04 milestone Apr 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: cpu Area: CPU/MCU ports CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Platform: ARM Platform: This PR/issue effects ARM-based platforms

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants