[PATCH v2 3/3] RFC: doc: Add documentation about devicetree usage

[Ilias Apalodimas]

Hi Simon,

+cc a few Linaro folks who have been looking at similar problems.

On Sat, 28 Aug 2021 at 19:46, Simon Glass <sjg at chromium.org> wrote:
>
> At present some of the ideas and techniques behind devicetree in U-Boot
> are assumed, implied or unsaid. Add some documentation to cover how
> devicetree is build, how it can be modified and the rules about using
> the various CONFIG_OF_... options.
>
> Signed-off-by: Simon Glass <sjg at chromium.org>
> ---
>
> Changes in v2:
> - Fix typos per Sean (thank you!) and a few others
> - Add a 'Use of U-Boot /config node' section
> - Drop mention of dm-verity since that actually uses the kernel cmdline
> - Explain that OF_BOARD will still work after these changes (in
>   'Once this bug is fixed...' paragraph)
> - Expand a bit on the reason why the 'Current situation' is bad
> - Clarify in a second place that Linux and U-Boot use the same devicetree
>   in 'To be clear, while U-Boot...'
> - Expand on why we should have rules for other projects in
>   'Devicetree in another project'
> - Add a comment as to why devicetree in U-Boot is not 'bad design'
> - Reword 'in-tree U-Boot devicetree' to 'devicetree source in U-Boot'
> - Rewrite 'Devicetree generated on-the-fly in another project' to cover
>   points raised on v1
> - Add 'Why does U-Boot have its nodes and properties?'
> - Add 'Why not have two devicetrees?'
>
>  doc/develop/index.rst              |   1 +
>  doc/develop/package/devicetree.rst | 563 +++++++++++++++++++++++++++++
>  doc/develop/package/index.rst      |   1 +
>  3 files changed, 565 insertions(+)
>  create mode 100644 doc/develop/package/devicetree.rst
>
> diff --git a/doc/develop/index.rst b/doc/develop/index.rst
> index 83c929babda..d5ad8f9fe53 100644
> --- a/doc/develop/index.rst
> +++ b/doc/develop/index.rst
> @@ -36,6 +36,7 @@ Packaging
>     :maxdepth: 1
>
>     package/index
> +   package/devicetree
>
>  Testing
>  -------
> diff --git a/doc/develop/package/devicetree.rst b/doc/develop/package/devicetree.rst
> new file mode 100644
> index 00000000000..d922d3f87ae
> --- /dev/null
> +++ b/doc/develop/package/devicetree.rst
> @@ -0,0 +1,563 @@
> +.. SPDX-License-Identifier: GPL-2.0+
> +
> +Updating the devicetree
> +=======================
> +
> +U-Boot uses devicetree for runtime configuration and storing required blobs or
> +any other information it needs to operate. It is possible to update the
> +devicetree separately from actually building U-Boot. This provides a good degree
> +of control and flexibility for firmware that uses U-Boot in conjunction with
> +other project.
> +
> +There are many reasons why it is useful to modify the devicetree after building
> +it:
> +
> +- Configuration can be changed, e.g. which UART to use
> +- A serial number can be added
> +- Public keys can be added to allow image verification
> +- Console output can be changed (e.g. to select serial or vidconsole)
> +
> +This section describes how to work with devicetree to accomplish your goals.
> +
> +See also :doc:`../devicetree/control` for a basic summary of the available
> +features.
> +
> +
> +Devicetree source
> +-----------------
> +
> +Every board in U-Boot must include a devicetree sufficient to build and boot
> +that board on suitable hardware (or emulation). This is specified using the
> +`CONFIG DEFAULT_DEVICE_TREE` option.
> +
> +
> +Current situation (August 2021)
> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> +
> +As an aside, at present U-Boot allows `CONFIG_DEFAULT_DEVICE_TREE` to be empty,
> +e.g. if `CONFIG_OF_BOARD` or `CONFIG_OF_PRIOR_STAGE` are used. This has
> +unfortunately created an enormous amount of confusion and some wasted effort.
> +This was not intended and this bug will be fixed soon.
> +
> +Some of the problems created are:
> +
> +- It is not obvious that the devicetree is coming from another project
> +
> +- There is no way to see even a sample devicetree for these platform in U-Boot,
> +  so it is hard to know what is going on, e.g. which devices are typically
> +  present
> +
> +- The other project may not provide a way to support U-Boot's requirements for
> +  devicetree, such as the /config node. Note: On the U-Boot mailing linst, this
> +  was only discovered after weeks of discussion and confusion

s/linst/list.

> +
> +- For QEMU specifically, consulting two QEMU source files is required, for which
> +  there are no references in U-Boot documentation. The code is generating a
> +  devicetree, but it is not clear what controls affect this generation.

