[PATCH v2 15/18] fdt: Allow the devicetree to come from a bloblist

Fri Dec 8 04:14:06 CET 2023

Hi Raymond,

On Wed, 6 Dec 2023 at 13:35, Raymond Mao <raymond.mao at linaro.org> wrote:
>
> Hi Simon,
>
> > The other thing missing here is SPL. The bloblist is designed such
> > that it can be set up in any phase of U-Boot, then is passed to
> > following phases. So using IS_ENABLED(BLOBLIST) doesn't do what we
> > need. As I noted, it breaks sandbox_spl. What happens if a board wants
> > to use bloblist in SPL but doesn't want the SPL DT to come from a
> > prior stage? What happens if SPL wants doesn't want to pass the U-Boot
> > DT through to U-Boot in the bloblist? It is all just horribly
> > confusing.
> I think the logic to decide whether to take the DT from bloblist really depends
> on the board vendor.
> I will move this kind of logics into board specific functions from fdtdec.c in the
> next revision.
> Board vendors can decide the behaviors they want. This can answer the question
> above and for example whether they want a fallback when no DT exists in the
> bloblist or the DT from bloblis is corrupted, or just print a warning message.
> For Qemu-arm, as an example of design reference, I prefer to take the DT from the
> bloblist automatically when (BLOBLIST && OF_BOARD) and keep the fallback
> compatibility to load from `CFG_SYS_SDRAM_BASE` when DT from bloblist is
> corrupted.
> I will take a look into the sandbox_spl failures you mentioned.

In the interests of my sanity, could you send just the bloblist
updates and move the other patches into a separate series?

Regards,
SImon

>
> Thanks and regards,
> Raymond
>
>
> On Tue, 5 Dec 2023 at 22:57, Simon Glass <sjg at chromium.org> wrote:
>>
>> Hi Ilias,
>>
>> On Mon, 4 Dec 2023 at 23:22, Ilias Apalodimas
>> <ilias.apalodimas at linaro.org> wrote:
>> >
>> > Hi Simon,
>> >
>> > We did discuss this in OSFC but perhaps you forgot.  The discussion
>> > was based on the mail here [0].
>>
>> Perhaps I did? Or perhaps we had a different understanding of it?
>>
>> >
>> > On Tue, 5 Dec 2023 at 02:52, Simon Glass <sjg at chromium.org> wrote:
>> > >
>> > > Hi Raymond,
>> > >
>> > > On Mon, 4 Dec 2023 at 12:25, Raymond Mao <raymond.mao at linaro.org> wrote:
>> > > >
>> > > > Hi Simon,
>> > > >
>> > > > When `OF_BOARD` is defined, the FDT should be from one of  the board-specific mechanisms:
>> > > > 1. FDT from a bloblist via boot args
>> > > > 2. FDT in memory
>> > > > I don't think we need a new build option to distinguish these two, since it can be done in runtime by checking whether the boot args follow the FW Handoff spec conventions and the FDT exists in the bloblist.
>> > >
>> > > No, I would really like this to be deterministic, so we can fail if
>> > > something goes wrong. The bloblist should be 'declared' as the way to
>> > > boot, for a board.
>> >
>> > We *are* deterministic. I am just going to repeat what we discussed in
>> > OSFC for completeness.
>> > OF_BLOBLIST makes little sense because we are not going to add a
>> > Kconfig per blolblist entry.
>> > I haven't looked at what the patch does yet but the idea was really simple.
>> > - Under OF_BOARD (which can arguably be renamed to OF_PREVIOUS_STAGE
>> > or something like that)  if CONFIG_BLOB is enabled we scan for a
>> > bloblist
>> > - If CONFIG_BLOB is enabled but no bloblist is provided we fail the boot
>> > - If CONFIG_BLOB is not enabled we do what we did up to now, maybe
>> > with a message that this is going to be obsoleted once enough boards
>> > migrate to a bloblist
>> >
>> > IOW if support for a bloblist is enabled we expect to find one from
>> > the previous bootloader. I've lost count of how many times I repeated
>> > this, but here it goes again
>> > The device tree can come
>> > - From u-boot
>> > - From a previous stage loader *somehow* (eg. passed on a bloblist,
>> > passed on a register, read from an EEPROM etc)
>> >
>> > We should keep that distinction and create subcategories for the
>> > 'comes from a previous stage loader', rather than adding arbitrary
>> > config options which only confuse people
>>
>> I don't know why you are so frustrated about this. This patch does not
>> even do what you have listed above, so far as I can tell.
>>
>> We have:
>> - OF_HAS_PRIOR_STAGE which indicates that U-Boot is not the first phase
>> - OF_BOARD which indicates that the board can do anything it likes
>> - BLOBLIST which indicates that there is a bloblist
>>
>> I proposed adding:
>> - OF_BLOBLIST which indicates that U-Boot gets the DT from the bloblist
>>
>> I understood that your intention OF_BOARD would go away since there
>> are almost no cases that need it. I was skeptical it could be done but
>> I definitely agreed (and still do) that it would be great if that
>> could go away, as we would not need OF_BLOBLIST since OF_BOARD would
>> effectively mean the same thing.
>>
>> But when I look at this patch, that is not what is happening. In fact,
>> there is no indication that the DT came from the bloblist. U-Boot will
>> just report 'devicetree: board'. We must say where it came from.
>>
>> The other thing missing here is SPL. The bloblist is designed such
>> that it can be set up in any phase of U-Boot, then is passed to
>> following phases. So using IS_ENABLED(BLOBLIST) doesn't do what we
>> need. As I noted, it breaks sandbox_spl. What happens if a board wants
>> to use bloblist in SPL but doesn't want the SPL DT to come from a
>> prior stage? What happens if SPL wants doesn't want to pass the U-Boot
>> DT through to U-Boot in the bloblist? It is all just horribly
>> confusing.
>>
>> So I believe, in fact, that we cannot get rid of OF_BOARD now, and
>> perhaps for quite a long time. There is another discussion about
>> Qualcomm where they want a gzipped DT attached to the end of U-Boot,
>> for example. Perhaps the key problem with this patch is that it would
>> ensure that OF_BOARD sticks around, since it means both a) Get the DT
>> from bloblist if BLOBLIST; b) Get the DT from a board-specific
>> mechanism if !BLOBLIST. That is not what we discussed.
>>
>> Why not just take my original patch and work from there? It will save
>> a lot of time. We can then progressively migrate boards from OF_BOARD
>> to OF_BLOBLIST and then perhaps one day remove OF_BOARD (I remain
>> skeptical, but hopeful). The whole 'standard passage' thing [1] was
>> carefully thought out *two years ago* and (I believe) dealt with all
>> of the issues above.
>>
>> For now, we should apply the patches which bring in the spec changes,
>> so we can figure out the rest of it. I have added my review tag for
>> those.
>>
>> Regards,
>> SImon
>> >
>> > Regards
>> > /Ilias
>> > >
>> > > If in the future all boards a reusing bloblist for this feature, then
>> > > sure, we can drop it. But we have things like rpi which do their own
>> > > thing.
>> > >
>> > > > If it fulfills the above conditions, we can take the FDT from bloblist, otherwise from the predefined memory address.
>> > > > This is fully backward compatible without adding a new build option.
>> > >
>> > > Again, we need to obtain the FDT from the bloblist if and only if the
>> > > board requires it. It creates a lot of confusion to have it as an
>> > > optional feature, when we know which way it should be.
>> > >
>> > > Ultimately OF_BOARD should go away. We just need to push bloblist to
>> > > other projects and closed-source firmware as the correct way to pass a
>> > > DT.
>> > >
>> > > Regards,
>> > > Simon
>> > > [.]
>> >
>> > [0] https://lore.kernel.org/u-boot/CAPnjgZ0SqVyj_drLEQrP21zPYCco3HOP-rYD+nKJMa0BVYpMDA@mail.gmail.com/
>>
>> [1] https://patchwork.ozlabs.org/project/uboot/list/?series=281465&state=*