[PATCH v4] fdt: Allow the devicetree to come from a bloblist

Thu Dec 28 16:28:01 CET 2023

On Thu, Dec 28, 2023 at 03:09:45PM +0000, Simon Glass wrote:
> Hi Tom,
> 
> On Thu, Dec 28, 2023 at 1:48 PM Tom Rini <trini at konsulko.com> wrote:
> >
> > On Thu, Dec 28, 2023 at 01:37:39PM +0000, Simon Glass wrote:
> > > Hi Tom,
> > >
> > >
> > > On Wed, Dec 27, 2023 at 8:06 PM Tom Rini <trini at konsulko.com> wrote:
> > > >
> > > > On Wed, Dec 27, 2023 at 05:48:42PM +0000, Simon Glass wrote:
> > > > > Hi Tom,
> > > > >
> > > > > On Tue, Dec 26, 2023 at 12:07 PM Tom Rini <trini at konsulko.com> wrote:
> > > > > >
> > > > > > On Tue, Dec 26, 2023 at 09:46:25AM +0000, Simon Glass wrote:
> > > > > >
> > > > > > > Standard passage provides for a bloblist to be passed from one firmware
> > > > > > > phase to the next. That can be used to pass the devicetree along as well.
> > > > > > > Add an option to support this.
> > > > > > >
> > > > > > > Tests for this will be added as part of the Universal Payload work.
> > > > > > >
> > > > > > > Signed-off-by: Simon Glass <sjg at chromium.org>
> > > > > > > ---
> > > > > > > The discussion on this was not resolved and is now important due to the
> > > > > > > bloblist series from Raymond. So I am sending it again since I believe
> > > > > > > this is a better starting point than building on OF_BOARD
> > > > > >
> > > > > > I really don't like adding another option for "DT is given to us". Why
> > > > > > isn't adding another enum to fdt_source_t sufficient
> > > > >
> > > > > That is added by this patch, but...
> > > > >
> > > > > >, and if we have
> > > > > > bloblist enabled, that will look for and use if found?
> > > > >
> > > > > ...this is the question. I would like to be able to enable bloblist
> > > > > without *requiring* the DT to come from there, hence the separate
> > > > > Kconfig option.
> > > > >
> > > > > > Maybe some other
> > > > > > code needs to be restructured and cleaned up too?
> > > > >
> > > > > Possibly, but I really am not keen on this board-specific solution. I
> > > > > believe Ilias & I agreed that OF_BOARD should fade away, so I don't
> > > > > like adding another thing onto it.
> > > >
> > > > I think you're missing something then. It should not be board specific
> > > > to just find the dtb via bloblist, without a new CONFIG.
> > >
> > > Yes, that is what I am looking for.
> > >
> > > > Whatever is
> > > > stopping that is what needs to be refactored and then you can just
> > > > extend fdt_source_t without a new CONFIG option.
> > >
> > > OK, but I need a way to turn it off, at least for development.
> > >
> > > [merging in the other thread:]
> > >
> > > On Wed, Dec 27, 2023 at 8:11 PM Tom Rini <trini at konsulko.com> wrote:
> > > >
> > > > On Wed, Dec 27, 2023 at 05:48:44PM +0000, Simon Glass wrote:
> > > > > Hi Ilias,
> > > > >
> > > > > On Wed, Dec 27, 2023 at 6:44 AM Ilias Apalodimas
> > > > > <ilias.apalodimas at linaro.org> wrote:
> > > > > >
> > > > > > Hi Tom,
> > > > > >
> > > > > > On Tue, 26 Dec 2023 at 14:07, Tom Rini <trini at konsulko.com> wrote:
> > > > > > >
> > > > > > > On Tue, Dec 26, 2023 at 09:46:25AM +0000, Simon Glass wrote:
> > > > > > >
> > > > > > > > Standard passage provides for a bloblist to be passed from one firmware
> > > > > > > > phase to the next. That can be used to pass the devicetree along as well.
> > > > > > > > Add an option to support this.
> > > > > > > >
> > > > > > > > Tests for this will be added as part of the Universal Payload work.
> > > > > > > >
> > > > > > > > Signed-off-by: Simon Glass <sjg at chromium.org>
> > > > > > > > ---
> > > > > > > > The discussion on this was not resolved and is now important due to the
> > > > > > > > bloblist series from Raymond. So I am sending it again since I believe
> > > > > > > > this is a better starting point than building on OF_BOARD
> > > > > > >
> > > > > > > I really don't like adding another option for "DT is given to us". Why
> > > > > > > isn't adding another enum to fdt_source_t sufficient, and if we have
> > > > > > > bloblist enabled, that will look for and use if found? Maybe some other
> > > > > > > code needs to be restructured and cleaned up too?
> > > > > >
> > > > > > Same here. On top of that the bloblist has a few items in there, e.g a
> > > > > > TPM eventlog. What are we going to do? Add a Kconfig for each item?
> > > > >
> > > > > No, but that is just a straw man. The DT is special and U-Boot reports
> > > > > where it comes from.
> > > > >
> > > > > >
> > > > > > This has been going back and forth for a while. I've lost count of how
> > > > > > many times I repeated the same proposal, but here it goes again. We
> > > > > > have OF_BOARD and BLOBLIST options. The bloblist and its properties
> > > > > > are scannable at runtime. Can't we use the combination of these 2 can
> > > > > > be used to imply we expect things from a bloblist. If we want to be
> > > > > > stricter in the future and explicitly expect the DT from a bloblist,
> > > > > > we could add a Kconfig option failing the boot if that's missing.
> > > > >
> > > > > I would like to have that Kconfig option now, not later. In my mind,
> > > > > the boot must be deterministic, so that if OF_BLOBLIST is enabled, the
> > > > > DT must come from there, or it is an error.
> > > >
> > > > Determinism doesn't require a CONFIG option, it just requires an if/else
> > > > tree where we say what the "correct" priority list should be and then
> > > > set a flag so that we can tell the user where we found it too. This also
> > > > means that we can get whatever is going to use this mechanism to
> > > > migrate over, and have less of a chicken-and-egg type of problem.
> > >
> > > OK I think I understand what is going on here. We need a way to enable
> > > OF_BLOBLIST while still allowing the prior stage to NOT provide a
> > > bloblist, at least for a transition period (= forever, I suspect, but
> > > that's another discussion).
> > >
> > > How about if OF_BLOBLIST enables the option, but we allow a fallback
> > > to OF_BOARD if the DT is not there?
> >
> > Oh, OK, so your use case is development. Why don't we just make
> > OF_EMBEDDED be the first clause in the if/else (and yes, if
> > (IS_ENABLED(CONFIG_OF_EMBEDDED) :)) and move on still not needing a knob
> > for "bloblist has it"?
> 
> Because OF_EMBED doesn't work with binman, and takes a lot of
> different code paths, etc. It is somewhat useful when loading U-Boot
> using a debugger, but that is about it.
> 
> I sent a v5 that I hope does what is needed.

It sounds like binman needs to be fixed to just handle the case of not
needing to do something with the device tree, since it's already
embedded? I'll look again at v5, but since you sent it out while we were
still talking about v4 and only just now understanding what your use
case is, I don't think it's right, still.

As you've said your use case is development and needing to ensure you
get the device tree you want (because you're modifying it), and OF_EMBED
is supposed to be used only for that exact use case, that's how we
should solve this problem.

-- 
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/20231228/d3f54cef/attachment.sig>