Skip to content

Conversation

@decsny
Copy link
Member

@decsny decsny commented Oct 5, 2022

Clarify in the board porting guide that the board name does not come from the directory name

@decsny decsny requested a review from nashif as a code owner October 5, 2022 22:01
Copy link
Contributor

@nordicjm nordicjm left a comment

Choose a reason for hiding this comment

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

Marking as NACK because this is not true for the v2 board rework - see #50305

@nordicjm
Copy link
Contributor

nordicjm commented Oct 6, 2022

CC: @tejlmand

@decsny
Copy link
Member Author

decsny commented Oct 10, 2022

Marking as NACK because this is not true for the v2 board rework - see #50305

So should I add a clarification that this is just for v1 boards? The reason I am adding this clarification is because I have found this is a source of confusion for some people, and not documented anywhere else as far as I can tell.

Copy link
Contributor

@tejlmand tejlmand left a comment

Choose a reason for hiding this comment

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

Thanks for highlighting this.
I'm sure the description of board dir vs. board itself can be improved, but the current proposal is not precise enough.

Comment on lines 357 to 358
Copy link
Contributor

Choose a reason for hiding this comment

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

Not precise.

The build system will use the BOARD provided by the user with -DBOARD=<board> (or -b with west build).

From there the build system looks up board related files, such as <board>_defconfig for Kconfig, <board>.dts for devicetree, doc/<board>.png for documentation, <board>.yaml for twister.

In an upstream board all those files are required, but for a downstream board only the Kconfig is mandatory, cause a downstream board doesn't have to use devicetree (although most does).

This is why the check for a _defconfig is the first thing that is checked, because if this file is missing, the user has typed a wrong board or the board impl is wrong.
Ref:

