Re: [PATCH 00/16] fdt: Make OF_BOARD a boolean option

["François Ozog" <francois.ozog@linaro.org>]

[-- Attachment #1: Type: text/plain, Size: 19713 bytes --]

Hi Tom

Le mer. 27 oct. 2021 à 21:06, Tom Rini <trini@konsulko.com> a écrit :

> On Wed, Oct 27, 2021 at 06:02:19PM +0200, François Ozog wrote:
> > Hi Mark,
> >
> > On Wed, 27 Oct 2021 at 17:10, Mark Kettenis <mark.kettenis@xs4all.nl>
> wrote:
> >
> > > > From: François Ozog <francois.ozog@linaro.org>
> > > > Date: Wed, 27 Oct 2021 15:15:01 +0200
> > > >
> > > > Hi,
> > > >
> > > > On Wed, 27 Oct 2021 at 14:48, Tom Rini <trini@konsulko.com> wrote:
> > > >
> > > > > On Fri, Oct 15, 2021 at 12:03:44PM -0600, Simon Glass wrote:
> > > > > > Hi all,
> > > > > >
> > > > > > On Thu, 14 Oct 2021 at 09:28, Tom Rini <trini@konsulko.com>
> wrote:
> > > > > > >
> > > > > > > On Thu, Oct 14, 2021 at 09:17:52AM -0600, Simon Glass wrote:
> > > > > > > > Hi Tom,
> > > > > > > >
> > > > > > > > On Thu, 14 Oct 2021 at 08:56, Tom Rini <trini@konsulko.com>
> > > wrote:
> > > > > > > > >
> > > > > > > > > On Wed, Oct 13, 2021 at 12:06:02PM -0600, Simon Glass
> wrote:
> > > > > > > > > > Hi François,
> > > > > > > > > >
> > > > > > > > > > On Wed, 13 Oct 2021 at 11:35, François Ozog <
> > > > > francois.ozog@linaro.org> wrote:
> > > > > > > > > > >
> > > > > > > > > > > Hi Simon
> > > > > > > > > > >
> > > > > > > > > > > Le mer. 13 oct. 2021 à 16:49, Simon Glass <
> > > sjg@chromium.org>
> > > > > a écrit :
> > > > > > > > > > >>
> > > > > > > > > > >> Hi Tom, Bin,François,
> > > > > > > > > > >>
> > > > > > > > > > >> On Tue, 12 Oct 2021 at 19:34, Tom Rini <
> > > trini@konsulko.com>
> > > > > wrote:
> > > > > > > > > > >> >
> > > > > > > > > > >> > On Wed, Oct 13, 2021 at 09:29:14AM +0800, Bin Meng
> > > wrote:
> > > > > > > > > > >> > > Hi Simon,
> > > > > > > > > > >> > >
> > > > > > > > > > >> > > On Wed, Oct 13, 2021 at 9:01 AM Simon Glass <
> > > > > sjg@chromium.org> wrote:
> > > > > > > > > > >> > > >
> > > > > > > > > > >> > > > 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
> at
> > > [1].
> > > > > > > > > > >> > > >
> > > > > > > > > > >> > > > 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.
> > > > > > > > > > >> > >
> > > > > > > > > > >> > > Adding device trees that are never used sounds
> like a
> > > > > hack to me.
> > > > > > > > > > >> > >
> > > > > > > > > > >> > > For QEMU, device tree is dynamically generated on
> the
> > > fly
> > > > > based on
> > > > > > > > > > >> > > command line parameters, and the device tree you
> put
> > > in
> > > > > this series
> > > > > > > > > > >> > > has various hardcoded <phandle> values which
> normally
> > > do
> > > > > not show up
> > > > > > > > > > >> > > in hand-written dts files.
> > > > > > > > > > >> > >
> > > > > > > > > > >> > > I am not sure I understand the whole point of
> this.
> > > > > > > > > > >> >
> > > > > > > > > > >> > I am also confused and do not like the idea of
> adding
> > > > > device trees for
> > > > > > > > > > >> > platforms that are capable of and can / do have a
> device
> > > > > tree to give us
> > > > > > > > > > >> > at run time.
> > > > > > > > > > >>
> > > > > > > > > > >> (I'll just reply to this one email, since the same
> points
> > > > > applies to
> > > > > > > > > > >> all replies I think)
> > > > > > > > > > >>
> > > > > > > > > > >> I have been thinking about this and discussing it with
> > > people
> > > > > for a
> > > > > > > > > > >> few months now. I've been signalling a change like
> this
> > > for
> > > > > over a
> > > > > > > > > > >> month now, on U-Boot contributor calls and in
> discussions
> > > > > with Linaro
> > > > > > > > > > >> people. I sent a patch (below) to try to explain
> things. I
> > > > > hope it is
> > > > > > > > > > >> not a surprise!
> > > > > > > > > > >>
> > > > > > > > > > >> The issue here is that we need a devicetree in-tree in
> > > > > U-Boot, to
> > > > > > > > > > >> avoid the mess that has been created by
> OF_PRIOR_STAGE,
> > > > > OF_BOARD,
> > > > > > > > > > >> BINMAN_STANDALONE_FDT and to a lesser extent,
> OF_HOSTFILE.
> > > > > Between
> > > > > > > > > > >> Ilias' series and this one we can get ourselves on a
> > > stronger
> > > > > footing.
> > > > > > > > > > >> There is just OF_SEPARATE, with OF_EMBED for
> debugging/ELF
> > > > > use.
> > > > > > > > > > >> For more context:
> > > > > > > > > > >>
> > > > > > > > > > >>
> > > > >
> > >
> http://patchwork.ozlabs.org/project/uboot/patch/20210919215111.3830278-3-sjg@chromium.org/
> > > > > > > > > > >>
> > > > > > > > > > >> BTW I did suggest to QEMU ARM that they support a way
> of
> > > > > adding the
> > > > > > > > > > >> u-boot.dtsi but there was not much interest there (in
> > > fact the
> > > > > > > > > > >> maintainer would prefer there was no special support
> even
> > > for
> > > > > booting
> > > > > > > > > > >> Linux directly!)
> > > > > > > > > > >
> > > > > > > > > > > i understand their point of view and agree with it.
> > > > > > > > > > >>
> > > > > > > > > > >> But in any case it doesn't really help U-Boot. I
> > > > > > > > > > >> think the path forward might be to run QEMU twice,
> once to
> > > > > get its
> > > > > > > > > > >> generated tree and once to give the 'merged' tree
> with the
> > > > > U-Boot
> > > > > > > > > > >> properties in it, if people want to use U-Boot
> features.
> > > > > > > > > > >>
> > > > > > > > > > >> I do strongly believe that OF_BOARD must be a run-time
> > > > > option, not a
> > > > > > > > > > >> build-time one. It creates all sorts of problems and
> > > > > obscurity which
> > > > > > > > > > >> have taken months to unpick. See the above patch for
> the
> > > > > rationale.
> > > > > > > > > > >>
> > > > > > > > > > >> To add to that rationale, OF_BOARD needs to be an
> option
> > > > > available to
> > > > > > > > > > >> any board. At some point in the future it may become a
> > > common
> > > > > way
> > > > > > > > > > >> things are done, e.g. TF-A calling U-Boot and
> providing a
> > > > > devicetree
> > > > > > > > > > >> to it. It doesn't make any sense to have people decide
> > > > > whether or not
> > > > > > > > > > >> to set OF_BOARD at build time, thus affecting how the
> > > image
> > > > > is put
> > > > > > > > > > >> together. We'll end up with different U-Boot build
> targets
> > > > > like
> > > > > > > > > > >> capricorn, capricorn_of_board and the like. It should
> be
> > > > > obvious where
> > > > > > > > > > >> that will lead. Instead, OF_BOARD needs to become a
> > > commonly
> > > > > used
> > > > > > > > > > >> option, perhaps enabled by most/all boards, so that
> this
> > > sort
> > > > > of build
> > > > > > > > > > >> explosion is not needed.
> > > > > > > > > > >
> > > > > > > > > > > If you mean that when boards are by construction
> providing
> > > a
> > > > > DTB to U-Boot then I agree very much. But I don’t understand how
> the
> > > patch
> > > > > set  supports it as it puts dts files for those boards to be built.
> > > > > > > > > > >>
> > > > > > > > > > >> U-Boot needs to be flexible enough to
> > > > > > > > > > >> function correctly in whatever runtime environment in
> > > which
> > > > > it finds
> > > > > > > > > > >> itself.
> > > > > > > > > > >>
> > > > > > > > > > >> Also as binman is pressed into service more and more
> to
> > > build
> > > > > the
> > > > > > > > > > >> complex firmware images that are becoming
> fashionable, it
> > > > > needs a
> > > > > > > > > > >> definition (in the devicetree) that describes how to
> > > create
> > > > > the image.
> > > > > > > > > > >> We can't support that unless we are building a
> devicetree,
> > > > > nor can the
> > > > > > > > > > >> running program access the image layout without that
> > > > > information.
> > > > > > > > > > >>
> > > > > > > > > > >> François's point about 'don't use this with any
> kernel' is
> > > > > > > > > > >> germane...but of course I am not suggesting doing
> that,
> > > since
> > > > > OF_BOARD
> > > > > > > > > > >> is, still, enabled. We already use OF_BOARD for
> various
> > > > > boards that
> > > > > > > > > > >> include an in-tree devicetree - Raspberry Pi 1, 2 and
> 3,
> > > for
> > > > > example
> > > > > > > > > > >> (as I said in the cover letter "Most boards do provide
> > > one,
> > > > > but some
> > > > > > > > > > >> don't."). So this series is just completing the
> picture by
> > > > > enforcing
> > > > > > > > > > >> that *some sort* of devicetree is always present.
> > > > > > > > > > >
> > > > > > > > > > > That seems inconsistent with the OF_BOARD becomes the
> > > default.
> > > > > > > > > >
> > > > > > > > > > I think the key point that will get you closer to where
> I am
> > > on
> > > > > this
> > > > > > > > > > issue, is that OF_BOARD needs to be a run-time option. At
> > > > > present it
> > > > > > > > > > has build-time effects and this is quite wrong. If you go
> > > > > through all
> > > > > > > > > > the material I have written on this I think I have
> motivated
> > > > > that very
> > > > > > > > > > clearly.
> > > > > > > > > >
> > > > > > > > > > Another big issue is that I believe we need ONE
> devicetree
> > > for
> > > > > U-Boot,
> > > > > > > > > > not two that get merged by U-Boot. Again I have gone
> through
> > > > > that in a
> > > > > > > > > > lot of detail.
> > > > > > > > >
> > > > > > > > > I have a long long reply to your first reply here saved,
> but,
> > > maybe
> > > > > > > > > here's the biggest sticking point.  To be clear, you agree
> that
> > > > > U-Boot
> > > > > > > > > needs to support being passed a device tree to use, at run
> > > time,
> > > > > yes?
> > > > > > > >
> > > > > > > > Yes. The OF_BOARD feature provides this.
> > > > > > > >
> > > > > > > > >
> > > > > > > > > And in that case, would not be using the "fake" tree we
> built
> > > in?
> > > > > > > >
> > > > > > > > Not at runtime.
> > > > > > >
> > > > > > > OK.
> > > > > > >
> > > > > > > > > So is the sticking point here that we really have two
> classes
> > > of
> > > > > > > > > devices, one class where we will never ever be given the
> device
> > > > > tree at
> > > > > > > > > run time (think BeagleBone Black) and one where we will
> always
> > > be
> > > > > given
> > > > > > > > > one at run time (think Raspberry Pi) ?
> > > > > > > >
> > > > > > > > I'm not sure it will be that black and white. I suspect there
> > > will be
> > > > > > > > (many) boards which can boot happily with the U-Boot
> devicetree
> > > but
> > > > > > > > can also accept one at runtime, if provided. For example,
> you may
> > > > > want
> > > > > > > > to boot with or without TF-A or some other, earlier stage.
> > > > > > >
> > > > > > > I'm not sure I see the value in making this a gray area.
> There's
> > > very
> > > > > > > much a class of "never" boards.  There's also the class of
> "can"
> > > today.
> > > > > > > Maybe as part of a developer iterative flow it would be nice
> to not
> > > > > have
> > > > > > > to re-flash the prior stage to change a DT, and just do it in
> > > U-Boot
> > > > > > > until things are happy, but I'm not sure what the use case is
> for
> > > > > > > overriding the previous stage.
> > > > > > >
> > > > > > > Especially since the pushback on this series I think has all
> been
> > > "why
> > > > > > > are we copying in a tree to build with?  We don't want to use
> it
> > > at run
> > > > > > > time!".  And then softer push back like "Well, U-Boot says we
> have
> > > to
> > > > > > > include the device tree file here, but we won't use it...".
> > > > > >
> > > > > > See below.
> > > > > >
> > > > > > >
> > > > > > > > I believe we have got unstuck because OF_BOARD (perhaps
> > > > > inadvertently)
> > > > > > > > provided a way to entirely omit a devicetree from U-Boot,
> thus
> > > making
> > > > > > > > things like binman and U-Boot /config impossible, for
> example.
> > > So I
> > > > > > > > want to claw that back, so there is always some sort of
> > > devicetree in
> > > > > > > > U-Boot, as we have for rpi_3, etc.
> > > > > > >
> > > > > > > I really want to see what the binary case looks like since we
> could
> > > > > then
> > > > > > > kill off rpi_{3,3_b,4}_defconfig and I would need to see if we
> > > could
> > > > > > > then also do a rpi_arm32_defconfig too.
> > > > > > >
> > > > > > > I want to see less device trees in U-Boot sources, if they can
> come
> > > > > > > functionally correct from the hardware/our caller.
> > > > > > >
> > > > > > > And I'm not seeing how we make use of "U-Boot /config" if we
> also
> > > don't
> > > > > > > use the device tree from build time at run time, ignoring the
> > > device
> > > > > > > tree provided to us at run time by the caller.
> > > > > >
> > > > > > Firstly I should say that I find building firmware very messy and
> > > > > > confusing these days. Lots of things to build and it's hard to
> find
> > > > > > the instructions. It doesn't have to be that way, but if we
> carry on
> > > > > > as we are, it will continue to be messy and in five years you
> will
> > > > > > need a Ph.D and a lucky charm to boot on any modern board. My
> > > > > > objective here is to simplify things, bringing some consistency
> to
> > > the
> > > > > > different components. Binman was one effort there. I feel that
> > > putting
> > > > > > at least the U-Boot house in order, in my role as devicetree
> > > > > > maintainer (and as author of devicetree support in U-Boot back in
> > > > > > 2011), is the next step.
> > > > >
> > > > > Yes, it's Not Great.  I don't like my handful of build-BOARD.sh
> scripts
> > > > > that know where to grab other known-good binaries of varying
> licenses
> > > > > that are needed to assemble something that boots.
> > > > >
> > > > > > If we set things up correctly and agree on the bindings,
> devicetree
> > > > > > can be the unifying configuration mechanism through the whole of
> > > > > > firmware (except for very early bits) and into the OS, this will
> set
> > > > > > us up very well to deal with the complexity that is coming.
> > > > > >
> > > > > > Anyway, here are the mental steps that I've gone through over the
> > > past
> > > > > > two months:
> > > > > >
> > > > > > Step 1: At present, some people think U-Boot is not even allowed
> to
> > > > > > have its own nodes/properties in the DT.
> > > >
> > > > In my view U-Boot shall be able to leverage device tree format
> (source
> > > and
> > > > binary) to store its own data.
> > > > When you say "the" DT, I always think this is "the" DT that is
> passed to
> > > OS
> > > > and in "that" DT, there should be no U-Boot entries.
> > >
> > > Why not?  As long as the device tree validates, it is perfectly fine
> > > to have additional nodes and properties present.  The propertiesand
> > > nodes will be simply ignored by the OS.
> >
> > Because of the way we want to organize the firmware supply chain: when
> the
> > board is built, it is "attached" a device tree.
> > At that moment, we don't know what "non trusted firmware" will be used.
> It
> > could be U-Boot or LinuxBoot (https://www.linuxboot.org) or even EDK2
> (yes
> > it works with DT).
> > And we aim at keeping device tree as close to the original intent:
> hardware
> > description only. It's not because we can stuff anything in the DT and
> that
> > it is simple to do that we should.
> > Driver parameters shall be in the OS facility built for that purpose.
> Using
> > device tree has been an ugly habit.
>
> So we're going to continue to re-litigate what does and doesn't live in
> the device tree for forever, aren't we?  To continue to be clear, I'm
> not saying that non-upstream bindings should be present.  But for
> example, Simon is working on the "U-Boot config node" binding, which is
> going to get re-spun next as /options/ as I read the thread right.
> Populate it and anyone can read it, and probably be getting information
> that's useful to U-Boot or LinuxBoot or EDK2 or anyone else.  That's why
> I keep repeating that projects need to push bindings upstream.  I'll
> repeat my comment about
>
> https://trustedfirmware-a.readthedocs.io/en/latest/components/fconf/index.html
> and the binman node both noting a common problem to solve.

i think you are right. Now tfa is comfortable being its own upstream for
the binding specifications. Could U-Boot community be comfortable doing so?
Now I also recognize that DT specification state is far from clean. If you
want full story on PCI ECAM you need Linux/documentation and IEEE text
(kind of hosted on DT.org but not easily browasable to). In the long run it
may be much better to have all bindings (including U-Boot ones) in DT.org.
We should also have information from Qemu about the DT it generates for all
its devices and how it is associated to command line .

>
>
> In so far as there's objections to "U-Boot" nodes, it seems to me like
> it comes down to "shouldn't need U-Boot internals expressed in DT nor
> added to the DTB by someone else".  And I've not objected to that
> either.  But I think we do have a subset of "how do we express ..."
> issues that have come down to "well, we buried the bodies over at ...
> before".  And it's time to dig them up and give them a proper burial
> perhaps now :)
>
> --
> Tom
>
-- 
François-Frédéric Ozog | *Director Business Development*
T: +33.67221.6485
francois.ozog@linaro.org | Skype: ffozog

[-- Attachment #2: Type: text/html, Size: 31166 bytes --]