On Thu, 2 Dec 2021 11:17:38 -0700 Simon Glass <s...@chromium.org> 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: > > - U-Boot should not have its own nodes/properties > - U-Boot should not have DTs in-tree > - U-Boot should have DTs only when essential > - U-Boot should have DTs in-tree for all boards > > What's the downside here anyway? > > > > 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? > > How do I get a real one for development? How do I turn off OF_BOARD > and use the in-tree one? > > The documentation approach is not good enough. > > > > > > > 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. > > So some people don't need it and it serves no purpose for them. But > why do they care? It is not hurting anyone. This is all overblown. > > > > > 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". > > No that's not correct. With x86, ppc, integrator, ast2500 and many > others we *need* the DT and *it is not* created by QEMU. Maybe because those are fixed platforms, and/or have ways to autodetect most of their hardware? I feel like U-Boot just has a DT here because it uses it as some kind of configuration file, short of hardcoding the hardware information. I think this is one reason that x86 PCs don't use DTs: there is only *one* fixed base platform (1981 IBM PC, with all its oddities), plus probeable hardware (PCI) for most of the devices. I personally absolutely love that QEMU creates the DT based on its command line parameters: that's the way it should be, and the only way it makes sense to me for this dynamic platform. Having some random QEMU DT in the U-Boot tree sounds wrong, IMHO. Cheers, Andre. > > > > > > > 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. > > How does it solve it? I don't even know how to get it in many cases. > If it is a dummy then I cannot actually use it for development, right? > > Regards, > Simon
