[PATCH] * mpc8313erdb.dts: Fixed eTSEC interrupt assignment.

[PATCH] * mpc8313erdb.dts: Fixed eTSEC interrupt assignment.

[Roland Lezuo]

The following patch is needed to correctly assign the IRQs for the gianfar driver on the MPC8313ERDB-revc boards. ERR and TX are swapped as well as the interrupt lines for the two devices.

Signed-off-by: Roland Lezuo <roland.lezuo@chello.at>

---
 arch/powerpc/boot/dts/mpc8313erdb.dts |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/boot/dts/mpc8313erdb.dts b/arch/powerpc/boot/dts/mpc8313erdb.dts
index 761faa7..907a445 100644
--- a/arch/powerpc/boot/dts/mpc8313erdb.dts
+++ b/arch/powerpc/boot/dts/mpc8313erdb.dts
@@ -188,7 +188,7 @@
 			compatible = "gianfar";
 			reg = <0x24000 0x1000>;
 			local-mac-address = [ 00 00 00 00 00 00 ];
-			interrupts = <37 0x8 36 0x8 35 0x8>;
+			interrupts = <32 0x8 33 0x8 34 0x8>;
 			interrupt-parent = <&ipic>;
 			tbi-handle = < &tbi0 >;
 			/* Vitesse 7385 isn't on the MDIO bus */
@@ -223,7 +223,7 @@
 			reg = <0x25000 0x1000>;
 			ranges = <0x0 0x25000 0x1000>;
 			local-mac-address = [ 00 00 00 00 00 00 ];
-			interrupts = <34 0x8 33 0x8 32 0x8>;
+			interrupts = <35 0x8 36 0x8 37 0x8>;
 			interrupt-parent = <&ipic>;
 			tbi-handle = < &tbi1 >;
 			phy-handle = < &phy4 >;
-- 
1.6.0.4

Regards
Roland Lezuo
please CC personally as I'm not subscribed.

Re: [PATCH] * mpc8313erdb.dts: Fixed eTSEC interrupt assignment.

[Scott Wood]

On Fri, Sep 04, 2009 at 12:31:25PM +0200, Roland Lezuo wrote:
> The following patch is needed to correctly assign the IRQs for the
> gianfar driver on the MPC8313ERDB-revc boards. ERR and TX are swapped
> as well as the interrupt lines for the two devices.

And it will incorrectly assign them on older revisions of the chip.

We really should have a u-boot fixup based on SVR.

-Scott

Re: [PATCH] * mpc8313erdb.dts: Fixed eTSEC interrupt assignment.

[Kumar Gala]

On Sep 9, 2009, at 1:22 PM, Scott Wood wrote:

> On Fri, Sep 04, 2009 at 12:31:25PM +0200, Roland Lezuo wrote:
>> The following patch is needed to correctly assign the IRQs for the
>> gianfar driver on the MPC8313ERDB-revc boards. ERR and TX are swapped
>> as well as the interrupt lines for the two devices.
>
> And it will incorrectly assign them on older revisions of the chip.
>
> We really should have a u-boot fixup based on SVR.

I felt like Kim did this.

- k

Re: [PATCH] * mpc8313erdb.dts: Fixed eTSEC interrupt assignment.

[Kim Phillips]

On Wed, 9 Sep 2009 15:28:01 -0500
Kumar Gala <galak@kernel.crashing.org> wrote:

> 
> On Sep 9, 2009, at 1:22 PM, Scott Wood wrote:
> 
> > On Fri, Sep 04, 2009 at 12:31:25PM +0200, Roland Lezuo wrote:
> >> The following patch is needed to correctly assign the IRQs for the
> >> gianfar driver on the MPC8313ERDB-revc boards. ERR and TX are swapped
> >> as well as the interrupt lines for the two devices.
> >
> > And it will incorrectly assign them on older revisions of the chip.
> >
> > We really should have a u-boot fixup based on SVR.

definitely.

> I felt like Kim did this.

I never got my hands on a Rev. C board.  What's the SVR on it?  I think
this is also valid for Rev. B boards, and I don't have one of those
either.

Kim

