[PATCH] add restart function for mpc52xx

[PATCH] add restart function for mpc52xx

[Sascha Hauer]

This patch adds restart support for mpx52xx systems.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>

---
 arch/powerpc/platforms/52xx/lite5200.c       |    1 +
 arch/powerpc/platforms/52xx/mpc52xx_common.c |   15 +++++++++++++++
 include/asm-powerpc/mpc52xx.h                |    2 ++
 3 files changed, 18 insertions(+)

Index: linux-2.6/arch/powerpc/platforms/52xx/lite5200.c
===================================================================
--- linux-2.6.orig/arch/powerpc/platforms/52xx/lite5200.c
+++ linux-2.6/arch/powerpc/platforms/52xx/lite5200.c
@@ -153,6 +153,7 @@ define_machine(lite52xx) {
 	.name 		= "lite52xx",
 	.probe 		= lite52xx_probe,
 	.setup_arch 	= lite52xx_setup_arch,
+	.restart	= mpc52xx_restart,
 	.init		= mpc52xx_declare_of_platform_devices,
 	.init_IRQ 	= mpc52xx_init_irq,
 	.get_irq 	= mpc52xx_get_irq,
Index: linux-2.6/arch/powerpc/platforms/52xx/mpc52xx_common.c
===================================================================
--- linux-2.6.orig/arch/powerpc/platforms/52xx/mpc52xx_common.c
+++ linux-2.6/arch/powerpc/platforms/52xx/mpc52xx_common.c
@@ -75,6 +75,21 @@ mpc52xx_find_ipb_freq(struct device_node
 }
 EXPORT_SYMBOL(mpc52xx_find_ipb_freq);
 
+void
+mpc52xx_restart(char *cmd)
+{
+	struct mpc52xx_gpt *gpt = mpc52xx_find_and_map("mpc52xx-gpt");
+
+	local_irq_disable();
+
+	/* Turn on the watchdog and wait for it to expire. It effectively
+	  does a reset */
+	out_be32(&gpt->mode, 0x00000000);
+	out_be32(&gpt->count, 0x0000000ff);
+	out_be32(&gpt->mode, 0x00009004);
+
+	while (1);
+}
 
 void __init
 mpc52xx_setup_cpu(void)
Index: linux-2.6/include/asm-powerpc/mpc52xx.h
===================================================================
--- linux-2.6.orig/include/asm-powerpc/mpc52xx.h
+++ linux-2.6/include/asm-powerpc/mpc52xx.h
@@ -249,6 +249,8 @@ extern void mpc52xx_declare_of_platform_
 extern void mpc52xx_init_irq(void);
 extern unsigned int mpc52xx_get_irq(void);
 
+extern void mpc52xx_restart(char *cmd);
+
 #endif /* __ASSEMBLY__ */
 
 #endif /* __ASM_POWERPC_MPC52xx_H__ */


-- 
 Dipl.-Ing. Sascha Hauer | http://www.pengutronix.de
  Pengutronix - Linux Solutions for Science and Industry
    Handelsregister: Amtsgericht Hildesheim, HRA 2686
      Hannoversche Str. 2, 31134 Hildesheim, Germany
    Phone: +49-5121-206917-0 |  Fax: +49-5121-206917-9

Re: [PATCH] add restart function for mpc52xx

[Sylvain Munaut]

Sascha Hauer wrote:
> This patch adds restart support for mpx52xx systems.
>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
>   
Thanks, I'll test and include this to my 2.6.21 patch set.
> ---
>  arch/powerpc/platforms/52xx/lite5200.c       |    1 +
>  arch/powerpc/platforms/52xx/mpc52xx_common.c |   15 +++++++++++++++
>  include/asm-powerpc/mpc52xx.h                |    2 ++
>  3 files changed, 18 insertions(+)
>
> Index: linux-2.6/arch/powerpc/platforms/52xx/lite5200.c
> ===================================================================
> --- linux-2.6.orig/arch/powerpc/platforms/52xx/lite5200.c
> +++ linux-2.6/arch/powerpc/platforms/52xx/lite5200.c
> @@ -153,6 +153,7 @@ define_machine(lite52xx) {
>  	.name 		= "lite52xx",
>  	.probe 		= lite52xx_probe,
>  	.setup_arch 	= lite52xx_setup_arch,
> +	.restart	= mpc52xx_restart,
>  	.init		= mpc52xx_declare_of_platform_devices,
>  	.init_IRQ 	= mpc52xx_init_irq,
>  	.get_irq 	= mpc52xx_get_irq,
> Index: linux-2.6/arch/powerpc/platforms/52xx/mpc52xx_common.c
> ===================================================================
> --- linux-2.6.orig/arch/powerpc/platforms/52xx/mpc52xx_common.c
> +++ linux-2.6/arch/powerpc/platforms/52xx/mpc52xx_common.c
> @@ -75,6 +75,21 @@ mpc52xx_find_ipb_freq(struct device_node
>  }
>  EXPORT_SYMBOL(mpc52xx_find_ipb_freq);
>  
> +void
> +mpc52xx_restart(char *cmd)
> +{
> +	struct mpc52xx_gpt *gpt = mpc52xx_find_and_map("mpc52xx-gpt");
>   
missing __iomem here, will most trigger a sparse warning.
It's ok I'll fix that up when applying to my internal tree and before fw
this upstream.
> +
> +	local_irq_disable();
> +
> +	/* Turn on the watchdog and wait for it to expire. It effectively
> +	  does a reset */
> +	out_be32(&gpt->mode, 0x00000000);
> +	out_be32(&gpt->count, 0x0000000ff);
> +	out_be32(&gpt->mode, 0x00009004);
> +
> +	while (1);
> +}
>  
>  void __init
>  mpc52xx_setup_cpu(void)
> Index: linux-2.6/include/asm-powerpc/mpc52xx.h
> ===================================================================
> --- linux-2.6.orig/include/asm-powerpc/mpc52xx.h
> +++ linux-2.6/include/asm-powerpc/mpc52xx.h
> @@ -249,6 +249,8 @@ extern void mpc52xx_declare_of_platform_
>  extern void mpc52xx_init_irq(void);
>  extern unsigned int mpc52xx_get_irq(void);
>  
> +extern void mpc52xx_restart(char *cmd);
> +
>  #endif /* __ASSEMBLY__ */
>  
>  #endif /* __ASM_POWERPC_MPC52xx_H__ */
>
>
>   

Re: [PATCH] add restart function for mpc52xx

[Kumar Gala]

On Jan 11, 2007, at 6:28 AM, Sascha Hauer wrote:

> This patch adds restart support for mpx52xx systems.
>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
>
> ---
>  arch/powerpc/platforms/52xx/lite5200.c       |    1 +
>  arch/powerpc/platforms/52xx/mpc52xx_common.c |   15 +++++++++++++++
>  include/asm-powerpc/mpc52xx.h                |    2 ++
>  3 files changed, 18 insertions(+)
>
> Index: linux-2.6/arch/powerpc/platforms/52xx/lite5200.c
> ===================================================================
> --- linux-2.6.orig/arch/powerpc/platforms/52xx/lite5200.c
> +++ linux-2.6/arch/powerpc/platforms/52xx/lite5200.c
> @@ -153,6 +153,7 @@ define_machine(lite52xx) {
>  	.name 		= "lite52xx",
>  	.probe 		= lite52xx_probe,
>  	.setup_arch 	= lite52xx_setup_arch,
> +	.restart	= mpc52xx_restart,
>  	.init		= mpc52xx_declare_of_platform_devices,
>  	.init_IRQ 	= mpc52xx_init_irq,
>  	.get_irq 	= mpc52xx_get_irq,
> Index: linux-2.6/arch/powerpc/platforms/52xx/mpc52xx_common.c
> ===================================================================
> --- linux-2.6.orig/arch/powerpc/platforms/52xx/mpc52xx_common.c
> +++ linux-2.6/arch/powerpc/platforms/52xx/mpc52xx_common.c
> @@ -75,6 +75,21 @@ mpc52xx_find_ipb_freq(struct device_node
>  }
>  EXPORT_SYMBOL(mpc52xx_find_ipb_freq);
>
> +void
> +mpc52xx_restart(char *cmd)
> +{
> +	struct mpc52xx_gpt *gpt = mpc52xx_find_and_map("mpc52xx-gpt");
> +

This suffers from the same bug mpc83xx_restart has.  We can NOT do an  
ioremap inside the restart function.  We may get called from  
interrupt context on a panic and will not be able to do the ioremap 
().  The simplest thing is to do the mapping earlier in an init call  
and save the pointer, its not perfect, but better.

> +	local_irq_disable();
> +
> +	/* Turn on the watchdog and wait for it to expire. It effectively
> +	  does a reset */
> +	out_be32(&gpt->mode, 0x00000000);
> +	out_be32(&gpt->count, 0x0000000ff);
> +	out_be32(&gpt->mode, 0x00009004);
> +
> +	while (1);
> +}
>
>  void __init
>  mpc52xx_setup_cpu(void)

Re: [PATCH] add restart function for mpc52xx

[Sascha Hauer]

> 
> This suffers from the same bug mpc83xx_restart has.  We can NOT do an  
> ioremap inside the restart function.  We may get called from  
> interrupt context on a panic and will not be able to do the ioremap 
> ().  The simplest thing is to do the mapping earlier in an init call  
> and save the pointer, its not perfect, but better.
> 

I'm beginning to hate this whole pseudo OF thing for embedded systems.
All we need to know is that we have a mpc52xx processor. Instead of
using this information directly we scatter it in many pieces, put a
dts file(-template) in the kernel, let the bootloader pass it back to
the kernel, evaluate it in an OF parser called from mpc52xx_setup_cpu()
and use it in mpc52xx_restart(). Quite a long way for one single SoC
register. Yes I know, I'm two years late for this rant...

BTW the watchdog timer is only implemented for gpt0. Is it guaranteed
that I get gpt0 with mpc52xx_find_and_map("mpc52xx-gpt")? As long as
all flat trees start with gpt0 I guess yes.

Anyway, how about this one?

Sascha


This patch adds restart support for mpx52xx systems.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>

---
 arch/powerpc/platforms/52xx/lite5200.c       |    1 +
 arch/powerpc/platforms/52xx/mpc52xx_common.c |   20 ++++++++++++++++++++
 include/asm-powerpc/mpc52xx.h                |    2 ++
 3 files changed, 23 insertions(+)

Index: linux-2.6/arch/powerpc/platforms/52xx/lite5200.c
===================================================================
--- linux-2.6.orig/arch/powerpc/platforms/52xx/lite5200.c
+++ linux-2.6/arch/powerpc/platforms/52xx/lite5200.c
@@ -153,6 +153,7 @@ define_machine(lite52xx) {
 	.name 		= "lite52xx",
 	.probe 		= lite52xx_probe,
 	.setup_arch 	= lite52xx_setup_arch,
+	.restart	= mpc52xx_restart,
 	.init		= mpc52xx_declare_of_platform_devices,
 	.init_IRQ 	= mpc52xx_init_irq,
 	.get_irq 	= mpc52xx_get_irq,
Index: linux-2.6/arch/powerpc/platforms/52xx/mpc52xx_common.c
===================================================================
--- linux-2.6.orig/arch/powerpc/platforms/52xx/mpc52xx_common.c
+++ linux-2.6/arch/powerpc/platforms/52xx/mpc52xx_common.c
@@ -75,6 +75,23 @@ mpc52xx_find_ipb_freq(struct device_node
 }
 EXPORT_SYMBOL(mpc52xx_find_ipb_freq);
 
+static struct __iomem mpc52xx_gpt *gpt = NULL;
+
+void
+mpc52xx_restart(char *cmd)
+{
+	local_irq_disable();
+
+	/* Turn on the watchdog and wait for it to expire. It effectively
+	  does a reset */
+	if (gpt) {
+		out_be32(&gpt->mode, 0x00000000);
+		out_be32(&gpt->count, 0x00010001);
+		out_be32(&gpt->mode, 0x00009004);
+	}
+
+	while (1);
+}
 
 void __init
 mpc52xx_setup_cpu(void)
@@ -82,6 +99,9 @@ mpc52xx_setup_cpu(void)
 	struct mpc52xx_cdm  __iomem *cdm;
 	struct mpc52xx_xlb  __iomem *xlb;
 
+	/* needed for mpc52xx_restart */
+	gpt = mpc52xx_find_and_map("mpc52xx-gpt");
+
 	/* Map zones */
 	cdm = mpc52xx_find_and_map("mpc52xx-cdm");
 	xlb = mpc52xx_find_and_map("mpc52xx-xlb");
Index: linux-2.6/include/asm-powerpc/mpc52xx.h
===================================================================
--- linux-2.6.orig/include/asm-powerpc/mpc52xx.h
+++ linux-2.6/include/asm-powerpc/mpc52xx.h
@@ -249,6 +249,8 @@ extern void mpc52xx_declare_of_platform_
 extern void mpc52xx_init_irq(void);
 extern unsigned int mpc52xx_get_irq(void);
 
+extern void mpc52xx_restart(char *cmd);
+
 #endif /* __ASSEMBLY__ */
 
 #endif /* __ASM_POWERPC_MPC52xx_H__ */

-- 
 Dipl.-Ing. Sascha Hauer | http://www.pengutronix.de
  Pengutronix - Linux Solutions for Science and Industry
    Handelsregister: Amtsgericht Hildesheim, HRA 2686
      Hannoversche Str. 2, 31134 Hildesheim, Germany
    Phone: +49-5121-206917-0 |  Fax: +49-5121-206917-9

Re: [PATCH] add restart function for mpc52xx

[Grant Likely]

On 1/11/07, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> BTW the watchdog timer is only implemented for gpt0. Is it guaranteed
> that I get gpt0 with mpc52xx_find_and_map("mpc52xx-gpt")? As long as
> all flat trees start with gpt0 I guess yes.

No, you cannot make this assumption.  There are no guarantees on order
that nodes are found.

g.

-- 
Grant Likely, B.Sc. P.Eng.
Secret Lab Technologies Ltd.
grant.likely@secretlab.ca
(403) 399-0195

Re: [PATCH] add restart function for mpc52xx

[Grant Likely]

On 1/11/07, Grant Likely <grant.likely@secretlab.ca> wrote:
> On 1/11/07, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> > BTW the watchdog timer is only implemented for gpt0. Is it guaranteed
> > that I get gpt0 with mpc52xx_find_and_map("mpc52xx-gpt")? As long as
> > all flat trees start with gpt0 I guess yes.
>
> No, you cannot make this assumption.  There are no guarantees on order
> that nodes are found.

Sylvain and I just discussed this a bit.  Since gpt0 is different (has
the watchdog bits) it should have a compatible property that reflects
it.  We're thinking about adding "mpc52xx-wdt" to the compatible list
for gpt0.

Cheers,
g.

-- 
Grant Likely, B.Sc. P.Eng.
Secret Lab Technologies Ltd.
grant.likely@secretlab.ca
(403) 399-0195

Re: [PATCH] add restart function for mpc52xx

[Segher Boessenkool]

> Sylvain and I just discussed this a bit.  Since gpt0 is different (has
> the watchdog bits) it should have a compatible property that reflects
> it.  We're thinking about adding "mpc52xx-wdt" to the compatible list
> for gpt0.

Or add an extra (empty) property "has-watchdog" or
something like that.


Segher

Re: [PATCH] add restart function for mpc52xx

[Paul Mackerras]

Sascha Hauer writes:

> > This suffers from the same bug mpc83xx_restart has.  We can NOT do an  
> > ioremap inside the restart function.  We may get called from  
> > interrupt context on a panic and will not be able to do the ioremap 
> > ().  The simplest thing is to do the mapping earlier in an init call  
> > and save the pointer, its not perfect, but better.
> > 
> 
> I'm beginning to hate this whole pseudo OF thing for embedded systems.

Not being able to ioremap at interrupt time has absolutely _nothing_
to do with the device tree.

> All we need to know is that we have a mpc52xx processor.

... until we get a system with a mpc52xx and some extra stuff.  Then
you say "OK, we just need a boardinfo_t" and then we get 57 different
variants of boardinfo_t and then we're back in the mess that arch/ppc
got into.

Paul.

Re: [PATCH] add restart function for mpc52xx

[Sascha Hauer]

On Fri, Jan 12, 2007 at 02:37:23PM +1100, Paul Mackerras wrote:
> Sascha Hauer writes:
> 
> > > This suffers from the same bug mpc83xx_restart has.  We can NOT do an  
> > > ioremap inside the restart function.  We may get called from  
> > > interrupt context on a panic and will not be able to do the ioremap 
> > > ().  The simplest thing is to do the mapping earlier in an init call  
> > > and save the pointer, its not perfect, but better.
> > > 
> > 
> > I'm beginning to hate this whole pseudo OF thing for embedded systems.
> 
> Not being able to ioremap at interrupt time has absolutely _nothing_
> to do with the device tree.

No, not directly..

> 
> > All we need to know is that we have a mpc52xx processor.
> 
> ... until we get a system with a mpc52xx and some extra stuff.  Then
> you say "OK, we just need a boardinfo_t" and then we get 57 different
> variants of boardinfo_t and then we're back in the mess that arch/ppc
> got into.

OK, nobody wants that. I can only compare to arm where we have one
single number per board (not per SoC, I misrepresented that). This
number is perfectly enough to know what SoC we are on (to map the
register space and select timing/irq code and the like). The rest of a
particular board is described in one single board file.
On system with real open firmware it's surely the way to go to use it,
but on systems without OF it's just painful. Not being able to implement
a restart function without changing the device tree shows that.

Sascha

-- 
 Dipl.-Ing. Sascha Hauer | http://www.pengutronix.de
  Pengutronix - Linux Solutions for Science and Industry
    Handelsregister: Amtsgericht Hildesheim, HRA 2686
      Hannoversche Str. 2, 31134 Hildesheim, Germany
    Phone: +49-5121-206917-0 |  Fax: +49-5121-206917-9

Re: [PATCH] add restart function for mpc52xx

[Sylvain Munaut]

Sascha Hauer wrote:
>> Not being able to ioremap at interrupt time has absolutely _nothing_
>> to do with the device tree.
>>     
>
> No, not directly..
>   
Not at all ... in arch/ppc we just have the bug and just never noticed.

> On system with real open firmware it's surely the way to go to use it,
> but on systems without OF it's just painful. Not being able to implement
> a restart function without changing the device tree shows that.
>   
It's because the device tree is not done correctly ... The bindings are
recent,
and may change soon. This whole issue has triggered some discussion on IRC
about them.

I'm not saying the system is perfect but this issue is more related to
recent
bindings that the of thing as a whole. That's more the problem with of
is that
when you need to define bindings for something that has none, you may not
anticipate everything ....

    Sylvain

Re: [PATCH] add restart function for mpc52xx

[Sascha Hauer]

On Fri, Jan 12, 2007 at 10:00:31AM +0100, Sylvain Munaut wrote:
> 
> I'm not saying the system is perfect but this issue is more related to
> recent
> bindings that the of thing as a whole. That's more the problem with of
> is that
> when you need to define bindings for something that has none, you may not
> anticipate everything ....

No, you can never do that. Lets hope there will never be incompatible
changes in the device tree so that we have to use this device tree for this
kernel version and another one for other kernel versions.

Sascha


-- 
 Dipl.-Ing. Sascha Hauer | http://www.pengutronix.de
  Pengutronix - Linux Solutions for Science and Industry
    Handelsregister: Amtsgericht Hildesheim, HRA 2686
      Hannoversche Str. 2, 31134 Hildesheim, Germany
    Phone: +49-5121-206917-0 |  Fax: +49-5121-206917-9

Re: [PATCH] add restart function for mpc52xx

[Sylvain Munaut]

Sascha Hauer wrote:
> On Fri, Jan 12, 2007 at 10:00:31AM +0100, Sylvain Munaut wrote:
>   
>> I'm not saying the system is perfect but this issue is more related to
>> recent
>> bindings that the of thing as a whole. That's more the problem with of
>> is that
>> when you need to define bindings for something that has none, you may not
>> anticipate everything ....
>>     
>
> No, you can never do that. Lets hope there will never be incompatible
> changes in the device tree so that we have to use this device tree for this
> kernel version and another one for other kernel versions.
>   
I would *not* base anything (production) on the current bindings, because
the changes we consider now are pretty "basic" and not compatible.

But it's clear that it's probably the last opportunity we have to do
such changes,
afterwards we'll be bound to whatever we decide in the next few weeks.


    Sylvain

Re: [PATCH] add restart function for mpc52xx

[Kumar Gala]

On Jan 12, 2007, at 4:43 AM, Sylvain Munaut wrote:

> Sascha Hauer wrote:
>> On Fri, Jan 12, 2007 at 10:00:31AM +0100, Sylvain Munaut wrote:
>>
>>> I'm not saying the system is perfect but this issue is more  
>>> related to
>>> recent
>>> bindings that the of thing as a whole. That's more the problem  
>>> with of
>>> is that
>>> when you need to define bindings for something that has none, you  
>>> may not
>>> anticipate everything ....
>>>
>>
>> No, you can never do that. Lets hope there will never be incompatible
>> changes in the device tree so that we have to use this device tree  
>> for this
>> kernel version and another one for other kernel versions.
>>
> I would *not* base anything (production) on the current bindings,  
> because
> the changes we consider now are pretty "basic" and not compatible.
>
> But it's clear that it's probably the last opportunity we have to do
> such changes,
> afterwards we'll be bound to whatever we decide in the next few weeks.

Do realize you can use the device tree in a much simpler form to pass  
the same basic information we got from the bd_t and ignore everything  
else.

The reason for all the churn (or complexity) is the lack of any  
'spec' on how to describe any of the SOC parts in the device-tree and  
thus we have to invent something.

At least that's how I see it.

- k

Re: [PATCH] add restart function for mpc52xx

[Segher Boessenkool]

>> I'm not saying the system is perfect but this issue is more related to
>> recent
>> bindings that the of thing as a whole. That's more the problem with of
>> is that
>> when you need to define bindings for something that has none, you may 
>> not
>> anticipate everything ....
>
> No, you can never do that.

It is perfectly possible to create a binding for a single
device without making any omissions/errors.

You can do a corrrect implementation, too.

Doing this for a whole platform is not more difficult,
just more works (and the law of big numbers kicks in,
you're likely to make some mistake).

> Lets hope there will never be incompatible
> changes in the device tree so that we have to use this device tree for 
> this
> kernel version and another one for other kernel versions.

That wouldn't ever be needed if your "binding" isn't
hopelessly lacking.


Segher

Re: [PATCH] add restart function for mpc52xx

[Robert Schwebel]

On Fri, Jan 12, 2007 at 01:27:14PM +0100, Segher Boessenkool wrote:
> It is perfectly possible to create a binding for a single
> device without making any omissions/errors.

Come on, come back to _this_ universe. People make mistakes, there is
even information unavailable when the firmware is delivered, firmwares
are being set in stone and may never be changed any more once an
embedded device has hit the street. For SoC devices, anything that
requires firmware changes to make Linux behave properly is simply plain
crap.

Look how rmk has solved it for ARM - Sascha has already described it.
The code that gets the information "this is an xyz board" knows
_everything_, starting from the CPU type, up to which peripherals are
there. So it simply can spawn the right platform devices, apply bugfixes
to everything a board vendor has never thought of and is even unwilling
to change in the future, because he simply doesn't care.

It's not that ARM is different than today's SoC PowerPC processors. It's
just that the arm-linux people solved the problems you are describing
here years ago.

Robert
-- 
 Dipl.-Ing. Robert Schwebel | http://www.pengutronix.de
 Pengutronix - Linux Solutions for Science and Industry
   Handelsregister:  Amtsgericht Hildesheim, HRA 2686
     Hannoversche Str. 2, 31134 Hildesheim, Germany
   Phone: +49-5121-206917-0 |  Fax: +49-5121-206917-9

Re: [PATCH] add restart function for mpc52xx

[Benjamin Herrenschmidt]

> Look how rmk has solved it for ARM - Sascha has already described it.
> The code that gets the information "this is an xyz board" knows
> _everything_, starting from the CPU type, up to which peripherals are
> there. So it simply can spawn the right platform devices, apply bugfixes
> to everything a board vendor has never thought of and is even unwilling
> to change in the future, because he simply doesn't care.
> 
> It's not that ARM is different than today's SoC PowerPC processors. It's
> just that the arm-linux people solved the problems you are describing
> here years ago.

Can we setup a filter on this mailing list rejecting anybody comparing
ARM to PowerPC -again- ? I'm tired of those useless rants.

Of course, the device-tree isn't there to solve world hunger and we
don't require people to constantly change their firmwares. Yes, a few
people on this list are probably attempting to "abuse" it and do some
kind of magic uber-board support that does everything and more and I
don't agree with that approach.

However it's actually quite nice and useful to have a well defined
firmware binding for common devices and things like interrupt routing. 

You might notice that the minimum device-tree as defined by the spec is
actually fairly small... only a couple of nodes & properties. One of
these is ... a board name. Which in a way is equivalent to your ARM
board number (except that we prefer ASCII strings to magic numbers here
is ppc land). From that is generally derived the board support data
structure.

The board code is then in total control, just like ARM or whoever else
you seem to like much better. Then, for various "services", like PCI,
interrupt routing, etc... we provide a way to easily define the whole
thing via the device-tree and the code "just works". Cool no ? Well, of
course, you -STILL- have the possibility of not using that for most
things, and to fix it up when it's wrong. You don't =have= to update the
firmware (heh, like if I had any chance to get Apple to fix their
firmwares when they have bugs).

So we are providing something that is a superset of the board number you
seem to like that much, adding more flexibility...

Anyway, enough of that. Either be constructive or return to
linuxppc-embeded.

Ben.

Re: [PATCH] add restart function for mpc52xx

[David Gibson]

On Mon, Jan 29, 2007 at 08:48:30AM +1100, Benjamin Herrenschmidt wrote:
> 
> > Look how rmk has solved it for ARM - Sascha has already described it.
> > The code that gets the information "this is an xyz board" knows
> > _everything_, starting from the CPU type, up to which peripherals are
> > there. So it simply can spawn the right platform devices, apply bugfixes
> > to everything a board vendor has never thought of and is even unwilling
> > to change in the future, because he simply doesn't care.
> > 
> > It's not that ARM is different than today's SoC PowerPC processors. It's
> > just that the arm-linux people solved the problems you are describing
> > here years ago.
> 
> Can we setup a filter on this mailing list rejecting anybody comparing
> ARM to PowerPC -again- ? I'm tired of those useless rants.
> 
> Of course, the device-tree isn't there to solve world hunger and we
> don't require people to constantly change their firmwares. Yes, a few
> people on this list are probably attempting to "abuse" it and do some
> kind of magic uber-board support that does everything and more and I
> don't agree with that approach.
> 
> However it's actually quite nice and useful to have a well defined
> firmware binding for common devices and things like interrupt routing. 
> 
> You might notice that the minimum device-tree as defined by the spec is
> actually fairly small... only a couple of nodes & properties. One of
> these is ... a board name. Which in a way is equivalent to your ARM
> board number (except that we prefer ASCII strings to magic numbers here
> is ppc land). From that is generally derived the board support data
> structure.
> 
> The board code is then in total control, just like ARM or whoever else
> you seem to like much better. Then, for various "services", like PCI,
> interrupt routing, etc... we provide a way to easily define the whole
> thing via the device-tree and the code "just works". Cool no ? Well, of
> course, you -STILL- have the possibility of not using that for most
> things, and to fix it up when it's wrong. You don't =have= to update the
> firmware (heh, like if I had any chance to get Apple to fix their
> firmwares when they have bugs).
> 
> So we are providing something that is a superset of the board number you
> seem to like that much, adding more flexibility...

And in any case the "magic board number" model can be supported within
the flat device tree model: there's no reason you can't have a
platform boot wrapper, built with the kernel, that reads the magic
board id and picks the appropriate flat device tree from its library
accordingly.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Re: [PATCH] add restart function for mpc52xx

[Segher Boessenkool]

> I'm not saying the system is perfect but this issue is more related to
> recent
> bindings that the of thing as a whole. That's more the problem with of
> is that
> when you need to define bindings for something that has none, you may 
> not
> anticipate everything ....

... which is why it is so important that you don't "define"
"bindings" "in-house".  (Sorry for all those quotes ;-) )


Segher

Re: [PATCH] add restart function for mpc52xx

[Sylvain Munaut]

Segher Boessenkool wrote:
>> I'm not saying the system is perfect but this issue is more related to
>> recent
>> bindings that the of thing as a whole. That's more the problem with of
>> is that
>> when you need to define bindings for something that has none, you may
>> not
>> anticipate everything ....
>
> ... which is why it is so important that you don't "define"
> "bindings" "in-house".  (Sorry for all those quotes ;-) )
>
Well, those were posted on the ml for everyone to read ;)


    Sylvain

Re: [PATCH] add restart function for mpc52xx

[Segher Boessenkool]

> On system with real open firmware it's surely the way to go to use it,
> but on systems without OF it's just painful. Not being able to 
> implement
> a restart function without changing the device tree shows that.

That's bullshit.  You use the watchdog timer to implement
system restart functionality; and you didn't describe the
watchdog hardware in the device tree.  You should have had
it in there already.


Segher