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.

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

Implement support for SAMV devices #681

Merged
merged 5 commits into from
Sep 22, 2021
Merged

Implement support for SAMV devices #681

merged 5 commits into from
Sep 22, 2021

Conversation

twasilczyk
Copy link
Contributor

This is a WIP PR to touch base on the new device family I'd like to add to modm. I'm primarily interested in SAMV70N20B (that's what my hobby project is running on) and there's no xplained board for this chip, so I plan to make simple examples based on bare chip config.

twasilczyk
twasilczyk commented Sep 10, 2021
View reviewed changes
README.md Outdated Show resolved Hide resolved
salkinium
salkinium reviewed Sep 10, 2021
View reviewed changes
Copy link
Member

@salkinium salkinium left a comment

Choose a reason for hiding this comment

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

The CI job checking itself for completeness is pretty useful and still makes me giggle.

repo.lb Outdated Show resolved Hide resolved
twasilczyk
twasilczyk commented Sep 12, 2021
View reviewed changes
Copy link
Contributor Author

@twasilczyk twasilczyk left a comment

Choose a reason for hiding this comment

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

I finished fixing the hal matrix generator script (with a few missing odd ones). Still haven't had a chance to flash and test if this works - GPIO doesn't build for this target yet.

README.md Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Show resolved Hide resolved
twasilczyk
twasilczyk commented Sep 17, 2021
View reviewed changes
src/modm/platform/clock/sam_pmc/clockgen.hpp.in Outdated Show resolved Hide resolved
examples/samv/blink/main.cpp Outdated Show resolved Hide resolved
@twasilczyk twasilczyk marked this pull request as ready for review September 20, 2021 05:54
@twasilczyk
Copy link
Contributor Author

twasilczyk commented Sep 20, 2021 •
edited
Loading

Okay, all the remaining TODOs are done. I learned much more about SAM clocks than I wanted to. Special thanks to Jeff for all the help with SAMG internals and as usual Niklas for this wonderful project.

Please pay extra scrutiny with reviews, since I still can't say I know what I'm doing.

I think I'll have nightmares about clocks...

salkinium
salkinium reviewed Sep 20, 2021
View reviewed changes
Copy link
Member

@salkinium salkinium left a comment

Choose a reason for hiding this comment

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

I'm generally happy with this PR, the question is are you motivated to de-template the clock functions? It's not terribly important, just very inconsistent with the STM32 API with regards to template use. Otherwise I'd merge this and leave this as tech debt for future contributors.

src/modm/platform/clock/sam_pmc/clockgen.hpp.in Outdated Show resolved Hide resolved
src/modm/platform/clock/sam_pmc/clockgen_impl.hpp.in Show resolved Hide resolved
src/modm/platform/clock/sam_pmc/clockgen_impl.hpp.in Outdated Show resolved Hide resolved
src/modm/platform/clock/sam_pmc/clockgen_impl.hpp.in Show resolved Hide resolved
src/modm/platform/clock/sam_pmc/clockgen_impl.hpp.in Show resolved Hide resolved
src/modm/platform/clock/sam_pmc/clockgen_impl.hpp.in Outdated Show resolved Hide resolved
src/modm/platform/clock/sam_pmc/clockgen.cpp.in Outdated Show resolved Hide resolved
@salkinium
Copy link
Member

I still can't say I know what I'm doing.

Not sure if there's a difference in embedded software 🤪 I feel like everyone is hacking everything together without plan, it's quite a scary profession.

mcbridejc
mcbridejc reviewed Sep 20, 2021
View reviewed changes
src/modm/platform/core/sam/module.lb Outdated Show resolved Hide resolved
src/modm/platform/clock/sam_pmc/module.lb Outdated Show resolved Hide resolved
salkinium
salkinium approved these changes Sep 21, 2021
View reviewed changes
Copy link
Member

@salkinium salkinium left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Ok, I'm happy with this now. Excellent work!

@salkinium salkinium added the ci:hal Triggers the exhaustive HAL compile CI jobs label Sep 22, 2021
@salkinium
Copy link
Member

Ah, this is missing the compile-all jobs for both SAMG and SAMV. I'll add them later, currently on holidays.

@salkinium salkinium added ci:hal Triggers the exhaustive HAL compile CI jobs and removed ci:hal Triggers the exhaustive HAL compile CI jobs labels Sep 22, 2021
@salkinium salkinium added ci:hal Triggers the exhaustive HAL compile CI jobs and removed ci:hal Triggers the exhaustive HAL compile CI jobs labels Sep 22, 2021
@salkinium salkinium added ci:hal Triggers the exhaustive HAL compile CI jobs and removed ci:hal Triggers the exhaustive HAL compile CI jobs labels Sep 22, 2021
@salkinium salkinium merged commit 9036666 into modm-io:develop Sep 22, 2021
@salkinium salkinium added this to the 2021q3 milestone Oct 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mcbridejc mcbridejc mcbridejc left review comments

@salkinium salkinium salkinium approved these changes

Assignees
No one assigned
Labels
ci:hal Triggers the exhaustive HAL compile CI jobs example 🔑 feature 🚧 modm-devices 🎛
Milestone
2021q3
Development

Successfully merging this pull request may close these issues.
3 participants
@twasilczyk @salkinium @mcbridejc