[PATCH 1/2] powerpc/44x: Fix PCI MSI support for Maui APM821xx SoC and Bluestone board

[PATCH 1/2] powerpc/44x: Fix PCI MSI support for Maui APM821xx SoC and Bluestone board

[Mai La]

This patch consists of:
- Enable PCI MSI as default for Bluestone board
- Define number of MSI interrupt for Maui APM821xx SoC using in Bluestone board
- Fix returning ENODEV as finding MSI node
- Fix MSI physical high and low address
- Keep MSI data logically

Signed-off-by: Mai La <mla@apm.com>
---
 arch/powerpc/platforms/44x/Kconfig |    2 ++
 arch/powerpc/sysdev/Kconfig        |    6 ++++++
 arch/powerpc/sysdev/ppc4xx_msi.c   |   27 ++++++++++++++++-----------
 3 files changed, 24 insertions(+), 11 deletions(-)

diff --git a/arch/powerpc/platforms/44x/Kconfig b/arch/powerpc/platforms/44x/Kconfig
index fcf6bf2..9f04ce3 100644
--- a/arch/powerpc/platforms/44x/Kconfig
+++ b/arch/powerpc/platforms/44x/Kconfig
@@ -23,6 +23,8 @@ config BLUESTONE
 	default n
 	select PPC44x_SIMPLE
 	select APM821xx
+	select PCI_MSI
+	select PPC4xx_MSI
 	select IBM_EMAC_RGMII
 	help
 	  This option enables support for the APM APM821xx Evaluation board.
diff --git a/arch/powerpc/sysdev/Kconfig b/arch/powerpc/sysdev/Kconfig
index 7b4df37..c86231e 100644
--- a/arch/powerpc/sysdev/Kconfig
+++ b/arch/powerpc/sysdev/Kconfig
@@ -20,6 +20,12 @@ config PPC_MSI_BITMAP
 	default y if FSL_PCI
 	default y if PPC4xx_MSI
 
+config PPC4xx_NR_MSI_IRQS
+	int "Number of PCI MSI interrupts"
+	depends on PCI_MSI && PPC4xx_MSI
+	default "4" if !APM821xx
+	default "8" if APM821xx
+
 source "arch/powerpc/sysdev/xics/Kconfig"
 
 config PPC_SCOM
diff --git a/arch/powerpc/sysdev/ppc4xx_msi.c b/arch/powerpc/sysdev/ppc4xx_msi.c
index 1c2d7af..3359237 100644
--- a/arch/powerpc/sysdev/ppc4xx_msi.c
+++ b/arch/powerpc/sysdev/ppc4xx_msi.c
@@ -28,10 +28,11 @@
 #include <linux/of_platform.h>
 #include <linux/interrupt.h>
 #include <linux/export.h>
+#include <linux/kernel.h>
 #include <asm/prom.h>
 #include <asm/hw_irq.h>
 #include <asm/ppc-pci.h>
-#include <boot/dcr.h>
+#include <asm/dcr.h>
 #include <asm/dcr-regs.h>
 #include <asm/msi_bitmap.h>
 
@@ -43,7 +44,8 @@
 #define PEIH_FLUSH0	0x30
 #define PEIH_FLUSH1	0x38
 #define PEIH_CNTRST	0x48
