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()
>
>
>> 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?
Not sure if I see how this would be better (or worse for that matter).
- k
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
