Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion cmake/modules/soc_v2.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,6 @@ if(HWMv2)
set(SOC_TOOLCHAIN_NAME ${CONFIG_SOC_TOOLCHAIN_NAME})
set(SOC_FAMILY ${CONFIG_SOC_FAMILY})
set(SOC_V2_DIR ${SOC_${SOC_NAME}_DIR})
set(SOC_FULL_DIR ${SOC_V2_DIR})
set(SOC_FULL_DIR ${SOC_V2_DIR} CACHE PATH "Path to the SoC directory." FORCE)
set_property(DIRECTORY APPEND PROPERTY CMAKE_CONFIGURE_DEPENDS ${SOC_V2_DIR}/soc.yml)
endif()
106 changes: 106 additions & 0 deletions doc/build/flashing/configuration.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
.. _flashing-soc-board-config:

Flashing configuration
######################

Zephyr supports setting up configuration for flash runners (invoked from
:ref:`west flash<west-flashing>`) which allows for customising how commands are used when
programming boards. This configuration is used for :ref:`sysbuild` projects and allows for
configuring when commands are ran for groups of board targets. As an example: a multi-core SoC
might want to only allow the ``--erase`` argument to be used once for all of the cores in the SoC
which would prevent multiple erase tasks running in a single ``west flash`` invocation, which
could wrongly clear the memory which is used by the other images being programmed.

Priority
********

Flashing configuration is singular, it will only be read from a single location, this
configuration can reside in the following files starting with the highest priority:

* ``soc.yml`` (in soc folder)
* ``board.yml`` (in board folder)

Configuration
*************

Configuration is applied in the yml file by using a ``runners`` map with a single ``run_once``
child, this then contains a map of commands as they would be provided to the flash runner e.g.
``--reset`` followed by a list which specifies the settings for each of these commands (these
are grouped by flash runner, and by qualifiers/boards). Commands have associated runners that
they apply to using a ``runners`` list value, this can contain ``all`` if it applies to all
runners, otherwise must contain each runner that it applies to using the runner-specific name.
Groups of board targets can be specified using the ``groups`` key which has a list of board
target sets. Board targets are regular expression matches, for ``soc.yml`` files each set of
board targets must be in a ``qualifiers`` key (only regular expression matches for board
qualifiers are allowed, board names must be omitted from these entries). For ``board.yml``
files each set of board targets must be in a ``boards`` key, these are lists containing the
matches which form a singular group. A final parameter ``run`` can be set to ``first`` which
means that the command will be ran once with the first image flashing process per set of board
targets, or to ``last`` which will be ran once for the final image flash per set of board targets.

An example flashing configuration for a ``soc.yml`` is shown below in which the ``--recover``
command will only be used once for any board targets which used the nRF5340 SoC application or
network CPU cores, and will only reset the network or application core after all images for the
respective core have been flashed.

.. code-block:: yaml

runners:
run_once:
'--recover':
- run: first
runners:
- nrfjprog
groups:
- qualifiers:
- nrf5340/cpunet
- nrf5340/cpuapp
- nrf5340/cpuapp/ns
'--reset':
- run: last
runners:
- nrfjprog
- jlink
groups:
- qualifiers:
- nrf5340/cpunet
- qualifiers:
- nrf5340/cpuapp
- nrf5340/cpuapp/ns
# Made up non-real world example to show how to specify different options for different
# flash runners
- run: first
runners:
- some_other_runner
groups:
- qualifiers:
- nrf5340/cpunet
- qualifiers:
- nrf5340/cpuapp
- nrf5340/cpuapp/ns

Usage
*****

Commands that are supported by flash runners can be used as normal when flashing non-sysbuild
applications, the run once configuration will not be used. When flashing a sysbuild project with
multiple images, the flash runner run once configuration will be applied.

For example, building the :zephyr:code-sample:`smp-svr` sample for the nrf5340dk which will
include MCUboot as a secondary image:

.. code-block:: console

cmake -GNinja -Sshare/sysbuild/ -Bbuild -DBOARD=nrf5340dk/nrf5340/cpuapp -DAPP_DIR=samples/subsys/mgmt/mcumgr/smp_svr
cmake --build build

Once built with an nrf5340dk connected, the following command can be used to flash the board with
both applications and will only perform a single device recovery operation when programming the
first image:

.. code-block:: console

west flash --recover

If the above was ran without the flashing configuration, the recovery process would be ran twice
and the device would be unbootable.
9 changes: 9 additions & 0 deletions doc/build/flashing/index.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
.. _flashing:

Flashing
########

.. toctree::
:maxdepth: 1

configuration.rst
1 change: 1 addition & 0 deletions doc/build/index.rst
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,4 @@ Build and Configuration Systems
zephyr_cmake_package.rst
sysbuild/index.rst
version/index.rst
flashing/index.rst
2 changes: 1 addition & 1 deletion doc/releases/release-notes-3.7.rst
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ Build system and Infrastructure
devicetree overlays that should apply to any board target using a particular SoC and board
qualifier.

