Hi Andre,
On 19 January 2017 at 18:53, Andre Przywara <andre.przyw...@arm.com> wrote:
>
> Currently the SPL FIT loader uses the spl_fit_select_fdt() function to
> find the offset to the right DTB within the FIT image.
> For this it iterates over all subnodes of the /configuration node in
> the FIT tree and compares all "description" strings therein using a
> board specific matching function.
> If that finds a match, it uses the string in the "fdt" property of that
> subnode to locate the matching subnode in the /images node, which points
> to the DTB data.
> Now this works very well, but is quite specific to cover this particular
> use case. To open up the door for a more generic usage, let's split this
> function into:
> 1) a function that just returns the node offset for the matching
> configuration node (spl_fit_find_config_node())
> 2) a function that returns the image data any given property in a given
> configuration node points to, additionally using a given index into
> a possbile list of strings (spl_fit_select_index())
> This allows us to replace the specific function above by asking for the
> image the _first string of the "fdt" property_ in the matching
> configuration subnode points to.
>
> This patch introduces no functional changes, it just refactors the code
> to allow reusing it later.
>
> (diff is overly clever here and produces a hard-to-read patch, so I
> recommend to throw a look at the result instead).
>
> Signed-off-by: Andre Przywara <andre.przyw...@arm.com>
> ---
> common/spl/spl_fit.c | 82
> ++++++++++++++++++++++++++++++++--------------------
> 1 file changed, 51 insertions(+), 31 deletions(-)
>
Reviewed-by: Simon Glass <s...@chromium.org>
Please check below.
> diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
> index aae556f..c4e2f02 100644
> --- a/common/spl/spl_fit.c
> +++ b/common/spl/spl_fit.c
> @@ -22,13 +22,11 @@ static ulong fdt_getprop_u32(const void *fdt, int node,
> const char *prop)
> return fdt32_to_cpu(*cell);
> }
>
> -static int spl_fit_select_fdt(const void *fdt, int images, int *fdt_offsetp)
> +static int spl_fit_find_config_node(const void *fdt)
> {
> - const char *name, *fdt_name;
> - int conf, node, fdt_node;
> - int len;
> + const char *name;
> + int conf, node, len;
>
> - *fdt_offsetp = 0;
> conf = fdt_path_offset(fdt, FIT_CONFS_PATH);
> if (conf < 0) {
> debug("%s: Cannot find /configurations node: %d\n", __func__,
> @@ -50,39 +48,60 @@ static int spl_fit_select_fdt(const void *fdt, int
> images, int *fdt_offsetp)
> continue;
>
> debug("Selecting config '%s'", name);
> - fdt_name = fdt_getprop(fdt, node, FIT_FDT_PROP, &len);
> - if (!fdt_name) {
> - debug("%s: Cannot find fdt name property: %d\n",
> - __func__, len);
> - return -EINVAL;
> - }
>
> - debug(", fdt '%s'\n", fdt_name);
> - fdt_node = fdt_subnode_offset(fdt, images, fdt_name);
> - if (fdt_node < 0) {
> - debug("%s: Cannot find fdt node '%s': %d\n",
> - __func__, fdt_name, fdt_node);
> - return -EINVAL;
> + return node;
> + }
> +
> + return -1;
> +}
> +
> +static int spl_fit_select_index(const void *fit, int images, int *offsetp,
> + const char *type, int index)
> +{
> + const char *name, *img_name;
> + int node, conf_node;
> + int len, i;
> +
> + *offsetp = 0;
> + conf_node = spl_fit_find_config_node(fit);
> + if (conf_node < 0) {
> +#ifdef CONFIG_SPL_LIBCOMMON_SUPPORT
> + printf("No matching DT out of these options:\n");
> + for (node = fdt_first_subnode(fit, conf_node);
> + node >= 0;
> + node = fdt_next_subnode(fit, node)) {
> + name = fdt_getprop(fit, node, "description", &len);
> + printf(" %s\n", name);
> }
> +#endif
> + return -ENOENT;
> + }
>
> - *fdt_offsetp = fdt_getprop_u32(fdt, fdt_node, "data-offset");
> - len = fdt_getprop_u32(fdt, fdt_node, "data-size");
> - debug("FIT: Selected '%s'\n", name);
> + img_name = fdt_getprop(fit, conf_node, type, &len);
> + if (!img_name) {
> + debug("cannot find property '%s': %d\n", type, len);
> + return -EINVAL;
> + }
>
> - return len;
> + for (i = 0; i < index; i++) {
> + img_name = strchr(img_name, '\0') + 1;
Don't you need to check against strchr() returning NULL?
> + if (*img_name == '\0') {
> + debug("no string for index %d\n", index);
> + return -E2BIG;
> + }
> }
>
> -#ifdef CONFIG_SPL_LIBCOMMON_SUPPORT
> - printf("No matching DT out of these options:\n");
> - for (node = fdt_first_subnode(fdt, conf);
> - node >= 0;
> - node = fdt_next_subnode(fdt, node)) {
> - name = fdt_getprop(fdt, node, "description", &len);
> - printf(" %s\n", name);
> + debug("%s: '%s'\n", type, img_name);
> + node = fdt_subnode_offset(fit, images, img_name);
> + if (node < 0) {
> + debug("cannot find image node '%s': %d\n", img_name, node);
> + return -EINVAL;
> }
> -#endif
>
> - return -ENOENT;
> + *offsetp = fdt_getprop_u32(fit, node, "data-offset");
> + len = fdt_getprop_u32(fit, node, "data-size");
> +
> + return len;
> }
>
> static int get_aligned_image_offset(struct spl_load_info *info, int offset)
> @@ -218,7 +237,8 @@ int spl_load_simple_fit(struct spl_image_info *spl_image,
> memcpy(dst, src, data_size);
>
> /* Figure out which device tree the board wants to use */
> - fdt_len = spl_fit_select_fdt(fit, images, &fdt_offset);
> + fdt_len = spl_fit_select_index(fit, images, &fdt_offset,
> + FIT_FDT_PROP, 0);
> if (fdt_len < 0)
> return fdt_len;
>
> --
> 2.8.2
>
Regards,
Simon
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
