* Re: [PATCH] powerpc: Create "rom" (MTD) device prpmc2800
From: Segher Boessenkool @ 2007-06-13 9:10 UTC (permalink / raw)
To: David Gibson; +Cc: linuxppc-dev, linux-mtd, David Woodhouse, Milton Miller
In-Reply-To: <20070613061152.GF16148@localhost.localdomain>
>> MTD does a lot of heuristic probing. It is pretty good,
>> but it would be nice if you could skip that and hand it
>> the exact values it needs. Also, this information should
>> be in the device tree no matter what, even if Linux
>> doesn't use it yet ;-)
>
> I agree. Of course, this means figuring out what the values actually
> are for Ebony. I presently have no idea, since the heuristic probing
> appears to work without problem.
Doesn't it show it in sysfs, or in the dmesg at least?
Maybe some other important stuff is there, too.
Segher
^ permalink raw reply
* Re: [patch 29/30] PS3: Device tree source.
From: Segher Boessenkool @ 2007-06-13 9:05 UTC (permalink / raw)
To: Olaf Hering; +Cc: linuxppc-dev, paulus
In-Reply-To: <20070613084023.GA28629@aepfle.de>
>>> +/ {
>>> + model = "PS3 Game Console";
>>
>> model = "sony,some-model-number" or similar
>>
>>> + compatible = "PS3";
>>
>> name = "sony,ps3"
>
> Both are wrong.
>
> Stick to the values used right now.
> Which is 'PS3PF'
It should include "sony,", in the "name" property at least.
"model" same as "name" cannot make sense either.
Please read up on what those properties mean exactly.
It won't matter too much for Linux, but if you have a
chance too make it right for a new port, why fail to
do so?
Segher
^ permalink raw reply
* Re: [patch 29/30] PS3: Device tree source.
From: Olaf Hering @ 2007-06-13 8:40 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: linuxppc-dev, paulus
In-Reply-To: <f7c4ca74d3e2031e01264f31f09e515a@kernel.crashing.org>
On Wed, Jun 13, Segher Boessenkool wrote:
> There is one real bug though.
Indeed.
> > +/ {
> > + model = "PS3 Game Console";
>
> model = "sony,some-model-number" or similar
>
> > + compatible = "PS3";
>
> name = "sony,ps3"
Both are wrong.
Stick to the values used right now.
Which is 'PS3PF'
^ permalink raw reply
* Re: [PATCH 1/5] Add the explanation and sample of RapidIO DTS sector to the document of booting-without-of.txt file.
From: Segher Boessenkool @ 2007-06-13 8:28 UTC (permalink / raw)
To: Zhang Wei-r63237; +Cc: linuxppc-dev, paulus, linux-kernel
In-Reply-To: <46B96294322F7D458F9648B60E15112C526DD9@zch01exm26.fsl.freescale.net>
>>> + - device_type : Should be "rapidio"
>>
>> There is no OF binding, so no.
>
> So, we need to define it.
If you want to. Until that has been done, don't use
a "device_type". Linux won't use it, anyway.
>>> + - compatible : Should be "fsl,rapidio-v0.0" or
>> "fsl,rapidio-v1.0"
>>> + and so on. The version number is got from IP Block Revision
>>> + Register of RapidIO controller.
>>
>> It's better to use real device names, just like everyone
>> else.
>
> Some silicons of Freescale processor are the same RapidIO controller,
> such as mpc8540/mpc8560 are the same (v0.0), mpc8548/mpc8641 are the
> same (v1.0). For v1.0 RapidIO controller, should we use mpc8548 or
> mpc8641? Those will make people confused.
Not at all. On an 8641 it could be
compatible = "fsl,mpc8641-rapidio" "fsl,mpc8548-rapidio";
which states "this is the 8641 thing and it is compatible
to the 8548 thing". Perfectly clear.
> Using IP Block Revision is a
> clear choice.
I don't think so. For one thing, it describes a version of
a cell design, not a version of an actual device. For another
thing, if I hear "8641" I know what you're talking about (sort
of, anyway), but I draw a blank stare if you say "v1.0". I'm
sure I'm not the only one. Concrete names are good.
>>> + - #address-cells : Address representation for
>> "rapidio" devices.
>>> + This field represents the number of cells needed to represent
>>> + the RapidIO address of the registers. For
>> supporting more than
>>> + 36-bits RapidIO address, this field should be <2>.
>>
>> More than 32 bit?
>
> Yes, RapidIO bus address width is 34 bits.
You said "more than 36 bit", I tried to ask if that is a typo
perhaps.
>> No. The format of an "interrupts" entry is defined by
>> the interrupt domain this device sits in, not by the
>> device itself.
>>
> Do you misunderstand the meaning of 'interrupts'?
Hahaha. No, I don't misunderstand what the "interrupts" property
means. Perhaps you do?
> These interrupts is
> issued from the RapidIO controller to the pic controller for tx, rx,
> err, doorbell and message.
But the rapidio node doesn't know or care what the interrupts
are connected to, and neither should it. That's what the
interrupt mapping recommended practice is for.
>>> For this sector, interrupts order should be
>>> + <err_irq bell_outb_irq bell_inb_irq msg1_tx_irq msg1_rx_irq
>>> + msg2_tx_irq msg2_rx_irq ... msgN_tx_irq msgN_rx_irq>.
>>
>> That's to be defined in the binding for your specific device,
>> not in a more generic rapidio binding.
>
> These description is just for compatible="fsl,rapidio-v*.*" rapidio
> controller.
Okay, good. Please make that way more obvious then :-)
>>> + #address-cells = <2>;
>>
>> You want a #size-cells as well.
>
> The size is not used in this sector, so no defined.
The size _is_ used; in the "ranges" property in this node,
for example. It is also needed to describe the "reg" for
any child node of this node.
A non-existant "#size-cells" means 1, and "#address-cells"
means 2, so in principle you could do without these
properties; but Linux doesn't parse the tree correctly in
that case (which reminds me, I have some more patches to
send).
> Thanks!
My pleasure,
Segher
^ permalink raw reply
* Re: [PATCH v4]: Fix e500 v2 core reboot bug
From: Zang Roy-r61911 @ 2007-06-13 8:27 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: linuxppc-dev list, Paul Mackerras
In-Reply-To: <26a24ad5f123db79f880f8c53cf2c71f@kernel.crashing.org>
On Wed, 2007-06-13 at 15:54, Segher Boessenkool wrote:
[snip]
>
> > If it does not matter, I'd like to put them in one line :-).
>
> Current thinking on this seems to be that long literal strings
> are fine. You are right that some people might challenge you
> on this, of course ;-)
Then, I will say Segher tell me to do that ...
Roy
^ permalink raw reply
* RE: [PATCH 1/5] Add the explanation and sample of RapidIO DTS sector to the document of booting-without-of.txt file.
From: Zhang Wei-r63237 @ 2007-06-13 8:14 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: linuxppc-dev, paulus, linux-kernel
In-Reply-To: <39c7d60cebf03badd106d7f75a08f449@kernel.crashing.org>
Hi, Segher,=20
> -----Original Message-----
> Subject: Re: [PATCH 1/5] Add the explanation and sample of=20
> RapidIO DTS sector to the document of booting-without-of.txt file.
>=20
> > + k) RapidIO
> > +
> > + Required properties:
> > +
> > + - device_type : Should be "rapidio"
>=20
> There is no OF binding, so no.
So, we need to define it.
>=20
> > + - compatible : Should be "fsl,rapidio-v0.0" or=20
> "fsl,rapidio-v1.0"
> > + and so on. The version number is got from IP Block Revision
> > + Register of RapidIO controller.
>=20
> It's better to use real device names, just like everyone
> else.
Some silicons of Freescale processor are the same RapidIO controller,
such as mpc8540/mpc8560 are the same (v0.0), mpc8548/mpc8641 are the
same (v1.0). For v1.0 RapidIO controller, should we use mpc8548 or
mpc8641? Those will make people confused. Using IP Block Revision is a
clear choice.
>=20
> > + - #address-cells : Address representation for=20
> "rapidio" devices.
> > + This field represents the number of cells needed to represent
> > + the RapidIO address of the registers. For=20
> supporting more than
> > + 36-bits RapidIO address, this field should be <2>.
>=20
> More than 32 bit?
Yes, RapidIO bus address width is 34 bits.
>=20
> > + - interrupt-parent : the phandle for the interrupt=20
> controller that
> > + services interrupts for this device.
>=20
> Not required, depends on the interrupt tree of the system.
>=20
> > + - interrupts : <a b> where a is the interrupt number and b is a
> > + field that represents an encoding of the sense and level
> > + information for the interrupt.
>=20
> No. The format of an "interrupts" entry is defined by
> the interrupt domain this device sits in, not by the
> device itself.
>=20
Do you misunderstand the meaning of 'interrupts'? These interrupts is
issued from the RapidIO controller to the pic controller for tx, rx,
err, doorbell and message.
> > For this sector, interrupts order should be
> > + <err_irq bell_outb_irq bell_inb_irq msg1_tx_irq msg1_rx_irq
> > + msg2_tx_irq msg2_rx_irq ... msgN_tx_irq msgN_rx_irq>.
>=20
> That's to be defined in the binding for your specific device,
> not in a more generic rapidio binding.
These description is just for compatible=3D"fsl,rapidio-v*.*" rapidio
controller.
>=20
> > + #address-cells =3D <2>;
>=20
> You want a #size-cells as well.
>=20
The size is not used in this sector, so no defined.
Thanks!
Wei.
^ permalink raw reply
* RE: [PATCH v4]: Fix e500 v2 core reboot bug
From: Li Yang-r58472 @ 2007-06-13 8:16 UTC (permalink / raw)
To: Zang Roy-r61911, Segher Boessenkool; +Cc: linuxppc-dev list, Paul Mackerras
In-Reply-To: <1181722043.13195.2.camel@localhost.localdomain>
> -----Original Message-----
> From: Zang Roy-r61911 [mailto:tie-fei.zang@freescale.com]
> Sent: Wednesday, June 13, 2007 4:07 PM
> To: Segher Boessenkool
> Cc: Li Yang-r58472; linuxppc-dev list; Paul Mackerras
> Subject: Re: [PATCH v4]: Fix e500 v2 core reboot bug
>=20
> On Wed, 2007-06-13 at 15:55, Segher Boessenkool wrote:
> > > It will be good if you do:
> > > + printk (KERN_EMERG "Error: reset
> > > control"
> > > + "register not mapped,
> > > spinning!\n");
> > >
> > > Instead of:
> > > + printk (KERN_EMERG "Error: reset
> > control
> > > \
> > > + register not mapped,
> > > spinning!\n");
> >
> > Except you forgot a space. Too much whitespace in the
> > resulting string is bad, but too little doesn't help
> > either ;-)
> I noticed that space, except that I am not sure the difference between
\
> and ".
The difference is that you will get all the indentation in the next line
into the string if you use \. :-)
The output will be "Error: reset control<tab><tab><tab><tab> register
not mapped, spinning!\n"
- Leo
^ permalink raw reply
* Re: [PATCH v4]: Fix e500 v2 core reboot bug
From: Zang Roy-r61911 @ 2007-06-13 8:07 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: linuxppc-dev list, Paul Mackerras
In-Reply-To: <4470838fec2256a98e512486f8486605@kernel.crashing.org>
On Wed, 2007-06-13 at 15:55, Segher Boessenkool wrote:
> > It will be good if you do:
> > + printk (KERN_EMERG "Error: reset
> > control"
> > + "register not mapped,
> > spinning!\n");
> >
> > Instead of:
> > + printk (KERN_EMERG "Error: reset
> control
> > \
> > + register not mapped,
> > spinning!\n");
>
> Except you forgot a space. Too much whitespace in the
> resulting string is bad, but too little doesn't help
> either ;-)
I noticed that space, except that I am not sure the difference between \
and ".
I decided to leave it as two line with " not \.
Thanks
Roy
^ permalink raw reply
* Re: [PATCH v4]: Fix e500 v2 core reboot bug
From: Segher Boessenkool @ 2007-06-13 7:55 UTC (permalink / raw)
To: Li Yang-r58472; +Cc: linuxppc-dev list, Paul Mackerras
In-Reply-To: <989B956029373F45A0B8AF0297081890D2AC84@zch01exm26.fsl.freescale.net>
> It will be good if you do:
> + printk (KERN_EMERG "Error: reset
> control"
> + "register not mapped,
> spinning!\n");
>
> Instead of:
> + printk (KERN_EMERG "Error: reset control
> \
> + register not mapped,
> spinning!\n");
Except you forgot a space. Too much whitespace in the
resulting string is bad, but too little doesn't help
either ;-)
Segher
^ permalink raw reply
* Re: [PATCH v4]: Fix e500 v2 core reboot bug
From: Segher Boessenkool @ 2007-06-13 7:54 UTC (permalink / raw)
To: Zang Roy-r61911; +Cc: linuxppc-dev list, Paul Mackerras
In-Reply-To: <1181720633.12936.19.camel@localhost.localdomain>
>> There is no real reason to break the string in two anyway, it
>> is fine to have a more-than-80-char line in printk messages.
>>
> I separate it, because I am afraid I will be challenged by 80 char
> column rule.
Yeah, I understand.
> Please check the CodeStyle file and its example:
[snip]
> If it does not matter, I'd like to put them in one line :-).
Current thinking on this seems to be that long literal strings
are fine. You are right that some people might challenge you
on this, of course ;-)
Segher
^ permalink raw reply
* RE: [PATCH v4]: Fix e500 v2 core reboot bug
From: Li Yang-r58472 @ 2007-06-13 7:49 UTC (permalink / raw)
To: Zang Roy-r61911, Segher Boessenkool; +Cc: linuxppc-dev list, Paul Mackerras
In-Reply-To: <1181720633.12936.19.camel@localhost.localdomain>
> -----Original Message-----
> From: linuxppc-dev-bounces+leoli=3Dfreescale.com@ozlabs.org
[mailto:linuxppc-dev-bounces+leoli=3Dfreescale.com@ozlabs.org] On Behalf
Of
> Zang Roy-r61911
> Sent: Wednesday, June 13, 2007 3:44 PM
> To: Segher Boessenkool
> Cc: linuxppc-dev list; Paul Mackerras
> Subject: Re: [PATCH v4]: Fix e500 v2 core reboot bug
>=20
> On Wed, 2007-06-13 at 15:26, Segher Boessenkool wrote:
> > >>> + printk (KERN_EMERG "Error: reset
> > >> control \
> > >>> + register not mapped,
> > >> spinning!\n");
> > >>
> > >> ...(... "bla bla bla "
> > >> "bla bla bla\n");
> > >>
> > >> instead? Or there'll be a lot of whitespace in your string.
> > > I just do not know the difference of these two style.
> >
> > Your code puts a bunch of extra whitespace in the string (all
> > the tabs before "register not mapped"). Two strings after
> > each other (like in my example) are just pasted together.
> >
> > There is no real reason to break the string in two anyway, it
> > is fine to have a more-than-80-char line in printk messages.
> >
> I separate it, because I am afraid I will be challenged by 80 char
> column rule.
>=20
> Please check the CodeStyle file and its example:
>=20
> Statements longer than 80 columns will be broken into sensible chunks.
> Descendants are always substantially shorter than the parent and are
> placed
> substantially to the right. The same applies to function headers with
a
> long
> argument list. Long strings are as well broken into shorter strings.
> [snip]
> void fun(int a, int b, int c)
> {
> if (condition)
> printk(KERN_WARNING "Warning this is a long printk with
"
> "3 parameters a: %u b:
%u "
> "c: %u \n", a, b, c);
> else
> next_statement;
> }
It will be good if you do:
+ printk (KERN_EMERG "Error: reset
control"
+ "register not mapped,
spinning!\n");
Instead of:
+ printk (KERN_EMERG "Error: reset control
\
+ register not mapped,
spinning!\n");
- Leo
^ permalink raw reply
* Re: [PATCH v4]: Fix e500 v2 core reboot bug
From: Zang Roy-r61911 @ 2007-06-13 7:43 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: linuxppc-dev list, Paul Mackerras
In-Reply-To: <cad6a9b0bd58df6298cb5f8402e512e7@kernel.crashing.org>
On Wed, 2007-06-13 at 15:26, Segher Boessenkool wrote:
> >>> + printk (KERN_EMERG "Error: reset
> >> control \
> >>> + register not mapped,
> >> spinning!\n");
> >>
> >> ...(... "bla bla bla "
> >> "bla bla bla\n");
> >>
> >> instead? Or there'll be a lot of whitespace in your string.
> > I just do not know the difference of these two style.
>
> Your code puts a bunch of extra whitespace in the string (all
> the tabs before "register not mapped"). Two strings after
> each other (like in my example) are just pasted together.
>
> There is no real reason to break the string in two anyway, it
> is fine to have a more-than-80-char line in printk messages.
>
I separate it, because I am afraid I will be challenged by 80 char
column rule.
Please check the CodeStyle file and its example:
Statements longer than 80 columns will be broken into sensible chunks.
Descendants are always substantially shorter than the parent and are
placed
substantially to the right. The same applies to function headers with a
long
argument list. Long strings are as well broken into shorter strings.
[snip]
void fun(int a, int b, int c)
{
if (condition)
printk(KERN_WARNING "Warning this is a long printk with "
"3 parameters a: %u b: %u "
"c: %u \n", a, b, c);
else
next_statement;
}
[snip]
If it does not matter, I'd like to put them in one line :-).
Roy
^ permalink raw reply
* Re: [PATCH v4]: Fix e500 v2 core reboot bug
From: Segher Boessenkool @ 2007-06-13 7:26 UTC (permalink / raw)
To: Zang Roy-r61911; +Cc: linuxppc-dev list, Paul Mackerras
In-Reply-To: <1181718898.12936.6.camel@localhost.localdomain>
>>> + printk (KERN_EMERG "Error: reset
>> control \
>>> + register not mapped,
>> spinning!\n");
>>
>> ...(... "bla bla bla "
>> "bla bla bla\n");
>>
>> instead? Or there'll be a lot of whitespace in your string.
> I just do not know the difference of these two style.
Your code puts a bunch of extra whitespace in the string (all
the tabs before "register not mapped"). Two strings after
each other (like in my example) are just pasted together.
There is no real reason to break the string in two anyway, it
is fine to have a more-than-80-char line in printk messages.
>> I don't see anything spinning like the warning says, btw (or
>> it must be something in abort()?)
> You can find the similar thing in 83xx/mis.c
> [snip]
> } else {
> printk (KERN_EMERG "Error: Restart registers not mapped,
> spinning!\n");
> }
> [snip]
You shouldn't have snipped, right after that it says
for (;;) ;
Segher
^ permalink raw reply
* Re: [PATCH v4]: Fix e500 v2 core reboot bug
From: Zang Roy-r61911 @ 2007-06-13 7:14 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: linuxppc-dev list, Paul Mackerras
In-Reply-To: <6dc1593b8ff8442f7b4cf7a01f69c07c@kernel.crashing.org>
On Wed, 2007-06-13 at 15:03, Segher Boessenkool wrote:
> > Fix the reset bug on 8548CDS board.
>
> > Segher, do you have any comment?
>
> This one looks fine, only some cosmetics left :-)
>
> > If yes, I will revise again; if No, pick it up!
>
> Well it's not me who picks things up around here...
I know, it's Paul or Kumar! I just like to hear your valuable comment...
>
> > + printk (KERN_EMERG "Error: reset
> control \
> > + register not mapped,
> spinning!\n");
>
> ...(... "bla bla bla "
> "bla bla bla\n");
>
> instead? Or there'll be a lot of whitespace in your string.
I just do not know the difference of these two style.
> I don't see anything spinning like the warning says, btw (or
> it must be something in abort()?)
You can find the similar thing in 83xx/mis.c
[snip]
} else {
printk (KERN_EMERG "Error: Restart registers not mapped,
spinning!\n");
}
[snip]
Roy
^ permalink raw reply
* Re: [PATCH v3]: Fix e500 v2 core reboot bug
From: Segher Boessenkool @ 2007-06-13 7:04 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: linuxppc-dev list
In-Reply-To: <4d678092dfbbed51d8b89bc92259b549@kernel.crashing.org>
> 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
^ permalink raw reply
* Re: [PATCH v4]: Fix e500 v2 core reboot bug
From: Segher Boessenkool @ 2007-06-13 7:03 UTC (permalink / raw)
To: Zang Roy-r61911; +Cc: linuxppc-dev list, Paul Mackerras
In-Reply-To: <1181717615.9397.40.camel@localhost.localdomain>
> Fix the reset bug on 8548CDS board.
> Segher, do you have any comment?
This one looks fine, only some cosmetics left :-)
> If yes, I will revise again; if No, pick it up!
Well it's not me who picks things up around here...
> + printk (KERN_EMERG "Error: reset control \
> + register not mapped, spinning!\n");
...(... "bla bla bla "
"bla bla bla\n");
instead? Or there'll be a lot of whitespace in your string.
I don't see anything spinning like the warning says, btw (or
it must be something in abort()?)
Segher
^ permalink raw reply
* Re: [PATCH v3]: Fix e500 v2 core reboot bug
From: Segher Boessenkool @ 2007-06-13 6:57 UTC (permalink / raw)
To: Zang Roy-r61911; +Cc: linuxppc-dev list
In-Reply-To: <1181717035.9397.30.camel@localhost.localdomain>
>> 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
^ permalink raw reply
* [PATCH v4]: Fix e500 v2 core reboot bug
From: Zang Roy-r61911 @ 2007-06-13 6:53 UTC (permalink / raw)
To: Segher Boessenkool, Paul Mackerras, Kumar Gala; +Cc: linuxppc-dev list
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 all the previous ones :-(
Segher, do you have any comment?
If yes, I will revise again; if No, pick it up!
Thanks!
arch/powerpc/boot/dts/mpc8548cds.dts | 6 ++++++
arch/powerpc/platforms/85xx/misc.c | 32 ++++++++++++++++++++++++++++++++
2 files changed, 38 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..8286393 100644
--- a/arch/powerpc/platforms/85xx/misc.c
+++ b/arch/powerpc/platforms/85xx/misc.c
@@ -13,11 +13,43 @@
#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");
+ if (np)
+ of_node_put(np);
+ 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
^ permalink raw reply related
* Re: [PATCH v3]: Fix e500 v2 core reboot bug
From: Zang Roy-r61911 @ 2007-06-13 6:43 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: linuxppc-dev list
In-Reply-To: <1181715851.9397.22.camel@localhost.localdomain>
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
^ permalink raw reply
* Re: [PATCH v3]: Fix e500 v2 core reboot bug
From: Zang Roy-r61911 @ 2007-06-13 6:31 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: linuxppc-dev list, Paul Mackerras
In-Reply-To: <4a8562ccb7046ccde1ab81c725139878@kernel.crashing.org>
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
^ permalink raw reply
* Re: [PATCH v3]: Fix e500 v2 core reboot bug
From: Segher Boessenkool @ 2007-06-13 6:28 UTC (permalink / raw)
To: Zang Roy-r61911; +Cc: linuxppc-dev list, Paul Mackerras
In-Reply-To: <1181715391.9397.16.camel@localhost.localdomain>
> + 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
^ permalink raw reply
* Re: [PATCH v3]: Fix e500 v2 core reboot bug
From: Zang Roy-r61911 @ 2007-06-13 6:24 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: linuxppc-dev list, Paul Mackerras
In-Reply-To: <1181713501.9397.10.camel@localhost.localdomain>
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
^ permalink raw reply related
* Re: [PATCH v3]: Fix e500 v2 core reboot bug
From: Zang Roy-r61911 @ 2007-06-13 6:16 UTC (permalink / raw)
To: Paul Mackerras; +Cc: linuxppc-dev list
In-Reply-To: <f81c6f149f20c52f547f39ca0cc8d67c@kernel.crashing.org>
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
^ permalink raw reply related
* Re: [PATCH] powerpc: Create "rom" (MTD) device prpmc2800
From: David Gibson @ 2007-06-13 6:12 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: linuxppc-dev
In-Reply-To: <9fbd7a7f5cdde58768569ab23c7aec7c@kernel.crashing.org>
On Tue, Jun 12, 2007 at 12:50:25PM +0200, Segher Boessenkool wrote:
> > So, what you're suggesting is a subnode for each described partition?
>
> I'm saying this is a reasonable way to describe the regions
> of flash the firmware itself cares about.
>
> This isn't anything new; it is done like this on some
> Apple systems, for example.
>
> > Seems an awfully verbose way of going about it,
>
> Not verbose, but flexible, and in line with everything
> else about the device tree.
>
> > and I don't see what
> > it buys us over the partitions/partition-names pair of properties.
>
> It is extensible. It makes parsing trivial. It
> represents a flash partition in a way similar to how
> a "whole" flash device is represented.
Hrm, ok. Between you and BenH you've convinced me. I'll try to come
up with something. I think we'll still need to support the old
properties as backwards compatibility thing.
--
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
^ permalink raw reply
* Re: [PATCH] powerpc: Create "rom" (MTD) device prpmc2800
From: David Gibson @ 2007-06-13 6:11 UTC (permalink / raw)
To: Segher Boessenkool
Cc: linuxppc-dev, linux-mtd, David Woodhouse, Milton Miller
In-Reply-To: <96f5b23c4f53e986e8caa2156f149cd8@kernel.crashing.org>
On Wed, Jun 13, 2007 at 07:05:31AM +0200, Segher Boessenkool wrote:
> >>> Of course, Ebony has flash which is JEDEC rather than CFI probed, so
> >>> I
> >>> have an interest in a binding for that, too.
> >>
> >> For JEDEC flash, you probably want extra properties
> >> for manufacturing ID and chip ID. Also, you need
> >> to describe the block sizes in some way (on CFI, this
> >> can be probed; on JEDEC, not). Anything else?
> >
> > Hrm. At the moment it appears to work without supplying this
> > information for the Ebony flash... I wonder if there are defaults in
> > the mtd layer or something.
>
> MTD does a lot of heuristic probing. It is pretty good,
> but it would be nice if you could skip that and hand it
> the exact values it needs. Also, this information should
> be in the device tree no matter what, even if Linux
> doesn't use it yet ;-)
I agree. Of course, this means figuring out what the values actually
are for Ebony. I presently have no idea, since the heuristic probing
appears to work without problem.
--
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
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox