RE: [PATCH 2/2] Adding PCI-E MSI support for PowerPC 460SX SOC.

2010-01-10 Thread Tirumala Reddy Marri
Please see my answers in line.

-Original Message-
From: Benjamin Herrenschmidt [mailto:b...@kernel.crashing.org] 
Sent: Sunday, January 03, 2010 10:25 PM
To: Tirumala Reddy Marri
Cc: jwbo...@linux.vnet.ibm.com; linuxppc-dev@lists.ozlabs.org; 
linuxppc-...@ozlabs.org; writetoma...@yahoo.com
Subject: Re: [PATCH 2/2] Adding PCI-E MSI support for PowerPC 460SX SOC.

On Wed, 2009-12-23 at 23:28 -0800, tma...@amcc.com wrote:
 From: Tirumala Marri tma...@amcc.com
 
 
 Signed-off-by: Tirumala Marri tma...@amcc.com
 ---
   When 460SX configured as root as a end point E1000(Intell Ethernet card)
   was plugged into the one of the PCI-E ports. I was able to run the 
 traffic
   with MSI interrupts.

So before I even ack or nack that patch, I need to better understand how
your HW works. I've read the doc of the 460EX twice and still don't
quite get it :-)

So my understanding so far is that when reception of MSIs is enabled,
writes to some magic address in the first 1K of BAR0 are interpreted ad
MSIs. The MSI interrupt value (low 16 bits of the 32-bit store in little
endian) is thus interpreted as an interrupt number and send to the UIC.

Is that correct ?

Marri: You are somewhat right. There are two ways to cause the interrupts. 
In first case MSI is generated to root complex by writing to a Address region 
from Endpoint which is mapped to root-complex side MSI Area. 
In second case 
root complex
writes the 64bit MSI address and data pattern in the Endpoint configuration
space. Then endpoint side cpu will write to register in the PCI-E interrupt 
handler register MSIASS(MSI software assert ), this would trigger a memory 
write transaction on the PCI-E bus with address from the config space and data
from data register. As soon as this trans action comes on the BUS PCI-E handler
on root complex snoops for this address and checks against the data received.
If it matches it would cause appropriate interrupt number based on the data 
Received. For example for interrupt 1 , data would be 0x and interrupt
2 data would be 0x0001 .


Now, which UIC ? There are at least 3 in the 460EX for example :-)
Marri: Each of 4 MSI is mapped to UIC0 12,13,14  15 interrupt numbers

Also, UICs have a limited amount of inputs and I don't see many
interrupt sources reserved for use as MSIs, can you enlighten me a bit
more on how you get to choose an interrupt source to use as MSI ?

Marri: There are 4 MIS's or 15 MSI-X interrupts can be enabled. Each MSI is hard
Wired to UIC0 12 to 15. For MSI-X UIC-3 12 to 27.

Or is there some translation done ? IE. In the 460EX manual, there seem
to be specific interrupt numbers dedicated to PCI0 MSI 0, 1 2 and 3
spread between UIC0 and UIC1, and a block of 8 interrupts in UIC3
reserved for PCI-E MSIs. Is there a renumbering done in HW here ?



IE. Your table shows for 460EX for example that PCI-E MSI 2 is UIC3
input 26. Do I need thus to program the device to write a 2 in the MSI
message or 26 to hit that interrupt ?

IE. Are you running the input message through a binary decoder that then
spreads into various UIC input lines ?


Marri: Yes each MSI is hard wired to different interrupt number in UIC 
registers.
MIS interrupt number to UIC is not programmable . It is fixed.


Now some comments:


 diff --git a/arch/powerpc/boot/dts/redwood.dts 
 b/arch/powerpc/boot/dts/redwood.dts
 index 81636c0..6c20faf 100644
 --- a/arch/powerpc/boot/dts/redwood.dts
 +++ b/arch/powerpc/boot/dts/redwood.dts
 @@ -357,6 +357,21 @@
   0x0 0x0 0x0 0x3 UIC3 0xa 0x4 /* swizzled int C 
 */
   0x0 0x0 0x0 0x4 UIC3 0xb 0x4 /* swizzled int D 
 */;
   };
 + MSI: ppc4xx-...@40030 {
 + compatible = amcc,ppc4xx-460sx-msi, ppc4xx-msi;
 + reg =  0x4 0x0030 0x100
 + 0x4 0x0030 0x100;
 + sdr-base = 0x3B0;
 + interrupts =0 1 2 3;
 + interrupt-parent = MSI;
 + #interrupt-cells = 1;
 + #address-cells = 0;
 + #size-cells = 0;
 + interrupt-map = 0 UIC0 0xC 1
 + 1 UIC0 0x0D 1
 + 2 UIC0 0x0E 1
 + 3 UIC0 0x0F 1;
 + };

Wow ! That's the mother of all device-tree hacks :-) So you are using
the interrupts property of the MSI node to represent the MSI
interrupts you hand out, and you make it its own interrupt-parent, using
an interrupt-map in itself to convert them into UIC interrupts :-)
Sneaky... Hell, it will work so why not ?


Marri: BTW there are some other processors using the similar way.


 +static struct ppc4xx_msi *ppc4xx_msi;
 +
 +struct ppc4xx_msi_feature {
 + u32 ppc4xx_pic_ip;
 + u32 msiir_offset;
 +};
 +
 +static int

Re: [PATCH 2/2] Adding PCI-E MSI support for PowerPC 460SX SOC.

2010-01-03 Thread Benjamin Herrenschmidt
On Wed, 2009-12-23 at 23:28 -0800, tma...@amcc.com wrote:
 From: Tirumala Marri tma...@amcc.com
 
 
 Signed-off-by: Tirumala Marri tma...@amcc.com
 ---
   When 460SX configured as root as a end point E1000(Intell Ethernet card)
   was plugged into the one of the PCI-E ports. I was able to run the 
 traffic
   with MSI interrupts.

So before I even ack or nack that patch, I need to better understand how
your HW works. I've read the doc of the 460EX twice and still don't
quite get it :-)

So my understanding so far is that when reception of MSIs is enabled,
writes to some magic address in the first 1K of BAR0 are interpreted ad
MSIs. The MSI interrupt value (low 16 bits of the 32-bit store in little
endian) is thus interpreted as an interrupt number and send to the UIC.

Is that correct ?

Now, which UIC ? There are at least 3 in the 460EX for example :-)

Also, UICs have a limited amount of inputs and I don't see many
interrupt sources reserved for use as MSIs, can you enlighten me a bit
more on how you get to choose an interrupt source to use as MSI ?

Or is there some translation done ? IE. In the 460EX manual, there seem
to be specific interrupt numbers dedicated to PCI0 MSI 0, 1 2 and 3
spread between UIC0 and UIC1, and a block of 8 interrupts in UIC3
reserved for PCI-E MSIs. Is there a renumbering done in HW here ?

IE. Your table shows for 460EX for example that PCI-E MSI 2 is UIC3
input 26. Do I need thus to program the device to write a 2 in the MSI
message or 26 to hit that interrupt ?

IE. Are you running the input message through a binary decoder that then
spreads into various UIC input lines ?

Now some comments:

 diff --git a/arch/powerpc/boot/dts/redwood.dts 
 b/arch/powerpc/boot/dts/redwood.dts
 index 81636c0..6c20faf 100644
 --- a/arch/powerpc/boot/dts/redwood.dts
 +++ b/arch/powerpc/boot/dts/redwood.dts
 @@ -357,6 +357,21 @@
   0x0 0x0 0x0 0x3 UIC3 0xa 0x4 /* swizzled int C 
 */
   0x0 0x0 0x0 0x4 UIC3 0xb 0x4 /* swizzled int D 
 */;
   };
 + MSI: ppc4xx-...@40030 {
 + compatible = amcc,ppc4xx-460sx-msi, ppc4xx-msi;
 + reg =  0x4 0x0030 0x100
 + 0x4 0x0030 0x100;
 + sdr-base = 0x3B0;
 + interrupts =0 1 2 3;
 + interrupt-parent = MSI;
 + #interrupt-cells = 1;
 + #address-cells = 0;
 + #size-cells = 0;
 + interrupt-map = 0 UIC0 0xC 1
 + 1 UIC0 0x0D 1
 + 2 UIC0 0x0E 1
 + 3 UIC0 0x0F 1;
 + };

Wow ! That's the mother of all device-tree hacks :-) So you are using
the interrupts property of the MSI node to represent the MSI
interrupts you hand out, and you make it its own interrupt-parent, using
an interrupt-map in itself to convert them into UIC interrupts :-)
Sneaky... Hell, it will work so why not ?

 +static struct ppc4xx_msi *ppc4xx_msi;
 +
 +struct ppc4xx_msi_feature {
 + u32 ppc4xx_pic_ip;
 + u32 msiir_offset;
 +};
 +
 +static int ppc4xx_msi_init_allocator(struct ppc4xx_msi *msi_data)
 +{
 + int rc;
 +
 + rc = msi_bitmap_alloc(msi_data-bitmap, NR_MSI_IRQS,
 + msi_data-irqhost-of_node);
 + if (rc)
 + return rc;
 + rc = msi_bitmap_reserve_dt_hwirqs(msi_data-bitmap);
 + if (rc  0) {
 + msi_bitmap_free(msi_data-bitmap);
 + return rc;
 + }
 + return 0;
 +}

Ok so here I start having problems :-) First you allocate a bitmap for
the MSIs of a fixed number, despite the fact that there seem to be a
different number of MSIs supported depending on what part/bridge you are
using. Then, you try to call msi_bitmap_reserve_dt_hwirqs()... why
that ? This is meant to be used when you don't know what interrupt
numbers are reserved for use by MSIs in your system, it's a bit fishy to
be honest, we use it on powermac mostly :-)

 +static void ppc4xx_msi_cascade(unsigned int irq, struct irq_desc *desc)
 +{
 + unsigned int cascade_irq;
 + struct ppc4xx_msi *msi_data = ppc4xx_msi;
 + int msir_index = -1;
 +
 + raw_spin_lock(desc-lock);
 + if (desc-chip-mask_ack) {
 + desc-chip-mask_ack(irq);
 + } else {
 + desc-chip-mask(irq);
 + desc-chip-ack(irq);
 + }
 +
 + if (unlikely(desc-status  IRQ_INPROGRESS))
 + goto unlock;
 +
 + msir_index = (int)desc-handler_data;
 +
 + if (msir_index = NR_MSI_IRQS)
 + cascade_irq = NO_IRQ;
 +
 + desc-status |= IRQ_INPROGRESS;
 +
 + cascade_irq = irq_linear_revmap(msi_data-irqhost, msir_index);
 + if (cascade_irq != NO_IRQ)
 + generic_handle_irq(cascade_irq);
 + desc-status = ~IRQ_INPROGRESS;
 +
 + if (!(desc-status  IRQ_DISABLED)  desc-chip-unmask)
 +   

Re: [PATCH 2/2] Adding PCI-E MSI support for PowerPC 460SX SOC.

2009-12-23 Thread Stefan Roese
On Wednesday 23 December 2009 08:52:23 tma...@amcc.com wrote:
 From: Tirumala Marri tma...@amcc.com

Please find some mostly nitpicking comments below.

BTW: Did you already test this on other 4xx platforms, like 440SPe or 405EX? 
What are your plans here?
 
 Signed-off-by: Tirumala Marri tma...@amcc.com
 ---
 Kernel version: 2.6.33-rc1
 Testing:
   When 460SX configured as root as a end point E1000(Intell Ethernet card)
   was plugged into the one of the PCI-E ports. I was able to run the 
 traffic
   with MSI interrupts.
 ---
  arch/powerpc/boot/dts/redwood.dts  |   15 ++
  arch/powerpc/configs/44x/redwood_defconfig |5 +-
  arch/powerpc/platforms/44x/Kconfig |1 +
  arch/powerpc/sysdev/Kconfig|7 +
  arch/powerpc/sysdev/Makefile   |1 +
  arch/powerpc/sysdev/ppc4xx_msi.c   |  342
   arch/powerpc/sysdev/ppc4xx_msi.h   | 
   39 
  7 files changed, 408 insertions(+), 2 deletions(-)
  create mode 100644 arch/powerpc/sysdev/ppc4xx_msi.c
  create mode 100644 arch/powerpc/sysdev/ppc4xx_msi.h
 
 diff --git a/arch/powerpc/boot/dts/redwood.dts
  b/arch/powerpc/boot/dts/redwood.dts index 81636c0..412d5f9 100644
 --- a/arch/powerpc/boot/dts/redwood.dts
 +++ b/arch/powerpc/boot/dts/redwood.dts
 @@ -357,6 +357,21 @@
   0x0 0x0 0x0 0x3 UIC3 0xa 0x4 /* swizzled int C 
 */
   0x0 0x0 0x0 0x4 UIC3 0xb 0x4 /* swizzled int D 
 */;
   };
 + MSI: ppc4xx-...@40030 {
 + compatible = amcc,ppc4xx-msi, ppc4xx-msi;

Better use something like this:

compatible = amcc,ppc4xx-msi-ppc460sx, 
amcc,ppc4xx-msi;

This way you could check for 460SX specials in the driver if needed.

 + reg =  0x4 0x0030 0x100
 + 0x4 0x0030 0x100;
 + sdr-base = 0x3B0;
 + interrupts =0 1 2 3;
 + interrupt-parent = MSI;
 + #interrupt-cells = 1;
 + #address-cells = 0;
 + #size-cells = 0;
 + interrupt-map = 0 UIC0 0xC 1
 + 1 UIC0 0x0D 1
 + 2 UIC0 0x0E 1
 + 3 UIC0 0x0F 1;
 + };
 
   };
 
 diff --git a/arch/powerpc/configs/44x/redwood_defconfig
  b/arch/powerpc/configs/44x/redwood_defconfig index ed31d4f..5d16c88 100644
 --- a/arch/powerpc/configs/44x/redwood_defconfig
 +++ b/arch/powerpc/configs/44x/redwood_defconfig
 @@ -158,6 +158,7 @@ CONFIG_DEFAULT_AS=y
  CONFIG_DEFAULT_IOSCHED=anticipatory
  # CONFIG_FREEZER is not set
  CONFIG_PPC4xx_PCI_EXPRESS=y
 +CONFIG_PPC_MSI_BITMAP=y
 
  #
  # Platform support
 @@ -264,7 +265,7 @@ CONFIG_PCIEPORTBUS=y
  CONFIG_PCIEAER=y
  # CONFIG_PCIEASPM is not set
  CONFIG_ARCH_SUPPORTS_MSI=y
 -# CONFIG_PCI_MSI is not set
 +CONFIG_PCI_MSI=y
  # CONFIG_PCI_LEGACY is not set
  # CONFIG_PCI_DEBUG is not set
  # CONFIG_PCI_STUB is not set
 @@ -1062,7 +1063,7 @@ CONFIG_PRINT_STACK_DEPTH=64
  # CONFIG_DEBUG_PAGEALLOC is not set
  # CONFIG_CODE_PATCHING_SELFTEST is not set
  # CONFIG_FTR_FIXUP_SELFTEST is not set
 -# CONFIG_MSI_BITMAP_SELFTEST is not set
 +CONFIG_MSI_BITMAP_SELFTEST=y
  # CONFIG_XMON is not set
  # CONFIG_IRQSTACKS is not set
  # CONFIG_VIRQ_DEBUG is not set
 diff --git a/arch/powerpc/platforms/44x/Kconfig
  b/arch/powerpc/platforms/44x/Kconfig index 7486bff..9c3b8ca 100644
 --- a/arch/powerpc/platforms/44x/Kconfig
 +++ b/arch/powerpc/platforms/44x/Kconfig
 @@ -126,6 +126,7 @@ config REDWOOD
   select 460SX
   select PCI
   select PPC4xx_PCI_EXPRESS
 + select PPC4xx_MSI
   help
 This option enables support for the AMCC PPC460SX Redwood board.
 
 diff --git a/arch/powerpc/sysdev/Kconfig b/arch/powerpc/sysdev/Kconfig
 index 3965828..c8f1486 100644
 --- a/arch/powerpc/sysdev/Kconfig
 +++ b/arch/powerpc/sysdev/Kconfig
 @@ -7,8 +7,15 @@ config PPC4xx_PCI_EXPRESS
   depends on PCI  4xx
   default n
 
 +config PPC4xx_MSI
 + bool
 + depends on PCI_MSI
 + depends on PCI  4xx
 + default n
 +
  config PPC_MSI_BITMAP
   bool
   depends on PCI_MSI
   default y if MPIC
   default y if FSL_PCI
 + default y if PPC4xx_MSI
 diff --git a/arch/powerpc/sysdev/Makefile b/arch/powerpc/sysdev/Makefile
 index 5642924..4c67d2d 100644
 --- a/arch/powerpc/sysdev/Makefile
 +++ b/arch/powerpc/sysdev/Makefile
 @@ -36,6 +36,7 @@ obj-$(CONFIG_PPC_I8259) += i8259.o
  obj-$(CONFIG_IPIC)   += ipic.o
  obj-$(CONFIG_4xx)+= uic.o
  obj-$(CONFIG_4xx_SOC)+= ppc4xx_soc.o
 +obj-$(CONFIG_PPC4xx_MSI) += ppc4xx_msi.o
  obj-$(CONFIG_XILINX_VIRTEX)  += xilinx_intc.o
  obj-$(CONFIG_XILINX_PCI) += xilinx_pci.o
  obj-$(CONFIG_OF_RTC) += of_rtc.o
 diff --git a/arch/powerpc/sysdev/ppc4xx_msi.c
  

RE: [PATCH 2/2] Adding PCI-E MSI support for PowerPC 460SX SOC.

2009-12-23 Thread Tirumala Reddy Marri
Thanks for the suggestions. I will try remove the extra lines . Add
changes you suggested.
-Marri

-Original Message-
From: linuxppc-dev-bounces+tmarri=amcc@lists.ozlabs.org
[mailto:linuxppc-dev-bounces+tmarri=amcc@lists.ozlabs.org] On Behalf
Of Stefan Roese
Sent: Wednesday, December 23, 2009 12:19 AM
To: linuxppc-dev@lists.ozlabs.org
Cc: linuxppc-...@ozlabs.org; writetoma...@yahoo.com; Tirumala Reddy
Marri
Subject: Re: [PATCH 2/2] Adding PCI-E MSI support for PowerPC 460SX SOC.

On Wednesday 23 December 2009 08:52:23 tma...@amcc.com wrote:
 From: Tirumala Marri tma...@amcc.com

Please find some mostly nitpicking comments below.

BTW: Did you already test this on other 4xx platforms, like 440SPe or
405EX? 
What are your plans here?
 
 Signed-off-by: Tirumala Marri tma...@amcc.com
 ---
 Kernel version: 2.6.33-rc1
 Testing:
   When 460SX configured as root as a end point E1000(Intell
Ethernet card)
   was plugged into the one of the PCI-E ports. I was able to run
the traffic
   with MSI interrupts.
 ---
  arch/powerpc/boot/dts/redwood.dts  |   15 ++
  arch/powerpc/configs/44x/redwood_defconfig |5 +-
  arch/powerpc/platforms/44x/Kconfig |1 +
  arch/powerpc/sysdev/Kconfig|7 +
  arch/powerpc/sysdev/Makefile   |1 +
  arch/powerpc/sysdev/ppc4xx_msi.c   |  342
   arch/powerpc/sysdev/ppc4xx_msi.h
| 
   39 
  7 files changed, 408 insertions(+), 2 deletions(-)
  create mode 100644 arch/powerpc/sysdev/ppc4xx_msi.c
  create mode 100644 arch/powerpc/sysdev/ppc4xx_msi.h
 
 diff --git a/arch/powerpc/boot/dts/redwood.dts
  b/arch/powerpc/boot/dts/redwood.dts index 81636c0..412d5f9 100644
 --- a/arch/powerpc/boot/dts/redwood.dts
 +++ b/arch/powerpc/boot/dts/redwood.dts
 @@ -357,6 +357,21 @@
   0x0 0x0 0x0 0x3 UIC3 0xa 0x4 /*
swizzled int C */
   0x0 0x0 0x0 0x4 UIC3 0xb 0x4 /*
swizzled int D */;
   };
 + MSI: ppc4xx-...@40030 {
 + compatible = amcc,ppc4xx-msi, ppc4xx-msi;

Better use something like this:

compatible = amcc,ppc4xx-msi-ppc460sx,
amcc,ppc4xx-msi;

This way you could check for 460SX specials in the driver if needed.

 + reg =  0x4 0x0030 0x100
 + 0x4 0x0030 0x100;
 + sdr-base = 0x3B0;
 + interrupts =0 1 2 3;
 + interrupt-parent = MSI;
 + #interrupt-cells = 1;
 + #address-cells = 0;
 + #size-cells = 0;
 + interrupt-map = 0 UIC0 0xC 1
 + 1 UIC0 0x0D 1
 + 2 UIC0 0x0E 1
 + 3 UIC0 0x0F 1;
 + };
 
   };
 
 diff --git a/arch/powerpc/configs/44x/redwood_defconfig
  b/arch/powerpc/configs/44x/redwood_defconfig index ed31d4f..5d16c88
