[U-Boot] [PATCH v4 01/20] SPL: FIT: refactor FDT loading

[Kever Yang]

On 04/26/2017 08:32 AM, Andre Przywara 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.przywara at arm.com>
> Reviewed-by: Lokesh Vutla <lokeshvuta at ti.com>
> Reviewed-by: Simon Glass <sjg at chromium.org>

Reviewed-by: Kever Yang <kever.yang at rock-chips.com>
Tested-by: Kever Yang <kever.yang at rock-chips.com>

Thanks,
- Kever
> ---
>   common/spl/spl_fit.c | 88 ++++++++++++++++++++++++++++++++++------------------
>   1 file changed, 57 insertions(+), 31 deletions(-)
>
> diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
> index aae556f..67372ca 100644
> --- a/common/spl/spl_fit.c
> +++ b/common/spl/spl_fit.c
> @@ -22,13 +22,16 @@ 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)
> +/*
> + * Iterate over all /configurations subnodes and call a platform specific
> + * function to find the matching configuration.
> + * Returns the node offset or a negative error number.
> + */
> +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 +53,61 @@ 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 -ENOENT;
> +}
> +
> +static int spl_fit_select_index(const void *fit, int images, int *offsetp,
> +				const char *type, int index)
> +{
> +	const char *name, *str;
> +	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 conf_node;
> +	}
>   
> -		*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);
> +	name = fdt_getprop(fit, conf_node, type, &len);
> +	if (!name) {
> +		debug("cannot find property '%s': %d\n", type, len);
> +		return -EINVAL;
> +	}
>   
> -		return len;
> +	str = name;
> +	for (i = 0; i < index; i++) {
> +		str = strchr(str, '\0') + 1;
> +		if (!str || (str - name >= len)) {
> +			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, str);
> +	node = fdt_subnode_offset(fit, images, str);
> +	if (node < 0) {
> +		debug("cannot find image node '%s': %d\n", str, 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 +243,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;
>