[PATCH v7 2/2] arm64: boot: Support Flat Image Tree

[Doug Anderson]

Hi,

On Sat, Dec 2, 2023 at 8:37 AM Simon Glass <sjg at chromium.org> wrote:
>
> Hi Ahmad,
>
> On Thu, 30 Nov 2023 at 19:04, Ahmad Fatoum <a.fatoum at pengutronix.de> wrote:
> >
> > Hello Simon,
> >
> > On 30.11.23 21:30, Simon Glass wrote:
> > > On Wed, 29 Nov 2023 at 12:54, Ahmad Fatoum <a.fatoum at pengutronix.de> wrote:
> > >> On 29.11.23 20:44, Simon Glass wrote:
> > >>> On Wed, 29 Nov 2023 at 12:33, Ahmad Fatoum <a.fatoum at pengutronix.de> wrote:
> > >>>>
> > >>>> On 29.11.23 20:27, Simon Glass wrote:
> > >>>>> On Wed, 29 Nov 2023 at 12:15, Ahmad Fatoum <a.fatoum at pengutronix.de> wrote:
> > >>>>>> On 29.11.23 20:02, Simon Glass wrote:
> > >>>>>>> On Wed, 29 Nov 2023 at 11:59, Ahmad Fatoum <a.fatoum at pengutronix.de> wrote:
> > >>>>>>>> The specification says that this is the root U-Boot compatible,
> > >>>>>>>> which I presume to mean the top-level compatible, which makes sense to me.
> > >>>>>>>>
> > >>>>>>>> The code here though adds all compatible strings from the device tree though,
> > >>>>>>>> is this intended?
> > >>>>>>>
> > >>>>>>> Yes, since it saves needing to read in each DT just to get the
> > >>>>>>> compatible stringlist.
> > >>>>>>
> > >>>>>> The spec reads as if only one string (root) is supposed to be in the list.
> > >>>>>> The script adds all compatibles though. This is not really useful as a bootloader
> > >>>>>> that's compatible with e.g. fsl,imx8mm would just take the first device tree
> > >>>>>> with that SoC, which is most likely to be wrong. It would be better to just
> > >>>>>> specify the top-level compatible, so the bootloader fails instead of taking
> > >>>>>> the first DT it finds.
> > >>>>>
> > >>>>> We do need to have a list, since we have to support different board revs, etc.
> > >>>>
> > >>>> Can you give me an example? The way I see it, a bootloader with
> > >>>> compatible "vendor,board" and a FIT with configuration with compatibles:
> > >>>>
> > >>>>   "vendor,board-rev-a", "vendor,board"
> > >>>>   "vendor,board-rev-b", "vendor,board"
> > >>>>
> > >>>> would just result in the bootloader booting the first configuration, even if
> > >>>> the device is actually rev-b.
> > >>>
> > >>> You need to find the best match, not just any match. This is
> > >>> documented in the function comment for fit_conf_find_compat().
> > >>
> > >> In my above example, both configuration are equally good.
> > >> Can you give me an example where it makes sense to have multiple
> > >> compatibles automatically extracted from the device tree compatible?
> > >>
> > >> The way I see it having more than one compatible here just has
> > >> downsides.
> > >
> > > I don't have an example to hand, but this is the required mechanism of
> > > FIT. This feature has been in place for many years and is used by
> > > ChromeOS, at least.
> >
> > I see the utility of a FIT configuration with
> >
> >     compatible = "vendor,board-rev-a", "vendor,board-rev-b";
> >
> > I fail to see a utility for a configuration with
> >
> >     compatible = "vendor,board", "vendor,SoM", "vendor,SoC";
> >
> > Any configuration that ends up being booted because "vendor,SoC" was matched is
> > most likely doomed to fail. Therefore, I would suggest that only the top level
> > configuration is written into the FIT configurations automatically.
>
> Firstly, I am not an expert on this.
>
> Say you have a board with variants. The compatible string in U-Boot
> may be something like:
>
> "google,veyron-brain-rev1", "google,veyron-brain", "google,veyron",
> "rockchip,rk3288";
>
> If you then have several FIT configurations, they may be something like:
>
> "google,veyron-brain-rev0", "google,veyron-brain", "google,veyron",
> "rockchip,rk3288";
> "google,veyron-brain-rev1", "google,veyron-brain", "google,veyron",
> "rockchip,rk3288";
> "google,veyron-brain-rev2", "google,veyron-brain", "google,veyron",
> "rockchip,rk3288";
>
> You want to choose the second one, since it is a better match than the others.
>
> +Doug Anderson who knows a lot more about this than me.

Hopefully this is all explained by:

https://docs.kernel.org/arch/arm/google/chromebook-boot-flow.html