100644
 --- a/arch/powerpc/configs/44x/redwood_defconfig
 +++ b/arch/powerpc/configs/44x/redwood_defconfig
 @@ -158,6 +158,7 @@ CONFIG_DEFAULT_AS=y
  CONFIG_DEFAULT_IOSCHED=anticipatory
  # CONFIG_FREEZER is not set
  CONFIG_PPC4xx_PCI_EXPRESS=y
 +CONFIG_PPC_MSI_BITMAP=y
 
  #
  # Platform support
 @@ -264,7 +265,7 @@ CONFIG_PCIEPORTBUS=y
  CONFIG_PCIEAER=y
  # CONFIG_PCIEASPM is not set
  CONFIG_ARCH_SUPPORTS_MSI=y
 -# CONFIG_PCI_MSI is not set
 +CONFIG_PCI_MSI=y
  # CONFIG_PCI_LEGACY is not set
  # CONFIG_PCI_DEBUG is not set
  # CONFIG_PCI_STUB is not set
 @@ -1062,7 +1063,7 @@ CONFIG_PRINT_STACK_DEPTH=64
  # CONFIG_DEBUG_PAGEALLOC is not set
  # CONFIG_CODE_PATCHING_SELFTEST is not set
  # CONFIG_FTR_FIXUP_SELFTEST is not set
 -# CONFIG_MSI_BITMAP_SELFTEST is not set
 +CONFIG_MSI_BITMAP_SELFTEST=y
  # CONFIG_XMON is not set
  # CONFIG_IRQSTACKS is not set
  # CONFIG_VIRQ_DEBUG is not set
 diff --git a/arch/powerpc/platforms/44x/Kconfig
  b/arch/powerpc/platforms/44x/Kconfig index 7486bff..9c3b8ca 100644
 --- a/arch/powerpc/platforms/44x/Kconfig
 +++ b/arch/powerpc/platforms/44x/Kconfig
 @@ -126,6 +126,7 @@ config REDWOOD
   select 460SX
   select PCI
   select PPC4xx_PCI_EXPRESS
 + select PPC4xx_MSI
   help
 This option enables support for the AMCC PPC460SX Redwood
board.
 
 diff --git a/arch/powerpc/sysdev/Kconfig b/arch/powerpc/sysdev/Kconfig
 index 3965828..c8f1486 100644
 --- a/arch/powerpc/sysdev/Kconfig
 +++ b/arch/powerpc/sysdev/Kconfig
 @@ -7,8 +7,15 @@ config PPC4xx_PCI_EXPRESS
   depends on PCI  4xx
   default n
 
 +config PPC4xx_MSI
 + bool
 + depends on PCI_MSI
 + depends on PCI  4xx
 + default n
 +
  config PPC_MSI_BITMAP
   bool
   depends on PCI_MSI
   default y if MPIC
   default y if FSL_PCI
 + default y if PPC4xx_MSI
 diff --git a/arch/powerpc/sysdev/Makefile
b/arch/powerpc/sysdev/Makefile
 index 5642924..4c67d2d 100644
 --- a/arch/powerpc/sysdev/Makefile

RE: [PATCH 2/2] Adding PCI-E MSI support for PowerPC 460SX SOC.

2009-12-23 Thread Tirumala Reddy Marri
BTW once this patch gets in I will add the 405Ex,460Ex and 440Spe
support to the same.

-Original Message-
From: linuxppc-dev-bounces+tmarri=amcc@lists.ozlabs.org
[mailto:linuxppc-dev-bounces+tmarri=amcc@lists.ozlabs.org] On Behalf
Of Tirumala Reddy Marri
Sent: Wednesday, December 23, 2009 9:23 AM
To: Stefan Roese; linuxppc-dev@lists.ozlabs.org
Cc: linuxppc-...@ozlabs.org; writetoma...@yahoo.com
Subject: RE: [PATCH 2/2] Adding PCI-E MSI support for PowerPC 460SX SOC.

Thanks for the suggestions. I will try remove the extra lines . Add
changes you suggested.
-Marri

-Original Message-
From: linuxppc-dev-bounces+tmarri=amcc@lists.ozlabs.org
[mailto:linuxppc-dev-bounces+tmarri=amcc@lists.ozlabs.org] On Behalf
Of Stefan Roese
Sent: Wednesday, December 23, 2009 12:19 AM
To: linuxppc-dev@lists.ozlabs.org
Cc: linuxppc-...@ozlabs.org; writetoma...@yahoo.com; Tirumala Reddy
Marri
Subject: Re: [PATCH 2/2] Adding PCI-E MSI support for PowerPC 460SX SOC.

On Wednesday 23 December 2009 08:52:23 tma...@amcc.com wrote:
 From: Tirumala Marri tma...@amcc.com

Please find some mostly nitpicking comments below.

BTW: Did you already test this on other 4xx platforms, like 440SPe or
405EX? 
What are your plans here?
 
 Signed-off-by: Tirumala Marri tma...@amcc.com
 ---
 Kernel version: 2.6.33-rc1
 Testing:
   When 460SX configured as root as a end point E1000(Intell
Ethernet card)
   was plugged into the one of the PCI-E ports. I was able to run
the traffic
   with MSI interrupts.
 ---
  arch/powerpc/boot/dts/redwood.dts  |   15 ++
  arch/powerpc/configs/44x/redwood_defconfig |5 +-
  arch/powerpc/platforms/44x/Kconfig |1 +
  arch/powerpc/sysdev/Kconfig|7 +
  arch/powerpc/sysdev/Makefile   |1 +
  arch/powerpc/sysdev/ppc4xx_msi.c   |  342
   arch/powerpc/sysdev/ppc4xx_msi.h
