[PATCH v2]: Fix e500 v2 core reboot bug

[PATCH v2]: Fix e500 v2 core reboot bug

[Zang Roy-r61911]

Fix the e500 v2 core reset bug.
For e500 v2 core, a new reset control register is added to
reset the core.
On 85xx CDS board with e500 v2 core, normal reboot code will 
induce DDR block in u-boot. This patch fixes this bug. It is 
also tested on legacy e500 v1 core. 


Signed-off-by: Roy Zang <tie-fei.zang@freescale.com>
---

 arch/powerpc/platforms/85xx/misc.c |   18 ++++++++++++++++++
 1 files changed, 18 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/platforms/85xx/misc.c b/arch/powerpc/platforms/85xx/misc.c
index 3e62fcb..886a13f 100644
--- a/arch/powerpc/platforms/85xx/misc.c
+++ b/arch/powerpc/platforms/85xx/misc.c
@@ -13,11 +13,29 @@
 #include <linux/irq.h>
 #include <linux/module.h>
 #include <asm/irq.h>
+#include <asm/io.h>
+#include <sysdev/fsl_soc.h>
+
+static __be32 __iomem *rstcr;
 
 extern void abort(void);
 
+static int __init mpc85xx_rstcr(void)
+{
+	/* map reset control register */
+	rstcr = ioremap(get_immrbase() + 0xE00B0, 0xff);
+	return 0;
+}
+
+arch_initcall(mpc85xx_rstcr);
+
 void mpc85xx_restart(char *cmd)
 {
 	local_irq_disable();
+	if (rstcr)
+		/* set reset control register */
+		out_be32(rstcr, 0x2);	/* HRESET_REQ */
+	else
+		printk (KERN_EMERG "Error: reset control register not mapped, spinning!\n");
 	abort();
 }
-- 
1.5.1

Re: [PATCH v2]: Fix e500 v2 core reboot bug

[Andy Fleming]

On May 28, 2007, at 21:36, Zang Roy-r61911 wrote:

> Fix the e500 v2 core reset bug.
> For e500 v2 core, a new reset control register is added to
> reset the core.
> On 85xx CDS board with e500 v2 core, normal reboot code will
> induce DDR block in u-boot. This patch fixes this bug. It is
> also tested on legacy e500 v1 core.
>
>
> Signed-off-by: Roy Zang <tie-fei.zang@freescale.com>

Acked-by: Andy Fleming <afleming@freescale.com>

Re: [PATCH v2]: Fix e500 v2 core reboot bug

[Kumar Gala]

On May 28, 2007, at 9:36 PM, Zang Roy-r61911 wrote:

> Fix the e500 v2 core reset bug.
> For e500 v2 core, a new reset control register is added to
> reset the core.
> On 85xx CDS board with e500 v2 core, normal reboot code will
> induce DDR block in u-boot. This patch fixes this bug. It is
> also tested on legacy e500 v1 core.

what happens on an e500 based 85xx system?

I'm not terrible happy with blindly writing to rstcr.

- k

