Hi Tom, On Fri, 3 Dec 2021 at 13:43, Tom Rini <tr...@konsulko.com> wrote: > > On Thu, Dec 02, 2021 at 07:03:30PM -0700, Simon Glass wrote: > > Hi Andre, > > > > On Thu, 2 Dec 2021 at 18:59, Andre Przywara <andre.przyw...@arm.com> wrote: > > > > > > On Thu, 2 Dec 2021 13:34:07 -0500 > > > Tom Rini <tr...@konsulko.com> wrote: > > > > > > Hi, > > > > > > > On Thu, Dec 02, 2021 at 11:17:38AM -0700, Simon Glass wrote: > > > > > Hi Tom, > > > > > > > > > > On Thu, 2 Dec 2021 at 11:03, Tom Rini <tr...@konsulko.com> wrote: > > > > > > > > > > > > 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 <tr...@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 <tr...@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 > > > > > > > > > > > <s...@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-...@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. > > > > > > > > > > > > > > > > Look, if I thought this was all wrong I would not be doing it. We have > > > > > a range of opinions: > > > > > > > > And the rest of us wouldn't keep trying to argue otherwise if we didn't > > > > see problems with it, still. > > > > > > > > > - U-Boot should not have its own nodes/properties > > > > > > > > The caveat there is that aren't documented upstream bindings. I think > > > > at this point the lack of screaming and otherwise "wait, no no no > > > > don't!" that your current patch has gotten means it's time for a pull > > > > request, and for that to go in, and so this line of argument would be > > > > simply removed. > > > > > > > > > - U-Boot should not have DTs in-tree > > > > > > > > ... for the cases where the DTs are not used at run time, yes. > > > > > > > > > - U-Boot should have DTs only when essential > > > > > > > > I don't understand this point. Can you please elaborate? > > > > > > > > > - U-Boot should have DTs in-tree for all boards > > > > > > > > This is the line you're pushing and almost every other reviewer > > > > disagrees with. > > > > > > > > > What's the downside here anyway? > > > > > > > > - A lack of clarity. We have dts files, you modify those dts files, > > > > they aren't used. What's the point? Oh, you forgot to tweak > > > > something else. Wait, now nothing works. Oh, it's a mismatch between > > > > what this dts was at one point, and what it needs to be now to > > > > actually work. > > > > - We're adding more ongoing sync-up work. While I loudly applaud the > > > > custodians that are keeping their dts files in sync very regularly, > > > > and I sympathize with the custodians that want to do it more, but are > > > > unable to find the time, I do not want to add more of this work. Even > > > > more so when it's unclear who would be doing it. Or what the use is. > > > > > > > > There's probably more if I think about it harder, but those are the > > > > first to spring to mind. > > > > > > > > > > > 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. > > > > > > > > > > Incorrect in what way? > > > > > > > > Well, in the QEMU instance, they're only as correct as the parameters > > > > passed to qemu-system-foo when you did -dumpdtb to start with. Lets > > > > take TPM as that now should show up in the device tree, or not, > > > > depending on if we have the backend side of it? Or all of the examples > > > > of how to arbitrarily configure a system as Heinrich noted. > > > > > > > > Or the Pi examples where we need to use the device tree passed to us > > > > because config.txt is the official way to modify things in the device > > > > tree on that platform. > > > > > > I feel like a lot of the confusion stems from the very different > > > roles that U-Boot plays on various platforms: > > > - In the traditional way U-Boot is the first and only piece of code > > > that runs between reset and the kernel. Having the DT as part of the > > > U-Boot image, and thus in the U-Boot source tree, makes sense, given > > > that we talk about a particular board only. > > > - Many platforms run other pieces of software (TF-A, SCP firmware) > > > alongside or before U-Boot, but still U-Boot is the main attraction, > > > and is doing firmware and setup duties. Most cheap ARM SoCs (sunxi, > > > RK) fall under this category. Depending on the particular firmware > > > setup, having the DTs in the tree (as a copy of the canonical Linux > > > source) again makes sense, and the DTB should probably be part of the > > > built U-Boot image as well, unless there is some better place. > > > > > > But there is a quite different category of boards, where U-Boot is a > > > mere *loader*, and UEFI provider. The heavy lifting of platform setup > > > (clocks, DRAM, power) is either done by prior firmware code, or by a > > > separate management processor. The RPis, ARM Juno boards, Highbank and > > > many other (typically advanced) platforms fall under this category. > > > U-Boot should be happy about the lesser burden, and just consume > > > whatever DTB it finds in memory. > > > Virtual/dynamic platforms like QEMU or the ARM FVP models also fall > > > into this category: for a virtual platform hardware setup is mostly not > > > needed (DRAM, clock gating), or the emulator takes care of this already. > > > > > > For all those platforms the DTB naturally lives with the other firmware > > > bits already, or is even amended by them, and U-Boot should not try to > > > duplicate this, especially when the hardware is somewhat dynamic. > > > > Yes that is all understood and have been bashed to death in various > > threads. Still, it is not unreasonable, I think, for U-Boot to have a > > way to use an in-tree devicetree for development and testing purposes. > > It also reasonable, I think, to require some in-tree documentation > > about how to get U-Boot running on the board. > > OK, but what at least I'm saying (and I think others are too), is that > it's also not unreasonable to say that on some platforms that > development and testing purpose might require the developer to enable > things, rather than be the out of the box case (and may be more or less > hard to do, depending on the platform).
Can we just say that *for now*, that is actually not acceptable and we'll move to that model when we have our various pre-conditions (bindings upstream, a central repo for DTs with easy downloading for builds, automated validation) set up? I am trying to keep firmware together and make it easy to build and understand. Perhaps I can indicate how easy I'd like it to be. I'd like to build the board in U-Boot. I'd like Binman (or whatever tool we end up with here) to spit out a - list of missing binaries along with instructions on where to get them from / how to build them - list of missing tools along with instructions on where to get them from / how to build them and once we have installed those once, we can build U-Boot again and that platform successfully boots. Future iteration in U-Boot should be simply a case of rebuilding U-Boot. Iteration in other projects should just involve building that project, then running the packaging tool again. Regards, Simon