Patchwork powerpc: dts: Fix canyonlands EMAC interrupt map

login
register
mail settings
Submitter Tanmay Inamdar
Date Nov. 22, 2011, 7:11 a.m.
Message ID <1321945877-22802-1-git-send-email-tinamdar@apm.com>
Download mbox | patch
Permalink /patch/127072/
State Rejected
Headers show

Comments

Tanmay Inamdar - Nov. 22, 2011, 7:11 a.m.
Fixing interrupt mapping of EMAC for canyonlands

Signed-off-by: Tanmay Inamdar <tinamdar@apm.com>
---
 arch/powerpc/boot/dts/canyonlands.dts |   16 ++++++----------
 1 files changed, 6 insertions(+), 10 deletions(-)
Tanmay Inamdar - Nov. 22, 2011, 2:15 p.m.
On Tue, Nov 22, 2011 at 5:00 PM, Josh Boyer <jwboyer@gmail.com> wrote:

> On Tue, Nov 22, 2011 at 2:11 AM, Tanmay Inamdar <tinamdar@apm.com> wrote:
> > Fixing interrupt mapping of EMAC for canyonlands
> >
> > Signed-off-by: Tanmay Inamdar <tinamdar@apm.com>
>
> As far as I can tell, your changes aren't really changing anything
> just making it a bit clearer, correct?  If so, do you mind if I change
> the commit log to "clear up" instead of fix?
>

Actually Rob Herring's commit (
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=commit;h=dc9372808412edbc653a675a526c2ee6c0c14a91)
breaks the interrupt mapping in EMAC driver.
I am trying to fix this issue by mapping interrupts in different way.

If you think "clear up" is fine, then please go ahead.

Thanks,
Tanmay

>
> josh
>
> > ---
> >  arch/powerpc/boot/dts/canyonlands.dts |   16 ++++++----------
> >  1 files changed, 6 insertions(+), 10 deletions(-)
> >
> > diff --git a/arch/powerpc/boot/dts/canyonlands.dts
> b/arch/powerpc/boot/dts/canyonlands.dts
> > index 3dc75de..c76bbcd 100644
> > --- a/arch/powerpc/boot/dts/canyonlands.dts
> > +++ b/arch/powerpc/boot/dts/canyonlands.dts
> > @@ -360,13 +360,11 @@
> >                        EMAC0: ethernet@ef600e00 {
> >                                device_type = "network";
> >                                compatible = "ibm,emac-460ex",
> "ibm,emac4sync";
> > -                               interrupt-parent = <&EMAC0>;
> > -                               interrupts = <0x0 0x1>;
> > -                               #interrupt-cells = <1>;
> > +                               interrupt-parent = <&UIC2>;
> >                                #address-cells = <0>;
> >                                #size-cells = <0>;
> > -                               interrupt-map = </*Status*/ 0x0 &UIC2
> 0x10 0x4
> > -                                                /*Wake*/   0x1 &UIC2
> 0x14 0x4>;
> > +                               interrupts = </*Status*/0x10 0x4
> > +                                               /*Wake*/0x14 0x4>;
> >                                reg = <0xef600e00 0x000000c4>;
> >                                local-mac-address = [000000000000]; /*
> Filled in by U-Boot */
> >                                mal-device = <&MAL0>;
> > @@ -390,13 +388,11 @@
> >                        EMAC1: ethernet@ef600f00 {
> >                                device_type = "network";
> >                                compatible = "ibm,emac-460ex",
> "ibm,emac4sync";
> > -                               interrupt-parent = <&EMAC1>;
> > -                               interrupts = <0x0 0x1>;
> > -                               #interrupt-cells = <1>;
> > +                               interrupt-parent = <&UIC2>;
> >                                #address-cells = <0>;
> >                                #size-cells = <0>;
> > -                               interrupt-map = </*Status*/ 0x0 &UIC2
> 0x11 0x4
> > -                                                /*Wake*/   0x1 &UIC2
> 0x15 0x4>;
> > +                               interrupts = </*Status*/0x11 0x4
> > +                                               /*Wake*/0x15 0x4>;
> >                                reg = <0xef600f00 0x000000c4>;
> >                                local-mac-address = [000000000000]; /*
> Filled in by U-Boot */
> >                                mal-device = <&MAL0>;
> > --
> > 1.6.1.rc3
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-kernel"
> in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at  http://www.tux.org/lkml/
> >
>

CONFIDENTIALITY NOTICE: This e-mail message, including any attachments, 
is for the sole use of the intended recipient(s) and contains information 
that is confidential and proprietary to AppliedMicro Corporation or its subsidiaries. 
It is to be used solely for the purpose of furthering the parties' business relationship. 
All unauthorized review, use, disclosure or distribution is prohibited. 
If you are not the intended recipient, please contact the sender by reply e-mail 
and destroy all copies of the original message.
Josh Boyer - Nov. 22, 2011, 2:31 p.m.
On Tue, Nov 22, 2011 at 9:15 AM, Tanmay Inamdar <tinamdar@apm.com> wrote:
>
> On Tue, Nov 22, 2011 at 5:00 PM, Josh Boyer <jwboyer@gmail.com> wrote:
>>
>> On Tue, Nov 22, 2011 at 2:11 AM, Tanmay Inamdar <tinamdar@apm.com> wrote:
>> > Fixing interrupt mapping of EMAC for canyonlands
>> >
>> > Signed-off-by: Tanmay Inamdar <tinamdar@apm.com>
>>
>> As far as I can tell, your changes aren't really changing anything
>> just making it a bit clearer, correct?  If so, do you mind if I change
>> the commit log to "clear up" instead of fix?
>
> Actually Rob Herring's commit
> (http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=commit;h=dc9372808412edbc653a675a526c2ee6c0c14a91)
> breaks the interrupt mapping in EMAC driver.
> I am trying to fix this issue by mapping interrupts in different way.

I see.  That should have been in the commit log then.  I'll add
something like that to it.

josh
Benjamin Herrenschmidt - Nov. 22, 2011, 9:16 p.m.
On Tue, 2011-11-22 at 12:41 +0530, Tanmay Inamdar wrote:
> Fixing interrupt mapping of EMAC for canyonlands

The previous stuff was odd .... but was it broken ?

It was done this way because the EMAC actually has more interrupts than
that which are routed to different UICs, and so doing a local map this
way allows to target multiple parents.

Cheers,
Ben.

> Signed-off-by: Tanmay Inamdar <tinamdar@apm.com>
> ---
>  arch/powerpc/boot/dts/canyonlands.dts |   16 ++++++----------
>  1 files changed, 6 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/powerpc/boot/dts/canyonlands.dts b/arch/powerpc/boot/dts/canyonlands.dts
> index 3dc75de..c76bbcd 100644
> --- a/arch/powerpc/boot/dts/canyonlands.dts
> +++ b/arch/powerpc/boot/dts/canyonlands.dts
> @@ -360,13 +360,11 @@
>  			EMAC0: ethernet@ef600e00 {
>  				device_type = "network";
>  				compatible = "ibm,emac-460ex", "ibm,emac4sync";
> -				interrupt-parent = <&EMAC0>;
> -				interrupts = <0x0 0x1>;
> -				#interrupt-cells = <1>;
> +				interrupt-parent = <&UIC2>;
>  				#address-cells = <0>;
>  				#size-cells = <0>;
> -				interrupt-map = </*Status*/ 0x0 &UIC2 0x10 0x4
> -						 /*Wake*/   0x1 &UIC2 0x14 0x4>;
> +				interrupts = </*Status*/0x10 0x4
> +						/*Wake*/0x14 0x4>;
>  				reg = <0xef600e00 0x000000c4>;
>  				local-mac-address = [000000000000]; /* Filled in by U-Boot */
>  				mal-device = <&MAL0>;
> @@ -390,13 +388,11 @@
>  			EMAC1: ethernet@ef600f00 {
>  				device_type = "network";
>  				compatible = "ibm,emac-460ex", "ibm,emac4sync";
> -				interrupt-parent = <&EMAC1>;
> -				interrupts = <0x0 0x1>;
> -				#interrupt-cells = <1>;
> +				interrupt-parent = <&UIC2>;
>  				#address-cells = <0>;
>  				#size-cells = <0>;
> -				interrupt-map = </*Status*/ 0x0 &UIC2 0x11 0x4
> -						 /*Wake*/   0x1 &UIC2 0x15 0x4>;
> +				interrupts = </*Status*/0x11 0x4
> +						/*Wake*/0x15 0x4>;
>  				reg = <0xef600f00 0x000000c4>;
>  				local-mac-address = [000000000000]; /* Filled in by U-Boot */
>  				mal-device = <&MAL0>;
Benjamin Herrenschmidt - Nov. 22, 2011, 10:05 p.m.
On Tue, 2011-11-22 at 19:45 +0530, Tanmay Inamdar wrote:
> 
> On Tue, Nov 22, 2011 at 5:00 PM, Josh Boyer <jwboyer@gmail.com> wrote:
>         On Tue, Nov 22, 2011 at 2:11 AM, Tanmay Inamdar
>         <tinamdar@apm.com> wrote:
>         > Fixing interrupt mapping of EMAC for canyonlands
>         >
>         > Signed-off-by: Tanmay Inamdar <tinamdar@apm.com>
>         
>         
>         As far as I can tell, your changes aren't really changing
>         anything
>         just making it a bit clearer, correct?  If so, do you mind if
>         I change
>         the commit log to "clear up" instead of fix?
> 
> Actually Rob Herring's commit
> (http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=commit;h=dc9372808412edbc653a675a526c2ee6c0c14a91) breaks the interrupt mapping in EMAC driver.
> I am trying to fix this issue by mapping interrupts in different way.

No. This commit needs to be reverted. It breaks existing practices.
Pointing to yourself as a parent in order to provide a map is an old
trick and it should be supported.

I'll ask Linus to revert.

Cheers,
Ben.

> If you think "clear up" is fine, then please go ahead.
> 
> Thanks,
> Tanmay
> 
>         
>         josh
>         
>         
>         > ---
>         >  arch/powerpc/boot/dts/canyonlands.dts |   16
>         ++++++----------
>         >  1 files changed, 6 insertions(+), 10 deletions(-)
>         >
>         > diff --git a/arch/powerpc/boot/dts/canyonlands.dts
>         b/arch/powerpc/boot/dts/canyonlands.dts
>         > index 3dc75de..c76bbcd 100644
>         > --- a/arch/powerpc/boot/dts/canyonlands.dts
>         > +++ b/arch/powerpc/boot/dts/canyonlands.dts
>         > @@ -360,13 +360,11 @@
>         >                        EMAC0: ethernet@ef600e00 {
>         >                                device_type = "network";
>         >                                compatible =
>         "ibm,emac-460ex", "ibm,emac4sync";
>         > -                               interrupt-parent = <&EMAC0>;
>         > -                               interrupts = <0x0 0x1>;
>         > -                               #interrupt-cells = <1>;
>         > +                               interrupt-parent = <&UIC2>;
>         >                                #address-cells = <0>;
>         >                                #size-cells = <0>;
>         > -                               interrupt-map = </*Status*/
>         0x0 &UIC2 0x10 0x4
>         > -                                                /*Wake*/
>         0x1 &UIC2 0x14 0x4>;
>         > +                               interrupts = </*Status*/0x10
>         0x4
>         > +                                               /*Wake*/0x14
>         0x4>;
>         >                                reg = <0xef600e00
>         0x000000c4>;
>         >                                local-mac-address =
>         [000000000000]; /* Filled in by U-Boot */
>         >                                mal-device = <&MAL0>;
>         > @@ -390,13 +388,11 @@
>         >                        EMAC1: ethernet@ef600f00 {
>         >                                device_type = "network";
>         >                                compatible =
>         "ibm,emac-460ex", "ibm,emac4sync";
>         > -                               interrupt-parent = <&EMAC1>;
>         > -                               interrupts = <0x0 0x1>;
>         > -                               #interrupt-cells = <1>;
>         > +                               interrupt-parent = <&UIC2>;
>         >                                #address-cells = <0>;
>         >                                #size-cells = <0>;
>         > -                               interrupt-map = </*Status*/
>         0x0 &UIC2 0x11 0x4
>         > -                                                /*Wake*/
>         0x1 &UIC2 0x15 0x4>;
>         > +                               interrupts = </*Status*/0x11
>         0x4
>         > +                                               /*Wake*/0x15
>         0x4>;
>         >                                reg = <0xef600f00
>         0x000000c4>;
>         >                                local-mac-address =
>         [000000000000]; /* Filled in by U-Boot */
>         >                                mal-device = <&MAL0>;
>         > --
>         > 1.6.1.rc3
>         >
>         
>         > --
>         > To unsubscribe from this list: send the line "unsubscribe
>         linux-kernel" in
>         > the body of a message to majordomo@vger.kernel.org
>         > More majordomo info at
>          http://vger.kernel.org/majordomo-info.html
>         > Please read the FAQ at  http://www.tux.org/lkml/
>         >
> 
> CONFIDENTIALITY NOTICE: This e-mail message, including any attachments, 
> is for the sole use of the intended recipient(s) and contains information 
> that is confidential and proprietary to AppliedMicro Corporation or its subsidiaries. 
> It is to be used solely for the purpose of furthering the parties' business relationship. 
> All unauthorized review, use, disclosure or distribution is prohibited. 
> If you are not the intended recipient, please contact the sender by reply e-mail 
> and destroy all copies of the original message.
David Gibson - Nov. 23, 2011, 1:15 a.m.
On Wed, Nov 23, 2011 at 08:16:40AM +1100, Benjamin Herrenschmidt wrote:
> On Tue, 2011-11-22 at 12:41 +0530, Tanmay Inamdar wrote:
> > Fixing interrupt mapping of EMAC for canyonlands
> 
> The previous stuff was odd .... but was it broken ?
> 
> It was done this way because the EMAC actually has more interrupts than
> that which are routed to different UICs, and so doing a local map this
> way allows to target multiple parents.

Well, in the canyonlands case, it appears that the interrupts went to
the same pic, so the simpler representation should be correct as
well.  However, there certainly are boards where they go to multiple
pics, so we need this interrupt-map trick.
Benjamin Herrenschmidt - Nov. 23, 2011, 1:42 a.m.
On Wed, 2011-11-23 at 12:15 +1100, David Gibson wrote:
> On Wed, Nov 23, 2011 at 08:16:40AM +1100, Benjamin Herrenschmidt wrote:
> > On Tue, 2011-11-22 at 12:41 +0530, Tanmay Inamdar wrote:
> > > Fixing interrupt mapping of EMAC for canyonlands
> > 
> > The previous stuff was odd .... but was it broken ?
> > 
> > It was done this way because the EMAC actually has more interrupts than
> > that which are routed to different UICs, and so doing a local map this
> > way allows to target multiple parents.
> 
> Well, in the canyonlands case, it appears that the interrupts went to
> the same pic, so the simpler representation should be correct as
> well.  However, there certainly are boards where they go to multiple
> pics, so we need this interrupt-map trick.

Doesn't emac has something like 4 more interrupts that we simply haven't
been bothered wiring up (because we don't use them ?)

Cheers,
Ben.

Patch

diff --git a/arch/powerpc/boot/dts/canyonlands.dts b/arch/powerpc/boot/dts/canyonlands.dts
index 3dc75de..c76bbcd 100644
--- a/arch/powerpc/boot/dts/canyonlands.dts
+++ b/arch/powerpc/boot/dts/canyonlands.dts
@@ -360,13 +360,11 @@ 
 			EMAC0: ethernet@ef600e00 {
 				device_type = "network";
 				compatible = "ibm,emac-460ex", "ibm,emac4sync";
-				interrupt-parent = <&EMAC0>;
-				interrupts = <0x0 0x1>;
-				#interrupt-cells = <1>;
+				interrupt-parent = <&UIC2>;
 				#address-cells = <0>;
 				#size-cells = <0>;
-				interrupt-map = </*Status*/ 0x0 &UIC2 0x10 0x4
-						 /*Wake*/   0x1 &UIC2 0x14 0x4>;
+				interrupts = </*Status*/0x10 0x4
+						/*Wake*/0x14 0x4>;
 				reg = <0xef600e00 0x000000c4>;
 				local-mac-address = [000000000000]; /* Filled in by U-Boot */
 				mal-device = <&MAL0>;
@@ -390,13 +388,11 @@ 
 			EMAC1: ethernet@ef600f00 {
 				device_type = "network";
 				compatible = "ibm,emac-460ex", "ibm,emac4sync";
-				interrupt-parent = <&EMAC1>;
-				interrupts = <0x0 0x1>;
-				#interrupt-cells = <1>;
+				interrupt-parent = <&UIC2>;
 				#address-cells = <0>;
 				#size-cells = <0>;
-				interrupt-map = </*Status*/ 0x0 &UIC2 0x11 0x4
-						 /*Wake*/   0x1 &UIC2 0x15 0x4>;
+				interrupts = </*Status*/0x11 0x4
+						/*Wake*/0x15 0x4>;
 				reg = <0xef600f00 0x000000c4>;
 				local-mac-address = [000000000000]; /* Filled in by U-Boot */
 				mal-device = <&MAL0>;