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