Re: [PATCH] * mpc8313erdb.dts: Fixed eTSEC interrupt assignment.

[Kim Phillips]

On Wed, 9 Sep 2009 15:28:01 -0500
Kumar Gala <galak@kernel.crashing.org> wrote:

> 
> On Sep 9, 2009, at 1:22 PM, Scott Wood wrote:
> 
> > On Fri, Sep 04, 2009 at 12:31:25PM +0200, Roland Lezuo wrote:
> >> The following patch is needed to correctly assign the IRQs for the
> >> gianfar driver on the MPC8313ERDB-revc boards. ERR and TX are swapped
> >> as well as the interrupt lines for the two devices.
> >
> > And it will incorrectly assign them on older revisions of the chip.
> >
> > We really should have a u-boot fixup based on SVR.

Roland, is it possible for you to test this u-boot patch?:

>From 44c73137acf20626e930e3f4142e60054d8bd46f Mon Sep 17 00:00:00 2001
From: Kim Phillips <kim.phillips@freescale.com>
Date: Mon, 12 Oct 2009 10:51:20 -0500
Subject: [PATCH] mpc83xx: mpc8313 - handle erratum IPIC1 (TSEC IRQ number swappage)

mpc8313e erratum IPIC1 swapped TSEC interrupt ID numbers on rev. 1
h/w (see AN3545).  The base device tree in use has rev. 1 ID numbers,
so if on Rev. 2 (and higher) h/w, we fix them up here.

Signed-off-by: Kim Phillips <kim.phillips@freescale.com>
---
 cpu/mpc83xx/fdt.c |   39 +++++++++++++++++++++++++++++++++++++++
 1 files changed, 39 insertions(+), 0 deletions(-)

diff --git a/cpu/mpc83xx/fdt.c b/cpu/mpc83xx/fdt.c
index 13443cb..daf73a6 100644
--- a/cpu/mpc83xx/fdt.c
+++ b/cpu/mpc83xx/fdt.c
@@ -69,6 +69,45 @@ void ft_cpu_setup(void *blob, bd_t *bd)
     defined(CONFIG_HAS_ETH2) || defined(CONFIG_HAS_ETH3) ||\
     defined(CONFIG_HAS_ETH4) || defined(CONFIG_HAS_ETH5)
 	fdt_fixup_ethernet(blob);
+#ifdef CONFIG_MPC8313
+	/*
+	* mpc8313e erratum IPIC1 swapped TSEC interrupt ID numbers on rev. 1
+	* h/w (see AN3545).  The base device tree in use has rev. 1 ID numbers,
+	* so if on Rev. 2 (and higher) h/w, we fix them up here
+	*/
+	if (REVID_MAJOR(immr->sysconf.spridr) >= 2) {
+		int nodeoffset, path;
+		const char *prop;
+
+		nodeoffset = fdt_path_offset(blob, "/aliases");
+		if (nodeoffset >= 0) {
+#if defined(CONFIG_HAS_ETH0)
+			prop = fdt_getprop(blob, nodeoffset, "ethernet0", NULL);
+			if (prop) {
+				u32 tmp[] = { 32, 0x8, 33, 0x8, 34, 0x8 };
+
+				path = fdt_path_offset(blob, prop);
+				prop = fdt_getprop(blob, path, "interrupts", 0);
+				if (prop)
+					fdt_setprop(blob, path, "interrupts",
+						    &tmp, sizeof(tmp));
+			}
+#endif
+#if defined(CONFIG_HAS_ETH1)
+			prop = fdt_getprop(blob, nodeoffset, "ethernet1", NULL);
+			if (prop) {
+				u32 tmp[] = { 35, 0x8, 36, 0x8, 37, 0x8 };
+
+				path = fdt_path_offset(blob, prop);
+				prop = fdt_getprop(blob, path, "interrupts", 0);
+				if (prop)
+					fdt_setprop(blob, path, "interrupts",
+						    &tmp, sizeof(tmp));
+			}
+#endif
+		}
+	}
+#endif
 #endif
 
 	do_fixup_by_prop_u32(blob, "device_type", "cpu", 4,
-- 
1.6.4.4

Re: [PATCH] * mpc8313erdb.dts: Fixed eTSEC interrupt assignment.

[Kim Phillips]

On Thu, 15 Oct 2009 21:40:43 +0200
Roland Lezuo <roland.lezuo@chello.at> wrote:

> On Mon, 2009-10-12 at 11:06 -0500, Kim Phillips wrote:
> > On Wed, 9 Sep 2009 15:28:01 -0500
> > Kumar Gala <galak@kernel.crashing.org> wrote:
> > > On Sep 9, 2009, at 1:22 PM, Scott Wood wrote:
> > > > We really should have a u-boot fixup based on SVR.
> > 
> > Roland, is it possible for you to test this u-boot patch?:
> 
> No sorry, I have no MPC8313ERDB at hands. But the patch at least looks
> good...

ok, thanks - I went ahead and applied it anyway; it'll be tested by
the next u-boot release.  I've added your reviewed-by.

Kim

Re: [PATCH] * mpc8313erdb.dts: Fixed eTSEC interrupt assignment.

[Mark Bishop]

Quoting Scott Wood <scottwood@freescale.com>:

> On Fri, Sep 04, 2009 at 12:31:25PM +0200, Roland Lezuo wrote:
>> The following patch is needed to correctly assign the IRQs for the
>> gianfar driver on the MPC8313ERDB-revc boards. ERR and TX are swapped
>> as well as the interrupt lines for the two devices.
>
> And it will incorrectly assign them on older revisions of the chip.
>
> We really should have a u-boot fixup based on SVR.
>
> -Scott
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
>
>

I have both versions of the boards and I use two versions of the .dts  
file.  Actually, four, two to support the old style uboot .dts that  
comes with Freescale's BSP and 2 that support the newer uboot.

RE: [PATCH] * mpc8313erdb.dts: Fixed eTSEC interrupt assignment.

[Richard Cochran]

>-----Original Message-----
>From: linuxppc-dev-bounces On Behalf Of Scott Wood
>Sent: Wednesday, September 09, 2009 8:22 PM
>To: Roland Lezuo
>Cc: linuxppc-dev@lists.ozlabs.org
>Subject: Re: [PATCH] * mpc8313erdb.dts: Fixed eTSEC interrupt assignment.
>
>On Fri, Sep 04, 2009 at 12:31:25PM +0200, Roland Lezuo wrote:
>> The following patch is needed to correctly assign the IRQs for the
>> gianfar driver on the MPC8313ERDB-revc boards. ERR and TX are swapped
>> as well as the interrupt lines for the two devices.
>
>And it will incorrectly assign them on older revisions of the chip.
>
>We really should have a u-boot fixup based on SVR.

Why not just offer different dts files, one for each board revision?

Something like:
mpc8313erdb-revA.dts
mpc8313erdb-revB.dts
mpc8313erdb-revC.dts

Richard

Re: [PATCH] * mpc8313erdb.dts: Fixed eTSEC interrupt assignment.

[Scott Wood]

On Wed, Oct 14, 2009 at 09:41:33AM +0200, Richard Cochran wrote:
> >-----Original Message-----
> >From: linuxppc-dev-bounces On Behalf Of Scott Wood
> >Sent: Wednesday, September 09, 2009 8:22 PM
> >To: Roland Lezuo
> >Cc: linuxppc-dev@lists.ozlabs.org
> >Subject: Re: [PATCH] * mpc8313erdb.dts: Fixed eTSEC interrupt assignment.
> >
> >On Fri, Sep 04, 2009 at 12:31:25PM +0200, Roland Lezuo wrote:
> >> The following patch is needed to correctly assign the IRQs for the
> >> gianfar driver on the MPC8313ERDB-revc boards. ERR and TX are swapped
> >> as well as the interrupt lines for the two devices.
> >
> >And it will incorrectly assign them on older revisions of the chip.
> >
> >We really should have a u-boot fixup based on SVR.
> 
> Why not just offer different dts files, one for each board revision?
> 
> Something like:
> mpc8313erdb-revA.dts
> mpc8313erdb-revB.dts
> mpc8313erdb-revC.dts

Because that would be three times the device trees to maintain, and a
source of user confusion.

-Scott

RE: [PATCH] * mpc8313erdb.dts: Fixed eTSEC interrupt assignment.

[Richard Cochran]

>-----Original Message----- From: Scott Wood [mailto:scottwood@freescale.co=
m]
>Because that would be three times the device trees to maintain, and a
>source of user confusion.

I wonder which is more confusing for the user:

1. Choosing one of three dts files.
2. Having only one dts for his board, but Ethernet doesn't work.

Richard

Re: [PATCH] * mpc8313erdb.dts: Fixed eTSEC interrupt assignment.

[Scott Wood]

On Thu, Oct 15, 2009 at 02:19:30PM +0200, Richard Cochran wrote:
> >-----Original Message----- From: Scott Wood [mailto:scottwood@freescale.com]
> >Because that would be three times the device trees to maintain, and a
> >source of user confusion.
> 
> I wonder which is more confusing for the user:
> 
> 1. Choosing one of three dts files.

Possibly incorrectly.

> 2. Having only one dts for his board, but Ethernet doesn't work.

The point is to fix u-boot so that it *does* work with only one dts.  If
people not upgrading u-boot is your concern, we could put the fixup in the
Linux platform code instead.

And feel free to ask through official Freescale support channels why the
U-Boot that shipped on these boards does not have such a fixup (or why they
decided it was better to make late-rev 8313's interrupt assignments match
other 83xx than for all revs of the same part number to have the same
interrupt assignments).

-Scott

RE: [PATCH] * mpc8313erdb.dts: Fixed eTSEC interrupt assignment.

[Richard Cochran]

> -----Original Message-----
> From: Scott Wood [mailto:scottwood@freescale.com]
> Subject: Re: [PATCH] * mpc8313erdb.dts: Fixed eTSEC interrupt assignment.
>
> On Thu, Oct 15, 2009 at 02:19:30PM +0200, Richard Cochran wrote:
> > 2. Having only one dts for his board, but Ethernet doesn't work.
>
> The point is to fix u-boot so that it *does* work with only one dts.  If
> people not upgrading u-boot is your concern, we could put the fixup in th=
e
> Linux platform code instead.

Yes, I was thinking that upgrading u-boot can be problematic for
people who don't have a JTAG flash tool, in case things go wrong. The
whole kernel-uboot-dts triangle is a constant source of confusion, IMHO.

But I thought the whole point of the device tree was, that it is easy
to produce a dts that exactly matches a particular physical board
design. If MPC8313-ERDB REVC has a different IRQ routing, then doesn't
it deserve its own dts? Or do only *some* of the REVC boards have this
routing?

> And feel free to ask through official Freescale support channels why the
> U-Boot that shipped on these boards does not have such a fixup (or why th=
ey
> decided it was better to make late-rev 8313's interrupt assignments match
> other 83xx than for all revs of the same part number to have the same
> interrupt assignments).

It sounds like you are not overly satisfied with Freescale's handling
of their BSPs ;)

To be fair, I am happy that Freescale appears to take Linux support
seriously, but I do think they drop the ball once a BSP is
published. For example, the MPC8313-ERDB ships with kernel 2.6.23
(with the old style dts) and a fair number of non-mainstream
patches. It is not so easy to get a recent kernel booting on that
board.

I have the LITE5200B, MPC8313-ERDB, MPC8572DS, and the P2020DS in
house, and it is really the same, sad story with each of
them. Wouldn't it be grand if the development boards would boot "out
of the box" when compiling the most recent kernel with default config?

Maybe that's only wishful thinking, but I am happy to contribute what I
can...

Richard

Re: [PATCH] * mpc8313erdb.dts: Fixed eTSEC interrupt assignment.

[Scott Wood]

On Fri, Oct 16, 2009 at 08:31:19AM +0200, Richard Cochran wrote:
> > -----Original Message-----
> > From: Scott Wood [mailto:scottwood@freescale.com]
> > Subject: Re: [PATCH] * mpc8313erdb.dts: Fixed eTSEC interrupt assignment.
> >
> > On Thu, Oct 15, 2009 at 02:19:30PM +0200, Richard Cochran wrote:
> > > 2. Having only one dts for his board, but Ethernet doesn't work.
> >
> > The point is to fix u-boot so that it *does* work with only one dts.  If
> > people not upgrading u-boot is your concern, we could put the fixup in the
> > Linux platform code instead.
> 
> Yes, I was thinking that upgrading u-boot can be problematic for
> people who don't have a JTAG flash tool, in case things go wrong.

Right, so we could put a fixup in Linux as well to handle that case (make
sure not to blindly reverse, though, in case u-boot has already fixed it
up).

> But I thought the whole point of the device tree was, that it is easy
> to produce a dts that exactly matches a particular physical board
> design.

It's the dtb that the kernel sees (possibly after platform fixups, though
that's not the ideal case) that is supposed to exactly match the hardware. 
The dts is just one source (albeit the most significant one) of the data
that goes into the final dtb.

There are boards that have several orthogonal configuration options that
can't be probed -- we'd have to provide dozens (or worse) of .dts files in
order to cover all of them without any runtime fixup.  Not to mention things
like memory size and clock frequency...

> If MPC8313-ERDB REVC has a different IRQ routing, then doesn't
> it deserve its own dts? 

I'm inclined to say no, though I wouldn't object too hard in this case if
people really want it.  But we should still put in a u-boot fixup so that if
you have a new u-boot it doesn't matter which dts you use.

> Or do only *some* of the REVC boards have this
> routing?

They should all have it.

> > And feel free to ask through official Freescale support channels why the
> > U-Boot that shipped on these boards does not have such a fixup (or why they
> > decided it was better to make late-rev 8313's interrupt assignments match
> > other 83xx than for all revs of the same part number to have the same
> > interrupt assignments).
> 
> It sounds like you are not overly satisfied with Freescale's handling
> of their BSPs ;)

:-)

Things are slowly getting better, but it'd be good for the BSP teams to hear
from customers (and not just other internal groups) saying they want more
up-to-date software that doesn't diverge as much from mainline.

And getting u-boot right before shipping it on a board is particularly
important.

> To be fair, I am happy that Freescale appears to take Linux support
> seriously, but I do think they drop the ball once a BSP is
> published. For example, the MPC8313-ERDB ships with kernel 2.6.23
> (with the old style dts) and a fair number of non-mainstream
> patches. It is not so easy to get a recent kernel booting on that
> board.

What problems have you been having with upstream kernels on mpc8313erdb,
other than this IRQ issue?  It should work, though the BSP may have extra
features that haven't been pushed upstream.

> I have the LITE5200B, MPC8313-ERDB, MPC8572DS, and the P2020DS in
> house, and it is really the same, sad story with each of
> them. Wouldn't it be grand if the development boards would boot "out
> of the box" when compiling the most recent kernel with default config?

That's what the group I'm in tries to make sure happens (for the
mpc8xxx/qoriq chips) -- but we're outnumbered by the BSP developers, so some
BSP features tend to be missing.  Basic board support should be there,
though.

I just booted 2.6.32-rc4 straight from Linus on an mpc8572ds yesterday. 
I've booted various upstream kernels on mpc8313erdb (typically an older rev,
though).

-Scott

Re: [PATCH] * mpc8313erdb.dts: Fixed eTSEC interrupt assignment.

[Kumar Gala]

On Oct 16, 2009, at 11:01 AM, Scott Wood wrote:

>>
>> I have the LITE5200B, MPC8313-ERDB, MPC8572DS, and the P2020DS in
>> house, and it is really the same, sad story with each of
>> them. Wouldn't it be grand if the development boards would boot "out
>> of the box" when compiling the most recent kernel with default  
>> config?
>
> That's what the group I'm in tries to make sure happens (for the
> mpc8xxx/qoriq chips) -- but we're outnumbered by the BSP developers,  
> so some
> BSP features tend to be missing.  Basic board support should be there,
> though.
>
> I just booted 2.6.32-rc4 straight from Linus on an mpc8572ds  
> yesterday.
> I've booted various upstream kernels on mpc8313erdb (typically an  
> older rev,
> though).

Also, if they don't boot out of the box report an issue on this list.   
That is part of the community testing aspect of open source.

- k

