On Feb 21, 2011, at 15:59, Wolfgang Denk wrote:
> In message <1298311199-18775-2-git-send-email-kyle.d.moff...@boeing.com> you
> wrote:
>> Some board models (such as the submitted P2020-based HWW-1U-1A hardware)
>> need specialized code to run when a reset is requested to ensure proper
>> synchronization with other hardware.
>>
>> In order to facilitate such board ports, we add a board_reset_r()
>> routine which is called from the do_reset() command function instead of
>> directly poking at the MPC85xx "RSTCR" (reset control) register.
>>
>> Signed-off-by: Kyle Moffett <kyle.d.moff...@boeing.com>
>> ---
>> arch/powerpc/cpu/mpc85xx/cpu.c | 7 +++++--
>> 1 files changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/powerpc/cpu/mpc85xx/cpu.c b/arch/powerpc/cpu/mpc85xx/cpu.c
>> index 1aad2ba..dbc662f 100644
>> --- a/arch/powerpc/cpu/mpc85xx/cpu.c
>> +++ b/arch/powerpc/cpu/mpc85xx/cpu.c
>> @@ -204,8 +204,10 @@ int checkcpu (void)
>>
>> int do_reset (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>> {
>> -/* Everything after the first generation of PQ3 parts has RSTCR */
>> -#if defined(CONFIG_MPC8540) || defined(CONFIG_MPC8541) || \
>> +#if defined(CONFIG_BOARD_RESET_R)
>> + extern void board_reset_r(void);
>> + board_reset_r();
>> +#elif defined(CONFIG_MPC8540) || defined(CONFIG_MPC8541) || \
>> defined(CONFIG_MPC8555) || defined(CONFIG_MPC8560)
>> unsigned long val, msr;
>
> Please implement without #ifdef's using a weak function - see
> arch/powerpc/cpu/mpc86xx/cpu.c for an example. Actually you might
> want to facto this out into common code.
>
> Side note: don't use ever "extern" function declarations in the code;
> use proper prototype declarations in some header file instead.
Hmm, ok. I thought that looked kind of funny but I copied it from somewhere
else in U-Boot so I figured it was fine.
I'll see if I can't come up with something a little more generic, otherwise
I'll use the weak function approach.
>> @@ -221,6 +223,7 @@ int do_reset (cmd_tbl_t *cmdtp, int flag, int argc, char
>> * const argv[])
>> val |= 0x70000000;
>> mtspr(DBCR0,val);
>> #else
>> + /* Everything after the first generation of PQ3 parts has RSTCR */
>> volatile ccsr_gur_t *gur = (void *)(CONFIG_SYS_MPC85xx_GUTS_ADDR);
>
> Note that this declaration is now basicy in the middle of code, which
> is ugly at best.
>
> What is the "volatile" needed for?
It was always in the middle of code, I just moved the comment. I also didn't
add the volatile, and I have no idea why anybody thought it was needed in the
first place.
I'll fix it in v4. Thanks!
Cheers,
Kyle Moffett
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
