[U-Boot] [PATCH v7 13/19] Makefile: u-boot-with-spl.bin: Fix SPL padding

[Scott Wood]

On 02/18/2013 12:52:51 PM, Benoît Thébaudeau wrote:
> On Monday, February 18, 2013 7:24:51 PM, Scott Wood wrote:
> > On 02/18/2013 12:22:58 PM, Benoît Thébaudeau wrote:
> > > On Monday, February 18, 2013 7:02:49 PM, Scott Wood wrote:
> > > > On 02/18/2013 12:00:52 PM, Benoît Thébaudeau wrote:
> > > > > The only question is if we may need to have an empty gap  
> between
> > > the
> > > > > SPL and
> > > > > U-Boot within the resulting image. I don't think so since that
> > > would
> > > > > mean that
> > > > > the target memory device has an area that is not really  
> available
> > > at
> > > > > the
> > > > > location of this gap.
> > > >
> > > > Why not allow that possibility?
> > >
> > > To save a config setting (there are already many for SPL) if this  
> is
> > > not
> > > strictly required, but also for the reason below.
> > >
> > > >  Maybe it's easier for the SPL to load
> > > > from a particular offset (e.g. NAND starting at the beginning  
> of a
> > > > block)?
> > >
> > > CONFIG_SPL_MAX_SIZE would be closer to a NAND mapping in that case
> > > (e.g. size of
> > > 1 NAND Flash block) than CONFIG_SPL_PAD_TO (address within RAM  
> that
> > > should be
> > > considered relatively to CONFIG_SPL_TEXT_BASE to get the NAND  
> offset).
> >
> > CONFIG_SPL_PAD_TO is for the placement of the payload
> 
> Correct.
> 
> > -- and it's not a
> > RAM address.
> 
> It doesn't have to be, but it may be for some configs.

Right.  My point is it shouldn't be defined as a RAM address.

> >  Currently it is a link address (or zero if the linker
> > script handles padding, or padding is not required for other  
> reasons).
> 
> Correct.
> 
> > With your patch it it is a file offset, IIUC.
> 
> With my patch, it is nothing at all since only CONFIG_SPL_MAX_SIZE is  
> used.

Sorry, I just meant with your change to how objcopy is invoked.  What  
you pass into objcopy is a file offset.

> > CONFIG_SPL_MAX_SIZE is what it says -- the maximum size that the SPL
> > may be, ideally to be enforced by the linker script.
> 
> Correct.
> 
> > They are different.  An SPL wanting the payload to begin as a block
> > boundary does not mean the hardware is suddenly capable of loading  
> an
> > entire block of SPL.
> 
> Sure, but my question is: Why would you want to have a 2-kiB SPL  
> followed by a
> 126-kiB gap before the payload? Why couldn't you place the payload on  
> the 1st
> page boundary after the SPL?

You can, and we usually do.  But size-limited SPLs may want to simplify  
(e.g. bad block detection needs some special logic to handle beginning  
inside a block), and it may not always be 126 KiB.

E.g. MPC8313ERDB uses small-page NAND, so it's only 12KiB that gets  
wasted.  It currently has MAX_SIZE of 4KiB but PAD_TO of base plus  
16KiB.

> If there are hardware constraints or something that make  
> CONFIG_SPL_PAD_TO
> useful in some cases, then let's use it, but otherwise, why keep it?

It's easier to mainain orthogonality (and use defaults for simplicity)  
than to restore it after the fact if we need it later.

> And if we keep it, do we change it to an image offset, or do we keep  
> it as a
> link address?

Changing to an image offset sounds good.

> > > Also, CONFIG_SPL_PAD_TO and CONFIG_SPL_MAX_SIZE depend on each  
> other:
> > > If both
> > > can be defined, you may change one forgetting the other one, which
> > > could e.g.
> > > result in an overlapping of SPL and U-Boot that won't show up at
> > > build time
> > > (with CONFIG_SPL_MAX_SIZE = 0x1000 and CONFIG_SPL_PAD_TO =
> > > CONFIG_SPL_TEXT_BASE
> > > + 0x800, the SPL would build fine, and objcopy wouldn't complain).
> >
> > So add a check that CONFIG_SPL_PAD_TO >= CONFIG_SPL_MAX_SIZE  
> (assuming
> > the new interpretation of CONFIG_SPL_PAD_TO as a file offset), and  
> let
> > CONFIG_SPL_PAD_TO default to CONFIG_SPL_MAX_SIZE if not set.
> 
> That would make sense. The current default value of 0 for  
> CONFIG_SPL_PAD_TO does
> not make sense since it means that the SPL can't know where the  
> payload is
> located within the image.

CONFIG_SPL_PAD_TO is not the mechanism that is used for finding the  
payload.  On mpc85xx it is unnecessary because the SPL will always be  
fixed size, because the reset vector goes at the end.  It's also  
possible that some SPLs could use linker symbols to find the end of the  
SPL, if they want to pack more tightly.

-Scott