| 
   39 
  7 files changed, 408 insertions(+), 2 deletions(-)
  create mode 100644 arch/powerpc/sysdev/ppc4xx_msi.c
  create mode 100644 arch/powerpc/sysdev/ppc4xx_msi.h
 
 diff --git a/arch/powerpc/boot/dts/redwood.dts
  b/arch/powerpc/boot/dts/redwood.dts index 81636c0..412d5f9 100644
 --- a/arch/powerpc/boot/dts/redwood.dts
 +++ b/arch/powerpc/boot/dts/redwood.dts
 @@ -357,6 +357,21 @@
   0x0 0x0 0x0 0x3 UIC3 0xa 0x4 /*
swizzled int C */
   0x0 0x0 0x0 0x4 UIC3 0xb 0x4 /*
swizzled int D */;
   };
 + MSI: ppc4xx-...@40030 {
 + compatible = amcc,ppc4xx-msi, ppc4xx-msi;

Better use something like this:

compatible = amcc,ppc4xx-msi-ppc460sx,
amcc,ppc4xx-msi;

This way you could check for 460SX specials in the driver if needed.

 + reg =  0x4 0x0030 0x100
 + 0x4 0x0030 0x100;
 + sdr-base = 0x3B0;
 + interrupts =0 1 2 3;
 + interrupt-parent = MSI;
 + #interrupt-cells = 1;
 + #address-cells = 0;
 + #size-cells = 0;
 + interrupt-map = 0 UIC0 0xC 1
 + 1 UIC0 0x0D 1
 + 2 UIC0 0x0E 1
 + 3 UIC0 0x0F 1;
 + };
 
   };
 
 diff --git a/arch/powerpc/configs/44x/redwood_defconfig
  b/arch/powerpc/configs/44x/redwood_defconfig index ed31d4f..5d16c88
100644
 --- a/arch/powerpc/configs/44x/redwood_defconfig
 +++ b/arch/powerpc/configs/44x/redwood_defconfig
 @@ -158,6 +158,7 @@ CONFIG_DEFAULT_AS=y
  CONFIG_DEFAULT_IOSCHED=anticipatory
  # CONFIG_FREEZER is not set
  CONFIG_PPC4xx_PCI_EXPRESS=y
 +CONFIG_PPC_MSI_BITMAP=y
 
  #
  # Platform support
 @@ -264,7 +265,7 @@ CONFIG_PCIEPORTBUS=y
  CONFIG_PCIEAER=y
  # CONFIG_PCIEASPM is not set
  CONFIG_ARCH_SUPPORTS_MSI=y
 -# CONFIG_PCI_MSI is not set
 +CONFIG_PCI_MSI=y
  # CONFIG_PCI_LEGACY is not set
  # CONFIG_PCI_DEBUG is not set
  # CONFIG_PCI_STUB is not set
 @@ -1062,7 +1063,7 @@ CONFIG_PRINT_STACK_DEPTH=64
  # CONFIG_DEBUG_PAGEALLOC is not set
  # CONFIG_CODE_PATCHING_SELFTEST is not set
  # CONFIG_FTR_FIXUP_SELFTEST is not set
 -# CONFIG_MSI_BITMAP_SELFTEST is not set
 +CONFIG_MSI_BITMAP_SELFTEST=y
  # CONFIG_XMON is not set
  # CONFIG_IRQSTACKS is not set
  # CONFIG_VIRQ_DEBUG is not set
 diff --git a/arch/powerpc/platforms/44x/Kconfig
  b/arch/powerpc/platforms/44x/Kconfig index 7486bff..9c3b8ca 100644
 --- a/arch/powerpc/platforms/44x/Kconfig
 +++ b/arch/powerpc/platforms/44x/Kconfig
 @@ -126,6 +126,7 @@ config REDWOOD
   select 460SX
   select PCI
   select PPC4xx_PCI_EXPRESS
 + select PPC4xx_MSI
   help
 This option enables support for the AMCC PPC460SX Redwood
board.
 
 diff --git a/arch/powerpc/sysdev/Kconfig b/arch/powerpc/sysdev/Kconfig
 index 3965828..c8f1486 100644
 --- a/arch/powerpc/sysdev/Kconfig
 +++ b/arch/powerpc/sysdev

Re: [PATCH 2/2] Adding PCI-E MSI support for PowerPC 460SX SOC.

2009-12-22 Thread Josh Boyer
On Tue, Dec 22, 2009 at 12:49:51AM -0800, tma...@amcc.com wrote:
From: Tirumala Marri tma...@amcc.com


Signed-off-by: Tirumala Marri tma...@amcc.com
---
Kernel version: 2.6.33-rc1
Testing:
   When 460SX configured as root as a end point E1000(Intell Ethernet 
 card) 
   was plugged into the one of the PCI-E ports. I was able to run the 
 traffic
   with MSI interrupts. 
---
 arch/powerpc/boot/dts/redwood.dts  |   15 ++
 arch/powerpc/configs/44x/redwood_defconfig |5 +-
 arch/powerpc/platforms/44x/Kconfig |1 +
 arch/powerpc/sysdev/Kconfig|7 +
 arch/powerpc/sysdev/Makefile   |1 +
 arch/powerpc/sysdev/ppc4xx_msi.c   |  335 
 arch/powerpc/sysdev/ppc4xx_msi.h   |   49 
 7 files changed, 411 insertions(+), 2 deletions(-)
 create mode 100644 arch/powerpc/sysdev/ppc4xx_msi.c
 create mode 100644 arch/powerpc/sysdev/ppc4xx_msi.h

diff --git a/arch/powerpc/boot/dts/redwood.dts 
b/arch/powerpc/boot/dts/redwood.dts
index 81636c0..412d5f9 100644
--- a/arch/powerpc/boot/dts/redwood.dts
+++ b/arch/powerpc/boot/dts/redwood.dts
@@ -357,6 +357,21 @@
   0x0 0x0 0x0 0x3 UIC3 0xa 0x4 /* swizzled int C 
 */
   0x0 0x0 0x0 0x4 UIC3 0xb 0x4 /* swizzled int D 
 */;
   };
+  MSI: ppc4xx-...@40030 {
+  compatible = amcc,ppc4xx-msi, ppc4xx-msi;
+  reg =  0x4 0x0030 0x100
+  0x4 0x0030 0x100;
+  sdr-base = 0x3B0;
+  interrupts =0 1 2 3;
+  interrupt-parent = MSI;
+  #interrupt-cells = 1;
+  #address-cells = 0;
+  #size-cells = 0;
+  interrupt-map = 0 UIC0 0xC 1
+  1 UIC0 0x0D 1
+  2 UIC0 0x0E 1
+  3 UIC0 0x0F 1;
+  };
 
   };
 
diff --git a/arch/powerpc/sysdev/Kconfig b/arch/powerpc/sysdev/Kconfig
index 3965828..32f5a40 100644
--- a/arch/powerpc/sysdev/Kconfig
+++ b/arch/powerpc/sysdev/Kconfig
@@ -7,8 +7,15 @@ config PPC4xx_PCI_EXPRESS
   depends on PCI  4xx
   default n
 
+config 4xx_MSI

This should probably be named PPC4xx_MSI, similar to how
PPC4xx_PCI_EXPRESS is named.

+  bool
+  depends on PCI_MSI
+  depends on PCI  4xx
+  default n
+
 config PPC_MSI_BITMAP
   bool
   depends on PCI_MSI
   default y if MPIC
   default y if FSL_PCI
+  default y if 4xx_MSI

diff --git a/arch/powerpc/sysdev/ppc4xx_msi.c 
b/arch/powerpc/sysdev/ppc4xx_msi.c
new file mode 100644
index 000..752da4b
--- /dev/null
+++ b/arch/powerpc/sysdev/ppc4xx_msi.c
@@ -0,0 +1,335 @@
+/*
+ * Copyright (C) 2009 Applied Micro Circuits corporation,
+ * All rights reserved.

Please don't add the 'All rights reserved.' to new files.  It is
inaccurate and confusing given that it's a GPLv2 file.

+ *
+ * Author: Feng Kan f...@amcc.com
+ *   Tirumala Marri tma...@amcc.com
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; version 2 of the
+ * License.
+ */
+#include linux/irq.h
+#include linux/bootmem.h
+#include linux/pci.h
+#include linux/msi.h
+#include linux/of_platform.h
+#include linux/interrupt.h
+#include linux/device.h
+#include asm/prom.h
+#include asm/hw_irq.h
+#include asm/ppc-pci.h
+#include asm/dcr.h
+#include asm/dcr-regs.h
+#include ppc4xx_msi.h
+
+
+static struct ppc4xx_msi *ppc4xx_msi;
+
+struct ppc4xx_msi_feature {
+  u32 ppc4xx_pic_ip;
+  u32 msiir_offset;
+};
+
+static int ppc4xx_msi_init_allocator(struct ppc4xx_msi *msi_data)
+{
+  int rc;
+
+  rc = msi_bitmap_alloc(msi_data-bitmap, NR_MSI_IRQS,
+  msi_data-irqhost-of_node);
+  if (rc)
+  return rc;
+  rc = msi_bitmap_reserve_dt_hwirqs(msi_data-bitmap);
+  if (rc  0) {
+  msi_bitmap_free(msi_data-bitmap);
+  return rc;
+  }
+  return 0;
+}
+
+
+static void ppc4xx_msi_cascade(unsigned int irq, struct irq_desc *desc)
+{
+  unsigned int cascade_irq;
+  struct ppc4xx_msi *msi_data = ppc4xx_msi;
+  int msir_index = -1;
+
+  raw_spin_lock(desc-lock);
+  if (desc-chip-mask_ack) {
+  desc-chip-mask_ack(irq);
+  } else {
+  desc-chip-mask(irq);
+  desc-chip-ack(irq);
+  }
+
+  if (unlikely(desc-status  IRQ_INPROGRESS))
+  goto unlock;
+
+  msir_index = (int)desc-handler_data;
+
+  if (msir_index = NR_MSI_IRQS)
+  cascade_irq = NO_IRQ;
+
+  desc-status |= IRQ_INPROGRESS;
+
+  cascade_irq = irq_linear_revmap(msi_data-irqhost, msir_index);
+  if (cascade_irq != NO_IRQ)
+  generic_handle_irq(cascade_irq);
+   

RE: [PATCH 2/2] Adding PCI-E MSI support for PowerPC 460SX SOC.

2009-12-22 Thread Tirumala Reddy Marri
Josh,
 Thanks for the comments. I will fix them re-submit it.
Regards,
Marri

-Original Message-
From: Josh Boyer [mailto:jwbo...@gmail.com] On Behalf Of Josh Boyer
Sent: Tuesday, December 22, 2009 4:08 AM
To: Tirumala Reddy Marri
Cc: linuxppc-dev@lists.ozlabs.org; writetoma...@yahoo.com
Subject: Re: [PATCH 2/2] Adding PCI-E MSI support for PowerPC 460SX SOC.

On Tue, Dec 22, 2009 at 12:49:51AM -0800, tma...@amcc.com wrote:
From: Tirumala Marri tma...@amcc.com


Signed-off-by: Tirumala Marri tma...@amcc.com
---
Kernel version: 2.6.33-rc1
Testing:
   When 460SX configured as root as a end point E1000(Intell
Ethernet card) 
   was plugged into the one of the PCI-E ports. I was able to run
the traffic
   with MSI interrupts. 
---
 arch/powerpc/boot/dts/redwood.dts  |   15 ++
 arch/powerpc/configs/44x/redwood_defconfig |5 +-
 arch/powerpc/platforms/44x/Kconfig |1 +
 arch/powerpc/sysdev/Kconfig|7 +
 arch/powerpc/sysdev/Makefile   |1 +
 arch/powerpc/sysdev/ppc4xx_msi.c   |  335

 arch/powerpc/sysdev/ppc4xx_msi.h   |   49 
 7 files changed, 411 insertions(+), 2 deletions(-)
 create mode 100644 arch/powerpc/sysdev/ppc4xx_msi.c
 create mode 100644 arch/powerpc/sysdev/ppc4xx_msi.h

diff --git a/arch/powerpc/boot/dts/redwood.dts
b/arch/powerpc/boot/dts/redwood.dts
index 81636c0..412d5f9 100644
--- a/arch/powerpc/boot/dts/redwood.dts
+++ b/arch/powerpc/boot/dts/redwood.dts
@@ -357,6 +357,21 @@
   0x0 0x0 0x0 0x3 UIC3 0xa 0x4 /*
swizzled int C */
   0x0 0x0 0x0 0x4 UIC3 0xb 0x4 /*
swizzled int D */;
   };
