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

Hello Simon,

On 29.11.23 20:44, Simon Glass wrote:

Hi Ahmad,

On Wed, 29 Nov 2023 at 12:33, Ahmad Fatoum [off-list ref] wrote:

quoted

On 29.11.23 20:27, Simon Glass wrote:

quoted

On Wed, 29 Nov 2023 at 12:15, Ahmad Fatoum [off-list ref] wrote:

quoted

On 29.11.23 20:02, Simon Glass wrote:

quoted

On Wed, 29 Nov 2023 at 11:59, Ahmad Fatoum [off-list ref] wrote:

quoted

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.

quoted

The configuration already has a compatible entry. What extra use is the compatible
entry in the FDT node?

It allows seeing the compatible stringlist without having to read the
FDT itself. I don't believe it is necessary though, so long as we are
scanning the configurations and not the FDT nodes.

I think it's better to drop this if it has no use.

Cheers,
Ahmad

Regards,
Simon

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel