Patchwork [U-Boot,1/7] mpc85xx: Support a board-specific processor reset routines

login
register
mail settings
Submitter Kyle Moffett
Date Feb. 21, 2011, 5:59 p.m.
Message ID <1298311199-18775-2-git-send-email-Kyle.D.Moffett@boeing.com>
Download mbox | patch
Permalink /patch/83914/
State Changes Requested
Headers show

Comments

Kyle Moffett - Feb. 21, 2011, 5:59 p.m.
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.Moffett@boeing.com>
---
 arch/powerpc/cpu/mpc85xx/cpu.c |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)
Wolfgang Denk - Feb. 21, 2011, 8:59 p.m.
Dear Kyle Moffett,

In message <1298311199-18775-2-git-send-email-Kyle.D.Moffett@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.Moffett@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.

> @@ -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?


Best regards,

Wolfgang Denk
Kyle Moffett - Feb. 21, 2011, 10:20 p.m.
On Feb 21, 2011, at 15:59, Wolfgang Denk wrote:
> In message <1298311199-18775-2-git-send-email-Kyle.D.Moffett@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.Moffett@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

Patch

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;
 
@@ -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);
 	out_be32(&gur->rstcr, 0x2);	/* HRESET_REQ */
 	udelay(100);