[PATCH 30/32] fdt: Allow the devicetree to come from a bloblist

[Ilias Apalodimas]

Hi Simon,

On Sun, 10 Sept 2023 at 22:14, Simon Glass <sjg at chromium.org> wrote:
>
> Hi Ilias,
>
> On Mon, 4 Sept 2023 at 03:31, Ilias Apalodimas
> <ilias.apalodimas at linaro.org> wrote:
> >
> > Hi Simon,
> >
> > On Fri, 1 Sept 2023 at 18:51, Simon Glass <sjg at chromium.org> wrote:
> > >
> > > Hi Ilias,
> > >
> > > On Fri, 1 Sept 2023 at 01:50, Ilias Apalodimas <ilias.apalodimas at linaro.org> wrote:
> > > >
> > > > [...]
> > > >
> > > > > >
> > > > > > > +config OF_BLOBLIST
> > > > > > > +     bool "DTB is provided by a bloblist"
> > > > > > > +     help
> > > > > > > +       Select this to read the devicetree from the bloblist. This allows
> > > > > > > +       using a bloblist to transfer the devicetree between  U-Boot phases.
> > > > > > > +       The devicetree is stored in the bloblist by an early phase so that
> > > > > > > +       U-Boot can read it.
> > > > > > > +
> > > > > >
> > > > > > I dont think this is a good idea.  We used to have 4-5 different options
> > > > > > here, which we tried to clean up and ended up with two very discrete
> > > > > > options.  Why do we have to reintroduce a new one?  Doesn't that bloblist
> > > > > > have a header of some sort?  The bloblist literally comes from a previous
> > > > > > stage bootloader which is what OF_BOARD is here for. So why can't we just
> > > > > > read the header and figure out if the magic of the bloblist matches?
> > > > >
> > > > > No, OF_BOARD is a hack to allow boards to do what they like with the FDT.
> > > > >
> > > > > This patch is a standard mechanism to pass the DT from one firmware
> > > > > phase to the next. We have spent quite a bit of time creating a spec
> > > > > for it, and we should use it.
> > > >
> > > > Where exactly am I objecting using the spec?   Can you please re-read my email?
> > > > I am actually pointing out we should use the spec *properly*.  So
> > > > instead of having a Kconfig option for the DT, which is pretty
> > > > pointless,  we should parse the bloblist.  If the header defined by
> > > > the *spec* is found, we should just search for the DT in there.
> > > > What you are doing here, is take the spec, pick a very specific item
> > > > that the list contains, and create a Kconfig option out of it.  Which
> > > > basically ignores the discoverable options of the bloblist.  For
> > > > example, that bloblist can also contain an entry to a TPM eventlog.
> > > > Should we start creating Kconfig options for all the firmware handoff
> > > > entries that are defined on that spec?
> > >
> > > OK so that is a different thing. What should it do if it expects to find a bloblist but cannot? I want it to throw an error, because I am trying to make the boot deterministic. What do you think?
> >
> > That's fine by me.  You can even put that under IS_ENABLED for the
> > bloblist inside the existing OF_BOARD check.  So I was thinking
> > - If no bloblist is required in Kconfig options we do the hacks we used to
> > - if bloblist is selected and the config option is OF_BOARD, throw an
> > error and mention that the previous stage loader should hand over a DT
> >
> > Is that what you had in mind?
>
> Yes, that sounds good to me.
>
> But I still think we need an OF_BLOBLIST option to control whether the
> devicetree comes from there.
>  Otherwise we will end up with people
> using OF_BOARD to work around it. We also have the SPL case which does
> not pass the DT in a bloblist...in fact SPL typically doesn't even
> have the full DT.

Wouldn't IS_ENABLED(BLOBLIST) || IS_ENABLED(SPL_BLOBLIST) be enough?
Inside the OF_BOARD portion of the function, search for a bloblist if
the option is enabled.  If you can't find a bloblist and the DT within
that bloblist, then error out

Thanks
/Ilias
>
> [..]
>
> Regards,
> Simon