[PATCH] doc: Add documentation about devicetree usage

Sun Aug 29 15:03:14 CEST 2021

Hi Mark,

On Sun, 29 Aug 2021 at 02:19, Mark Kettenis <mark.kettenis at xs4all.nl> wrote:
>
> > From: Simon Glass <sjg at chromium.org>
> > Date: Sat, 28 Aug 2021 20:07:27 -0600
> >
> > Hi Bin,
> >
> > On Sat, 28 Aug 2021 at 19:29, Bin Meng <bmeng.cn at gmail.com> wrote:
> > >
> > > Hi Simon,
> > >
> > > On Sun, Aug 29, 2021 at 12:45 AM Simon Glass <sjg at chromium.org> wrote:
> > > >
> > > > Hi Bin,
> > > >
> > > > On Sat, 28 Aug 2021 at 07:15, Bin Meng <bmeng.cn at gmail.com> wrote:
> > > > >
> > > > > Hi Simon,
> > > > >
> > > > > On Sat, Aug 28, 2021 at 11:23 AM 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>
> > > > > > ---
> > > > > >
> > > > > >  doc/develop/index.rst              |   1 +
> > > > > >  doc/develop/package/devicetree.rst | 315 +++++++++++++++++++++++++++++
> > > > > >  doc/develop/package/index.rst      |   1 +
> > > > > >  3 files changed, 317 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..fccbb182f3e
> > > > > > --- /dev/null
> > > > > > +++ b/doc/develop/package/devicetree.rst
> > > > > > @@ -0,0 +1,315 @@
> > > > > > +.. 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. Specifically:
> > > > > > +
> > > > > > +- `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.
> > > > > > +
> > > > > > +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
> > > > > > +CONFIG_OF_SEPARATE for the in-tree build. So these two will become options,
> > > > > > +moving out of the 'choice' in `dts/Kconfig`
> > > > > > +
> > > > > > +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.
> > > > >
> > > > > Do you have some idea of how to support the QEMU case, if not using
> > > > > CONFIG_OF_PRIOR_STAGE?
> > > >
> > > > To be clear, I am not planning to remove this option. It will still work.
> > > >
> > > > But I do want to have an in-tree devicetree for all boards, and
> > > > someone will need to update qemu to support adding U-Boot
> > > > nodes/properties. It always has an array of options controlling what
> > > > it presents to BIOS/UEFI, for example, so this should not be too
> > > > controversial.
> > >
> > > I think there is some misunderstanding.
> > >
> > > Adding U-Boot nodes/properties is not a problem for QEMU, but the
> > > thing is that these QEMU targets don't require U-Boot specific
> > > nodes/properties. It's just that QEMU will generate the DTB on the fly
> > > based on different command-line options, so the DTB is dynamic and we
> > > can't hardcode one in the U-Boot tree.
> >
> > That's fine, but I do want some sort of sample in the tree, like we
> > have for qemu-x86 and others. For some reason ARM and RISC-V don't
> > have one.
> >
> > Also, QEMU needs a way to add properties and nodes that are specific
> > to U-Boot, since at present there is no way at all to pass these
> > through (/config node, u-boot,dm-xxx tags, etc.).
>
> I suppose some of the /config stuff could be nice and that the device
> tree is indeed the most appropriate way to pass runtime options from
> QEMU to U-Boot.  But I'd say U-Boot should still work (with reasonable
> defaults) if no U-Boot specific options are present in the device
> tree.

Well I believe it does and actually I think CI currently checks
this.The config node is certainly optional. But it would not support
booting signed images, for example, without the public key.

>
> My understanding is that the u-boot,dm-xxx tags are for SPL.  It seems
> that for most scenarios having where there is some prior firmware that
> passes a device tree, SPL isn't used.  That certainly is the case for
> QEMU, Raspberry Pi and my Apple M1 work.

Yes that is true and I suppose is how things are able to work at present.

But I do see QMU for RISC-V with an SPL boot, although it is only
mentioned in the docs, so perhaps it is not in the CI yet. If there
are few enough devices and enough memory (and you don't care about
boot speed) then the u-boot,dm-xxx tags are not strictly necessary.
But then we are using QEMU in a somewhat unrealistic state.

>
> > Perhaps something like the -dtb option that it already has, but just
> > for something to merge with what it uses.
> >
> > > My understanding is that we should remove CONFIG_OF_BOARD but keep
> > > CONFIG_OF_PRIOR_STAGE for such use case like QEMU.
> >
> > Fair enough if OF_BOARD is basically a duplicate. I'm actually not sure...

Regards,
Simon