Skip to content

boards/stm32-based: model Kconfig #17374

Merged
fjmolinas merged 13 commits intoRIOT-OS:masterfrom
MrKevinWeiss:pr/fix/kconfig/stms2
Dec 15, 2021
Merged

boards/stm32-based: model Kconfig #17374
fjmolinas merged 13 commits intoRIOT-OS:masterfrom
MrKevinWeiss:pr/fix/kconfig/stms2

Conversation

@MrKevinWeiss
Copy link
Copy Markdown
Contributor

@MrKevinWeiss MrKevinWeiss commented Dec 10, 2021 •
edited
Loading

Contribution description

Fixes modelling in many (but not all) stm32 boards:

  • alientek-pandora
  • blackpill
  • bluepill-128kib
  • bluepill-stm32f030c8
  • bluepill
  • iotlab-a8-m3
  • iotlab-m3
  • lobaro-lorabox
  • olimexino-stm32
  • p-l496g-cell02
  • stm32f429i-disco
  • stm32f769i-disco
  • stm32f723e-disco
  • stm32l0538-disco
  • stm32l476g-disco
  • stm32mp157c-dk2
  • ublox-c030-u201
  • weact-f401cc
  • weact-f401ce
  • weact-f411ce
  • ublox-c030-u201

Some boards will require more attention but any relevant modeling is done:

  • stm32mp157c-dk2 (clocks)
  • stm32f3discovery (? size diff)
  • stm32f030f4-demo (? size diff)
  • pyboard (? size the same)
  • nucleo-l011k4 (picolib)
  • lora-e5-dev (clocks)
  • nucleo-wl55jc(clocks)

Some drivers needed to be touched to allow saul_default to select them:

  • isl29020
  • l3g4200d
  • lm75
  • lpsxxx
  • lsm303dlhc
  • saul

Testing procedure

Green CI and a look over if it makes sense

Issues/PRs references

Part of #16875
Depends on #17376

@MrKevinWeiss MrKevinWeiss added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR CI: full build disable CI build filter labels Dec 10, 2021
@github-actions github-actions bot added Area: boards Area: Board ports Area: CI Area: Continuous Integration of RIOT components Area: cpu Area: CPU/MCU ports Area: drivers Area: Device drivers Area: Kconfig Area: Kconfig integration Area: SAUL Area: Sensor/Actuator Uber Layer Area: tests Area: tests and testing framework Platform: ARM Platform: This PR/issue effects ARM-based platforms labels Dec 10, 2021
MrKevinWeiss
MrKevinWeiss commented Dec 10, 2021
View reviewed changes
boards/stm32f429i-disco/Kconfig
select BOARD_HAS_LSE

source "$(RIOTBOARD)/common/stm32/Kconfig"
source "$(RIOTBOARD)/stm32f429i-disc1/Kconfig"
Copy link
Copy Markdown
Contributor Author

@MrKevinWeiss MrKevinWeiss Dec 10, 2021

Choose a reason for hiding this comment

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

Note that this is not the normal way we do it, usually this would be factored into a common board or something instead of just taking another board... This is what is done in the makefile and TBH, we should probably deprecate this board as it is super obsolete.

Copy link
Copy Markdown
Contributor

@fjmolinas fjmolinas Dec 14, 2021

Choose a reason for hiding this comment

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

@aabadie do you have the old or new version?

leandrolanzieri
leandrolanzieri reviewed Dec 10, 2021
View reviewed changes
boards/common/weact-f4x1cx/Makefile.features

# This configuration enables modules that are only available when using Kconfig
# module modelling
ifeq (1, $(TEST_KCONFIG))
Copy link
Copy Markdown
Contributor

@leandrolanzieri leandrolanzieri Dec 10, 2021

Choose a reason for hiding this comment

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

See #17376

Copy link
Copy Markdown
Contributor Author

@MrKevinWeiss MrKevinWeiss Dec 13, 2021

Choose a reason for hiding this comment

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

I can rebase after that gets in.

@MrKevinWeiss
Copy link
Copy Markdown
Contributor Author

MrKevinWeiss commented Dec 13, 2021

with #17376 the picolib issues can be resolved and I can add the nucleo-l011k4 and stm32f030f4-demo to the passing list.

@MrKevinWeiss MrKevinWeiss removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR CI: full build disable CI build filter labels Dec 13, 2021
@MrKevinWeiss
Copy link
Copy Markdown
Contributor Author

