[U-Boot] Where I'm going with x86 board.c

[Simon Glass]

Hi Graeme,

On Thu, Dec 22, 2011 at 3:27 AM, Graeme Russ <graeme.russ at gmail.com> wrote:
> Hi Simon,
>
> On 22/12/11 17:44, Simon Glass wrote:
>> Hi Graeme,
>>
>> On Tue, Dec 20, 2011 at 4:06 AM, Graeme Russ <graeme.russ at gmail.com> wrote:
>>> With Simon's work on generic relocation support, I thought I would throw in
>>> what I am planning for arch/x86/lib/board.c
>>>
>>> Now this is not a patch, it is a work-in-progress complete version of the
>>> file (compiles, will test soon) - If feedback is positive, I'll add this to
>>> an upcoming patch set
>>
>> Looks good to me+++
>
> Thanks for having a look :)
>
>>> Notice the amount of wrapping around void functions - If all other arch's
>>> follow this lead, most of this wrapping can be removed by changing the
>>> function signatures.
>>>
>>> Lines 428 - 585 are effectively the generic init sequence - The rest is
>>> wrappers, init sequence arrays, or fluff that should be moved
>>>
>>> I noticed something along the way - gd is no longer special... let me
>>> explain...
>>>
>>> Some arch's use a dedicated register for the gd pointer - This allows the
>>> pointer to be written to prior to relocation. For x86, the gd pointer is
>>> simply passed around as a function parameter early - If the init_sequence_f
>>> functions accepted a gd pointer as a parameter, there would be no need for
>>> it to be global prior to relocation and therefore no need to allocate a
>>> permanent register for it - Of course do_init_loop() would no longer be
>>> generic for both pre and post relocation. This does mess with the
>>> stand-alone API, but as discussed before, stand alone applications should
>>> not be accessing gd anyway, so there should be no API to break ;)
>>
>> Actually as it happens I did a bit of an experiment with this some
>> weeks ago and my original board stuff had gd as a parameter for the
>> pre-reloc functions (some with stubs to other ones like
>> console_init_f()). On ARM it really doesn't make a lot of sense to use
>> a global variable instead of a parameter. I decided that it was a bit
>> much to bite off in one go :-) Partly that was because IMO gd really
>> only makes sense prior to relocation - for the post-relocation init
>> calls they don't have a lot of need for gd.
>
> I don't think this is 100% correct - I'm sure gd is used all over the place
> post init

Not enough that it needs to be in its own register. This compile-time
decision affects the whole of U-Boot post-relocation. In some routines
there is a small code size / performance advantage to having r8
available.

Also it is a bit of a misnomer when used after relocation. There is
lots of global data (the whole BSS) not just one structure. Arguably
some things like the network code would be smaller/faster using
global_data and without total reliance on global variables, but anyway
it seems to me that gd is mostly a convenience for pre-relocation
where memory may not be available.

>
>> So it has my vote. Once I get somewhere on the reboard series I will
>> post my common/board.c. As I may have mentioned I have elected so far
>> to leave the initcall list and local functions in
>> arch/xxx/lib/board.c.
>>
>> Would be good to get all this moving.
>
> Well, I have a more thorough patch in the making - I creates
> init_wrappers.c and init_helpers.c and board.c collapses very dramatically
> (see below) - with a lot of #ifdef hell...

I thing that all pre-reloc functions that are called should have the
same signature - i.e. return an int. if we (later) move to passing gd
around then they would change in that way also.

>
> Notice the new board_init_f_r() - This is where U-Boot is running from
> Flash, but SDRAM has been initialised and the stack is now in RAM - So this
> is where we move gd to RAM and perform relocation. As a bonus, relocation
> can be done with cache enabled :)

Yes I see. It is not so easy on ARM since we need the MMU also, but I
think it can be done. Once data is set up / copied in its new location
we can use it even prior to relocation. I also recently created a
patch to delay console init until after relocation (cue screams on the
list) which saves time.