* Compiler
* :ref:`Board/SoC flashing configuration<flashing-soc-board-config>` settings have been added.

* Deprecated the global CSTD cmake property in favor of the :kconfig:option:`CONFIG_STD_C`
choice to select the C Standard version. Additionally subsystems can select a minimum
Expand Down
35 changes: 35 additions & 0 deletions scripts/list_hardware.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import sys
from typing import List
import yaml
import re


SOC_SCHEMA_PATH = str(Path(__file__).parent / 'schemas' / 'soc-schema.yml')
Expand Down Expand Up @@ -40,6 +41,40 @@ def __init__(self, folder='', soc_yaml=None):
except (yaml.YAMLError, pykwalify.errors.SchemaError) as e:
sys.exit(f'ERROR: Malformed yaml {soc_yaml.as_posix()}', e)

# Ensure that any runner configuration matches socs and cpuclusters declared in the same
# soc.yml file
if 'runners' in data and 'run_once' in data['runners']:
for grp in data['runners']['run_once']:
for item_data in data['runners']['run_once'][grp]:
for group in item_data['groups']:
for qualifiers in group['qualifiers']:
components = qualifiers.split('/')
soc = components.pop(0)
found_match = False

# Allow 'ns' as final qualifier until "virtual" CPUs are ported to soc.yml
# https://github.com/zephyrproject-rtos/zephyr/issues/70721
if len(components) > 0 and components[len(components)-1] == 'ns':
components.pop(len(components)-1)

for f in data.get('family', []):
for s in f.get('series', []):
for socs in s.get('socs', []):
if re.match(fr'^{soc}$', socs.get('name')) is not None:
if 'cpuclusters' in socs and len(components) > 0:
check_string = '/'.join(components)
for cpucluster in socs.get('cpuclusters', []):
if re.match(fr'^{check_string}$', cpucluster.get('name')) is not None:
found_match = True
break
elif 'cpuclusters' not in socs and len(components) == 0:
found_match = True
break


if found_match is False:
sys.exit(f'ERROR: SoC qualifier match unresolved: {qualifiers}')

for f in data.get('family', []):
family = Family(f['name'], folder, [], [])
for s in f.get('series', []):
Expand Down
51 changes: 51 additions & 0 deletions scripts/schemas/board-schema.yml
Original file line number Diff line number Diff line change
Expand Up @@ -78,3 +78,54 @@ mapping:
type: seq
sequence:
- include: board-schema
runners:
type: map
mapping:
run_once:
Copy link
Contributor

Choose a reason for hiding this comment

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

Now that run: ['first', 'last'] exists, then why should we have this extra run_once key ?

Running something first or last would imply only running it once.

Also, removing run_once at this level would make run: ['first', 'last'] extensible with something like force, always.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because additional config might be added in future that has no relation to running once

Copy link
Contributor

Choose a reason for hiding this comment

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

doesn't really answer the question of run_once.

Could just be run or another keyword, as there is no reason to separated run_once from another run_foo when we have run: ['first', 'last']. When needed, then enum can support more run types in future instead of multiple levels.

So once just limit ourselves for no practical reason.

type: map
desc: |
Allows for restricting west flash commands when using sysbuild to run once per given
grouping of board targets. This is to allow for future image program cycles to not
erase the flash of a device which has just been programmed by another image.
mapping:
regex;(.*):
Copy link
Contributor

Choose a reason for hiding this comment

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

this makes the command to become a key in the yaml file, not a value.

As I see it, there are two main implecations of such approach

  • The free form .* means there is effectively no schema check, as anything is allowed for a value.
  • A : must suddenly follow the value.
    In yaml, a value is either after the colon, like:
    <key>: <value>
    or a list of values, like:
    my_list:
    - value1
    - value2
    - value3
    - ...
    

but you're creating a list of maps, but where the key in that list is actually the value.

I don't see why regular key: value mapping cannot be used, and thereby avoid tricks like this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unification with snippets

Copy link
Contributor

Choose a reason for hiding this comment

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

but not unified with boards.yml.

For example, the board.yml for nrf9160dk does:

board:
  name: nrf9160dk
  vendor: nordic
  socs:
  - name: nrf9160
    variants:
    - name: 'ns'
  - name: nrf52840

and not:

board:
  name: nrf9160dk
  vendor: nordic
  socs:
  - nrf9160:
      variants:
      - name: 'ns'
  - nrf52840:

The fact that snippet does this does not justify this principle in board or soc yaml files.

Especially not when this proposal introduces inconsistency on how things are done within the same yaml file.

Copy link
Contributor

Choose a reason for hiding this comment

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

See also comment here: #69748 (comment)

type: seq
desc: |
A dictionary of commands which should be limited to running once per invocation
of west flash for a given set of flash runners and board targets.
sequence:
- type: map
mapping:
run:
required: true
type: str
enum: ['first', 'last']
desc: |
If first, will run this command once when the first image is flashed, if
last, will run this command once when the final image is flashed.
runners:
required: true
type: seq
sequence:
- type: str
Comment on lines +109 to +110
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems to allow users to refer to any board, regardless of this board being described in this yaml or not.

