Re: [PATCH 2/8] FIT: skip possible overlay config nodes

[Sascha Hauer <s.hauer@pengutronix.de>]

On Tue, Jun 11, 2024 at 10:36:47AM +0200, Marco Felsch wrote:
> Hi,
> 
> sorry for the delay on this patchset.
> 
> On 24-03-25, Sascha Hauer wrote:
> > Hi Marco,
> > 
> > On Fri, Mar 22, 2024 at 05:49:47PM +0100, Marco Felsch wrote:
> > > The FIT spec is not very specific when it comes to device-tree overlay
> > > handling.
> > 
> > By FIT spec you mean
> > https://github.com/u-boot/u-boot/blob/master/doc/usage/fit/overlay-fdt-boot.rst,
> > right, or is there more?
> 
> this is just an example which is not complete e.g. it misses the
> signature node in case of verified boot. I used [1] as reference but
> after reading it again I see that this reference list the kernel or
> firmware properties as mandatory.
> 
> [1] https://docs.u-boot.org/en/latest/usage/fit/source_file_format.html#configurations-node
> 
> > > Overlays can be added directely to an config node:
> > > 
> > > 	config-a {
> > > 		compatible = "machine-compatible";
> > > 		kernel = "kernel-img-name";
> > > 		fdt = "fdt-base-name", "fdt-overlay1-name", "...";
> > > 	}
> > > 
> > > or they are supplied via dedicated config nodes:
> > > 
> > > 	overlay-2 {
> > > 		fdt = "fdt-overlay2-name";
> > > 	}
> > > 
> > > Of course these config nodes can have compatibles as well:
> > > 
> > > 	overlay-3 {
> > > 		compatible = "machine-compatible";
> > > 		fdt = "fdt-overlay3-name";
> > > 	}
> > 
> > The text I referenced above doesn't mention compatible properties in
> > overlay config nodes.
> 
> You're right, but the format description chapter [1] does.
> 
> > > The current fit_find_compatible_unit() code would skip the overlay node
> > > if the config-a compatible has the same score as the overlay-3
> > > compatible and if the overlay-3 config-node is listed after the config-a
> > > config-node. But if the compatible of config-a config-node has a lower
> > > score or the overlay-3 config-note is listed first (the spec does not
> > > specify any order) we end up in taking the overlay-3 config-node instead
> > > of config-a config-node.
> > 
> > You could distinguish overlay config nodes from full config nodes by the
> > presence of a "kernel" property. Overlay config nodes do not have it.
> 
> Of course this could be done but I wanted to make it more explicit since
> the FIT spec is not very clear when it comes to overlays. Instead of
> adding an explicit image type like:
> 
>   - type = "flat_dt_overlay";
> 
> they used the already existing definitions. Therefore I went this way so
> it is up to user to specify the overlay config nodes explicit.

I don't buy this argumentation. A node that doesn't have a 'kernel'
property can not be booted, hence you can ignore it when looking for a
bootable config. A node that does have a 'kernel' property by
definition doesn't describe an overlay.

I agree that it's unfortunate that a overlay config node can only be
indirectly detected as such. Nevertheless I don't see a need for
adding another globalvar for that.

Sascha

-- 
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 |