LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH] [POWERPC] Improved documentation of device tree 'ranges'.
From: David Gibson @ 2007-12-04  0:52 UTC (permalink / raw)
  To: Stephen Neuendorffer; +Cc: linuxppc-dev
In-Reply-To: <20071203174403.C740719B0073@mail119-sin.bigfish.com>

On Mon, Dec 03, 2007 at 09:45:03AM -0800, Stephen Neuendorffer wrote:
> I was misled by the prior language.  I've attempted to clarify how
> 'ranges' are used, in particular, how to get a 1:1 mapping.

Sounds good, except for one detail.  I think we should avoid using
the "1:1" terminology: to a mathematician, at least, "1:1" is much
weaker than "identity mapping" which is what an empty ranges actually
implies.

So, I think we should explicitly say that an empty ranges implies that
parent bus address space is identical to child bus address space.

> Signed-off-by: Stephen Neuendorffer <stephen.neuendorffer@xilinx.com>
> ---
>  Documentation/powerpc/booting-without-of.txt |   11 +++++++----
>  1 files changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/powerpc/booting-without-of.txt b/Documentation/powerpc/booting-without-of.txt
> index e9a3cb1..aad8bf5 100644
> --- a/Documentation/powerpc/booting-without-of.txt
> +++ b/Documentation/powerpc/booting-without-of.txt
> @@ -711,13 +711,14 @@ define a bus type with a more complex address format, including things
>  like address space bits, you'll have to add a bus translator to the
>  prom_parse.c file of the recent kernels for your bus type.
>  
> -The "reg" property only defines addresses and sizes (if #size-cells
> -is non-0) within a given bus. In order to translate addresses upward
> +The "reg" property only defines addresses and sizes (if #size-cells is
> +non-0) within a given bus. In order to translate addresses upward
>  (that is into parent bus addresses, and possibly into CPU physical
>  addresses), all busses must contain a "ranges" property. If the
>  "ranges" property is missing at a given level, it's assumed that
> -translation isn't possible. The format of the "ranges" property for a
> -bus is a list of:
> +translation isn't possible, i.e., the registers are not visible on the
> +parent bus.  The format of the "ranges" property for a bus is a list
> +of:
>  
>  	bus address, parent bus address, size
>  
> @@ -735,6 +736,8 @@ fit in a single 32-bit word.   New 32-bit powerpc boards should use a
>  1/1 format, unless the processor supports physical addresses greater
>  than 32-bits, in which case a 2/1 format is recommended.
>  
> +Alternatively, the "ranges" property may be empty, indicating that the
> +registers are visible on the parent bus, with 1:1 address translation.
>  
>  2) Note about "compatible" properties
>  -------------------------------------

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

* [PATCH] [POWERPC] [v2] Improved documentation of device tree 'ranges'.
From: Stephen Neuendorffer @ 2007-12-04  1:08 UTC (permalink / raw)
  To: grant.likely, david, linuxppc-dev

I was misled by the prior language.  I've attempted to clarify how
'ranges' are used, in particular, how to get an identity mapping.

Signed-off-by: Stephen Neuendorffer <stephen.neuendorffer@xilinx.com>
---
 Documentation/powerpc/booting-without-of.txt |   13 +++++++++----
 1 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/Documentation/powerpc/booting-without-of.txt b/Documentation/powerpc/booting-without-of.txt
index e9a3cb1..7327f37 100644
--- a/Documentation/powerpc/booting-without-of.txt
+++ b/Documentation/powerpc/booting-without-of.txt
@@ -711,13 +711,14 @@ define a bus type with a more complex address format, including things
 like address space bits, you'll have to add a bus translator to the
 prom_parse.c file of the recent kernels for your bus type.
 
-The "reg" property only defines addresses and sizes (if #size-cells
-is non-0) within a given bus. In order to translate addresses upward
+The "reg" property only defines addresses and sizes (if #size-cells is
+non-0) within a given bus. In order to translate addresses upward
 (that is into parent bus addresses, and possibly into CPU physical
 addresses), all busses must contain a "ranges" property. If the
 "ranges" property is missing at a given level, it's assumed that
-translation isn't possible. The format of the "ranges" property for a
-bus is a list of:
+translation isn't possible, i.e., the registers are not visible on the
+parent bus.  The format of the "ranges" property for a bus is a list
+of:
 
 	bus address, parent bus address, size
 
@@ -735,6 +736,10 @@ fit in a single 32-bit word.   New 32-bit powerpc boards should use a
 1/1 format, unless the processor supports physical addresses greater
 than 32-bits, in which case a 2/1 format is recommended.
 
+Alternatively, the "ranges" property may be empty, indicating that the
+registers are visible on the parent bus using an identity mapping
+translation.  In other words, the parent bus address space is the same
+as the child bus address space.
 
 2) Note about "compatible" properties
 -------------------------------------
-- 
1.5.3.4-dirty

^ permalink raw reply related

* Re: [PATCH 1/5] PowerPC 74xx: Katana Qp device tree
From: Mark A. Greer @ 2007-12-04  1:23 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linuxppc-dev
In-Reply-To: <1196715170.13230.228.camel@pasglop>

On Tue, Dec 04, 2007 at 07:52:50AM +1100, Benjamin Herrenschmidt wrote:
> > 	#address-cells = <1>;
> > +	#size-cells = <1>;
> > +	model = "Katana-Qp"; /* Default */
> > +	compatible = "emerson,Katana-Qp";
> > +	coherency-off;
> > +
> 
> What do that mean (coherency-off) ?
> 
> Somebody is trying again to use a 74xx with non cache coherent DMA ?

Hi Ben.

I suspect Andrei got that from the prpmc2800 dts which I made so I'll
jump in.  (BTW, this is the same debate we have every year or two. :)

By looking at the dts, that board has an mv64460 which has a couple
issues when it comes to coherency (depending on the rev of the chip).

One is about not being able to use DCBST instructions with coherency on
and the other is about limiting the length of one of the traces (which
at least one board manufacturer that I know of refuses to implement).
The first one is supposed to be fixed by rev A1 of the part and the second
is supposed to be fixed by rev B0 of the part.  I don't know what rev(s)
are on the board(s) Andrei is using.  If its B0 or later, in theory, the
part should work with coherency on.

Andrei, have you tested with coherency on?

--

As far as the prpmc2800 (mv64360)...

[I don't know what an NDA let's me say or not so I'll just give
summaries of the errata.  If you or another reader has signed the NDA
you/they can look them up.]

I don't recall all of the details anymore but these are the errata I saw
by quickly scanning the 64360's list.

- "FEr CPU-#1":

Basically the CPU could read a stale cache line.  Supposed to be fixed
in rev A2 & B0 but I haven't verified.

- "FEr MPSC-#1":

The MPSC can't access a coherent memory region.
This is pretty much a show stopper for the prpmc2800.

There are no plans to fix that erratum.

- "FEr PCI-#4" (Detailed by Application Note AN-84):

[This isn't strictly a coherency issue but having coherency enabled
exacerbates the problem.]  Basically, the bridge can let the cpu read
a pci device's registers before all of the data the PCI devices has
written has actually made it to memory.  This and the fact that the
device's write data may be stuck in the PCI Slave Write Buffer
(which isn't checked for coherency), the cpu can get stale data.

There are no plans to fix that erratum.

- "FEr PCI-#5" (Detailed by Application Note AN-85):

With certain PCI devices and with coherency enabled, some combinations
of PCI transactions can cause a deadlock.  There is a workaround
documented but I've tried it and it didn't work for me (but I can't be
sure that was the erratum I was bumping into).

There are no plans to fix that erratum.
--

So, the answer depends on what part & what rev of the part you have
(e.g., the pegasos doesn't use the MPSC and apparently has the other
issues worked around so it can turn on coherency but the prpmc2800
doesn't so it needs coherency off).

BTW, I haven't forgotten the inherent bug you described when coherency
is off (/me too lazy to find link to the email) but AFAIK I've never run
into it.  However, if I turn on coherency and stress the PCI bus, it
hangs (I can't even look at memory thru a bdi).

Mark

^ permalink raw reply

* RE: current ARCH=powerpc for v2pro.
From: Stephen Neuendorffer @ 2007-12-04  1:28 UTC (permalink / raw)
  To: Stephen Neuendorffer, Grant Likely; +Cc: linuxppc-embedded
In-Reply-To: <20071204004843.8B5FB1C20046@mail6-sin.bigfish.com>

Hmm...  This code (from arch/ppc/boot/simple/embed_config.c) appears to
help:

	static const unsigned long line_size =3D 32;
	static const unsigned long congruence_classes =3D 256;
	unsigned long addr;
	unsigned long dccr;

	/*
	 * Invalidate the data cache if the data cache is turned off.
	 * - The 405 core does not invalidate the data cache on power-up
	 *   or reset but does turn off the data cache. We cannot assume
	 *   that the cache contents are valid.
	 * - If the data cache is turned on this must have been done by
	 *   a bootloader and we assume that the cache contents are
	 *   valid.
	 */
	__asm__("mfdccr %0": "=3Dr" (dccr));
	if (dccr =3D=3D 0) {
		for (addr =3D 0;
		     addr < (congruence_classes * line_size);
		     addr +=3D line_size) {
			__asm__("dccci 0,%0": :"b"(addr));
		}
	}

Although, I'm not sure why that should be virtex specific.

Steve=20

> -----Original Message-----
> From:=20
> linuxppc-embedded-bounces+stephen=3Dneuendorffer.name@ozlabs.org
> =20
> [mailto:linuxppc-embedded-bounces+stephen=3Dneuendorffer.name@oz
> labs.org] On Behalf Of Stephen Neuendorffer
> Sent: Monday, December 03, 2007 4:49 PM
> To: Grant Likely
> Cc: linuxppc-embedded
> Subject: RE: current ARCH=3Dpowerpc for v2pro.
>=20
> I tried that, which essentially differed from what I was=20
> trying in that
> interrupts were turned off.
> It fails in the same way as before.
>=20
> I've booted ARCH=3Dppc from your tree on the exact same hardware =
design,
> and as near as I can tell, the code that runs in the kernel=20
> proper up to
> the point where I see the machine check is almost identical.
>=20
> The machine check (a trap into the Machine Check handler at 0x200)
> occurs at a nondeterministic point during the execution of=20
> memset_io in
> early_init.
>=20
> In the kernel I have, _bss_start is c02c8000, and these are the
> registers in the trap handler on two different runs of the kernel:
>=20
>     r3: c02c80cc r5: 00022874
>     r3: c02c8248 r5: 000226f4
>=20
> r3 is the current point being initialized, and r5 is the=20
> count remaining
> in the .bss.
>=20
> So, what would cause a machine check in the middle of a loop, in the
> middle of the almost the simplest code absolutely possible, and not on
> an obvious memory boundary?
>=20
> Steve
>=20
> > -----Original Message-----
> > From: glikely@secretlab.ca [mailto:glikely@secretlab.ca] On=20
> > Behalf Of Grant Likely
> > Sent: Friday, November 30, 2007 10:40 PM
> > To: Stephen Neuendorffer
> > Cc: linuxppc-embedded
> > Subject: Re: current ARCH=3Dpowerpc for v2pro.
> >=20
> > On 11/30/07, Stephen Neuendorffer=20
> > <stephen.neuendorffer@xilinx.com> wrote:
> > >
> > > Grant,
> > >
> > > I'm trying to bring up your arch/powerpc work, using a compiled in
> > > device tree.  I added this:
> > >
> > <snip>
> > >
> > > Which seems bizarre, because that code is very simple. =20
> I'm guessing
> > > that something in the memory configuration is wierd (or maybe
> > > zImage.virtex is not the right way to do this?) but I'm a=20
> > little lost
> > > where to look from here.  I also tried it with both=20
> > paulus_master and
> > > your virtex-for-2.6.24 branch.
> >=20
> >=20
> > I've got a patch that adds 'raw' image support (originally=20
> written by
> > Scott Wood) which somewhat works for booting (but not entirely; I
> > haven't had time to dig into it properly yet).  It's not suitable to
> > go into mainline yet.  I'll try to get the patch out to my tree this
> > evening... actually I've been trying to get my tree pushed out all
> > today, but other things keep coming up.  :-)
> >=20
> > <several hours after I wrote the above>
> >=20
> > Okay, I pushed my current patch set out to the master branch of my
> > linux-2.6-virtex tree.  Give it a whirl.  It's not perfect, but it
> > should be usable for booting.
> >=20
> > Cheers,
> > g.
> >=20
> > --=20
> > Grant Likely, B.Sc., P.Eng.
> > Secret Lab Technologies Ltd.
> > grant.likely@secretlab.ca
> > (403) 399-0195
> >=20
> >=20
>=20
> _______________________________________________
> Linuxppc-embedded mailing list
> Linuxppc-embedded@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-embedded
>=20
>=20

^ permalink raw reply

* Re: [PATCH] Add IPIC MSI interrupt support
From: Li Li @ 2007-12-04  1:41 UTC (permalink / raw)
  To: benh; +Cc: linuxppc-dev, Gala Kumar, Li Tony
In-Reply-To: <1196715812.13230.234.camel@pasglop>

On Tue, 2007-12-04 at 05:03 +0800, Benjamin Herrenschmidt wrote:
> 
> On Mon, 2007-12-03 at 17:07 +0800, Li Li wrote: 
> > 
> > In IPIC, the 8 MSI interrupts is handled as level intrrupt. 
> > I just provide a versatile in case it is changed.
> 
> Level ? Are you sure ? MSIs are by definition edge interrupts... Or
> do 
> you have some weird logic that asserts a level input until you go ack 
> something ? Sounds weird...
> 

The second one.
The MSI is edge interrupt. The 256 MSI interrupts are divided into 8
groups. Each group can generate a interrupt to core. This interrupts are
level and asserted untile ack MSI interrupt. A little weird.

> Ben.
> 
> 

- Tony

^ permalink raw reply

* Re: Merge dtc
From: David Woodhouse @ 2007-12-04  1:59 UTC (permalink / raw)
  To: David Gibson; +Cc: linuxppc-dev, Paul Mackerras
In-Reply-To: <20071016050217.GA9052@localhost.localdomain>


On Tue, 2007-10-16 at 15:02 +1000, David Gibson wrote:
> This very large patch incorporates a copy of dtc into the kernel
> source, in arch/powerpc/boot/dtc-src.  This means that dtc is no
> longer an external dependency to build kernels with configurations
> which need a dtb file.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> 
> Too big for the list, full patch at
> http://ozlabs.org/~dgibson/home/merge-dtc.patch

I think this is a bad idea -- it's hardly a difficult for those people
who _do_ need dts to obtain it separately.

It's bad enough that I have to separate out the bootwrapper code, which
probably ought to live outside the kernel. We shouldn't be merging
_more_ stuff in.

-- 
dwmw2

^ permalink raw reply

* RE: [PATCH] ipic: change ack operation that register is accessedonly when needed
From: Li Yang @ 2007-12-04  2:06 UTC (permalink / raw)
  To: benh; +Cc: linuxppc-dev
In-Reply-To: <1196715747.13230.233.camel@pasglop>

> -----Original Message-----
> From: Benjamin Herrenschmidt [mailto:benh@kernel.crashing.org]=20
> Sent: Tuesday, December 04, 2007 5:02 AM
> To: Li Yang
> Cc: galak@kernel.crashing.org; linuxppc-dev@ozlabs.org
> Subject: Re: [PATCH] ipic: change ack operation that register=20
> is accessedonly when needed
>=20
>=20
> >  static void ipic_ack_irq(unsigned int virq)  {
> > -	struct ipic *ipic =3D ipic_from_irq(virq);
> >  	unsigned int src =3D ipic_irq_to_hw(virq);
> > -	unsigned long flags;
> > -	u32 temp;
> > =20
> > -	spin_lock_irqsave(&ipic_lock, flags);
> > +	/* Only external interrupts in edge mode support ACK */
> > +	if (unlikely(ipic_info[src].ack &&
> > +			((get_irq_desc(virq)->status &=20
> IRQ_TYPE_SENSE_MASK) =3D=3D
> > +			IRQ_TYPE_EDGE_FALLING))) {
> > +		struct ipic *ipic =3D ipic_from_irq(virq);
> > +		unsigned long flags;
> > +		u32 temp;
> > =20
> > -	temp =3D ipic_read(ipic->regs, ipic_info[src].pend);
> > -	temp |=3D (1 << (31 - ipic_info[src].bit));
> > -	ipic_write(ipic->regs, ipic_info[src].pend, temp);
> > +		spin_lock_irqsave(&ipic_lock, flags);
> > =20
> > -	spin_unlock_irqrestore(&ipic_lock, flags);
> > +		temp =3D ipic_read(ipic->regs, ipic_info[src].ack);
> > +		temp |=3D (1 << (31 - ipic_info[src].bit));
> > +		ipic_write(ipic->regs, ipic_info[src].ack, temp);
> > +
> > +		spin_unlock_irqrestore(&ipic_lock, flags);
> > +	}
> >  }
>=20
> That doesn't look right...=20
>=20
> That should be handled by the higher level flow handler. The=20
> generic edge one calls ack and the level one mask_and_ack.=20
> Just make them do the right thing, no need to test for the=20
> flow type in the low level function.

But actually ack is called by edge and per cpu handlers.  Mask_and_ack
is also called by edge handler when the same interrupt is already in
progress.  So I don't think that ack/mask_and_ack implicates flow type
by design.

- Leo

^ permalink raw reply

* Re: [PATCH 1/5] PowerPC 74xx: Katana Qp device tree
From: Mark A. Greer @ 2007-12-04  2:10 UTC (permalink / raw)
  To: Andrei Dolnikov, linuxppc-dev
In-Reply-To: <20071203015018.GC26919@localhost.localdomain>

On Mon, Dec 03, 2007 at 12:50:18PM +1100, David Gibson wrote:
> On Thu, Nov 29, 2007 at 06:28:36PM +0300, Andrei Dolnikov wrote:
> > Device tree source file for the Emerson Katana Qp board
> > 
> > Signed-off-by: Andrei Dolnikov <adolnikov@ru.mvisa.com>
> > 
> > ---
> >  katanaqp.dts |  360 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  1 files changed, 360 insertions(+)
> > 
> > diff --git a/arch/powerpc/boot/dts/katanaqp.dts b/arch/powerpc/boot/dts/katanaqp.dts
> > new file mode 100644
> > index 0000000..98257a2
> > --- /dev/null
> > +++ b/arch/powerpc/boot/dts/katanaqp.dts
> > @@ -0,0 +1,360 @@
> > +/* Device Tree Source for Emerson Katana Qp
> > + *
> > + * Authors: Vladislav Buzov <vbuzov@ru.mvista.com>
> > + *	    Andrei Dolnikov <adolnikov@ru.mvista.com>
> > + * 
> > + * Based on prpmc8200.dts by Mark A. Greer <mgreer@mvista.com>
> > + *
> > + * 2007 (c) MontaVista, Software, Inc.  This file is licensed under
> > + * the terms of the GNU General Public License version 2.  This program
> > + * is licensed "as is" without any warranty of any kind, whether express
> > + * or implied.
> > + *
> > + */
> > +
> > +/ {
> > +	#address-cells = <1>;
> > +	#size-cells = <1>;
> > +	model = "Katana-Qp"; /* Default */
> > +	compatible = "emerson,Katana-Qp";
> > +	coherency-off;
> 
> What is this property for?

Its needed to tell the bootwrapper that the platform does not have
coherency enabled (since our policy is that you can't use a CONFIG_ option).
The bootwrapper needs to know that if/when it sets up the windows for
its I/O devices (e.g., enet, mpsc) to system memory.  I needed to do
this on the prpmc2800 because the firmware didn't set up those windows
correctly.  I don't know if the Katana Qp's firmware sets the up
correctly or not.

> > +	mv64x60@f8100000 { /* Marvell Discovery */
> > +		#address-cells = <1>;
> > +		#size-cells = <1>;
> > +		model = "mv64460";			/* Default */
> > +		compatible = "marvell,mv64x60";
> 
> Compatible properties should not have "x" asn in 64x60 here.  If
> there's a suitable name for the general register interface use that,
> otherwise use the specific model number of the earliest device to
> implement this register interface.  Later models should have a
> compatible property which lists their specific model, followed by the
> earlier model number with which they're compatible.

This came from the prpmc2800's dts which has become out-of-date.
Both dts files need to be updated.

> > +		ethernet@2000 {
> > +			reg = <2000 2000>;
> 
> Are the registers for the 3 ethernets really all together?  This bank
> can't be subdivided into seperate register blocks for each MAC?

Unfortunately there are some registers that are shared so you can't
divide them up nicely.

> > +			eth0 {
> > +				device_type = "network";
> > +				compatible = "marvell,mv64x60-eth";
> > +				block-index = <0>;
> 
> This block-index thing is crap.  If you really need to subindex nodes
> like this, use "reg", with an appropriate #address-cells in the
> parent, then the nodes will also get sensible unit addresses.

So how would that work for the "PHY Address Register 0x2000", say,
where bits 0-4 set the device addr for PHY 0; bits 5-9 set the device
addr for PHY 1; bts 10-14 set the devce addr for PHY 2?

> > +				interrupts = <20>;
> > +				interrupt-parent = <&/mv64x60/pic>;
> 
> You should use a label for the PIC to make things more readable.

More that needs to be updated in prpmc2800.  :(

> > +		sdma@4000 {
> > +			compatible = "marvell,mv64x60-sdma";
> > +			reg = <4000 c18>;
> > +			virtual-reg = <f8104000>;
> 
> Why does this node have virtual-reg?

"virtual-reg" is a special property that doesn't get translated thru
the parent mappings.  It should never be used in the kernel.  Its
purpose is to give code in the bootwrapper the exact address that it
should use to access a register or block of registers or ...
Its needed here because the MPSC (serial) driver uses the SDMA unit
to perform console I/O in the bootwrapper (e.g., cmdline editing, printf's).

Yes, this needs to be documented.

> > +		mpsc@8000 {
> > +			device_type = "serial";
> > +			compatible = "marvell,mpsc";
> > +			reg = <8000 38>;
> > +			virtual-reg = <f8108000>;
> > +			sdma = <&/mv64x60/sdma@4000>;
> > +			brg = <&/mv64x60/brg@b200>;
> > +			cunit = <&/mv64x60/cunit@f200>;
> > +			mpscrouting = <&/mv64x60/mpscrouting@b400>;
> > +			mpscintr = <&/mv64x60/mpscintr@b800>;
> > +			block-index = <0>;
> 
> What is this block-index thing about here?  Since the devices are
> disambiguated by their register address, why do you need it?

This particular one is needed to access the correct MPSC interrupt reg.
Maybe it would be better to make a new property for this but it was only
one reg and block-index was already there and basically served that
purpose so I used it.  I'd be happy to use an alternative if you have
something you think is better.

> > +		pic {
> 
> Needs a unit address.

Okay.

> > +		cpu-error@0070 {
> 
> The unit address should notr include leading zeroes.

Okay.

Mark

^ permalink raw reply

* Re: [PATCH 4/5] PowerPC 74xx: Katana Qp base support
From: Mark A. Greer @ 2007-12-04  2:12 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linuxppc-dev
In-Reply-To: <1196715299.13230.230.camel@pasglop>

On Tue, Dec 04, 2007 at 07:54:59AM +1100, Benjamin Herrenschmidt wrote:
> 
> On Thu, 2007-11-29 at 18:42 +0300, Andrei Dolnikov wrote:
> > +config PPC_KATANAQP
> > +       bool "Emerson-Katana Qp"
> > +       depends on EMBEDDED6xx
> > +       select MV64X60
> > +       select NOT_COHERENT_CACHE
>                  ^^^^^^^^^^^^^^^^^^
> 
> Just one word: ARGHHHHHHHH !
> 
> Oh and another one: WHY ?

I responded to your other email regarding this.

Mark

^ permalink raw reply

* Re: [PATCH 1/5] PowerPC 74xx: Katana Qp device tree
From: Benjamin Herrenschmidt @ 2007-12-04  2:14 UTC (permalink / raw)
  To: Mark A. Greer; +Cc: linuxppc-dev
In-Reply-To: <20071204012329.GA18903@mag.az.mvista.com>


 .../... (snip scary bunch of errata)

> - "FEr PCI-#4" (Detailed by Application Note AN-84):
> 
> [This isn't strictly a coherency issue but having coherency enabled
> exacerbates the problem.]  Basically, the bridge can let the cpu read
> a pci device's registers before all of the data the PCI devices has
> written has actually made it to memory.  This and the fact that the
> device's write data may be stuck in the PCI Slave Write Buffer
> (which isn't checked for coherency), the cpu can get stale data.
> 
> There are no plans to fix that erratum.

So if I understand correctly, there's no plan to fix a major PCI spec
violation which prevent any kind of reliable implementation whatsoever ?

Or rather... the part just doesn't work, period. Don't use it. If you
do, you're on your own.

> - "FEr PCI-#5" (Detailed by Application Note AN-85):
> 
> With certain PCI devices and with coherency enabled, some combinations
> of PCI transactions can cause a deadlock.  There is a workaround
> documented but I've tried it and it didn't work for me (but I can't be
> sure that was the erratum I was bumping into).
> 
> There are no plans to fix that erratum.

Yeah... great. Oh well, Paul, what about we just don't support people
using that chip ?

> So, the answer depends on what part & what rev of the part you have
> (e.g., the pegasos doesn't use the MPSC and apparently has the other
> issues worked around so it can turn on coherency but the prpmc2800
> doesn't so it needs coherency off).
> 
> BTW, I haven't forgotten the inherent bug you described when coherency
> is off (/me too lazy to find link to the email) but AFAIK I've never run
> into it.  However, if I turn on coherency and stress the PCI bus, it
> hangs (I can't even look at memory thru a bdi).

Well, as it is today, the "classic" MMU code cannot deal with !coherent.
The entire linear mapping is always mapped cacheable with BATs, so stuff
may be brought into the cache at any time, potentially polluting DMA
data.

Dealing with that would be hard. It might be possible by using G on the
entire linear mapping like we do on 4xx (yuck), and/or by not using
D-BATs (the kernel will blow up in various areas without I-BATs).

Ben.

^ permalink raw reply

* RE: [PATCH] ipic: change ack operation that register is accessedonly when needed
From: Benjamin Herrenschmidt @ 2007-12-04  2:15 UTC (permalink / raw)
  To: Li Yang; +Cc: linuxppc-dev
In-Reply-To: <989B956029373F45A0B8AF029708189001B4D2DE@zch01exm26.fsl.freescale.net>


On Tue, 2007-12-04 at 10:06 +0800, Li Yang wrote:
> > That should be handled by the higher level flow handler. The 
> > generic edge one calls ack and the level one mask_and_ack. 
> > Just make them do the right thing, no need to test for the 
> > flow type in the low level function.
> 
> But actually ack is called by edge and per cpu handlers.  Mask_and_ack
> is also called by edge handler when the same interrupt is already in
> progress.  So I don't think that ack/mask_and_ack implicates flow type
> by design.

They do and you can pass different irq_chip with different mask/ack
routines if necessary.

Ben.

^ permalink raw reply

* Re: [PATCH] [POWERPC] [v2] Improved documentation of device tree 'ranges'.
From: David Gibson @ 2007-12-04  2:40 UTC (permalink / raw)
  To: Stephen Neuendorffer; +Cc: linuxppc-dev
In-Reply-To: <20071204010818.AD4231798062@mail212-sin.bigfish.com>


On Mon, Dec 03, 2007 at 05:08:57PM -0800, Stephen Neuendorffer wrote:
> I was misled by the prior language.  I've attempted to clarify how
> 'ranges' are used, in particular, how to get an identity mapping.
> 
> Signed-off-by: Stephen Neuendorffer <stephen.neuendorffer@xilinx.com>

Acked-by: David Gibson <david@gibson.dropbear.id.au>

Thanks for the update.  I particularly dislike the "1:1" terminology,
because in maths-speak *any* ranges translation is 1:1, unless it
includes overlapping ranges.

-- 
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 1/5] PowerPC 74xx: Katana Qp device tree
From: David Gibson @ 2007-12-04  2:50 UTC (permalink / raw)
  To: Mark A. Greer; +Cc: linuxppc-dev
In-Reply-To: <20071204021026.GB18903@mag.az.mvista.com>

On Mon, Dec 03, 2007 at 07:10:26PM -0700, Mark A. Greer wrote:
> On Mon, Dec 03, 2007 at 12:50:18PM +1100, David Gibson wrote:
> > On Thu, Nov 29, 2007 at 06:28:36PM +0300, Andrei Dolnikov wrote:
[snip]
> > > +		ethernet@2000 {
> > > +			reg = <2000 2000>;
> > 
> > Are the registers for the 3 ethernets really all together?  This bank
> > can't be subdivided into seperate register blocks for each MAC?
> 
> Unfortunately there are some registers that are shared so you can't
> divide them up nicely.

Ok, fair enough then.  But, see below..

> > > +			eth0 {
> > > +				device_type = "network";
> > > +				compatible = "marvell,mv64x60-eth";
> > > +				block-index = <0>;
> > 
> > This block-index thing is crap.  If you really need to subindex nodes
> > like this, use "reg", with an appropriate #address-cells in the
> > parent, then the nodes will also get sensible unit addresses.
> 
> So how would that work for the "PHY Address Register 0x2000", say,
> where bits 0-4 set the device addr for PHY 0; bits 5-9 set the device
> addr for PHY 1; bts 10-14 set the devce addr for PHY 2?

So use 'reg' to do the indexing.  As long as you have no 'ranges'
property in the parent 'ethernet' node, which you don't, you can use
'reg' as a private index.  That's basically what non-translatable reg
values are about.

Incidentally you should probably call the subnodes "ethernet@0"
etc. and the parent one "multiethernet" or something.  It's the
subnodes that represent an individual ethernet interface, so they
should take the "ethernet" name and not the parent, by generic names
conventions.

[snip]
> > > +		sdma@4000 {
> > > +			compatible = "marvell,mv64x60-sdma";
> > > +			reg = <4000 c18>;
> > > +			virtual-reg = <f8104000>;
> > 
> > Why does this node have virtual-reg?
> 
> "virtual-reg" is a special property that doesn't get translated thru
> the parent mappings.  It should never be used in the kernel.  Its
> purpose is to give code in the bootwrapper the exact address that it
> should use to access a register or block of registers or ...
> Its needed here because the MPSC (serial) driver uses the SDMA unit
> to perform console I/O in the bootwrapper (e.g., cmdline editing, printf's).
> 
> Yes, this needs to be documented.

Ok.  "it's used for serial in the bootwrapper" would have sufficed - I
questioned it because it wasn't obvious that this was needed to use
the mpsc.

> 
> > > +		mpsc@8000 {
> > > +			device_type = "serial";
> > > +			compatible = "marvell,mpsc";
> > > +			reg = <8000 38>;
> > > +			virtual-reg = <f8108000>;
> > > +			sdma = <&/mv64x60/sdma@4000>;
> > > +			brg = <&/mv64x60/brg@b200>;
> > > +			cunit = <&/mv64x60/cunit@f200>;
> > > +			mpscrouting = <&/mv64x60/mpscrouting@b400>;
> > > +			mpscintr = <&/mv64x60/mpscintr@b800>;
> > > +			block-index = <0>;
> > 
> > What is this block-index thing about here?  Since the devices are
> > disambiguated by their register address, why do you need it?
> 
> This particular one is needed to access the correct MPSC interrupt reg.
> Maybe it would be better to make a new property for this but it was only
> one reg and block-index was already there and basically served that
> purpose so I used it.  I'd be happy to use an alternative if you have
> something you think is better.

No, that's an acceptable use for something like this, except that
"cell-index" seems to be the name we've standardised on for other
similar cases.

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

* iSeries_defconfig update
From: Stephen Rothwell @ 2007-12-04  3:06 UTC (permalink / raw)
  To: paulus; +Cc: ppc-dev

[-- Attachment #1: Type: text/plain, Size: 248 bytes --]

Hi Paul,

I think http://patchwork.ozlabs.org/linuxppc/patch?id=14835 should go
into 2.6.24 (along with some other defconfig updates).

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply

* Re: Merge dtc
From: David Gibson @ 2007-12-04  3:10 UTC (permalink / raw)
  To: David Woodhouse; +Cc: linuxppc-dev, Paul Mackerras
In-Reply-To: <1196733544.13978.201.camel@pmac.infradead.org>

On Tue, Dec 04, 2007 at 01:59:04AM +0000, David Woodhouse wrote:
> 
> On Tue, 2007-10-16 at 15:02 +1000, David Gibson wrote:
> > This very large patch incorporates a copy of dtc into the kernel
> > source, in arch/powerpc/boot/dtc-src.  This means that dtc is no
> > longer an external dependency to build kernels with configurations
> > which need a dtb file.
> > 
> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > 
> > Too big for the list, full patch at
> > http://ozlabs.org/~dgibson/home/merge-dtc.patch
> 
> I think this is a bad idea -- it's hardly a difficult for those people
> who _do_ need dts to obtain it separately.
> 
> It's bad enough that I have to separate out the bootwrapper code, which
> probably ought to live outside the kernel. We shouldn't be merging
> _more_ stuff in.

We've been back and forth on this several times, Paul and I finally
concluded this was the better option.

It means we can feel free to use dtc for whatever new platforms we
wish to without people whinging about having to install a new tool.
And it means as dtc evolves to have new useful features, we just need
to ensure that the dts files in the kernel are buildable with the dtc
in the kernel, rather than requireing people to keep updating their
dtc to match what the kernel build needs.

-- 
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 2/2] [POWERPC] Use of_register_driver to implement of_register_platform_driver
From: Stephen Rothwell @ 2007-12-04  3:11 UTC (permalink / raw)
  To: paulus; +Cc: ppc-dev

[-- Attachment #1: Type: text/plain, Size: 221 bytes --]

Hi Paul,

Is there some reason http://patchwork.ozlabs.org/linuxppc/patch?id=14223
should not be in 2.6.25?

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply

* Re: [PATCH] [POWERPC] [v2] Improved documentation of device tree 'ranges'.
From: Grant Likely @ 2007-12-04  3:22 UTC (permalink / raw)
  To: Stephen Neuendorffer, grant.likely, linuxppc-dev
In-Reply-To: <20071204024025.GG32577@localhost.localdomain>

On 12/3/07, David Gibson <david@gibson.dropbear.id.au> wrote:
>
> On Mon, Dec 03, 2007 at 05:08:57PM -0800, Stephen Neuendorffer wrote:
> > I was misled by the prior language.  I've attempted to clarify how
> > 'ranges' are used, in particular, how to get an identity mapping.
> >
> > Signed-off-by: Stephen Neuendorffer <stephen.neuendorffer@xilinx.com>
>
> Acked-by: David Gibson <david@gibson.dropbear.id.au>

Acked-by: Grant Likely <grant.likely@secretlab.ca>

>
> Thanks for the update.  I particularly dislike the "1:1" terminology,
> because in maths-speak *any* ranges translation is 1:1, unless it
> includes overlapping ranges.
>
> --
> 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
>


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

^ permalink raw reply

* dtc:  Add may const qualifications
From: David Gibson @ 2007-12-04  3:26 UTC (permalink / raw)
  To: Jon Loeliger; +Cc: linuxppc-dev

This adds 'const' qualifiers to many variables and functions.  In
particular it's now used for passing names to the tree accesor
functions.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>

Index: dtc/data.c
===================================================================
--- dtc.orig/data.c	2007-12-04 11:49:54.000000000 +1100
+++ dtc/data.c	2007-12-04 14:11:35.000000000 +1100
@@ -64,7 +64,7 @@ struct data data_grow_for(struct data d,
 	return nd;
 }
 
-struct data data_copy_mem(char *mem, int len)
+struct data data_copy_mem(const char *mem, int len)
 {
 	struct data d;
 
@@ -76,7 +76,7 @@ struct data data_copy_mem(char *mem, int
 	return d;
 }
 
-static char get_oct_char(char *s, int *i)
+static char get_oct_char(const char *s, int *i)
 {
 	char x[4];
 	char *endx;
@@ -98,7 +98,7 @@ static char get_oct_char(char *s, int *i
 	return val;
 }
 
-static char get_hex_char(char *s, int *i)
+static char get_hex_char(const char *s, int *i)
 {
 	char x[3];
 	char *endx;
@@ -117,7 +117,7 @@ static char get_hex_char(char *s, int *i
 	return val;
 }
 
-struct data data_copy_escape_string(char *s, int len)
+struct data data_copy_escape_string(const char *s, int len)
 {
 	int i = 0;
 	struct data d;
@@ -194,7 +194,7 @@ struct data data_copy_file(FILE *f, size
 	return d;
 }
 
-struct data data_append_data(struct data d, void *p, int len)
+struct data data_append_data(struct data d, const void *p, int len)
 {
 	d = data_grow_for(d, len);
 	memcpy(d.val + d.len, p, len);
@@ -237,7 +237,7 @@ struct data data_append_cell(struct data
 	return data_append_data(d, &beword, sizeof(beword));
 }
 
-struct data data_append_re(struct data d, struct fdt_reserve_entry *re)
+struct data data_append_re(struct data d, const struct fdt_reserve_entry *re)
 {
 	struct fdt_reserve_entry bere;
 
Index: dtc/dtc.c
===================================================================
--- dtc.orig/dtc.c	2007-12-04 11:49:55.000000000 +1100
+++ dtc/dtc.c	2007-12-04 14:16:51.000000000 +1100
@@ -30,7 +30,7 @@ int quiet;		/* Level of quietness */
 int reservenum;		/* Number of memory reservation slots */
 int minsize;		/* Minimum blob size */
 
-char *join_path(char *path, char *name)
+char *join_path(const char *path, const char *name)
 {
 	int lenp = strlen(path);
 	int lenn = strlen(name);
@@ -54,10 +54,10 @@ char *join_path(char *path, char *name)
 	return str;
 }
 
-void fill_fullpaths(struct node *tree, char *prefix)
+void fill_fullpaths(struct node *tree, const char *prefix)
 {
 	struct node *child;
-	char *unit;
+	const char *unit;
 
 	tree->fullpath = join_path(prefix, tree->name);
 
@@ -109,11 +109,11 @@ static void  __attribute__ ((noreturn)) 
 int main(int argc, char *argv[])
 {
 	struct boot_info *bi;
-	char *inform = "dts";
-	char *outform = "dts";
-	char *outname = "-";
+	const char *inform = "dts";
+	const char *outform = "dts";
+	const char *outname = "-";
 	int force = 0, check = 0;
-	char *arg;
+	const char *arg;
 	int opt;
 	FILE *inf = NULL;
 	FILE *outf = NULL;
Index: dtc/flattree.c
===================================================================
--- dtc.orig/flattree.c	2007-12-04 11:49:54.000000000 +1100
+++ dtc/flattree.c	2007-12-04 14:22:47.000000000 +1100
@@ -51,9 +51,9 @@ struct emitter {
 	void (*string)(void *, char *, int);
 	void (*align)(void *, int);
 	void (*data)(void *, struct data);
-	void (*beginnode)(void *, char *);
-	void (*endnode)(void *, char *);
-	void (*property)(void *, char *);
+	void (*beginnode)(void *, const char *);
+	void (*endnode)(void *, const char *);
+	void (*property)(void *, const char *);
 };
 
 static void bin_emit_cell(void *e, cell_t val)
@@ -88,17 +88,17 @@ static void bin_emit_data(void *e, struc
 	*dtbuf = data_append_data(*dtbuf, d.val, d.len);
 }
 
-static void bin_emit_beginnode(void *e, char *label)
+static void bin_emit_beginnode(void *e, const char *label)
 {
 	bin_emit_cell(e, FDT_BEGIN_NODE);
 }
 
-static void bin_emit_endnode(void *e, char *label)
+static void bin_emit_endnode(void *e, const char *label)
 {
 	bin_emit_cell(e, FDT_END_NODE);
 }
 
-static void bin_emit_property(void *e, char *label)
+static void bin_emit_property(void *e, const char *label)
 {
 	bin_emit_cell(e, FDT_PROP);
 }
@@ -113,14 +113,14 @@ static struct emitter bin_emitter = {
 	.property = bin_emit_property,
 };
 
-static void emit_label(FILE *f, char *prefix, char *label)
+static void emit_label(FILE *f, const char *prefix, const char *label)
 {
 	fprintf(f, "\t.globl\t%s_%s\n", prefix, label);
 	fprintf(f, "%s_%s:\n", prefix, label);
 	fprintf(f, "_%s_%s:\n", prefix, label);
 }
 
-static void emit_offset_label(FILE *f, char *label, int offset)
+static void emit_offset_label(FILE *f, const char *label, int offset)
 {
 	fprintf(f, "\t.globl\t%s\n", label);
 	fprintf(f, "%s\t= . + %d\n", label, offset);
@@ -191,7 +191,7 @@ static void asm_emit_data(void *e, struc
 	assert(off == d.len);
 }
 
-static void asm_emit_beginnode(void *e, char *label)
+static void asm_emit_beginnode(void *e, const char *label)
 {
 	FILE *f = e;
 
@@ -202,7 +202,7 @@ static void asm_emit_beginnode(void *e, 
 	fprintf(f, "\t.long\tFDT_BEGIN_NODE\n");
 }
 
-static void asm_emit_endnode(void *e, char *label)
+static void asm_emit_endnode(void *e, const char *label)
 {
 	FILE *f = e;
 
@@ -213,7 +213,7 @@ static void asm_emit_endnode(void *e, ch
 	}
 }
 
-static void asm_emit_property(void *e, char *label)
+static void asm_emit_property(void *e, const char *label)
 {
 	FILE *f = e;
 
@@ -234,7 +234,7 @@ static struct emitter asm_emitter = {
 	.property = asm_emit_property,
 };
 
-static int stringtable_insert(struct data *d, char *str)
+static int stringtable_insert(struct data *d, const char *str)
 {
 	int i;
 
@@ -432,7 +432,7 @@ void dt_to_blob(FILE *f, struct boot_inf
 
 static void dump_stringtable_asm(FILE *f, struct data strbuf)
 {
-	char *p;
+	const char *p;
 	int len;
 
 	p = strbuf.val;
@@ -450,7 +450,7 @@ void dt_to_asm(FILE *f, struct boot_info
 	int i;
 	struct data strbuf = empty_data;
 	struct reserve_info *re;
-	char *symprefix = "dt";
+	const char *symprefix = "dt";
 
 	for (i = 0; i < ARRAY_SIZE(version_table); i++) {
 		if (version_table[i].version == version)
@@ -594,7 +594,7 @@ static void flat_realign(struct inbuf *i
 static char *flat_read_string(struct inbuf *inb)
 {
 	int len = 0;
-	char *p = inb->ptr;
+	const char *p = inb->ptr;
 	char *str;
 
 	do {
@@ -631,7 +631,7 @@ static struct data flat_read_data(struct
 
 static char *flat_read_stringtable(struct inbuf *inb, int offset)
 {
-	char *p;
+	const char *p;
 
 	p = inb->base + offset;
 	while (1) {
@@ -673,7 +673,7 @@ static struct reserve_info *flat_read_me
 {
 	struct reserve_info *reservelist = NULL;
 	struct reserve_info *new;
-	char *p;
+	const char *p;
 	struct fdt_reserve_entry re;
 
 	/*
@@ -698,9 +698,9 @@ static struct reserve_info *flat_read_me
 }
 
 
-static char *nodename_from_path(char *ppath, char *cpath)
+static char *nodename_from_path(const char *ppath, const char *cpath)
 {
-	char *lslash;
+	const char *lslash;
 	int plen;
 
 	lslash = strrchr(cpath, '/');
@@ -724,9 +724,9 @@ static char *nodename_from_path(char *pp
 static const char PROPCHAR[] = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789,._+*#?-";
 static const char UNITCHAR[] = "0123456789abcdef,";
 
-static int check_node_name(char *name)
+static int check_node_name(const char *name)
 {
-	char *atpos;
+	const char *atpos;
 	int basenamelen;
 
 	atpos = strrchr(name, '@');
@@ -748,7 +748,7 @@ static int check_node_name(char *name)
 
 static struct node *unflatten_tree(struct inbuf *dtbuf,
 				   struct inbuf *strbuf,
-				   char *parent_path, int flags)
+				   const char *parent_path, int flags)
 {
 	struct node *node;
 	u32 val;
Index: dtc/livetree.c
===================================================================
--- dtc.orig/livetree.c	2007-12-04 11:49:54.000000000 +1100
+++ dtc/livetree.c	2007-12-04 14:13:56.000000000 +1100
@@ -180,7 +180,7 @@ struct boot_info *build_boot_info(struct
  * Tree accessor functions
  */
 
-char *get_unitname(struct node *node)
+const char *get_unitname(struct node *node)
 {
 	if (node->name[node->basenamelen] == '\0')
 		return "";
@@ -188,7 +188,7 @@ char *get_unitname(struct node *node)
 		return node->name + node->basenamelen + 1;
 }
 
-struct property *get_property(struct node *node, char *propname)
+struct property *get_property(struct node *node, const char *propname)
 {
 	struct property *prop;
 
@@ -205,7 +205,7 @@ cell_t propval_cell(struct property *pro
 	return be32_to_cpu(*((cell_t *)prop->val.val));
 }
 
-struct node *get_subnode(struct node *node, char *nodename)
+struct node *get_subnode(struct node *node, const char *nodename)
 {
 	struct node *child;
 
@@ -216,9 +216,9 @@ struct node *get_subnode(struct node *no
 	return NULL;
 }
 
-struct node *get_node_by_path(struct node *tree, char *path)
+struct node *get_node_by_path(struct node *tree, const char *path)
 {
-	char *p;
+	const char *p;
 	struct node *child;
 
 	if (!path || ! (*path))
@@ -275,7 +275,7 @@ struct node *get_node_by_phandle(struct 
 	return NULL;
 }
 
-struct node *get_node_by_ref(struct node *tree, char *ref)
+struct node *get_node_by_ref(struct node *tree, const char *ref)
 {
 	if (ref[0] == '/')
 		return get_node_by_path(tree, ref);
Index: dtc/dtc.h
===================================================================
--- dtc.orig/dtc.h	2007-12-04 11:49:55.000000000 +1100
+++ dtc/dtc.h	2007-12-04 14:17:24.000000000 +1100
@@ -133,14 +133,14 @@ void data_free(struct data d);
 
 struct data data_grow_for(struct data d, int xlen);
 
-struct data data_copy_mem(char *mem, int len);
-struct data data_copy_escape_string(char *s, int len);
+struct data data_copy_mem(const char *mem, int len);
+struct data data_copy_escape_string(const char *s, int len);
 struct data data_copy_file(FILE *f, size_t len);
 
-struct data data_append_data(struct data d, void *p, int len);
+struct data data_append_data(struct data d, const void *p, int len);
 struct data data_merge(struct data d1, struct data d2);
 struct data data_append_cell(struct data d, cell_t word);
-struct data data_append_re(struct data d, struct fdt_reserve_entry *re);
+struct data data_append_re(struct data d, const struct fdt_reserve_entry *re);
 struct data data_append_addr(struct data d, u64 addr);
 struct data data_append_byte(struct data d, uint8_t byte);
 struct data data_append_zeroes(struct data d, int len);
@@ -199,14 +199,14 @@ struct node *chain_node(struct node *fir
 void add_property(struct node *node, struct property *prop);
 void add_child(struct node *parent, struct node *child);
 
-char *get_unitname(struct node *node);
-struct property *get_property(struct node *node, char *propname);
+const char *get_unitname(struct node *node);
+struct property *get_property(struct node *node, const char *propname);
 cell_t propval_cell(struct property *prop);
-struct node *get_subnode(struct node *node, char *nodename);
-struct node *get_node_by_path(struct node *tree, char *path);
+struct node *get_subnode(struct node *node, const char *nodename);
+struct node *get_node_by_path(struct node *tree, const char *path);
 struct node *get_node_by_label(struct node *tree, const char *label);
 struct node *get_node_by_phandle(struct node *tree, cell_t phandle);
-struct node *get_node_by_ref(struct node *tree, char *ref);
+struct node *get_node_by_ref(struct node *tree, const char *ref);
 cell_t get_node_phandle(struct node *root, struct node *node);
 
 /* Boot info (tree plus memreserve information */
@@ -255,11 +255,11 @@ struct boot_info *dt_from_source(const c
 
 /* FS trees */
 
-struct boot_info *dt_from_fs(char *dirname);
+struct boot_info *dt_from_fs(const char *dirname);
 
 /* misc */
 
-char *join_path(char *path, char *name);
-void fill_fullpaths(struct node *tree, char *prefix);
+char *join_path(const char *path, const char *name);
+void fill_fullpaths(struct node *tree, const char *prefix);
 
 #endif /* _DTC_H */
Index: dtc/fstree.c
===================================================================
--- dtc.orig/fstree.c	2007-12-04 14:17:03.000000000 +1100
+++ dtc/fstree.c	2007-12-04 14:17:34.000000000 +1100
@@ -23,7 +23,7 @@
 #include <dirent.h>
 #include <sys/stat.h>
 
-static struct node *read_fstree(char *dirname)
+static struct node *read_fstree(const char *dirname)
 {
 	DIR *d;
 	struct dirent *de;
@@ -80,7 +80,7 @@ static struct node *read_fstree(char *di
 	return tree;
 }
 
-struct boot_info *dt_from_fs(char *dirname)
+struct boot_info *dt_from_fs(const char *dirname)
 {
 	struct node *tree;
 
Index: dtc/treesource.c
===================================================================
--- dtc.orig/treesource.c	2007-12-04 14:18:20.000000000 +1100
+++ dtc/treesource.c	2007-12-04 14:18:46.000000000 +1100
@@ -58,7 +58,7 @@ int isstring(char c)
 
 static void write_propval_string(FILE *f, struct data val)
 {
-	char *str = val.val;
+	const char *str = val.val;
 	int i;
 	int newchunk = 1;
 	struct marker *m = val.markers;
@@ -161,7 +161,7 @@ static void write_propval_cells(FILE *f,
 static void write_propval_bytes(FILE *f, struct data val)
 {
 	void *propend = val.val + val.len;
-	char *bp = val.val;
+	const char *bp = val.val;
 	struct marker *m = val.markers;
 
 	fprintf(f, "[");
@@ -189,7 +189,7 @@ static void write_propval_bytes(FILE *f,
 static void write_propval(FILE *f, struct property *prop)
 {
 	int len = prop->val.len;
-	char *p = prop->val.val;
+	const char *p = prop->val.val;
 	struct marker *m = prop->val.markers;
 	int nnotstring = 0, nnul = 0;
 	int nnotstringlbl = 0, nnotcelllbl = 0;

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

* [PATCH 2.6.24] pasemi_mac: Fix reuse of free'd skb
From: Olof Johansson @ 2007-12-04  3:34 UTC (permalink / raw)
  To: jgarzik; +Cc: ranger, netdev, dwmw2, linuxppc-dev

Turns out we're freeing the skb when we detect CRC error, but we're
not clearing out info->skb. We could either clear it and have the stack
reallocate it, or just leave it and the rx ring refill code will reuse
the one that was allocated.

Reusing a freed skb obviously caused some nasty crashes of various kind,
as reported by Brent Baude and David Woodhouse.


Signed-off-by: Olof Johansson <olof@lixom.net>

---

Jeff, I'd like to see this in 2.6.24, it's causing some real problems
out there. It's not needed in the 2.6.25 queue since the other changes
there have already covered these cases.

My test network at home is quiet enough to not cause CRC errors, we
mainly get those during interface bringup before speed is configured.

diff --git a/drivers/net/pasemi_mac.c b/drivers/net/pasemi_mac.c
index 09b4fde..6617e24 100644
--- a/drivers/net/pasemi_mac.c
+++ b/drivers/net/pasemi_mac.c
@@ -586,7 +586,7 @@ static int pasemi_mac_clean_rx(struct pasemi_mac *mac, int limit)
 			/* CRC error flagged */
 			mac->netdev->stats.rx_errors++;
 			mac->netdev->stats.rx_crc_errors++;
-			dev_kfree_skb_irq(skb);
+			/* No need to free skb, it'll be reused */
 			goto next;
 		}
 

^ permalink raw reply related

* [PATCH] Fix hardware IRQ time accounting problem.
From: Tony Breeds @ 2007-12-04  4:43 UTC (permalink / raw)
  To: Paul Mackerras, LinuxPPC-dev, Linux Kernel ML
  Cc: Johannes Berg, Frederik Himpe, linux-usb-devel

The commit fa13a5a1f25f671d084d8884be96fc48d9b68275, unconditionally calls
update_process_tick() in system context.  In the deterministic accounting case
this is the correct thing to do.  However, in the non-deterministic accounting
case we need to not do this, and results in the time accounted as
hardware irq time being artificially elevated.

Also this patch collapses 2 consecutive '#ifdef CONFIG_VIRT_CPU_ACCOUNTING'
checks in time.h into for neatness.

Signed-off-by: Tony Breeds <tony@bakeyournoodle.com>
---
The problem was seen and reported by Johannes Berg  and Frederik Himpe.
Paul, I think this is good for 2.6.24.


 arch/powerpc/kernel/process.c |    2 +-
 include/asm-powerpc/time.h    |   12 ++++--------
 2 files changed, 5 insertions(+), 9 deletions(-)

diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index 41e13f4..b9d8837 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -350,7 +350,7 @@ struct task_struct *__switch_to(struct task_struct *prev,
 	local_irq_save(flags);
 
 	account_system_vtime(current);
-	account_process_tick(current, 0);
+	account_process_vtime(current);
 	calculate_steal_time();
 
 	last = _switch(old_thread, new_thread);
diff --git a/include/asm-powerpc/time.h b/include/asm-powerpc/time.h
index 780f826..8a2c8db 100644
--- a/include/asm-powerpc/time.h
+++ b/include/asm-powerpc/time.h
@@ -237,18 +237,14 @@ struct cpu_usage {
 
 DECLARE_PER_CPU(struct cpu_usage, cpu_usage_array);
 
-#ifdef CONFIG_VIRT_CPU_ACCOUNTING
-extern void account_process_vtime(struct task_struct *tsk);
-#else
-#define account_process_vtime(tsk)		do { } while (0)
-#endif
-
 #if defined(CONFIG_VIRT_CPU_ACCOUNTING)
 extern void calculate_steal_time(void);
 extern void snapshot_timebases(void);
+#define account_process_vtime(tsk)	account_process_tick(current, 0);
 #else
-#define calculate_steal_time()			do { } while (0)
-#define snapshot_timebases()			do { } while (0)
+#define calculate_steal_time()		do { } while (0)
+#define snapshot_timebases()		do { } while (0)
+#define account_process_vtime(tsk)	do { } while (0)
 #endif
 
 extern void secondary_cpu_time_init(void);
-- 
1.5.3.6


Yours Tony

  linux.conf.au        http://linux.conf.au/ || http://lca2008.linux.org.au/
  Jan 28 - Feb 02 2008 The Australian Linux Technical Conference!

^ permalink raw reply related

* Re: [PATCH 1/5] PowerPC 74xx: Katana Qp device tree
From: Mark A. Greer @ 2007-12-04  5:30 UTC (permalink / raw)
  To: Mark A. Greer, Andrei Dolnikov, linuxppc-dev
In-Reply-To: <20071204025032.GH32577@localhost.localdomain>

On Tue, Dec 04, 2007 at 01:50:32PM +1100, David Gibson wrote:
> On Mon, Dec 03, 2007 at 07:10:26PM -0700, Mark A. Greer wrote:
> > On Mon, Dec 03, 2007 at 12:50:18PM +1100, David Gibson wrote:
> > > On Thu, Nov 29, 2007 at 06:28:36PM +0300, Andrei Dolnikov wrote:

> > > > +			eth0 {
> > > > +				device_type = "network";
> > > > +				compatible = "marvell,mv64x60-eth";
> > > > +				block-index = <0>;
> > > 
> > > This block-index thing is crap.  If you really need to subindex nodes
> > > like this, use "reg", with an appropriate #address-cells in the
> > > parent, then the nodes will also get sensible unit addresses.
> > 
> > So how would that work for the "PHY Address Register 0x2000", say,
> > where bits 0-4 set the device addr for PHY 0; bits 5-9 set the device
> > addr for PHY 1; bts 10-14 set the devce addr for PHY 2?
> 
> So use 'reg' to do the indexing.  As long as you have no 'ranges'
> property in the parent 'ethernet' node, which you don't, you can use
> 'reg' as a private index.  That's basically what non-translatable reg
> values are about.
> 
> Incidentally you should probably call the subnodes "ethernet@0"
> etc. and the parent one "multiethernet" or something.  It's the
> subnodes that represent an individual ethernet interface, so they
> should take the "ethernet" name and not the parent, by generic names
> conventions.

Okay, thanks for the advice.  I'll fix the prpmc2800 dts file.
Presumably Andrei will fix his.

> [snip]
> > > > +		sdma@4000 {
> > > > +			compatible = "marvell,mv64x60-sdma";
> > > > +			reg = <4000 c18>;
> > > > +			virtual-reg = <f8104000>;
> > > 
> > > Why does this node have virtual-reg?
> > 
> > "virtual-reg" is a special property that doesn't get translated thru
> > the parent mappings.  It should never be used in the kernel.  Its
> > purpose is to give code in the bootwrapper the exact address that it
> > should use to access a register or block of registers or ...
> > Its needed here because the MPSC (serial) driver uses the SDMA unit
> > to perform console I/O in the bootwrapper (e.g., cmdline editing, printf's).
> > 
> > Yes, this needs to be documented.
> 
> Ok.  "it's used for serial in the bootwrapper" would have sufficed - I
> questioned it because it wasn't obvious that this was needed to use
> the mpsc.

Sorry  :)

> > 
> > > > +		mpsc@8000 {
> > > > +			device_type = "serial";
> > > > +			compatible = "marvell,mpsc";
> > > > +			reg = <8000 38>;
> > > > +			virtual-reg = <f8108000>;
> > > > +			sdma = <&/mv64x60/sdma@4000>;
> > > > +			brg = <&/mv64x60/brg@b200>;
> > > > +			cunit = <&/mv64x60/cunit@f200>;
> > > > +			mpscrouting = <&/mv64x60/mpscrouting@b400>;
> > > > +			mpscintr = <&/mv64x60/mpscintr@b800>;
> > > > +			block-index = <0>;
> > > 
> > > What is this block-index thing about here?  Since the devices are
> > > disambiguated by their register address, why do you need it?
> > 
> > This particular one is needed to access the correct MPSC interrupt reg.
> > Maybe it would be better to make a new property for this but it was only
> > one reg and block-index was already there and basically served that
> > purpose so I used it.  I'd be happy to use an alternative if you have
> > something you think is better.
> 
> No, that's an acceptable use for something like this, except that
> "cell-index" seems to be the name we've standardised on for other
> similar cases.

Yeah, I realize that but block-index was here first!
More seriously, I don't like "cell" because it isn't a cell, its a
block or an instance or an...I dunno but its not a cell IMHO.
Anyway, I'll think about changing it to cell but I already feel
dirty just thinking about it.

Mark

^ permalink raw reply

* RE: [PATCH] [POWERPC] [v2] Improved documentation of device tree'ranges'.
From: Stephen Neuendorffer @ 2007-12-04  5:27 UTC (permalink / raw)
  To: David Gibson; +Cc: linuxppc-dev
In-Reply-To: <20071204024025.GG32577@localhost.localdomain>

[-- Attachment #1: Type: text/plain, Size: 1077 bytes --]


I'm actually a little embarrassed that I didn't pick that nit myself.. :)

-----Original Message-----
From: David Gibson [mailto:david@gibson.dropbear.id.au]
Sent: Mon 12/3/2007 6:40 PM
To: Stephen Neuendorffer
Cc: grant.likely@secretlab.ca; linuxppc-dev@ozlabs.org
Subject: Re: [PATCH] [POWERPC] [v2] Improved documentation of device tree'ranges'.
 

On Mon, Dec 03, 2007 at 05:08:57PM -0800, Stephen Neuendorffer wrote:
> I was misled by the prior language.  I've attempted to clarify how
> 'ranges' are used, in particular, how to get an identity mapping.
> 
> Signed-off-by: Stephen Neuendorffer <stephen.neuendorffer@xilinx.com>

Acked-by: David Gibson <david@gibson.dropbear.id.au>

Thanks for the update.  I particularly dislike the "1:1" terminology,
because in maths-speak *any* ranges translation is 1:1, unless it
includes overlapping ranges.

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



[-- Attachment #2: Type: text/html, Size: 2029 bytes --]

^ permalink raw reply

* Re: [PATCH 1/5] PowerPC 74xx: Katana Qp device tree
From: Mark A. Greer @ 2007-12-04  5:34 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linuxppc-dev
In-Reply-To: <1196734483.13230.256.camel@pasglop>

On Tue, Dec 04, 2007 at 01:14:43PM +1100, Benjamin Herrenschmidt wrote:
> 
>  .../... (snip scary bunch of errata)
> 
> > - "FEr PCI-#4" (Detailed by Application Note AN-84):
> > 
> > [This isn't strictly a coherency issue but having coherency enabled
> > exacerbates the problem.]  Basically, the bridge can let the cpu read
> > a pci device's registers before all of the data the PCI devices has
> > written has actually made it to memory.  This and the fact that the
> > device's write data may be stuck in the PCI Slave Write Buffer
> > (which isn't checked for coherency), the cpu can get stale data.
> > 
> > There are no plans to fix that erratum.
> 
> So if I understand correctly, there's no plan to fix a major PCI spec
> violation which prevent any kind of reliable implementation whatsoever ?

That's just for that particular part (e.g., 64360).  Newer parts like
the 64460 have it fixed.

> > So, the answer depends on what part & what rev of the part you have
> > (e.g., the pegasos doesn't use the MPSC and apparently has the other
> > issues worked around so it can turn on coherency but the prpmc2800
> > doesn't so it needs coherency off).
> > 
> > BTW, I haven't forgotten the inherent bug you described when coherency
> > is off (/me too lazy to find link to the email) but AFAIK I've never run
> > into it.  However, if I turn on coherency and stress the PCI bus, it
> > hangs (I can't even look at memory thru a bdi).
> 
> Well, as it is today, the "classic" MMU code cannot deal with !coherent.
> The entire linear mapping is always mapped cacheable with BATs, so stuff
> may be brought into the cache at any time, potentially polluting DMA
> data.
>
> Dealing with that would be hard. It might be possible by using G on the
> entire linear mapping like we do on 4xx (yuck), and/or by not using
> D-BATs (the kernel will blow up in various areas without I-BATs).

Hrm, I didn't realize it was in such bad shape.  I'll have to take a
closer look.

Mark

^ permalink raw reply

* [RFC 0/7] powerpc: Rework pm_power_off & machine_[restart|power_off|halt]
From: Mark A. Greer @ 2007-12-04  5:37 UTC (permalink / raw)
  To: linuxppc-dev

We seem to have the pm_power_off hook wrong in arch/powerpc.  From the
other arches and from how its used by the rest of the kernel (e.g., ipmi),
it should point to the lowest-level power off function not to
machine_power_off().  Actually, machine_power_off() should call pm_power_off
since AFAICT it is the one and only interface used by the rest of the kernel
to power a machine off (with one exception which I believe to be a bug and
have a patch in this series to fix).

While looking at this, I found several bits of code that needed minor
rework and/or cleaning up.  These bits include: refactoring some
common code used by the machine_xxx routines, having machine_power_off
call ppc_md.halt if there ppc_md.power_off is NULL or returns, having
machine_halt call ppc_md.power_off it ppc_md.halt is NULL or returns,
and removing some useless xxx_halt and xxx_power_off routines in
platform code.

With the new usage of pm_power_off, the ppc_md.power_off hook is
no longer needed and pm_power_off will be assigned in the platform
probe routine.

The end result of all of these patches should make the check of
pm_power_off being NULL in kernel/sys.c:sys_reboot useful for powerpc,
eliminate the need for platform halt routines to call the power off routine
(and vice versa), allow things like IPMI to take over pm_power_off
when they need to, and make the power off/halt related code a bit cleaner
& consistent.

I still have to make sure I didn't miss anything and I haven't compiled
all the files I've touched so these aren't submission candidates yet.

I'd appreciate any feedback.

Thanks,

Mark

^ permalink raw reply

* Re: [PATCH] Add IPIC MSI interrupt support
From: Michael Ellerman @ 2007-12-04  5:38 UTC (permalink / raw)
  To: Li Li; +Cc: linuxppc-dev, Gala Kumar, Li Tony
In-Reply-To: <1196672870.14353.21.camel@Guyver>

[-- Attachment #1: Type: text/plain, Size: 3172 bytes --]

On Mon, 2007-12-03 at 17:07 +0800, Li Li wrote:
> Hi Michael,
> 
> I emulate mpic to write this IPIC MSI routines. :)
> 
> 
> > > diff --git a/arch/powerpc/platforms/83xx/mpc837x_mds.c b/arch/powerpc/platforms/83xx/mpc837x_mds.c
> > > index 6048f1b..dbea34b 100644
> > > --- a/arch/powerpc/platforms/83xx/mpc837x_mds.c
> > > +++ b/arch/powerpc/platforms/83xx/mpc837x_mds.c

> > > +
> > > +#define	ipic_msi_irq_to_hw(virq)	((unsigned int)irq_map[virq].hwirq)
> > 
> > What's wrong with virq_to_hw() ?
> > 
> 
> viqr_to_hw is not __inline__.

Hmm, ok. The three places you use it you also take a spin lock, so I'm
not sure the function call's really going to kill you performance wise.

> > > +
> > > +static void ipic_msi_compose_msg(struct ipic_msi *msi, int hwirq,
> > > +						struct msi_msg *msg)
> > > +{
> > > +	unsigned int srs;
> > > +	unsigned int ibs;
> > > +
> > > +	srs = hwirq / msi->int_per_msir;
> > > +	ibs = hwirq - srs * msi->int_per_msir;
> > > +
> > > +	msg->address_lo = msi->msi_addr_lo;
> > > +	msg->address_hi = msi->msi_addr_hi;
> > > +	msg->data = (srs << 5) | (ibs & 0x1F);
> > > +
> > > +	pr_debug("%s: allocated srs: %d, ibs: %d\n",
> > > +		__FUNCTION__, srs, ibs);
> > > +
> > > +}
> > > +
> > > +static int ipic_msi_setup_irqs(struct pci_dev *pdev, int nvec, int type)
> > > +{
> > > +	struct ipic_msi *msi = ipic_msi;
> > > +	irq_hw_number_t hwirq;
> > > +	unsigned int virq;
> > > +	struct msi_desc *entry;
> > > +	struct msi_msg msg;
> > > +
> > > +	list_for_each_entry(entry, &pdev->msi_list, list) {
> > > +		hwirq = ipic_msi_alloc_hwirqs(msi, 1);
> > > +		if (hwirq < 0) {
> > > +			pr_debug("%s: fail allocating msi interrupt\n",
> > > +					__FUNCTION__);
> > > +			return hwirq;
> > > +		}
> > > +
> > > +		/* This hwirq belongs to the irq_host other than irq_host of IPIC
> > > + 		 * So, it is independent to hwirq of IPIC */
> > > +		virq = irq_create_mapping(msi->irqhost, hwirq);
> > > +		if (virq == NO_IRQ) {
> > > +			pr_debug("%s: fail mapping hwirq 0x%lx\n",
> > > +					__FUNCTION__, hwirq);
> > > +			ipic_msi_free_hwirqs(msi, hwirq, 1);
> > > +			return -ENOSPC;
> > > +		}
> > > +		set_irq_msi(virq, entry);
> > > +		ipic_msi_compose_msg(msi, hwirq, &msg);
> > > +		write_msi_msg(virq, &msg);
> > > +
> > > +		hwirq++;
> > 
> >                   ^^^^ this looks like my bug
> 
> I have a question here. Do we support more MSI interrupts on ONE pci
> device?

I'm not sure what you mean? For MSI there is only one MSI per device,
but this code is used also for MSI-X which supports > 1 MSI per device.

Either way we shouldn't be incrementing hwirq by hand, it's reassigned
at the top of the loop. I think that's left over from old code that
allocated nvec hwirqs in a block and then created virq mappings for each
one, whereas the new code allocates each hwirq separately.

cheers

-- 
Michael Ellerman
OzLabs, IBM Australia Development Lab

wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)

We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox