Hi Simon,
Le jeu. 28 oct. 2021 à 04:51, Simon Glass <s...@chromium.org> a écrit :
> Hi Ilias,
>
> On Tue, 26 Oct 2021 at 00:46, Ilias Apalodimas
> <ilias.apalodi...@linaro.org> wrote:
> >
> > Hi Simon,
> >
> > A bit late to the party, sorry!
>
> (Did you remember the beer? I am replying to this but I don't think it
> is all that helpful for me to reply to a lot of things on this thread,
> since I would not be adding much to my cover letter and patches)
>
> >
> > [...]
> >
> > > >
> > > > 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.
> > >
> > > 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. It is an abuse of the
> > > devicetree standard, like the /chosen node but with less history. We
> > > should sacrifice efficiency, expedience and expandability on the altar
> > > of 'devicetree is a hardware description'. How do we get over that
> > > one? Wel, I just think we need to accept that U-Boot uses devicetree
> > > for its own purposes, as well as for booting the OS. I am not saying
> > > it always has to have those properties, but with existing features
> > > like verified boot, SPL as well as complex firmware images where
> > > U-Boot needs to be able to find things in the image, it is essential.
> > > So let's just assume that we need this everywhere, since we certainly
> > > need it in at least some places.
> > >
> > > (stop reading here if you disagree, because nothing below will make
> > > any sense...you can still use U-Boot v2011.06 which doesn't have
> > > OF_CONTROL :-)
> >
> > Having U-Boot keep it's *internal* config state in DTs is fine. Adding
> > that to the DTs that are copied over from linux isn't imho. There are
> > various reasons for that. First of all syncing device trees is a huge
> pain
> > and that's probably one of the main reasons our DTs are out of sync for a
> > large number of boards.
> > The point is this was fine in 2011 were we had SPL only, but the reality
> > today is completely different. There's previous stage boot loaders (and
> > enough cases were vendors prefer those over SPL). If that bootloader
> needs
> > to use it's own device tree for whatever reason, imposing restrictions
> on
> > it wrt to the device tree it has to include, and require them to have
> > knowledge of U-Boot and it's internal config mechanism makes no sense not
> > to mention it doesn't scale at all.
>
> I think the solution here may be the binman image packer. It works
> from a description of the image (i.e. is data-driver) and can collect
> all the pieces together. The U-Boot properties (and the ones required
> by TF-A, etc.) can be added at package time.
>
> If you think about it, it doesn't matter what properties are in the DT
> that is put into the firmware image. TF-A, for example, is presumably
> reading a devicetree from flash, so what does it care if it has some
> U-Boot properties in it?
I am going to change my position in all mail threads I participate.
I was trying to make patches relevant in the future and conceptually clean.
That may not be the most effective position: I should just care about
Linaro and its members being able to implement SystemReady concepts.
If you mandate U-Boot has nodes in the device tree passed to the OS, we can
put DT fragment in the nt_fw_config section of the fip and merge it at
boot time. So there is a solution compatible with SystemReady.
If you want to put fake, non future proof, DT sources in the dts for
platforms that are organized to provide the authoritative DT to U-Boot at
runtime, that's kind of your choice (hopefully representing the rest of
U-Boot community). There will be quirk code in U-Boot to redo the
adaptations on its non authoritative DT that the platform previous stage
firmware does (already saw one in the past month); as Mark said there will
be issues over time; and it will confuse people about the role of the DT.
But I am fine with it as it does not impair Linaro and its members ability
to implement SystemReady way of handling DT.
>
> As to syncing, we have solved this using u-boot.dtsi files in U-Boot,
> so I think this can be dealt with.
>
> >
> > >
> > > Step 2: Assume U-Boot has its own nodes/properties. How do they get
> > > there? Well, we have u-boot.dtsi files for that (the 2016 patch
> > > "6d427c6b1fa binman: Automatically include a U-Boot .dtsi file"), we
> > > have binman definitions, etc. So we need a way to overlay those things
> > > into the DT. We already support this for in-tree DTs, so IMO this is
> > > easy. Just require every board to have an in-tree DT. It helps with
> > > discoverability and documentation, anyway. That is this series.
> > >
> >
> > Again, the board might decide for it's own reason to provide it's own DT.
> > IMHO U-Boot must be able to cope with that and asking DTs to be included
> in
> > U-Boot source is not the right way to do that, not to mention cases were
> > that's completely unrealistic (e.g QEMU or a board that reads the DTB
> from
> > it's flash).
>
> I think you are at step 2. See above for my response.
>
> >
> > > (I think most of us are at the beginning of step 2, unsure about it
> > > and worried about step 3)
> > >
> > > Step 3: Ah, but there are flows (i.e. boards that use a particular
> > > flow only, or boards that sometimes use a flow) which need the DT to
> > > come from a prior stage. How to handle that? IMO that is only going to
> > > grow as every man and his dog get into the write-a-bootloader
> > > business.
> >
> > And that's exactly why we have to come up with something that scales,
> without
> > having to add a bunch of unusable DTs in U-Boot.
>
> In what way does this not scale? How are the DTs unusable? If there is
> a standard binding, we should be fine.
>
> >
> > > We need a way to provide the U-Boot nodes/properties in a
> > > form that the prior stage can consume and integrate with its build
> > > system. Is TF-A the only thing being discussed here? If so, let's just
> > > do it. We have the u-boot.dtsi and we can use binman to put the image
> > > together, for example. Or we can get clever and create some sort of
> > > overlay dtb.
> > >
> > > Step 3a. But I don't want to do that. a) If U-Boot needs this stuff
> > > then it will need to build it in and use two devicetrees, one internal
> > > and one from the prior stage....well that is not very efficient and it
> > > is going to be confusing for people to figure out what U-Boot is
> > > actually doing. But we actually already do that in a lot of cases
> > > where U-Boot passes a DT to the kernel which is different to the one
> > > it uses. So perhaps we have three devicetrees? OMG.
> >
> > No we don't. That's a moot point. If you separate the DTs U-Boot
> > provides the internal one and inherits one 'generic'. Linux will be
> able to use
> > that. So the only case were you'll need 3 DTs is if the *vendor* breaks
> the
> > DT across kernel versions, In which case there's not much you can do to
> > begin with and that's already a case we have to deal with.
>
> Linux actually doesn't care if the U-Boot properties are in the tree,
> so long as we have proper bindings. My point here is we only need
> either:
>
> a. one devicetree, shared with Linux and U-Boot (and TF-A?)
> b. two devicetrees, one for use in firmware and one for passing to Linux
>
> We don't need to separate out the U-Boot properties into a second (or
> third) devicetree. There just isn't any point.
>
> >
> > > b) Well then
> > > U-Boot can have its own small devicetree with its bits and then U-Boot
> > > can merge the two when it starts. Again that is not very efficient. It
> > > means that U-Boot cannot be controlled by the prior stage (e.g. to get
> > > its public key from there or to enable/disable the console), so
> > > unified firmware config is not possible. It will get very confusing,
> > > particularly for debugging U-Boot. c) Some other scheme to avoid
> > > accepting step 3...please stop!
> > >
> > > Step 4: Yes, but there is QEMU, which makes the devicetree up out of
> > > whole cloth. What about that? Well, we are just going to have to deal
> > > with that. We can easily merge in the U-Boot nodes/properties and
> > > update the U-Boot CI scripts to do this, as needed, e.g. with
> > > qemu-riscv64_spl. It's only one use case, although Xen might do
> > > something similar.
> > >
> > > To my mind, that deals with both the build-time and run-time issues.
> > > We have a discoverable DT in U-Boot, which should be considered the
> > > source of truth for most boards. We can sync it with Linux
> > > automatically with the tooling that I hope Rob Herring will come up
> > > with. We can use an empty one where there really is no default,
> > > although I'd argue that is making perfect an enemy of the good.
> > >
> > > Step 5: If we get clever and want to remove them from the U-Boot tree
> > > and pick them up from somewhere else, we can do that with sufficient
> > > tooling. Perhaps we should set a timeline for that? A year? Two? Six?
> >
> > We can start slowly migrating boards and see how that works out.
> > We could either use 2 device trees as you proposed, or have u-boot merge
> > the 'u-boot' DTB and the inherited DTB before DM comes up. OTOH I'd
> prefer
> > if linux gets handed a clean device tree without the u-boot internals in
> > it, so I think 2 discrete DTs is cleaner overall.
>
> I know you would prefer that, but does it really matter in practice?
> What is the objection, actually?
>
> As I mentioned on the call, I think the prior stage should do any
> merging or fixing up. Trying to do that sort of thing in 'early' code
> in U-Boot (or any other program, including Linux) is such a pain. With
> U-Boot, for example, we don't even have any RAM available to do it
> with half the time and it would dramatically increase the amount of
> memory needed prior to relocation. It just isn't a very good idea to
> try to do this in early code. It is also completely unnecessary, once
> you get past the philosophical objections.
>
> If TF-A wants to be in the picture, let it deal with the implications
> and responsibility thus incurred. TF-A has no right to tell U-Boot how
> to handle its config. TF-A is 0.5m LOC, i.e. a lot, almost a quarter
> of the size of U-Boot. It duplicates loads of things in there. No one
> will even *notice* an FDT merge function, which is actually only 70
> LOC:
>
> /**
> * overlay_apply_node - Merges a node into the base device tree
> * @fdt: Base Device Tree blob
> * @target: Node offset in the base device tree to apply the fragment to
> * @fdto: Device tree overlay blob
> * @node: Node offset in the overlay holding the changes to merge
> *
> * overlay_apply_node() merges a node into a target base device tree
> * node pointed.
> *
> * This is part of the final step in the device tree overlay
> * application process, when all the phandles have been adjusted and
> * resolved and you just have to merge overlay into the base device
> * tree.
> *
> * returns:
> * 0 on success
> * Negative error code on failure
> */
> static int overlay_apply_node(void *fdt, int target,
> void *fdto, int node)
> {
> int property;
> int subnode;
>
> fdt_for_each_property_offset(property, fdto, node) {
> const char *name;
> const void *prop;
> int prop_len;
> int ret;
>
> prop = fdt_getprop_by_offset(fdto, property, &name,
> &prop_len);
> if (prop_len == -FDT_ERR_NOTFOUND)
> return -FDT_ERR_INTERNAL;
> if (prop_len < 0)
> return prop_len;
>
> ret = fdt_setprop(fdt, target, name, prop, prop_len);
> if (ret)
> return ret;
> }
>
> fdt_for_each_subnode(subnode, fdto, node) {
> const char *name = fdt_get_name(fdto, subnode, NULL);
> int nnode;
> int ret;
>
> nnode = fdt_add_subnode(fdt, target, name);
> if (nnode == -FDT_ERR_EXISTS) {
> nnode = fdt_subnode_offset(fdt, target, name);
> if (nnode == -FDT_ERR_NOTFOUND)
> return -FDT_ERR_INTERNAL;
> }
>
> if (nnode < 0)
> return nnode;
>
> ret = overlay_apply_node(fdt, nnode, fdto, subnode);
> if (ret)
> return ret;
> }
>
> return 0;
>
>
>
> }
>
>
> >
> > Regards
> > /Ilias
> > >
> > > To repeat, 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. I feel
> > > this will set us up very well to deal with the complexity that is
> > > coming.
> > >
>
> Regards,
> Simon
>