QEMU command line args mostly.

> +
> +Specifically on the changes in U-Boot:
> +
> +- `CONFIG_OF_BOARD` was added in rpi_patch_ for Raspberry Pi, which does have
> +  an in-tree devicetree, but this feature has since been used for boards that
> +  don't
> +- `CONFIG_OF_PRIOR_STAGE` was added in bcm_patch_ as part of a larger Broadcom
> +  change with a tag indicating it only affected one board, so the change in
> +  behaviour was not noticed at the time. It has since been used by RISC-V qemu
> +  boards.
> +

I don't see why we need 2 config options tbh. Some of the existing
boards use CONFIG_OF_BOARD and have a built in function that reads a
specific memory address (eg. board/xilinx/common/board.c).
This fells like the exact same functionality named differently. It
basically means "someone else will provide the DTB". Any chance we can
get rid of one of them ?

> +Once this bug is fixed, CONFIG_OF_BOARD and CONFIG_OF_PRIOR_STAGE will override
> +(at runtime) the devicetree suppled with U-Boot, but will otherwise use

s/suppled/supplied

> +CONFIG_OF_SEPARATE for the in-tree build. So these two will become options,
> +moving out of the 'choice' in `dts/Kconfig`.

This isn't very clear, at least for me.  You are essentially saying
U-Boot will be provided with 2 devices trees? Aren't those board
suscptible to the problems you mentions in the "Two device trees
chapter"?

> +
> +This means that there is a basic devicetree build in the U-Boot tree, for
> +build-testing, consistency and documentation purposes, but at runtime U-Boot can
> +accept its devicetree from another source.
> +
> +To be clear, while U-Boot has its own copy of the devicetree source for each
> +board, this must match the Linux source, perhaps with some u-boot.dtsi
> +additions. The intent here is not to create a separate binding, just to provide
> +a representative devicetree in U-Boot.
> +
> +Offending boards are:
> +
> +- bcm7260
> +- bcm7445
> +- qemu_arm64
> +- qemu_arm
> +- qemu-ppce500
> +- qemu-riscv32
> +- qemu-riscv32_smode
> +- qemu-riscv64
> +- qemu-riscv64_smode
> +
> +All of these need to have a devicetree added in-tree. This is targeted to be
> +fixed in the 2022.01 release.
> +
> +
> +Building the devicetree
> +-----------------------
> +
> +U-Boot automatically builds the devicetree for a board, from the
> +`arch/<arch>/dts` directory. The Makefile in those directories has rules for
> +building devicetree files. It is preferable to avoid target-specific rules in
> +those files: i.e. all boards for a particular SoC should be built at once,
> +where practical. Apart from simplifying the Makefile, this helps to efficiently
> +(and immediately) ensure that changes in one board's DT do not break others that
> +are related. Building devicetrees is fast, so performance is seldom a concern
> +here.
> +
> +
> +Overriding the default devicetree
> +---------------------------------
> +
> +When building U-Boot, the `DEVICE_TREE` environment variable allows the
> +default devicetree file to be overridden at build time. This can be useful if
> +modifications have to be made to the in-tree devicetree file, for the benefit
> +of a downstream build system. Note that the in-tree devicetree must be
> +sufficient to build and boot, so this is not a way to bypass that requirement.
> +
> +
> +Modifying the devicetree after building
> +---------------------------------------
> +
> +While it is generally painful and hacky to modify the code or rodata of a
> +program after it is built, in many cases it is useful to do so, e.g. to add
> +configuration information like serial numbers, enabling/disabling features, etc.
> +
> +Devicetree provides a very nice solution to these problems since it is
> +structured data and it is relatively easy to change it, even in binary form
> +(see fdtput).
> +
> +U-Boot takes care that the devicetree is easily accessible after the build
> +process. In fact it is placed in a separate file called `u-boot.dtb`. If the
> +build system wants to modify or replace that file, it can do so. Then all that
> +is needed is to run `binman update` to update the file inside the image. If
> +binman is not used, then `u-boot-nodtb.bin` and the new `u-boot.dtb` can simply
> +be concatenated to achieve the desired result. U-Boot happily copes with the
> +devicetree growing or shrinking.
> +
> +The `u-boot.bin` image contains both pieces. While it is possible to locate the
> +devicetree within the image using the signature at the start of the file, this
> +is a bit messy.
> +
> +This is why `CONFIG_OF_SEPARATE` should always be used when building U-Boot.
> +The `CONFIG_OF_EMBED` option embeds the devicetree somewhere in the U-Boot ELF
> +image as rodata, meaning that it is hard to find it and it cannot increase in
> +size.
> +
> +When modifying the devicetree, the different cases to consider are as follows:
> +
> +- CONFIG_OF_SEPARATE
> +    This is easy, described above. Just change, replace or rebuild the
> +    devicetree so it suits your needs, then rerun binman or redo the `cat`
> +    operation to join `u-boot-nodtb.bin` and the new `u-boot.dtb`
> +
> +- CONFIG_OF_EMBED
> +    This is tricky, since the devicetree cannot easily be located. If the EFL
> +    file is available, then the _dtb_dt_begin and __dtb_dt_end symbols can be
> +    examined to find it. While it is possible to contract the file, it is not
> +    possible to expand the file since that would involve re-linking
> +
> +- CONFIG_OF_PRIOR_STAGE
> +    In this case the devicetree must be modified in the project which provides
> +    it, as described below