RE: [PATCH] * mpc8313erdb.dts: Fixed eTSEC interrupt assignment.

[Richard Cochran]

> -----Original Message-----
> From: Scott Wood [mailto:scottwood@freescale.com]
>
> What problems have you been having with upstream kernels on mpc8313erdb,
> other than this IRQ issue?  It should work, though the BSP may have extra
> features that haven't been pushed upstream.

I have been working from kernel 2.6.30 (although the very latest
kernel is just the same WRT these problems, AFAICT). I had to patch in
order to sovle the following three problems:

1. The flash layout in the DTS does not match the default partitioning
   from Freescale. In the current dts, the NAND partitioning is wrong,
   and there is no partitioning for the NOR flash given.

2. The eTSEC interrupt issue that started this thread.

3. The PTP IO signals are not configured in a vanilla linux. For this,
   you need parts of a Freescale patch. [1] Their PTP implementation
   gianfar driver is horrible, but still, the IO configuration in
   these two files in the patch is necessary to get any external
   signals from the PTP clock:

   arch/powerpc/platforms/83xx/mpc8313_rdb.c
   arch/powerpc/platforms/83xx/mpc83xx.h

It may be that point 1 is not so important, since the flash layout is
perhaps a very personal thing, but I would still expect the default
dts to match the default partitioning from Freescale.

Point 2 is obviously essential, and if you buy the mpc8313 for its PTP
capabilities, as I did, then point 3 is a show stopper, as well.

> I just booted 2.6.32-rc4 straight from Linus on an mpc8572ds yesterday.
> I've booted various upstream kernels on mpc8313erdb (typically an older r=
ev,
> though).

Okay, you are right, the mpc8572ds does seem to work pretty well.

Perhaps we can fix up the above three issues?

Richard

[1] http://www.bitshrine.org/gpp_info/linux-fsl-2.6.23-MPC8313ERDB-IEEE-158=
8.patch.html

Re: [PATCH] * mpc8313erdb.dts: Fixed eTSEC interrupt assignment.

[Scott Wood]

On Tue, Oct 20, 2009 at 12:01:19PM +0200, Richard Cochran wrote:
> > -----Original Message-----
> > From: Scott Wood [mailto:scottwood@freescale.com]
> >
> > What problems have you been having with upstream kernels on mpc8313erdb,
> > other than this IRQ issue?  It should work, though the BSP may have extra
> > features that haven't been pushed upstream.
> 
> I have been working from kernel 2.6.30 (although the very latest
> kernel is just the same WRT these problems, AFAICT). I had to patch in
> order to sovle the following three problems:
> 
> 1. The flash layout in the DTS does not match the default partitioning
>    from Freescale. In the current dts, the NAND partitioning is wrong,
>    and there is no partitioning for the NOR flash given.

OK, I wasn't aware of that -- that kind of thing is an artifact of the BSP
development process being fairly separate from upstream development.  The
"open source team" works on upstream Linux and upstream U-Boot.  We don't
run the BSP u-boot (since we're developing the newer u-boot), and it's easy
to miss when things like this diverge.

In this case, I'd be surprised if the NAND partitioning that is upstream
came from anywhere but an early BSP's Linux tree.  There's not much we (the
upstream-focused developers) can do if different BSPs have different layouts
(other than not specify a layout at all)...  What is the layout you see in
your BSP?

> 2. The eTSEC interrupt issue that started this thread.

Well, yes. :-)

> 3. The PTP IO signals are not configured in a vanilla linux. For this,
>    you need parts of a Freescale patch. [1] Their PTP implementation
>    gianfar driver is horrible, but still, the IO configuration in
>    these two files in the patch is necessary to get any external
>    signals from the PTP clock:
> 
>    arch/powerpc/platforms/83xx/mpc8313_rdb.c
>    arch/powerpc/platforms/83xx/mpc83xx.h

Yes, this is one of those features that is currently BSP-only (partly due to
it needing work, as you note above).  That's different than the board simply
not being supported upstream, but if you need it, you need it.  I'll raise
the issue in my team, but I suggest also letting Freescale support (or other
official feedback channels) know what you'd like to see in terms of
improvements in that code and getting it upstream.

-Scott