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

[Simon Glass]

Hi Tom,

On Sun, 5 Dec 2021 at 06:32, Tom Rini <trini at konsulko.com> wrote:
>
> On Sat, Dec 04, 2021 at 03:09:59PM -0700, Simon Glass wrote:
> > Hi Tom,
> >
> > On Sat, 4 Dec 2021 at 11:03, Tom Rini <trini at konsulko.com> wrote:
> > >
> > > On Sat, Dec 04, 2021 at 08:20:55AM -0700, Simon Glass wrote:
> > > > Hi Tom,
> > > >
> > > > On Sat, 4 Dec 2021 at 06:52, Tom Rini <trini at konsulko.com> wrote:
> > > > >
> > > > > On Fri, Dec 03, 2021 at 06:01:56PM -0700, Simon Glass wrote:
> > > > >
> > > > > [huge snip]
> > > > > > > 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.
> > > > > >
> > > > > > Yes, agreed those are the cases. To me this needs to be a run-time choice.
> > > > >
> > > > > But it's not possible.  That's the problem we keep going around and
> > > > > around about.  People keep raising real life examples where you cannot
> > > > > make a run time choice between "device tree we're passed at run time"
> > > > > and "device tree we're compiled with".
> > > >
> > > > I haven't seen one. The most extreme case is QEMU and it works fine. I
> > > > even added a test with it. What am I missing?
> > >
> > > QEMU and Xen should both never have an in-source-tree dts as they are
> > > dynamic.  I think you missed the explanation about how U-Boot + Xen
> > > works?  You're running the same U-Boot under Xen on any arbitrary ARMv8
> > > (with required features...) system.  For QEMU virtual machines you're
> > > not supposed to do what you're doing, for production.
> > >
> > > > > And it's not helpful.  It is ALWAYS the case that we know that we want
> > > > > to override the run time device tree with our own, because it's a
> > > > > developer developing things or it's a user / production case where we
> > > > > must use the provided tree.  NOT doing that is what leads to madness
> > > > > like we see for example on Pi where if we don't use the passed tree we
> > > > > still need to copy X/Y/Z out of it.
> > > >
> > > > Aren't you talking about the distro DT there, rather than the the one
> > > > on the boot disk? That is my reading of that patch. If we need to do
> > > > that sort of thing, it doesn't matter where the the cointrol DT comes
> > > > from. You are still going to have to do that sort of thing.
> > > >
> > > > It is not ALWAYS the case. I've shown you how easy it is to disable
> > > > OF_BOARD and still boot / iterate.
> > >
> > > The DT we're passed in is the DT to pass to the OS.  That's the hook for
> > > putting a DTB on the device as it ships, the OS will just work.  The
> > > production case of needing to update that stored DTB is handled.  It's
> > > always what should be used, again outside of developer doing
> > > development.
> > >
> > > Maybe that's part of the confusion here too.  The DTB U-Boot is using is
> > > the DTB the OS will consume too, in the passed at run time case.  Unless
> > > we're instead going to save that DTB aside?  Which leaves me with a
> > > different set of design questions...
> > >
> > > > > > > > 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!
> > > > > >
> > > > > > Well we could add another Kconfig which tells U-Boot not to include a
> > > > > > devicetree in u-boot.bin, if that would resolve this?
> > > > > >
> > > > > > I just want to make sure that we always build the devicetrees and that
> > > > > > it is easy for a knowledgeable dev to switch over to use them, without
> > > > > > spelunking through dozens of other projects to discover the secret DT
> > > > > > that no one will tell us about.
> > > > >
> > > > > Should we demand better documentation for boards?  Yes.  But it's still
> > > > > a valid case to have zero device trees for a given platform in-tree.
> > > > > Xen is an example of this.  QEMU is an example of this.  Platforms need
> > > > > to work without adding special tweaks for us.  Maybe that means some
> > > > > features can't be tested in QEMU-as-virtual-platform and only in
> > > > > QEMU-faithfully-emulating-specific-physical-platforms.
> > > >
> > > > You mention QEMU (for ARM and RISC-V) and now XEN. They are a special
> > > > case, I think. How about we create a special Kconfig for that case? We
> > > > need to make some progress here.
> > >
> > > Yes, because there's a small number of OF_BOARD=y configs in tree right
> > > now, most of which are QEMU virtual machines, others of which are Pi
> > > (which we've talked to death), highbank (which Andre has explained), and
> > > then the octeontx stuff I don't know how works.  I keep pushing to say
> > > that OF_BOARD=y is the special case we need to not overrule the device
> > > tree provided to us on.  Unless we start saving off that passed-to-us DTB
> > > and then something-something for our own run-time DTB, and letting the
> > > OS consume that passed-to-us one with only normal fixups applied.
> > >
> > > > > > > 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.
> > > > > >
> > > > > > I think upstreaming the bindings is the solution there. I've made a
> > > > > > start, but we need to make progress with this series and all the other
> > > > > > things in flight. I think a lot of people want U-Boot to not have a
> > > > > > devicetree source files in it for ARMv8 platforms. I am strongly
> > > > > > opposed to that. I've laid out my reasons very clearly in the past. I
> > > > > > think this is a good summary:
> > > > > >
> > > > > > https://lists.gnu.org/archive/html/qemu-devel/2021-10/msg03480.html
> > > > >
> > > > > Yes, there are some ARMv8 platforms we will have to have the source
> > > > > files to, in tree, because they won't come to us at run time.  But
> > > > > others we won't for practical reasons, namely that we can't statically
> > > > > provide something that exists dynamically without massive duplication of
> > > > > code or just taking things from that passed to us tree.
> > > >
> > > > So let's require that the static ones have the Linux DT in our tree
> > > > for now. The dynamic ones are just QEMU for ARM and Xen, I think. If
> > > > that's it then I can agree a special case for them, so long as we sort
> > > > out the docs for Xen.
> > >
> > > I'm agreeable to saying that if the dts files exist in Linux (or other
> > > official source locations) we should have those in-tree, and the board
> > > maintainer doesn't object, yes.  But I think that might be a slightly
> > > different list than what you say here.  And that punts the run-time
> > > decision down the road.  And I'm assuming you're OK with also not
> > > including a dtb in the image.
> > >
> > > > > > I believe I have been consistent in this although with all the
> > > > > > discussion I'm really not sure anymore.
> > > > >
> > > > > Yes, everyone has been consistent in these discussions.
> > > >
> > > > I'd like to think more people accept that U-Boot is allowed its own
> > > > properties than did at the start.
> > > >
> > > > >
> > > > > > The problem is that various people have various views about how U-Boot
> > > > > > should work with devicetree. I strongly believe that until we have
> > > > > > bindings upstream, a central repo for DTs with easy downloading for
> > > > > > builds, automated validation, among other things, we must maintain the
> > > > > > devicetree in U-Boot. Just from the POV of energy expended, I do not
> > > > > > want to be arguing with the Linaro folks about what U-Boot is allowed
> > > > > > to do every month for the next two years. I'd rather set out the stall
> > > > > > now and then deal with the problems it causes from that perspective.
> > > > >
> > > > > The problems of the last going on 12 years won't be solved instantly.
> > > > > The conflict as I see it is that you're insisting that all platforms
> > > > > must have statically usable device trees, and I (and I believe others)
> > > > > are saying that's unreasonable in cases where the trees are dynamic at
> > > > > heart, lets just ensure we have good enough documentation for them,
> > > > > which we don't today.
> > > > >
> > > > > To be clear and pick an example, I don't want Pi dts files in U-Boot,
> > > > > but, OK, it's an easy enough case to sync them up and so long as we
> > > > > aren't yet at the "now we pick at run time between compiled in or passed
> > > > > to us dtb", I can accept them in tree, but not in the resulting binary
> > > > > for OF_CONTROL=y.  But as the Xen folks have also noted, there's no
> > > > > reasonable tree to include there.  It does need to be better documented
> > > > > how to fire it up however, in our sources.
> > > >
> > > > I'm OK with us copying in the Linux devicetree and using that. But
> > > > OF_BOARD must be a run-time option and able to be disabled. The
> > > > devicetree must be built, so it is actually real. We can have a
> > > > separate OF_OMIT or something like that to omit the devicetree from
> > > > the output image, perhaps.
> > >
> > > You're changing the meaning of CONFIG options.  I like the idea Mark
> > > suggested (probably after you wrote this..) of introducing something new
> > > for what you're talking about.  But no, it's unreasonable to say that
> > > every U-Boot binary will bundle one or more dtb and make a run time
> > > decision about what to use as the normal way the world works.
> > >
> > > > All of the other things need to wait until we make progress with
> > > > devicetree bindings, validation,
> > > >
> > > > How can we make progress on this? We have different goals, as I have
> > > > explained, so we are not going to agree on everything.
> > >
> > > Replace patches 4 to 14 with syncing platforms dts files from current
> > > Linux release, and when OF_BOARD=y don't include a device tree in the
> > > resulting image?  That might also need adding some documentation for
> > > some platforms on where the device tree is and how to extract it.
> >
> > How about we create a new option for that, as we discussed on IRC a
> > while back? After all, sandbox uses OF_BOARD. It doesn't mean
> > OF_PRIOR_STAGE, just that we call board_fdt_blob_setup().
> >
> > So something like OF_OMIT_DTB? For U-Boot as a whole this is an uncommon case.
> >
> > We also need to think about the protections again, perhaps we define
> > OF_HAS_PRIOR_STAGE to indicate that the board works this way, then
> > make it hard to disable OF_BOARD if that is set.
>
> I guess my hard sticking point here is I'm still entirely unconvinced
> that the future step of "now we make run-time choice between built in or
> provided dtb" makes sense.  And I worry the way these CONFIG options
> keep being presented is setting the ground work for this change that I'm
> not agreed to conceptually.
>
> So, yes, we could add more CONFIG symbols and then maybe even follow up
> and migrate a few more platforms over to them.  I'm unclear why we need
> more symbols, but, OK, yes, to make progress, lets try it.  Sometimes I
> understand patches better than paragraphs.

OK I am trying it. I will send v7 soon.

I am going to add the warnings we talked about in v7. I think the
whole 'confusion' thing is overblown, but hopefully that will help get
things moving too.

Note that the difference between 'built-in' and 'provided' is just the
run-time difference. The build puts the DT in the firmware image
anyway, otherwise of course the board cannot boot. The image includes
U-Boot and anything else that is needed. E.g. in the case of TF-A
doing this, the build includes TF-A, U-Boot and devicetree. TF-A reads
the devicetree, fiddles with it and passes it to U-Boot. So the
firmware includes a built-in devicetree, one way or another. The
question is what happens at runtime. This is related to my 'Binman'
point.

Regards,
Simon