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

[Tom Rini]

On Fri, Dec 03, 2021 at 09:45:12AM -0700, Simon Glass wrote:
>  is generallyHi Tom,
> 
> On Fri, 3 Dec 2021 at 09:31, Tom Rini <trini at konsulko.com> wrote:
> >
> > On Fri, Dec 03, 2021 at 09:18:27AM -0700, Simon Glass wrote:
> > > Hi Tom,
> > >
> > > On Fri, 3 Dec 2021 at 08:57, Tom Rini <trini at konsulko.com> wrote:
> > > >
> > > > On Fri, Dec 03, 2021 at 08:39:34AM -0700, Simon Glass wrote:
> > > > > Hi Tom,
> > > > >
> > > > > On Fri, 3 Dec 2021 at 07:55, Tom Rini <trini at konsulko.com> wrote:
> > > > > >
> > > > > > On Thu, Dec 02, 2021 at 08:58:54AM -0700, Simon Glass wrote:
> > > > > >
> > > > > > > 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.
> > > > > >
> > > > > > Note that I can't run-time test this as the last patch fails to apply
> > > > > > and is dependent on non-trivial missing changes ("/* The devicetree is
> > > > > > typically appended to U-Boot */" doesn't exist at all in lib/fdtdec.c
> > > > > > and that's part of the unchanging context where things fail to apply).
> > > > >
> > > > > That code is the penultimate patch ("fdt: Drop remaining preprocessor
> > > > > macros in fdtdec_setup()"). Did that patch apply OK? It is based on
> > > > > -next and is at dm/ofb-working if you want to compare.
> > > >
> > > > I just fetch'd and built that instead, thanks.
> > > >
> > > > > > So, here's my first bit of confusion.  Today, I build for rpi_arm64 and
> > > > > > no dtb files are built.  I run this on my Pi 3 and everything works.
> > > > > > With your series, I see all the dtbs have been built, and dts/dt.dtb and
> > > > > > u-boot.dtb have a Pi 4 dtb in them.  Should this even run now?
> > > > >
> > > > > Yes, so long as OF_BOARD is enabled, which it is in this series. This
> > > > > is basically the same as the situation with rpi3, except it uses
> > > > > OF_EMBED (need to fix...)
> > > >
> > > > OK, so my Pi 3 still boots on rpi_arm64, good.  But why did I embed a
> > > > rpi4 device tree to u-boot.bin ?  CONFIG_OF_BOARD=y is set in the
> > > > .config, so I'm telling it to use the run-time device tree.  It will
> > > > never ever use this dtb, under any circumstance, right?
> > >
> > > That's right, unless you disable OF_BOARD and build U-Boot again.
> >
> > And then it would fail to boot, because I'm on a 3, not a 4.
> 
> Yes, but that's because the DT is wrong, right? The build builds all
> the different DTs but just selects one (the default) to put into
> u-boot.dtb and u-boot.bin
> 
> We could make it generate images for all of them. I have thought about
> that as it stops us needing different boards just to handle having
> multiple DT options. But I haven't really looked at it.

It would be wrong because it's the wrong device tree for the hardware,
yes.  And we don't want to build N different binaries, the point of the
target is that everything works like a good data-driven platform should.
One binary, N platforms work because we get the configuration at
run-time.

> > > Oddly enough with rpi_3_32b it uses EMBED and ignores the DT provided.
> > > I didn't even know that...yet another example of the confusion of the
> > > current state.
> >
> > So, I'm trying to use this example here to lead to what I think is a
> > reasonable compromise.  As you note, with CONFIG_OF_BOARD=y all of those
> > built trees, and the embedded tree, will not ever be used.  But it makes
> 
> (except during development and for build testing)

But that's not relevant.  For development you're changing the config.
And for build testing zero device trees should be built, if zero device
trees are included.

> > the make logic a tiny bit easier to have some tree, not no tree.  Why
> > can't we:
> > - When CONFIG_OF_BOARD=y not build those trees as part of "all"
> >   (individual rules should still work).
> > - For linking, use an empty minimal valid dts.
> >
> > Because when CONFIG_OF_BOARD=y you will HAVE TO change the configuration
> > to know what device tree you want it to even use if you disable
> > CONFIG_OF_BOARD.  You cannot assume that CONFIG_DEFAULT_DEVICE_TREE is
> > correct, which is why it's unset currently.
> >
> > Does this make sense?  If not, why not?  And I have thoughts about other
> > platforms too, but I want to stick with a fairly concrete example first.
> 
> Well CONFIG_DEFAULT_DEVICE_TREE is generally correct. As above we
> could support generating multiple outputs, perhaps. I think Binman
> would be the best way to handle that, e.g. from a list of DTs.

It's correct when we set it to non-zero, yes.  But otherwise it's empty
on purpose.  And we already have a mechanism for when there's a single
dtb that's common enough for N platforms, and we select the right one at
run time.  Nothing new needed here.

> It makes some sense.
> 
> I don't agree with not even building the trees. We should continue to
> build all the related DTs so we know that switching off OF_BOARD will
> work. If it isn't built, people won't even add it, right?

We don't know that it will work just because we built some device trees.
There's not even for certain device trees to add to the source trees.

> Also you are going back to OF_BOARD having build-time effects, which I
> want to get away from. It should be a run-time option. That is my
> bottom line, really. In the future I hope that standard passage will
> provide a DT and we will be able to display where it came from,
> including what program provided it, for example. But disabling
> OF_BOARD should simply make the built-in one kick in. If it isn't
> there, how can it?

I don't see how run-time option makes any sense when we know that there
are cases where it's never the correct choice.  You keep aiming to
optimize a corner of the U-Boot internal developer workflow.  That I
think is maybe the big center of the disagreement.

> Don't you find the current situation really confusing? It just makes
> no sense to me.

There's things that need to be cleaned up because we have some small
number of platforms that went off and did their own thing.  But largely
yes, things make sense to me.  We have:
- We embedded the device tree that will configure U-Boot, because there
  is no way for the hardware to have provided us one.
- We do not embed the device tree that will configure U-Boot, because
  there is already one present in memory for us to use.

Then we have the developer option of:
- We embedded the device tree that will configure U-Boot, because we're
  developing something.

> Are you looking to have an empty DT in u-boot.bin? Perhaps we should
> provide a way to do that? But what is driving that desire?

I'm looking for ways to convince you that we do not need to include a
device tree in the binary.  There's a growing set of devices where the
device tree exists with the device.  If it's missing, that's a huge
fatal error we can't do all that much about.  If we need to do something
to that device tree for U-Boot, yes, fine, we should make it straight
forward for the developer to do that.  But that's not the common case!

I guess another part of the problem is that historically almost all
platforms were in the first case I list above, no run time provided
device tree, so we took the kernel one and added our bindings to it.
Now we're being bit by the growing number of platforms that are the
second case, and how do we get our properties in there, and which ones
even make sense to do that for.

-- 
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/20211203/6c4e738f/attachment.sig>