[U-Boot] [PATCH v2 4/4] ARM: fix CONFIG_SPL_MAX_SIZE semantics

[Tom Rini]

On Thu, Apr 11, 2013 at 04:32:53PM +0200, Albert ARIBAUD wrote:
> Hi Stephen,
> 
> On Wed, 10 Apr 2013 17:16:49 -0600, Stephen Warren
> <swarren at wwwdotorg.org> wrote:
> 
> > On 04/10/2013 05:09 PM, Albert ARIBAUD wrote:
> > > On Thu, 11 Apr 2013 00:50:01 +0200, Albert ARIBAUD
> > > <albert.u.boot at aribaud.net> wrote:
> > > 
> > >> What we could do, though, is subdivide testing based on the existence or
> > >> non-existence of CONFIG_SPL_BSS_START_ADDR:
> > >>
> > >> - if CONFIG_SPL_BSS_START_ADDR exists, then we assume SPL image and
> > >>   BSS are disjoint and we test each one against its max size, as this
> > >>   patch series does;
> > >>
> > >> - if CONFIG_SPL_BSS_START_ADDR does not exist, then we assume SPL image
> > >>   and BSS are contiguous and we test the whole of SPL against the sum
> > >>   of CONFIG_SPL_MAX_SIZE and CONFIG_SPL_BSS_MAX_SIZE.
> > >>
> > >> I guess this will be considered useless complication -- after all,
> > >> once you have artificially partitioned your SPL space into image+BSS --
> > >> and you know from the build command how much should be allotted to each
> > >> of them -- the worst that can happen is that a later build fails with
> > >> an explicit error message forcing you to look at current image and BSS
> > >> size and adjust one or both of the max values accordingly.
> > > 
> > > P.S. In any case, the proposal above will go in, if at all, as a
> > > separate patch; the current patch series is going in right now as it is.
> > 
> > I wonder what the point of code-review is if you're just going to ignore it.
> 
> Can we please avoid this kind of talk? It has no argumentative value
> and can only lead to conflicts. Your account below is sufficient to
> convey your argumentation without any of the potential ill-effects of
> the above.
> 
> > What's really odd here is that by my reading of the relevant threads,
> > TomR already pointed out this exact issue earlier on, and you had agreed
> > that you'd resolve it in a way that didn't have this issue, yet the
> > patch has this issue???
> 
> Then our reading of the thread do not agree. Here is mine:
> 
> - in <http://article.gmane.org/gmane.comp.boot-loaders.u-boot/158046>,
>   Tom clearly asks for separate text+data+rodata size on one hand and
>   BSS size on the other hand (his #2 case, which he wants applied
>   uniformally and a solution found for Tegra.
> 
> - in <http://article.gmane.org/gmane.comp.boot-loaders.u-boot/158073> I
>   replied to Tom with a proposal in two parts, the first implementing
>   his #2 case strictly, the second implementing case #1 at the cost of
>   some minor added complexity and of muddying the symbol's semantics; I
>   suggested that if Tom really did not want the second part of my
>   proposal, it could be dropped and only the first part implemented.
> 
> - in <http://article.gmane.org/gmane.comp.boot-loaders.u-boot/158094>,
>   Tom asked that I keep part 1 and drop part 2 -- which I did.
> 
> Additionally, I did ask Tom on IRC if V2 was ok with him, and had his
> agreement.

To be clear, I think the fake partitioning scheme we use when the case
is "we care about text/data/rodata/bss fitting in $X" is not optimal,
but it covers all of the cases without adding more complexity / another
way to achieve the same ends.  If it turns out that platforms end up
needing to re-tweak this value we can come back and say it's time to add
a different mechanism for this hard-limit case.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20130411/2a1970ed/attachment.pgp>