SPL framework re-design

To make sure I understand your new proposals, let me consolidate them here. Please correct me if I am wrong. Also, in the end I have some questions about your new proposal. Some of the questions are getting into the details. But I need the details to re-work my patches according to the new proposal.

Current Design:
* Currently a single board level Makefile determines what gets built into SPL.

* This Makefile chooses all the files to be built. Makes symbolic links to them in the board level SPL directory and builds and links them to create the SPL image.
* This structure is duplicated for different types of SPLs, nand_spl, onenand_ipl, mmc_spl etc.
* Directory structure is something like:
nand_spl/board/<vendor>/<board>/

New Design Proposed by Wolfgang:
* Have a top-level Makefile in the SPL root-directory - for instance 'nand_spl/Makefile'

* nand_spl/Makefile builds a generic library with the generic source files at this level.

* It then descends into sub-directories(SoC, board etc) to make the respective libraries at those levels.

* These libraries are finally linked together by nand_spl/Makefile to build the SPL image.

Open questions about the new proposal:
1. We may need more layers than just generic, SoC and board. For instance all SPLs typically use start.S from the CPU directory. Also, a lot of SoC code is typically SoC family generic. How about something like this for the directory structure:

nand_spl/<cpu>/

	      <soc-family>/
	      <soc1>/<board>/
	      <soc2>/<board>/

Maybe, <arch> needs to be added too.

2. How do we handle the type of SPLs that handle different media. For instance omap3 spl will support mmc and NAND. Can we have a directory tree starting with 'spl/'? If so, how does this tree share generic code

available in media specific directories such as nand_spl/ and mmc_spl/. Symbolic links?

3. Customizability - In the existing scheme what gets built into the SPL for a given board was completely customizable by the board level Makefile. That's no longer the case with the proposed scheme. Source files and Makefiles in the generic and CPU directory are shared by many boards. How do we allow customizability for individual boards. using CONFIG_ flags? This may be needed for the boards to make the right trade-offs based SRAM budget/requirements etc. Maybe, --gc-sections and -ffunction-sections help in dealing with this?

4. How do we handle the case where a given SoC doesn't need any board level code for the SPL(This is the case with OMAP4). Is this handled by just not have any <board> directories?

5. If so, how does the top-level SPL Makefile handle this? By checking for the existence of <board> directory before descending into it? How

does it handle the link step where the board level library is not available?

Dear Aneesh,

Quoted 6 lines · In message<[email protected]> you wrote:

To make sure I understand your new proposals, let me consolidate them here. Please correct me if I am wrong. Also, in the end I have some questions about your new proposal. Some of the questions are getting into the details. But I need the details to re-work my patches according to the new proposal.

Thanks for going into this!

Quoted 3 lines

Current Design:
* Currently a single board level Makefile determines what gets built into SPL.

And as we have learned so far, these board level Makefiles contain a large lot of duplicated code, especially when dealing with several boards based on the same or similar Soc(s).

Quoted 7 lines

* This Makefile chooses all the files to be built. Makes symbolic links to them in the board level SPL directory and builds and links them to create the SPL image.
* This structure is duplicated for different types of SPLs, nand_spl, onenand_ipl, mmc_spl etc.
* Directory structure is something like:
nand_spl/board/<vendor>/<board>/

All correct so far.

Quoted 3 lines

New Design Proposed by Wolfgang:
* Have a top-level Makefile in the SPL root-directory - for instance 'nand_spl/Makefile'

The longer I think about this the more I feel we should even take this one step further. Looking at what we have so far:

mmc_spl/
nand_spl/
onenand_ipl/

we are also duplicating the structure across different boot media. I think we should re-organize this as follows:

spl/
spl/common/
spl/mmc/
spl/nand/
spl/onenand/

This can probably done in an initial step which is more or less plain renaming and without any functional changes.

We would then have

	spl/Makefile
	...
	spl/common/Makefile
	...
	spl/nand/Makefile
	...

Quoted 2 lines

* nand_spl/Makefile builds a generic library with the generic source files at this level.

...this changes to: "spl/Makefile, spl/common/Makefile and spl/<bootdevice>/Makefile build libraries with the generic object files at their respective level (assuming these exist).

Quoted 3 lines

* It then descends into sub-directories(SoC, board etc) to make the respective libraries at those levels.

...again with the addition that these may or may not exist - depending if any board specific code is needed or not.

Quoted 2 lines

* These libraries are finally linked together by nand_spl/Makefile to build the SPL image.

...together by spl/Makefile ...

Quoted 11 lines

Open questions about the new proposal:
1. We may need more layers than just generic, SoC and board. For instance all SPLs typically use start.S from the CPU directory. Also, a lot of SoC code is typically SoC family generic. How about something like this for the directory structure:

nand_spl/<cpu>/

	<soc-family>/
	<soc1>/<board>/
	<soc2>/<board>/

Maybe,<arch> needs to be added too.

If this seems necessary, we may do this. But I would like to avoid to copying basicly the whole source tree (either as verbatim copies or as symlinks).

We should try to get rid of the need to create symbolic links. If we use the same source files as for the "normal", then we should also use the normal object files.

Quoted 4 lines

2. How do we handle the type of SPLs that handle different media. For instance omap3 spl will support mmc and NAND. Can we have a directory tree starting with 'spl/'? If so, how does this tree share generic code

Yes, this makes a lot of sense to me - see above.

Quoted 2 lines

available in media specific directories such as nand_spl/ and mmc_spl/. Symbolic links?

No. Let's put this stuff into spl/common/

Quoted 9 lines

3. Customizability - In the existing scheme what gets built into the SPL for a given board was completely customizable by the board level Makefile. That's no longer the case with the proposed scheme. Source files and Makefiles in the generic and CPU directory are shared by many boards. How do we allow customizability for individual boards. using CONFIG_ flags? This may be needed for the boards to make the right trade-offs based SRAM budget/requirements etc. Maybe, --gc-sections and -ffunction-sections help in dealing with this?

As far as building is concerned, the files to be built should always (unless truly common) be selected based on CONFIG_ settings in the Makefiles.

As far as linking is concerned, we can do the same for most cases (keep in mind that all linker scripts are run through the C preprocessor, so we can do a lot of things). For those cases where even more flexibility is needed, we can use custom linker scripts in the board directories.

Quoted 4 lines

we are also duplicating the structure across different boot media. I think we should re-organize this as follows:

Quoted 5 lines

spl/
spl/common/
spl/mmc/
spl/nand/
spl/onenand/

Can you please extend this to show the SoC/board directories etc. I guess they will go under spl/ and not under each media.

Quoted 3 lines

...this changes to: "spl/Makefile, spl/common/Makefile and spl/<bootdevice>/Makefile build libraries with the generic object files at their respective level (assuming these exist).

What's the distinction between spl/Makefile and spl/common/Makefile? I guess we won't have any source files at spl/ level?

Quoted 3 lines

We should try to get rid of the need to create symbolic links. If we use the same source files as for the "normal", then we should also use the normal object files.

You mean you will re-use *.o files with normal u-boot? If so, do you want to create symbolic links to them in the SPL directories or use them in-place?

Whether you reuse the source code or the object files we still need directories for all the levels for the respective Makefiles, right?

Also, at least some files will have to be built separately for SPL because they have conditional compilation with CONFIG_PRELOADER kind of flags.

I meant code that is media specific, such as MMC support, NAND support etc. I think these should still go in spl/mmc, spl/nand etc right?

A multi-device SPL will have to use 2 or more such libraries.

So, is the logic like this: If there is a linker script in the board directory use it, else look for a linker script in SoC directory?

BTW, my question was more about the contents of final image than the memory map. But, I think this can be handled with appropriate use of --gc-sections, -ffunction-sections, and -fdata-sections. The two main entry points board_init_f() and board_init_r() are typically implemented in the SoC layer or board layer of an SPL. This along with the use of above compiler switches will help SoCs/boards to link in only what they need, right?

Dear Aneesh V,

Quoted 10 lines · In message<[email protected]> you wrote:

Quoted 9 lines

we are also duplicating the structure across different boot media. I think we should re-organize this as follows:

spl/
spl/common/
spl/mmc/
spl/nand/
spl/onenand/

Quoted 2 lines

Can you please extend this to show the SoC/board directories etc. I guess they will go under spl/ and not under each media.

Correct, i. e. please add for example:

	spl/board/freescale/mx31pdk/
	spl/board/freescale/mx31pdk/Makefile
	...

Quoted 7 lines

Quoted 4 lines

...this changes to: "spl/Makefile, spl/common/Makefile and spl/<bootdevice>/Makefile build libraries with the generic object files at their respective level (assuming these exist).

What's the distinction between spl/Makefile and spl/common/Makefile? I guess we won't have any source files at spl/ level?

I think we can implement the logic to decide which directories need to be entered and built into spl/Makefile, so we can keep this out of the top level Makefile.

spl/common/Makefile is responsible for building the common code in the spl/common/ directory.

Correct, I do not see need for any sources in spl/

Quoted 7 lines

Quoted 3 lines

We should try to get rid of the need to create symbolic links. If we use the same source files as for the "normal", then we should also use the normal object files.

You mean you will re-use *.o files with normal u-boot? If so, do you want to create symbolic links to them in the SPL directories or use them in-place?

We should use them in-place. Using --gc-sections and -ffunction-sections we have enough granularity to select only what we really need.

Quoted 2 lines

Whether you reuse the source code or the object files we still need directories for all the levels for the respective Makefiles, right?

Can we not use the objects that get normally built, with the existing Makefiles?

Quoted 4 lines

Also, at least some files will have to be built separately for SPL because they have conditional compilation with CONFIG_PRELOADER kind of flags.

Please let's check where this is needed, and how we can handle this. I'd really like to get rid of this symlinking.

Quoted 4 lines

Quoted 3 lines

Can you please extend this to show the SoC/board directories etc. I guess they will go under spl/ and not under each media.

Quoted 5 lines

Correct, i. e. please add for example:

	spl/board/freescale/mx31pdk/
	spl/board/freescale/mx31pdk/Makefile
	...

I thought we were going to have cpu directory SoC directory etc in this directory structure.

Quoted 2 lines

Can we not use the objects that get normally built, with the existing Makefiles?

But where do you add the reference to SoC level and CPU level object files? Which library do you make them part of? I thought the board level Makefile was meant only to build the board specific library.

For example, let's say we have board 'a' and 'b' of same SoC(soc). Unless we have a SoC directory we may have to do something like this.

Quoted 6 lines

Quoted 3 lines

Also, at least some files will have to be built separately for SPL because they have conditional compilation with CONFIG_PRELOADER kind of flags.

Please let's check where this is needed, and how we can handle this. I'd really like to get rid of this symlinking.

One case is start.S. We need a simplified start.S for SPL(no relocation, no interrupt handling etc).

There are places where CONFIG_PRELOADER is used today. But maybe, these could be avoided it we try.

Dear Aneesh,

The longer I think about this the more I feel we should even take this one step further. ?Looking at what we have so far:

