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

Thu Dec 2 23:52:56 CET 2021

On Thu, Dec 02, 2021 at 11:36:59PM +0100, François Ozog wrote:
> Hi  Simon,Tom
> 
> Le jeu. 2 déc. 2021 à 19:34, Tom Rini <trini at konsulko.com> a écrit :
> 
> > 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 <trini at 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 <trini at 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 <trini at 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 <sjg at 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-sjg@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.
> 
> For the sake of getting interesting parts of the patch set in and “move
> on”, what about:
>  providing an empty (see below) DT for boards for which U-Boot receives the
> “source of truth” so that there are no build issues.
> The empty DT contains a comment that explains the DT lifecycle for the
> board and points to the documentation tree for sample DTs.
> The boards would the ones we talked about and SystemReady boards
> https://www.arm.com/why-arm/architecture/systems/systemready-certification-program/ir?_ga=2.35159527.79382615.1638484482-159131740.1638484482

My ROCKPro64 is on the way still, but I'm not sure what SystemReady IR
has to do with this exactly.  I mean this very much seriously as I was
doing some testing with another platform in preparation for
certification and I was just using the device tree that in U-Boot.
Perhaps I just missed a section in one of the documents however.

-- 
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/20211202/e0d5081f/attachment.sig>