>
>
> Signed-off-by: Roy Zang <tie-fei.zang@freescale.com>
> ---
>
>  arch/powerpc/platforms/85xx/misc.c |   18 ++++++++++++++++++
>  1 files changed, 18 insertions(+), 0 deletions(-)
>
> diff --git a/arch/powerpc/platforms/85xx/misc.c b/arch/powerpc/ 
> platforms/85xx/misc.c
> index 3e62fcb..886a13f 100644
> --- a/arch/powerpc/platforms/85xx/misc.c
> +++ b/arch/powerpc/platforms/85xx/misc.c
> @@ -13,11 +13,29 @@
>  #include <linux/irq.h>
>  #include <linux/module.h>
>  #include <asm/irq.h>
> +#include <asm/io.h>
> +#include <sysdev/fsl_soc.h>
> +
> +static __be32 __iomem *rstcr;
>
>  extern void abort(void);
>
> +static int __init mpc85xx_rstcr(void)
> +{
> +	/* map reset control register */
> +	rstcr = ioremap(get_immrbase() + 0xE00B0, 0xff);
> +	return 0;
> +}
> +
> +arch_initcall(mpc85xx_rstcr);
> +
>  void mpc85xx_restart(char *cmd)
>  {
>  	local_irq_disable();
> +	if (rstcr)
> +		/* set reset control register */
> +		out_be32(rstcr, 0x2);	/* HRESET_REQ */
> +	else
> +		printk (KERN_EMERG "Error: reset control register not mapped,  
> spinning!\n");
>  	abort();
>  }
> -- 
> 1.5.1
>

Re: [PATCH v2]: Fix e500 v2 core reboot bug

[Zang Roy-r61911]

On Wed, 2007-05-30 at 03:29, Kumar Gala wrote:
> On May 28, 2007, at 9:36 PM, Zang Roy-r61911 wrote:
> 
> > Fix the e500 v2 core reset bug.
> > For e500 v2 core, a new reset control register is added to
> > reset the core.
> > On 85xx CDS board with e500 v2 core, normal reboot code will
> > induce DDR block in u-boot. This patch fixes this bug. It is
> > also tested on legacy e500 v1 core.
> 
> what happens on an e500 based 85xx system?

Without this patch on MPC8548CDS board, after key in "reboot" command,
the u-boot will hang at DDR init. See the following log without this
patch:

--- log start ---
~ # uname -a
Linux mpc8548cds 2.6.22-rc2-g843b8c89-dirty #5 Tue May 29 16:37:21 CST
2007 ppc
unknown
~ # reboot
~ # Stopping the boa webserver:
Stopping the port mapper:
Stopping inetd:
Syncing hardware clock to system time
hwclock: Could not set the RTC time: Invalid argument
Unmounting filesystems
umount: Cannot open /proc/mounts
The system is going down NOW !!
Sending SIGTERM to all processes.
Please stand by while rebooting the system.
Restarting system.


U-Boot 1.1.6-gcd32691c-dirty (Apr  2 2007 - 17:42:01)

CPU:   8548_E, Version: 2.0, (0x80390020)
Core:  E500, Version: 2.0, (0x80210020)
Clock Configuration:
       CPU: 990 MHz, CCB: 396 MHz,
       DDR: 198 MHz, LBC:  49 MHz
L1:    D-cache 32 kB enabled
       I-cache 32 kB enabled
Board: CDS Version 0x13, PCI Slot 1
CPU Board Revision 114.121 (0x7279)
    PCI1: 32 bit, 33 MHz, async
    PCI2: disabled
I2C:   ready
DRAM:  Initializing
    SDRAM: 64 MB
    DDR: 256 MB

--- log end --

This patch has been tested on 8548/55 CDS board and 8540/60 ADS board.

> I'm not terrible happy with blindly writing to rstcr.
> 
I can understand you.
But I jut want to make things simple and workable.
Any idea?
Roy

Re: [PATCH v2]: Fix e500 v2 core reboot bug

[Kumar Gala]

On May 29, 2007, at 9:34 PM, Zang Roy-r61911 wrote:

> On Wed, 2007-05-30 at 03:29, Kumar Gala wrote:
>> On May 28, 2007, at 9:36 PM, Zang Roy-r61911 wrote:
>>
>>> Fix the e500 v2 core reset bug.
>>> For e500 v2 core, a new reset control register is added to
>>> reset the core.
>>> On 85xx CDS board with e500 v2 core, normal reboot code will
>>> induce DDR block in u-boot. This patch fixes this bug. It is
>>> also tested on legacy e500 v1 core.
>>
>> what happens on an e500 based 85xx system?
>
> Without this patch on MPC8548CDS board, after key in "reboot" command,
> the u-boot will hang at DDR init. See the following log without this
> patch:

Sorry I meant what happens on a e500 v1 based 85xx system w/this patch.

- k

>
> --- log start ---
> ~ # uname -a
> Linux mpc8548cds 2.6.22-rc2-g843b8c89-dirty #5 Tue May 29 16:37:21 CST
> 2007 ppc
> unknown
> ~ # reboot
> ~ # Stopping the boa webserver:
> Stopping the port mapper:
> Stopping inetd:
> Syncing hardware clock to system time
> hwclock: Could not set the RTC time: Invalid argument
> Unmounting filesystems
> umount: Cannot open /proc/mounts
> The system is going down NOW !!
> Sending SIGTERM to all processes.
> Please stand by while rebooting the system.
> Restarting system.
>
>
> U-Boot 1.1.6-gcd32691c-dirty (Apr  2 2007 - 17:42:01)
>
> CPU:   8548_E, Version: 2.0, (0x80390020)
> Core:  E500, Version: 2.0, (0x80210020)
> Clock Configuration:
>        CPU: 990 MHz, CCB: 396 MHz,
>        DDR: 198 MHz, LBC:  49 MHz
> L1:    D-cache 32 kB enabled
>        I-cache 32 kB enabled
> Board: CDS Version 0x13, PCI Slot 1
> CPU Board Revision 114.121 (0x7279)
>     PCI1: 32 bit, 33 MHz, async
>     PCI2: disabled
> I2C:   ready
> DRAM:  Initializing
>     SDRAM: 64 MB
>     DDR: 256 MB
>
> --- log end --
>
> This patch has been tested on 8548/55 CDS board and 8540/60 ADS board.
>
>> I'm not terrible happy with blindly writing to rstcr.
>>
> I can understand you.
> But I jut want to make things simple and workable.
> Any idea?

one idea would be for us to add a property on the soc node.   
Something like has-rstcr or something similar in a guts node?

- k

Re: [PATCH v2]: Fix e500 v2 core reboot bug

[Zang Roy-r61911]

On Wed, 2007-05-30 at 10:40, Kumar Gala wrote:
> On May 29, 2007, at 9:34 PM, Zang Roy-r61911 wrote:
> 
> > On Wed, 2007-05-30 at 03:29, Kumar Gala wrote:
> >> On May 28, 2007, at 9:36 PM, Zang Roy-r61911 wrote:
> >>
> >>> Fix the e500 v2 core reset bug.
> >>> For e500 v2 core, a new reset control register is added to
> >>> reset the core.
> >>> On 85xx CDS board with e500 v2 core, normal reboot code will
> >>> induce DDR block in u-boot. This patch fixes this bug. It is
> >>> also tested on legacy e500 v1 core.
> >>
> >> what happens on an e500 based 85xx system?
> >
> > Without this patch on MPC8548CDS board, after key in "reboot"
> command,
> > the u-boot will hang at DDR init. See the following log without this
> > patch:
> 
> Sorry I meant what happens on a e500 v1 based 85xx system w/this
> patch.

E500 V1 core can boot up normally with/without this patch. Such as
8555CDS system.

> 
> - k
> 
> >
> > --- log start ---
> > ~ # uname -a
> > Linux mpc8548cds 2.6.22-rc2-g843b8c89-dirty #5 Tue May 29 16:37:21
> CST
> > 2007 ppc
> > unknown
> > ~ # reboot
> > ~ # Stopping the boa webserver:
> > Stopping the port mapper:
> > Stopping inetd:
> > Syncing hardware clock to system time
> > hwclock: Could not set the RTC time: Invalid argument
> > Unmounting filesystems
> > umount: Cannot open /proc/mounts
> > The system is going down NOW !!
> > Sending SIGTERM to all processes.
> > Please stand by while rebooting the system.
> > Restarting system.
> >
> >
> > U-Boot 1.1.6-gcd32691c-dirty (Apr  2 2007 - 17:42:01)
> >
> > CPU:   8548_E, Version: 2.0, (0x80390020)
> > Core:  E500, Version: 2.0, (0x80210020)
> > Clock Configuration:
> >        CPU: 990 MHz, CCB: 396 MHz,
> >        DDR: 198 MHz, LBC:  49 MHz
> > L1:    D-cache 32 kB enabled
> >        I-cache 32 kB enabled
> > Board: CDS Version 0x13, PCI Slot 1
> > CPU Board Revision 114.121 (0x7279)
> >     PCI1: 32 bit, 33 MHz, async
> >     PCI2: disabled
> > I2C:   ready
> > DRAM:  Initializing
> >     SDRAM: 64 MB
> >     DDR: 256 MB
> >
> > --- log end --
> >
> > This patch has been tested on 8548/55 CDS board and 8540/60 ADS
> board.
> >
> >> I'm not terrible happy with blindly writing to rstcr.
> >>
> > I can understand you.
> > But I jut want to make things simple and workable.
> > Any idea?
> 
> one idea would be for us to add a property on the soc node.   
> Something like has-rstcr or something similar in a guts node?
I have seen your suggestion before to add a property in device tree.
But I still think the current implementation is simple.
Anyway, I can try your suggestion.
Thanks.
Roy 

Re: [PATCH v2]: Fix e500 v2 core reboot bug

[Kumar Gala]

On May 30, 2007, at 12:46 AM, Zang Roy-r61911 wrote:

> On Wed, 2007-05-30 at 10:40, Kumar Gala wrote:
>> On May 29, 2007, at 9:34 PM, Zang Roy-r61911 wrote:
>>
>>> On Wed, 2007-05-30 at 03:29, Kumar Gala wrote:
>>>> On May 28, 2007, at 9:36 PM, Zang Roy-r61911 wrote:
>>>>
>>>>> Fix the e500 v2 core reset bug.
>>>>> For e500 v2 core, a new reset control register is added to
>>>>> reset the core.
>>>>> On 85xx CDS board with e500 v2 core, normal reboot code will
>>>>> induce DDR block in u-boot. This patch fixes this bug. It is
>>>>> also tested on legacy e500 v1 core.
>>>>
>>>> what happens on an e500 based 85xx system?
>>>
>>> Without this patch on MPC8548CDS board, after key in "reboot"
>> command,
>>> the u-boot will hang at DDR init. See the following log without this
>>> patch:
>>
>> Sorry I meant what happens on a e500 v1 based 85xx system w/this
>> patch.
>
> E500 V1 core can boot up normally with/without this patch. Such as
> 8555CDS system.

Sure, but what happens when you reboot on e500v1 based part?

[snip]

>>>> I'm not terrible happy with blindly writing to rstcr.
>>>>
>>> I can understand you.
>>> But I jut want to make things simple and workable.
>>> Any idea?
>>
>> one idea would be for us to add a property on the soc node.
>> Something like has-rstcr or something similar in a guts node?
> I have seen your suggestion before to add a property in device tree.
> But I still think the current implementation is simple.

While it simple you are depending on the fact that a given  
implementation may or may not have something at the particular  
offset.  Who knows if 8599 or some future part could put the 'cause  
my part to smoke and self-destruct' at the same offset in the future :)

> Anyway, I can try your suggestion.

I'm thinking have a guts block and putting a property in it makes the  
most sense.

- k

Re: [PATCH v2]: Fix e500 v2 core reboot bug

[Kumar Gala]

>>>>> I'm not terrible happy with blindly writing to rstcr.
>>>>>
>>>> I can understand you.
>>>> But I jut want to make things simple and workable.
>>>> Any idea?
>>>
>>> one idea would be for us to add a property on the soc node.
>>> Something like has-rstcr or something similar in a guts node?
>> I have seen your suggestion before to add a property in device tree.
>> But I still think the current implementation is simple.
>
> While it simple you are depending on the fact that a given
> implementation may or may not have something at the particular
> offset.  Who knows if 8599 or some future part could put the 'cause
> my part to smoke and self-destruct' at the same offset in the  
> future :)
>
>> Anyway, I can try your suggestion.
>
> I'm thinking have a guts block and putting a property in it makes the
> most sense.

