Skip to content

boards/same54-xpro: Model features in Kconfig#14483

Merged
aabadie merged 3 commits intoRIOT-OS:masterfrom
leandrolanzieri:pr/kconfig/samd5x_based_symbols
Jul 16, 2020
Merged

boards/same54-xpro: Model features in Kconfig#14483
aabadie merged 3 commits intoRIOT-OS:masterfrom
leandrolanzieri:pr/kconfig/samd5x_based_symbols

Conversation

@leandrolanzieri
Copy link
Copy Markdown
Contributor

Contribution description

This models the features of the same54-xpro board, the only one that uses the samd5x CPU so far.

Testing procedure

  • Check that symbol organization and naming are correct
  • Green CI: tests/kconfig_features should pass

Issues/PRs references

Part of #14148

@leandrolanzieri leandrolanzieri added Type: new feature The issue requests / The PR implemements a new feature for RIOT Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer Area: Kconfig Area: Kconfig integration labels Jul 10, 2020
Comment on lines +32 to +33
config CPU_MODEL
default "same54p20a" if CPU_MODEL_SAME54P20A
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.

Just noticed that now, but do we have to maintain a list of all MCUs here again?
I was quite happy that we would just have to set CPU_MODEL and the right vendor file and parameters get selected automatically just based on that.

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.

Well, when we switch to Kconfig CPU_MODEL will no longer be set in the Makefiles, we will be able to use the variables CONFIG_CPU_MODEL_<model> (boolean) and CONFIG_CPU_MODEL (string). This last one will be the same as CPU_MODEL currently.


## CPU common symbols
config CPU_FAM
default "samd5x" if CPU_FAM_SAMD5X
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.

In #14485 you split that up into CPU_FAM_SAML10 and CPU_FAM_SAML11.
Should we have at least CPU_FAM_SAMD51 and CPU_FAM_SAME54 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.

In #14485 I followed the existing families. If it makes sense I can do the same here, but it should also be changed in the Makefile.features.

Copy link
Copy Markdown
Contributor

@aabadie aabadie left a comment

Choose a reason for hiding this comment

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

Looks good, please rebase!

@leandrolanzieri leandrolanzieri force-pushed the pr/kconfig/samd5x_based_symbols branch from b7d0bb6 to 580bd08 Compare July 16, 2020 13:34
@leandrolanzieri leandrolanzieri added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Jul 16, 2020
@leandrolanzieri leandrolanzieri added this to the Release 2020.10 milestone Jul 16, 2020
@aabadie
Copy link
Copy Markdown
Contributor

aabadie commented Jul 16, 2020

All green, GO!

@aabadie aabadie merged commit a4129d0 into RIOT-OS:master Jul 16, 2020
@leandrolanzieri leandrolanzieri deleted the pr/kconfig/samd5x_based_symbols branch July 16, 2020 14:40
@leandrolanzieri
Copy link
Copy Markdown
Contributor Author

Thanks for reviewing !

@benpicco
Copy link
Copy Markdown
Contributor

I'm still unhappy about that CPU_MODEL ordeal as it makes dealing with external boards unnecessary cumbersome.
I've seen the same in the stm32 PR.

I'm afraid there is no way around maintaining such lists, Zephyr does it too.
I'll try to come up with a script to generate those automatically from vendor files

@aabadie
Copy link
Copy Markdown
Contributor

aabadie commented Jul 16, 2020

I'll try to come up with a script to generate those automatically from vendor files

For STM32, I don't see how this can be achieved automatically from vendor files, since some of them are used for multiple CPUs. The specific digit/letters at the end of the CPU model name are here to determine the number of pins, RAM len, ROM len, etc.

@benpicco
Copy link
Copy Markdown
Contributor

The specific digit/letters at the end of the CPU model name are here to determine the number of pins, RAM len, ROM len, etc.

Can't we do something like this and use XX there?

@aabadie
Copy link
Copy Markdown
Contributor

aabadie commented Jul 16, 2020

In RIOT, the precise STM32 CPU model name is required to determine RAM/ROM len, and other things. We have to set this information somewhere. I don't know yet how it will be handled latter. For the moment, during the migration, we must stick to the way it's done with Make, so using complete CPU model names.

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

Labels

Area: Kconfig Area: Kconfig integration CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer Type: new feature The issue requests / The PR implemements a new feature for RIOT

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants