[U-Boot] [PATCH v3 01/20] spl: fix binman_sym output check

[Maxime Ripard]

Hi Simon,

On Wed, Feb 28, 2018 at 08:51:43PM +0100, Miquel Raynal wrote:
> A previous commit introduced the use of binman in the SPL.
> 
> After the binman_sym call over the 'pos' symbol, the output value is
> checked against BINMAN_SYM_MISSING (-1UL). According to the
> documentation (tools/binman/README), when it comes to the 'pos'
> attribute:
> 
> pos:
> 	This sets the position of an entry within the image. The first
> 	byte of the image is normally at position 0. If 'pos' is not
> 	provided, binman sets it to the end of the previous region, or
> 	the start of the image's entry area (normally 0) if there is no
> 	previous region.
> 
> So instead of checking if the return value is BINMAN_SYM_MISSING, we
> should also check if the value is not null.
> 
> The failure happens when using both the SPL file and the U-Boot file
> independently instead of the concatenated file (SPL + padding + U-Boot).
> This is because the U-Boot binary file alone does not have the U-Boot
> header while it is present in the concatenation file. Not having the
> header forces the SPL to discover where it should load U-Boot. The
> binman_sym call is supposed to do that but fails. Because of the wrong
> check, the destination address was set to 0 while it should have been
> somewhere in RAM. This, obviously, stalls the board.
> 
> Fixes: 8bee2d251afb ("binman: Add binman symbol support to SPL")
> Signed-off-by: Miquel Raynal <miquel.raynal at bootlin.com>

I'm not sure why it wasn't sent to you, but could you please have a
look at that patch?

Thanks!
Maxime

> ---
>  common/spl/spl.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/common/spl/spl.c b/common/spl/spl.c
> index b1ce56d0d0..61d3071324 100644
> --- a/common/spl/spl.c
> +++ b/common/spl/spl.c
> @@ -127,8 +127,14 @@ void spl_set_header_raw_uboot(struct spl_image_info *spl_image)
>  	ulong u_boot_pos = binman_sym(ulong, u_boot_any, pos);
>  
>  	spl_image->size = CONFIG_SYS_MONITOR_LEN;
> -	if (u_boot_pos != BINMAN_SYM_MISSING) {
> -		/* biman does not support separate entry addresses at present */
> +
> +	/*
> +	 * Binman error cases: address of the end of the previous region or the
> +	 * start of the image's entry area (usually 0) if there is no previous
> +	 * region.
> +	 */
> +	if (u_boot_pos && u_boot_pos != BINMAN_SYM_MISSING) {
> +		/* Binman does not support separated entry addresses */
>  		spl_image->entry_point = u_boot_pos;
>  		spl_image->load_addr = u_boot_pos;
>  	} else {
> -- 
> 2.14.1
> 

-- 
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180313/94202066/attachment.sig>