After some discussion on IRC I think the following is the suggested  
node we should add in.

guts@e00000 {
	compatible = "fsl,mpc8548-guts";
	reg = <e00000 1000>;
	fsl,has-rstcr;
};

- k

Re: [PATCH v2]: Fix e500 v2 core reboot bug

[Segher Boessenkool]

>> I'm thinking have a guts block and putting a property in it makes the
>> most sense.
>
> After some discussion on IRC I think the following is the suggested 
> node we should add in.
>
> guts@e00000 {
> 	compatible = "fsl,mpc8548-guts";
> 	reg = <e00000 1000>;
> 	fsl,has-rstcr;
> };

Except you shouldn't call it "guts".


Segher

Re: [PATCH v2]: Fix e500 v2 core reboot bug

[Zang Roy-r61911]

On Wed, 2007-05-30 at 23:21, Segher Boessenkool wrote:
> >> I'm thinking have a guts block and putting a property in it makes
> the
> >> most sense.
> >
> > After some discussion on IRC I think the following is the suggested 
> > node we should add in.
> >
> > guts@e00000 {
> >       compatible = "fsl,mpc8548-guts";
> >       reg = <e00000 1000>;
> >       fsl,has-rstcr;
> > };
> 
> Except you shouldn't call it "guts".

guts has a confused meaning :-)

