Wolfgang Denk wd@denx.de wrote on 03/01/2010 20:51:53:
Dear Joakim Tjernlund,
In message <OF3EBC8B6E.2C26AE15-ONC12576A0.003379DE-C12576A0. 0039F518@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