+  MSI: ppc4xx-...@40030 {
+  compatible = amcc,ppc4xx-msi, ppc4xx-msi;
+  reg =  0x4 0x0030 0x100
+  0x4 0x0030 0x100;
+  sdr-base = 0x3B0;
+  interrupts =0 1 2 3;
+  interrupt-parent = MSI;
+  #interrupt-cells = 1;
+  #address-cells = 0;
+  #size-cells = 0;
+  interrupt-map = 0 UIC0 0xC 1
+  1 UIC0 0x0D 1
+  2 UIC0 0x0E 1
+  3 UIC0 0x0F 1;
+  };
 
   };
 
diff --git a/arch/powerpc/sysdev/Kconfig b/arch/powerpc/sysdev/Kconfig
index 3965828..32f5a40 100644
--- a/arch/powerpc/sysdev/Kconfig
+++ b/arch/powerpc/sysdev/Kconfig
@@ -7,8 +7,15 @@ config PPC4xx_PCI_EXPRESS
   depends on PCI  4xx
   default n
 
+config 4xx_MSI

This should probably be named PPC4xx_MSI, similar to how
PPC4xx_PCI_EXPRESS is named.

+  bool
+  depends on PCI_MSI
+  depends on PCI  4xx
+  default n
+
 config PPC_MSI_BITMAP
   bool
   depends on PCI_MSI
   default y if MPIC
   default y if FSL_PCI
+  default y if 4xx_MSI

diff --git a/arch/powerpc/sysdev/ppc4xx_msi.c
b/arch/powerpc/sysdev/ppc4xx_msi.c
new file mode 100644
index 000..752da4b
--- /dev/null
+++ b/arch/powerpc/sysdev/ppc4xx_msi.c
@@ -0,0 +1,335 @@
+/*
+ * Copyright (C) 2009 Applied Micro Circuits corporation,
+ * All rights reserved.

Please don't add the 'All rights reserved.' to new files.  It is
inaccurate and confusing given that it's a GPLv2 file.

+ *
+ * Author: Feng Kan f...@amcc.com
+ *   Tirumala Marri tma...@amcc.com
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; version 2 of the
+ * License.
+ */
+#include linux/irq.h
+#include linux/bootmem.h
+#include linux/pci.h
+#include linux/msi.h
+#include linux/of_platform.h
+#include linux/interrupt.h
+#include linux/device.h
+#include asm/prom.h
+#include asm/hw_irq.h
+#include asm/ppc-pci.h
+#include asm/dcr.h
+#include asm/dcr-regs.h
+#include ppc4xx_msi.h
+
+
+static struct ppc4xx_msi *ppc4xx_msi;
+
+struct ppc4xx_msi_feature {
+  u32 ppc4xx_pic_ip;
+  u32 msiir_offset;
+};
+
+static int ppc4xx_msi_init_allocator(struct ppc4xx_msi *msi_data)
+{
+  int rc;
+
+  rc = msi_bitmap_alloc(msi_data-bitmap, NR_MSI_IRQS,
+  msi_data-irqhost-of_node);
+  if (rc)
+  return rc;
+  rc = msi_bitmap_reserve_dt_hwirqs(msi_data-bitmap);
+  if (rc  0) {
+  msi_bitmap_free(msi_data-bitmap);
+  return rc;
+  }
+  return 0;
+}
+
+
+static void ppc4xx_msi_cascade(unsigned int irq, struct irq_desc
*desc)
+{
+  unsigned int cascade_irq;
+  struct ppc4xx_msi *msi_data = ppc4xx_msi;
+  int msir_index = -1;
+
+  raw_spin_lock(desc-lock);
+  if (desc-chip-mask_ack) {
+  desc-chip-mask_ack(irq);
+  } else {
+  desc-chip-mask(irq);
+  desc-chip-ack(irq);
+  }
+
+  if (unlikely(desc-status