Create generic board init and move ARM and x86 to it

We want to unify the global_data structure. Most fields are common across architectures, but there are a fair number of SOC-specific additions. It isn't clear how best to deal with these, but for now we just use #ifdef.

Dear Simon,

Quoted 3 lines · Simon Glass

We want to unify the global_data structure. Most fields are common across architectures, but there are a fair number of SOC-specific additions. It isn't clear how best to deal with these, but for now we just use #ifdef.

I would like to see some more 'clustering' here. Especially the timer stuff is implementation specific and should therefore not coded directly but with some pointer to the specific stuff i.e. introduce some timer_t which is specific for arm in general and maybe has some more specialization for atmel devices or what else and just save a pointer to that struct in gd_t like it is done with bd_t.

Well maybe having a pointer here is stated critical so we could just insert the struct directly and let the compiler decide about the size (BTW this could be done with bd_t now too cause we have a pre-calculated size of gd_t since some time, saves some little steps when instantiating gd_t when running).

Beside that we could also introduce some more structs holding specific stuff i.e. console_t, reloc_t, ...
(but yes, it should be done in a second step ...)

What do you think about that?

best regards

Andreas Bie?mann

Hi Andreas,

Quoted 7 lines · Andreas Bie?mann

Dear Simon,

Quoted 3 lines · Simon Glass

We want to unify the global_data structure. Most fields are common across architectures, but there are a fair number of SOC-specific additions. It isn't clear how best to deal with these, but for now we just use #ifdef.

Sorry your email got a little buried this week.

Quoted 6 lines

I would like to see some more 'clustering' here. Especially the timer stuff is implementation specific and should therefore not coded directly but with some pointer to the specific stuff i.e. introduce some timer_t which is specific for arm in general and maybe has some more specialization for atmel devices or what else and just save a pointer to that struct in gd_t like it is done with bd_t.

Actually I wonder if we should an architecture-specific structure, something like

struct global_data {
   /* generic things first */
   ulong baud_rate;
   ulong ram_size;
   /* hopefully lots of other things that are mostly common */

   /* architecture-specific things */
   struct arch_global_data arch;
};

struct arch_global_data can be defined in include/asm/global_data.h before it includes include/asm-generic/global_data.h, or the other way around might be better.

One thing I notice is that some architectures share some fields, and others omit them.I feel we should include them at the global level if at least 2 architectures need them. This means that the 'minimal' architectures with hardly any fields will have quite a few unused fields in global_data. I don't think that matters, and the use will

increase as that arch gets more functionality.

Quoted 5 lines

Well maybe having a pointer here is stated critical so we could just insert the struct directly and let the compiler decide about the size (BTW this could be done with bd_t now too cause we have a pre-calculated size of gd_t since some time, saves some little steps when instantiating gd_t when running).

Yes I agree that just including the structure seems better. A pointer is slower and we just point to a place very close anyway.

It would be nice to subsume bd_t into global_data.

Quoted 5 lines

Beside that we could also introduce some more structs holding specific stuff i.e. console_t, reloc_t, ...
(but yes, it should be done in a second step ...)

What do you think about that?

Certainly agree for reloc and perhaps there are other cases too. It is far too long at present. Something to think about.

For now I would prefer to do nothing on either point, since bringing everything into one place shows up the conflicts and similarities. Part of the purpose of the generic board effort is to minimise these, and they are hard to spot if they are all in separate files.

But when we get through this, and find that there are things which genuinely are specific to a single arch, then I think breaking them out is reasonable, hopefully combined with joining global_data and bd_t.

Hi Simon,

Quoted 30 lines · Simon Glass

Hi Andreas,

Quoted 7 lines · Andreas Bie?mann

Dear Simon,

Quoted 3 lines · Simon Glass

We want to unify the global_data structure. Most fields are common across architectures, but there are a fair number of SOC-specific additions. It isn't clear how best to deal with these, but for now we just use #ifdef.

Sorry your email got a little buried this week.

Quoted 6 lines

I would like to see some more 'clustering' here. Especially the timer stuff is implementation specific and should therefore not coded directly but with some pointer to the specific stuff i.e. introduce some timer_t which is specific for arm in general and maybe has some more specialization for atmel devices or what else and just save a pointer to that struct in gd_t like it is done with bd_t.

Actually I wonder if we should an architecture-specific structure, something like

struct global_data {
? ?/* generic things first */
? ?ulong baud_rate;
? ?ulong ram_size;
? ?/* hopefully lots of other things that are mostly common */

Quoted 3 lines

? ?/* architecture-specific things */
? ?struct arch_global_data arch;
};

This makes auto-generated asm offsets a little trickier I think

Quoted 3 lines

struct arch_global_data ?can be defined in include/asm/global_data.h before it includes include/asm-generic/global_data.h, or the other way around might be better.

include/asm-generic/global_data.h would include include/asm/global_data.h

But what of the corner case that an arch has no specific global data?

Quoted 6 lines

One thing I notice is that some architectures share some fields, and others omit them.I feel we should include them at the global level if at least 2 architectures need them. This means that the 'minimal' architectures with hardly any fields will have quite a few unused fields in global_data. I don't think that matters, and the use will

I would not bank on that, and I think you will get strong objection from Wolfgang if you try to litter global data with members that are not strictly necessary

Quoted 9 lines

increase as that arch gets more functionality.

Quoted 5 lines

Well maybe having a pointer here is stated critical so we could just insert the struct directly and let the compiler decide about the size (BTW this could be done with bd_t now too cause we have a pre-calculated size of gd_t since some time, saves some little steps when instantiating gd_t when running).

Quoted 2 lines

Yes I agree that just including the structure seems better. A pointer is slower and we just point to a place very close anyway.

Not so fast - The idea is that gd is writeable pre-relocation and therefore can be instantiated in the very limit pre-SDRAM environment such as the CPU cache. bd is not needed until after SDRAM has been initialised, so it's size constraints aren't so bad.

Quoted 3 lines

It would be nice to subsume bd_t into global_data.

NAK

Quoted 5 lines

Quoted 5 lines

Beside that we could also introduce some more structs holding specific stuff i.e. console_t, reloc_t, ...
(but yes, it should be done in a second step ...)

Quoted 4 lines

Quoted 1 line

What do you think about that?

Certainly agree for reloc and perhaps there are other cases too. It is far too long at present. Something to think about.

I agree that gd could be cleaned up - particularly the types - sub-structuring it is probably a bit over the top though (as I mentioned, how will this impact asm-offsets?

Quoted 5 lines

For now I would prefer to do nothing on either point, since bringing everything into one place shows up the conflicts and similarities. Part of the purpose of the generic board effort is to minimise these, and they are hard to spot if they are all in separate files.

Hmm, I'm starting to wonder if we should instead have:

struct gd_generic {
? ? ? ?/* Relocation stuff */

? ? ? ?/* pre-console stuff */

? ? ? ?/* Jump table stuff */
}

struct gd {
? ? ? ?struct gd_generic common;

? ? ? ?/* Arch specific stuff */
}

This eliminates the 'no arch specific global data' corner case

Quoted 5 lines

But when we get through this, and find that there are things which genuinely are specific to a single arch, then I think breaking them out is reasonable, hopefully combined with joining global_data and bd_t.

Regards,

Graeme

Hi Graeme,

Quoted 10 lines · Graeme Russ

Hi Simon,

Quoted 7 lines · Simon Glass

Hi Andreas,

Quoted 3 lines · Andreas Bie?mann

Dear Simon,

On 28.12.11 07:35, Simon Glass wrote:

Quoted 4 lines

Quoted 4 lines

For now I would prefer to do nothing on either point, since bringing everything into one place shows up the conflicts and similarities. Part of the purpose of the generic board effort is to minimise these, and they are hard to spot if they are all in separate files.

Quoted 10 lines

Hmm, I'm starting to wonder if we should instead have:

struct gd_generic {
       /* Relocation stuff */

       /* pre-console stuff */

       /* Jump table stuff */
}

Quoted 7 lines

struct gd {
       struct gd_generic common;

       /* Arch specific stuff */
}

This eliminates the 'no arch specific global data' corner case

Hi Simon and Graeme,

Quoted 13 lines · Simon Glass

Hi Graeme,

Quoted 10 lines · Graeme Russ

Hi Simon,

Quoted 7 lines · Simon Glass

Hi Andreas,

Quoted 3 lines · Andreas Bie?mann

Dear Simon,

On 28.12.11 07:35, Simon Glass wrote:

<snip discussion about 'how to restructure gd_t' ... should be done later>

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

For now I would prefer to do nothing on either point, since bringing everything into one place shows up the conflicts and similarities. Part of the purpose of the generic board effort is to minimise these, and they are hard to spot if they are all in separate files.

Currently I agree with you both. It would be best to just move all the specific stuff into one file but let the structure as is (from view of SoC/arch). Later on we could shrink it down by one of the mentioned possibilities.

Quoted 14 lines

Quoted 14 lines

Hmm, I'm starting to wonder if we should instead have:

struct gd_generic {
? ? ? ?/* Relocation stuff */

? ? ? ?/* pre-console stuff */

? ? ? ?/* Jump table stuff */
}

struct gd {
? ? ? ?struct gd_generic common;

Quoted 4 lines

Quoted 4 lines

? ? ? ?/* Arch specific stuff */
}

This eliminates the 'no arch specific global data' corner case

Sounds good to me, but if we restructure gd_t in any way we should introduce some ability to version the struct for future changes (keep stand alone apps in mind!).

best regards

Andreas Bie?mann

This series creates a generic board.c implementation which contains the essential functions of the various arch/xxx/lib/board.c files.

Dear Simon,

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

This series creates a generic board.c implementation which contains the essential functions of the various arch/xxx/lib/board.c files.

I'll be loking into this deeper when I'm back at work (i. e. starting with week 2 next year), but I'd like to make this comment as early as possible:

When making such changes, I _strongly_ recommend to use the Power architecture implementation as reference. ?There are a number of reasons for this, which I don't intend to explain in detail here and now (lack of time). ?But PPC is the most complete, longest living and most thoroughly designed architecture. ?Things that other architectures aquire only over time (relocation, cache support, FDT support, multiple busses [I2C, PCI, ...]) have "always" been supported by PPC.

I fear that unifying any architectures and not including PPC rightr from the beginning will result in additional efforts later, so please have a look is this can be changed.

Thanks.

Best regards,

Wolfgang Denk

--
DENX Software Engineering GmbH, ? ? MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de Cigarette, n.: A fire at one end, a fool at the other, and a ?bit ?of tobacco in between.

This series creates a generic board.c implementation which contains the essential functions of the various arch/xxx/lib/board.c files.

What is the motivation for this change?

All the functions of board_init_f() and board_init_r() are broken into separate function calls so that they can easily be included or excluded for a particular architecture. It also makes it easier to adopt Graeme's initcall proposal later if desired.

Generic relocation is used (see previous series) but now rather than calling relocate_code() to do everything, we call the individual relocation steps one by one. Again this makes it easier to leave things out, particularly for SPL.

ARM is a relatively large board.c file and one which I can test, therefore I think it is a good target for this series. On the other hand, x86 is relatively small and simple, but different enough that it introduces a few issues to be solved. So I have chosen both ARM and x86 for this series.

The next target should probably be PowerPC, since it is large and has some additional features. I suspect we may want to leave some of these very architecture-specific functions in arch/powerpc/lib/board.c, taking out only the generic code. I haven't felt a strong need to do this for ARM/x86, but we could even go as far as putting the initcall list into the architecture-specific board file if the architecture adds a lot of unusual calls.

A generic global_data structure is also required. This might upset a few people. Here is my basic reasoning: most fields are the same, all architectures include and need it, most global_data.h files already have
#ifdefs to select fields for a particular SOC, so it is hard to
see why architecures are different in this area. We can perhaps add a way to put architecture-specific fields into a separate header file, but for now I have judged that to be counter-productive.

There was dicussion on the list about passing gd_t around as a parameter to pre-relocation init functions. I think this makes sense, but it can be done as a separate change, and this series does not require it.

While this series needs to stand on its own (as with the link script cleanup series and the generic relocation series) the goal is the unification of the board init code. So I hope we can address issues with this in mind, rather than focusing too narrowly on particular ARM or x86 issues.

Comments welcome. Note that the x86 side of this still needs a fair bit of work, sorry.

Simon Glass (19):
Introduce generic global_data
Make relocation functions global
Add basic initcall implementation
define CONFIG_SYS_LEGACY_BOARD everywhere Add generic post-relocation board_r.c
Add generic pre-relocation board_f.c
Add spl load feature
switch ARM over to generic board
arm: Remove unused code in board.c, global_data.h Add CONFIG_SYS_SYM_OFFSETS to support offset symbols
x86: Remove compiler warning in sc520_timer.c

x86: Remove dead code in eNET
x86: Add processor library and relocation functions Tidy up asm/generic sections.h to include x86 symbols Add fields required by x86 to global_data
x86: Change stub example to use asm-generic/sections.h
x86: Add initial memory barrier macros
Bring in x86 to unified board architecture
x86: Remove unused board/global_data code

I honestly think we should get the x86 init sequence patches finalised first for several reasons:

- Because x86 is so small, it provides a good test-bed - ELF relocation was first finalised on x86 (it came and went with varying levels of success previously)
- They bring x86 in line with other arches re: global data
- They are now fully run-tested

Hi Simon,

Sorry for the delay in reviewing this - I've been doing a lot of work on the x86 side of things. I now have a working solution to the board_init_f_r() / global data clobbering problem which involves having the gd 'variable' as a register like all other arch's. The solution is non-trivial and gd access is slightly more expensive than the vanilla variable approach, but it makes this a lot cleaner cross-arch wise...

Here's a hint ;)

static inline gd_t *get_fs_gd_ptr(void)
{
? ? ? gd_t *gd_ptr;

? ? ? asm volatile("fs movl 0, %0\n" : "=r" (gd_ptr));

? ? ? return gd_ptr;
}

#define gd ? ? ?get_fs_gd_ptr()

Quoted 4 lines · Simon Glass

This series creates a generic board.c implementation which contains the essential functions of the various arch/xxx/lib/board.c files.

What is the motivation for this change?

[snip]

I think that we can all agree that there is strong motivation for change.

However, I think this approach is not the right one - and I think the CFI driver backs me up. Your plan is to create generic code which you want ALL arches to cross over to, but you only look to migrate two initially and migrate the rest 'later'. This is similar to what happened with the CFI driver, and there are still boards with custom flash.c files which are completely redundant.

But, creating a single patch-set to migrate everyone in one go is going to be too massive a job to do in one go, and too prone to introducing breakage.

Quoted 5 lines

All the functions of board_init_f() and board_init_r() are broken into separate function calls so that they can easily be included or excluded for a particular architecture. It also makes it easier to adopt Graeme's initcall proposal later if desired.

I think we should look at this sooner rather than later. I've shown with x86 that the init sequence has three distinct phases:

?1) Execute in flash using temporary RAM
?2) Copy U-Boot from flash to RAM
?3) Execute in RAM

My latest work now has all init functions having the same signature (i.e. void parameter returning int). For x86, there is a little 'magic' that needs to be done as gd is copied from temporary RAM to SDRAM, but for other arches using a dedicated register, this is otherwise trivial.

So instead of trying to pluck out something (relocation in this case) from the molasses of ten different board.c files and having to perform open-heart surgery on them all to get the code grafted in, I think we should approach it from the generic init sequence angle.

If we work each individual arch to use a generic init sequence (like the proposed x86 code) then the init processing naturally falls out as common code and the patch to realise this is trivial. From there, we can start to pull out common init code like init_baudrate() and hang() and change the function signatures of the functions that require wrappers and move some #ifdef's into more appropriate locations - One example in board.c:

#ifdef CONFIG_BITBANGMII
int bb_miiphy_init_r(void)
{
? ? ? ?bb_miiphy_init();

? ? ? ?return 0;
}
#endif

Ouch!

The other big benefit is that you only touch one architecture at a time up until you 'pull the switch'. And when you do pull the switch, you should be factoring out identical code so the chances of breaking something should be vastly reduced. Take a look at the history of ARM relocation for example - that was constrained to one arch but still the amount of breakage was massive.

Hi Graeme,

Quoted 2 lines · Graeme Russ

Hi Simon,

Quoted 7 lines

However, I think this approach is not the right one - and I think the CFI driver backs me up. Your plan is to create generic code which you want ALL arches to cross over to, but you only look to migrate two initially and migrate the rest 'later'. This is similar to what happened with the CFI driver, and there are still boards with custom flash.c files which are completely redundant.

Quoted 2 lines

But, creating a single patch-set to migrate everyone in one go is going to be too massive a job to do in one go, and too prone to introducing breakage.

Yes to some extent. However, my patch basically splits the two board_init_x() functions into parts, and puts them in a function table.

I don't think it is a huge job to do this for PowerPC also, and that seems to be the most feature-full architecture.

Also it does depend on expectations. I would hope that moving an architecture over would be a fairly small task:

- getting generic relocation working
- adding functions for anything that is missing from board init code
- removing things which don't work on the architecture?
- worrying about differences in ordering between functions

Quoted 6 lines

If we work each individual arch to use a generic init sequence (like the proposed x86 code) then the init processing naturally falls out as common code and the patch to realise this is trivial. From there, we can start to pull out common init code like init_baudrate() and hang() and change the function signatures of the functions that require wrappers and move some #ifdef's into more appropriate locations - One example in board.c:

Well it seems like a lot of work to refactor each arch/xxx/board.c file into functions with a function pointer list, then later remove this code function by function.

My feeling is that with a generic board, it should hopefully be a fairly small amount of work for someone familiar with an architecture to find the bugs and patch the generic code to suit their

architecture. It is something that needs to be done once, not every time there is a new patch removing (almost) common code.

Quoted 7 lines

The other big benefit is that you only touch one architecture at a time up until you 'pull the switch'. And when you do pull the switch, you should be factoring out identical code so the chances of breaking something should be vastly reduced. Take a look at the history of ARM relocation for example - that was constrained to one arch but still the amount of breakage was massive.

From previous discussions, if something is optional then the switch

will never happen. The code you are talking about is sometimes identical, sometimes slightly different. In some cases the order is different. I see many ways of introducing breakages. I do agree that

doing one architecture at a time is best - with the proviso that we need to pick archs that have the most features (so ARM and PowerPC I suppose) to make sure we are not deluding ourselves as to the simplicity of the task.

So perhaps a generic board init that is the default can be switched off on board-by-board basic would be the right approach. Then we can flick the switch while providing for those affected to still get work done until bugs are reported and found?

Quoted 6 lines

Quoted 5 lines

Generic relocation is used (see previous series) but now rather than calling relocate_code() to do everything, we call the individual relocation steps one by one. Again this makes it easier to leave things out, particularly for SPL.

Quoted 2 lines

Note that not all arches need and/or use ELF relocation - Attacking this first does not move towards unity of board.c

It is a feature used by about half, and it does include the complexity of jumping from pre-reloc to post-reloc init. I think it was a reasonable first target.

Quoted 2 lines

The big issue I have is that we now have two RFC's which have major impacts on x86 and one of us is going to have to give way to the other :)

Well I posted my x86 patches so they are not just sitting in the long grass on my machine. They are not complete, and I would likely need your input on these anyway.

Quoted 3 lines

My patch set is mostly re-factoring and has no impact on anybody else and is practically complete - I'll do one last RFC, but I think it's already close to 'PATCH' status

That's a separate series from my point of view. I can't comment on the wierd x96-isms in the series, but I did read through each patch and it all seems sensible to me.

Quoted 2 lines

There are several x86 patches which simply do not belong in this series (and some of which already have non-RFC patches submitted)

OK, these are the warnings I think. Probably I should build without -Werrors and try to ignore them!

Quoted 8 lines

I honestly think we should get the x86 init sequence patches finalised first for several reasons:

- Because x86 is so small, it provides a good test-bed - ELF relocation was first finalised on x86 (it came and went with varying levels of success previously)
- They bring x86 in line with other arches re: global data
- They are now fully run-tested

No disagreement there, and anyway as you can see the relocation stuff mostly came from your x86 code. It will provide a good template for getting x86 working properly with the generic board init.

What do you need from me to get that through?

Per Wolfgang's request to go with PPC as an early-adopter, this is somewhat in conflict, since as you say, x86 is less feature-full than ARM and much less than PowerPC.

Can anyone recommend a PowerPC board with a quick U-Boot program-run cycle that I can easily get that will let me try out things there?

Hi Simon,

Quoted 5 lines · Simon Glass

Hi Graeme,

Quoted 2 lines · Graeme Russ

Hi Simon,

[snip]

Quoted 7 lines

Quoted 7 lines

However, I think this approach is not the right one - and I think the CFI driver backs me up. Your plan is to create generic code which you want ALL arches to cross over to, but you only look to migrate two initially and migrate the rest 'later'. This is similar to what happened with the CFI driver, and there are still boards with custom flash.c files which are completely redundant.

Quoted 6 lines

Quoted 2 lines

But, creating a single patch-set to migrate everyone in one go is going to be too massive a job to do in one go, and too prone to introducing breakage.

Yes to some extent. However, my patch basically splits the two board_init_x() functions into parts, and puts them in a function table.

Ah yes, looking a bit closer I see this now in patches 5 & 6. However, I think if you look at my patches 9, 10, 12 and 13 I'm doing essentially the same thing for x86. The difference is, I'm moving live code, rather than creating dead code then switching over. I personally do not think your approach is very safe - If there is a breakage, it is spread over multiple patches - With the 'move code around' approach, all breakages are confined to a single patch

Quoted 3 lines

I don't think it is a huge job to do this for PowerPC also, and that seems to be the most feature-full architecture.

I agree - The init architecture is the same, but the sequence is bigger (and not in the same order)

Quoted 8 lines

Also it does depend on expectations. I would hope that moving an architecture over would be a fairly small task:

- getting generic relocation working
- adding functions for anything that is missing from board init code
- removing things which don't work on the architecture?
- worrying about differences in ordering between functions

I see it as:
?1) Rework the arch init sequence to be a pure list of 'int foo(void)' ? ?functions, adding helpers and wrappers where necessary ?2) Switch over to use the generic init processing loop
?3) Factor out common helpers and wrappers across all arches ?4) Factor out common functionality (e.g. relocation)
?5) After all arches are switched, remove wrappers by changing the ? ?function signature of the wrapped function
?6) Move helpers into relevant driver/common source file

I think 1 & 2 need to be done for at least ARM, x86 and PPC before starting 3 & 4 and 1-4 needs to be done for all arches before doing 5 & 6

[snip]

Quoted 7 lines

Quoted 6 lines

If we work each individual arch to use a generic init sequence (like the proposed x86 code) then the init processing naturally falls out as common code and the patch to realise this is trivial. From there, we can start to pull out common init code like init_baudrate() and hang() and change the function signatures of the functions that require wrappers and move some #ifdef's into more appropriate locations - One example in board.c:

Quoted 3 lines

Well it seems like a lot of work to refactor each arch/xxx/board.c file into functions with a function pointer list, then later remove this code function by function.

Yes, it is more work, but it is intrinsically safer (and git bisect-able)

Quoted 4 lines

My feeling is that with a generic board, it should hopefully be a fairly small amount of work for someone familiar with an architecture to find the bugs and patch the generic code to suit their

You seem to admit that your approach can introduce bugs

Quoted 3 lines

architecture. It is something that needs to be done once, not every time there is a new patch removing (almost) common code.

The safer approach is to tackle each arch independently - Convert each board.c to something similar to what I have shown with x86 and test those arch-specific changes to make sure nothing has been broken. Then it is a trivial matter of checking that there are no incompatibilities between the arch and the common case (and adjusting as required) and switching over to the common case

[snip]

Quoted 7 lines

Quoted 7 lines

The other big benefit is that you only touch one architecture at a time up until you 'pull the switch'. And when you do pull the switch, you should be factoring out identical code so the chances of breaking something should be vastly reduced. Take a look at the history of ARM relocation for example - that was constrained to one arch but still the amount of breakage was massive.

From previous discussions, if something is optional then the switch

Quoted 3 lines

will never happen. The code you are talking about is sometimes identical, sometimes slightly different. In some cases the order is different. I see many ways of introducing breakages. I do agree that

The init sequence order is dictated arch-specifically through the init arrays which will (for the time being) remain in the arch specific board.c. But that is all arch/foo/lib/board.c will contain - init sequence arrays, no 'code'

Quoted 5 lines

doing one architecture at a time is best - with the proviso that we need to pick archs that have the most features (so ARM and PowerPC I suppose) to make sure we are not deluding ourselves as to the simplicity of the task.

ARM + PPC + x86 = 95% of the complexity. If we pull them off, the others are trivial cases.

Quoted 15 lines

So perhaps a generic board init that is the default can be switched off on board-by-board basic would be the right approach. Then we can flick the switch while providing for those affected to still get work done until bugs are reported and found?

Quoted 8 lines

Quoted 5 lines

Generic relocation is used (see previous series) but now rather than calling relocate_code() to do everything, we call the individual relocation steps one by one. Again this makes it easier to leave things out, particularly for SPL.

Note that not all arches need and/or use ELF relocation - Attacking this first does not move towards unity of board.c

Quoted 3 lines

It is a feature used by about half, and it does include the complexity of jumping from pre-reloc to post-reloc init. I think it was a reasonable first target.

It is an excellent first target after making the init sequence generic - skipping relocation is then trivial (you just don't include it in the init sequence)

[snip]

Quoted 3 lines

Quoted 2 lines

The big issue I have is that we now have two RFC's which have major impacts on x86 and one of us is going to have to give way to the other :)

Quoted 3 lines

Well I posted my x86 patches so they are not just sitting in the long grass on my machine. They are not complete, and I would likely need your input on these anyway.

Most of your patches are a subset of my complete init re-write (and credit where it is due, you initial relocation RFC was the catalyst - I wanted to make your life a little easier by simplifying things in x86-land)

Quoted 8 lines

Quoted 4 lines

My patch set is mostly re-factoring and has no impact on anybody else and is practically complete - I'll do one last RFC, but I think it's already close to 'PATCH' status

That's a separate series from my point of view. I can't comment on the wierd x96-isms in the series, but I did read through each patch and it all seems sensible to me.

Thanks, that is enough for me for now. I'll request a proper review when I submit the [PATCH] series

[snip]

Quoted 3 lines

Quoted 2 lines

There are several x86 patches which simply do not belong in this series (and some of which already have non-RFC patches submitted)

Quoted 2 lines

OK, these are the warnings I think. Probably I should build without -Werrors and try to ignore them!

Propagate them to the top of your list to get them out of the way. I need to add them to my series (with original attribution of course) or commit them as is (some need tweaking)

Quoted 10 lines

Quoted 9 lines

I honestly think we should get the x86 init sequence patches finalised first for several reasons:

?- Because x86 is so small, it provides a good test-bed - ELF relocation ? was first finalised on x86 (it came and went with varying levels of ? success previously)
?- They bring x86 in line with other arches re: global data ?- They are now fully run-tested

Quoted 5 lines

No disagreement there, and anyway as you can see the relocation stuff mostly came from your x86 code. It will provide a good template for getting x86 working properly with the generic board init.

What do you need from me to get that through?

Patience :) - I'll try to get a final series up this week

Quoted 3 lines

Per Wolfgang's request to go with PPC as an early-adopter, this is somewhat in conflict, since as you say, x86 is less feature-full than ARM and much less than PowerPC.

We need to get the PPC (plus ARM and x86) init sequence distilled to pure init loops before moving forward.

Quoted 3 lines

Can anyone recommend a PowerPC board with a quick U-Boot program-run cycle that I can easily get that will let me try out things there?

If all we are doing is moving and wrapping code, the review and test should be fairly straighfoward

Regards,

Graeme

Hi Graeme,

Quoted 8 lines · Graeme Russ

Hi Simon,

Quoted 5 lines · Simon Glass

Hi Graeme,

Quoted 2 lines · Graeme Russ

Hi Simon,

Quoted 8 lines

Ah yes, looking a bit closer I see this now in patches 5 & 6. However, I think if you look at my patches 9, 10, 12 and 13 I'm doing essentially the same thing for x86. The difference is, I'm moving live code, rather than creating dead code then switching over. I personally do not think your approach is very safe - If there is a breakage, it is spread over multiple patches - With the 'move code around' approach, all breakages are confined to a single patch

See my other email on this topic though - I feel that one test/fix cycle is better than testing every board for every little patch over an extended period.

Quoted 5 lines

Quoted 2 lines

I don't think it is a huge job to do this for PowerPC also, and that seems to be the most feature-full architecture.

I agree - The init architecture is the same, but the sequence is bigger (and not in the same order)

Yes that's what I'm finding.

Quoted 19 lines

Quoted 8 lines

Also it does depend on expectations. I would hope that moving an architecture over would be a fairly small task:

- getting generic relocation working
- adding functions for anything that is missing from board init code
- removing things which don't work on the architecture?
- worrying about differences in ordering between functions

I see it as:
1) Rework the arch init sequence to be a pure list of 'int foo(void)' functions, adding helpers and wrappers where necessary
2) Switch over to use the generic init processing loop
3) Factor out common helpers and wrappers across all arches
4) Factor out common functionality (e.g. relocation)
5) After all arches are switched, remove wrappers by changing the function signature of the wrapped function
6) Move helpers into relevant driver/common source file

Quoted 2 lines

I think 1 & 2 need to be done for at least ARM, x86 and PPC before starting 3 & 4 and 1-4 needs to be done for all arches before doing 5 & 6

OK yes we could do this. But I think the end result is different.

I am keen to create a single common/board_f.c implementation for pre-reloc init and common/board_r.c for post-reloc init. I believe where your approach ends up with with separate arch/xxx/board.c files still, with their own initcall list, and a number of private

functions. Now this is better than where we are now, but my original complaint was that I want to be able to add features to a single place and have them work across all architectures. With your end-result I still need to figure out where to put the new feature in each initcall list and create 10 patches, one for each arch, to plumb it in.

IMO the ordering differences between architectures are mostly unnecessary - an artifact of forking the code - and we should aim to minimise these differences.

Putting everything in one file helps to clarify exactly what the differences are between archs, and discussions can take place as to why and whether this differences can be removed. If we accept that PPC is the one true way (in terms of U-Boot code base), then the job is to make other arch's code more like PPC. This is best done in one place I think.

After a bit of thought, I worry also that your approach will create an extended period of time where U-Boot is unstable, as patch are patch

is added to refactor the code. Granted, many of these patches are small and the risk is small. But why bother? At the end we still have 10 different board files, lots of differences and no one willing to change anything because it requires an entire test cycle again.

Maybe I am being too ambitious here, but since everything started with PPC's board.c, I don't see why we shouldn't head back in that direction and just create a single file again. It does create a big test problem, but only once :-)

Anyway if we do go with what you propose I still think it is better than what we have.

Quoted 10 lines

[snip]

Quoted 7 lines

Quoted 6 lines

If we work each individual arch to use a generic init sequence (like the proposed x86 code) then the init processing naturally falls out as common code and the patch to realise this is trivial. From there, we can start to pull out common init code like init_baudrate() and hang() and change the function signatures of the functions that require wrappers and move some #ifdef's into more appropriate locations - One example in board.c:

Quoted 5 lines

Quoted 3 lines

Well it seems like a lot of work to refactor each arch/xxx/board.c file into functions with a function pointer list, then later remove this code function by function.

Yes, it is more work, but it is intrinsically safer (and git bisect-able)

Well I don't propose to create things which are not bisect-able. I

think what you mean is that there will be a commit which switches over to generic board for an architecture, and board init before that patch is quite different from board init afterwards. Yes that's right. Once the patch is reviewed, testing and committed, presumably most boards will work fine. The exception will be the boards which have unique features and no one had time to check them. That problem is not unique to either approach. Future patches welcome :-)

Quoted 5 lines

Quoted 3 lines

My feeling is that with a generic board, it should hopefully be a fairly small amount of work for someone familiar with an architecture to find the bugs and patch the generic code to suit their

You seem to admit that your approach can introduce bugs

Yes I do. Both approaches introduce bugs. I think that is unavoidable so we should try to do this once and get the best result out of it that we can.

Quoted 9 lines

Quoted 2 lines

architecture. It is something that needs to be done once, not every time there is a new patch removing (almost) common code.

The safer approach is to tackle each arch independently - Convert each board.c to something similar to what I have shown with x86 and test those arch-specific changes to make sure nothing has been broken. Then it is a trivial matter of checking that there are no incompatibilities between the arch and the common case (and adjusting as required) and switching over to the common case

This switch-over is really no different from what I propose (except that I want to go further and have a single board file). It introduces the same bisect issue you mentioned, and I feel it doesn't really

improve the situation. We have instability with each patch as we refactor the board init code on each arch, then instability when we switch over. Why not just have this pain once?

Quoted 13 lines

[snip]

Quoted 10 lines

Quoted 7 lines

The other big benefit is that you only touch one architecture at a time up until you 'pull the switch'. And when you do pull the switch, you should be factoring out identical code so the chances of breaking something should be vastly reduced. Take a look at the history of ARM relocation for example - that was constrained to one arch but still the amount of breakage was massive.

From previous discussions, if something is optional then the switch

will never happen. The code you are talking about is sometimes identical, sometimes slightly different. In some cases the order is different. I see many ways of introducing breakages. I do agree that

Quoted 4 lines

The init sequence order is dictated arch-specifically through the init arrays which will (for the time being) remain in the arch specific board.c. But that is all arch/foo/lib/board.c will contain - init sequence arrays, no 'code'

OK, see above. Better than what we have, for sure. I am hoping we can go further.

The problem is not one of how sparsely the test/fix cycles are spread over time, it is one of spreading the breakage over multiple patches - If you are replacing functionality then add the new functionality, add the hooks to use it and delete the old in a single patch. That way, if you change breaks something, the revert is trivial. If you multi-patch approach breaks something, the revert becomes more difficult.

Quoted 1 line

Well I don't propose to create things which are not bisect-able. I

But you have - You create new functionality in one patch, add a number of patches, then finally use that new functionality in a later patch.

Dear Graeme Russ,

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

The problem is not one of how sparsely the test/fix cycles are spread over time, it is one of spreading the breakage over multiple patches - If you are replacing functionality then add the new functionality, add the hooks to use it and delete the old in a single patch. That way, if you change breaks something, the revert is trivial. If you multi-patch approach breaks something, the revert becomes more difficult.

True. ?Especially as it's likely that different patches will break different boards, so there will not even be a chance so revert.

Quoted 5 lines

Quoted 2 lines

Well I don't propose to create things which are not bisect-able. I

But you have - You create new functionality in one patch, add a number of patches, then finally use that new functionality in a later patch.

This is what I fear as well.

Best regards,

Wolfgang Denk

--
DENX Software Engineering GmbH, ? ? MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de Disc space - the final frontier!

Well let's see how we go with the incremental approach - hopefully we can get the same result with less pain and risk, and not too much work.

Dear Simon Glass,

Quoted 4 lines · In message <CAPnjgZ2jS8kzCt06LbYnXSyEKm6ck8apm=P1YqqDtnZ0+q9dhw@mail.gmail.com>…

Well let's see how we go with the incremental approach - hopefully we can get the same result with less pain and risk, and not too much work.

Did you miss my proposal not to change the existing coe for all boards, but to provide anew implementation and convert boards one by one? What do you think about such an approach?

Dear Simon Glass,

Quoted 1 line · In message <CAPnjgZ2jS8kzCt06LbYnXSyEKm6ck8apm=P1YqqDtnZ0+q9dhw@mail.gmail.com>…

Quoted 3 lines

Well let's see how we go with the incremental approach - hopefully we can get the same result with less pain and risk, and not too much work.

Did you miss my proposal not to change the existing coe for all boards, but to provide anew implementation and convert boards one by one? ?What do you think about such an approach?

Best regards,

Wolfgang Denk

--
DENX Software Engineering GmbH, ? ? MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de "The X11 source code style is ATROCIOUS and should not be used ?as ?a model." ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? - Doug Gwyn

I have brought in PPC to board_f.c and board_r.c but not dealt with relocation. Once I have done that, gone through the new x86 series again and MAKEALL doesn't scream too loudly I'll do another RFC along these lines.

Dear Simon Glass,

Quoted 5 lines · In message <CAPnjgZ3592m_m=TMaFptwzO+_O94veYkVRFFdGPP599igefNyg@mail.gmail.com>…

I have brought in PPC to board_f.c and board_r.c but not dealt with relocation. Once I have done that, gone through the new x86 series again and MAKEALL doesn't scream too loudly I'll do another RFC along these lines.

I'm seriously concerned about you insisting to include relocation here. ?This has nothign to do with it. ?Leave that out.

Best regards,

Wolfgang Denk

--
DENX Software Engineering GmbH, ? ? MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de "When the only ?tool ?you ?have ?is ?a ?hammer, ?you ?tend ?to ?treat everything as if it were a nail." ? ? ? ? ? ? ? ? ? ?- Abraham Maslow

Hi Simon,

In case you haven't noticed (I did not Cc you or Wolfgang, sorry 'bout that) I've posted the cleaned up version of my x86 init refactoring...

Quoted 6 lines

functions. Now this is better than where we are now, but my original complaint was that I want to be able to add features to a single place and have them work across all architectures. With your end-result I still need to figure out where to put the new feature in each initcall list and create 10 patches, one for each arch, to plumb it in.

Agree, and my ultimate end goal (as per my previous initcall patch series) is to remove the per-arch init list as well...

Regards,

Graeme

Also it does depend on expectations. I would hope that moving an architecture over would be a fairly small task:

- getting generic relocation working

- adding functions for anything that is missing from board init code

- removing things which don't work on the architecture?

- worrying about differences in ordering between functions

Quoted 3 lines

pull out common init code like init_baudrate() and hang() and change the function signatures of the functions that require wrappers and move some #ifdef's into more appropriate locations - One example in board.c:

Well it seems like a lot of work to refactor each arch/xxx/board.c file into functions with a function pointer list, then later remove this code function by function.

My feeling is that with a generic board, it should hopefully be a fairly small amount of work for someone familiar with an architecture to find the bugs and patch the generic code to suit their architecture. It is something that needs to be done once, not every time there is a new patch removing (almost) common code.

From previous discussions, if something is optional then the switch will never happen. The code you are talking about is sometimes identical, sometimes slightly different. In some cases the order is different. I see many ways of introducing breakages. I do agree that

doing one architecture at a time is best - with the proviso that we need to pick archs that have the most features (so ARM and PowerPC I suppose) to make sure we are not deluding ourselves as to the simplicity of the task.

So perhaps a generic board init that is the default can be switched off on board-by-board basic would be the right approach. Then we can flick the switch while providing for those affected to still get work done until bugs are reported and found?

Quoted 2 lines

Note that not all arches need and/or use ELF relocation - Attacking this first does not move towards unity of board.c

It is a feature used by about half, and it does include the complexity of jumping from pre-reloc to post-reloc init. I think it was a reasonable first target.

Per Wolfgang's request to go with PPC as an early-adopter, this is

Can anyone recommend a PowerPC board with a quick U-Boot program-run cycle that I can easily get that will let me try out things there?

Dear Simon Glass,

Quoted 1 line · In message <CAPnjgZ1Zim6=u3rLcnAKE-0Bw3sO_hRY+U67jkpkNoNi8g7ctw@mail.gmail.com>…

Quoted 4 lines

Also it does depend on expectations. I would hope that moving an architecture over would be a fairly small task:

- getting generic relocation working

I don;t see why this would be a direct prerequisite here. ?We want to have that, no boubt about that. ?But it appears to be an unrelated change to me.

Quoted 2 lines

- adding functions for anything that is missing from board init code

"anything that is missing from board init code" ?

Quoted 2 lines

- removing things which don't work on the architecture?

That would probably reander systems broken that need these things?

Quoted 2 lines

- worrying about differences in ordering between functions

This is indeed the biggest issue.

Keep in mind that my original ida of providing a function call table was to allow to define this table in a board specific way (i. e. in the board config file).

[Not that this idea found many friends...]

Quoted 8 lines

Quoted 4 lines

pull out common init code like init_baudrate() and hang() and change the function signatures of the functions that require wrappers and move some #ifdef's into more appropriate locations - One example in board.c:

Well it seems like a lot of work to refactor each arch/xxx/board.c file into functions with a function pointer list, then later remove this code function by function.

Would that really be a clever approach?

Quoted 6 lines

My feeling is that with a generic board, it should hopefully be a fairly small amount of work for someone familiar with an architecture to find the bugs and patch the generic code to suit their architecture. It is something that needs to be done once, not every time there is a new patch removing (almost) common code.

It is definitely not that easy. ?You fix one thing here, and break another board there. ?Ideally you would have to re-test any change on _all_ boards.

Quoted 5 lines

From previous discussions, if something is optional then the switch will never happen. The code you are talking about is sometimes identical, sometimes slightly different. In some cases the order is different. I see many ways of introducing breakages. I do agree that

And I bet most of them _will_ introduce breakages.

Quoted 4 lines

doing one architecture at a time is best - with the proviso that we need to pick archs that have the most features (so ARM and PowerPC I suppose) to make sure we are not deluding ourselves as to the simplicity of the task.

I would suggest to attempt to merge ARM into PPC.

Quoted 5 lines

So perhaps a generic board init that is the default can be switched off on board-by-board basic would be the right approach. Then we can flick the switch while providing for those affected to still get work done until bugs are reported and found?

Heh! ?Board specific init tables!

Quoted 7 lines

Quoted 3 lines

Note that not all arches need and/or use ELF relocation - Attacking this first does not move towards unity of board.c

It is a feature used by about half, and it does include the complexity of jumping from pre-reloc to post-reloc init. I think it was a reasonable first target.

What exactly is the problem here?

Quoted 2 lines

Per Wolfgang's request to go with PPC as an early-adopter, this is

No. ?It's the other way round. ?PPC is what should be adopted to.

Quoted 3 lines

Can anyone recommend a PowerPC board with a quick U-Boot program-run cycle that I can easily get that will let me try out things there?

Define "get easily".

Best regards,

Wolfgang Denk

--
DENX Software Engineering GmbH, ? ? MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de People have one thing in common: they are all different.

Quoted 6 lines

Quoted 2 lines

- getting generic relocation working

I don;t see why this would be a direct prerequisite here. =A0We want to have that, no boubt about that. =A0But it appears to be an unrelated change to me.

I don't think it's essential, but it is desirable, which I why it is on the list.

Quoted 3 lines

Quoted 1 line

- adding functions for anything that is missing from board init code

"anything that is missing from board init code" ?

By that I mean that if the architecture has its own little things which aren't supported by generic board init, then these would need to be worked into the generic board init somehow, perhaps initially with a new function in the initcall list.

Quoted 3 lines

Quoted 1 line

- removing things which don't work on the architecture?

That would probably reander systems broken that need these things?

Sorry for being so vague. What I mean is that if an arch does not support a feature, then we don't want to call the init functions for it on that architecture. Sometimes this is handled automatically - e.g. if an arch does not support CONFIG_POST then the generic board init code will skip this anyway. But there may be other cases where features available in the generic board init cannot be supported in all archs.

Quoted 5 lines

Keep in mind that my original ida of providing a function call table was to allow to define this table in a board specific way (i. e. in the board config file).

[Not that this idea found many friends...]

With this series I have moved over to purely having a function call table, with no actual init code in board_init_f() and board_init_r().

Having done this, I have found that the list of functions is very long, So we probably don't want boards to duplicate this list and #ifdefs may be the lesser of two evils. If we can move to Graham'

For now I have tried to simply move the code into separate functions, since this makes it easier to compare against the original code in the arch/xxx/lib/board.c files, to make sure nothing is left out and the ordering is OK.

I'm not convinced it is the best idea. A trivial change of just removing duplicated code from all the archs and inserting it into generic board should be safe enough. But is that really a common case? I have found lots of subtle differences that need to be worked out, and there is the ordering to worry about. Every time there is a patch we need to re-test all boards. It might be easier to just create a generic board implementation one, and then test it once on each board, rather than testing on each board with every patch. Of course every time you patch generic board you might introduce a breakage in an existing (previously working) board, but that is the normal situation with development so I don't see a problem there.

Perhaps the biggest problem is the difficulty of testing, and if we can avoid months of little patches (each one potentially breaking lots of boards at run-time) then I would prefer that.

Well I suppose that's what I'm saying. Doing this once, accepting that some boards will thereby be broken, and then accepting patches to fix them feels easier than trying to keep everything working through a long series of little changes.

Quoted 5 lines

Quoted 3 lines

It is a feature used by about half, and it does include the complexity of jumping from pre-reloc to post-reloc init. I think it was a reasonable first target.

What exactly is the problem here?

The problem is the code duplication inherent in relocation - each architecture has its own way of doing it, but really it can mostly be written generically in C. ARM is even worse with each CPU implementing essentially the same code!