All of lore.kernel.org
 help / color / mirror / Atom feed
From: Graeme Russ <graeme.russ@gmail.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [RFC PATCH 0/19] Create generic board init and move ARM and x86 to it
Date: Sat, 31 Dec 2011 02:49:17 +1100	[thread overview]
Message-ID: <4EFDDD7D.50105@gmail.com> (raw)
In-Reply-To: <1325054160-24894-1-git-send-email-sjg@chromium.org>

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()

On 28/12/11 17:35, Simon Glass wrote:
> 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.

> 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.

> 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

> 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.

This has been raised quite some time ago - The resulting code size increase
is not worth it. x86 can now emulate the 'gd is a register' so gd is now
available and writtable always on every arch. As I said before, gd access
is slightly more expensive (one instruction) and initialising the gd
pointer is _very_ expensive (rebuilding the Global Descriptor Table) but
this only happens three times in total, so I am happy to accept the penalty
for a unified architecture.

> 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.

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 :)

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

> 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

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

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

Regards,

Graeme

  parent reply	other threads:[~2011-12-30 15:49 UTC|newest]

Thread overview: 45+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-12-28  6:35 [U-Boot] [RFC PATCH 0/19] Create generic board init and move ARM and x86 to it Simon Glass
2011-12-28  6:35 ` [U-Boot] [RFC PATCH 01/19] Introduce generic global_data Simon Glass
2011-12-29  7:47   ` Andreas Bießmann
2012-01-07 22:33     ` Simon Glass
2012-01-08 10:48       ` Graeme Russ
2012-01-08 18:13         ` Simon Glass
2012-01-09 11:21           ` Andreas Bießmann
2012-01-09 18:17             ` Simon Glass
2011-12-28  6:35 ` [U-Boot] [RFC PATCH 02/19] Make relocation functions global Simon Glass
2011-12-28  6:35 ` [U-Boot] [RFC PATCH 03/19] Add basic initcall implementation Simon Glass
2011-12-28  6:35 ` [U-Boot] [RFC PATCH 04/19] define CONFIG_SYS_LEGACY_BOARD everywhere Simon Glass
2011-12-28  6:35 ` [U-Boot] [RFC PATCH 05/19] Add generic post-relocation board_r.c Simon Glass
2011-12-28  6:35 ` [U-Boot] [RFC PATCH 06/19] Add generic pre-relocation board_f.c Simon Glass
2011-12-28  6:35 ` [U-Boot] [RFC PATCH 07/19] Add spl load feature Simon Glass
2011-12-28  6:35 ` [U-Boot] [RFC PATCH 08/19] switch ARM over to generic board Simon Glass
2011-12-28  6:35 ` [U-Boot] [RFC PATCH 09/19] arm: Remove unused code in board.c, global_data.h Simon Glass
2011-12-28  6:35 ` [U-Boot] [RFC PATCH 10/19] Add CONFIG_SYS_SYM_OFFSETS to support offset symbols Simon Glass
2011-12-28  6:35 ` [U-Boot] [RFC PATCH 11/19] x86: Remove compiler warning in sc520_timer.c Simon Glass
2011-12-28  6:35 ` [U-Boot] [RFC PATCH 12/19] x86: Remove dead code in eNET Simon Glass
2011-12-28  6:35 ` [U-Boot] [RFC PATCH 13/19] x86: Add processor library and relocation functions Simon Glass
2011-12-28  6:35 ` [U-Boot] [RFC PATCH 14/19] Tidy up asm/generic sections.h to include x86 symbols Simon Glass
2011-12-28  6:35 ` [U-Boot] [RFC PATCH 15/19] Add fields required by x86 to global_data Simon Glass
2011-12-28  6:35 ` [U-Boot] [RFC PATCH 16/19] x86: Change stub example to use asm-generic/sections.h Simon Glass
2011-12-28  6:35 ` [U-Boot] [RFC PATCH 17/19] x86: Add initial memory barrier macros Simon Glass
2011-12-28  6:35 ` [U-Boot] [RFC PATCH 18/19] Bring in x86 to unified board architecture Simon Glass
2011-12-28  6:36 ` [U-Boot] [RFC PATCH 19/19] x86: Remove unused board/global_data code Simon Glass
2011-12-28 17:30 ` [U-Boot] [RFC PATCH 0/19] Create generic board init and move ARM and x86 to it Wolfgang Denk
2011-12-28 18:13   ` Simon Glass
2011-12-30 15:49 ` Graeme Russ [this message]
2011-12-31  0:03   ` Wolfgang Denk
2011-12-31  2:02   ` Simon Glass
2011-12-31 11:52     ` Graeme Russ
2012-01-01 23:48       ` Simon Glass
2012-01-02 11:26         ` Graeme Russ
2012-01-02 14:46           ` Wolfgang Denk
2012-01-03  5:33             ` Simon Glass
2012-01-03  8:12               ` Wolfgang Denk
2012-01-03  9:15                 ` Graeme Russ
2012-01-03 15:55                 ` Simon Glass
2012-01-03 22:35                   ` Wolfgang Denk
2012-01-03 22:52                     ` Simon Glass
2012-01-03  5:28           ` Simon Glass
2012-01-01  1:54     ` Wolfgang Denk
2012-01-01 23:47       ` Simon Glass
2012-01-02  6:50         ` Wolfgang Denk

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4EFDDD7D.50105@gmail.com \
    --to=graeme.russ@gmail.com \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.