How about
gub: global utility block
or 
guf: global utility feature
?
Any other idea?
Roy

Re: [PATCH v2]: Fix e500 v2 core reboot bug

[Benjamin Herrenschmidt]

On Wed, 2007-05-30 at 17:21 +0200, Segher Boessenkool wrote:
> >> I'm thinking have a guts block and putting a property in it makes the
> >> most sense.
> >
> > After some discussion on IRC I think the following is the suggested 
> > node we should add in.
> >
> > guts@e00000 {
> > 	compatible = "fsl,mpc8548-guts";
> > 	reg = <e00000 1000>;
> > 	fsl,has-rstcr;
> > };
> 
> Except you shouldn't call it "guts".

You prefer "bowels" ? :-)

Ben.

Re: [PATCH v2]: Fix e500 v2 core reboot bug

[Zang Roy-r61911]

On Wed, 2007-05-30 at 21:49, Kumar Gala wrote:
> >>>>> I'm not terrible happy with blindly writing to rstcr.
> >>>>>
> >>>> I can understand you.
> >>>> But I jut want to make things simple and workable.
> >>>> Any idea?
> >>>
> >>> one idea would be for us to add a property on the soc node.
> >>> Something like has-rstcr or something similar in a guts node?
> >> I have seen your suggestion before to add a property in device
> tree.
> >> But I still think the current implementation is simple.
> >
> > While it simple you are depending on the fact that a given
> > implementation may or may not have something at the particular
> > offset.  Who knows if 8599 or some future part could put the 'cause
> > my part to smoke and self-destruct' at the same offset in the  
> > future :)
> >
> >> Anyway, I can try your suggestion.
> >
> > I'm thinking have a guts block and putting a property in it makes
> the
> > most sense.
> 
> After some discussion on IRC I think the following is the suggested  
> node we should add in.
> 
> guts@e00000 {
>         compatible = "fsl,mpc8548-guts";
>         reg = <e00000 1000>;
>         fsl,has-rstcr;
> };

Great!
Roy

Re: [PATCH v2]: Fix e500 v2 core reboot bug

[Zang Roy-r61911]

On Wed, 2007-05-30 at 21:49, Kumar Gala wrote:
> >>>>> I'm not terrible happy with blindly writing to rstcr.
> >>>>>
> >>>> I can understand you.
> >>>> But I jut want to make things simple and workable.
> >>>> Any idea?
> >>>
> >>> one idea would be for us to add a property on the soc node.
> >>> Something like has-rstcr or something similar in a guts node?
> >> I have seen your suggestion before to add a property in device
> tree.
> >> But I still think the current implementation is simple.
> >
> > While it simple you are depending on the fact that a given
> > implementation may or may not have something at the particular
> > offset.  Who knows if 8599 or some future part could put the 'cause
> > my part to smoke and self-destruct' at the same offset in the  
> > future :)
> >
> >> Anyway, I can try your suggestion.
> >
> > I'm thinking have a guts block and putting a property in it makes
> the
> > most sense.
> 
> After some discussion on IRC I think the following is the suggested  
> node we should add in.
> 
> guts@e00000 {
>         compatible = "fsl,mpc8548-guts";
>         reg = <e00000 1000>;
one more zero!
>         fsl,has-rstcr;
> };

Re: [PATCH v2]: Fix e500 v2 core reboot bug

[Segher Boessenkool]

>> After some discussion on IRC I think the following is the suggested
>> node we should add in.
>>
>> guts@e00000 {
>>         compatible = "fsl,mpc8548-guts";
>>         reg = <e00000 1000>;
> one more zero!

Yeah.

>>         fsl,has-rstcr;
>> };

If "guts" is some kind of official name (i.e., the
block is called that in the user manual), it looks
okay to me.  Otherwise, please change; in fact, just
change the name anyway (not the compatible), to
"shared-soc-regs" or something like that; "name"
should be descriptive (but terse).


Segher

Re: [PATCH v2]: Fix e500 v2 core reboot bug

[Zang Roy-r61911]

On Mon, 2007-06-04 at 16:41, Segher Boessenkool wrote:
> >> After some discussion on IRC I think the following is the suggested
> >> node we should add in.
> >>
> >> guts@e00000 {
> >>         compatible = "fsl,mpc8548-guts";
> >>         reg = <e00000 1000>;
> > one more zero!
> 
> Yeah.
> 
> >>         fsl,has-rstcr;
> >> };
> 
> If "guts" is some kind of official name (i.e., the
> block is called that in the user manual), it looks
> okay to me.  
No! It is not official name. It is provided by Kumar. It should be a
abbr. standing for "Global Utilities" as far as my understand.
> Otherwise, please change; in fact, just
> change the name anyway (not the compatible), to
> "shared-soc-regs" or something like that;
I can not agree with you. "shared-soc-regs" can not describe the
property of this kind of register.
The official description for this set of registers:

"The global utilities block controls power management, I/O device
enabling, power-on-reset (POR) configuration monitoring, general-purpose
I/O signal configuration, alternate function selection for multiplexed
signals, and clock control.

gub (Kumar's style?): global utilities block
or 
global-uti-regs or global-uti (Segher's style?)
may be better.
 
>  "name"
> should be descriptive (but terse).
> 
Agree!
But we should reach a agreement!
Thanks.
Roy

Re: [PATCH v2]: Fix e500 v2 core reboot bug

[Segher Boessenkool]

>>>> guts@e00000 {

>> If "guts" is some kind of official name (i.e., the
>> block is called that in the user manual), it looks
>> okay to me.
> No! It is not official name. It is provided by Kumar. It should be a
> abbr. standing for "Global Utilities" as far as my understand.

So the official name is "global utilities block"?

>> Otherwise, please change; in fact, just
>> change the name anyway (not the compatible), to
>> "shared-soc-regs" or something like that;
> I can not agree with you. "shared-soc-regs" can not describe the
> property of this kind of register.
> The official description for this set of registers:
>
> "The global utilities block controls power management, I/O device
> enabling, power-on-reset (POR) configuration monitoring, 
> general-purpose
> I/O signal configuration, alternate function selection for multiplexed
> signals, and clock control.
>
> gub (Kumar's style?): global utilities block
> or
> global-uti-regs or global-uti (Segher's style?)
> may be better.

"global-utilities" sounds fine to me, although the
name doesn't actually say anything.  Let's avoid
abbr.s unless they are very widely known.

>>  "name"
>> should be descriptive (but terse).
>>
> Agree!
> But we should reach a agreement!

Nah, you guys just need to come up with a good name :-)
I would just call it "control" perhaps ;-)

It doesn't matter much, but "name" should be human-
readable, human-understandable, and short.  A made-up
acronym doesn't work (although I like "guts", sure --
put it in your "compatible" property ;-) )


Segher

Re: [PATCH v2]: Fix e500 v2 core reboot bug

[Zang Roy-r61911]

On Mon, 2007-06-04 at 18:37, Segher Boessenkool wrote:
> >>>> guts@e00000 {
> 
> >> If "guts" is some kind of official name (i.e., the
> >> block is called that in the user manual), it looks
> >> okay to me.
> > No! It is not official name. It is provided by Kumar. It should be a
> > abbr. standing for "Global Utilities" as far as my understand.
> 
> So the official name is "global utilities block"?
> 
> >> Otherwise, please change; in fact, just
> >> change the name anyway (not the compatible), to
> >> "shared-soc-regs" or something like that;
> > I can not agree with you. "shared-soc-regs" can not describe the
> > property of this kind of register.
> > The official description for this set of registers:
> >
> > "The global utilities block controls power management, I/O device
> > enabling, power-on-reset (POR) configuration monitoring, 
> > general-purpose
> > I/O signal configuration, alternate function selection for
> multiplexed
> > signals, and clock control.
> >
> > gub (Kumar's style?): global utilities block
> > or
> > global-uti-regs or global-uti (Segher's style?)
> > may be better.
> 
> "global-utilities" sounds fine to me, although the
> name doesn't actually say anything.  Let's avoid
> abbr.s unless they are very widely known.
"global-utilities" seems good to me!
> 
> >>  "name"
> >> should be descriptive (but terse).
> >>
> > Agree!
> > But we should reach a agreement!
> 
> Nah, you guys just need to come up with a good name :-)
> I would just call it "control" perhaps ;-)
> 
> It doesn't matter much, but "name" should be human-
> readable, human-understandable, and short.  A made-up
> acronym doesn't work (although I like "guts", sure --
> put it in your "compatible" property ;-) )
Agree!
Thanks.
Roy

Re: [PATCH v2]: Fix e500 v2 core reboot bug

[Zang Roy-r61911]

On Wed, 2007-05-30 at 20:25, Kumar Gala wrote:
> On May 30, 2007, at 12:46 AM, Zang Roy-r61911 wrote:
> 
> > On Wed, 2007-05-30 at 10:40, Kumar Gala wrote:
> >> On May 29, 2007, at 9:34 PM, Zang Roy-r61911 wrote:
> >>
> >>> On Wed, 2007-05-30 at 03:29, Kumar Gala wrote:
> >>>> On May 28, 2007, at 9:36 PM, Zang Roy-r61911 wrote:
> >>>>
> >>>>> Fix the e500 v2 core reset bug.
> >>>>> For e500 v2 core, a new reset control register is added to
> >>>>> reset the core.
> >>>>> On 85xx CDS board with e500 v2 core, normal reboot code will
> >>>>> induce DDR block in u-boot. This patch fixes this bug. It is
> >>>>> also tested on legacy e500 v1 core.
> >>>>
> >>>> what happens on an e500 based 85xx system?
> >>>
> >>> Without this patch on MPC8548CDS board, after key in "reboot"
> >> command,
> >>> the u-boot will hang at DDR init. See the following log without
> this
> >>> patch:
> >>
> >> Sorry I meant what happens on a e500 v1 based 85xx system w/this
> >> patch.
> >
> > E500 V1 core can boot up normally with/without this patch. Such as
> > 8555CDS system.
> 
> Sure, but what happens when you reboot on e500v1 based part?
Sorry, Maybe I do not understand clearly.
The u-boot boot up correctly, when I reboot on a e500v1 based part.
This patch do not affect the e500v1 core boot up. Current code is enough
for e500v1 core. While for E500 v2 core, the u-boot will block without
this patch as I posted before.