Personally I don't like this at all.  If the aforementioned changes
were part of the DT spec, that would make sense. Teaching
OenSBI/TF-A/whatever U-Boot internals, makes no sense to me. U-Boot
invented those bindings for it's own sake, so imho they should remain
there.

> +
> +- CONFIG_OF_BOARD
> +    This is a board-specific situation, so needs to be considered on a
> +    case-by-case base. The devicetree must be modified so that the correct
> +    one is provided to U-Boot. How this is done depends entirely on the
> +    implementation of this option for the board. It might require injecting the
> +    changes into a different project somehow using tooling available there, or
> +    it might involve merging an overlay file at runtime to obtain the desired
> +    result.
> +
> +
> +Use of U-Boot /config node
> +--------------------------
> +
> +A common problem with firmware is that many builds are needed to deal with the
> +slight variations between different, related models. For example, one model may
> +have a TPM and another may not. Devicetree provides an excellent solution to
> +this problem, in that the devicetree to actually use on a platform can be
> +injected in the factory based on which model is being manufactured at the time.
> +
> +A related problem causing build proliferation is dealing with the differences
> +between development firmware, developer-friendly firmware (e.g. with all
> +security features present but with the ability to access the command line),
> +test firmware (which runs tests used in the factory), final production firmware
> +(before signing), signed firmware (where the signatures have been inserted) and
> +the like. Ideally all or most of these should use the same U-Boot build, with
> +just some options to determine the features available. For example, being able
> +to control whether the UART console or JTAG are available, on any image, is a
> +great debugging aid.
> +
> +When the firmware consists of multiple parts, it is helpful that all operate
> +the same way at runtime, regardless of how they were built. This can be achieved
> +by passing the runtime configuration (e.g. 'enable UART console) along the chain
> +through each firmware stage. It is frustrating to have to replicate a bug on
> +production firmware which does happen on developer firmware, because they are
> +completely different builds.
> +
> +The /config node provides useful functionality for this. It allows the different
> +controls to be 'factored out' of the U-Boot binary, so they can be controlled
> +separately from the initial source-code build. The node can be easily updated by
> +a build or factory tool and can control various features in U-Boot. It is
> +similar in concept to a Kconfig option, except that it can be changed after
> +U-Boot is built.
> +
> +The /config node is similar in concept to the `/chosen node`_ except that it is
> +for passing information *into* firmware instead of from firmware to the
> +Operating System. Also, while Linux has a (sometimes extremely long) command
> +line, U-Boot does not support this. The devicetree provides a more structured
> +approach in any case.
> +
> +
> +Devicetree in another project
> +-----------------------------
> +
> +In some cases U-Boot receive its devicetree at runtime from a program that calls
> +it. For example ARM's Trusted Firmware A (`TF-A`_) may have a devicetree that it
> +passes to U-Boot. This overrides any devicetree build by U-Boot. When packaging
> +the firmware, the U-Boot devicetree may in fact be left out if it can be
> +guaranteed that it will receive one from another project.
> +
> +In this case, the devicetree in the other project must track U-Boot's use of
> +device tree, for the following reasons:
> +
> +- U-Boot only has one devicetree. See `Why not have two devicetrees?`_.
> +- For a consistent firmware build, decisions made in early stages should be
> +  communicated to later ones at runtime. For example, if the serial console is
> +  enabled in an early stage, it should be enabled in U-Boot too.
> +- U-Boot is quite capable of managing its own copy of the devicetree. If
> +  another project wants to bypass this (often for good reason), it is reasonable
> +  that it should take on the (fairly small) requirements that U-Boot features
> +  that rely on devicetree are still available
> +- The point here is not that *U-Boot needs this extra node*, or *U-Boot needs
> +  to have this public key*. These features are present in U-Boot in service of
> +  the entire firmware system. If the U-Boot features are used, but cannot be
> +  supported in the normal way, then there is pressure to implement these
> +  features in other ways. In the end, we would have a different mechanism for
> +  every other project that uses U-Boot. This introduces duplicate ways of doing
> +  the same thing, needlessly increases the complexity of the U-Boot source code,
> +  forces authors to consider parallel implementations when writing new features,
> +  makes U-Boot harder to test, complicates documentation and confuses the
> +  runtime flow of U-Boot. If every board did things its own way rather than
> +  contributing to the common code, U-Boot would lose a lot of its cross-platform
> +  value.
> +
> +The above does not indicate *bad design* within U-Boot. Devicetree is a core
> +component of U-Boot and U-Boot makes use of it to the full. It solves a myriad
> +of problems that would otherwise need their own special C struct, binary format,
> +special property, tooling for viewing and updating, etc.
> +
> +Specifically, the other project must provide a way to add configuration and
> +other information to the devicetree for use by U-Boot, such as the /config node.
> +Note that the U-Boot in-tree devicetree source must be sufficient to build and
> +boot, so this is not a way to bypass that requirement.
> +
> +If binman is used, the devicetree source in U-Boot must contain the binman
> +definition so that a valid image can be build. This helps people discover what
> +other firmware components are needed and seek out appropriate documentation.
> +
> +If verified boot is used, the project must provide a way to inject a public key,
> +certificate or other material into the U-Boot devicetree so that it is available
> +to U-Boot at runtime. See `Signing with U-Boot devicetree`_. This may be
> +through tooling in the project itself or by making use of U-Boot's tooling.
> +
> +

[...]

> +Why not have two devicetrees?
> +-----------------------------
> +
> +Setting aside the argument for restricting U-Boot from having its own nodes and
> +properties, another idea proposed is to have two devicetrees, one for the
> +U-Boot-specific bits (here called `special`) and one for everything else (here
> +called `linux`).
> +
> +On the positive side, it might quieten the discussion alluded to in the section
> +above. But there are many negatives to consider and many open questions to
> +resolve.
> +
> +- **Bindings** - Presumably the special devicetree would have its own bindings.
> +  It would not be necessary to put a `u-boot,` prefix on anything. People coming
> +  across the devicetree source would wonder how it fits in with the Linux
> +  devicetree.
> +
> +- **Access** - U-Boot has a nice `ofnode` API for accessing the devicetree. This
> +  would need to be expanded to support two trees. Features which need to access
> +  both (such as a device driver which reads the special devicetree to get some
> +  configuration info) could become quite confusing to read and write.
> +
> +- **Merging** - Can the two devicetree be merged if a platform desires it? If
> +  so, how is this managed in tooling? Does it happen during the build, in which
> +  case they are not really separate at all. Or does U-Boot merge them at
> +  runtime, in which case this adds time and memory?
> +
> +- **Efficiency** - A second device tree adds more code and more code paths. It
> +  requires that both be made available to the code in U-Boot, e.g. via a
> +  separate pointer or argument or API. Overall the separation would certainly
> +  not speed up U-Boot, nor decrease its size.
> +
> +- **Source code** - At present `u-boot.dtsi` files provide the pieces needed for
> +  U-Boot for a particular board. Would we use these same files for the special
> +  devicetree?
> +
> +- **Complexity** - Two devicetrees complicates the build system since it must
> +  build and package them both. Errors must be reported in such a way that it
> +  is obvious which one is failing.
> +
> +- **Referencing each other** - The `u-boot,dm-xxx` tags used by driver model
> +  are currently placed in the nodes they relate to. How would these tags
> +  reference a node that is in a separate devicetree? What extra validation would
> +  be needed?
> +
> +- **Storage** - How would the two devicetrees be stored in the image? At present
> +  we simply concatenate the U-Boot binary and the devicetree. We could add the
> +  special devicetree before the Linux one, so two are concatenated, but it is
> +  not pretty. We could use binman to support more complex arrangements, but only
> +  some boards use this at present, so it would be a big change.
> +
> +- **API** - How would another project provide two devicetree files to U-Boot at
> +  runtime? Presumably this would just be too painful. But if it doesn't, it
> +  would be unable to configure run-time features of U-Boot during the boot.
> +
> +- **Confusion** - No other project has two devicetrees. U-Boot would be in the
> +  unfortunate position of having to describe this fact to new users, along with
> +  the (arguably contrived) reason for the arrangement.
> +
> +- **Signing flow** - The current signing flow is simple as it involves running
> +  `mkimage` with the U-Boot devicetree. This would have to be updated to use the
> +  special devicetree. Some way of telling the user that they have done it wrong
> +  would have to be invented.
> +
> +Overall, adding a second devicetree would create enormous confusion and
> +complexity. It seems a lot cheaper to solve this by a change of attitude.
> +
> +

[...]

>

Regards
/Ilias