Mirrors/u-boot
1
0
Fork 0
mirror of https://source.denx.de/u-boot/u-boot.git synced 2026-06-02 09:46:37 +03:00
Code Issues Packages Projects Releases Wiki Activity

Merge patch series "boot/fit: use fdt_for_each_subnode() in image-fit.c"

Browse Source 
Aristo Chen <aristo.chen@canonical.com> says:

This series ends with replacing the verbose fdt_next_node() + ndepth
idiom in boot/image-fit.c with fdt_for_each_subnode(), bringing the
file in line with boot/image-fit-sig.c. Six of the seven sites in
image-fit.c predate the macro by 2-6 years; the seventh was
copy-pasted from a neighbour in 2015 just after the macro landed.
The old idiom is legacy, not a deliberate technical choice.

Converting straight to the macro turned out to need a prerequisite,
which is patch 1. fit_print_contents() reads the default-config
property using the loop variable left over after iterating /images
children. With /images defined first in the source (the conventional
layout) libfdt's walker happens to leave that variable pointing at
/configurations and the read works. With /configurations defined
first the read returns NULL and the "Default Configuration" line is
silently omitted. fdt_for_each_subnode()'s post-loop value is
unconditionally a negative error code, so a naive conversion would
have made the missing line the unconditional behaviour. Patch 1
reads the property from confs_noffset directly and removes the
layout dependency.

Patch 2 adds a regression test for the configs-before-images
layout, which had no coverage.

Patch 3 is the mechanical conversion at all seven sites,
equivalence-preserving as described in the per-patch message.

Link: https://lore.kernel.org/r/20260508213217.3807786-1-aristo.chen@canonical.com
This commit is contained in:
Tom Rini
2026-05-25 13:44:28 -06:00
parent a219f64c27 2c9b117aa4
commit 77efd55f89
2 changed files with 81 additions and 81 deletions
Download Patch File Download Diff File Expand all files Collapse all files

109
boot/image-fit.c
View File

