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.

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

boards: set CONFIG_BOARD default based on devicetree #49260

Closed
wants to merge 4 commits into from

Conversation

galak
Copy link
Collaborator

@galak galak commented Aug 19, 2022

No description provided.

Add "model" property to base.yaml as its a standard property defined
by the devicetree spec.  Being a simple standard property, add it to
the list of properties that are automatically handled.

Signed-off-by: Kumar Gala <galak@kernel.org>
Add function to return a string property value and "" under all
other conditions.

Signed-off-by: Kumar Gala <galak@kernel.org>
Set the default value of CONFIG_BOARD bases on root node "model"
property in devicetree.

Signed-off-by: Kumar Gala <galak@kernel.org>
Make <BOARD_DIR>/Kconfig.defconfig optional since for some boards
the only thing <BOARD_DIR>/Kconfig.defconfig is doing is setting
the default value for CONFIG_BOARD.  Its reasonable that now
the default can be set from devicetree from the root level
"model" property and thus boards don't need to specify a
Kconfig.default.

Signed-off-by: Kumar Gala <galak@kernel.org>
@mbolivar-nordic
Copy link
Contributor

This is cool, but makes some problems we have worse.

@tejlmand and I have been talking about how it's a problem that CONFIG_BOARD in Kconfig doesn't have to match BOARD in CMake. You see this most often in Arm v8-M boards with trustzone, where BOARD=foo_ns and BOARD=foo both have CONFIG_BOARD=foo. That's a bug in our opinion; consistency between BOARD and CONFIG_BOARD should be enforced. This PR would make that harder to do.

@tejlmand
Copy link
Collaborator

tejlmand commented Aug 19, 2022

Agree with @mbolivar-nordic.

The principle is cool, but makes inconsistency worse.
Another issue that will arise is the sysbuild intregration.

Already now it's hard impossible to reuse the Kconfig tree for boards and SoC, but ideally we would like to be able to source part of the tree into sysbuild.
Namely the generic parts containing names but potentially other things, this is not fully decided.
Reason for this, is one may be like to have images available based on board / SoC / hw.

For example enabling a netcore image, such as HCI RPMsg, when building on a nRF5340, but not other boards.
Same could be true for features like Trustzone.

See also: #43488

And sysbuild is not (yet) using devicetree, so sourcing information from there makes things more complex.
Though system devicetree may change the situation quite significantly in future.

@galak
Copy link
Collaborator Author

galak commented Aug 19, 2022

This is cool, but makes some problems we have worse.

@tejlmand and I have been talking about how it's a problem that CONFIG_BOARD in Kconfig doesn't have to match BOARD in CMake. You see this most often in Arm v8-M boards with trustzone, where BOARD=foo_ns and BOARD=foo both have CONFIG_BOARD=foo. That's a bug in our opinion; consistency between BOARD and CONFIG_BOARD should be enforced. This PR would make that harder to do.

@mbolivar-nordic @tejlmand so why don't we effectively add -DCONFIG_BOARD= to set this to match and remove the Kconfig.defconfig setting of CONFIG_BOARD?

model:
type: string
required: false
description: specifies the manufacturer’s model number of the device
Copy link
Member

Choose a reason for hiding this comment

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

s/number/name

@mbolivar-nordic
Copy link
Contributor

why don't we effectively add -DCONFIG_BOARD= to set this to match and remove the Kconfig.defconfig setting of CONFIG_BOARD?

I don't get what you mean, sorry.

@galak
Copy link
Collaborator Author

galak commented Aug 30, 2022

I don't get what you mean, sorry.

I mean if CONFIG_BOARD is suppose to match the cmake -DBOARD, why don't we enforce/pass that through from Cmake to Kconfig. Instead of having CONFIG_BOARD set in Kconfig files?

@tejlmand
Copy link
Collaborator

I don't get what you mean, sorry.

I mean if CONFIG_BOARD is suppose to match the cmake -DBOARD, why don't we enforce/pass that through from Cmake to Kconfig. Instead of having CONFIG_BOARD set in Kconfig files?

work has been taken up by @nordicjm on this, if curious, see this commit for details: ab05d91

@mbolivar-nordic
Copy link
Contributor

see this commit for details: ab05d91

This commit isn't helpful; do you have a branch pointer?

@galak
Copy link
Collaborator Author

galak commented Sep 13, 2022

This commit isn't helpful; do you have a branch pointer?

I assume #50154 is related to what @tejlmand mentioned.

@tejlmand
Copy link
Collaborator

tejlmand commented Sep 14, 2022

This commit isn't helpful; do you have a branch pointer?

@mbolivar-nordic github is playing tricks here. 👿
Cause the link takes you directly to a commit, without PR information, but if you look at the real link in the message (click edit on that message), then this is the link it contains:

https://github.com/zephyrproject-rtos/zephyr/pull/50154/commits/ab05d915496d40f0007a9f95f7aa15960ba2bb9a

which includes the PR number 50154, which is why I posted the link like that.
The interesting PR is part of the commit link, so everyone should be able to locate both pieces of information 😄

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

Successfully merging this pull request may close these issues.

None yet

5 participants