? ? ? ?mmc_spl/
? ? ? ?nand_spl/
? ? ? ?onenand_ipl/

we are also duplicating the structure across different boot media. I think we should re-organize this as follows:

? ? ? ?spl/
? ? ? ?spl/common/
? ? ? ?spl/mmc/
? ? ? ?spl/nand/
? ? ? ?spl/onenand/

This can probably done in an initial step which is more or less plain renaming and without any functional changes.

We would then have

? ? ? ?spl/Makefile
? ? ? ?...
? ? ? ?spl/common/Makefile
? ? ? ?...
? ? ? ?spl/nand/Makefile
? ? ? ?...

Quoted 2 lines

* nand_spl/Makefile builds a generic library with the generic source files at this level.

...this changes to: "spl/Makefile, spl/common/Makefile and spl/<bootdevice>/Makefile build libraries with the generic object files at their respective level (assuming these exist).

Quoted 2 lines

* It then descends into sub-directories(SoC, board etc) to make the respective libraries at those levels.

...again with the addition that these may or may not exist - depending if any board specific code is needed or not.

Quoted 2 lines

* These libraries are finally linked together by nand_spl/Makefile to build the SPL image.

...together by spl/Makefile ...

Quoted 11 lines

Open questions about the new proposal:
1. We may need more layers than just generic, SoC and board. For instance all SPLs typically use start.S from the CPU directory. Also, a lot of SoC code is typically SoC family generic. How about something like this for the directory structure:

nand_spl/<cpu>/
? ? ? ? ? ? <soc-family>/
? ? ? ? ? ? <soc1>/<board>/
? ? ? ? ? ? <soc2>/<board>/
Maybe, <arch> needs to be added too.

If this seems necessary, we may do this. But I would like to avoid to copying basicly the whole source tree (either as verbatim copies or as symlinks).

We should try to get rid of the need to create symbolic links. If we use the same source files as for the "normal", then we should also use the normal object files.

Quoted 4 lines

2. How do we handle the type of SPLs that handle different media. For instance omap3 spl will support mmc and NAND. Can we have a directory tree starting with 'spl/'? If so, how does this tree share generic code

Yes, this makes a lot of sense to me - see above.

Quoted 2 lines

available in media specific directories such as nand_spl/ and mmc_spl/. Symbolic links?

No. ?Let's put this stuff into ?spl/common/

Quoted 9 lines

3. Customizability - In the existing scheme what gets built into the SPL for a given board was completely customizable by the board level Makefile. That's no longer the case with the proposed scheme. Source files and Makefiles in the generic and CPU directory are shared by many boards. How do we allow customizability for individual boards. using CONFIG_ flags? This may be needed for the boards to make the right trade-offs based SRAM budget/requirements etc. Maybe, --gc-sections and -ffunction-sections help in dealing with this?

As far as building is concerned, the files to be built should always (unless truly common) be selected based on CONFIG_ settings in the Makefiles.

As far as linking is concerned, we can do the same for most cases (keep in mind that all linker scripts are run through the C preprocessor, so we can do a lot of things). ?For those cases where even more flexibility is needed, we can use custom linker scripts in the board directories.

Quoted 3 lines

4. How do we handle the case where a given SoC doesn't need any board level code for the SPL(This is the case with OMAP4). Is this handled by just not have any <board> directories?

Right. ?Or, if a board directory exists for other reasons, by just building an empty board library.

Quoted 2 lines

5. If so, how does the top-level SPL Makefile handle this? By checking for the existence of <board> directory before descending into it? How

Yes.

Quoted 2 lines

does it handle the link step where the board level library is not available?

In the same way: we test if the <board> directory exists, and only if this is the case it gets added to the list of directories to descend into for building, and to the list of libraries to be linked together.

Best regards,

Wolfgang Denk

Quoted 1 line

Dear Aneesh,

Quoted 3 lines

We should try to get rid of the need to create symbolic links. If we use the same source files as for the "normal", then we should also use the normal object files.

By using something like CONFIG_UBOOT_SPL_BUILD in the make environment and as CFLAG all code can be reused without symlinking or copying. You need only a separate directory for putting the object files in.

Quoted 9 lines

Quoted 3 lines

2. How do we handle the type of SPLs that handle different media. For instance omap3 spl will support mmc and NAND. Can we have a directory tree starting with 'spl/'? If so, how does this tree share generic code

Yes, this makes a lot of sense to me - see above.

Quoted 2 lines

available in media specific directories such as nand_spl/ and mmc_spl/. Symbolic links?

Quoted 1 line

No. Let's put this stuff into spl/common/

To use the spl directory as remote build directory, the obj and src variables must be tweaked a little. To keep this changes minimal, it is not possible to have further source files and directories inside the spl directory. I suggest to put common spl code in TOPDIR/lib/spl/common, TOPDIR/lib/spl/nand and so on.

Hello everybody,

I've read the whole thread and I really like what Daniel suggests but I just want to speak it in a little bit different words.

I wonder why do we need this whole spl thing in the first place (well, surely I know what they are used for but why do we need a separate entity for this)? Isn't it just the same U-Boot in, well, very special configuration (minimal set of drivers, no shell, etc)? Why do we need a whole shadow tree at spl/ instead of just providing the _configuration_?

Am I missing something?

Aneesh, what's the state of your patches? I'm especially interrested in OMAP3 (AM3517) support. Maybe I will be able to help you.

Quoted 8 lines

I wonder why do we need this whole spl thing in the first place (well, surely I know what they are used for but why do we need a separate entity for this)? Isn't it just the same U-Boot in, well, very special configuration
(minimal set of drivers, no shell, etc)? Why do we need a whole shadow tree
at spl/ instead of just providing the _configuration_?

Quoted 1 line

Am I missing something?

The reason is that the regular U-Boot is not configurable enough to build the extremely small images that should fit in internal RAM. The last time I attempted, I ended up getting an ~60KB image for OMAP4(that too without any of the hardware initialization I am adding in my SPL work).

Quoted 2 lines

Aneesh, what's the state of your patches? I'm especially interrested in OMAP3 (AM3517) support. Maybe I will be able to help you.

I should be able to send out an updated revision of my series once we finalize on the new framework for SPL.

BTW, John Rigby had sent out a series sometime back for OMAP3 NAND SPL. That can be integrated with my work and we will get an SPL that supports both MMC and NAND. I guess Simon Schwarz is also doing some work lately on OMAP3.

Hi Aneesh,

Quoted 8 lines · Aneesh V

Quoted 8 lines

I wonder why do we need this whole spl thing in the first place (well, surely I know what they are used for but why do we need a separate entity for this)? Isn't it just the same U-Boot in, well, very special configuration
(minimal set of drivers, no shell, etc)? Why do we need a whole shadow tree
at spl/ instead of just providing the _configuration_?

Quoted 7 lines

Quoted 1 line

Am I missing something?

The reason is that the regular U-Boot is not configurable enough to build the extremely small images that should fit in internal RAM. The last time I attempted, I ended up getting an ~60KB image for OMAP4(that too without any of the hardware initialization I am adding in my SPL work).

Yes, surely I understand that currently U-Boot is not configurable enough to meet hard SPL constraints. But why don't we add the required configuration options instead of implementing the SPL thing separately? Again, maybe I'm missing something but it looks like not very difficult task to add the required configuration options and this approach seems to be more straight to me...

You mentioned that /spl can not be used for source files. Isn't there a way to workaround this problem?

Also, I agree with Scott's opinion that re-compiling some files while re-using the binary of some other files won't be a good idea. In this case, CONFIG_PRELOADER will be honored in some files but not in other files. That will be a source of confusion for developers.

BTW, John Rigby had sent out a series sometime back for OMAP3 NAND SPL. That can be integrated with my work and we will get an SPL that supports both MMC and NAND. I guess Simon Schwarz is also doing some work lately on OMAP3.

Quoted 7 lines

I wonder why do we need this whole spl thing in the first place (well, surely I know what they are used for but why do we need a separate entity for this)? Isn't it just the same U-Boot in, well, very special configuration (minimal set of drivers, no shell, etc)? Why do we need a whole shadow tree at spl/ instead of just providing the _configuration_?

Quoted 1 line

Am I missing something?

The reason is that the regular U-Boot is not configurable enough to build the extremely small images that should fit in internal RAM. The last time I attempted, I ended up getting an ~60KB image for OMAP4(that too without any of the hardware initialization I am adding in my SPL work).

BTW, John Rigby had sent out a series sometime back for OMAP3 NAND SPL.

That can be integrated with my work and we will get an SPL that supports both MMC and NAND. I guess Simon Schwarz is also doing some work lately on OMAP3.

Dear Aneesh,

Quoted 7 lines · In message <[email protected]> you wrote:

Quoted 7 lines

I wonder why do we need this whole spl thing in the first place (well, surely I know what they are used for but why do we need a separate entity for this)? Isn't it just the same U-Boot in, well, very special configuration (minimal set of drivers, no shell, etc)? Why do we need a whole shadow tree at spl/ instead of just providing the _configuration_?

Quoted 7 lines

Quoted 1 line

Am I missing something?

The reason is that the regular U-Boot is not configurable enough to build the extremely small images that should fit in internal RAM. The last time I attempted, I ended up getting an ~60KB image for OMAP4(that too without any of the hardware initialization I am adding in my SPL work).

This statement does not make much sense to me. If we can do it in the spl/ directory, we should be able to do it in any other directory as well. The worst to happen is that we have to keep two setsof object files separated, but chosing a different suffix should be sufficient.

Quoted 5 lines

This statement does not make much sense to me. If we can do it in the spl/ directory, we should be able to do it in any other directory as well. The worst to happen is that we have to keep two setsof object files separated, but chosing a different suffix should be sufficient.

We do it in the spl/ directory by bypassing the normal makefile config system, specifying a board-and-spl-specific list of objects instead.

It could of course be done with the standard config system, but it will require that every bit of code in U-Boot be enabled only with a particular config option -- no "always on" code. This would be a good thing anyway, but it will take some work to do it cleanly. A first step would probably be a global "finegrained/small" flag that configs use to opt into the new system, but eventually all bits of functionality should have appropriate individual config symbols.

I wonder why do we need this whole spl thing in the first place (well, surely I know what they are used for but why do we need a separate entity for this)? Isn't it just the same U-Boot in, well, very special configuration (minimal set of drivers, no shell, etc)? Why do we need a whole shadow tree at spl/ instead of just providing the _configuration_?

Dear Ilya,

Quoted 6 lines · In message <[email protected]> you wrote:

I wonder why do we need this whole spl thing in the first place (well, surely I know what they are used for but why do we need a separate entity for this)? Isn't it just the same U-Boot in, well, very special configuration (minimal set of drivers, no shell, etc)? Why do we need a whole shadow tree at spl/ instead of just providing the _configuration_?

Good point. ?Eventually we can ?just add additional build rules for new object files (say, ".splo" instead of ".o") ?

Quoted 3 lines

Good point. Eventually we can just add additional build rules for
new object files (say, ".splo" instead of ".o") ?

I agree this approach seems to be the best one.
But then we have to create SPL-specific libraries too, right? (e.g. lib$(ARCH).splo, lib$(CPU).splo)

Dear Ilya,

Quoted 1 line · In message <[email protected]> you wrote:

Quoted 5 lines

I wonder why do we need this whole spl thing in the first place (well, surely I know what they are used for but why do we need a separate entity for this)? Isn't it just the same U-Boot in, well, very special configuration (minimal set of drivers, no shell, etc)? Why do we need a whole shadow tree at spl/ instead of just providing the _configuration_?

Good point. Eventually we can just add additional build rules for new object files (say, ".splo" instead of ".o") ?

Quoted 3 lines

Good point. Eventually we can just add additional build rules for new object files (say, ".splo" instead of ".o") ?

No need for new extensions -- we should be able to use the target directory to influence rule selection.

Dear Scott Wood,

Quoted 4 lines · In message <[email protected]> you wrote:

Quoted 3 lines

Good point. Eventually we can just add additional build rules for new object files (say, ".splo" instead of ".o") ?

Quoted 2 lines

No need for new extensions -- we should be able to use the target directory to influence rule selection.

But if we do not create a new hierarchy of target directories we will have the "normal" and the "spl" objects in parallel (and I don't want to delete one when building the other).

Quoted 4 lines

But if we do not create a new hierarchy of target directories we will have the "normal" and the "spl" objects in parallel (and I don't want to delete one when building the other).

What's wrong with creating a new hierarchy of target directories? It would be like specifying a different output directory.

Dear Scott Wood,

Quoted 1 line · In message <[email protected]> you wrote:

Quoted 6 lines

Quoted 3 lines

But if we do not create a new hierarchy of target directories we will have the "normal" and the "spl" objects in parallel (and I don't want to delete one when building the other).

What's wrong with creating a new hierarchy of target directories? It would be like specifying a different output directory.

The question came up what we need it for.

Quoted 5 lines

Quoted 5 lines

Quoted 4 lines

But if we do not create a new hierarchy of target directories we will have the "normal" and the "spl" objects in parallel (and I don't want to delete one when building the other).

Quoted 4 lines

Quoted 2 lines

What's wrong with creating a new hierarchy of target directories? It would be like specifying a different output directory.

The question came up what we need it for.

Just seems cleaner to me than jamming it into the file extension. If we're treating it as a separate build, it should go into a separate place. It's not really a different type of file.

Dear Scott Wood,

Quoted 8 lines · In message<[email protected]> you wrote:

Quoted 8 lines

Quoted 7 lines

Quoted 4 lines

But if we do not create a new hierarchy of target directories we will have the "normal" and the "spl" objects in parallel (and I don't want to delete one when building the other).

What's wrong with creating a new hierarchy of target directories? It would be like specifying a different output directory.

Quoted 5 lines

Quoted 1 line

The question came up what we need it for.

Just seems cleaner to me than jamming it into the file extension. If we're treating it as a separate build, it should go into a separate place. It's not really a different type of file.

I'm fine with that as well.

1. If there are SPL customized generic files like the nand_spl/nand_boot.c where do we keep them? I suggest that we keep them in spl/nand, spl/onenand etc. And for the object file hierarchy let's have something like spl/obj. How about that?

Quoted 4 lines · On Tue, 28 Jun 2011 12:24:11 +0530 Aneesh V<[email protected]> wrote:

1. If there are SPL customized generic files like the nand_spl/nand_boot.c where do we keep them? I suggest that we keep them in spl/nand, spl/onenand etc. And for the object file hierarchy let's have something like spl/obj. How about that?

How about drivers/nand/generic_spl.c, drivers/nand/fsl_elbc_spl.c, etc.?

Hi all,

for my MIPS based boards I tested a approach similar to Wolfgang's one in the last weeks.
My goal was to create a SPL image, that is able to boot from a SPI flash.

The basic idea is to have a spl directory that is used as remote build directory for all object files
needed for the SPL image. It contains only the Makefile and nothing else. This Makefile is
a strongly simplified version of the TOPDIR/Makefile, uses TOPDIR/config.mk and creates
a u-boot-spl.bin. The SPL build can be enabled by defining CONFIG_UBOOT_SPL in the board config file.

If the spl/Makefiles becomes active, an additional variable name CONFIG_UBOOT_SPL_BUILD wiil be
exported in the make environment. Additionally -DCONFIG_UBOOT_SPL_BUILD will be added
to the CFLAGS (similar to -DCONFIG_PRELOADER). This allows us to reuse almost of the arch, SoC and
board code.

Hi Daniel,

This looks like an interesting alternative.

Quoted 7 lines · Daniel Schwierzeck

Hi all,

for my MIPS based boards I tested a approach similar to Wolfgang's one in the last weeks.
My goal was to create a SPL image, that is able to boot from a SPI flash.

Quoted 8 lines

The basic idea is to have a spl directory that is used as remote build directory for all object files
needed for the SPL image. It contains only the Makefile and nothing else. This Makefile is
a strongly simplified version of the TOPDIR/Makefile, uses TOPDIR/config.mk and creates
a u-boot-spl.bin. The SPL build can be enabled by defining CONFIG_UBOOT_SPL in the board config file.

In the last few mails Wolfgang was suggesting re-use of object files themselves, not the source files. In this respect his approach may be different from yours. But I think his objective was to avoid the symbolic link business, which this approach achieves.

Quoted 9 lines

If the spl/Makefiles becomes active, an additional variable name CONFIG_UBOOT_SPL_BUILD wiil be
exported in the make environment. Additionally -DCONFIG_UBOOT_SPL_BUILD will be added
to the CFLAGS (similar to -DCONFIG_PRELOADER). This allows us to reuse almost of the arch, SoC and
board code.

So, you are essentially re-using the make infrastructure of normal U-Boot with a different top-level Makefile and the additional flags, right?

With this scheme can you build two different SPLs for a given board, let's say a NAND spl and another MMC spl. I don't know if this is a valid case, but just wondering. I guess you have a unique u-boot- spl.bin much like you have single u-boot.bin, right?

Will you be sending your patches to the list?

In the last few mails Wolfgang was suggesting re-use of object files themselves, not the source files. In this respect his approach may be different from yours. But I think his objective was to avoid the symbolic link business, which this approach achieves.

The relocate_code and board_init_r functions must not be compiled, they are not needed anyway. This
can be simply controlled with -DCONFIG_UBOOT_SPL_BUILD.

Quoted 4 lines

Quoted 2 lines

available in media specific directories such as nand_spl/ and mmc_spl/. Symbolic links?

No. Let's put this stuff into spl/common/

To use the spl directory as remote build directory, the obj and src variables must be tweaked a little. To keep this changes minimal, it is not possible to have further source files and directories inside the spl directory. I suggest to put common spl code in TOPDIR/lib/spl/common, TOPDIR/lib/spl/nand and so on.

Dear Daniel Schwierzeck,

Quoted 1 line · In message <[email protected]> you wrote:

Quoted 3 lines

The relocate_code and board_init_r functions must not be compiled, they are not needed anyway. This
can be simply controlled with -DCONFIG_UBOOT_SPL_BUILD.

This is very much wrong. ?In the general case, you still need relocation (because the final start address of the U-Boot code canonly be determined at runtime), and you definitely need the code in board_init_r().

Quoted 5 lines

Quoted 5 lines

Quoted 4 lines

available in media specific directories such as nand_spl/ and mmc_spl/. Symbolic links?

Quoted 6 lines

Quoted 1 line

No. ?Let's put this stuff into ?spl/common/

To use the spl directory as remote build directory, the obj and src variables must be tweaked a little. To keep this changes minimal, it is not possible to have further source files and directories inside the spl directory. I suggest to put common spl code in TOPDIR/lib/spl/common, TOPDIR/lib/spl/nand and so on.

No. ?All SPL related stuff should go into spl/

Dear Wolfgang,

Quoted 4 lines · Wolfgang Denk

Dear Daniel Schwierzeck,

Quoted 1 line · In message <[email protected]> you wrote:

Quoted 8 lines

Quoted 3 lines

The relocate_code and board_init_r functions must not be compiled, they are not needed anyway. This
can be simply controlled with -DCONFIG_UBOOT_SPL_BUILD.

This is very much wrong. ?In the general case, you still need relocation (because the final start address of the U-Boot code canonly be determined at runtime), and you definitely need the code in board_init_r().

I guess we are talking about different kinds of SPL. In my understanding a SPL runs inside an internal SRAM, initializes any external RAM and loads U-Boot from
any kind of memory or flash device into that RAM at a fixed load address and then jumps to that address. In this case no relocation is needed. Some kind of SoC specific booting mechanism like a NAND-IPL or hard-wired Boot-ROM loads the SPL initially into SRAM and jumps to it.

This is an extention of Daniel Schwierzeck's work [1] on a new SPL framework and is intented only as a prototype to facilitate further discussion.

Please refer [2] for an overview of this approach:

I have extended his work to make it a little more generic, did some minor modifications and adapted it for OMAP4 as a prototype.

Appreciate your feedback.

[1] https://github.com/danielschwierzeck/u-boot-spl/commits/spl
[2] http://marc.info/?l=u-boot&m=130823112116502&w=2

Aneesh V (3):
armv7: adapt Makefile for spl building
omap: common spl support for OMAP3/4
omap4: adapt Makefile for spl building

Daniel Schwierzeck (4):
Adapt config.mk for usage in spl/Makefile
Use ALL-y style instead of ifeq blocks for better readability and
upgradeability
Add new folder and build system for SPL
Hook spl directory into main Makefile

  Makefile                                |   26 ++++----
  arch/arm/cpu/armv7/Makefile             |    9 ++-
  arch/arm/cpu/armv7/omap-common/Makefile |    9 ++-
  arch/arm/cpu/armv7/omap-common/spl.c    |   56 ++++++++++++++++
  arch/arm/cpu/armv7/omap-common/spl.lds  |   62 ++++++++++++++++++
  arch/arm/cpu/armv7/omap4/Makefile       |   12 ++--
  config.mk                               |   32 ++++++++--
  include/configs/omap4_sdp4430.h         |    8 +++
  spl/Makefile                            |  105 +++++++++++++++++++++++++++++++
  9 files changed, 287 insertions(+), 32 deletions(-)

create mode 100644 arch/arm/cpu/armv7/omap-common/spl.c create mode 100644 arch/arm/cpu/armv7/omap-common/spl.lds create mode 100644 spl/Makefile

[aneesh at ti.com:
1. Changed the definition of OBJTREE and the way 'obj' is defined.
2. Defined a flag to identify regular U-Boot build
3. Taking care of autoconf.mk etc for SPL due to changed OBJTREE ]

[daniel.schwierzeck at googlemail.com:
1. reverted changes on implicite make rules
2. readded definition of -DCONFIG_UBOOT_SPL_BUILD in CPPFLAGS ]

Signed-off-by Aneesh V<[email protected]>

Signed-off-by Daniel Schwierzeck<[email protected]>

---
  config.mk |   28 ++++++++++++++++++++++++++++
  1 files changed, 28 insertions(+), 0 deletions(-)

config.mk +6 -0

index 2eb7fa2..a16e761 100644
@@ -116,8 +116,14 @@ RANLIB	= $(CROSS_COMPILE)RANLIB
  #########################################################################
 
  # Load generated board configuration
+ifdef CONFIG_UBOOT_SPL_BUILD
+# OBJTREE points to /spl/obj inside the original OBJTREE for SPL
+sinclude $(OBJTREE)/../../include/autoconf.mk
+sinclude $(OBJTREE)/../../include/config.mk
+else
  sinclude $(OBJTREE)/include/autoconf.mk
  sinclude $(OBJTREE)/include/config.mk
+endif
 
  # Some architecture config.mk files need to know what CPUDIR is set to,
  # so calculate CPUDIR before including ARCH/SOC/CPU config.mk files.

config.mk +3 -0

+
+CONFIG_NORMAL_UBOOT = $(if $(CONFIG_UBOOT_SPL_BUILD),,y)
+
  ifneq ($(RESET_VECTOR_ADDRESS),)
  CPPFLAGS += -DRESET_VECTOR_ADDRESS=$(RESET_VECTOR_ADDRESS)
  endif
 

[aneesh at ti.com:
1. Changed the definition of OBJTREE and the way 'obj' is defined.
2. Defined a flag to identify regular U-Boot build
3. Taking care of autoconf.mk etc for SPL due to changed OBJTREE ]

[daniel.schwierzeck at googlemail.com:
1. reverted changes on implicite make rules
2. readded definition of -DCONFIG_UBOOT_SPL_BUILD in CPPFLAGS ]

+CONFIG_NORMAL_UBOOT = $(if $(CONFIG_UBOOT_SPL_BUILD),,y)

Dear Daniel Schwierzeck,

Quoted 10 lines · In message<1309883182-12854-2-git-send-email-daniel.schwierzeck@googlemail.com>…

[aneesh at ti.com:
1. Changed the definition of OBJTREE and the way 'obj' is defined.
2. Defined a flag to identify regular U-Boot build
3. Taking care of autoconf.mk etc for SPL due to changed OBJTREE ]

[daniel.schwierzeck at googlemail.com:
1. reverted changes on implicite make rules
2. readded definition of -DCONFIG_UBOOT_SPL_BUILD in CPPFLAGS ]

...

config.mk ifneq ($(CONFIG_SYS_TEXT_BASE),) · line 181

Quoted 5 lines

config.mk +5 -0

+# Allow compiling of C and ASM code parts in different way.
+# Serves also as a replacement for CONFIG_PRELOADER.
+ifneq ($(CONFIG_UBOOT_SPL_BUILD),)
+CPPFLAGS += -DCONFIG_UBOOT_SPL_BUILD
+endif

I don't understand this comment.

Quoted 1 line

+CONFIG_NORMAL_UBOOT = $(if $(CONFIG_UBOOT_SPL_BUILD),,y)

Is this not redundant, then?

Quoted 4 lines

Quoted 2 lines

+CONFIG_NORMAL_UBOOT = $(if $(CONFIG_UBOOT_SPL_BUILD),,y)

Is this not redundant, then?

Yes, it is. But it was handy for conditional inclusion of source files in the Makefiles, like this:

(quoted diff) +6 -0

+# used only by normal u-boot
+COBJS-$(CONFIG_NORMAL_UBOOT)	+= fileA.o
+# used only by SPL
+COBJS-$(CONFIG_UBOOT_SPL_BUILD)	+= fileB.o
+# used by SPL and normal u-boot
+COBJS-y	+= fileC.o

Without CONFIG_NORMAL_UBOOT this becomes a little cumbersome.

Dear Aneesh V,

Quoted 15 lines · In message<[email protected]> you wrote:

Quoted 4 lines

Quoted 2 lines

+CONFIG_NORMAL_UBOOT = $(if $(CONFIG_UBOOT_SPL_BUILD),,y)

Is this not redundant, then?

Yes, it is. But it was handy for conditional inclusion of source files in the Makefiles, like this:

(quoted diff) +6 -0

+# used only by normal u-boot
+COBJS-$(CONFIG_NORMAL_UBOOT)	+= fileA.o
+# used only by SPL
+COBJS-$(CONFIG_UBOOT_SPL_BUILD)	+= fileB.o
+# used by SPL and normal u-boot
+COBJS-y	+= fileC.o

Quoted 1 line

Without CONFIG_NORMAL_UBOOT this becomes a little cumbersome.

Hm... instead of

COBJS-$(CONFIG_NORMAL_UBOOT) += fileA.o

we could use

COBJS-$(if $(CONFIG_UBOOT_SPL_BUILD),,y)

Quoted 5 lines

Quoted 2 lines

Without CONFIG_NORMAL_UBOOT this becomes a little cumbersome.

Hm... instead of

Quoted 5 lines

COBJS-$(CONFIG_NORMAL_UBOOT) += fileA.o

we could use

COBJS-$(if $(CONFIG_UBOOT_SPL_BUILD),,y)

This is what I was trying to avoid. Isn't the above more obvious for lay-users of make?

Dear Daniel Schwierzeck,

Quoted 10 lines · In message <1309883182-12854-2-git-send-email-daniel.schwierzeck@googlemail.com…

[aneesh at ti.com:
1. Changed the definition of OBJTREE and the way 'obj' is defined.
2. Defined a flag to identify regular U-Boot build
3. Taking care of autoconf.mk etc for SPL due to changed OBJTREE ]

[daniel.schwierzeck at googlemail.com:
1. reverted changes on implicite make rules
2. readded definition of -DCONFIG_UBOOT_SPL_BUILD in CPPFLAGS ]

...

config.mk ifneq ($(CONFIG_SYS_TEXT_BASE),) · line 181

Quoted 5 lines

config.mk +5 -0

+# Allow compiling of C and ASM code parts in different way.
+# Serves also as a replacement for CONFIG_PRELOADER.
+ifneq ($(CONFIG_UBOOT_SPL_BUILD),)
+CPPFLAGS += -DCONFIG_UBOOT_SPL_BUILD
+endif

I don't understand this comment.

Quoted 8 lines

config.mk ifneq ($(CONFIG_SYS_TEXT_BASE),) · line 181

Quoted 6 lines

config.mk +5 -0

+# Allow compiling of C and ASM code parts in different way.
+# Serves also as a replacement for CONFIG_PRELOADER.
+ifneq ($(CONFIG_UBOOT_SPL_BUILD),)
+CPPFLAGS += -DCONFIG_UBOOT_SPL_BUILD
+endif

I don't understand this comment.

Currently we have CONFIG_PRELOADER (and maybe others?) to conditionally compile start.S and other code. The idea here is to consolidate those defines and to use a common name convention for SPL. My suggestion is something like CONFIG_UBOOT_SPL_BUILD. Are there better ideas?

Dear Daniel Schwierzeck,

Quoted 9 lines · In message<CACUy__VhV1_eXGW1NuSgLEB1nyVJ51xUXa0vwbY3Tdb=Kvmp2w@mail.gmail.com>…

Quoted 8 lines

config.mk ifneq ($(CONFIG_SYS_TEXT_BASE),) · line 181

Quoted 6 lines

config.mk +5 -0

+# Allow compiling of C and ASM code parts in different way.
+# Serves also as a replacement for CONFIG_PRELOADER.
+ifneq ($(CONFIG_UBOOT_SPL_BUILD),)
+CPPFLAGS += -DCONFIG_UBOOT_SPL_BUILD
+endif

I don't understand this comment.

Quoted 4 lines

Currently we have CONFIG_PRELOADER (and maybe others?) to conditionally compile start.S and other code. The idea here is to consolidate those defines and to use a common name convention for SPL. My suggestion is something like CONFIG_UBOOT_SPL_BUILD. Are there better ideas?

No, I agree with you. I just don't understand the comment above, i. e. in which way does this "allow compiling of C and ASM code parts in
different way"? CPPFLAGS applies the same to .c and .S files, and I
don't see a how this setting would cause different options to be used for C versus assembler code.

Do you confirm on replacing all instances of CONFIG_PRELOADER with CONFIG_UBOOT_SPL_BUILD?

Dear Aneesh V,

Quoted 3 lines · In message<[email protected]> you wrote:

Do you confirm on replacing all instances of CONFIG_PRELOADER with CONFIG_UBOOT_SPL_BUILD?

Well, both are just names. CONFIG_PRELOADER is shorter, but we are moving more and more to the "spl" names, so maybe we can agree on

CONFIG_SPL_BUILD

[daniel.schwierzeck at googlemail.com:
1. changed all appendings of ALL target in several config.mk files as Mike Frysinger suggested
]

Dear Daniel Schwierzeck,

Quoted 4 lines · In message<1309883182-12854-3-git-send-email-daniel.schwierzeck@googlemail.com>…

[daniel.schwierzeck at googlemail.com:
1. changed all appendings of ALL target in several config.mk files as Mike Frysinger suggested
]

Please omit this '[...]' stuff for the real commits, then.

Quoted 3 lines

Please omit this '[...]' stuff for the real commits, then.

In patches 1 and 3 of this series I had done some modifications on top of Daniel's original work. So, I mentioned my changes in [...]. Hope that is fine?

Dear Daniel Schwierzeck,

In message<1309883182-12854-4-git-send-email-daniel.schwierzeck@googlemail.com> you wrote:
...

spl/Makefile line 19

Quoted 5 lines

spl/Makefile new file +5 -0

+# create 'spl/obj' within OBJTREE for spl
+OBJTREE := $(OBJTREE)/spl/obj
+LNDIR	:= $(OBJTREE)
+
+include $(TOPDIR)/config.mk

spl/Makefile line 21

Quoted 2 lines

spl/Makefile new file +2 -0

+# We want the final binaries in this directory
+obj	:= $(obj)../

Quoted 1 line

+START = $(OBJTREE)/$(CPUDIR)/start.o

I don't really like these "../" parts. Can we please try to omit these? We can use $(TOPDIR) and/or $(OBJTREE) instead to express the path names, can't we?

Quoted 3 lines

spl/Makefile line 16

Quoted 3 lines

spl/Makefile new file +2 -0

+# create 'spl/obj' within OBJTREE for spl
+OBJTREE := $(OBJTREE)/spl/obj

Quoted 2 lines

spl/Makefile line 21

Quoted 2 lines

spl/Makefile new file +2 -0

+# We want the final binaries in this directory
+obj	:= $(obj)../

First of all, this was really intended to be:

+obj := $(obj)../../

Quoted 3 lines

I don't really like these "../" parts. Can we please try to omit these? We can use $(TOPDIR) and/or $(OBJTREE) instead to express the path names, can't we?

Please note that even if we use $(OBJTREE), to achieve the above, we will need:

+obj := $(OBJTREE)/../

Please note that $(OBJTREE) needs to be updated before including config.mk while $(obj) needs to be updated after it.

I used BUILD_DIR/spl/obj as OBJTREE to avoid this problem.

Dear Aneesh V,

Quoted 3 lines · In message<[email protected]> you wrote:

Quoted 3 lines

spl/Makefile line 16

Quoted 3 lines

spl/Makefile new file +2 -0

+# create 'spl/obj' within OBJTREE for spl
+OBJTREE := $(OBJTREE)/spl/obj

This is the part I dislike: we redefine variables and lose track of their original values.

Quoted 5 lines

Quoted 2 lines

spl/Makefile line 21

Quoted 2 lines

spl/Makefile new file +2 -0

+# We want the final binaries in this directory
+obj	:= $(obj)../

First of all, this was really intended to be:

+obj := $(obj)../../

Now we have already two levels oif such redefinitions, one going down and the other going up.

This is, well, confusing.

I think, we should remember the original value of $(OBJTREE), so we can use it here where we need it.

Quoted 7 lines

Quoted 3 lines

I don't really like these "../" parts. Can we please try to omit these? We can use $(TOPDIR) and/or $(OBJTREE) instead to express the path names, can't we?

Please note that even if we use $(OBJTREE), to achieve the above, we will need:

Quoted 4 lines

+obj := $(OBJTREE)/../

Please note that $(OBJTREE) needs to be updated before including config.mk while $(obj) needs to be updated after it.

Well, we can of course:

- use a different name instead of OBJTREE everywhere, and leave

OBJTREE as reference to the origonal value for global use, or
- introduce a new variable that stores the original setting of OBJTREE so we can refer to this later

Quoted 1 line

I used BUILD_DIR/spl/obj as OBJTREE to avoid this problem.

I understand what you are doing, and I agree with it. I just dislike the tiny implementation detaild of using relative chnages up and down in the directory structure. I would rather be able to use absolute directory names (which are way easier to verify for debugging, etc.).

Quoted 2 lines

- use a different name instead of OBJTREE everywhere, and leave

What Daniel suggested about using something like SPLTREE(or SPLOBJTREE) seems to be be right solution then. Are you ok with that?

Quoted 4 lines

- introduce a new variable that stores the original setting of OBJTREE so we can refer to this later

Quoted 1 line

I used BUILD_DIR/spl/obj as OBJTREE to avoid this problem.

You want to continue to maintain nand_boot.c kind of files within $(TOPDIR)/spl/ , right?

Dear Aneesh V,

Quoted 5 lines · In message<[email protected]> you wrote:

Quoted 2 lines

- use a different name instead of OBJTREE everywhere, and leave

What Daniel suggested about using something like SPLTREE(or SPLOBJTREE) seems to be be right solution then. Are you ok with that?

Yes, if it works without too much effort.

Quoted 4 lines

Quoted 4 lines

- introduce a new variable that stores the original setting of OBJTREE so we can refer to this later

Quoted 1 line

I used BUILD_DIR/spl/obj as OBJTREE to avoid this problem.

Well, this may cause nasty problems. Note that BUILD_DIR is usually not set for local builds; current Makefile looks like this:

OBJTREE         := $(if $(BUILD_DIR),$(BUILD_DIR),$(CURDIR))
SRCTREE         := $(CURDIR)
TOPDIR          := $(SRCTREE)

...
ifneq ($(OBJTREE),$(SRCTREE))
...

Tests like this here will fail if you use BUILD_DIR (which is not set) instead of OBJTREE (which is set to CURDIR).

Quoted 3 lines

You want to continue to maintain nand_boot.c kind of files within $(TOPDIR)/spl/ , right?

Yes. Copies of, or symlinks to, source files should be avoided.

Symlinks, copies are not involved in either case. My question was this:
Where should something like 'nand_spl/nand_boot.c' go in the new framework?

1) spl/nand/nand_boot.c OR
2) drivers/nand/spl_nand_boot.c

(2) Helps in having:
SPLOBJTREE := (OBJTREE)/spl

where as with (1) we have:
SPLOBJTREE := (OBJTREE)/spl/obj
and with this nand_boot.o will be in
(OBJTREE)/spl/obj/spl/nand/nand_boot.o !

To me (2) looks cleaner, and goes well with the idea of having all source files in the original source tree.

Dear Aneesh V,

Quoted 7 lines · In message<[email protected]> you wrote:

Symlinks, copies are not involved in either case. My question was this:
Where should something like 'nand_spl/nand_boot.c' go in the new framework?

1) spl/nand/nand_boot.c OR
2) drivers/nand/spl_nand_boot.c

I have no strong preferences here...

Quoted 10 lines

(2) Helps in having:
SPLOBJTREE := (OBJTREE)/spl

where as with (1) we have:
SPLOBJTREE := (OBJTREE)/spl/obj
and with this nand_boot.o will be in
(OBJTREE)/spl/obj/spl/nand/nand_boot.o !

To me (2) looks cleaner, and goes well with the idea of having all source files in the original source tree.

Agreed. But maybe we can remove a redundant "nand" from the file name, like this:

drivers/nand/spl_boot.c

?

Dear Daniel Schwierzeck,

In message <1309883182-12854-4-git-send-email-daniel.schwierzeck@googlemail.com> you wrote:
...

spl/Makefile

Quoted 8 lines

spl/Makefile new file +8 -0

+# create 'spl/obj' within OBJTREE for spl
+OBJTREE := $(OBJTREE)/spl/obj
+LNDIR ? ? ? ?:= $(OBJTREE)
+
+include $(TOPDIR)/config.mk
+# We want the final binaries in this directory
+obj ?:= $(obj)../
+START ? ? ? ?= $(OBJTREE)/$(CPUDIR)/start.o

I don't really like these "../" parts. ?Can we please try to omit these? ?We can use $(TOPDIR) and/or $(OBJTREE) instead to express the path names, can't we?

I have to admit that I don't understand what the old and the new values of $(obj) are above...

[The same applies to the "CPPFLAGS += -I$(OBJTREE)/../../include2 -I$(OBJTREE)/../../include" part in the first patch.]

[daniel.schwierzeck at googlemail.com:
1. fixed clean target for spl directory
2. removed image concatenation of normal U-Boot and SPL image ]

Signed-off-by Daniel Schwierzeck <[email protected]>

---
 Makefile |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

Hi Aneesh,

I tested your changes with my boards and it worked well after fixing some minor issues. Additionally I did some cleanups and included feedback from Mike Frysinger.

Best regards,
Daniel

Daniel Schwierzeck (4):
Adapt config.mk for usage in spl/Makefile
Use ALL-y style instead of ifeq blocks for better readability and
upgradeability
Add new folder and build system for SPL
Hook spl directory into main Makefile

  Makefile                       |   23 ++++-----
  arch/blackfin/config.mk        |    2 +-
  board/pcs440ep/config.mk       |    2 +-
  board/spear/spear300/config.mk |    2 +-
  board/spear/spear310/config.mk |    2 +-
  board/spear/spear320/config.mk |    2 +-
  board/spear/spear600/config.mk |    2 +-
  config.mk                      |   28 +++++++++++
  spl/Makefile                   |   97 ++++++++++++++++++++++++++++++++++++++++
  9 files changed, 141 insertions(+), 19 deletions(-)

create mode 100644 spl/Makefile

--
1.7.5.4

-$(obj)%.s: %.S
+$(obj)%.s: $(src)%.S

Quoted 2 lines · On Wednesday, June 29, 2011 09:09:21 Aneesh V wrote:

-$(obj)%.s: %.S
+$(obj)%.s: $(src)%.S

Quoted 5 lines · Mike Frysinger

Quoted 3 lines · On Wednesday, June 29, 2011 09:09:21 Aneesh V wrote:

-$(obj)%.s: ? ? %.S
+$(obj)%.s: ? ? $(src)%.S

I too didn't understand why that was done. But I noticed it just before sending the patches so decided to give it a try later. I tried removing them now and it doesn't harm. Maybe, some of the changes I did to the definition of OBJTREE obj etc would have rendered it irrelevant?

Daniel,
Can you throw some light on this?

 Makefile |   18 +++++-------------
 1 files changed, 5 insertions(+), 13 deletions(-)

# Always append ALL so that arch config.mk's can add custom ones

Quoted 4 lines · On Wednesday, June 29, 2011 09:09:22 Aneesh V wrote:

  Makefile |   18 +++++-------------
  1 files changed, 5 insertions(+), 13 deletions(-)

# Always append ALL so that arch config.mk's can add custom ones

as this comment indicates, you forgot to up date arch config.mk's that append ALL. they'll need converting to ALL-y too.
-mike

Signed-off-by Aneesh V <[email protected]>

---
 arch/arm/cpu/armv7/omap-common/Makefile |    9 ++--
 arch/arm/cpu/armv7/omap-common/spl.c    |   56 ++++++++++++++++++++++++++++
 arch/arm/cpu/armv7/omap-common/spl.lds  |   62 +++++++++++++++++++++++++++++++
 3 files changed, 123 insertions(+), 4 deletions(-)
 create mode 100644 arch/arm/cpu/armv7/omap-common/spl.c
 create mode 100644 arch/arm/cpu/armv7/omap-common/spl.lds

arch/arm/cpu/armv7/omap-common/spl.c new file +0 -0

new file mode 100644
index 0000000..b5a5f3c

Hello Aneesh,

Quoted 9 lines · Aneesh V

Signed-off-by Aneesh V<[email protected]>

---
  arch/arm/cpu/armv7/omap-common/Makefile |    9 ++--
  arch/arm/cpu/armv7/omap-common/spl.c    |   56 ++++++++++++++++++++++++++++
  arch/arm/cpu/armv7/omap-common/spl.lds  |   62 +++++++++++++++++++++++++++++++
  3 files changed, 123 insertions(+), 4 deletions(-)

create mode 100644 arch/arm/cpu/armv7/omap-common/spl.c create mode 100644 arch/arm/cpu/armv7/omap-common/spl.lds

[...]

Quoted 5 lines

arch/arm/cpu/armv7/omap-common/spl.c new file +0 -0

new file mode 100644
index 0000000..b5a5f3c

[...]

arch/arm/cpu/armv7/omap-common/spl.c line 5

Quoted 6 lines

arch/arm/cpu/armv7/omap-common/spl.c new file +5 -0

@@ -0,0 +1,56 @@
+void board_init_f(ulong dummy)
+{
+	debug(">>board_init_f()\n");
+	relocate_code(CONFIG_SYS_SPL_STACK,&gdata, CONFIG_SYS_SPL_TEXT_BASE);
+	debug("<<board_init_f()\n");

This debug printf will never occur ;-)

BTW: Do you really need to relocate code? You could just load the 2nd stage loader to ram from board_init_f, or?

Hi Heiko,

Quoted 19 lines · Heiko Schocher

Hello Aneesh,

Quoted 9 lines · Aneesh V

Signed-off-by Aneesh V<[email protected]>

---
 arch/arm/cpu/armv7/omap-common/Makefile |    9 ++--
 arch/arm/cpu/armv7/omap-common/spl.c    |   56 ++++++++++++++++++++++++++++
 arch/arm/cpu/armv7/omap-common/spl.lds  |   62 +++++++++++++++++++++++++++++++
 3 files changed, 123 insertions(+), 4 deletions(-)
 create mode 100644 arch/arm/cpu/armv7/omap-common/spl.c
 create mode 100644 arch/arm/cpu/armv7/omap-common/spl.lds

[...]

Quoted 5 lines

arch/arm/cpu/armv7/omap-common/spl.c new file +0 -0

new file mode 100644
index 0000000..b5a5f3c

[...]

Quoted 8 lines

arch/arm/cpu/armv7/omap-common/spl.c line 5

Quoted 6 lines

arch/arm/cpu/armv7/omap-common/spl.c new file +5 -0

@@ -0,0 +1,56 @@
+void board_init_f(ulong dummy)
+{
+	debug(">>board_init_f()\n");
+	relocate_code(CONFIG_SYS_SPL_STACK,&gdata, CONFIG_SYS_SPL_TEXT_BASE);
+	debug("<<board_init_f()\n");

This debug printf will never occur ;-)

Indeed. Thanks for pointing out.

Quoted 1 line

Quoted 2 lines

BTW: Do you really need to relocate code? You could just load the 2nd stage loader to ram from board_init_f, or?

I am passing the same address as I am executing from as the target for the relocation, so the relocation will not happen, instead BSS will be initialized. That's what I am calling it for. Initially I had my own routine for clearing BSS. Then I decided to re-use it from start.S

Dear Aneesh V,

Am 30.06.2011 um 08:12 schrieb Aneesh V:

Quoted 37 lines

Hi Heiko,

Quoted 19 lines · Heiko Schocher

Hello Aneesh,

Quoted 9 lines · Aneesh V

Signed-off-by Aneesh V<[email protected]>

---
  arch/arm/cpu/armv7/omap-common/Makefile |    9 ++--
  arch/arm/cpu/armv7/omap-common/spl.c    |   56 ++++++++++++++++++++++++++++
  arch/arm/cpu/armv7/omap-common/spl.lds  |   62 +++++++++++++++++++++++++++++++
  3 files changed, 123 insertions(+), 4 deletions(-)

create mode 100644 arch/arm/cpu/armv7/omap-common/spl.c create mode 100644 arch/arm/cpu/armv7/omap-common/spl.lds

[...]

Quoted 5 lines

arch/arm/cpu/armv7/omap-common/spl.c new file +0 -0

new file mode 100644
index 0000000..b5a5f3c

[...]

Quoted 8 lines

arch/arm/cpu/armv7/omap-common/spl.c line 5

Quoted 6 lines

arch/arm/cpu/armv7/omap-common/spl.c new file +5 -0

@@ -0,0 +1,56 @@
+void board_init_f(ulong dummy)
+{
+	debug(">>board_init_f()\n");
+	relocate_code(CONFIG_SYS_SPL_STACK,&gdata, CONFIG_SYS_SPL_TEXT_BASE);
+	debug("<<board_init_f()\n");

This debug printf will never occur ;-)

Indeed. Thanks for pointing out.

Quoted 3 lines

BTW: Do you really need to relocate code? You could just load the 2nd stage loader to ram from board_init_f, or?

Quoted 4 lines

I am passing the same address as I am executing from as the target for the relocation, so the relocation will not happen, instead BSS will be initialized. That's what I am calling it for. Initially I had my own routine for clearing BSS. Then I decided to re-use it from start.S

So you could just call clear_bss(void) and skip relocate_code. But I think you need to adopt the __bss_start_ofs, __bss_end_ofs markers, cause your linker skript places them in SDRAM.

BTW: I think Simon Schwarz is also working on this, can one comment on his first version of patchset?

Dear Andreas,

Quoted 11 lines · Andreas Bie?mann

Dear Aneesh V,

Am 30.06.2011 um 08:12 schrieb Aneesh V:

Quoted 7 lines

Hi Heiko,

Quoted 4 lines · Heiko Schocher

Hello Aneesh,

Quoted 1 line · Aneesh V

Signed-off-by Aneesh V<[email protected]>

Quoted 14 lines

Quoted 14 lines

Quoted 14 lines

Quoted 6 lines

arch/arm/cpu/armv7/omap-common/spl.c new file +0 -0

new file mode 100644
index 0000000..b5a5f3c

[...]

Quoted 7 lines

arch/arm/cpu/armv7/omap-common/spl.c new file +4 -0

@@ -0,0 +1,56 @@
+void board_init_f(ulong dummy)
+{
+    debug(">>board_init_f()\n");
+    relocate_code(CONFIG_SYS_SPL_STACK,&gdata,

CONFIG_SYS_SPL_TEXT_BASE);
+ debug("<<board_init_f()\n");

Quoted 8 lines

Quoted 7 lines

Quoted 2 lines

BTW: Do you really need to relocate code? You could just load the 2nd stage loader to ram from board_init_f, or?

I am passing the same address as I am executing from as the target for the relocation, so the relocation will not happen, instead BSS will be initialized. That's what I am calling it for. Initially I had my own routine for clearing BSS. Then I decided to re-use it from start.S

Quoted 3 lines

So you could just call clear_bss(void) and skip relocate_code. But I think you need to adopt the __bss_start_ofs, __bss_end_ofs markers, cause your linker skript places them in SDRAM.

Is that really needed, or is it ok to just comment this fact clearly as Heiko suggested?

Quoted 2 lines

BTW: I think Simon Schwarz is also working on this, can one comment on his first version of patchset?

We have decided to co-ordinate our work so that there won't be any duplication of efforts. As per this plan, these parts will be taken care in my OMAP4 MMC spl series and then he will extend it for OMAP3 and NAND.

Dear Aneesh,

But the second part is not clear to me. I saw in your linker, that bss is placed in SDRAM. In start.S the boundaries for clear_bss are calculated at compile time to

---8<---
_bss_start_ofs:
.word __bss_start - _start
--->8---

Will that also work with e.g. SDRAM adress space is before SRAM, SDRAM addressing is far away (> 4GiB) ... So in you special case it may work, but if this is a blueprint for SPL on arm(v7) we should consider this.

Dear Andreas,

Quoted 1 line · Andreas Bie?mann

Dear Aneesh,

[snip ..]

Quoted 4 lines

But the second part is not clear to me. I saw in your linker, that bss is placed in SDRAM. In start.S the boundaries for clear_bss are calculated at compile time to

Quoted 9 lines

---8<---
_bss_start_ofs:
.word __bss_start - _start
--->8---

Will that also work with e.g. SDRAM adress space is before SRAM, SDRAM
addressing is far away (> 4GiB) ... So in you special case it may work,
but if this is a blueprint for SPL on arm(v7) we should consider this.

Nice catch. Actually, in my original OMAP4 series I tried to add support for disjoint bss to support my case. But now I realize that it works only for non-relocation case and that too only when the bss is at higher address compared to .text

Basically disjoint bss is not relocation friendly. So here is what I propose:

1. Modify existing clear_bss sub-routine in start.S to take absolute addresses.
2. In regular u-boot, calculate the relocated bss address and pass to this function.
3. In SPL don't try to calculate the relocated address and directly pass the absolute address.

If this is fine I will make the necessary changes in start.S in the next revision.

best regards,
Aneesh

Hi Aneesh,

Le 01/07/2011 13:48, Aneesh V a ?crit :

Quoted 34 lines

Dear Andreas,

Quoted 1 line · Andreas Bie?mann

Dear Aneesh,

[snip ..]

Quoted 4 lines

But the second part is not clear to me. I saw in your linker, that bss is placed in SDRAM. In start.S the boundaries for clear_bss are calculated at compile time to

Quoted 9 lines

---8<---
_bss_start_ofs:
.word __bss_start - _start
--->8---

Will that also work with e.g. SDRAM adress space is before SRAM, SDRAM addressing is far away (> 4GiB) ... So in you special case it may work, but if this is a blueprint for SPL on arm(v7) we should consider this.

Nice catch. Actually, in my original OMAP4 series I tried to add support for disjoint bss to support my case. But now I realize that it works only for non-relocation case and that too only when the bss is at higher address compared to .text

Basically disjoint bss is not relocation friendly. So here is what I propose:

1. Modify existing clear_bss sub-routine in start.S to take absolute addresses.
2. In regular u-boot, calculate the relocated bss address and pass to this function.
3. In SPL don't try to calculate the relocated address and directly pass the absolute address.

Quoted 2 lines

If this is fine I will make the necessary changes in start.S in the next revision.

So you would compute the BSS location in board_init_f() and pass that to relocate_code()?

Hi Albert,

Quoted 37 lines · Albert ARIBAUD

Hi Aneesh,

Le 01/07/2011 13:48, Aneesh V a ?crit :

Quoted 34 lines

Dear Andreas,

Quoted 1 line · Andreas Bie?mann

Dear Aneesh,

[snip ..]

Quoted 4 lines

But the second part is not clear to me. I saw in your linker, that bss is placed in SDRAM. In start.S the boundaries for clear_bss are calculated at compile time to

Quoted 9 lines

---8<---
_bss_start_ofs:
.word __bss_start - _start
--->8---

Will that also work with e.g. SDRAM adress space is before SRAM, SDRAM addressing is far away (> 4GiB) ... So in you special case it may work, but if this is a blueprint for SPL on arm(v7) we should consider this.

Nice catch. Actually, in my original OMAP4 series I tried to add support for disjoint bss to support my case. But now I realize that it works only for non-relocation case and that too only when the bss is at higher address compared to .text

Basically disjoint bss is not relocation friendly. So here is what I propose:

1. Modify existing clear_bss sub-routine in start.S to take absolute addresses.
2. In regular u-boot, calculate the relocated bss address and pass to this function.
3. In SPL don't try to calculate the relocated address and directly pass the absolute address.

Quoted 5 lines

Quoted 2 lines

If this is fine I will make the necessary changes in start.S in the next revision.

So you would compute the BSS location in board_init_f() and pass that to relocate_code()?

I was thinking of doing that in start.S itself. I haven't looked at all the details though.

BTW, please note that I am not trying to support disjoint BSS in regular u-boot. I think it becomes complex with relocation + it doesn't seem to be worth when all SDRAM is at our disposal.

So:
1. #ifdef CONFIG_PRELOADER part in start.s will just pass __bss_start and __bss_end to the clear_bss function(assumes no relocation).
2. #else part of above will assume that bss follows text and data(or at least that __bss_start > _start), so add relocation offset to __bss_start and __bss_end, and pass them to the clear_bss()

Does that sound ok?

br,
Aneesh

Hi Aneesh,

Le 03/07/2011 06:47, Aneesh V a ?crit :

Quoted 5 lines

Hi Albert,

Quoted 2 lines · Albert ARIBAUD

Hi Aneesh,

Quoted 14 lines

I was thinking of doing that in start.S itself. I haven't looked at all the details though.

BTW, please note that I am not trying to support disjoint BSS in regular u-boot. I think it becomes complex with relocation + it doesn't seem to be worth when all SDRAM is at our disposal.

So:
1. #ifdef CONFIG_PRELOADER part in start.s will just pass __bss_start and __bss_end to the clear_bss function(assumes no relocation).
2. #else part of above will assume that bss follows text and data(or at least that __bss_start > _start), so add relocation offset to __bss_start and __bss_end, and pass them to the clear_bss()

Quoted 1 line

Does that sound ok?

(note clear_bss is not a function in arch/arm/cpu/armv7/start.S, it is only a label)

So, considering what is already in arch/arm/cpu/armv7/start.S, you would just add a conditional variant to the BSS clearing code for the preloader case?

If so, and considering that you'll pass an offset of 0, why would you need that variant for? With offset=0, the code already does what you want, does it not?

Dear Albert Aribaud,

Am 03.07.2011 um 08:56 schrieb Albert ARIBAUD:

Quoted 8 lines

Hi Aneesh,

Le 03/07/2011 06:47, Aneesh V a ?crit :

Quoted 5 lines

Hi Albert,

Quoted 2 lines · Albert ARIBAUD

Hi Aneesh,

<snip>

Quoted 14 lines

Quoted 14 lines

I was thinking of doing that in start.S itself. I haven't looked at all the details though.

BTW, please note that I am not trying to support disjoint BSS in regular u-boot. I think it becomes complex with relocation + it doesn't seem to be worth when all SDRAM is at our disposal.

So:
1. #ifdef CONFIG_PRELOADER part in start.s will just pass __bss_start and __bss_end to the clear_bss function(assumes no relocation).
2. #else part of above will assume that bss follows text and data(or at least that __bss_start> _start), so add relocation offset to __bss_start and __bss_end, and pass them to the clear_bss()

Quoted 7 lines

Quoted 1 line

Does that sound ok?

(note clear_bss is not a function in arch/arm/cpu/armv7/start.S, it is only a label)

So, considering what is already in arch/arm/cpu/armv7/start.S, you would just add a conditional variant to the BSS clearing code for the preloader case?

If so, and considering that you'll pass an offset of 0, why would you need that variant for? With offset=0, the code already does what you want, does it not?

we are looking forward to have relocate_code(), clear_bss(), a.s.o. implemented in c in future (as I understood the discussion around christmas about relocation). Wouldn't it be nice to implement now a SPL version of clear_bss(int start, int size) in c and skip the relocate_code() stuff in start.S completely?

As I see the normal way to boot is:

-> some start vector
-> lowlevel_init
-> board_init_f

    ->  relocate_code
     ->  board_init_r

for SPL we do not need the steps after board_init_f. board_init_f for SPL should implement the generic way to load data from predefined (compile time) source to predefined target address and branch it. Therefore a simple memset() in board_init_f could be sufficient to clear the bss.

regards

Andreas Bie?mann

Hi Aneesh,

Le 03/07/2011 06:47, Aneesh V a ?crit :

Quoted 54 lines

Hi Albert,

Quoted 38 lines · Albert ARIBAUD

Hi Aneesh,

Le 01/07/2011 13:48, Aneesh V a ?crit :

Quoted 35 lines

Dear Andreas,

Quoted 1 line · Andreas Bie?mann

Dear Aneesh,

[snip ..]

Quoted 9 lines

But the second part is not clear to me. I saw in your linker, that bss is placed in SDRAM. In start.S the boundaries for clear_bss are calculated at compile time to

---8<---
_bss_start_ofs:
.word __bss_start - _start
--->8---

Quoted 5 lines

Will that also work with e.g. SDRAM adress space is before SRAM, SDRAM addressing is far away (> 4GiB) ... So in you special case it may work,
but if this is a blueprint for SPL on arm(v7) we should consider this.

Nice catch. Actually, in my original OMAP4 series I tried to add support for disjoint bss to support my case. But now I realize that it works only for non-relocation case and that too only when the bss is at higher address compared to .text

Basically disjoint bss is not relocation friendly. So here is what I propose:

1. Modify existing clear_bss sub-routine in start.S to take absolute addresses.
2. In regular u-boot, calculate the relocated bss address and pass to this function.
3. In SPL don't try to calculate the relocated address and directly pass the absolute address.

Quoted 5 lines

Quoted 2 lines

If this is fine I will make the necessary changes in start.S in the next revision.

So you would compute the BSS location in board_init_f() and pass that to relocate_code()?

I was thinking of doing that in start.S itself. I haven't looked at all the details though.

BTW, please note that I am not trying to support disjoint BSS in regular u-boot. I think it becomes complex with relocation + it doesn't seem to be worth when all SDRAM is at our disposal.

Quoted 8 lines

So:
1. #ifdef CONFIG_PRELOADER part in start.s will just pass __bss_start and __bss_end to the clear_bss function(assumes no relocation).
2. #else part of above will assume that bss follows text and data(or at least that __bss_start > _start), so add relocation offset to __bss_start and __bss_end, and pass them to the clear_bss()

Does that sound ok?

(note clear_bss is not a function in arch/arm/cpu/armv7/start.S, it is only a label)

So, considering what is already in arch/arm/cpu/armv7/start.S, you would just add a conditional variant to the BSS clearing code for the preloader case?

If so, and considering that you'll pass an offset of 0, why would you need that variant for? With offset=0, the code already does what you want, does it not?

Quoted 22 lines · Heiko Schocher

Hello Aneesh,

Quoted 11 lines · Aneesh V

Signed-off-by Aneesh V<[email protected]>

---
  arch/arm/cpu/armv7/omap-common/Makefile |    9 ++--
  arch/arm/cpu/armv7/omap-common/spl.c    |   56
++++++++++++++++++++++++++++
  arch/arm/cpu/armv7/omap-common/spl.lds  |   62
+++++++++++++++++++++++++++++++
  3 files changed, 123 insertions(+), 4 deletions(-)

create mode 100644 arch/arm/cpu/armv7/omap-common/spl.c create mode 100644 arch/arm/cpu/armv7/omap-common/spl.lds

[...]

Quoted 6 lines

arch/arm/cpu/armv7/omap-common/spl.c new file +0 -0

new file mode 100644
index 0000000..b5a5f3c

[...]

Quoted 3 lines

BTW: Do you really need to relocate code? You could just load the 2nd stage loader to ram from board_init_f, or?

I am passing the same address as I am executing from as the target for the relocation, so the relocation will not happen, instead BSS will be initialized. That's what I am calling it for. Initially I had my own routine for clearing BSS. Then I decided to re-use it from start.S

BTW, this series is not complete as far as SPL support is concerned. It doesn't even build due to many undefined references. It serves only as a prototype for the framework part.

Hmm, I actually porting a dm368 based board with nand_spl support (patches following soon), and there I have an empty bss section, so I can direct copy the u-boot image from nand to ram in board_init_f().

Hi,

Quoted 3 lines

Hmm, I actually porting a dm368 based board with nand_spl support (patches following soon), and there I have an empty bss section, so I can direct copy the u-boot image from nand to ram in board_init_f().

This is what I'am doing with OMAP3 also. IMHO it would be more readable if relocate_code is not called if there is no relocation and instead the clear_bss()-function is used as Andreas suggested. (Or to just not use clear_bss if bss is empty)

Hi,

Quoted 3 lines

Hmm, I actually porting a dm368 based board with nand_spl support (patches following soon), and there I have an empty bss section, so I can direct copy the u-boot image from nand to ram in board_init_f().

This is what I'am doing with OMAP3 also. IMHO it would be more readable if relocate_code is not called if there is no relocation and instead the clear_bss()-function is used as Andreas suggested. (Or to just not use clear_bss if bss is empty)

Regards
Simon

Hi Simon,

Le 30/06/2011 10:21, Simon Schwarz a ?crit :

Quoted 6 lines

Hi,

Quoted 3 lines

Hmm, I actually porting a dm368 based board with nand_spl support (patches following soon), and there I have an empty bss section, so I can direct copy the u-boot image from nand to ram in board_init_f().

Quoted 4 lines

This is what I'am doing with OMAP3 also. IMHO it would be more readable if relocate_code is not called if there is no relocation and instead the clear_bss()-function is used as Andreas suggested. (Or to just not use clear_bss if bss is empty)

IMO, for the sake of rpbustness, the clear_bss code should handle the case where the BSS is empty, and for the sake of simplicity, it should be called always.

(quoted diff) +0 -1

@@ -1158,6 +1164,7 @@ clobber:	clean
  	@[ ! -d $(obj)nand_spl ] || find $(obj)nand_spl -name "*" -type l 
-print | xargs rm -f
  	@[ ! -d $(obj)onenand_ipl ] || find $(obj)onenand_ipl -name "*" -type 

l -print | xargs rm -f
@[ ! -d $(obj)mmc_spl ] || find $(obj)mmc_spl -name "*" -type l

(quoted diff) +1 -1

-print | xargs rm -f
+	@[ ! -d $(obj)spl ] || find $(obj)mmc_spl -name "*" -type l -print | 

xargs rm -f

Quoted 9 lines · On Fri, 17 Jun 2011 22:18:57 +0530 Aneesh V <[email protected]> wrote:

(quoted diff) +0 -0

@@ -1158,6 +1164,7 @@ clobber: ? ? ? ?clean

? ? ? @[ ! -d $(obj)nand_spl ] || find $(obj)nand_spl -name "*" -type l -print | xargs rm -f
? ? ? @[ ! -d $(obj)onenand_ipl ] || find $(obj)onenand_ipl -name "*" -type l -print | xargs rm -f
? ? ? @[ ! -d $(obj)mmc_spl ] || find $(obj)mmc_spl -name "*" -type l -print | xargs rm -f
+ ? ? @[ ! -d $(obj)spl ] || find $(obj)mmc_spl -name "*" -type l -print | xargs rm -f

That last mmc_spl should just be spl.

Review

Quoted 2 lines

(quoted diff) +2 -0

+LIBS-$(CONFIG_SYS_SPL_NAND_SUPPORT) += mmc/libnand.o
+LIBS-$(CONFIG_SYS_SPL_ONENAND_SUPPORT) += mmc/libonenand.o

Why are these in mmc?

What is it you propose would be in these files?

Quoted 2 lines

+$(LIBS-y): ? depend
+ ? ? ? ? ? ? $(MAKE) -C $(dir $(subst $(obj),,$@)) all

If no libraries are selected, this will produce a rule with an empty target, which is undefined behavior.

-Scott

Quoted 15 lines · Scott Wood

Quoted 9 lines · On Fri, 17 Jun 2011 22:18:57 +0530 Aneesh V <[email protected]> wrote:

(quoted diff) +0 -0

@@ -1158,6 +1164,7 @@ clobber: ? ? ? ?clean

? ? ? @[ ! -d $(obj)nand_spl ] || find $(obj)nand_spl -name "*" -type l -print | xargs rm -f
? ? ? @[ ! -d $(obj)onenand_ipl ] || find $(obj)onenand_ipl -name "*" -type l -print | xargs rm -f
? ? ? @[ ! -d $(obj)mmc_spl ] || find $(obj)mmc_spl -name "*" -type l -print | xargs rm -f
+ ? ? @[ ! -d $(obj)spl ] || find $(obj)mmc_spl -name "*" -type l -print | xargs rm -f

That last mmc_spl should just be spl.

Quoted 2 lines

Review

Quoted 2 lines

(quoted diff) +2 -0

+LIBS-$(CONFIG_SYS_SPL_NAND_SUPPORT) += mmc/libnand.o
+LIBS-$(CONFIG_SYS_SPL_ONENAND_SUPPORT) += mmc/libonenand.o

Oops!! That was a copy paste error. It was intended to be:

The top-level Makefile doesn't include any source files by itself. All SPL content comes from one or more of libraries. So, there will be at least one library defined, I believe(unless there is an error, of course).

Quoted 16 lines · Aneesh

Quoted 15 lines · Scott Wood

Quoted 9 lines · On Fri, 17 Jun 2011 22:18:57 +0530 Aneesh V<[email protected]> wrote:

(quoted diff) +0 -1

@@ -1158,6 +1164,7 @@ clobber:        clean
       @[ ! -d $(obj)nand_spl ] || find $(obj)nand_spl -name "*" -type l
-print | xargs rm -f
       @[ ! -d $(obj)onenand_ipl ] || find $(obj)onenand_ipl -name "*" -type

l -print | xargs rm -f

       @[ ! -d $(obj)mmc_spl ] || find $(obj)mmc_spl -name "*" -type l

(quoted diff) +1 -1

-print | xargs rm -f
+     @[ ! -d $(obj)spl ] || find $(obj)mmc_spl -name "*" -type l -print |

xargs rm -f

That last mmc_spl should just be spl.

Review

Quoted 6 lines

Quoted 2 lines

Review

Quoted 2 lines

(quoted diff) +2 -0

+LIBS-$(CONFIG_SYS_SPL_NAND_SUPPORT) += mmc/libnand.o
+LIBS-$(CONFIG_SYS_SPL_ONENAND_SUPPORT) += mmc/libonenand.o

Oops!! That was a copy paste error. It was intended to be:

(quoted diff) +2 -0

+LIBS-$(CONFIG_SYS_SPL_NAND_SUPPORT) += nand/libnand.o
+LIBS-$(CONFIG_SYS_SPL_ONENAND_SUPPORT) += onenand/libonenand.o

Still, what would go in those files?

It'd have to be something more specific, like:

LIBS-$(CONFIG_SYS_SPL_NAND_SIMPLE) += nand/nand_boot.o LIBS-$(CONFIG_SYS_SPL_NAND_FSL_ELBC) += nand/nand_boot_fsl_elbc.o
...

Hmm, I guess you'd stick this in a recursive makefile. Seems like overkill.

Quoted 4 lines

The top-level Makefile doesn't include any source files by itself. All SPL content comes from one or more of libraries. So, there will be at least one library defined, I believe(unless there is an error, of course).

Couldn't there be only OBJS?

Dear Wolfgang,

Here is a crude implementation of the top-down approach you had been suggesting (or my interpretation of it). This is not complete yet and serves only as a material for further discussions on this topic.

Dear Wolfgang,

Quoted 2 lines · Aneesh V

Dear Wolfgang,

Quoted 3 lines

Here is a crude implementation of the top-down approach you had been suggesting (or my interpretation of it). This is not complete yet and serves only as a material for further discussions on this topic.

Here is an updated version of my prototype implementation with fixes for some issues pointed out by Scott. Please let me know your views about this.

Quoted 4 lines

Here is a crude implementation of the top-down approach you had been suggesting (or my interpretation of it). This is not complete yet and serves only as a material for further discussions on this topic.

Here is an updated version of my prototype implementation with fixes for some issues pointed out by Scott. Please let me know your views about this.

---
  Makefile                        |    5 ++
  include/configs/omap4_sdp4430.h |    1 +
  spl/Makefile                    |   94 
+++++++++++++++++++++++++++++++++++++++
  spl/mmc/Makefile                |   55 +++++++++++++++++++++++
  4 files changed, 155 insertions(+), 0 deletions(-)

create mode 100644 spl/Makefile
create mode 100644 spl/mmc/Makefile

Makefile +0 -0

index 8540e39..0321634 100644

Makefile +0 -0

  	@rm -f $(ONENAND_BIN)

  	@rm -f $(obj)onenand_ipl/u-boot.lds

Dear Aneesh V,

Quoted 24 lines · In message<[email protected]> you wrote:

Quoted 4 lines

Here is a crude implementation of the top-down approach you had been suggesting (or my interpretation of it). This is not complete yet and serves only as a material for further discussions on this topic.

Here is an updated version of my prototype implementation with fixes for some issues pointed out by Scott. Please let me know your views about this.

---
   Makefile                        |    5 ++
   include/configs/omap4_sdp4430.h |    1 +
   spl/Makefile                    |   94
+++++++++++++++++++++++++++++++++++++++
   spl/mmc/Makefile                |   55 +++++++++++++++++++++++
   4 files changed, 155 insertions(+), 0 deletions(-)

create mode 100644 spl/Makefile
create mode 100644 spl/mmc/Makefile

Makefile +0 -0

index 8540e39..0321634 100644

Makefile BOARD_SIZE_CHECK = · line 320

Quoted 5 lines

Makefile +1 -0

@@ -316,6 +316,7 @@ BOARD_SIZE_CHECK =
   endif
 
   # Always append ALL so that arch config.mk's can add custom ones
+ALL += spl
   ALL += $(obj)u-boot.srec $(obj)u-boot.bin $(obj)System.map

This patch seems strangely white-space corrupted.

Makefile $(obj)u-boot-onenand.bin: onenand_ipl $(obj)u-boot.bin · line 435

Quoted 9 lines

Makefile +3 -0

   ifeq ($(CONFIG_NAND_U_BOOT),y)

@@ -428,6 +429,9 @@ $(obj)u-boot-onenand.bin:	onenand_ipl $(obj)u-boot.bin
   mmc_spl:	$(TIMESTAMP_FILE) $(VERSION_FILE) depend
   		$(MAKE) -C mmc_spl/board/$(BOARDDIR) all
 
+spl:	$(TIMESTAMP_FILE) $(VERSION_FILE) depend
+	$(MAKE) -C spl/ all
+
   $(obj)mmc_spl/u-boot-mmc-spl.bin:	mmc_spl

The mmc_spl/ is suppoed to be moved into spl/, isn't it?

Makefile clean: · line 1147

Quoted 4 lines

Makefile +0 -0

   $(VERSION_FILE):

@@ -1142,6 +1146,7 @@ clean:
   	@rm -f $(obj)spl/{u-boot-spl-generated.lds,u-boot-spl,u-boot-spl.map}
   	@rm -f $(obj)onenand_ipl/onenand-{ipl,ipl.bin,ipl.map}

Dito here for onenand_ipl/ ?

Makefile clean: · line 1149

Quoted 2 lines

Makefile +0 -0

   	@rm -f
 $(obj)mmc_spl/{u-boot.lds,u-boot-spl,u-boot-spl.map,u-boot-spl.bin,u-boot-mmc-spl.bin}

...also line-wrapped.

Makefile clean: · line 1151

Quoted 2 lines

Makefile +1 -0

+	@rm -f
 $(obj)spl/{u-boot.lds,u-boot-spl,u-boot-spl.map,u-boot-spl.bin,u-boot-mmc-spl.bin}

Make this:

	@rm -f $(obj)spl/u-boot{.lds,-spl,-spl.map,-spl.bin,-mmc-spl.bin}

instead.

Quoted 2 lines

Makefile +0 -0

   	@rm -f $(ONENAND_BIN)

   	@rm -f $(obj)onenand_ipl/u-boot.lds

Goes away?

spl/Makefile line 2

Quoted 5 lines

spl/Makefile new file +2 -0

@@ -0,0 +1,94 @@
+#
+# (C) Copyright 2011 Daniel Schwierzeck, daniel.schwierzeck at googlemail.com.

Really???

spl/Makefile

Quoted 11 lines

spl/Makefile new file +2 -0

+# This file is released under the terms of GPL v2 and any later version.
+# See the file COPYING in the root directory of the source tree for

details.

spl/Makefile new file +7 -0

+#
+
+include $(TOPDIR)/config.mk
+LIBS-$(CONFIG_SYS_SPL_MMC_SUPPORT) = mmc/libmmc.o
+LIBS-$(CONFIG_SYS_SPL_FAT_SUPPORT) += fat/libfat.o
+LIBS-$(CONFIG_SYS_SPL_NAND_SUPPORT) += nand/libnand.o
+LIBS-$(CONFIG_SYS_SPL_ONENAND_SUPPORT) += onenand/libonenand.o

As Mike mentioned, we can eventually directly include the OBJSs here and omit the building of libraries?

spl/Makefile

Quoted 10 lines

spl/Makefile new file +7 -0

+LIBS-y += $(shell if [ -f $(ARCH)/Makefile ]; then echo \
+	"$(ARCH)/lib$(ARCH).o"; fi)
+LIBS-y += $(shell if [ -f $(ARCH)/$(CPU)/Makefile ]; then echo \
+	"$(ARCH)/$(CPU)/lib$(CPU).o"; fi)
+LIBS-y += $(shell if [ -f $(ARCH)/$(CPU)/$(SOC)/Makefile ]; then echo \
+	"$(ARCH)/$(CPU)/$(SOC)/lib$(SOC).o"; fi)
+LIBS-y += $(shell if [ -f $(ARCH)/$(CPU)/$(SOC)/$(BOARD)/Makefile ];

then echo \

spl/Makefile new file +1 -0

+	"$(ARCH)/$(CPU)/$(SOC)/$(BOARD)/lib$(BOARD).o"; fi)

We should probably use /$(BOARDDIR)? here instead of /$(BOARD)/ to allow for vendor directories (where "BOARDDIR = $(VENDOR)/$(BOARD)").

spl/Makefile

Quoted 3 lines

spl/Makefile new file +3 -0

+ALL	= $(obj)u-boot-spl.bin
+
+all:	$(ALL)

Do we need ALL then at all?

...

spl/mmc/Makefile line 3

Quoted 9 lines

spl/mmc/Makefile new file +3 -0

new file mode 100644
index 0000000..b4f7efd
@@ -0,0 +1,55 @@
+#
+# (C) Copyright 2000-2003
+# Wolfgang Denk, DENX Software Engineering, wd at denx.de.

Really???

spl/mmc/Makefile line 34

Quoted 7 lines

spl/mmc/Makefile new file +7 -0

+$(obj)mmc.c:
+	@rm -f $@
+	@ln -s $(TOPDIR)/drivers/mmc/mmc.c $@
+
+$(obj)omap_hsmmc.c:
+	@rm -f $@
+	@ln -s $(TOPDIR)/drivers/mmc/omap_hsmmc.c $@

Hm... can we try to do without the symlinks?