Is it clear?

> 
> [snip]
> 
> >>>> I'm not terrible happy with blindly writing to rstcr.
> >>>>
> >>> I can understand you.
> >>> But I jut want to make things simple and workable.
> >>> Any idea?
> >>
> >> one idea would be for us to add a property on the soc node.
> >> Something like has-rstcr or something similar in a guts node?
> > I have seen your suggestion before to add a property in device tree.
> > But I still think the current implementation is simple.
> 
> While it simple you are depending on the fact that a given  
> implementation may or may not have something at the particular  
> offset.  Who knows if 8599 or some future part could put the 'cause  
> my part to smoke and self-destruct' at the same offset in the future
> :)
I do not know the future, but I think the 85xx processor should have a
compatible register define. I do not think the future 85xx processor
will use this offset for other usage. It will cause big confusion.
If you study the road map of 85xx processor register define, you can see
that next generation processors only use reserved offset for expanded
usage.

> 
> > Anyway, I can try your suggestion.
> 
> I'm thinking have a guts block and putting a property in it makes the 
> most sense.
> 
> - k
> 

[PATCH v3]: Fix e500 v2 core reboot bug

[Zang Roy-r61911]

From: roy zang <tie-fei.zang@freescale.com>

Fix the reset bug on 8548CDS board.
>From MPC8548CDS with e500 v2 core, a new reset control register is added.
This register is used for the cpu reset.

Signed-off-by: Roy Zang <tie-fei.zang@freescale.com>
---
I will update the booting-without-of.txt and apply
this method to 8544/8641 in next serial patches.

 arch/powerpc/boot/dts/mpc8548cds.dts |    6 ++++++
 arch/powerpc/platforms/85xx/misc.c   |   30 ++++++++++++++++++++++++++++++
 2 files changed, 36 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/boot/dts/mpc8548cds.dts b/arch/powerpc/boot/dts/mpc8548cds.dts
index ad96381..0550a3c 100644
--- a/arch/powerpc/boot/dts/mpc8548cds.dts
+++ b/arch/powerpc/boot/dts/mpc8548cds.dts
@@ -177,6 +177,12 @@
 			interrupt-parent = <&mpic>;
 		};
 
+		global-utilities@e0000 {	//global utilities reg
+			compatible = "fsl,mpc8548-guts";
+			reg = <e0000 1000>;
+			fsl,has-rstcr;
+		};
+
 		pci1: pci@8000 {
 			interrupt-map-mask = <1f800 0 0 7>;
 			interrupt-map = <
diff --git a/arch/powerpc/platforms/85xx/misc.c b/arch/powerpc/platforms/85xx/misc.c
index 3e62fcb..6d9451c 100644
--- a/arch/powerpc/platforms/85xx/misc.c
+++ b/arch/powerpc/platforms/85xx/misc.c
@@ -13,11 +13,41 @@
 #include <linux/irq.h>
 #include <linux/module.h>
 #include <asm/irq.h>
+#include <asm/io.h>
+#include <asm/prom.h>
+#include <sysdev/fsl_soc.h>
+
+static __be32 __iomem *rstcr;
 
 extern void abort(void);
 
+static int __init mpc85xx_rstcr(void)
+{
+	struct device_node *np;
+	np = of_find_node_by_name(NULL, "global-utilities");
+	if ((np && of_get_property(np, "fsl,has-rstcr", NULL))) {
+		const u32 *prop = of_get_property(np, "reg", NULL);
+		if (prop) {
+		/* map reset control register
+		 * 0xE00B0 is offset of reset control register
+		 */
+			rstcr = ioremap(get_immrbase() + *prop + 0xB0, 0xff);
+			if (!rstcr)
+				printk (KERN_EMERG "Error: reset control register not mapped, spinning!\n");
+		}
+	else
+		printk (KERN_INFO "rstcr compatible register does not exist!\n");
+	}
+	return 0;
+}
+
+arch_initcall(mpc85xx_rstcr);
+
 void mpc85xx_restart(char *cmd)
 {
 	local_irq_disable();
+	if (rstcr)
+		/* set reset control register */
+		out_be32(rstcr, 0x2);	/* HRESET_REQ */
 	abort();
 }
-- 
1.5.1

Re: [PATCH v3]: Fix e500 v2 core reboot bug

[Timur Tabi]

Zang Roy-r61911 wrote:
> From: roy zang <tie-fei.zang@freescale.com>
> 
> Fix the reset bug on 8548CDS board.
>>From MPC8548CDS with e500 v2 core, a new reset control register is added.
> This register is used for the cpu reset.
> 
> Signed-off-by: Roy Zang <tie-fei.zang@freescale.com>

Acked-by: Timur Tabi <timur@freescale.com>


-- 
Timur Tabi
Linux Kernel Developer @ Freescale

Re: [PATCH v3]: Fix e500 v2 core reboot bug

[Segher Boessenkool]

> +		global-utilities@e0000 {	//global utilities reg
> +			compatible = "fsl,mpc8548-guts";
> +			reg = <e0000 1000>;
> +			fsl,has-rstcr;
> +		};

Looks good.

> +	np = of_find_node_by_name(NULL, "global-utilities");
> +	if ((np && of_get_property(np, "fsl,has-rstcr", NULL))) {
> +		const u32 *prop = of_get_property(np, "reg", NULL);
> +		if (prop) {
> +		/* map reset control register
> +		 * 0xE00B0 is offset of reset control register
> +		 */
> +			rstcr = ioremap(get_immrbase() + *prop + 0xB0, 0xff);
> +			if (!rstcr)
> +				printk (KERN_EMERG "Error: reset control register not mapped, 
> spinning!\n");
> +		}
> +	else
> +		printk (KERN_INFO "rstcr compatible register does not exist!\n");
> +	}

This needs an of_node_put(np).  Oh and the indenting
is off.


Segher

Re: [PATCH v3]: Fix e500 v2 core reboot bug

[Zang Roy-r61911]

On Wed, 2007-06-13 at 12:44, Segher Boessenkool wrote:
> > +             global-utilities@e0000 {        //global utilities reg
> > +                     compatible = "fsl,mpc8548-guts";
> > +                     reg = <e0000 1000>;
> > +                     fsl,has-rstcr;
> > +             };
> 
> Looks good.
> 
> > +     np = of_find_node_by_name(NULL, "global-utilities");
> > +     if ((np && of_get_property(np, "fsl,has-rstcr", NULL))) {
> > +             const u32 *prop = of_get_property(np, "reg", NULL);
> > +             if (prop) {
> > +             /* map reset control register
> > +              * 0xE00B0 is offset of reset control register
> > +              */
> > +                     rstcr = ioremap(get_immrbase() + *prop + 0xB0,
> 0xff);
> > +                     if (!rstcr)
> > +                             printk (KERN_EMERG "Error: reset
> control register not mapped, 
> > spinning!\n");
> > +             }
> > +     else
> > +             printk (KERN_INFO "rstcr compatible register does not
> exist!\n");
> > +     }
> 
> This needs an of_node_put(np).  Oh and the indenting
> is off.
It is my oversight!
I had thought to add of_node_put(), but forget.
I just checkout the code and decide to have a small revise.

Thanks.
Roy

Re: [PATCH v3]: Fix e500 v2 core reboot bug

[Zang Roy-r61911]

From: roy zang <tie-fei.zang@freescale.com>

Fix the reset bug on 8548CDS board.
>From MPC8548CDS with e500 v2 core, a new reset control register is added.
This register is used for the cpu reset.

Signed-off-by: Roy Zang <tie-fei.zang@freescale.com>
---
Please ignore the mail sent just now and pick up this one.
Sorry for the garbage!
Thanks for Segher's comment.

 arch/powerpc/boot/dts/mpc8548cds.dts |    6 ++++++
 arch/powerpc/platforms/85xx/misc.c   |   31 +++++++++++++++++++++++++++++++
 2 files changed, 37 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/boot/dts/mpc8548cds.dts b/arch/powerpc/boot/dts/mpc8548cds.dts
index ad96381..0550a3c 100644
--- a/arch/powerpc/boot/dts/mpc8548cds.dts
+++ b/arch/powerpc/boot/dts/mpc8548cds.dts
@@ -177,6 +177,12 @@
 			interrupt-parent = <&mpic>;
 		};
 
+		global-utilities@e0000 {	//global utilities reg
+			compatible = "fsl,mpc8548-guts";
+			reg = <e0000 1000>;
+			fsl,has-rstcr;
+		};
+
 		pci1: pci@8000 {
 			interrupt-map-mask = <1f800 0 0 7>;
 			interrupt-map = <
diff --git a/arch/powerpc/platforms/85xx/misc.c b/arch/powerpc/platforms/85xx/misc.c
index 3e62fcb..76f66d4 100644
--- a/arch/powerpc/platforms/85xx/misc.c
+++ b/arch/powerpc/platforms/85xx/misc.c
@@ -13,11 +13,42 @@
 #include <linux/irq.h>
 #include <linux/module.h>
 #include <asm/irq.h>
+#include <asm/io.h>
+#include <asm/prom.h>
+#include <sysdev/fsl_soc.h>
+
+static __be32 __iomem *rstcr;
 
 extern void abort(void);
 
+static int __init mpc85xx_rstcr(void)
+{
+	struct device_node *np;
+	np = of_find_node_by_name(NULL, "global-utilities");
+	if ((np && of_get_property(np, "fsl,has-rstcr", NULL))) {
+		const u32 *prop = of_get_property(np, "reg", NULL);
+		if (prop) {
+			/* map reset control register
+			 * 0xE00B0 is offset of reset control register
+			 */
+			rstcr = ioremap(get_immrbase() + *prop + 0xB0, 0xff);
+			if (!rstcr)
+				printk (KERN_EMERG "Error: reset control \
+					register not mapped, spinning!\n");
+		}
+		of_node_put(np);
+	} else
+		printk (KERN_INFO "rstcr compatible register does not exist!\n");
+	return 0;
+}
+
+arch_initcall(mpc85xx_rstcr);
+
 void mpc85xx_restart(char *cmd)
 {
 	local_irq_disable();
+	if (rstcr)
+		/* set reset control register */
+		out_be32(rstcr, 0x2);	/* HRESET_REQ */
 	abort();
 }
-- 
1.5.1

Re: [PATCH v3]: Fix e500 v2 core reboot bug

[Zang Roy-r61911]

On Wed, 2007-06-13 at 14:24, Zang Roy-r61911 wrote:
> From: roy zang <tie-fei.zang@freescale.com>
> 
> Fix the reset bug on 8548CDS board.
> >From MPC8548CDS with e500 v2 core, a new reset control register is
> added.
I am also curious who adds the extra symbol ">" before From, the
mailer(I am using evolution) or else ...
:-)
Roy

Re: [PATCH v3]: Fix e500 v2 core reboot bug

[Segher Boessenkool]

>> Fix the reset bug on 8548CDS board.
>>> From MPC8548CDS with e500 v2 core, a new reset control register is
>> added.
> I am also curious who adds the extra symbol ">" before From, the
> mailer(I am using evolution) or else ...

The mailer yes, since "From " at the start of line
would be confused for an SMTP transport header.

Mail clients traditionally use ">" to prevent this; some
mail clients do something different though (a space, for
example).


Segher

Re: [PATCH v3]: Fix e500 v2 core reboot bug

[Segher Boessenkool]

> The mailer yes, since "From " at the start of line
> would be confused for an SMTP transport header.

Erm, an mbox header I meant of course.


Segher

Re: [PATCH v3]: Fix e500 v2 core reboot bug

[Zang Roy-r61911]

From: roy zang <tie-fei.zang@freescale.com>

Fix the reset bug on 8548CDS board.
>From MPC8548CDS with e500 v2 core, a new reset control register is added.
This register is used for the cpu reset.

Signed-off-by: Roy Zang <tie-fei.zang@freescale.com>
---
Please pick up this one.
Thanks for Segher's comment!

 arch/powerpc/boot/dts/mpc8548cds.dts |    6 ++++++
 arch/powerpc/platforms/85xx/misc.c   |   31 +++++++++++++++++++++++++++++++
 2 files changed, 37 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/boot/dts/mpc8548cds.dts b/arch/powerpc/boot/dts/mpc8548cds.dts
index ad96381..0550a3c 100644
--- a/arch/powerpc/boot/dts/mpc8548cds.dts
+++ b/arch/powerpc/boot/dts/mpc8548cds.dts
@@ -177,6 +177,12 @@
 			interrupt-parent = <&mpic>;
 		};
 
+		global-utilities@e0000 {	//global utilities reg
+			compatible = "fsl,mpc8548-guts";
+			reg = <e0000 1000>;
+			fsl,has-rstcr;
+		};
+
 		pci1: pci@8000 {
 			interrupt-map-mask = <1f800 0 0 7>;
 			interrupt-map = <
diff --git a/arch/powerpc/platforms/85xx/misc.c b/arch/powerpc/platforms/85xx/misc.c
index 3e62fcb..76f66d4 100644
--- a/arch/powerpc/platforms/85xx/misc.c
+++ b/arch/powerpc/platforms/85xx/misc.c
@@ -13,11 +13,42 @@
 #include <linux/irq.h>
 #include <linux/module.h>
 #include <asm/irq.h>
+#include <asm/io.h>
+#include <asm/prom.h>
+#include <sysdev/fsl_soc.h>
+
+static __be32 __iomem *rstcr;
 
 extern void abort(void);
 
+static int __init mpc85xx_rstcr(void)
+{
+	struct device_node *np;
+	np = of_find_node_by_name(NULL, "global-utilities");
+	if ((np && of_get_property(np, "fsl,has-rstcr", NULL))) {
+		const u32 *prop = of_get_property(np, "reg", NULL);
+		if (prop) {
+			/* map reset control register
+			 * 0xE00B0 is offset of reset control register
+			 */
+			rstcr = ioremap(get_immrbase() + *prop + 0xB0, 0xff);
+			if (!rstcr)
+				printk (KERN_EMERG "Error: reset control \
+					register not mapped, spinning!\n");
+		}
+		of_node_put(np);
+	} else
+		printk (KERN_INFO "rstcr compatible register does not exist!\n");
+	return 0;
+}
+
+arch_initcall(mpc85xx_rstcr);
+
 void mpc85xx_restart(char *cmd)
 {
 	local_irq_disable();
+	if (rstcr)
+		/* set reset control register */
+		out_be32(rstcr, 0x2);	/* HRESET_REQ */
 	abort();
 }
-- 
1.5.1

Re: [PATCH v3]: Fix e500 v2 core reboot bug

[Segher Boessenkool]

> +	np = of_find_node_by_name(NULL, "global-utilities");
> +	if ((np && of_get_property(np, "fsl,has-rstcr", NULL))) {

If this of_get_property fails...

> +		const u32 *prop = of_get_property(np, "reg", NULL);
> +		if (prop) {
> +			/* map reset control register
> +			 * 0xE00B0 is offset of reset control register
> +			 */
> +			rstcr = ioremap(get_immrbase() + *prop + 0xB0, 0xff);
> +			if (!rstcr)
> +				printk (KERN_EMERG "Error: reset control \
> +					register not mapped, spinning!\n");
> +		}
> +		of_node_put(np);

...you still leak a reference.


Segher

Re: [PATCH v3]: Fix e500 v2 core reboot bug

[Zang Roy-r61911]

On Wed, 2007-06-13 at 14:28, Segher Boessenkool wrote:
> > +     np = of_find_node_by_name(NULL, "global-utilities");
> > +     if ((np && of_get_property(np, "fsl,has-rstcr", NULL))) {
> 
> If this of_get_property fails...
> 
> > +             const u32 *prop = of_get_property(np, "reg", NULL);
> > +             if (prop) {
> > +                     /* map reset control register
> > +                      * 0xE00B0 is offset of reset control register
> > +                      */
> > +                     rstcr = ioremap(get_immrbase() + *prop + 0xB0,
> 0xff);
> > +                     if (!rstcr)
> > +                             printk (KERN_EMERG "Error: reset
> control \
> > +                                     register not mapped,
> spinning!\n");
> > +             }
> > +             of_node_put(np);
> 
> ...you still leak a reference.
You are right!!!
Roy