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

Reply via email to