On Sat, Sep 18, 2021 at 03:38:45AM -0600, Simon Glass wrote:
Hi,
On Fri, 10 Sept 2021 at 16:44, Tom Rini trini@konsulko.com wrote:
On Sat, Sep 11, 2021 at 12:09:40AM +0200, Mark Kettenis wrote:
Date: Fri, 10 Sep 2021 17:17:37 -0400 From: Tom Rini trini@konsulko.com
On Fri, Sep 10, 2021 at 11:12:20PM +0200, Mark Kettenis wrote:
Date: Fri, 10 Sep 2021 08:34:20 -0400 From: Tom Rini trini@konsulko.com
On Fri, Sep 10, 2021 at 10:38:17AM +0200, Heinrich Schuchardt wrote: > > > On 9/9/21 10:10 PM, Simon Glass 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@chromium.org > > Reviewed-by: Marcel Ziswiler marcel.ziswiler@toradex.com > > --- > > > > Changes in v3: > > - Fix typos linst suppled receive EFL > > - Drop 'and' before 'self-defeating' > > - Reword mention of control of QEMU's devicetree generation > > - Add mention of dropping CONFIG_OF_BOARD > > - Clarify the 'Once this bug is fixed' paragraph a bit > > - Expand ways that CONFIG_OF_PRIOR_STAGE can support the U-Boot devicetree > > - Add a note at the top explaining that his patch covers 'now', not 'future' > > - Add note 'Note: Some boards use a devicetree in U-Boot which does not match' > > > > 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 | 583 +++++++++++++++++++++++++++++ > > doc/develop/package/index.rst | 1 + > > 3 files changed, 585 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..b1bd310d906 > > --- /dev/null > > +++ b/doc/develop/package/devicetree.rst > > @@ -0,0 +1,583 @@ > > +.. SPDX-License-Identifier: GPL-2.0+ > > + > > +Updating the devicetree > > +======================= > > + > > +Note: This documentation describes how things are today, mostly, with some > > +mention of things that need to be fixed. It is not intended to point the way to > > +what might be done in the future. That should be the subject of discussions on > > +the mailing list. > > + > > +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 list, this > > + was only discovered after weeks of discussion and confusion > > + > > +- 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, with some control from command-line args, but it is not clear > > + how to add properties required by U-Boot. > > + > > +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. > > + > > +Note: It is not clear that we actually need both of these. Possibly > > +`CONFIG_OF_BOARD` can be dropped. > > + > > +Once this bug is fixed, CONFIG_OF_BOARD and CONFIG_OF_PRIOR_STAGE will override > > What does "bug" refer to? Above you describe the current design not a bug.
The bug is that we have two options to provide seemingly the same functionality. Is there a functional difference between CONFIG_OF_BOARD and CONFIG_OF_PRIOR_STAGE ?
With CONFIG_OF_BOARD there is a function that returns the pointer to the DTB, so you can do all sort of things with it.
With CONFIG_OF_PRIOR_STAGE there is a variable that you need to set in low-level code to point at the DTB and there is a pre-defined function that returns that pointer.
CONFIG_OF_BOARD is more flexible than CONFIG_OF_PRIOR_STAGE, but if the only thing you want to do is to pass on a DTB that is passed in a CPU register to U-Boot then CONFIG_OF_PRIOR_STAGE is probably easier to use.
I'm not convinced there is a bug here.
Thanks for explaining. Couldn't CONFIG_OF_PRIOR_STAGE be rewritten as an implementation of CONFIG_OF_BOARD, possibly at the same or less overall code size? That I think is the potential bug.
Probably a little bit more code:
void * board_fdt_blob_setup(void) { return (void *)(uintptr_t)prior_stage_fdt_address; }
Tiny bit more. Probably worth doing to make the choices clearer on which to select when? Bin, Rick, thoughts on this since riscv is the main user of CONFIG_OF_PRIOR_STAGE at this point?
Bin, Rick?
What is the prior stage in the RISC-V stage? Could we get it to set up a bloblist? Then we can add a devicetree in there, with the option to add more things in future.
I'm suggesting we don't need to do anything upstream of us, just rework things to use the other hook for "provided a DTB by caller, use it", so that we have a single hook for that.