@@ -156,18 +156,10 @@ static void fit_get_debug(const void *fit, int noffset,
int fit_get_subimage_count(const void *fit, int images_noffset)
{
int noffset;
int ndepth;
int count = 0;
/* Process its subnodes, print out component images details */
for (ndepth = 0, count = 0,
noffset = fdt_next_node(fit, images_noffset, &ndepth);
(noffset >= 0) && (ndepth > 0);
noffset = fdt_next_node(fit, noffset, &ndepth)) {
if (ndepth == 1) {
count++;
}
}
fdt_for_each_subnode(noffset, fit, images_noffset)
count++;
return count;
}
@@ -291,7 +283,7 @@ static void fit_conf_print(const void *fit, int noffset, const char *p)
const char *uname;
int ret;
int fdt_index, loadables_index;
int ndepth;
int sub_noffset;
/* Mandatory properties */
ret = fit_get_desc(fit, noffset, &desc);
@@ -357,14 +349,8 @@ static void fit_conf_print(const void *fit, int noffset, const char *p)
}
/* Process all hash subnodes of the component configuration node */
for (ndepth = 0, noffset = fdt_next_node(fit, noffset, &ndepth);
(noffset >= 0) && (ndepth > 0);
noffset = fdt_next_node(fit, noffset, &ndepth)) {
if (ndepth == 1) {
/* Direct child node of the component configuration node */
fit_image_print_verification_data(fit, noffset, p);
}
}
fdt_for_each_subnode(sub_noffset, fit, noffset)
fit_image_print_verification_data(fit, sub_noffset, p);
}
/**
@@ -386,8 +372,7 @@ void fit_print_contents(const void *fit)
int images_noffset;
int confs_noffset;
int noffset;
int ndepth;
int count = 0;
int count;
int ret;
const char *p;
time_t timestamp;
@@ -424,20 +409,12 @@ void fit_print_contents(const void *fit)
}
/* Process its subnodes, print out component images details */
for (ndepth = 0, count = 0,
noffset = fdt_next_node(fit, images_noffset, &ndepth);
(noffset >= 0) && (ndepth > 0);
noffset = fdt_next_node(fit, noffset, &ndepth)) {
if (ndepth == 1) {
/*
* Direct child node of the images parent node,
* i.e. component image node.
*/
printf("%s Image %u (%s)\n", p, count++,
fit_get_name(fit, noffset, NULL));
count = 0;
fdt_for_each_subnode(noffset, fit, images_noffset) {
printf("%s Image %u (%s)\n", p, count++,
fit_get_name(fit, noffset, NULL));
fit_image_print(fit, noffset, p);
}
fit_image_print(fit, noffset, p);
}
/* Find configurations parent node offset */
@@ -449,25 +426,17 @@ void fit_print_contents(const void *fit)
}
/* get default configuration unit name from default property */
uname = (char *)fdt_getprop(fit, noffset, FIT_DEFAULT_PROP, NULL);
uname = (char *)fdt_getprop(fit, confs_noffset, FIT_DEFAULT_PROP, NULL);
if (uname)
printf("%s Default Configuration: '%s'\n", p, uname);
/* Process its subnodes, print out configurations details */
for (ndepth = 0, count = 0,
noffset = fdt_next_node(fit, confs_noffset, &ndepth);
(noffset >= 0) && (ndepth > 0);
noffset = fdt_next_node(fit, noffset, &ndepth)) {
if (ndepth == 1) {
/*
* Direct child node of the configurations parent node,
* i.e. configuration node.
*/
printf("%s Configuration %u (%s)\n", p, count++,
fit_get_name(fit, noffset, NULL));
count = 0;
fdt_for_each_subnode(noffset, fit, confs_noffset) {
printf("%s Configuration %u (%s)\n", p, count++,
fit_get_name(fit, noffset, NULL));
fit_conf_print(fit, noffset, p);
}
fit_conf_print(fit, noffset, p);
}
}
@@ -494,7 +463,6 @@ void fit_image_print(const void *fit, int image_noffset, const char *p)
ulong load, entry;
const void *data;
int noffset;
int ndepth;
int ret;
if (!CONFIG_IS_ENABLED(FIT_PRINT))
@@ -584,14 +552,8 @@ void fit_image_print(const void *fit, int image_noffset, const char *p)
}
/* Process all hash subnodes of the component image node */
for (ndepth = 0, noffset = fdt_next_node(fit, image_noffset, &ndepth);
(noffset >= 0) && (ndepth > 0);
noffset = fdt_next_node(fit, noffset, &ndepth)) {
if (ndepth == 1) {
/* Direct child node of the component image node */
fit_image_print_verification_data(fit, noffset, p);
}
}
fdt_for_each_subnode(noffset, fit, image_noffset)
fit_image_print_verification_data(fit, noffset, p);
}
/**
@@ -1477,7 +1439,6 @@ int fit_all_image_verify(const void *fit)
{
int images_noffset;
int noffset;
int ndepth;
int count;
/* Find images parent node offset */
@@ -1491,23 +1452,15 @@ int fit_all_image_verify(const void *fit)
/* Process all image subnodes, check hashes for each */
printf("## Checking hash(es) for FIT Image at %08lx ...\n",
(ulong)fit);
for (ndepth = 0, count = 0,
noffset = fdt_next_node(fit, images_noffset, &ndepth);
(noffset >= 0) && (ndepth > 0);
noffset = fdt_next_node(fit, noffset, &ndepth)) {
if (ndepth == 1) {
/*
* Direct child node of the images parent node,
* i.e. component image node.
*/
printf(" Hash(es) for Image %u (%s): ", count,
fit_get_name(fit, noffset, NULL));
count++;
count = 0;
fdt_for_each_subnode(noffset, fit, images_noffset) {
printf(" Hash(es) for Image %u (%s): ", count,
fit_get_name(fit, noffset, NULL));
count++;
if (!fit_image_verify(fit, noffset))
return 0;
printf("\n");
}
if (!fit_image_verify(fit, noffset))
return 0;
printf("\n");
}
return 1;
}
@@ -1734,7 +1687,6 @@ int fit_check_format(const void *fit, ulong size)
int fit_conf_find_compat(const void *fit, const void *fdt)
{
int ndepth = 0;
int noffset, confs_noffset, images_noffset;
const void *fdt_compat;
int fdt_compat_len;
@@ -1757,9 +1709,7 @@ int fit_conf_find_compat(const void *fit, const void *fdt)
/*
* Loop over the configurations in the FIT image.
*/
for (noffset = fdt_next_node(fit, confs_noffset, &ndepth);
(noffset >= 0) && (ndepth > 0);
noffset = fdt_next_node(fit, noffset, &ndepth)) {
fdt_for_each_subnode(noffset, fit, confs_noffset) {
const void *fdt;
const char *kfdt_name;
int kfdt_noffset, compat_noffset;
@@ -1768,9 +1718,6 @@ int fit_conf_find_compat(const void *fit, const void *fdt)
size_t sz;
int i;
if (ndepth > 1)
continue;
/* If there's a compat property in the config node, use that. */
if (fdt_getprop(fit, noffset, FIT_COMPAT_PROP, NULL)) {
fdt = fit; /* search in FIT image */

53
test/py/tests/test_fit.py
View File

@@ -426,3 +426,56 @@ class TestFitImage:
output = ubman.run_command_list(cmds)
assert "can't get kernel image!" in '\n'.join(output)
def test_fit_iminfo_configs_first(self, ubman, fsetup):
"""Regression: iminfo prints "Default Configuration" even when
/configurations is defined before /images in the source.
fit_print_contents() in boot/image-fit.c used to read the default
configuration name from whatever offset libfdt happened to return
after iterating /images children. With /images defined first that
offset accidentally landed on /configurations; with /configurations
defined first the read returned NULL and the line silently went
missing. Fixed in commit "boot/fit: read default-config property
from the configurations node".
"""
configs_first_its = '''
/dts-v1/;
/ {
description = "FIT with /configurations before /images";
#address-cells = <1>;
configurations {
default = "conf-1";
conf-1 {
description = "first config";
kernel = "kernel-1";
};
};
images {
kernel-1 {
description = "first image";
data = /incbin/("%(kernel)s");
type = "kernel";
arch = "sandbox";
os = "linux";
compression = "none";
load = <0x40000>;
entry = <0x40000>;
};
};
};
'''
fit = fit_util.make_fit(ubman, fsetup['mkimage'], configs_first_its,
fsetup, basename='configs-first.fit')
cmds = [
'host load hostfs 0 %#x %s' % (fsetup['fit_addr'], fit),
'iminfo %#x' % fsetup['fit_addr'],
]
output = '\n'.join(ubman.run_command_list(cmds))
assert "Default Configuration: 'conf-1'" in output, (
'iminfo output is missing the "Default Configuration" line for a '
'FIT whose /configurations node precedes /images. Output was:\n'
+ output)