[PATCH v6 00/25] fdt: Make OF_BOARD a boolean option

Thu Dec 2 19:03:22 CET 2021

On Thu, Dec 02, 2021 at 10:07:13AM -0700, Simon Glass wrote:
> Hi Tom,
> 
> On Thu, 2 Dec 2021 at 09:59, Tom Rini <trini at konsulko.com> wrote:
> >
> > On Thu, Dec 02, 2021 at 09:49:51AM -0700, Simon Glass wrote:
> > > Hi Tom,
> > >
> > > On Thu, 2 Dec 2021 at 09:38, Tom Rini <trini at konsulko.com> wrote:
> > > >
> > > > On Thu, Dec 02, 2021 at 05:33:53PM +0100, François Ozog wrote:
> > > > > Hi Simon
> > > > >
> > > > > Le jeu. 2 déc. 2021 à 17:00, Simon Glass <sjg at chromium.org> a écrit :
> > > > >
> > > > > > With Ilias' efforts we have dropped OF_PRIOR_STAGE and OF_HOSTFILE so
> > > > > > there are only three ways to obtain a devicetree:
> > > > > >
> > > > > >    - OF_SEPARATE - the normal way, where the devicetree is built and
> > > > > >       appended to U-Boot
> > > > > >    - OF_EMBED - for development purposes, the devicetree is embedded in
> > > > > >       the ELF file (also used for EFI)
> > > > > >    - OF_BOARD - the board figures it out on its own
> > > > > >
> > > > > > The last one is currently set up so that no devicetree is needed at all
> > > > > > in the U-Boot tree. Most boards do provide one, but some don't. Some
> > > > > > don't even provide instructions on how to boot on the board.
> > > > > >
> > > > > > The problems with this approach are documented in another patch in this
> > > > > > series: "doc: Add documentation about devicetree usage"
> > > > > >
> > > > > > In practice, OF_BOARD is not really distinct from OF_SEPARATE. Any board
> > > > > > can obtain its devicetree at runtime, even it is has a devicetree built
> > > > > > in U-Boot. This is because U-Boot may be a second-stage bootloader and its
> > > > > > caller may have a better idea about the hardware available in the machine.
> > > > > > This is the case with a few QEMU boards, for example.
> > > > > >
> > > > > > So it makes no sense to have OF_BOARD as a 'choice'. It should be an
> > > > > > option, available with either OF_SEPARATE or OF_EMBED.
> > > > > >
> > > > > > This series makes this change, adding various missing devicetree files
> > > > > > (and placeholders) to make the build work.
> > > > > >
> > > > > > Note: If board maintainers are able to add their own patch to add the
> > > > > > files, some patches in this series can be dropped.
> > > > > >
> > > > > > It also provides a few qemu clean-ups discovered along the way. The
> > > > > > qemu-riscv64_spl problem is fixed.
> > > > > >
> > > > > > [1]
> > > > > > https://patchwork.ozlabs.org/project/uboot/patch/20210919215111.3830278-3-sjg@chromium.org/
> > > > > >
> > > > > > Changes in v6:
> > > > > > - Fix description of OF_BOARD so it refers just to the current state
> > > > > > - Explain that the 'two devicetrees' refers to two *control* devicetrees
> > > > > > - Expand the commit message based on comments
> > > > > > - Expand the commit message based on comments
> > > > >
> > > > > You haven’t addressed any concerns expressed on the mailing list.so I am
> > > > > not in favor of this new version either.
> > > > > If you make a version without « fake DTs » as you name them, there are good
> > > > > advances in the documentation and other areas that would be better in
> > > > > mainline….
> > > > > If I am the only one thinking this way and the patch can be accepted, I
> > > > > would love there is a warning in capital letters at the top of the DTS fake
> > > > > files that explains the intent of this fake DT, the possible outcomes of
> > > > > not using the one provided by the platform and the right way of dealing
> > > > > with DTs for the platform.
> > > >
> > > > This is the part that I too am still unhappy about.  I do not want
> > > > reference or fake or whatever device trees in the U-Boot source tree.
> > > > We should be able to _remove_ the ones we have, that are not required,
> > > > with doc/board/...rst explaining how to get / view one.  Not adding
> > > > more.
> > >
> > > I understand you don't like it and that others don't as well. I wish
> > > it had not come to this.
> > >
> > > However we are only talking about 10 boards, three of which don't even
> > > have a devicetree anywhere I can find.
> > >
> > > I think on balance this is a substantial clean-up. I am happy to add
> > > whatever caveats and documentation people want to clarify what is
> > > going on here. I'm happy to look at future options where the
> > > devicetree is hosted elsewhere, so long as it is trivial to build it
> > > within U-Boot for development purposes.
> > >
> > > I'll also note that the bootstd series shows the devicetree source:
> > >
> > > Core:  246 devices, 88 uclasses, devicetree: board
> > >
> > > But for now, I still feel this is the best path forward.
> >
> > I'm not sure how to proceed here.  The reviews are rather strongly
> > against the "include a device tree that won't be used".  The use case of
> > "but for development someone might need to modify the device tree" is
> > handled by platforms documenting where / how to get the real one.  We
> > should even update the Kconfig help to note that if you enable this your
> > board docs MUST explain where the device tree can be seen (or have some
> > legal reason you think it's OK to not...).
> 
> Right, we can do lots of things as we have discussed. I am very
> willing to work on these and make sure it is hard to do the thing. But
> this series is long enough already.

Yes, I think the rest of us had hoped you would come around to all of
our reasoning by this point, is why this is taking so long.

> It is more than just development. A devicetree is needed for binman to
> work, even if it is empty. The documentation idea doesn't really work,
> as I think I have proven by the difficulty in getting this series
> together. An automated mechanism that runs in CI might be acceptable,
> but that is in the future. For now, I believe it just HAS to be
> in-tree.

I still don't see any reason why we need these incorrect and not
functionally used device trees in-tree when a dummy invalid tree is
enough to make things link.  We're dealing with real "we must have X.bin
in the output for things to function" issues on other platforms with
binman right now.  Using a dummy dts should be fine.

> > And yes, we're "only" talking about 10 platforms, which include things
> > like the "everyone" has one Pi family, the extraordinarily flexible (and
> > so easy for the reference device tree to be very wrong) QEMU families
> > and then platforms that are including a dts in-tree now because they
> > were told that was required.
> 
> Actually it is only rpi4 that doesn't have an in-tree DT. There is
> actually no reason for it not to, e.g. Linux has it. Why not U-Boot?
> The argument is the same.

Because we don't need it!  It serves no purpose!  It exists in Linux
as that's the primary device tree source repository.  We could _copy_ it
in, if it was useful.  But then we need to re-sync it every so often,
and for less clear reasons than all of the platforms that we need to
sync with the kernel for, AND we use the tree.

There's even an argument to be made that it IS in Linux because when you
build that dtb, it's what the firmware then ships and uses and provides
to everyone at run time, possibly along with whatever other
modifications the binary firmware did (see the assorted threads,
including one this week about the problems we have because we don't just
always use the dtb provided to us at run time).

> Most QEMU boards have an in-tree devicetree. It is only ARM (now
> copied by RISC-V) which doesn't.

Yes, these are more examples of "someone said we need to copy it in, so
we copy it in".

> > How about adjusting the make logic so that if a tree isn't found, we use
> > a dummy minimal valid dts file?
> 
> This is what I have done for the boards where I could not figure out
> how to get any sort of DT, yes. But I don't think that should be the
> default.

The more I think about this, the more I think dummy minimal valid dts
should be the fall-back default.  This then solves the "I'm a developer,
I need to modify the dts files" case because you then just provide the
dts instead where it should go, and it's used.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20211202/e1ffcb0e/attachment.sig>