LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: dtc: data.c doesn't need to include dtc-parser.tab.h
From: Jon Loeliger @ 2007-10-22 16:39 UTC (permalink / raw)
  To: David Gibson; +Cc: linuxppc-dev
In-Reply-To: <20071022040923.GA21138@localhost.localdomain>

So, like, the other day David Gibson mumbled:
> Presumably we used this #include once, but it's certainly not
> necessary now.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>

Applied.

jdl

^ permalink raw reply

* Re: [PATCH 1/4] DTC: Reformat grammar rules to not mix language syntax and yacc syntax.
From: Jon Loeliger @ 2007-10-22 16:41 UTC (permalink / raw)
  To: David Gibson; +Cc: linuxppc-dev
In-Reply-To: <20071020071224.GD26642@localhost.localdomain>

So, like, the other day David Gibson mumbled:
> On Fri, Oct 19, 2007 at 12:42:32PM -0500, Jon Loeliger wrote:
> > 
> > Use consistent indenting on all rule actions.
> > 
> > Signed-off-by: Jon Loeliger <jdl@freescale.com>
> 
> Meh, I kind of did it that way to keep the simple rules less bulky,
> but I don't really care much either way.
> 
> Acked-by: David Gibson <david@gibson.dropbear.id.au>

Cool.  Thanks.  Applied.

jdl

^ permalink raw reply

* request_irq fails to get interrupt 12
From: Alan Bennett @ 2007-10-22 16:47 UTC (permalink / raw)
  To: linuxppc-dev

Freescale experts.  Why on earth can't I request the IRQ for Timer1?

static int __init
dvr_ph_init(void)
{
	u32	rv;
	int k;
	//rv = driver_register(&dvr_ph_driver);
	for (k=0;k<64;k++) {
		rv = request_irq(k,dvrph_isr , 0, "dvr_ph", NULL);
		if (rv!=-38) printkplus("request_irq for %d returns %d", k,rv);
	}
	return rv;
}

Results in:
dvr_ph_init    145 - enter the routine
dvr_ph_init    155 -
dvr_ph_init    161 - request_irq for 16 returns -16  (vector 16 = TMCNT)
dvr_ph_init    161 - request_irq for 32 returns 0  (vector 32 = FCC1)
dvr_ph_init    161 - request_irq for 33 returns -16  (vector 33 = FCC2)
dvr_ph_init    161 - request_irq for 40 returns 0  (vector 40 = SCC1)
dvr_ph_init    161 - request_irq for 43 returns 0  (vector 43 = SCC4)

^ permalink raw reply

* Re: [PATCH 3/4] DTC: Appease the printf() format $Gods with a correct type.
From: Jon Loeliger @ 2007-10-22 16:50 UTC (permalink / raw)
  To: David Gibson; +Cc: linuxppc-dev
In-Reply-To: <20071020090633.GH26642@localhost.localdomain>

So, like, the other day David Gibson mumbled:
> On Sat, Oct 20, 2007 at 10:51:29AM +0200, Andreas Schwab wrote:
> > David Gibson <david@gibson.dropbear.id.au> writes:
> > 
> > > What compiler/platform is this?  I can't off the top of my head think
> > > of one where size_t shouldn't be promoted to int automatically.
> > 
> > Only types narrower than int are subject to integer promotion.
> 
> Duh, yes.  Sorry.

So, Hmm.  You want the patch as I wrote it applied?
Or do you want to use %z in the format instead?  It does
seem like there is precedent to do that (in the kernel):

    % grep -r '%z' arch/
    arch/alpha/boot/tools/mkbb.c:   fprintf(stderr, "expected %zd, got %d\n", sizeof(bootblock), nread);
    arch/alpha/boot/tools/mkbb.c:   fprintf(stderr, "expected %zd, got %d\n", sizeof(bootblock), nread);
    arch/m68k/ifpsp060/src/itest.S: mulu.l          (0x00.w,%a3,%za4.l*8),%d2:%d3
    arch/m68k/ifpsp060/src/itest.S: mulu.l          (-0x10.w,%za3,%a4.l*1),%d2:%d3
    arch/m68k/ifpsp060/src/itest.S: mulu.l          (ea_77_mem+0x00.w,%pc,%za4.l*8),%d2:%d3
    ...
    arch/m68k/ifpsp060/src/itest.S: mulu.l          ([EASTORE.l,%zpc,%zd4.l*1]),%d2:%d3
    arch/m68k/ifpsp060/src/itest.S: mulu.l          ([EASTORE.l,%pc],%zd4.l*8,0x20.l),%d2:%d3
    arch/m68k/ifpsp060/src/itest.S: mulu.l          ([EASTORE.l,%zpc],%d4.l*8),%d2:%d3
    arch/um/drivers/cow_user.c:             /* Below, %zd is for a size_t value */
    arch/um/drivers/cow_user.c:                        "limited to %zd characters\n", backing_file,
    arch/x86/kernel/pci-nommu_64.c:                     "nommu_%s: overflow %Lx+%zu of device mask %Lx\n",

Opinions?

Thanks,
jdl

^ permalink raw reply

* Re: [PATCH 3/4] DTC: Appease the printf() format $Gods with a correct type.
From: Josh Boyer @ 2007-10-22 16:55 UTC (permalink / raw)
  To: Jon Loeliger; +Cc: linuxppc-dev, David Gibson
In-Reply-To: <E1Ik0UO-000389-AN@jdl.com>

On Mon, 22 Oct 2007 11:50:52 -0500
Jon Loeliger <jdl@jdl.com> wrote:

> So, like, the other day David Gibson mumbled:
> > On Sat, Oct 20, 2007 at 10:51:29AM +0200, Andreas Schwab wrote:
> > > David Gibson <david@gibson.dropbear.id.au> writes:
> > > 
> > > > What compiler/platform is this?  I can't off the top of my head think
> > > > of one where size_t shouldn't be promoted to int automatically.
> > > 
> > > Only types narrower than int are subject to integer promotion.
> > 
> > Duh, yes.  Sorry.
> 
> So, Hmm.  You want the patch as I wrote it applied?
> Or do you want to use %z in the format instead?  It does
> seem like there is precedent to do that (in the kernel):
> 
>     % grep -r '%z' arch/
>     arch/alpha/boot/tools/mkbb.c:   fprintf(stderr, "expected %zd, got %d\n", sizeof(bootblock), nread);
>     arch/alpha/boot/tools/mkbb.c:   fprintf(stderr, "expected %zd, got %d\n", sizeof(bootblock), nread);
>     arch/m68k/ifpsp060/src/itest.S: mulu.l          (0x00.w,%a3,%za4.l*8),%d2:%d3
>     arch/m68k/ifpsp060/src/itest.S: mulu.l          (-0x10.w,%za3,%a4.l*1),%d2:%d3
>     arch/m68k/ifpsp060/src/itest.S: mulu.l          (ea_77_mem+0x00.w,%pc,%za4.l*8),%d2:%d3
>     ...
>     arch/m68k/ifpsp060/src/itest.S: mulu.l          ([EASTORE.l,%zpc,%zd4.l*1]),%d2:%d3
>     arch/m68k/ifpsp060/src/itest.S: mulu.l          ([EASTORE.l,%pc],%zd4.l*8,0x20.l),%d2:%d3
>     arch/m68k/ifpsp060/src/itest.S: mulu.l          ([EASTORE.l,%zpc],%d4.l*8),%d2:%d3
>     arch/um/drivers/cow_user.c:             /* Below, %zd is for a size_t value */
>     arch/um/drivers/cow_user.c:                        "limited to %zd characters\n", backing_file,
>     arch/x86/kernel/pci-nommu_64.c:                     "nommu_%s: overflow %Lx+%zu of device mask %Lx\n",
> 
> Opinions?

%z is fairly common now-a-days.  The kernel janitors tend to LART
people for not using it rather quickly.

josh

^ permalink raw reply

* Re: request_irq fails to get interrupt 12
From: S. Fricke @ 2007-10-22 17:21 UTC (permalink / raw)
  To: linuxppc-dev
In-Reply-To: <bfa0697f0710220947t657b209ek6f6a011cd1e6e5cf@mail.gmail.com>

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

Hello,

> Freescale experts.  Why on earth can't I request the IRQ for Timer1?

Please consule my question on [1] and the answers.

[1] http://ozlabs.org/pipermail/linuxppc-dev/2007-September/042061.html

bye
Silvio Fricke

-- 
-- S. Fricke ----------------------------- MAILTO:silvio.fricke@gmail.com --
   Diplom-Informatiker (FH)
   Linux-Entwicklung
----------------------------------------------------------------------------


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

^ permalink raw reply

* RE: [microblaze-uclinux] Re: [microblaze-uclinux] RE: [PATCH v3] Device tree bindings for Xilinx devices
From: Stephen Neuendorffer @ 2007-10-22 18:06 UTC (permalink / raw)
  To: microblaze-uclinux
  Cc: Leonid, Wolfgang Reissnegger, Arnd Bergmann, linuxppc-dev
In-Reply-To: <1300.2720-24480-247293449-1192847306@seznam.cz>

> -----Original Message-----
> From: owner-microblaze-uclinux@itee.uq.edu.au=20
> [mailto:owner-microblaze-uclinux@itee.uq.edu.au] On Behalf Of=20
> Michal Simek
> Sent: Friday, October 19, 2007 7:28 PM
> To: microblaze-uclinux@itee.uq.edu.au
> Cc: Stephen Neuendorffer; Grant Likely; Leonid; Arnd=20
> Bergmann; linuxppc-dev@ozlabs.org; Wolfgang Reissnegger
> Subject: [microblaze-uclinux] Re: [microblaze-uclinux] RE:=20
> [PATCH v3] Device tree bindings for Xilinx devices
>=20
> Hi Steve and all,
> >Here's a full .dts generated using an updated version of
> >gen_mhs_devtree.py, following the proposal.
> >It happens to be a microblaze system, but you get the idea.
>=20
> I think that is no good idea generate dts with all information.
> Especially information about PVR - number 2 means - Full PVR=20
> and you can
> obtain information directly from PVR. It is waste of memory space.
> 			xilinx,pvr =3D <2>;

PowerPC does something with the powerpc equivalent of the PVR.
We should just do what they do...
=20
> In my opinion will be better generate only parameters which=20
> you want not all.
> That smells with unusable parameters.

In the long term, this may be true.  In the short term:
1) dtb size is not the key problem
2) making sure that everything works is a key problem.
3) The code that generates the dts should be as simple as possible,
so that we can easily document what it does.

In the long term, I'm all for optimizing the device tree that gets
built,
assuming that it appears to be a problem in real systems.

Steve

^ permalink raw reply

* Re: [PATCH 5/7] pci: Export the pci_restore_msi_state() function
From: Linas Vepstas @ 2007-10-22 18:13 UTC (permalink / raw)
  To: Michael Ellerman
  Cc: netdev, mcarlson, linuxppc-dev list, mchan, linux-pci,
	David Miller
In-Reply-To: <1193017764.10318.17.camel@concordia>

On Mon, Oct 22, 2007 at 11:49:24AM +1000, Michael Ellerman wrote:
> 
> On pseries there's a chance it will work for PCI error recovery, but if
> so it's just lucky that firmware has left everything configured the same
> way. 

? The papr is quite clear that i is up to the OS to restore the msi
state after an eeh error.

> Yes I think so. That way we can properly reconfigure via the firmware
> interface. The other option would be to design some new arch hook to do
> resume, but just doing a disable/enable seems simpler to me.

Err, If you read the code for suspend/resume, it never actually calls
disable/enable (and thus doesn't go to the firmware); it calls 
restore_msi_state() function!

If suspend/resume needs to call firmware to restore the state, then,
at the moment, suspend/resume is broken.  As I mentioned earlier,
I presumed that no powerpc laptops currently use msi-enabled devices,
as otherwise, this would have been flushed out.

--linas

^ permalink raw reply

* Re: Device trees and audio codecs
From: Segher Boessenkool @ 2007-10-22 18:43 UTC (permalink / raw)
  To: Jon Smirl; +Cc: PowerPC dev list
In-Reply-To: <9e4733910710211729k2f2a065cm56670d1b0f90a1af@mail.gmail.com>

It seems to me there are four issues here:

1) How to describe the audio transport controller;
2) How to describe the codecs;
3) How the various parts are connected together in the device tree;
4) How to describe the "layout".

>>> How do we want to be consistent with the Efika which uses an AC97
>>> codec that only connects to i2s?
>>
>> Huh?  AC'97 isn't I2S.  Yeah you probably could hook it up to some I2S
>> device if you do all the interleaving and whatever stuff by hand -- 
>> but
>> then you probably shouldn't call the I2S "host" an I2S anymore, but 
>> name
>> it "ac97" in the device tree, or 
>> "this-or-that-i2s-controller-hooked-up-
>> in-this-particular-crazy-way".  You will want to know which driver to
>> use for the device, and if it's hooked up in "strange and unforeseen"
>> ways you want to know about it.
>
> I meant an ac97 bus. ac97 is conceptually the same as i2s with the
> control signals also routed over it. ac97 and i2s are handled in the
> same PSC on the 5200.

Okay.  On any specific board, there can _only_ be ac97 or _only_ i2s
codecs hooked onto the bus; the device node for the PSC should say
which it is, either by having different "compatible" entries for the
different modes, or by having some device-specific property in there
saying which it is.  Ideally, the name of the node would be "i2s" resp.
"ac97" as well.

This handles 1).

> I have received conflicting opinions as to whether a codec hooked to
> an ac97 bus should get a chip specific codec entry in the device tree.

Every codec gets its own device node, and its "compatible" entry 
describes
it uniquely.  This is a very basic property of device trees.

This handles 2).

> Without the codec specific entry only generic ac97 features can be
> used. The Efika has a STA9766. Looking at the data sheet for the chip
> I see that it implements some proprietary functions in addition to the
> standard ones.
>
> asoc has a generic ac97 driver. Should the ac97 bus be required to
> have a entry for the generic ac97 device? It would make loading the
> driver much easier.

If the kernel driver does _not_ recognise the codec in the device tree,
it could use a "generic" ac97 codec driver, if such a thing indeed 
exists.
If on the other hand it knows about the specific codec, it can use a 
chip-
specific driver.

AC'97 codecs are addressed over the AC'97 bus, so the codecs should be
children of the ac97 bus in the device tree, and have fully qualified
names like "audio@0" or "modem@1" etc.  I2S codecs that are addressed
(== its configuration registers written) over I2C should be children of
their I2C bus in the device tree, with fully qualified names like
"audio@6c"; they should show which I2S bus they sit on, for example by
having an "i2s-bus" property (containing the phandle of the i2s bus) in
the codec node.

This is 3).


Now for 4), the layout thing; you could try to describe the layout in
the device tree, but that would probably fail; there just is too much
variance.  If I understand you correctly, you have a platform-specific
layout driver; now you need to find a nice way to probe this from the
device tree.  Maybe you should just look at the properties in the root
node of the device tree for this.

>> _Please_ don't name busses that are not plain I2S "i2s" in the device
>> tree.  At best this means you'll need a quirk in the kernel code to 
>> deal
>> with this later.
>
> the i2s and ac97 drivers for the mpc5200 already exist. I'm using
> these preexisting drivers.

Sure, but that doesn't mean you cannot fix the way things are probed in
those drivers.  Bugs are there to fix ;-)


Segher

^ permalink raw reply

* Re: [BUG] powerpc does not save msi state [was Re: [PATCH 5/7] pci: Export the pci_restore_msi_state() function
From: Linas Vepstas @ 2007-10-22 19:54 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, mcarlson, linuxppc-dev, mchan, linux-pci
In-Reply-To: <20071019.175308.54212640.davem@davemloft.net>

On Fri, Oct 19, 2007 at 05:53:08PM -0700, David Miller wrote:
> From: linas@austin.ibm.com (Linas Vepstas)
> Date: Fri, 19 Oct 2007 19:46:10 -0500
> 
> > FWIW, it looks like not all that many arches do this; the output
> > for grep -r address_hi * is pretty thin. Then, looking at
> > i386/kernel/io_apic.c as an example, one can see that the 
> > msi state save happens "by accident" if CONFIG_SMP is enabled;
> > and so its surely broekn on uniprocesor machines.
> 
> I don't see this, in all cases write_msi_msg() will transfer
> the given "*msg" to entry->msg by this assignment in
> drivers/pci/msi.c:
> 
> void write_msi_msg(unsigned int irq, struct msi_msg *msg)
> {
>  ...
> 	entry->msg = *msg;
> }
> 
> So as long as write_msi_msg() is invoked, it will be saved
> properly.

As Michael Ellerman points out, the pseries msi setup is done
by firmware, and so this bit never happens. 

As discussed in the other thread, I'll try to set up a patch
for an arch callback for restoring msi state.

-linas

^ permalink raw reply

* Re: request_irq fails to get interrupt 12
From: Alan Bennett @ 2007-10-22 19:55 UTC (permalink / raw)
  To: linuxppc-dev

Ok, so what does it take to expose an interrupt vector on a pq2 PIC??
-Alan

Current:
        /
              localbus{
        ...
 	       fundevice1 {
		       interrupts = <c 8>;
		       interrupt-parent = <&PIC>;
	       };
          ...
               soc@e0000000 {
                      PIC: interrupt-controller@10c00 {
                          #interrupt-cells = <2>;
                          interrupt-controller;
                          reg = <10c00 80>;
                          compatible = "fsl,mpc8248-pic", "fsl,pq2-pic";
                        };
Is the above device tree enough on its own?
Do I have to write some platform code beyond:
  static void __init ep8248_pic_init(void)
  {
	struct device_node *np = of_find_compatible_node(NULL,   NULL, "fsl,pq2-pic");
	if (!np) {
		printk(KERN_ERR "PIC init: can not find cpm-pic node\n");
		return;
	}

	cpm2_pic_init(np);
	of_node_put(np);
}

-Alan

Hello,

> Freescale experts.  Why on earth can't I request the IRQ for Timer1?

Please consule my question on [1] and the answers.

[1] http://ozlabs.org/pipermail/linuxppc-dev/2007-September/042061.html

bye
Silvio Fricke

--
-- S. Fricke ----------------------------- MAILTO:silvio.fricke@gmail.com --
   Diplom-Informatiker (FH)
   Linux-Entwicklung
----------------------------------------------------------------------------

^ permalink raw reply

* Re: [RFC/PATCH 2/2] powerpc: irqtrace support to 64-bit powerpc
From: Benjamin Herrenschmidt @ 2007-10-22 20:14 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linuxppc-dev
In-Reply-To: <1193060107.9793.8.camel@johannes.berg>


On Mon, 2007-10-22 at 15:35 +0200, Johannes Berg wrote:
> On Mon, 2007-10-15 at 17:28 +1000, Benjamin Herrenschmidt wrote:
> > This adds the low level irq tracing hooks to the powerpc architecture
> > needed to enable full lockdep functionality
> > 
> > Some rework from Johannes initial version, removing the asm trampoline that
> > isn't needed (thus improving perfs) and fixing a couple of bugs such as
> > incorrect initial preempt_count on the softirq alternate stack.
> 
> Hmm. It breaks booting on my quad, I finally got around to testing
> without this patch and it turns out that it caused the boot failure I
> reported in another mail. This is with v2.6.23-6623-g55b70a0, a clean
> tree boots fine but with these two patches applied and configured in it
> fails during boot, hanging at some point, apparently when userspace is
> entered.
> 
> On the other hand, it is fine with my original version. I'll have to
> find some time to check the differences hunk by hunk I guess.

Or I can try on my quad :-) I'll have a look. It did boot on some other
machines though. Funny.

Ben.

^ permalink raw reply

* Re: request_irq fails to get interrupt 12
From: Benjamin Herrenschmidt @ 2007-10-22 20:33 UTC (permalink / raw)
  To: Alan Bennett; +Cc: linuxppc-dev
In-Reply-To: <bfa0697f0710221255ubbe8c91pa09640f9b9cd0775@mail.gmail.com>


On Mon, 2007-10-22 at 13:55 -0600, Alan Bennett wrote:
> Ok, so what does it take to expose an interrupt vector on a pq2 PIC??
> -Alan

Also, if it's the default PIC or if you happen to have the PIC struct
irq_host pointer at hand, a quickish way for internal device interrupts
is to directly call irq_create_mapping() though using the device-tree is
nicer.

Ben.

> Current:
>         /
>               localbus{
>         ...
>  	       fundevice1 {
> 		       interrupts = <c 8>;
> 		       interrupt-parent = <&PIC>;
> 	       };
>           ...
>                soc@e0000000 {
>                       PIC: interrupt-controller@10c00 {
>                           #interrupt-cells = <2>;
>                           interrupt-controller;
>                           reg = <10c00 80>;
>                           compatible = "fsl,mpc8248-pic", "fsl,pq2-pic";
>                         };
> Is the above device tree enough on its own?
> Do I have to write some platform code beyond:
>   static void __init ep8248_pic_init(void)
>   {
> 	struct device_node *np = of_find_compatible_node(NULL,   NULL, "fsl,pq2-pic");
> 	if (!np) {
> 		printk(KERN_ERR "PIC init: can not find cpm-pic node\n");
> 		return;
> 	}
> 
> 	cpm2_pic_init(np);
> 	of_node_put(np);
> }
> 
> -Alan
> 
> Hello,
> 
> > Freescale experts.  Why on earth can't I request the IRQ for Timer1?
> 
> Please consule my question on [1] and the answers.
> 
> [1] http://ozlabs.org/pipermail/linuxppc-dev/2007-September/042061.html
> 
> bye
> Silvio Fricke
> 
> --
> -- S. Fricke ----------------------------- MAILTO:silvio.fricke@gmail.com --
>    Diplom-Informatiker (FH)
>    Linux-Entwicklung
> ----------------------------------------------------------------------------
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev

^ permalink raw reply

* [PATCH] DTC:  Remove the need for the GLR Parser.
From: Jon Loeliger @ 2007-10-22 21:13 UTC (permalink / raw)
  To: linuxppc-dev

Previously, there were a few shift/reduce and reduce/reduce
errors in the grammar that were being handled by the not-so-popular
GLR Parser technique.

Flip a right-recursive stack-abusing rule into a left-recursive
stack-friendly rule and clear up three messes in one shot: No more
conflicts, no need for the GLR parser, and friendlier stackness.

Signed-off-by: Jon Loeliger <jdl@freescale.com>
---
 Makefile     |    1 -
 dtc-parser.y |    5 ++---
 2 files changed, 2 insertions(+), 4 deletions(-)

Both "make check" and the "compare all old and new DTB kernel files"
tests are happy with this change.


diff --git a/Makefile b/Makefile
index d7d1af5..84f0efe 100644
--- a/Makefile
+++ b/Makefile
@@ -207,7 +207,6 @@ clean: libfdt_clean tests_clean
 
 %.tab.c %.tab.h %.output: %.y
 	@$(VECHO) BISON $@
-	@$(VECHO) ---- Expect 2 s/r and 2 r/r. ----
 	$(BISON) -d $<
 
 FORCE:
diff --git a/dtc-parser.y b/dtc-parser.y
index 4698793..0d140e5 100644
--- a/dtc-parser.y
+++ b/dtc-parser.y
@@ -18,7 +18,6 @@
  *                                                                   USA
  */
 
-%glr-parser
 %locations
 
 %{
@@ -123,9 +122,9 @@ nodedef:
 	;
 
 proplist:
-	  propdef proplist
+	  proplist propdef
 		{
-			$$ = chain_property($1, $2);
+			$$ = chain_property($2, $1);
 		}
 	| /* empty */
 		{
-- 
1.5.3.1.139.g9346b

^ permalink raw reply related

* Re: [PATCH 5/7] pci: Export the pci_restore_msi_state() function
From: Benjamin Herrenschmidt @ 2007-10-22 21:24 UTC (permalink / raw)
  To: Linas Vepstas
  Cc: netdev, mcarlson, linuxppc-dev list, mchan, linux-pci,
	David Miller
In-Reply-To: <20071022181336.GC4280@austin.ibm.com>


On Mon, 2007-10-22 at 13:13 -0500, Linas Vepstas wrote:
> On Mon, Oct 22, 2007 at 11:49:24AM +1000, Michael Ellerman wrote:
> > 
> > On pseries there's a chance it will work for PCI error recovery, but if
> > so it's just lucky that firmware has left everything configured the same
> > way. 
> 
> ? The papr is quite clear that i is up to the OS to restore the msi
> state after an eeh error.

Via direct config space access or via firmware change-msi calls ?

> > Yes I think so. That way we can properly reconfigure via the firmware
> > interface. The other option would be to design some new arch hook to do
> > resume, but just doing a disable/enable seems simpler to me.
> 
> Err, If you read the code for suspend/resume, it never actually calls
> disable/enable (and thus doesn't go to the firmware); it calls 
> restore_msi_state() function!
> 
> If suspend/resume needs to call firmware to restore the state, then,
> at the moment, suspend/resume is broken.  As I mentioned earlier,
> I presumed that no powerpc laptops currently use msi-enabled devices,
> as otherwise, this would have been flushed out.

I don't know why you keep talking about powerpc laptops here ... 

Ben.

^ permalink raw reply

* [PATCH] DTC: Minor grammar rule shuffle.
From: Jon Loeliger @ 2007-10-22 21:59 UTC (permalink / raw)
  To: linuxppc-dev

I like to see the basis cases established early in
the rule sets, so place  "empty" reduction first.
Purely cosmetic.

Signed-off-by: Jon Loeliger <jdl@freescale.com>
---
 dtc-parser.y |   62 +++++++++++++++++++++++++++++-----------------------------
 1 files changed, 31 insertions(+), 31 deletions(-)

diff --git a/dtc-parser.y b/dtc-parser.y
index 0d140e5..c213526 100644
--- a/dtc-parser.y
+++ b/dtc-parser.y
@@ -86,13 +86,13 @@ sourcefile:
 	;
 
 memreserves:
-	  memreserve memreserves
+	  /* empty */
 		{
-			$$ = chain_reserve_entry($1, $2);
+			$$ = NULL;
 		}
-	| /* empty */
+	| memreserve memreserves
 		{
-			$$ = NULL;
+			$$ = chain_reserve_entry($1, $2);
 		}
 	;
 
@@ -122,13 +122,13 @@ nodedef:
 	;
 
 proplist:
-	  proplist propdef
+	  /* empty */
 		{
-			$$ = chain_property($2, $1);
+			$$ = NULL;
 		}
-	| /* empty */
+	| proplist propdef
 		{
-			$$ = NULL;
+			$$ = chain_property($2, $1);
 		}
 	;
 
@@ -164,7 +164,11 @@ propdata:
 	;
 
 propdataprefix:
-	  propdata ','
+	  /* empty */
+		{
+			$$ = empty_data;
+		}
+	| propdata ','
 		{
 			$$ = $1;
 		}
@@ -172,10 +176,6 @@ propdataprefix:
 		{
 			$$ = data_add_label($1, $2);
 		}
-	| /* empty */
-		{
-			$$ = empty_data;
-		}
 	;
 
 opt_cell_base:
@@ -187,7 +187,11 @@ opt_cell_base:
 	;
 
 celllist:
-	  celllist opt_cell_base DT_CELL
+	  /* empty */
+		{
+			$$ = empty_data;
+		}
+	| celllist opt_cell_base DT_CELL
 		{
 			$$ = data_append_cell($1,
 					      cell_from_string($3, $2));
@@ -200,14 +204,14 @@ celllist:
 		{
 			$$ = data_add_label($1, $2);
 		}
-	| /* empty */
-		{
-			$$ = empty_data;
-		}
 	;
 
 bytestring:
-	  bytestring DT_BYTE
+	  /* empty */
+		{
+			$$ = empty_data;
+		}
+	| bytestring DT_BYTE
 		{
 			$$ = data_append_byte($1, $2);
 		}
@@ -215,20 +219,16 @@ bytestring:
 		{
 			$$ = data_add_label($1, $2);
 		}
-	| /* empty */
-		{
-			$$ = empty_data;
-		}
 	;
 
 subnodes:
-	  subnode subnodes
+	  /* empty */
 		{
-			$$ = chain_node($1, $2);
+			$$ = NULL;
 		}
-	| /* empty */
+	|  subnode subnodes
 		{
-			$$ = NULL;
+			$$ = chain_node($1, $2);
 		}
 	;
 
@@ -251,13 +251,13 @@ nodename:
 	;
 
 label:
-	  DT_LABEL
+	  /* empty */
 		{
-			$$ = $1;
+			$$ = NULL;
 		}
-	| /* empty */
+	| DT_LABEL
 		{
-			$$ = NULL;
+			$$ = $1;
 		}
 	;
 
-- 
1.5.3.1.139.g9346b

^ permalink raw reply related

* [PATCH] DTC: Remove an unneeded %token definition.
From: Jon Loeliger @ 2007-10-22 22:00 UTC (permalink / raw)
  To: linuxppc-dev

Signed-off-by: Jon Loeliger <jdl@freescale.com>
---
 dtc-parser.y |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/dtc-parser.y b/dtc-parser.y
index c213526..61ed250 100644
--- a/dtc-parser.y
+++ b/dtc-parser.y
@@ -55,7 +55,6 @@ extern struct boot_info *the_boot_info;
 %token <str> DT_CELL
 %token <byte> DT_BYTE
 %token <data> DT_STRING
-%token <str> DT_UNIT
 %token <str> DT_LABEL
 %token <str> DT_REF
 
-- 
1.5.3.1.139.g9346b

^ permalink raw reply related

* [PATCH] DTC: Rewrite the propdata and propdataprefix rules.
From: Jon Loeliger @ 2007-10-22 22:01 UTC (permalink / raw)
  To: linuxppc-dev


After staring at these two rules for a bit, and not quite
groking them, I rewrote them into something I think is a
bit clearer.  With any luck, it's the same language still.

Signed-off-by: Jon Loeliger <jdl@freescale.com>
---

You be the judge too!

 dtc-parser.y |   31 ++++++++++++++-----------------
 1 files changed, 14 insertions(+), 17 deletions(-)

diff --git a/dtc-parser.y b/dtc-parser.y
index 61ed250..f0b0178 100644
--- a/dtc-parser.y
+++ b/dtc-parser.y
@@ -59,7 +59,6 @@ extern struct boot_info *the_boot_info;
 %token <str> DT_REF
 
 %type <data> propdata
-%type <data> propdataprefix
 %type <re> memreserve
 %type <re> memreserves
 %type <cbase> opt_cell_base
@@ -143,37 +142,35 @@ propdef:
 	;
 
 propdata:
-	  propdataprefix DT_STRING
+	  DT_STRING
 		{
-			$$ = data_merge($1, $2);
+			$$ = data_merge(empty_data, $1);
 		}
-	| propdataprefix '<' celllist '>'
+	| '<' celllist '>'
 		{
-			$$ = data_merge(data_append_align($1,
-							  sizeof(cell_t)), $3);
+			$$ = data_merge(data_append_align(empty_data,
+							  sizeof(cell_t)), $2);
 		}
-	| propdataprefix '[' bytestring ']'
+	| '[' bytestring ']'
 		{
-			$$ = data_merge($1, $3);
+			$$ = data_merge(empty_data, $2);
 		}
 	| propdata DT_LABEL
 		{
 			$$ = data_add_label($1, $2);
 		}
-	;
-
-propdataprefix:
-	  /* empty */
+	| propdata ',' DT_STRING
 		{
-			$$ = empty_data;
+			$$ = data_merge($1, $3);
 		}
-	| propdata ','
+	| propdata ',' '<' celllist '>'
 		{
-			$$ = $1;
+			$$ = data_merge(data_append_align($1,
+							  sizeof(cell_t)), $4);
 		}
-	| propdataprefix DT_LABEL
+	| propdata ',' '[' bytestring ']'
 		{
-			$$ = data_add_label($1, $2);
+			$$ = data_merge($1, $4);
 		}
 	;
 
-- 
1.5.3.1.139.g9346b

^ permalink raw reply related

* Re: [PATCH] DTC: Rewrite the propdata and propdataprefix rules.
From: Jon Loeliger @ 2007-10-22 22:37 UTC (permalink / raw)
  To: linuxppc-dev
In-Reply-To: <E1Ik5LQ-0000Ef-UF@jdl.com>

So, like, the other day Jon Loeliger mumbled:
> 
> After staring at these two rules for a bit, and not quite
> groking them, I rewrote them into something I think is a
> bit clearer.  With any luck, it's the same language still.
> 
> Signed-off-by: Jon Loeliger <jdl@freescale.com>
> ---
> 
> You be the judge too!

Yeah, so, I botched this one.  It's NOT the same
language, so don't apply it or think about it at all...

jdl

^ permalink raw reply

* [PATCH] Bugfix to commit 4f9a58d75bfe82ab2b8ba5b8506dfb190a267834
From: Grant Likely @ 2007-10-22 22:38 UTC (permalink / raw)
  To: Olaf Hering, linux-kernel, Linus Torvalds, Paul Mackerras,
	linuxppc-dev

From: Grant Likely <grant.likely@secretlab.ca>

Commit 4f9a58d75bfe82ab2b8ba5b8506dfb190a267834 changes the size of
AT_VECTOR_SIZE from hard coded '44' to a calculation based on the value
of AT_VECTOR_SIZE_ARCH and AT_VECTOR_SIZE_BASE.  The change works for
arch/powerpc, but it breaks arch/ppc because the needed
AT_VECTOR_SIZE_ARCH is not present in include/asm-ppc/system.h and a
default value of 0 is used instead.  This results in AT_VECTOR_SIZE
being too small and it causes a kernel crash on loading init.

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

I think this bug fix needs to go in ASAP.  I cannot boot my Virtex ppc405
platform without it.

Olaf, do I have the correct solution here?

Thanks,
g.

 include/asm-ppc/system.h |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/include/asm-ppc/system.h b/include/asm-ppc/system.h
index cc45780..51df94c 100644
--- a/include/asm-ppc/system.h
+++ b/include/asm-ppc/system.h
@@ -33,6 +33,7 @@
 
 #define set_mb(var, value)	do { var = value; mb(); } while (0)
 
+#define AT_VECTOR_SIZE_ARCH 6 /* entries in ARCH_DLINFO */
 #ifdef CONFIG_SMP
 #define smp_mb()	mb()
 #define smp_rmb()	rmb()

^ permalink raw reply related

* Please pull powerpc.git master branch
From: Paul Mackerras @ 2007-10-23  0:08 UTC (permalink / raw)
  To: torvalds; +Cc: linuxppc-dev

Linus,

Please do

git pull \
git://git.kernel.org/pub/scm/linux/kernel/git/paulus/powerpc.git master

to get five more commits that fix some bugs for systems using the
Freescale embedded 52xx processors and enable them to restart
properly.

Thanks,
Paul.

 .../powerpc/mpc52xx-device-tree-bindings.txt       |    4 +
 arch/powerpc/boot/dts/lite5200.dts                 |   26 +++----
 arch/powerpc/boot/dts/lite5200b.dts                |   26 +++----
 arch/powerpc/platforms/52xx/lite5200.c             |    4 +
 arch/powerpc/platforms/52xx/mpc52xx_common.c       |   71 +++++++++++++++++++-
 arch/powerpc/sysdev/bestcomm/bestcomm.c            |    9 ++-
 drivers/watchdog/mpc5200_wdt.c                     |    3 +
 include/asm-powerpc/mpc52xx.h                      |    9 +++
 8 files changed, 109 insertions(+), 43 deletions(-)

Grant Likely (1):
      [POWERPC] bestcomm: Restrict bus prefetch bugfix to original mpc5200 silicon.

Marian Balakowicz (4):
      [POWERPC] Add mpc52xx_find_and_map_path(), refactor utility functions
      [POWERPC] Update device tree binding for mpc5200 gpt
      [POWERPC] Add restart support for mpc52xx based platforms
      [POWERPC] Enable restart support for lite5200 board

^ permalink raw reply

* Re: [PATCH 5/7] pci: Export the pci_restore_msi_state() function
From: Linas Vepstas @ 2007-10-23  0:13 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: netdev, mcarlson, linuxppc-dev list, mchan, linux-pci,
	David Miller
In-Reply-To: <1193088267.6745.108.camel@pasglop>

On Tue, Oct 23, 2007 at 07:24:27AM +1000, Benjamin Herrenschmidt wrote:
> 
> On Mon, 2007-10-22 at 13:13 -0500, Linas Vepstas wrote:
> > On Mon, Oct 22, 2007 at 11:49:24AM +1000, Michael Ellerman wrote:
> > > 
> > > On pseries there's a chance it will work for PCI error recovery, but if
> > > so it's just lucky that firmware has left everything configured the same
> > > way. 
> > 
> > ? The papr is quite clear that i is up to the OS to restore the msi
> > state after an eeh error.
> 
> Via direct config space access or via firmware change-msi calls ?

Direct config space access. It says that the OS is supposed to read the
MSI config (after its been set up), save it, and restore it, (via direct
config space writes) if the device is ever reset.

> I don't know why you keep talking about powerpc laptops here ... 

Well, there are Apple laptops, right?  Aren't those the "powermac" 
platform?  Now, I don't know if they support MSI, but if they do,
I get the impression that they might not restore msi state correctly,
after being put into hardware suspend.  But perhaps I'm mistaken;
I was simply grepping for various msi-related functions in various
arch subdirectories, comparing x86 to other arches, and noticed 
that code that would restore msi state seems to be missing for
most arches and most powerpc platforms.

--linas

^ permalink raw reply

* Re: [BUG] powerpc does not save msi state [was Re: [PATCH 5/7] pci: Export the pci_restore_msi_state() function
From: David Miller @ 2007-10-23  0:23 UTC (permalink / raw)
  To: linas; +Cc: netdev, mcarlson, linuxppc-dev, mchan, linux-pci
In-Reply-To: <20071022195451.GE4280@austin.ibm.com>

From: linas@austin.ibm.com (Linas Vepstas)
Date: Mon, 22 Oct 2007 14:54:52 -0500

> As discussed in the other thread, I'll try to set up a patch
> for an arch callback for restoring msi state.

Thank you.

^ permalink raw reply

* Re: [PATCH 5/7] pci: Export the pci_restore_msi_state() function
From: Benjamin Herrenschmidt @ 2007-10-23  0:29 UTC (permalink / raw)
  To: Linas Vepstas
  Cc: netdev, mcarlson, linuxppc-dev list, mchan, linux-pci,
	David Miller
In-Reply-To: <20071023001307.GF4280@austin.ibm.com>


> > I don't know why you keep talking about powerpc laptops here ... 
> 
> Well, there are Apple laptops, right?  Aren't those the "powermac" 
> platform?  Now, I don't know if they support MSI, but if they do,
> I get the impression that they might not restore msi state correctly,
> after being put into hardware suspend.  But perhaps I'm mistaken;
> I was simply grepping for various msi-related functions in various
> arch subdirectories, comparing x86 to other arches, and noticed 
> that code that would restore msi state seems to be missing for
> most arches and most powerpc platforms.

Ah ok, i see. Well, platforms that use write_msi_msg() shouldn't need
anything special right ? So only pSeries is an issue here....

PowerBooks don't indeed have MSI support, though G5's do and some people
have been toying around with suspend/resume on them (hibernation only at
that stage) but it doesn't matter at this stage. We are specifically
talking about pSeries which is the "special" case here.

Ben.

^ permalink raw reply

* Re: [BUG] powerpc does not save msi state [was Re: [PATCH 5/7] pci: Export the pci_restore_msi_state() function
From: Benjamin Herrenschmidt @ 2007-10-23  0:32 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, mcarlson, linuxppc-dev, mchan, linux-pci
In-Reply-To: <20071022.172332.112621497.davem@davemloft.net>


On Mon, 2007-10-22 at 17:23 -0700, David Miller wrote:
> From: linas@austin.ibm.com (Linas Vepstas)
> Date: Mon, 22 Oct 2007 14:54:52 -0500
> 
> > As discussed in the other thread, I'll try to set up a patch
> > for an arch callback for restoring msi state.

>From what it looks like at this stage, pSeries might need to
differenciate restoring MSI state after a device reset (PCI error
recovery) from restoring MSI state after suspend/resume (if we ever
implement that one).

The former apparently require manual saving & restoring of the config
space bits. (Linas, do you have a pointer to the bit of PAPR spec that
specifies that we need to save & restore the MSI message ourselves ?)

For the later (suspend/resume), that will definitely not work, or at
least, will not be enough, especially with something like suspend to
disk, where we'll need to have the firmware reconfigure the MSIs for us
(to make sure, among others, that the interrupt controllers are properly
configured for MSIs etc...).

Ben.

^ 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