[U-Boot] [PATCH] Add ability for arch code to make changes before we boot

[Kumar Gala]

On Aug 26, 2009, at 3:46 PM, Wolfgang Denk wrote:

> Dear Kumar Gala,
>
> In message <1250276442-28463-1-git-send-email-galak at 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 at 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