-#define NR_MSI_IRQS	4
+
+#define NR_MSI_IRQS CONFIG_PPC4xx_NR_MSI_IRQS
 
 struct ppc4xx_msi {
 	u32 msi_addr_lo;
@@ -150,12 +152,11 @@ static int ppc4xx_setup_pcieh_hw(struct platform_device *dev,
 	if (!sdr_addr)
 		return -1;
 
-	SDR0_WRITE(sdr_addr, (u64)res.start >> 32);	 /*HIGH addr */
-	SDR0_WRITE(sdr_addr + 1, res.start & 0xFFFFFFFF); /* Low addr */
-
+	mtdcri(SDR0, *sdr_addr, upper_32_bits(res.start));	/*HIGH addr */
+	mtdcri(SDR0, *sdr_addr + 1, lower_32_bits(res.start));	/* Low addr */
 
 	msi->msi_dev = of_find_node_by_name(NULL, "ppc4xx-msi");
-	if (msi->msi_dev)
+	if (!msi->msi_dev)
 		return -ENODEV;
 
 	msi->msi_regs = of_iomap(msi->msi_dev, 0);
@@ -167,9 +168,12 @@ static int ppc4xx_setup_pcieh_hw(struct platform_device *dev,
 		(u32) (msi->msi_regs + PEIH_TERMADH), (u32) (msi->msi_regs));
 
 	msi_virt = dma_alloc_coherent(&dev->dev, 64, &msi_phys, GFP_KERNEL);
-	msi->msi_addr_hi = 0x0;
-	msi->msi_addr_lo = (u32) msi_phys;
-	dev_dbg(&dev->dev, "PCIE-MSI: msi address 0x%x\n", msi->msi_addr_lo);
+	if (!msi_virt)
+		return -ENOMEM;
+	msi->msi_addr_hi = (u32)(msi_phys >> 32);
+	msi->msi_addr_lo = (u32)(msi_phys & 0xffffffff);
+	dev_dbg(&dev->dev, "PCIE-MSI: msi address high 0x%x, low 0x%x\n",
+		msi->msi_addr_hi, msi->msi_addr_lo);
 
 	/* Progam the Interrupt handler Termination addr registers */
 	out_be32(msi->msi_regs + PEIH_TERMADH, msi->msi_addr_hi);
@@ -185,6 +189,8 @@ static int ppc4xx_setup_pcieh_hw(struct platform_device *dev,
 	out_be32(msi->msi_regs + PEIH_MSIED, *msi_data);
 	out_be32(msi->msi_regs + PEIH_MSIMK, *msi_mask);
 
+	dma_free_coherent(&dev->dev, 64, msi_virt, msi_phys);
+
 	return 0;
 }
 
@@ -215,8 +221,6 @@ static int __devinit ppc4xx_msi_probe(struct platform_device *dev)
 	struct resource res;
 	int err = 0;
 
-	msi = &ppc4xx_msi;/*keep the msi data for further use*/
-
 	dev_dbg(&dev->dev, "PCIE-MSI: Setting up MSI support...\n");
 
 	msi = kzalloc(sizeof(struct ppc4xx_msi), GFP_KERNEL);
@@ -242,6 +246,7 @@ static int __devinit ppc4xx_msi_probe(struct platform_device *dev)
 		dev_err(&dev->dev, "Error allocating MSI bitmap\n");
 		goto error_out;
 	}
+	ppc4xx_msi = *msi;
 
 	ppc_md.setup_msi_irqs = ppc4xx_setup_msi_irqs;
 	ppc_md.teardown_msi_irqs = ppc4xx_teardown_msi_irqs;
-- 
1.7.3.4

Re: [PATCH 1/2] powerpc/44x: Fix PCI MSI support for Maui APM821xx SoC and Bluestone board

[Benjamin Herrenschmidt]

On Fri, 2012-03-09 at 10:17 +0700, Mai La wrote:
> This patch consists of:
> - Enable PCI MSI as default for Bluestone board
> - Define number of MSI interrupt for Maui APM821xx SoC using in Bluestone board
> - Fix returning ENODEV as finding MSI node
> - Fix MSI physical high and low address
> - Keep MSI data logically
> 
> Signed-off-by: Mai La <mla@apm.com>
> ---

> index 7b4df37..c86231e 100644
> --- a/arch/powerpc/sysdev/Kconfig
> +++ b/arch/powerpc/sysdev/Kconfig
> @@ -20,6 +20,12 @@ config PPC_MSI_BITMAP
>  	default y if FSL_PCI
>  	default y if PPC4xx_MSI
>  
> +config PPC4xx_NR_MSI_IRQS
> +	int "Number of PCI MSI interrupts"
> +	depends on PCI_MSI && PPC4xx_MSI
> +	default "4" if !APM821xx
> +	default "8" if APM821xx
> +

 .../...

> +#define NR_MSI_IRQS CONFIG_PPC4xx_NR_MSI_IRQS

ARGH.

We asked to -NOT MAKE THIS A COMPILE TIME OPTION-

CONFIG_foo is as bad as your previous ifdef, in fact you just added a
useless config option here.

Make that number dynamic. Count the entries in the device-tree (or add
a property with the number in it, whatever you fancy the most) but make
it something detected at RUNTIME !!!!!

Ben.

Re: [PATCH 1/2] powerpc/44x: Fix PCI MSI support for Maui APM821xx SoC and Bluestone board

[Josh Boyer]

On Thu, Mar 8, 2012 at 10:35 PM, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
> On Fri, 2012-03-09 at 10:17 +0700, Mai La wrote:
>> This patch consists of:
>> - Enable PCI MSI as default for Bluestone board
>> - Define number of MSI interrupt for Maui APM821xx SoC using in Blueston=
e board
>> - Fix returning ENODEV as finding MSI node
>> - Fix MSI physical high and low address
>> - Keep MSI data logically
>>
>> Signed-off-by: Mai La <mla@apm.com>
>> ---
>
>> index 7b4df37..c86231e 100644
>> --- a/arch/powerpc/sysdev/Kconfig
>> +++ b/arch/powerpc/sysdev/Kconfig
>> @@ -20,6 +20,12 @@ config PPC_MSI_BITMAP
>> =A0 =A0 =A0 default y if FSL_PCI
>> =A0 =A0 =A0 default y if PPC4xx_MSI
>>
>> +config PPC4xx_NR_MSI_IRQS
>> + =A0 =A0 int "Number of PCI MSI interrupts"
>> + =A0 =A0 depends on PCI_MSI && PPC4xx_MSI
>> + =A0 =A0 default "4" if !APM821xx
>> + =A0 =A0 default "8" if APM821xx
>> +
>
> =A0.../...
>
>> +#define NR_MSI_IRQS CONFIG_PPC4xx_NR_MSI_IRQS
>
> ARGH.
>
> We asked to -NOT MAKE THIS A COMPILE TIME OPTION-

Actually, I asked for basically exactly above.  I was _wrong_ in asking,
but it was asked for.

> CONFIG_foo is as bad as your previous ifdef, in fact you just added a
> useless config option here.
>
> Make that number dynamic. Count the entries in the device-tree (or add
> a property with the number in it, whatever you fancy the most) but make
> it something detected at RUNTIME !!!!!

My apologies Mai.  Ben is correct here and I should have thought more
about my suggestion before I made it.

josh

Re: [PATCH 1/2] powerpc/44x: Fix PCI MSI support for Maui APM821xx SoC and Bluestone board

[Wolfgang Denk]

Dear Mai La,

In message <1331524918-22515-1-git-send-email-mla@apm.com> you wrote:
> This patch consists of:
> - Enable PCI MSI as default for Bluestone board
> - Change definition of number of MSI interrupt as it depends on SoC
> - Fix returning ENODEV as finding MSI node
> - Fix MSI physical high and low address
> - Keep MSI data logically
> 
> Signed-off-by: Mai La <mla@apm.com>

This is an updated version of your patch of March 09, right -
http://article.gmane.org/gmane.linux.kernel/1264255  ??

If so, you should mark ot as patch V2 in the Subject:, and add a
description of what you changed.

> -	SDR0_WRITE(sdr_addr, (u64)res.start >> 32);	 /*HIGH addr */
> -	SDR0_WRITE(sdr_addr + 1, res.start & 0xFFFFFFFF); /* Low addr */
> -
> +	mtdcri(SDR0, *sdr_addr, upper_32_bits(res.start));	/*HIGH addr */
> +	mtdcri(SDR0, *sdr_addr + 1, lower_32_bits(res.start));	/* Low addr */
...
> +	msi->msi_addr_hi = (u32)(msi_phys >> 32);
> +	msi->msi_addr_lo = (u32)(msi_phys & 0xffffffff);

Is there any reason for not using upper_32_bits() / lower_32_bits()
consistently?

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de
Real computer scientists despise the idea of actual  hardware.  Hard-
ware has limitations, software doesn't. It's a real shame that Turing
machines are so poor at I/O.

[PATCH 1/2] powerpc/44x: Fix PCI MSI support for Maui APM821xx SoC and Bluestone board

[Mai La]

This patch consists of:
- Enable PCI MSI as default for Bluestone board
- Change definition of number of MSI interrupt as it depends on SoC
- Fix returning ENODEV as finding MSI node
- Fix MSI physical high and low address
- Keep MSI data logically

Signed-off-by: Mai La <mla@apm.com>
---
 arch/powerpc/platforms/44x/Kconfig |    2 +
 arch/powerpc/sysdev/ppc4xx_msi.c   |   40 +++++++++++++++++++++++------------
 2 files changed, 28 insertions(+), 14 deletions(-)

diff --git a/arch/powerpc/platforms/44x/Kconfig b/arch/powerpc/platforms/44x/Kconfig
index fcf6bf2..9f04ce3 100644
--- a/arch/powerpc/platforms/44x/Kconfig
+++ b/arch/powerpc/platforms/44x/Kconfig
@@ -23,6 +23,8 @@ config BLUESTONE
 	default n
 	select PPC44x_SIMPLE
 	select APM821xx
+	select PCI_MSI
+	select PPC4xx_MSI
 	select IBM_EMAC_RGMII
 	help
 	  This option enables support for the APM APM821xx Evaluation board.
diff --git a/arch/powerpc/sysdev/ppc4xx_msi.c b/arch/powerpc/sysdev/ppc4xx_msi.c
index 1c2d7af..63989d0 100644
--- a/arch/powerpc/sysdev/ppc4xx_msi.c
+++ b/arch/powerpc/sysdev/ppc4xx_msi.c
@@ -28,10 +28,11 @@
 #include <linux/of_platform.h>
 #include <linux/interrupt.h>
 #include <linux/export.h>
+#include <linux/kernel.h>
 #include <asm/prom.h>
 #include <asm/hw_irq.h>
 #include <asm/ppc-pci.h>
-#include <boot/dcr.h>
+#include <asm/dcr.h>
 #include <asm/dcr-regs.h>
 #include <asm/msi_bitmap.h>
 
@@ -43,13 +44,14 @@
 #define PEIH_FLUSH0	0x30
 #define PEIH_FLUSH1	0x38
 #define PEIH_CNTRST	0x48
-#define NR_MSI_IRQS	4
+
+int msi_irqs;
 
 struct ppc4xx_msi {
 	u32 msi_addr_lo;
 	u32 msi_addr_hi;
 	void __iomem *msi_regs;
-	int msi_virqs[NR_MSI_IRQS];
+	int *msi_virqs;
 	struct msi_bitmap bitmap;
 	struct device_node *msi_dev;
 };
@@ -61,7 +63,7 @@ static int ppc4xx_msi_init_allocator(struct platform_device *dev,
 {
 	int err;
 
-	err = msi_bitmap_alloc(&msi_data->bitmap, NR_MSI_IRQS,
+	err = msi_bitmap_alloc(&msi_data->bitmap, msi_irqs,
 			      dev->dev.of_node);
 	if (err)
 		return err;
@@ -83,6 +85,9 @@ static int ppc4xx_setup_msi_irqs(struct pci_dev *dev, int nvec, int type)
 	struct msi_desc *entry;
 	struct ppc4xx_msi *msi_data = &ppc4xx_msi;
 
+	msi_data->msi_virqs = kmalloc((msi_irqs) * sizeof(int),
+					    GFP_KERNEL);
+
 	list_for_each_entry(entry, &dev->msi_list, list) {
 		int_no = msi_bitmap_alloc_hwirqs(&msi_data->bitmap, 1);
 		if (int_no >= 0)
@@ -150,12 +155,11 @@ static int ppc4xx_setup_pcieh_hw(struct platform_device *dev,
 	if (!sdr_addr)
 		return -1;
 
-	SDR0_WRITE(sdr_addr, (u64)res.start >> 32);	 /*HIGH addr */
-	SDR0_WRITE(sdr_addr + 1, res.start & 0xFFFFFFFF); /* Low addr */
-
+	mtdcri(SDR0, *sdr_addr, upper_32_bits(res.start));	/*HIGH addr */
+	mtdcri(SDR0, *sdr_addr + 1, lower_32_bits(res.start));	/* Low addr */
 
 	msi->msi_dev = of_find_node_by_name(NULL, "ppc4xx-msi");
-	if (msi->msi_dev)
+	if (!msi->msi_dev)
 		return -ENODEV;
 
 	msi->msi_regs = of_iomap(msi->msi_dev, 0);
@@ -167,9 +171,12 @@ static int ppc4xx_setup_pcieh_hw(struct platform_device *dev,
 		(u32) (msi->msi_regs + PEIH_TERMADH), (u32) (msi->msi_regs));
 
 	msi_virt = dma_alloc_coherent(&dev->dev, 64, &msi_phys, GFP_KERNEL);
-	msi->msi_addr_hi = 0x0;
-	msi->msi_addr_lo = (u32) msi_phys;
-	dev_dbg(&dev->dev, "PCIE-MSI: msi address 0x%x\n", msi->msi_addr_lo);
+	if (!msi_virt)
+		return -ENOMEM;
+	msi->msi_addr_hi = (u32)(msi_phys >> 32);
+	msi->msi_addr_lo = (u32)(msi_phys & 0xffffffff);
+	dev_dbg(&dev->dev, "PCIE-MSI: msi address high 0x%x, low 0x%x\n",
+		msi->msi_addr_hi, msi->msi_addr_lo);
 
 	/* Progam the Interrupt handler Termination addr registers */
 	out_be32(msi->msi_regs + PEIH_TERMADH, msi->msi_addr_hi);
@@ -185,6 +192,8 @@ static int ppc4xx_setup_pcieh_hw(struct platform_device *dev,
 	out_be32(msi->msi_regs + PEIH_MSIED, *msi_data);
 	out_be32(msi->msi_regs + PEIH_MSIMK, *msi_mask);
 
+	dma_free_coherent(&dev->dev, 64, msi_virt, msi_phys);
+
 	return 0;
 }
 
@@ -194,7 +203,7 @@ static int ppc4xx_of_msi_remove(struct platform_device *dev)
 	int i;
 	int virq;
 
-	for (i = 0; i < NR_MSI_IRQS; i++) {
+	for (i = 0; i < msi_irqs; i++) {
 		virq = msi->msi_virqs[i];
 		if (virq != NO_IRQ)
 			irq_dispose_mapping(virq);
@@ -215,8 +224,6 @@ static int __devinit ppc4xx_msi_probe(struct platform_device *dev)
 	struct resource res;
 	int err = 0;
 
-	msi = &ppc4xx_msi;/*keep the msi data for further use*/
-
 	dev_dbg(&dev->dev, "PCIE-MSI: Setting up MSI support...\n");
 
 	msi = kzalloc(sizeof(struct ppc4xx_msi), GFP_KERNEL);
@@ -234,6 +241,10 @@ static int __devinit ppc4xx_msi_probe(struct platform_device *dev)
 		goto error_out;
 	}
 
+	msi_irqs = of_irq_count(dev->dev.of_node);
+	if (!msi_irqs)
+		return -1;
+
 	if (ppc4xx_setup_pcieh_hw(dev, res, msi))
 		goto error_out;
 
@@ -242,6 +253,7 @@ static int __devinit ppc4xx_msi_probe(struct platform_device *dev)
 		dev_err(&dev->dev, "Error allocating MSI bitmap\n");
 		goto error_out;
 	}
+	ppc4xx_msi = *msi;
 
 	ppc_md.setup_msi_irqs = ppc4xx_setup_msi_irqs;
 	ppc_md.teardown_msi_irqs = ppc4xx_teardown_msi_irqs;
-- 
1.7.3.4