For example:

board:
  name: plank
  socs:
  - name: soc1
run_once:
  - '--a-command':
     - boards:
        - 'foo'

It should not be possible to reference boards beside those defined by this board.yml.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We have a huge problem with west boards being incredibly slow because of schema validation, and to add extra overhead on top of this of then doing regex matching every time it is invoked does not really make sense. Verification as part of compliance in CI makes sense but whenever someone runs west flash no

Copy link
Contributor

Choose a reason for hiding this comment

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

and to add extra overhead on top of this of then doing regex

are your really arguing that a regex like this is faster ?

        mapping:
          regex;(.*):

then I would like to see some performance numbers.

Copy link
Contributor

@tejlmand tejlmand Mar 19, 2024

Choose a reason for hiding this comment

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

I still lack some performance numbers.

Also, this principle of using values for keys through the regex has high risk of users wrongly indenting yaml code, for example:

    - '--recover':
      run: first
      runners:
      - nrfjprog
      - jlink

is wrong, but this works:

    - '--recover':
            run: first
            runners:
            - nrfjprog
            - jlink

This minor detail of remembering the extra indentation is extremely important but may not be obvious to everyone, and the schema validation failure is not helpful at all.

Thus, I still argue that a normal key-value pair is much better, in this case like this:

    - command: '--recover'
      run: first
      runners:
      - nrfjprog
      - jlink

No surprises to users on how to do indentation.

Copy link
Contributor

@tejlmand tejlmand Mar 19, 2024

Choose a reason for hiding this comment

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

as for performance, the first reason for west boards being slow is the split in almost 500 boards.yml files, which all need to be validated, not whether you try to sub-optimize the key.

In fact, the overhead of west itself is quite significant.
Running west boards --board foo gives: (--board foo is to avoid time overhead of printing)

$ time west boards --board foo
_______________________________________________________
Executed in  715.04 millis    fish           external
   usr time  698.32 millis    0.00 micros  698.32 millis
   sys time   16.36 millis  304.00 micros   16.05 millis

but removing west and use list_boards.py directly, give you:

$ time ./scripts/list_boards.py --board-root=(pwd) --arch-root=(pwd) --soc-root=(pwd) --board=foo
________________________________________________________
Executed in  543.97 millis    fish           external
   usr time  514.78 millis    0.00 micros  514.78 millis
   sys time   28.28 millis  350.00 micros   27.93 millis

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also, this principle of using values for keys through the regex has high risk of users wrongly indenting yaml code, for example:
This minor detail of remembering the extra indentation is extremely important but may not be obvious to everyone, and the schema validation failure is not helpful at all.

The user won't get into that situation because the schema would fail

Copy link
Contributor

Choose a reason for hiding this comment

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

The user won't get into that situation because the schema would fail

any user creating a oot board has the risk of falling into this.

And any Zephyr contributor can waste a lot of time on this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Then I suggest you improve the error reporting by submitting a PR to https://pypi.org/project/pykwalify/

desc: |
A list of flash runners that this applies to, can use `all` to apply
to all runners.
groups:
required: true
type: seq
sequence:
- type: map
desc: |
A grouping of board targets which the command should apply to. Can
be used multiple times to have multiple groups.
mapping:
boards:
required: true
type: seq
sequence:
- type: str
desc: |
A board target to match against in regex. Must be one entry
per board target, a single regex entry will not match two
board targets even if they both match.
51 changes: 51 additions & 0 deletions scripts/schemas/soc-schema.yml
Original file line number Diff line number Diff line change
Expand Up @@ -70,3 +70,54 @@ mapping:
required: false
type: str
desc: Free form comment with extra information regarding the SoC.
runners:
type: map
mapping:
run_once:
type: map
desc: |
Allows for restricting west flash commands when using sysbuild to run once per given
grouping of board targets. This is to allow for future image program cycles to not
erase the flash of a device which has just been programmed by another image.
mapping:
regex;(.*):
type: seq
desc: |
A dictionary of commands which should be limited to running once per invocation
of west flash for a given set of flash runners and board targets.
sequence:
- type: map
mapping:
run:
required: true
type: str
enum: ['first', 'last']
desc: |
If first, will run this command once when the first image is flashed, if
last, will run this command once when the final image is flashed.
runners:
required: true
type: seq
sequence:
- type: str
desc: |
A list of flash runners that this applies to, can use `all` to apply
to all runners.
groups:
required: true
type: seq
sequence:
- type: map
desc: |
A grouping of board targets which the command should apply to. Can
be used multiple times to have multiple groups.
mapping:
qualifiers:
required: true
type: seq
sequence:
- type: str
desc: |
A board qualifier to match against in regex form. Must be one
entry per board target, a single regex entry will not match
two board targets even if they both match.
Loading