From mboxrd@z Thu Jan 1 00:00:00 1970 From: Maxime Ripard Date: Tue, 13 Mar 2018 14:36:06 +0100 Subject: [U-Boot] [PATCH v3 01/20] spl: fix binman_sym output check In-Reply-To: <20180228195202.8183-2-miquel.raynal@bootlin.com> References: <20180228195202.8183-1-miquel.raynal@bootlin.com> <20180228195202.8183-2-miquel.raynal@bootlin.com> Message-ID: <20180313133606.eb7geqnlvvcjtf47@flea> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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 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: