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

Albert ARIBAUD
2013-04-11T14:32:53Z

Hi Stephen,

On Wed, 10 Apr 2013 17:16:49 -0600, Stephen Warren
<swarren@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@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.

Amicalement,
--
Albert.