find_path(BOARD_DIR
NAMES ${BOARD}_defconfig

Also because the _defconfig is the only mandatory file, then west boards uses this file to search all valid board names.

I agree, the fact that the name of the board dir doesn't need to be identical the board itself can cause confusion, for example like: nrf5340dk_nrf5340 dir contains the boards nrf5340dk_nrf5340_cpunet, nrf5340dk_nrf5340_cpuapp, nrf5340dk_nrf5340_cpuapp_ns.
But this should be fixed by improving the overall description of the board folder vs. the actual board, not by making imprecise description of the files describing the actual board.

@decsny decsny requested review from nordicjm and tejlmand October 23, 2022 14:21
Copy link
Contributor

Choose a reason for hiding this comment

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

not sure that helped.

Users must give a board when invoking a build, else you get an error like this:

CMake Error at /projects/github/ncs/zephyr/cmake/modules/extensions.cmake:2501 (message):
  BOARD is not being defined on the CMake command-line, in the environment or
  by the app.

Copy link
Member Author

Choose a reason for hiding this comment

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

Do you have any suggestion about where/what the clarification about the board name should be in the docs?

Copy link
Contributor

@tejlmand tejlmand Oct 31, 2022

Choose a reason for hiding this comment

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

I gave a recommendation here:
#51011 (comment)

But this should be fixed by improving the overall description of the board folder vs. the actual board, not by making imprecise description of the files describing the actual board.

So probably at the start of this:
https://docs.zephyrproject.org/latest/hardware/porting/board_porting.html#create-your-board-directory

Either update the existing text, or create a note.

In case of a note, could be something along:

.. note::

   A board and the board dir can be named differently. In fact a board dir can contain multi boards,
   this is used for multi-core SoC where the board folder may be named `plank`, but each core is
   referenced according to the SoC, like:
   `boards/<ARCH>/plank` may hold files for `plank_core_a`, `plank_core_b`, etc.

   Each logical board in such a setup must contain all individual files described below for each core.

Feel free to adjust the description in any way you like, so that you believe it would have made it easier to understand the behavior.

Copy link
Member Author

@decsny decsny Nov 1, 2022

Choose a reason for hiding this comment

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

sorry about that, I misunderstood what you meant before but I reread it and now I get your points

I updated the PR, do you agree with the new changes? @tejlmand

@decsny decsny requested a review from tejlmand November 1, 2022 14:18
Copy link
Contributor

@tejlmand tejlmand left a comment

Choose a reason for hiding this comment

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

looks much better.

Be careful on the line length.

Copy link
Contributor

Choose a reason for hiding this comment

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

target can mean a lot of things in the build system.

How about:

Suggested change
The board directory name does not actually need to match the name of the board. Multiple boards can even defined be in one directory. The most common example of this is for boards with multiple targets, such as each core of a multi-core soc.
The board directory name does not need to match the name of the board.
Multiple boards can be defined in one directory.
The most common example of this is boards with a muti-core SoC where each core
is created as a logical board following the board naming scheme
`<board>_<soc-core>`.

Also please pay attention to you line length.

Copy link
Member Author

Choose a reason for hiding this comment

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

thank you for your suggestions

Comment on lines 165 to 172
Copy link
Contributor

Choose a reason for hiding this comment

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

For downstream boards, the only mandatory file is the _defconfig file. This is because downstream boards only really need to use Kconfig, while upstream boards are required to be documented and support devicetree, twister, and Kconfig.

Those files are already described as The mandatory files are: so why also a note ?

Also, in the board porting guide we should not describe that devicetree file is optional, cause that is considered very advanced usage.

Describing dts this way can lead some users into confusion and other things will not work as expected if they start to not create the dts file.

If this file does not exist, then the build system will assume the user has selected an invalid board.

this is really an implementation detail of how the build system work.
If anything should be done in this regard, then it will probably be better to improve the error message and not just print valid boards.

Copy link
Member Author

Choose a reason for hiding this comment

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

Okay mainly I just wanted to point out the difference for downstream boards, I will fix this

@decsny decsny requested a review from tejlmand November 8, 2022 17:29
Comment on lines 169 to 172
Copy link
Contributor

Choose a reason for hiding this comment

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

no, this is still not correct.

If you follow the board porting guide, then the devicetree file is mandatory.
As well as if you use and architecture natively supported by Zephyr, such as arm, riscv, etc.

But if you're implementing your own arch, soc, and board, then you are able to not use dt under some very strict conditions. Therefore, this is really advanced users, and not something to describe in board porting guide (which targets users needing to create a new board based on an existing SoC in Zephyr (and thus is based on an existing arch).

For a board porting guide that describes how to add your own board (using an arch defined in Zephyr), then the dts file is mandatory.

From the build system perspective though, things are a little more complex, hence the reason I mentioned why the dts file cannot be used to determine if a board is valid.

Suggested change
.. note::
For **downstream** boards, the only mandatory file is the _defconfig file.

Copy link
Member Author

Choose a reason for hiding this comment

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

okay, I will just delete this part. It wasn't the main purpose of this PR

@decsny decsny requested a review from tejlmand November 15, 2022 16:15
@decsny
Copy link
Member Author

decsny commented Nov 28, 2022

@tejlmand @nordicjm Could you guys take another look at this to see if the changes address your concerns

Comment on lines 135 to 136
Multiple boards can even defined be in one directory.
The most common example of this is for boards with multi-core SoC,
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Multiple boards can even defined be in one directory.
The most common example of this is for boards with multi-core SoC,
Multiple boards can even be defined in one directory.
The most common example of this is for boards with a multi-core SoC,

Multiple boards can even defined be in one directory.
The most common example of this is for boards with multi-core SoC,
where each core is usually created as a logical board following the
naming scheme `<board>_<soc-core>`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure I really like this. E.g. thingy53_nrf5340_cpuapp, thingy53_nrf5340_cpuapp_ns, thingy53_nrf5340_cpunet
The board is thingy53, the soc is nrf5340, the core, which uses an underscore, is cpuapp/cpunet, and there is optionally "_ns" for the application core for the non-secure domain. I don't think this text here really explains it, more so it makes it more confusing.

Copy link
Contributor

@tejlmand tejlmand Dec 1, 2022

Choose a reason for hiding this comment

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

I guess the point here is that a board dir may contain multiple board definitions.
Cause that itself is not always clear to users.

The board is thingy53, the soc is nrf5340

Technically yes, but in Zephyr, the thingy53_nrf5340 is considered the board in Zephyr context.
It's only because Nordic has decided to include the SoC in the name of the board.
Other boards may not do so, like the mimxrt1170_evk

So the questions which remains are:

  • How do we inform the users that a board dir may contain multiple board definitions
  • Why and when is this possibility used ?
    For example users may do this for a board with a multi core SoC as described, but they should not start to put board foo and bar into a common board folder, even though that technically is possible.

@nordicjm do you have any proposal on how to provide this information to the reader in a short text, which doesn't have to deal with the fact that company A uses one naming convention, and company B another ?

Notice the text says:

where each core is usually created as a logical board following the naming scheme <board>_<soc-core>

so that means some boards may not do it like this or follow this naming scheme.

Because this text is short, and the main point is to inform users that a board dir may contain multiple board definitions with a small reasoning of why, then I'm giving +1 because I believe it's improving the existing documentation.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah the text here is just supposed to be an example and is supposed to show that the board directory name doesn't necessarily match the board name. But I suppose I could add a sentence clarifying this even further if it is a point of confusion still.

tejlmand
tejlmand previously approved these changes Dec 1, 2022
Copy link
Contributor

@tejlmand tejlmand left a comment

Choose a reason for hiding this comment

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

Thanks for this improvement.

Please address the comment here: #51011 (comment)

Clarify:
- Board directory name does not need to match board name
- Give example of multi board directory

Signed-off-by: Declan Snyder <[email protected]>
@decsny decsny requested a review from tejlmand December 5, 2022 15:45
@fabiobaltieri fabiobaltieri merged commit 62abf16 into zephyrproject-rtos:main Dec 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants