[U-Boot] A bit about board.c, board.c

[U-Boot] A bit about board.c, board.c

[Simon Glass]

Hi,

Each architecture has its own board.c but they are mostly quite similar.

New features such as fdt, boot timing, trace, profiling, etc. must be
done separately for each arch, even if there are no
architecture-specific bits.

What would you say to adding something like lib/board.c which is a
simplified cleaned-up copy of one of the existing board.c files, and a
CONFIG_ARCH_GENERIC_BOARD option to select that in preference to the
architecture-specific one. Then perhaps people could try it out and we
could slowly move to it over time...

Regards,
Simon

[U-Boot] A bit about board.c, board.c

[Albert ARIBAUD]

Le 22/10/2011 07:11, Simon Glass a ?crit :
> Hi,
>
> Each architecture has its own board.c but they are mostly quite similar.
>
> New features such as fdt, boot timing, trace, profiling, etc. must be
> done separately for each arch, even if there are no
> architecture-specific bits.
>
> What would you say to adding something like lib/board.c which is a
> simplified cleaned-up copy of one of the existing board.c files, and a
> CONFIG_ARCH_GENERIC_BOARD option to select that in preference to the
> architecture-specific one. Then perhaps people could try it out and we
> could slowly move to it over time...

I'd say anything that factorizes ARM code is a good thing. :)

However I'm not in favor of a CONFIG option that would force the board 
code to provide some functions just in odrer to override one, not if 
weak definitions can do a finer job. I am looking into that already for 
cache management function specialization, and it seems like there is the 
same kind of possibility here, with lib functions defined weak and the 
board code overriding the ones it deems necessary to.

I will look into it a bit more this morning.

Amicalement,
-- 
Albert.

[U-Boot] A bit about board.c, board.c

[Simon Glass]

Hi Albert,

On Sat, Oct 22, 2011 at 12:17 AM, Albert ARIBAUD
<albert.u.boot@aribaud.net> wrote:
> Le 22/10/2011 07:11, Simon Glass a ?crit :
>>
>> Hi,
>>
>> Each architecture has its own board.c but they are mostly quite similar.
>>
>> New features such as fdt, boot timing, trace, profiling, etc. must be
>> done separately for each arch, even if there are no
>> architecture-specific bits.
>>
>> What would you say to adding something like lib/board.c which is a
>> simplified cleaned-up copy of one of the existing board.c files, and a
>> CONFIG_ARCH_GENERIC_BOARD option to select that in preference to the
>> architecture-specific one. Then perhaps people could try it out and we
>> could slowly move to it over time...
>
> I'd say anything that factorizes ARM code is a good thing. :)
>
> However I'm not in favor of a CONFIG option that would force the board code
> to provide some functions just in odrer to override one, not if weak

My suggestion was:

CONFIG_ARCH_GENERIC_BOARD - you get lib/board.c
not CONFIG_ARCH_GENERIC_BOARD - you get arch/xxx/lib/board.c

> definitions can do a finer job. I am looking into that already for cache
> management function specialization, and it seems like there is the same kind
> of possibility here, with lib functions defined weak and the board code
> overriding the ones it deems necessary to.

ok. Just one little point. Weak symbols are the punishment our C++
overlords gave us for not having to understand what code is executed
when we instantiate a C++ subclass. It is nice to know what code is
being linked in and avoid having to disassemble to find out. I think
using weak symbols to avoid a OBJS-$(CONFIG_ARCH_GENERIC_BOARD) in a
couple of Makefiles risks going too far.

Regards,
Simon

Regards,
Simon


Regards,
Simon

>
> I will look into it a bit more this morning.
>
> Amicalement,
> --
> Albert.
>

[U-Boot] A bit about board.c, board.c

[Wolfgang Denk]

Dear Simon Glass,

In message <CAPnjgZ3ZdkF-dnLnC8=L55ZDWznWW6wqu0eJ7puTAT=3rcMcPQ@mail.gmail.com> you wrote:
> 
> My suggestion was:
> 
> CONFIG_ARCH_GENERIC_BOARD - you get lib/board.c
> not CONFIG_ARCH_GENERIC_BOARD - you get arch/xxx/lib/board.c

If we do something like this, then the other way round.  There should
never be any #defines needed for the standard case - i. e. for using
lib/board.c,

Also, "ARCH_GENERIC_BOARD" is not a good name as it does not really
tell the reader that you are referencing to a specific source file
here - most people will think you are referring to some hardware
properties.

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
"My name is Linus Torvalds, you messed with my kernel, prepare to die"
                                                  - Linus Torvalds in
      <Pine.LNX.3.91.960426110644.24860I-100000@linux.cs.Helsinki.FI>

[U-Boot] A bit about board.c, board.c

[Simon Glass]

Hi Wolfgang,

On Sun, Oct 23, 2011 at 1:14 AM, Wolfgang Denk <wd@denx.de> wrote:
> Dear Simon Glass,
>
> In message <CAPnjgZ3ZdkF-dnLnC8=L55ZDWznWW6wqu0eJ7puTAT=3rcMcPQ@mail.gmail.com> you wrote:
>>
>> My suggestion was:
>>
>> CONFIG_ARCH_GENERIC_BOARD - you get lib/board.c
>> not CONFIG_ARCH_GENERIC_BOARD - you get arch/xxx/lib/board.c
>
> If we do something like this, then the other way round. ?There should
> never be any #defines needed for the standard case - i. e. for using
> lib/board.c,

OK, makes sense.

>
> Also, "ARCH_GENERIC_BOARD" is not a good name as it does not really
> tell the reader that you are referencing to a specific source file
> here - most people will think you are referring to some hardware
> properties.

OK, in my testing I have put this into config.mk in arch/xxx/config.mk:

# Move to unified board system later
CONFIG_SYS_LEGACY_BOARD := y

We can remove it arch by arch as things move over.

So far I have some tentative patches which move ARM and x86 over to a
general board setup. The main issues are relocation and SPL.

For relocation I feel that a C implementation makes more sense. Please
scream if you disagree, but I can see no good reason so far for
anything but the very last bit (where the stack pointer is adjusted)
being in assembler. x86 actually does this.

With ARM there are a few functions in its board.c which don't appear
in other archs. Bearing in mind that I want to create a digestible
patch series I am thinking of leaving the function arrays from
arch/xxx/lib/board.c in these architecture-specific files for now, and
just moving the common functions over to the common file. This means
that init_baudrate() will be in the generic board file, for example,
but board_init (which may in fact be mis-named) stays in
arch/arm/lib/board.c. Both will be referenced from the function array
in arch/arm/lib/board.c.

The obvious step after that is to move the function arrays into the
generic code, but I think it is reasonable to do that afterwards since
at present it is full of #ifdefs and will only get worse when archs
merge.

As a later step we can examine some of the odder and more esoteric
functions and see if they can be moved to board files, drivers, etc.
But it is too big a step I think to do this now. This is particularly
true of PowerPC which has a huge amount of stuff in there... Doing
this later also reduces the potential for breakages.

Also as a later step we have Graeme's initcalls. To me this offer
great benefits but they are not absolutely required for this clean up.
Let' s see how things look.

The generic board files therefore become (unless there are objections
to source files in the board/ directory):

board/board_f.c
board/board_r.c
board/reloc.c

with a small architecture-specific relocation function in:

arch/xxx/lib/arch-reloc.c

With this also it would be nice to standardise the link symbols a
little more. Mostly they are the same but there are variations. I plan
to add something like include/asm-generic/link_symbols.h to declare
these for C code in one place. One annoying thing is that people use
.lds files to add additional code (other than start.S) at the start of
the image. It would be nice to have a Makefile variable which collects
these files as the build proceeds instead of putting then in the .lds
file (one less reason to even need a board-specific .lds file).

My plan at present is to send several small patch series over the next
month or two in this sequence:

- ARM .lds clean-up (sent, although I have a few more cleans-ups for
it and it is missing SPL)
- Very simple general relocation framework (with link symbol stuff)
- Generic board support (but with no archs using it)
- Move ARM over and clean up dead code
- Move x86 over
- Move PPC over

The benefits of this effort will be:

- one copy of most of the various functions in arch/xxx/lib/board.c
instead of 10
- reduced assembler and code duplicate in start.S (I am thinking of ARM here)
- naturally bring less-developed archs closer to the functionality of
more-developed

Regards,
Simon

>
> 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
> "My name is Linus Torvalds, you messed with my kernel, prepare to die"
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?- Linus Torvalds in
> ? ? ?<Pine.LNX.3.91.960426110644.24860I-100000@linux.cs.Helsinki.FI>
>

[U-Boot] A bit about board.c, board.c

[Albert ARIBAUD]

Hi Simon,

Le 22/10/2011 18:40, Simon Glass a ?crit :
> Hi Albert,
>
> On Sat, Oct 22, 2011 at 12:17 AM, Albert ARIBAUD
> <albert.u.boot@aribaud.net>  wrote:
>> Le 22/10/2011 07:11, Simon Glass a ?crit :
>>>
>>> Hi,
>>>
>>> Each architecture has its own board.c but they are mostly quite similar.
>>>
>>> New features such as fdt, boot timing, trace, profiling, etc. must be
>>> done separately for each arch, even if there are no
>>> architecture-specific bits.
>>>
>>> What would you say to adding something like lib/board.c which is a
>>> simplified cleaned-up copy of one of the existing board.c files, and a
>>> CONFIG_ARCH_GENERIC_BOARD option to select that in preference to the
>>> architecture-specific one. Then perhaps people could try it out and we
>>> could slowly move to it over time...
>>
>> I'd say anything that factorizes ARM code is a good thing. :)
>>
>> However I'm not in favor of a CONFIG option that would force the board code
>> to provide some functions just in odrer to override one, not if weak
>
> My suggestion was:
>
> CONFIG_ARCH_GENERIC_BOARD - you get lib/board.c
> not CONFIG_ARCH_GENERIC_BOARD - you get arch/xxx/lib/board.c
>
>> definitions can do a finer job. I am looking into that already for cache
>> management function specialization, and it seems like there is the same kind
>> of possibility here, with lib functions defined weak and the board code
>> overriding the ones it deems necessary to.
>
> ok. Just one little point. Weak symbols are the punishment our C++
> overlords gave us for not having to understand what code is executed
> when we instantiate a C++ subclass. It is nice to know what code is
> being linked in and avoid having to disassemble to find out. I think
> using weak symbols to avoid a OBJS-$(CONFIG_ARCH_GENERIC_BOARD) in a
> couple of Makefiles risks going too far.

The problem I see with a config option is that it is all-or-nothing: 
either all cache functions are defined at the lib level, or none is. I 
want a finer-grain, function-by-function, approach. For the moment I 
think weak symbols are a nice solution, although I do recognize that it 
will be less visible than configuration options.

> Regards,
> Simon

Amicalement,
-- 
Albert.

[U-Boot] A bit about board.c, board.c

[Graeme Russ]

Hi Simon,


On 23/10/11 03:36, Simon Glass wrote:
> Hi Graeme,
> 
> Did you mean to send to list?

Oops, yes I did - Done now

> On Fri, Oct 21, 2011 at 10:49 PM, Graeme Russ <graeme.russ@gmail.com> wrote:
>> Hi Simon
>>
>> On Oct 22, 2011 4:11 PM, "Simon Glass" <sjg@chromium.org> wrote:
>>>
>>> Hi,
>>>
>>> Each architecture has its own board.c but they are mostly quite similar.
>>>
>>> New features such as fdt, boot timing, trace, profiling, etc. must be
>>> done separately for each arch, even if there are no
>>> architecture-specific bits.
>>
>> Yes, it is a mess - something my unit sequence patches tried to clean up
>>
>>> What would you say to adding something like lib/board.c which is a
>>> simplified cleaned-up copy of one of the existing board.c files, and a
>>
>> 100% agree
>>
>>> CONFIG_ARCH_GENERIC_BOARD option to select that in preference to the
>>> architecture-specific one. Then perhaps people could try it out and we
>>> could slowly move to it over time...
>>
>> I vote to pick an arch, convert it to a unified style and move it to
>> lib/board.c and then merge each arch over. This should be done in a single
>> series rather than the ol' 'migrate over time' which never happens.
> 
> I thing you mean merge each arch over in its own series?
> 
>>
>> x86 is a good arch to start with because the number of boards is so small
>> (1)
>>
>> Also, I'd personally like the init sequence patches I posted earlier to be
>> re-examined
> 
> I already examined and thought it was good. Let's be careful to keep
> it simple in the sense that we only need a very small number of init
> functions. Most of the board_init_r() code should not be there as I
> understand it (e.g. on ARM MMC, USB, NAND should be inited if/when
> used and not before). Need to be careful not to confuse this bit with
> driver init or any refactor of the driver model. So we have things
> like
> 
> - init memory and make it so we can relocate code, etc. (this is
> called from start.S at present)
> - init the CPU core
> - arch init like turn off caches, MMU, flush TLBs, etc.
> - early board init (hopefully just requires an initcall in board code if needed)
> - the current init sequence like banner, serial, etc.
> - relocate
> - console init
> - board_init (initcall in board code if needed)
> - (hopefully all other post-relocation init can be punted)
> 
> So board.c becomes a few functions and about a dozen initcalls. Albert
> will want to use weak symbols instead of #ifdef, and we will be done.
> 
> Regards,
> Simon
> 
>>
>> Regards,
>>
>> Graeme
>>
> 

[U-Boot] A bit about board.c, board.c

[Graeme Russ]

Hi Simon,

On 23/10/11 08:41, Graeme Russ wrote:
> Hi Simon,
> 
> 
> On 23/10/11 03:36, Simon Glass wrote:
>> Hi Graeme,

[snip]

>>> I vote to pick an arch, convert it to a unified style and move it to
>>> lib/board.c and then merge each arch over. This should be done in a single
>>> series rather than the ol' 'migrate over time' which never happens.
>>
>> I thing you mean merge each arch over in its own series?

I mean merge all arches in one series

>>> x86 is a good arch to start with because the number of boards is so small
>>> (1)
>>>
>>> Also, I'd personally like the init sequence patches I posted earlier to be
>>> re-examined
>>
>> I already examined and thought it was good. Let's be careful to keep
>> it simple in the sense that we only need a very small number of init
>> functions. Most of the board_init_r() code should not be there as I
>> understand it (e.g. on ARM MMC, USB, NAND should be inited if/when
>> used and not before). Need to be careful not to confuse this bit with
>> driver init or any refactor of the driver model. So we have things
>> like

My biggest gripe with the init code is the lack of consistency. There are
two prime examples of this:

1) The pre-relocation sequence and post-relocation sequences can be
implemented using identical code (a sequence loop) - x86 almost gets there,
other arches only loop for one, and have sequential logic for the other
2) Even with the init loop, there is a lot of sequential code after the
loop that could be merged into the loop

>> - init memory and make it so we can relocate code, etc. (this is
>> called from start.S at present)
>> - init the CPU core
>> - arch init like turn off caches, MMU, flush TLBs, etc.
>> - early board init (hopefully just requires an initcall in board code if needed)
>> - the current init sequence like banner, serial, etc.
>> - relocate
>> - console init
>> - board_init (initcall in board code if needed)
>> - (hopefully all other post-relocation init can be punted)
>>
>> So board.c becomes a few functions and about a dozen initcalls. Albert
>> will want to use weak symbols instead of #ifdef, and we will be done.

The INIT_CALL approach eliminated this need - If the .c file does not get
compiled in, the INIT_CALL does not get included. And adding a new
INIT_CALL was as simple as including the macro in the .c file without
needing to mess with board.c at all - simple :)

INIT_CALL is a completely separate issue to what you are looking at and
would be easier to implement after board.c was merged into lib

Regard,

Graeme