Regards,
Simon

>
> Regards,
>
> Graeme
>
>
>
> /*
>  * (C) Copyright 2008-2011
>  * Graeme Russ, <graeme.russ at gmail.com>
>  *
>  * (C) Copyright 2002
>  * Daniel Engström, Omicron Ceti AB, <daniel at omicron.se>
>  *
>  * (C) Copyright 2002
>  * Wolfgang Denk, DENX Software Engineering, <wd at denx.de>
>  *
>  * (C) Copyright 2002
>  * Sysgo Real-Time Solutions, GmbH <www.elinos.com>
>  * Marius Groeger <mgroeger at sysgo.de>
>  *
>  * See file CREDITS for list of people who contributed to this
>  * project.
>  *
>  * This program is free software; you can redistribute it and/or
>  * modify it under the terms of the GNU General Public License as
>  * published by the Free Software Foundation; either version 2 of
>  * the License, or (at your option) any later version.
>  *
>  * This program is distributed in the hope that it will be useful,
>  * but WITHOUT ANY WARRANTY; without even the implied warranty of
>  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>  * GNU General Public License for more details.
>  *
>  * You should have received a copy of the GNU General Public License
>  * along with this program; if not, write to the Free Software
>  * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
>  * MA 02111-1307 USA
>  */
>
> #include <common.h>
> #include <watchdog.h>
> #include <stdio_dev.h>
> #include <asm/u-boot-x86.h>
>
> #include <asm/init_helpers.h>
> #include <asm/init_wrappers.h>
>
> /*
>  * Pointer to initial global data area
>  *
>  * Here we initialize it.
>  */
> #undef  XTRN_DECLARE_GLOBAL_DATA_PTR
> #define XTRN_DECLARE_GLOBAL_DATA_PTR    /* empty = allocate here */
> DECLARE_GLOBAL_DATA_PTR = (gd_t *) (CONFIG_SYS_INIT_GD_ADDR);
>
> /*
>  * Breath some life into the board...
>  *
>  * Initialize an SMC for serial comms, and carry out some hardware
>  * tests.
>  *
>  * The first part of initialization is running from Flash memory;
>  * its main purpose is to initialize the RAM so that we
>  * can relocate the monitor code to RAM.
>  */
>
> /*
>  * All attempts to come up with a "common" initialization sequence
>  * that works for all boards and architectures failed: some of the
>  * requirements are just _too_ different. To get rid of the resulting
>  * mess of board dependend #ifdef'ed code we now make the whole
>  * initialization sequence configurable to the user.
>  *
>  * The requirements for any new initalization function is simple: it
>  * receives a pointer to the "global data" structure as it's only
>  * argument, and returns an integer return code, where 0 means
>  * "continue" and != 0 means "fatal error, hang the system".
>  */
> typedef int (init_fnc_t) (void);
>
> init_fnc_t *init_sequence_f[] = {
>        cpu_init_f,
>        board_early_init_f,
>        env_init,
>        init_baudrate_f,
>        serial_init,
>        console_init_f,
>        dram_init_f,
>
>        NULL,
> };
>
> init_fnc_t *init_sequence_r[] = {
>        init_bd_struct_r,
>        mem_malloc_init_r,
>        cpu_init_r,
>        board_early_init_r,
>        dram_init,
>        interrupt_init,
>        timer_init,
>        display_banner,
>        display_dram_config,
> #ifdef CONFIG_SERIAL_MULTI
>        serial_initialize_r,
> #endif
> #ifndef CONFIG_SYS_NO_FLASH
>        flash_init_r,
> #endif
>        env_relocate_r,
> #ifdef CONFIG_CMD_NET
>        init_ip_address_r,
> #endif
> #ifdef CONFIG_PCI
>        pci_init_r,
> #endif
>        stdio_init,
>        jumptable_init_r,
>        console_init_r,
> #ifdef CONFIG_MISC_INIT_R
>        misc_init_r,
> #endif
> #if defined(CONFIG_CMD_PCMCIA) && !defined(CONFIG_CMD_IDE)
>        pci_init_r,
> #endif
> #if defined(CONFIG_CMD_KGDB)
>        kgdb_init_r,
> #endif
>        enable_interrupts_r,
> #ifdef CONFIG_STATUS_LED
>        status_led_set_r,
> #endif
>        set_load_addr_r,
> #if defined(CONFIG_CMD_NET)
>        set_bootfile_r,
> #endif
> #if defined(CONFIG_CMD_IDE)
>        ide_init_r,
> #endif
> #if defined(CONFIG_CMD_SCSI)
>        scsi_init_r,
> #endif
> #if defined(CONFIG_CMD_DOC)
>        doc_init_r,
> #endif
> #ifdef CONFIG_BITBANGMII
>        bb_miiphy_init_r,
> #endif
> #if defined(CONFIG_CMD_NET)
>        eth_initialize_r,
> #ifdef CONFIG_RESET_PHY_R
>        reset_phy_r,
> #endif
> #endif
> #ifdef CONFIG_LAST_STAGE_INIT
>        last_stage_init,
> #endif
>        NULL,
> };
>
> static void do_init_loop(init_fnc_t **init_fnc_ptr)
> {
>        for (; *init_fnc_ptr; ++init_fnc_ptr) {
>                WATCHDOG_RESET();
>                if ((*init_fnc_ptr)() != 0)
>                        hang();
>        }
> }
>
> /* Perform all steps necessary to get RAM initialised ready for relocation */
> void board_init_f(ulong boot_flags)
> {
>        gd->flags = boot_flags;
>
>        do_init_loop(init_sequence_f);
>
>        /*
>         * SDRAM is now initialised setup a new stack in SDRAM
>         *
>         * Code execution will continue in Flash, but with the stack
>         * in SDRAM. This allows us to copy global data out of the CPU
>         * cache prior to copying U-Boot into RAM which means we can
>         * enable caching for the copy operation (which speeds it up
>         * considerably)
>         */
>        move_stack_to_sdram(gd->ram_size);
>
>        /* NOTREACHED - move_stack_to_sdram() does not return */
>        while (1)
>                ;
> }
>
> void board_init_f_r(gd_t *id)
> {
>        /*
>         * U-Boot is running from flash, but the stack is now in SDRAM
>         * (id = top of stack = final location of global data)
>         */
>
>        /*
>         * gd is still in CAR - Copy it into SDRAM
>         *
>         * NOTE: We cannot set the gd variable yet as it lives in .data
>         * which has not been relocated to RAM yet
>         */
>        memcpy(id, gd, sizeof(gd_t));
>
>        /* Initialise the CPU cache(s) */
>        if (init_cache() != 0)
>                hang();
>
>        /* Copy U-Boot to RAM and resume code execution in RAM */
>        relocate_code(0, id, 0);
>
>        /* NOTREACHED - relocate_code() does not return */
>        while (1)
>                ;
> }
>
> __attribute__ ((__noreturn__))
> void board_init_r(gd_t *id, ulong dest_addr)
> {
>        /* Global data pointer is now writable */
>        gd = id;
>
>        gd->flags |= GD_FLG_RELOC;
>
>        /* compiler optimization barrier needed for GCC >= 3.4 */
>        __asm__ __volatile__("" : : : "memory");
>
>        do_init_loop(init_sequence_r);
>
>        /* main_loop() can return to retry autoboot, if so just run it again. */
>        for (;;)
>                main_loop();
>
>        /* NOTREACHED - no way out of command loop except booting */
> }
>
> __attribute__ ((__noreturn__))
> void hang(void)
> {
>        puts("### ERROR ### Please RESET the board ###\n");
>        for (;;)
>                ;
> }