Skip to content

cpu/samd5x: improve isr buffer handling #21486

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
May 22, 2025
Merged

cpu/samd5x: improve isr buffer handling #21486

merged 2 commits into from
May 22, 2025

Conversation

kfessel
Copy link
Contributor

@kfessel kfessel commented May 13, 2025 •
edited
Loading

Contribution description

  • this improves the buffer handling and improves readability of the samd5x can-driver
  • avoid assertion failure if the enable pin is not configured
  • separates isr handling to ease readability

Testing procedure

  • read
  • do can tests

Issues/PRs references

@kfessel kfessel requested review from benpicco and dylad as code owners May 13, 2025 09:19
@github-actions github-actions bot added Platform: ARM Platform: This PR/issue effects ARM-based platforms Area: cpu Area: CPU/MCU ports labels May 13, 2025
@kfessel kfessel requested a review from maribu May 13, 2025 09:19
mguetschow
mguetschow reviewed May 13, 2025
View reviewed changes
cpu/samd5x/periph/can.c Outdated
printf("Tx buffer|\t0x%08" PRIx32 "|\t%" PRIu32 "\n",
(uint32_t)(dev->msg_ram_conf.tx_fifo_queue),
(uint32_t)(ARRAY_SIZE(dev->msg_ram_conf.tx_fifo_queue)));
printf("Standard filters|\t%p|\t%u\n", (dev->msg_ram.std_filter),
Copy link
Contributor

Choose a reason for hiding this comment

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

Hu, why going from formatting macros back to %p and %u?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

readability avoid unnecessary cast - i somehow thought newlib is C99 compliant -- will fix that

Copy link
Contributor Author

Choose a reason for hiding this comment

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

for that array size thing i stayed with u

benpicco
benpicco reviewed May 13, 2025
View reviewed changes
benpicco
benpicco reviewed May 13, 2025
View reviewed changes
benpicco
benpicco reviewed May 13, 2025
View reviewed changes
@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 May 13, 2025
@kfessel kfessel changed the title cpu/samd5x: improver isr buffer handling cpu/samd5x: improve isr buffer handling May 13, 2025
@riot-ci
Copy link

riot-ci commented May 13, 2025 •
edited
Loading

Murdock results

✔️ PASSED

ca5a322 cpu/samd5x: seperate irq handlers

Success Failures Total Runtime
10362 0 10363 11m:21s

Artifacts

@kfessel
Copy link
Contributor Author

kfessel commented May 13, 2025 •
edited
Loading

May I squash ?

@kfessel kfessel force-pushed the p-can-isr-buffer-handling branch from 1fd9f34 to 1d3fea3 Compare May 14, 2025 07:29
@crasbe crasbe added Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation labels May 14, 2025
@kfessel kfessel requested review from mguetschow and benpicco May 15, 2025 08:54
kfessel
kfessel commented May 16, 2025
View reviewed changes
maribu
maribu reviewed May 20, 2025
View reviewed changes
maribu
maribu reviewed May 20, 2025
View reviewed changes
Copy link
Member

@maribu maribu left a comment

Choose a reason for hiding this comment

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

see inline. feel free to squash at will

@kfessel kfessel force-pushed the p-can-isr-buffer-handling branch from 415f3b2 to 3139df7 Compare May 20, 2025 16:11
maribu
maribu reviewed May 21, 2025
View reviewed changes
@kfessel kfessel force-pushed the p-can-isr-buffer-handling branch from c44244c to ca5a322 Compare May 21, 2025 20:34
@kfessel kfessel added this pull request to the merge queue May 22, 2025
@kfessel
Copy link
Contributor Author

kfessel commented May 22, 2025

Thanks for the reviewes

@kfessel kfessel removed this pull request from the merge queue due to a manual request May 22, 2025
@kfessel kfessel added this pull request to the merge queue May 22, 2025
Merged via the queue into RIOT-OS:master with commit a81adc4 May 22, 2025
26 checks passed
@kfessel kfessel mentioned this pull request May 22, 2025
Open
@Teufelchen1 Teufelchen1 added this to the Release 2025.07 milestone Jul 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@maribu maribu maribu approved these changes

@dylad dylad Awaiting requested review from dylad dylad is a code owner

@mguetschow mguetschow Awaiting requested review from mguetschow

@benpicco benpicco Awaiting requested review from benpicco benpicco is a code owner

Assignees
No one assigned
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 Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation
Projects
None yet
Milestone
Release 2025.07
Development

Successfully merging this pull request may close these issues.
7 participants
@kfessel @riot-ci @benpicco @maribu @mguetschow @crasbe @Teufelchen1