Hi Simon, On Sat, Dec 04, 2021 at 10:25:29AM -0700, Simon Glass wrote: > Hi Ilias, > > On Sat, 4 Dec 2021 at 08:58, Ilias Apalodimas > <ilias.apalodi...@linaro.org> wrote: > > > > Hi Simon, > > > > [...] > > > > [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. > > > > > > > > I am not convinced the case is "we are developing something". The > > arguments for this are something along the lines of: > > 1. U-Boot will fail to compile if OF_SEPARATE is selected and you provide > > no DT so we need to include it. With which I disagree. The config option > > says "You must provide an external device tree" for a reason. > > Where are you reading that? I'm lost.
In the XEN patch commit message [1] > > OF_SEPARATE means it is not embedded in U-Boot but is attached to the end of > it. > > > 2. The DT's are hard to find. The 'hard to find' RPI DTs are in fact > > committed in the kernel. I am not sure about the rest, but honestly this > > isn't a convincing argument for me. > > Great, so you've solved that one but even that was confusing for me, > as my patch should make clear. Every single board would be its own > special snowflake if we went that way. > > So if they are in the kernel, we just need to put them in U-Boot too, > so problem solved. The auto-sync thing that I believe Rob is working > on will make things easy. My problem is not usable DTs like the PRI4. My problem is the OF_BOARD being a runtime option and always building that DT, while at the same time introduce a third option to not include it in the binary. Another problem is 'empty' DTs. > > > > > What else are we gaining with it being a run time choice? One of the > > We are requiring a DT to be present in the U-Boot tree for > development, documentation and discoverability purposes. Devs can turn > OF_BOARD on and off as it suits them when iterating on a platform. They can also do the same thing without tangling the 2 options. The *real* problem is that in a month from now we'll get a patch saying "I need to change X on RPI4 DTB because I have this special reason and I want a specific u-boot binding" and then we are back at the start. We also have OF_EMBED for the 'developer' option which is also mentioned explicitly in the Kconfig. > > > things this approach does address, but we keep conveniently ignoring, is > > that it tries to preserve the current status quo. You can go and add the > > special missing U-Boot nodes in those DT's. So that would bypass problems > > if the bindings get NAK'ed. But this is going to work against the > > fragmentation we are trying to resolve. > > Well that's another reason why we need in-tree DTs at the moment. That > reason goes away once we have our bindings upstream and are able to do > what we need with DT there. > Again, that's a huge if. I am honestly not saying this in bad faith, but I have my concerns on if and what gets upstreamed. So what this really does in my head is preserve the current functionality. So what I am trying to avoid here is, in case the bindings get NAK'ed, we go back and say "that's fine we got this covered, look it's in docs and commit messages!" > > > > > > 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? > > (I think my point there is made) > > > > > IMHO 3b595da441cf is the commit that started the problem and tangled those > > 2 options. Note that this support has been removed from the dragonboard > > later in 0204d1b56b2f .... > > Yes it is one of them. There may be cases where we want to patch up > the DT that U-Boot builds. In fact OF_BOARD does not mean it came from > a prior stage. All sorts of things could be going on. We should not > conflate building a DT with OF_BOARD. It says 'board specific way'. To means that means we either create it on the fly or inherit it. In the case you want to fixup the DTB provided by OF_SEPARATE, there are _weak platform functions to patch that up. You don't really need OF_BOARD for that, you can do that fine with OF_SEPARATE and check fdtdec_board_setup(). > > > > > > > > > > > > > > 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. > > > > > > > > > > > > > > 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. > > > > So the translation of this for me is: > > We have 2 discrete options (that can be cleaned up further) which choose > > either to embed or receive the DTB. Let's tangle them and introduce a > > *new* third option to separate them if someone needs to. Which makes no > > sense to me. > > Embed is confusing because OF_EMBED means to link it into U-Boot (just > used for debugging) > Which as I said, it's what development should use? > I think this is the core of the problem. There are just lots of ideas > about how all of this should work. Please try out the series and see > if you find any problems. Then we can talk about actual issues rather > than things that might happen. I am pretty sure the patches work. I never doubted that. What I don't love is the architecture and the way it entangles the existing options. > > The two options you refer to are OF_SEPARATE and OF_BOARD. We will > perhaps have OF_PASSAGE at some point. We already have OF_EMBED. > > To me, OF_BOARD and OF_PASSAGE are run-time things because they > indicate what we expect to happen at runtime. If something goes wrong, > we might still be able to print an error message, if we have a backup > DT. I don't see a point saving a developer from a mistake. I'd much rather crash as loudly as possible to indicate "HEY YOU MESSED UP", instead of hiding a problem he might miss under the mat. Maybe I am missing something, but OF_PASSAGE seems superfluous to me. OF_SEPARATE and OF_BOARD cover all the cases I can think of. > > But OF_BOARD and OF_PASSAGE are not related to the build itself. IMO > OF_BOARD is underspecified. One day I would like to move towards > defining these cases better, e.g. > > - DT is passed in a register (rpi and apple_m1, stm32mp, RISC-V, qemu > RISC-V, bcmstb, Octeon, Xen) > - DT is at a fixed address (qemu, highbank, socrates, qemu ppc) > - DT is in a file (sandbox) Again I might be missing something but to me: 1. OF_SEPARATE = provided by u-boot. If someone wants to fix that up we got a number of ways already. 2. OF_BOARD providing with magic somehow at runtime (prior loader, constructed at runtime, read from a flash, whatever). 3. OF_EMBED whatever special developer case we want to use it for. > > IMO many of these could all be handled with standard passage, i.e. in > a standard way. > [...] > > Regards, > Simon [1] https://lore.kernel.org/u-boot/20211204180318.GV1220664@bill-the-cat/T/#m543a10a4b558ccd540fa425d61106ea515393105 Regards /Ilias
