Re: [PATCH] image-fdt: save name of FIT configuration in '/chosen' node

Daniel Golle
2022-03-22T13:19:15Z

Hi Heinrich,

On Tue, Mar 22, 2022 at 09:29:54AM +0100, Heinrich Schuchardt wrote:
> On 3/22/22 00:22, Daniel Golle wrote:
> > It can be useful for the OS (Linux) to know which configuration has
> > been chosen by U-Boot when launching a FIT image.
> > Store the name of the FIT configuration node used in a new string
> > attribute called 'bootconf' in the '/chosen' node in device tree.
>
> Please, point out where you want to use that information. I cannot see
> that the Linux kernel will make any use of it.

It can be useful when parsing FIT image in kernel, but can also be
useful in userspace (via /sys/firmware/devicetree/base/chosen/bootconf).
For OpenWrt I've written a uImage.FIT partition parser in order to
allow a single type of firmware image containing kernel, dtb and rootfs
to work accross all storage types (SPI-NOR/mtd, NAND/ubi, eMMC/block).

I haven't yet submitted it upstream, exactly because some edges like
selection of configuration would need to be discussed first.

https://git.openwrt.org/?p=openwrt/openwrt.git;a=blob;f=target/linux/generic/files/block/partitions/fit.c;h=89b5fb3454f0b69cdcfe53958169fb2a6f9c48a9;hb=HEAD

However, it's doing the job for a bunch of OpenWrt boards already,
incl. some popular devices

https://openwrt.org/toh/linksys/e8450
(boots from SPI-NAND/ubi)

https://openwrt.org/toh/sinovoip/bananapi_bpi-r64_v1.1
(can boot from SD Card, eMMC or SPI-NAND/ubi)

Using uImage.FIT to store the squashfs besides kernel and dtb has
several obvious advantages which are hard to obtain in any other way:
* single image accross NOR/mtd, NAND/ubi and MMC/block devices
* dynamically sized sub-partitions for kernel and rootfs
* hash also for rootfs checked by U-Boot before launching kernel
* images may include additional filesystems e.g. for localization or
brandinge (hence the need to parse the boot configuration when
mapping the 'filesystem' subimages into block partitions).

>
> >
> > Signed-off-by: Daniel Golle <daniel@makrotopia.org>
> > ---
> > boot/image-fdt.c | 6 ++++++
> > 1 file changed, 6 insertions(+)
> >
> > diff --git a/boot/image-fdt.c b/boot/image-fdt.c
> > index 692a9ad3e4..4017bc94a6 100644
> > --- a/boot/image-fdt.c
> > +++ b/boot/image-fdt.c
> > @@ -601,6 +601,12 @@ int image_setup_libfdt(bootm_headers_t *images, void *blob,
> > goto err;
> > }
> >
> > + /* Store name of configuration node as bootconf in /chosen node */
> > + if (images->fit_uname_cfg)
> > + fdt_find_and_setprop(blob, "/chosen", "bootconf",
> > + images->fit_uname_cfg,
> > + strlen(images->fit_uname_cfg) + 1, 1);
> > +
>
> We should not inject arbitrary properties into the device-tree. Where is
> the property /chosen/bootconf defined in the Linux documentation or in
> the device-tree specification?

From Linux Documenation (chosen.txt):
"The chosen node does not represent a real device, but serves as a place
for passing data between firmware and the operating system, like boot
arguments."

From Device Tree specification (Release v0.3):
"The /chosen node does not represent a real device in the system but
describes parameters chosen or specified by the system firmware at run
time."

So to me it sounds like the right place for an additional attribute
serving the above purpose. It could as well become an additional kernel
cmdline argument in (ie. appended to 'bootargs' in '/chosen'), but that
seemed more complex and harder to deal with.

The name could be changed into something like 'denx,boot-configuration'
if 'bootconf' is perceived to be too generic.

Cheers

Daniel