Skip to content

boards/samd21-based: model Kconfig #17355

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 10 commits into from
Dec 15, 2021
Merged

boards/samd21-based: model Kconfig #17355

merged 10 commits into from
Dec 15, 2021

Conversation

leandrolanzieri
Copy link
Contributor

Contribution description

This models Kconfig for all samd21-based boards. Currently this includes 28d8445 and c9b81f2 from #17299 as a dependency. Some minor changes on drivers are also included.

The list of boards is:

  • adafruit-itsybitsy-m4
  • arduino-mkr1000
  • arduino-mkrfox1200
  • arduino-mkrwan1300
  • arduino-mkrzero
  • arduino-nano-33-iot
  • arduino-zero
  • feather-m0
  • feather-m0-lora
  • feather-m0-wifi
  • hamilton
  • samd20-xpro
  • samd21-xpro
  • seeeduino_xiao
  • sensebox_samd21
  • serpente
  • sodaq-autonomo
  • sodaq-explorer
  • sodaq-one
  • sodaq-sara-aff
  • sodaq-sara-sff
  • wemos-zero

Testing procedure

  • Review the modelling and default configurations
  • Green CI

Issues/PRs references

Depends on #17299
Part of #16875

@leandrolanzieri leandrolanzieri added this to the Release 2022.01 milestone Dec 8, 2021
@github-actions github-actions bot added Area: boards Area: Board ports Area: CI Area: Continuous Integration of RIOT components Area: drivers Area: Device drivers Area: Kconfig Area: Kconfig integration Area: SAUL Area: Sensor/Actuator Uber Layer Area: sys Area: System Area: tests Area: tests and testing framework labels Dec 8, 2021
@leandrolanzieri leandrolanzieri mentioned this pull request Dec 8, 2021
Closed
@fjmolinas fjmolinas added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Dec 8, 2021
@fjmolinas
Copy link
Contributor

For hamilton the feature conflict between rtc and rtt is activating I think the way out is forcing this in the application configuration, since which one is currently selected is only based on the order of inclusion.

@fjmolinas
Copy link
Contributor

There also seems to be an issue with the arduino feature which is forcing periph_uart on make and not on Kconfig.

@fjmolinas
Copy link
Contributor

T

There also seems to be an issue with the arduino feature which is forcing periph_uart on make and not on Kconfig.

This is missing from Kconfig actually

@github-actions github-actions bot added the Area: arduino API Area: Arduino wrapper API label Dec 9, 2021
@leandrolanzieri leandrolanzieri force-pushed the pr/boards/samd21/model_kconfig branch from 5c1747d to c93fa56 Compare December 9, 2021 14:47
@leandrolanzieri
Copy link
Contributor Author

T

There also seems to be an issue with the arduino feature which is forcing periph_uart on make and not on Kconfig.

This is missing from Kconfig actually

Thanks, I fixed this now

@github-actions github-actions bot removed the Area: SAUL Area: Sensor/Actuator Uber Layer label Dec 9, 2021
@leandrolanzieri
Copy link
Contributor Author

For hamilton the feature conflict between rtc and rtt is activating I think the way out is forcing this in the application configuration, since which one is currently selected is only based on the order of inclusion.

Let's see how this works

@leandrolanzieri
Copy link
Contributor Author

BTW: I rebased to resolve conflicts

@MrKevinWeiss
Copy link
Contributor

ugg, tests/periph_pm still has some issues...

@fjmolinas
Copy link
Contributor

This one now needs a rebase!

@fjmolinas fjmolinas force-pushed the pr/boards/samd21/model_kconfig branch from c93fa56 to 456e146 Compare December 14, 2021 22:54
@fjmolinas
Copy link
Contributor

Rebased and pushed a fix attempt

MrKevinWeiss
MrKevinWeiss reviewed Dec 15, 2021
View reviewed changes
Copy link
Contributor

@MrKevinWeiss MrKevinWeiss left a comment

Choose a reason for hiding this comment

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

It would be nice to have another maintainers opinion on the use of cpu specific module selection in an application.

tests/periph_pm/Makefile.board.dep
# selects ztimer_no_periph_rtt to select the same modules in Kconfig and make
ifneq (,$(filter-out sam3,$(filter sam%,$(CPU))))
ifneq (,$(filter ztimer%,$(USEMODULE)))
USEMODULE += ztimer_no_periph_rtt
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems this works and it is commented well enough. I feel like cpu specific code should not be in the application, but as this is somehow a workaround for the rather complex rtt/ztimer maybe we allow it for now until a better solution is found.

Copy link
Contributor

Choose a reason for hiding this comment

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

Makefile.board.dep is there exactly for this reason, sure most of the time its based on $(BOARD) but IMO this is exactly the kind of usage. So far we have force Kconfig to match make weird behaviour, I see no harm in doing it the other way around here. IMO we shouldn't stall this one!

Copy link
Contributor

Choose a reason for hiding this comment

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

Fair enough. It is documented and everything is passing. Please squash!

@MrKevinWeiss
Copy link
Contributor

maybe ping @maribu or @kaspar030 ?

@fjmolinas fjmolinas force-pushed the pr/boards/samd21/model_kconfig branch from 456e146 to f18bd93 Compare December 15, 2021 09:30
leandrolanzieri and others added 10 commits December 15, 2021 10:30
@leandrolanzieri @fjmolinas
drivers: add Kconfig hardware feature to various drivers
14d986d 
- fxos8700
- hdc1000
- pir
- pulse_counter
@leandrolanzieri @fjmolinas
boards/common/samdx1-arduino-bootloader: model Kconfig
d249fc5
@leandrolanzieri @fjmolinas
boards/common/arduino-mkr: model Kconfig
7fe6eba
@leandrolanzieri @fjmolinas
boards/common/arduino-zero: model Kconfig
ea0039f
@leandrolanzieri @fjmolinas
boards/common/sodaq: model Kconfig
6ecbfe9
@leandrolanzieri @fjmolinas
boards/samd21-based: model Kconfig
8e7691e
@leandrolanzieri @fjmolinas
.murdock: add subset of samd21 boards to test Kconfig
359fdda
@leandrolanzieri @fjmolinas
sys/arduino/kconfig: use periph_uart
8bcd7e5
@leandrolanzieri @fjmolinas
tests/periph_rtc/kconfig: do not use rtt on ztimer
1dd3b38
@fjmolinas
tests/periph_pm: select ztimer_no_periph_rtt for sam0 boards
600acb7 
For sam0 there is a conflict between rtt and rtc, make resolves
this based on feature conflicts and the feature to be included
depends on dependency resolution.

Kconfig can't rely on order of inclusion therefore ztimer_no_periph_rtt
is implied to aboid ztimer_msec selecting rtt (its the case for BOARDs
using stdio_rtt
@fjmolinas fjmolinas force-pushed the pr/boards/samd21/model_kconfig branch from f18bd93 to 600acb7 Compare December 15, 2021 09:31
@fjmolinas
Copy link
Contributor

Rebase to fix conflict

MrKevinWeiss
MrKevinWeiss approved these changes Dec 15, 2021
View reviewed changes
Copy link
Contributor

@MrKevinWeiss MrKevinWeiss left a comment

Choose a reason for hiding this comment

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

ACK, if everything is documented and everything is green it is better to have the boards in then not. This should prevent the whack-a-mole problem with the Kconfig migration.

fjmolinas
fjmolinas approved these changes Dec 15, 2021
View reviewed changes
Copy link
Contributor

@fjmolinas fjmolinas left a comment

Choose a reason for hiding this comment

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

ACK! (all but my lately force-pushed changes)

@MrKevinWeiss MrKevinWeiss merged commit e3f6212 into RIOT-OS:master Dec 15, 2021
@MrKevinWeiss
Copy link
Contributor

Next will be #17402

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MrKevinWeiss MrKevinWeiss MrKevinWeiss approved these changes

@fjmolinas fjmolinas fjmolinas approved these changes

@benpicco benpicco Awaiting requested review from benpicco

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

@cgundogan cgundogan Awaiting requested review from cgundogan

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

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

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

@smlng smlng Awaiting requested review from smlng

Assignees
No one assigned
Labels
Area: arduino API Area: Arduino wrapper API Area: boards Area: Board ports Area: CI Area: Continuous Integration of RIOT components Area: drivers Area: Device drivers Area: Kconfig Area: Kconfig integration Area: sys Area: System Area: tests Area: tests and testing framework CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR
Projects
None yet
Milestone
Release 2022.01
Development

Successfully merging this pull request may close these issues.
3 participants
@leandrolanzieri @fjmolinas @MrKevinWeiss