From: Wolfgang Denk <wd@denx.de>
Date: Wed, 26 Aug 2009 22:46:43 +0200
Subject: [U-Boot] [PATCH] Add ability for arch code to make changes
	before we boot
In-Reply-To: <1250276442-28463-1-git-send-email-galak@kernel.crashing.org>
References: <1250276442-28463-1-git-send-email-galak@kernel.crashing.org>
Message-ID: <20090826204643.D808C833DBD2@gemini.denx.de>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

Dear Kumar Gala,

In message <1250276442-28463-1-git-send-email-galak@kernel.crashing.org> you wrote:
> Added a arch_preboot() function that cpu specific code can implement to
> allow for various modifications to the state of the machine right before
> we boot.  This can be useful to setup register state to a specific
> configuration.
> 
> Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
> ---
>  common/cmd_bootm.c |   10 ++++++++++
>  1 files changed, 10 insertions(+), 0 deletions(-)
> 
> diff --git a/common/cmd_bootm.c b/common/cmd_bootm.c
> index 86c8122..766889a 100644
> --- a/common/cmd_bootm.c
> +++ b/common/cmd_bootm.c
> @@ -166,6 +166,13 @@ void __arch_lmb_reserve(struct lmb *lmb)
>  }
>  void arch_lmb_reserve(struct lmb *lmb) __attribute__((weak, alias("__arch_lmb_reserve")));
>  
> +/* Allow for arch specific config before we boot */
> +void __arch_preboot(void)

As this is only used when booting an OS (and not for example when
starting a standalone program) we should probably write:

	/* Allow for arch specific code before booting the OS */
	void __arch_preboot_os()
	...

> +{
> +	/* please define platform specific arch_preboot() */
> +}
> +void arch_preboot(void) __attribute__((weak, alias("__arch_preboot")));
> +
>  #if defined(__ARM__)
>    #define IH_INITRD_ARCH IH_ARCH_ARM
>  #elif defined(__avr32__)
> @@ -543,6 +550,7 @@ int do_bootm_subcommand (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
>  			break;
>  		case BOOTM_STATE_OS_GO:

Hm... maybe this could / should be handled in BOOTM_STATE_OS_PREP
state? 

>  			disable_interrupts();
> +			arch_preboot();

And maybe we should mode disable_interrupts() into the default
implementation of arch_preboot_os() ?

>  			boot_fn(BOOTM_STATE_OS_GO, argc, argv, &images);
>  			break;
>  	}
> @@ -673,6 +681,8 @@ int do_bootm (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
>  		return 1;
>  	}
>  
> +	arch_preboot();
> +
>  	boot_fn(0, argc, argv, &images);

But this would change behaviour here. Eventually to the better?

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
Niklaus Wirth has lamented that, whereas Europeans pronounce his name
correctly  (Ni-klows  Virt),  Americans  invariably  mangle  it  into
(Nick-les  Worth).  Which  is to say that Europeans call him by name,
but Americans call him by value.