MrKevinWeiss commented Dec 13, 2021

Hmmm it seems that the current forcing of

# all cortex MCU's use newlib as libc
CONFIG_MODULE_NEWLIB=y

# use the nano-specs of Newlib when available
CONFIG_MODULE_NEWLIB_NANO=y

in cpu/cortexm_common/cortexm_common.config prevents the picolib from being selected from the board because it must select these things. Maybe I can try to override the choice instead...

@MrKevinWeiss MrKevinWeiss added the CI: full build disable CI build filter label Dec 13, 2021
@MrKevinWeiss MrKevinWeiss added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Dec 13, 2021
@MrKevinWeiss
Copy link
Copy Markdown
Contributor Author

MrKevinWeiss commented Dec 13, 2021

I think the choice thing is the way to go.

I now override the libc_implementation in the nucleo-l011k4 and stm32f030f4-demo and got rid of the hardcoding in the cortex common.

I am running the CI to see all failing stm boards, some I know will fail I just want an overview. Maybe I can get a few more in this PR but it might also be worth fixing what we have now and doing a follow-up later. Possibly after another check on all boards.

@MrKevinWeiss
Copy link
Copy Markdown
Contributor Author

MrKevinWeiss commented Dec 14, 2021

OK, another round. I think I got all obvious things (only binary diffs with the same modules are left). In the murdock file I indicated them with 2 spaces in front, but I guess this will be testing all boards anyways.
lora-e5-dev, nucleo-wl55jc, pyboard, stm32f3discovery, stm32mp157c-dk2.

Many of these are due to clock configurations which will be a different PR.

If everything is passes as expected I will clean things up, run another limited build and record the blacklisted boards and hopefully get this acked (then remove the limiting build commit)

@MrKevinWeiss
Copy link
Copy Markdown
Contributor Author

MrKevinWeiss commented Dec 14, 2021

OK rebased and now I am only checking the boards that should pass. After the review/ack I will squash and remove the murdock change (though I may add some boards to the list).

fjmolinas
fjmolinas approved these changes Dec 14, 2021
View reviewed changes
Copy link
Copy Markdown
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, looks good, please squash if murdock passes the limited build

@fjmolinas
Copy link
Copy Markdown
Contributor

fjmolinas commented Dec 14, 2021

All green please squash!

@fjmolinas fjmolinas mentioned this pull request Dec 14, 2021
Closed
@MrKevinWeiss
Copy link
Copy Markdown
Contributor Author

MrKevinWeiss commented Dec 15, 2021

OK, squashed and added a few boards to the regular testing. There may still be some errors in the few boards that were added as I only check a subset of apps.

We will see.

@MrKevinWeiss
Copy link
Copy Markdown
Contributor Author

MrKevinWeiss commented Dec 15, 2021

It seems like some periph_uart_fc_hw is missing from the makefile.features for the ublox and stm32f7***-disco board. I will add them in and directly squash.

@MrKevinWeiss
Copy link
Copy Markdown
Contributor Author

MrKevinWeiss commented Dec 15, 2021

Everything looks good if we want to merge?

@fjmolinas
Copy link
Copy Markdown
Contributor

fjmolinas commented Dec 15, 2021

Gooo!

@fjmolinas fjmolinas merged commit 31f5899 into RIOT-OS:master Dec 15, 2021
@MrKevinWeiss MrKevinWeiss deleted the pr/fix/kconfig/stms2 branch December 15, 2021 09:45
@fjmolinas fjmolinas added this to the Release 2022.01 milestone Jan 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@leandrolanzieri leandrolanzieri leandrolanzieri left review comments

@fjmolinas fjmolinas fjmolinas approved these changes

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

@cgundogan cgundogan Awaiting requested review from cgundogan

@DipSwitch DipSwitch Awaiting requested review from DipSwitch

@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

@vincent-d vincent-d Awaiting requested review from vincent-d

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

Assignees

No one assigned

Labels

Area: boards Area: Board ports Area: CI Area: Continuous Integration of RIOT components Area: cpu Area: CPU/MCU ports Area: drivers Area: Device drivers Area: Kconfig Area: Kconfig integration Area: SAUL Area: Sensor/Actuator Uber Layer Area: tests Area: tests and testing framework CI: full build disable CI build filter 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

Milestone

Release 2022.01

Development

Successfully merging this pull request may close these issues.

3 participants

@MrKevinWeiss @fjmolinas @leandrolanzieri