[U-Boot] [PATCH 2/4] Use LINK_OFF to access global data

[Joakim Tjernlund]

Wolfgang Denk <wd at denx.de> wrote on 03/01/2010 20:51:53:
>
> Dear Joakim Tjernlund,
>
> In message <OF3EBC8B6E.2C26AE15-ONC12576A0.003379DE-C12576A0.
> 0039F518 at transmode.se> you wrote:
> >
> > > Will such changes be needed to all functions that work on (constant?)
> > > strings?  Or why here?
> >
> > Only those that work on constant strings before relocation to RAM. One alternative
> > is to change all call sites to puts(LINK_OFF(s)).
>
> Well, I understand this is not only puts(), but also all places where
> a constant strings are used. And this is a lot of places and a lot of
> functions.

It is not too bad when you limit the scope to pre RAM functions.
Consider I got my fairly complex 83xx board working with these
patches and that wasn't too bad was it?

>
> > > Yet another of these really, really invasive changes.  Is this
> > > really unavoidable?
> >
> > Yes, global data and strings accessed before relocation to RAM needs
> > to be wrapped with LINK_OFF to calculate the real address.
>
> Ouch.

Yeah, I whish ppc at least could access strings relative to text :(

>
> > I selected the changes I needed to for my 83xx board and then some. I don't
> > think serial.c needs any more changes.
>
> Understood. But that means we don't see the real scope of this change
> yet, as adapting a new board to use this featue will become a
True.

> trial-and-error procedure, adding incrementally more and more of these
> changes (unless someone performs a thorough review and catches most of
> these places in an initial commit).

So much else is trial-and-error so I don't think this a killer.
After all, this is a fairly advanced feature so it is no surprise it
will cost some effort to add support for a board.

>
> > Remember you only have to consider code that uses global data and
> > is executed before relocation to RAM and that limits the scope quite a lot.
> > The places to look at you find by following board.c's init_sequence.
> > I think my changes are fairly complete for PowerPC,83xx. There are missing
> > bits to do, mainly in other archs I think, but boards that doesn't need/want
> > this functionality don't have to change anything.
>
> I have to admit that I dislike the impact of this change, and I'm not
> sure if we really want to take this route.

This was exactly my thinking too. In the end I had to impl. it
to see how bad it would be. After seeing the result I decided
it was worth it.

>
> Comments from others are welcome and needed here!
>
> I think as follows:
>
> In the past, the majority of systems supported by U-Boot where
> booting from NOR flash or other memory devices. This made it easy to
> use common code (like library functions) both before and after
> relocation to the final location in RAM. For your current changes
> this means that we have a large number of places where we have to add
> this LINK_OFF stuff. This makes the code harder to read, much harder

You don't HAVE to add LINK_OFF all over. Only boards that whishes to use
this feature needs to add some more LINK_OFF calls.

> to understand (especially if it's not working during the initial
> bringup on new hardware), and harder to debug in general.
>
> If I try to see trends in the development of U-Boot I notice a
> growing number of systems that boot from NAND flash, DataFlash or
> that come with on-chip ROM code to load images from SDCard and other
> storage media. Such systems cannot make real benefit from the
> original design of U-Boot, as here U-Boot is inherently a
> second-stage boot loader which gets loaded by some other means. Even
> for NAND booting systems where we have the NAND boot code included
> within the U-Boot source tree we often cannot share much of the code
> between the primary and the secondary loader stages as there are
> usually tight restrictions on the maximum size for the primary loader
> image. Here a sharper separation of "primary" and "secondary" boot
> code within U-Boot would be benefical.
>
> I feel (but this is really just a feeling, and I definitely would like
> to hear what others think about this!) your PIC changes would be (or
> have been) useful for the former usage mode, but they come at a pretty
> heavy cost as they are really invasive to the code.  For the second
> usage mode they are not usable, or at least not useful.  This makes me
> wonder if we really should continue to work in this direction.

I have not worked with NAND based boards yet so I can't really say
if this is useful to such systems. I do think though that
NOR based board will be common for quite some time to come so
this new feature will be useful for a long time. Who knows, perhaps
some new designs will be NOR based just because u-boot has this feature.

FWIW, I do agree with you to not split u-boot into a 2 stage
loader, for pretty much the same reasons you listed.

   Jocke