On Aug 26, 2009, at 10:27 PM, Kumar Gala wrote: > > On Aug 26, 2009, at 3:46 PM, Wolfgang Denk wrote: > >> Dear Kumar Gala, >> >> In message <1250276442-28463-1-git-send-email-ga...@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 <ga...@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() >> ... > > no problem, will change the name. > >>> +{ >>> + /* 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? > > I'd prefer to keep arch_preboot_os() as close to boot_fn() as possible > >>> disable_interrupts(); >>> + arch_preboot(); >> >> And maybe we should mode disable_interrupts() into the default >> implementation of arch_preboot_os() ? > > Got no issue with moving disable_interrupts into the default > implementation of arch_preboot_os()
Now that I look at how do_bootm() calls and uses disable_interrupts() this is a bit more tricky. I'd prefer to leave disable_interrupts() alone. - k _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot