Hi Graeme,
On Sun, Jan 1, 2012 at 8:09 PM, Graeme Russ <graeme.r...@gmail.com> wrote:
>
> Signed-off-by: Graeme Russ <graeme.r...@gmail.com>
> ---
> arch/x86/include/asm/init_helpers.h | 39 +++++
> arch/x86/include/asm/init_wrappers.h | 42 +++++
> arch/x86/lib/Makefile | 2 +
> arch/x86/lib/board.c | 297
> +++++++++-------------------------
> arch/x86/lib/init_helpers.c | 142 ++++++++++++++++
> arch/x86/lib/init_wrappers.c | 137 ++++++++++++++++
What is the rationale for putting these into separate files? If you
didn't then they would be static and presumably less code size.
Also for the commit message, is this really a simplification? It looks
more like you are splitting the code into separate functions.
Regards,
Simon
> 6 files changed, 438 insertions(+), 221 deletions(-)
> create mode 100644 arch/x86/include/asm/init_helpers.h
> create mode 100644 arch/x86/include/asm/init_wrappers.h
> create mode 100644 arch/x86/lib/init_helpers.c
> create mode 100644 arch/x86/lib/init_wrappers.c
>
> diff --git a/arch/x86/include/asm/init_helpers.h
> b/arch/x86/include/asm/init_helpers.h
> new file mode 100644
> index 0000000..14ef11a
> --- /dev/null
> +++ b/arch/x86/include/asm/init_helpers.h
> @@ -0,0 +1,39 @@
> +/*
> + * (C) Copyright 2011
> + * Graeme Russ, <graeme.r...@gmail.com>
> + *
> + * 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
> + */
> +
> +#ifndef _INIT_HELPERS_H_
> +#define _INIT_HELPERS_H_
> +
> +int display_banner(void);
> +int display_dram_config(void);
> +int init_baudrate_f(void);
> +
> +int mem_malloc_init_r(void);
> +int init_bd_struct_r(void);
> +int flash_init_r(void);
> +int init_ip_address_r(void);
> +int status_led_set_r(void);
> +int set_bootfile_r(void);
> +int set_load_addr_r(void);
> +
> +#endif /* !_INIT_HELPERS_H_ */
> diff --git a/arch/x86/include/asm/init_wrappers.h
> b/arch/x86/include/asm/init_wrappers.h
> new file mode 100644
> index 0000000..899ffb1
> --- /dev/null
> +++ b/arch/x86/include/asm/init_wrappers.h
> @@ -0,0 +1,42 @@
> +/*
> + * (C) Copyright 2011
> + * Graeme Russ, <graeme.r...@gmail.com>
> + *
> + * 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
> + */
> +
> +#ifndef _INIT_WRAPPERS_H_
> +#define _INIT_WRAPPERS_H_
> +
> +int serial_initialize_r(void);
> +int env_relocate_r(void);
> +int pci_init_r(void);
> +int jumptable_init_r(void);
> +int pcmcia_init_r(void);
> +int kgdb_init_r(void);
> +int enable_interrupts_r(void);
> +int eth_initialize_r(void);
> +int reset_phy_r(void);
> +int ide_init_r(void);
> +int scsi_init_r(void);
> +int doc_init_r(void);
> +int bb_miiphy_init_r(void);
> +int post_run_r(void);
> +
> +#endif /* !_INIT_WRAPPERS_H_ */
> diff --git a/arch/x86/lib/Makefile b/arch/x86/lib/Makefile
> index 57b6896..51836da 100644
> --- a/arch/x86/lib/Makefile
> +++ b/arch/x86/lib/Makefile
> @@ -34,6 +34,8 @@ COBJS-y += board.o
> COBJS-y += bootm.o
> COBJS-y += cmd_boot.o
> COBJS-y += gcc.o
> +COBJS-y += init_helpers.o
> +COBJS-y += init_wrappers.o
> COBJS-y += interrupts.o
> COBJS-$(CONFIG_SYS_PCAT_INTERRUPTS) += pcat_interrupts.o
> COBJS-$(CONFIG_SYS_GENERIC_TIMER) += pcat_timer.o
> diff --git a/arch/x86/lib/board.c b/arch/x86/lib/board.c
> index fc7d0b7..f0a8e9c 100644
> --- a/arch/x86/lib/board.c
> +++ b/arch/x86/lib/board.c
> @@ -33,62 +33,12 @@
>
> #include <common.h>
> #include <watchdog.h>
> -#include <command.h>
> #include <stdio_dev.h>
> -#include <version.h>
> -#include <malloc.h>
> -#include <net.h>
> -#include <ide.h>
> -#include <serial.h>
> #include <asm/u-boot-x86.h>
> #include <asm/processor.h>
>
> -#ifdef CONFIG_BITBANGMII
> -#include <miiphy.h>
> -#endif
> -
> -/************************************************************************
> - * Init Utilities *
> - ************************************************************************
> - * Some of this code should be moved into the core functions,
> - * or dropped completely,
> - * but let's get it working (again) first...
> - */
> -static int init_baudrate(void)
> -{
> - gd->baudrate = getenv_ulong("baudrate", 10, CONFIG_BAUDRATE);
> - return 0;
> -}
> -
> -static int display_banner(void)
> -{
> -
> - printf("\n\n%s\n\n", version_string);
> -
> - return 0;
> -}
> -
> -static int display_dram_config(void)
> -{
> - int i;
> -
> - puts("DRAM Configuration:\n");
> -
> - for (i = 0; i < CONFIG_NR_DRAM_BANKS; i++) {
> - printf("Bank #%d: %08lx ", i, gd->bd->bi_dram[i].start);
> - print_size(gd->bd->bi_dram[i].size, "\n");
> - }
> -
> - return 0;
> -}
> -
> -#ifndef CONFIG_SYS_NO_FLASH
> -static void display_flash_config(ulong size)
> -{
> - puts("Flash: ");
> - print_size(size, "\n");
> -}
> -#endif
> +#include <asm/init_helpers.h>
> +#include <asm/init_wrappers.h>
>
> /*
> * Breath some life into the board...
> @@ -122,7 +72,7 @@ init_fnc_t *init_sequence_f[] = {
> cpu_init_f,
> board_early_init_f,
> env_init,
> - init_baudrate,
> + init_baudrate_f,
> serial_init,
> console_init_f,
> dram_init_f,
> @@ -132,17 +82,81 @@ init_fnc_t *init_sequence_f[] = {
> };
>
> init_fnc_t *init_sequence_r[] = {
> - cpu_init_r, /* basic cpu dependent setup */
> - board_early_init_r, /* basic board dependent setup */
> - dram_init, /* configure available RAM banks */
> - interrupt_init, /* set up exceptions */
> + 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();
> + }
> +}
> +
> static int calculate_relocation_address(void)
> {
> ulong text_start = (ulong)&__text_start;
> @@ -179,17 +193,12 @@ static int calculate_relocation_address(void)
> return 0;
> }
>
> -/* Load U-Boot into RAM, initialize BSS, perform relocation adjustments */
> +/* Perform all steps necessary to get RAM initialised ready for relocation */
> void board_init_f(ulong boot_flags)
> {
> - init_fnc_t **init_fnc_ptr;
> -
> gd->flags = boot_flags;
>
> - for (init_fnc_ptr = init_sequence_f; *init_fnc_ptr; ++init_fnc_ptr) {
> - if ((*init_fnc_ptr)() != 0)
> - hang();
> - }
> + do_init_loop(init_sequence_f);
>
> /*
> * SDRAM is now initialised, U-Boot has been copied into SDRAM,
> @@ -247,166 +256,12 @@ static int copy_gd_to_ram(void)
>
> void board_init_r(gd_t *id, ulong dest_addr)
> {
> -#if defined(CONFIG_CMD_NET)
> - char *s;
> -#endif
> -#ifndef CONFIG_SYS_NO_FLASH
> - ulong size;
> -#endif
> - static bd_t bd_data;
> - init_fnc_t **init_fnc_ptr;
> -
> - show_boot_progress(0x21);
> + gd->flags |= GD_FLG_RELOC;
>
> /* compiler optimization barrier needed for GCC >= 3.4 */
> __asm__ __volatile__("" : : : "memory");
>
> - gd->flags |= GD_FLG_RELOC;
> -
> - gd->bd = &bd_data;
> - memset(gd->bd, 0, sizeof(bd_t));
> - show_boot_progress(0x22);
> -
> - gd->baudrate = CONFIG_BAUDRATE;
> -
> - mem_malloc_init((((ulong)dest_addr - CONFIG_SYS_MALLOC_LEN)+3)&~3,
> - CONFIG_SYS_MALLOC_LEN);
> -
> - for (init_fnc_ptr = init_sequence_r; *init_fnc_ptr; ++init_fnc_ptr) {
> - if ((*init_fnc_ptr)() != 0)
> - hang();
> - }
> - show_boot_progress(0x23);
> -
> -#ifdef CONFIG_SERIAL_MULTI
> - serial_initialize();
> -#endif
> -
> -#ifndef CONFIG_SYS_NO_FLASH
> - /* configure available FLASH banks */
> - size = flash_init();
> - display_flash_config(size);
> - show_boot_progress(0x24);
> -#endif
> -
> - show_boot_progress(0x25);
> -
> - /* initialize environment */
> - env_relocate();
> - show_boot_progress(0x26);
> -
> -
> -#ifdef CONFIG_CMD_NET
> - /* IP Address */
> - bd_data.bi_ip_addr = getenv_IPaddr("ipaddr");
> -#endif
> -
> -#if defined(CONFIG_PCI)
> - /*
> - * Do pci configuration
> - */
> - pci_init();
> -#endif
> -
> - show_boot_progress(0x27);
> -
> -
> - stdio_init();
> -
> - jumptable_init();
> -
> - /* Initialize the console (after the relocation and devices init) */
> - console_init_r();
> -
> -#ifdef CONFIG_MISC_INIT_R
> - /* miscellaneous platform dependent initialisations */
> - misc_init_r();
> -#endif
> -
> -#if defined(CONFIG_CMD_PCMCIA) && !defined(CONFIG_CMD_IDE)
> - WATCHDOG_RESET();
> - puts("PCMCIA:");
> - pcmcia_init();
> -#endif
> -
> -#if defined(CONFIG_CMD_KGDB)
> - WATCHDOG_RESET();
> - puts("KGDB: ");
> - kgdb_init();
> -#endif
> -
> - /* enable exceptions */
> - enable_interrupts();
> - show_boot_progress(0x28);
> -
> -#ifdef CONFIG_STATUS_LED
> - status_led_set(STATUS_LED_BOOT, STATUS_LED_BLINKING);
> -#endif
> -
> - udelay(20);
> -
> - /* Initialize from environment */
> - load_addr = getenv_ulong("loadaddr", 16, load_addr);
> -#if defined(CONFIG_CMD_NET)
> - s = getenv("bootfile");
> -
> - if (s != NULL)
> - copy_filename(BootFile, s, sizeof(BootFile));
> -#endif
> -
> - WATCHDOG_RESET();
> -
> -#if defined(CONFIG_CMD_IDE)
> - WATCHDOG_RESET();
> - puts("IDE: ");
> - ide_init();
> -#endif
> -
> -#if defined(CONFIG_CMD_SCSI)
> - WATCHDOG_RESET();
> - puts("SCSI: ");
> - scsi_init();
> -#endif
> -
> -#if defined(CONFIG_CMD_DOC)
> - WATCHDOG_RESET();
> - puts("DOC: ");
> - doc_init();
> -#endif
> -
> -#ifdef CONFIG_BITBANGMII
> - bb_miiphy_init();
> -#endif
> -#if defined(CONFIG_CMD_NET)
> - WATCHDOG_RESET();
> - puts("Net: ");
> - eth_initialize(gd->bd);
> -#endif
> -
> -#if (defined(CONFIG_CMD_NET)) && (0)
> - WATCHDOG_RESET();
> -# ifdef DEBUG
> - puts("Reset Ethernet PHY\n");
> -# endif
> - reset_phy();
> -#endif
> -
> -#ifdef CONFIG_LAST_STAGE_INIT
> - WATCHDOG_RESET();
> - /*
> - * Some parts can be only initialized if all others (like
> - * Interrupts) are up and running (i.e. the PC-style ISA
> - * keyboard).
> - */
> - last_stage_init();
> -#endif
> -
> -
> -#ifdef CONFIG_POST
> - post_run(NULL, POST_RAM | post_bootmode_get(0));
> -#endif
> -
> - show_boot_progress(0x29);
> + do_init_loop(init_sequence_r);
>
> /* main_loop() can return to retry autoboot, if so just run it again.
> */
> for (;;)
> diff --git a/arch/x86/lib/init_helpers.c b/arch/x86/lib/init_helpers.c
> new file mode 100644
> index 0000000..547b180
> --- /dev/null
> +++ b/arch/x86/lib/init_helpers.c
> @@ -0,0 +1,142 @@
> +/*
> + * (C) Copyright 2011
> + * Graeme Russ, <graeme.r...@gmail.com>
> + *
> + * 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 <command.h>
> +#include <stdio_dev.h>
> +#include <version.h>
> +#include <malloc.h>
> +#include <net.h>
> +#include <ide.h>
> +#include <serial.h>
> +#include <status_led.h>
> +#include <asm/u-boot-x86.h>
> +
> +#include <asm/init_helpers.h>
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +/************************************************************************
> + * Init Utilities *
> + ************************************************************************
> + * Some of this code should be moved into the core functions,
> + * or dropped completely,
> + * but let's get it working (again) first...
> + */
> +
> +int display_banner(void)
> +{
> + printf("\n\n%s\n\n", version_string);
> +
> + return 0;
> +}
> +
> +int display_dram_config(void)
> +{
> + int i;
> +
> + puts("DRAM Configuration:\n");
> +
> + for (i = 0; i < CONFIG_NR_DRAM_BANKS; i++) {
> + printf("Bank #%d: %08lx ", i, gd->bd->bi_dram[i].start);
> + print_size(gd->bd->bi_dram[i].size, "\n");
> + }
> +
> + return 0;
> +}
> +
> +int init_baudrate_f(void)
> +{
> + gd->baudrate = getenv_ulong("baudrate", 10, CONFIG_BAUDRATE);
> + return 0;
> +}
> +
> +int mem_malloc_init_r(void)
> +{
> + mem_malloc_init(((gd->relocaddr - CONFIG_SYS_MALLOC_LEN)+3)&~3,
> + CONFIG_SYS_MALLOC_LEN);
> +
> + return 0;
> +}
> +
> +bd_t bd_data;
> +
> +int init_bd_struct_r(void)
> +{
> + gd->bd = &bd_data;
> + memset(gd->bd, 0, sizeof(bd_t));
> +
> + return 0;
> +}
> +
> +#ifndef CONFIG_SYS_NO_FLASH
> +int flash_init_r(void)
> +{
> + ulong size;
> +
> + puts("Flash: ");
> +
> + /* configure available FLASH banks */
> + size = flash_init();
> +
> + print_size(size, "\n");
> +
> + return 0;
> +}
> +#endif
> +
> +int init_ip_address_r(void)
> +{
> + /* IP Address */
> + bd_data.bi_ip_addr = getenv_IPaddr("ipaddr");
> +
> + return 0;
> +}
> +
> +#ifdef CONFIG_STATUS_LED
> +int status_led_set_r(void)
> +{
> + status_led_set(STATUS_LED_BOOT, STATUS_LED_BLINKING);
> +
> + return 0;
> +}
> +#endif
> +
> +int set_bootfile_r(void)
> +{
> + char *s;
> +
> + s = getenv("bootfile");
> +
> + if (s != NULL)
> + copy_filename(BootFile, s, sizeof(BootFile));
> +
> + return 0;
> +}
> +
> +int set_load_addr_r(void)
> +{
> + /* Initialize from environment */
> + load_addr = getenv_ulong("loadaddr", 16, load_addr);
> +
> + return 0;
> +}
> diff --git a/arch/x86/lib/init_wrappers.c b/arch/x86/lib/init_wrappers.c
> new file mode 100644
> index 0000000..71449fe
> --- /dev/null
> +++ b/arch/x86/lib/init_wrappers.c
> @@ -0,0 +1,137 @@
> +/*
> + * (C) Copyright 2011
> + * Graeme Russ, <graeme.r...@gmail.com>
> + *
> + * 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 <serial.h>
> +#include <kgdb.h>
> +#include <scsi.h>
> +#include <post.h>
> +#include <miiphy.h>
> +
> +#include <asm/init_wrappers.h>
> +
> +int serial_initialize_r(void)
> +{
> + serial_initialize();
> +
> + return 0;
> +}
> +
> +int env_relocate_r(void)
> +{
> + /* initialize environment */
> + env_relocate();
> +
> + return 0;
> +}
> +
> +
> +int pci_init_r(void)
> +{
> + /* Do pci configuration */
> + pci_init();
> +
> + return 0;
> +}
> +
> +int jumptable_init_r(void)
> +{
> + jumptable_init();
> +
> + return 0;
> +}
> +
> +int pcmcia_init_r(void)
> +{
> + puts("PCMCIA:");
> + pcmcia_init();
> +
> + return 0;
> +}
> +
> +int kgdb_init_r(void)
> +{
> + puts("KGDB: ");
> + kgdb_init();
> +
> + return 0;
> +}
> +
> +int enable_interrupts_r(void)
> +{
> + /* enable exceptions */
> + enable_interrupts();
> +
> + return 0;
> +}
> +
> +int eth_initialize_r(void)
> +{
> + puts("Net: ");
> + eth_initialize(gd->bd);
> +
> + return 0;
> +}
> +
> +int reset_phy_r(void)
> +{
> +#ifdef DEBUG
> + puts("Reset Ethernet PHY\n");
> +#endif
> + reset_phy();
> +
> + return 0;
> +}
> +
> +int ide_init_r(void)
> +{
> + puts("IDE: ");
> + ide_init();
> +
> + return 0;
> +}
> +
> +int scsi_init_r(void)
> +{
> + puts("SCSI: ");
> + scsi_init();
> +
> + return 0;
> +}
> +
> +#ifdef CONFIG_BITBANGMII
> +int bb_miiphy_init_r(void)
> +{
> + bb_miiphy_init();
> +
> + return 0;
> +}
> +#endif
> +
> +#ifdef CONFIG_POST
> +int post_run_r(void)
> +{
> + post_run(NULL, POST_RAM | post_bootmode_get(0));
> +
> + return 0;
> +}
> +#endif
> --
> 1.7.5.2.317.g391b14
>
> _______________________________________________
> U-Boot mailing list
> U-Boot@lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
