[PATCH 1/4] fsl_msi: fix the conflict of virt_msir's chip_data

[PATCH 1/4] fsl_msi: fix the conflict of virt_msir's chip_data

[Li Yang]

From: Zhao Chenhui <b26998@freescale.com>

In fsl_of_msi_probe(), the virt_msir's chip_data have been stored
the pointer to struct mpic. We add a struct fsl_msi_cascade_data
to store the pointer to struct fsl_msi and msir_index.  Otherwise,
the pointer to struct mpic will be over-written, and will cause
problem when calling eoi() of the irq.

Signed-off-by: Zhao Chenhui <b26998@freescale.com>
Signed-off-by: Li Yang <leoli@freescale.com>
---
 arch/powerpc/sysdev/fsl_msi.c |   30 +++++++++++++++++++++++++-----
 1 files changed, 25 insertions(+), 5 deletions(-)

diff --git a/arch/powerpc/sysdev/fsl_msi.c b/arch/powerpc/sysdev/fsl_msi.c
index ad453ca..716862f 100644
--- a/arch/powerpc/sysdev/fsl_msi.c
+++ b/arch/powerpc/sysdev/fsl_msi.c
@@ -21,6 +21,7 @@
 #include <asm/prom.h>
 #include <asm/hw_irq.h>
 #include <asm/ppc-pci.h>
+#include <asm/mpic.h>
 #include "fsl_msi.h"
 
 struct fsl_msi_feature {
@@ -28,6 +29,10 @@ struct fsl_msi_feature {
 	u32 msiir_offset;
 };
 
+struct fsl_msi_cascade_data {
+	struct fsl_msi *data;
+	int index;
+};
 
 static inline u32 fsl_msi_read(u32 __iomem *base, unsigned int reg)
 {
@@ -132,7 +137,7 @@ static void fsl_compose_msi_msg(struct pci_dev *pdev, int hwirq,
 
 static int fsl_setup_msi_irqs(struct pci_dev *pdev, int nvec, int type)
 {
-	int rc, hwirq;
+	int rc, hwirq = NO_IRQ;
 	unsigned int virq;
 	struct msi_desc *entry;
 	struct msi_msg msg;
@@ -172,11 +177,15 @@ out_free:
 static void fsl_msi_cascade(unsigned int irq, struct irq_desc *desc)
 {
 	unsigned int cascade_irq;
-	struct fsl_msi *msi_data = get_irq_chip_data(irq);
+	struct fsl_msi *msi_data;
 	int msir_index = -1;
 	u32 msir_value = 0;
 	u32 intr_index;
 	u32 have_shift = 0;
+	struct fsl_msi_cascade_data *cascade_data;
+
+	cascade_data = (struct fsl_msi_cascade_data *)get_irq_data(irq);
+	msi_data = cascade_data->data;
 
 	spin_lock(&desc->lock);
 	if ((msi_data->feature &  FSL_PIC_IP_MASK) == FSL_PIC_IP_IPIC) {
@@ -191,7 +200,7 @@ static void fsl_msi_cascade(unsigned int irq, struct irq_desc *desc)
 	if (unlikely(desc->status & IRQ_INPROGRESS))
 		goto unlock;
 
-	msir_index = (int)desc->handler_data;
+	msir_index = cascade_data->index;
 
 	if (msir_index >= NR_MSI_REG)
 		cascade_irq = NO_IRQ;
@@ -243,6 +252,7 @@ static int __devinit fsl_of_msi_probe(struct of_device *dev,
 	int virt_msir;
 	const u32 *p;
 	struct fsl_msi_feature *features = match->data;
+	struct fsl_msi_cascade_data *cascade_data = NULL;
 
 	printk(KERN_DEBUG "Setting up Freescale MSI support\n");
 
@@ -309,9 +319,19 @@ static int __devinit fsl_of_msi_probe(struct of_device *dev,
 			break;
 		virt_msir = irq_of_parse_and_map(dev->node, i);
 		if (virt_msir != NO_IRQ) {
-			set_irq_data(virt_msir, (void *)i);
+			cascade_data = kzalloc(
+					sizeof(struct fsl_msi_cascade_data),
+					GFP_KERNEL);
+			if (!cascade_data) {
+				dev_err(&dev->dev,
+					"No memory for MSI cascade data\n");
+				err = -ENOMEM;
+				goto error_out;
+			}
+			cascade_data->index = i;
+			cascade_data->data = msi;
+			set_irq_data(virt_msir, (void *)cascade_data);
 			set_irq_chained_handler(virt_msir, fsl_msi_cascade);
-			set_irq_chip_data(virt_msir, msi);
 		}
 	}
 
-- 
1.6.6-rc1.GIT

[PATCH 2/4] fsl_msi: enable msi allocation in all banks

[Li Yang]

From: Zhao Chenhui <b26998@freescale.com>

Put all fsl_msi banks in a linked list. The list of banks then can be
traversed when allocating new msi interrupts.

Signed-off-by: Zhao Chenhui <b26998@freescale.com>
Signed-off-by: Li Yang <leoli@freescale.com>
---
 arch/powerpc/sysdev/fsl_msi.c |   29 ++++++++++++++++++++++-------
 arch/powerpc/sysdev/fsl_msi.h |    2 ++
 2 files changed, 24 insertions(+), 7 deletions(-)

diff --git a/arch/powerpc/sysdev/fsl_msi.c b/arch/powerpc/sysdev/fsl_msi.c
index 716862f..c46db75 100644
--- a/arch/powerpc/sysdev/fsl_msi.c
+++ b/arch/powerpc/sysdev/fsl_msi.c
@@ -24,6 +24,8 @@
 #include <asm/mpic.h>
 #include "fsl_msi.h"
 
+LIST_HEAD(msi_head);
+
 struct fsl_msi_feature {
 	u32 fsl_pic_ip;
 	u32 msiir_offset;
@@ -143,15 +145,26 @@ static int fsl_setup_msi_irqs(struct pci_dev *pdev, int nvec, int type)
 	struct msi_msg msg;
 	struct fsl_msi *msi_data;
 
+	if (list_empty(&msi_head)) {
+		pr_debug("%s: msi init error\n", __func__);
+		rc = -EFAULT;
+		goto out_free;
+	}
+
 	list_for_each_entry(entry, &pdev->msi_list, list) {
-		msi_data = get_irq_chip_data(entry->irq);
 
-		hwirq = msi_bitmap_alloc_hwirqs(&msi_data->bitmap, 1);
-		if (hwirq < 0) {
-			rc = hwirq;
-			pr_debug("%s: fail allocating msi interrupt\n",
-					__func__);
-			goto out_free;
+		list_for_each_entry(msi_data, &msi_head, list) {
+
+			hwirq = msi_bitmap_alloc_hwirqs(&msi_data->bitmap, 1);
+			if (hwirq >= 0)
+				break;
+
+			if (list_is_last(&msi_data->list, &msi_head)) {
+				rc = hwirq;
+				pr_debug("%s: fail allocating msi interrupt\n",
+						__func__);
+				goto out_free;
+			}
 		}
 
 		virq = irq_create_mapping(msi_data->irqhost, hwirq);
@@ -335,6 +348,8 @@ static int __devinit fsl_of_msi_probe(struct of_device *dev,
 		}
 	}
 
+	list_add_tail(&msi->list, &msi_head);
+
 	/* The multiple setting ppc_md.setup_msi_irqs will not harm things */
 	if (!ppc_md.setup_msi_irqs) {
 		ppc_md.setup_msi_irqs = fsl_setup_msi_irqs;
diff --git a/arch/powerpc/sysdev/fsl_msi.h b/arch/powerpc/sysdev/fsl_msi.h
index 331c7e7..8fc5523 100644
--- a/arch/powerpc/sysdev/fsl_msi.h
+++ b/arch/powerpc/sysdev/fsl_msi.h
@@ -34,6 +34,8 @@ struct fsl_msi {
 	u32 feature;
 
 	struct msi_bitmap bitmap;
+
+	struct list_head list;          /* support multiple MSI banks */
 };
 
 #endif /* _POWERPC_SYSDEV_FSL_MSI_H */
-- 
1.6.6-rc1.GIT

[PATCH 3/4] fsl_msi: enable msi sharing through AMP OSes

[Li Yang]

From: Zhao Chenhui <b26998@freescale.com>

Make a single PCIe MSI bank shareable through CAMP OSes. The number of
MSI used by each core can be configured by dts file.

Signed-off-by: Zhao Chenhui <b26998@freescale.com>
Signed-off-by: Li Yang <leoli@freescale.com>
---
 arch/powerpc/sysdev/fsl_msi.c    |    8 +++++++-
 arch/powerpc/sysdev/msi_bitmap.c |    4 ++--
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/sysdev/fsl_msi.c b/arch/powerpc/sysdev/fsl_msi.c
index c46db75..ec5bdb4 100644
--- a/arch/powerpc/sysdev/fsl_msi.c
+++ b/arch/powerpc/sysdev/fsl_msi.c
@@ -266,6 +266,8 @@ static int __devinit fsl_of_msi_probe(struct of_device *dev,
 	const u32 *p;
 	struct fsl_msi_feature *features = match->data;
 	struct fsl_msi_cascade_data *cascade_data = NULL;
+	int len;
+	u32 offset;
 
 	printk(KERN_DEBUG "Setting up Freescale MSI support\n");
 
@@ -325,6 +327,10 @@ static int __devinit fsl_of_msi_probe(struct of_device *dev,
 		err = -EINVAL;
 		goto error_out;
 	}
+	offset = 0;
+	p = of_get_property(dev->node, "msi-available-ranges", &len);
+	if (p)
+		offset = *p / IRQS_PER_MSI_REG;
 
 	count /= sizeof(u32);
 	for (i = 0; i < count / 2; i++) {
@@ -341,7 +347,7 @@ static int __devinit fsl_of_msi_probe(struct of_device *dev,
 				err = -ENOMEM;
 				goto error_out;
 			}
-			cascade_data->index = i;
+			cascade_data->index = i + offset;
 			cascade_data->data = msi;
 			set_irq_data(virt_msir, (void *)cascade_data);
 			set_irq_chained_handler(virt_msir, fsl_msi_cascade);
diff --git a/arch/powerpc/sysdev/msi_bitmap.c b/arch/powerpc/sysdev/msi_bitmap.c
index 5a32cbe..b41db96 100644
--- a/arch/powerpc/sysdev/msi_bitmap.c
+++ b/arch/powerpc/sysdev/msi_bitmap.c
@@ -95,8 +95,8 @@ int msi_bitmap_reserve_dt_hwirqs(struct msi_bitmap *bmp)
 	/* Format is: (<u32 start> <u32 count>)+ */
 	len /= 2 * sizeof(u32);
 	for (i = 0; i < len; i++, p += 2) {
-		for (j = 0; j < *(p + 1); j++)
-			bitmap_release_region(bmp->bitmap, *p + j, 0);
+		for (j = *p; j < *(p + 1); j++)
+			bitmap_release_region(bmp->bitmap, j, 0);
 	}
 
 	spin_unlock(&bmp->lock);
-- 
1.6.6-rc1.GIT

[PATCH 4/4] mpc8572ds: change camp dtses for MSI sharing

[Li Yang]

Enable the sharing of MSI interrupt through AMP OSes in the mpc8572ds
dtses.

Signed-off-by: Zhao Chenhui <b26998@freescale.com>
Signed-off-by: Li Yang <leoli@freescale.com>
---
 arch/powerpc/boot/dts/mpc8572ds_camp_core0.dts |   15 +++++++++++++--
 arch/powerpc/boot/dts/mpc8572ds_camp_core1.dts |    7 ++-----
 2 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/arch/powerpc/boot/dts/mpc8572ds_camp_core0.dts b/arch/powerpc/boot/dts/mpc8572ds_camp_core0.dts
index 5bd1011..3375c2a 100644
--- a/arch/powerpc/boot/dts/mpc8572ds_camp_core0.dts
+++ b/arch/powerpc/boot/dts/mpc8572ds_camp_core0.dts
@@ -215,6 +215,18 @@
 			clock-frequency = <0>;
 		};
 
+		msi@41600 {
+			compatible = "fsl,mpc8572-msi", "fsl,mpic-msi";
+			reg = <0x41600 0x80>;
+			msi-available-ranges = <0 0x80>;
+			interrupts = <
+				0xe0 0
+				0xe1 0
+				0xe2 0
+				0xe3 0>;
+			interrupt-parent = <&mpic>;
+		};
+
 		global-utilities@e0000 {	//global utilities block
 			compatible = "fsl,mpc8572-guts";
 			reg = <0xe0000 0x1000>;
@@ -243,8 +255,7 @@
 			protected-sources = <
 			31 32 33 37 38 39       /* enet2 enet3 */
 			76 77 78 79 26 42	/* dma2 pci2 serial*/
-			0xe0 0xe1 0xe2 0xe3     /* msi */
-			0xe4 0xe5 0xe6 0xe7
+			0xe4 0xe5 0xe6 0xe7	/* msi */
 			>;
 		};
 	};
diff --git a/arch/powerpc/boot/dts/mpc8572ds_camp_core1.dts b/arch/powerpc/boot/dts/mpc8572ds_camp_core1.dts
index 0efc345..3f0e8bd 100644
--- a/arch/powerpc/boot/dts/mpc8572ds_camp_core1.dts
+++ b/arch/powerpc/boot/dts/mpc8572ds_camp_core1.dts
@@ -154,12 +154,8 @@
 		msi@41600 {
 			compatible = "fsl,mpc8572-msi", "fsl,mpic-msi";
 			reg = <0x41600 0x80>;
-			msi-available-ranges = <0 0x100>;
+			msi-available-ranges = <0x80 0x100>;
 			interrupts = <
-				0xe0 0
-				0xe1 0
-				0xe2 0
-				0xe3 0
 				0xe4 0
 				0xe5 0
 				0xe6 0
@@ -190,6 +186,7 @@
 			0x1 0x2 0x3 0x4         /* pci slot */
 			0x9 0xa 0xb 0xc         /* usb */
 			0x6 0x7 0xe 0x5         /* Audio elgacy SATA */
+			0xe0 0xe1 0xe2 0xe3	/* msi */
 			>;
 		};
 	};
-- 
1.6.6-rc1.GIT

Re: [PATCH 3/4] fsl_msi: enable msi sharing through AMP OSes

[Michael Ellerman]

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

On Fri, 2010-04-16 at 15:34 +0800, Li Yang wrote:
> From: Zhao Chenhui <b26998@freescale.com>
> 
> Make a single PCIe MSI bank shareable through CAMP OSes. The number of
> MSI used by each core can be configured by dts file.
> 
> Signed-off-by: Zhao Chenhui <b26998@freescale.com>
> Signed-off-by: Li Yang <leoli@freescale.com>
> ---
>  arch/powerpc/sysdev/fsl_msi.c    |    8 +++++++-
>  arch/powerpc/sysdev/msi_bitmap.c |    4 ++--
>  2 files changed, 9 insertions(+), 3 deletions(-)
..
> diff --git a/arch/powerpc/sysdev/msi_bitmap.c b/arch/powerpc/sysdev/msi_bitmap.c
> index 5a32cbe..b41db96 100644
> --- a/arch/powerpc/sysdev/msi_bitmap.c
> +++ b/arch/powerpc/sysdev/msi_bitmap.c
> @@ -95,8 +95,8 @@ int msi_bitmap_reserve_dt_hwirqs(struct msi_bitmap *bmp)
>  	/* Format is: (<u32 start> <u32 count>)+ */
>  	len /= 2 * sizeof(u32);
>  	for (i = 0; i < len; i++, p += 2) {
> -		for (j = 0; j < *(p + 1); j++)
> -			bitmap_release_region(bmp->bitmap, *p + j, 0);
> +		for (j = *p; j < *(p + 1); j++)
> +			bitmap_release_region(bmp->bitmap, j, 0);
>  	}
>  
>  	spin_unlock(&bmp->lock);

This looks like a bug fix to the msi bitmap code, is it not? Please
split it into a separate commit with an explanatory changelog.

cheers



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH 1/4] fsl_msi: fix the conflict of virt_msir's chip_data

[Michael Ellerman]

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

On Fri, 2010-04-16 at 15:34 +0800, Li Yang wrote:
> From: Zhao Chenhui <b26998@freescale.com>
> 
> In fsl_of_msi_probe(), the virt_msir's chip_data have been stored
> the pointer to struct mpic. We add a struct fsl_msi_cascade_data
> to store the pointer to struct fsl_msi and msir_index.  Otherwise,
> the pointer to struct mpic will be over-written, and will cause
> problem when calling eoi() of the irq.

I don't quite understand. Do you mean someone was overwriting
handler_data somewhere?
 
> @@ -309,9 +319,19 @@ static int __devinit fsl_of_msi_probe(struct of_device *dev,
>  			break;
>  		virt_msir = irq_of_parse_and_map(dev->node, i);
>  		if (virt_msir != NO_IRQ) {
> -			set_irq_data(virt_msir, (void *)i);
> +			cascade_data = kzalloc(
> +					sizeof(struct fsl_msi_cascade_data),
> +					GFP_KERNEL);
> +			if (!cascade_data) {
> +				dev_err(&dev->dev,
> +					"No memory for MSI cascade data\n");
> +				err = -ENOMEM;
> +				goto error_out;

The error handling in this routine is not great, most of the setup is
not torn down properly in the error paths AFAICS, this adds another.

> +			}
> +			cascade_data->index = i;
> +			cascade_data->data = msi;
> +			set_irq_data(virt_msir, (void *)cascade_data);
>  			set_irq_chained_handler(virt_msir, fsl_msi_cascade);
> -			set_irq_chip_data(virt_msir, msi);
>  		}
>  	}
>  

cheers

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH 2/4] fsl_msi: enable msi allocation in all banks

[Michael Ellerman]

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

On Fri, 2010-04-16 at 15:34 +0800, Li Yang wrote:
> From: Zhao Chenhui <b26998@freescale.com>
> 
> Put all fsl_msi banks in a linked list. The list of banks then can be
> traversed when allocating new msi interrupts.

So there are multiple banks, and you just use the first one that has an
empty slot in it's bitmap?

> Signed-off-by: Zhao Chenhui <b26998@freescale.com>
> Signed-off-by: Li Yang <leoli@freescale.com>
> ---
>  arch/powerpc/sysdev/fsl_msi.c |   29 ++++++++++++++++++++++-------
>  arch/powerpc/sysdev/fsl_msi.h |    2 ++
>  2 files changed, 24 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/powerpc/sysdev/fsl_msi.c b/arch/powerpc/sysdev/fsl_msi.c
> index 716862f..c46db75 100644
> --- a/arch/powerpc/sysdev/fsl_msi.c
> +++ b/arch/powerpc/sysdev/fsl_msi.c
> @@ -24,6 +24,8 @@
>  #include <asm/mpic.h>
>  #include "fsl_msi.h"
>  
> +LIST_HEAD(msi_head);
> +
>  struct fsl_msi_feature {
>  	u32 fsl_pic_ip;
>  	u32 msiir_offset;
> @@ -143,15 +145,26 @@ static int fsl_setup_msi_irqs(struct pci_dev *pdev, int nvec, int type)
>  	struct msi_msg msg;
>  	struct fsl_msi *msi_data;
>  
> +	if (list_empty(&msi_head)) {
> +		pr_debug("%s: msi init error\n", __func__);
> +		rc = -EFAULT;
> +		goto out_free;
> +	}

If there's an error probing then fsl_setup_msi_irqs() should not be
installed as ppc_md.setup_msi_irqs(), and this code should never run.
ie. You shouldn't need this check.

> +
>  	list_for_each_entry(entry, &pdev->msi_list, list) {
> -		msi_data = get_irq_chip_data(entry->irq);
>  
> -		hwirq = msi_bitmap_alloc_hwirqs(&msi_data->bitmap, 1);
> -		if (hwirq < 0) {
> -			rc = hwirq;
> -			pr_debug("%s: fail allocating msi interrupt\n",
> -					__func__);
> -			goto out_free;
> +		list_for_each_entry(msi_data, &msi_head, list) {
> +
> +			hwirq = msi_bitmap_alloc_hwirqs(&msi_data->bitmap, 1);
> +			if (hwirq >= 0)
> +				break;
> +
> +			if (list_is_last(&msi_data->list, &msi_head)) {
> +				rc = hwirq;
> +				pr_debug("%s: fail allocating msi interrupt\n",
> +						__func__);
> +				goto out_free;
> +			}

You could make this cleaner by pulling the inner loop into a separate
function, and when you fall off the end of the list you return < 0. That
would avoid needing the list_is_last() check.

cheers

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH 1/4] fsl_msi: fix the conflict of virt_msir's chip_data

[Li Yang]

On 4/19/2010 10:40 AM, Michael Ellerman wrote:
> On Fri, 2010-04-16 at 15:34 +0800, Li Yang wrote:
>    
>> From: Zhao Chenhui<b26998@freescale.com>
>>
>> In fsl_of_msi_probe(), the virt_msir's chip_data have been stored
>> the pointer to struct mpic. We add a struct fsl_msi_cascade_data
>> to store the pointer to struct fsl_msi and msir_index.  Otherwise,
>> the pointer to struct mpic will be over-written, and will cause
>> problem when calling eoi() of the irq.
>>      
> I don't quite understand. Do you mean someone was overwriting
> handler_data somewhere?
>    

The patch at http://patchwork.ozlabs.org/patch/48794/ was overwriting 
the chip_data.  We move the newly added pointer to fsl_msi structure to 
the handler data.

>
>    
>> @@ -309,9 +319,19 @@ static int __devinit fsl_of_msi_probe(struct of_device *dev,
>>   			break;
>>   		virt_msir = irq_of_parse_and_map(dev->node, i);
>>   		if (virt_msir != NO_IRQ) {
>> -			set_irq_data(virt_msir, (void *)i);
>> +			cascade_data = kzalloc(
>> +					sizeof(struct fsl_msi_cascade_data),
>> +					GFP_KERNEL);
>> +			if (!cascade_data) {
>> +				dev_err(&dev->dev,
>> +					"No memory for MSI cascade data\n");
>> +				err = -ENOMEM;
>> +				goto error_out;
>>      
> The error handling in this routine is not great, most of the setup is
> not torn down properly in the error paths AFAICS, this adds another.
>    

You are right.  Need to add a separate patch to fix all these.

Thanks.
- Leo

Re: [PATCH 3/4] fsl_msi: enable msi sharing through AMP OSes

[Li Yang]

On Mon, Apr 19, 2010 at 10:34 AM, Michael Ellerman
<michael@ellerman.id.au> wrote:
> On Fri, 2010-04-16 at 15:34 +0800, Li Yang wrote:
>> From: Zhao Chenhui <b26998@freescale.com>
>>
>> Make a single PCIe MSI bank shareable through CAMP OSes. The number of
>> MSI used by each core can be configured by dts file.
>>
>> Signed-off-by: Zhao Chenhui <b26998@freescale.com>
>> Signed-off-by: Li Yang <leoli@freescale.com>
>> ---
>> =C2=A0arch/powerpc/sysdev/fsl_msi.c =C2=A0 =C2=A0| =C2=A0 =C2=A08 ++++++=
+-
>> =C2=A0arch/powerpc/sysdev/msi_bitmap.c | =C2=A0 =C2=A04 ++--
>> =C2=A02 files changed, 9 insertions(+), 3 deletions(-)
> ..
>> diff --git a/arch/powerpc/sysdev/msi_bitmap.c b/arch/powerpc/sysdev/msi_=
bitmap.c
>> index 5a32cbe..b41db96 100644
>> --- a/arch/powerpc/sysdev/msi_bitmap.c
>> +++ b/arch/powerpc/sysdev/msi_bitmap.c
>> @@ -95,8 +95,8 @@ int msi_bitmap_reserve_dt_hwirqs(struct msi_bitmap *bm=
p)
>> =C2=A0 =C2=A0 =C2=A0 /* Format is: (<u32 start> <u32 count>)+ */
>> =C2=A0 =C2=A0 =C2=A0 len /=3D 2 * sizeof(u32);
>> =C2=A0 =C2=A0 =C2=A0 for (i =3D 0; i < len; i++, p +=3D 2) {
>> - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 for (j =3D 0; j < *(p + 1); =
j++)
>> - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
bitmap_release_region(bmp->bitmap, *p + j, 0);
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 for (j =3D *p; j < *(p + 1);=
 j++)
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
bitmap_release_region(bmp->bitmap, j, 0);
>> =C2=A0 =C2=A0 =C2=A0 }
>>
>> =C2=A0 =C2=A0 =C2=A0 spin_unlock(&bmp->lock);
>
> This looks like a bug fix to the msi bitmap code, is it not? Please
> split it into a separate commit with an explanatory changelog.

Oops.  We thought the property to be {start end} by mistake.  The
change shouldn't be needed.

Thanks,
Leo

Re: [PATCH 2/4] fsl_msi: enable msi allocation in all banks

[Li Yang]

On Mon, Apr 19, 2010 at 10:46 AM, Michael Ellerman
<michael@ellerman.id.au> wrote:
> On Fri, 2010-04-16 at 15:34 +0800, Li Yang wrote:
>> From: Zhao Chenhui <b26998@freescale.com>
>>
>> Put all fsl_msi banks in a linked list. The list of banks then can be
>> traversed when allocating new msi interrupts.
>
> So there are multiple banks, and you just use the first one that has an
> empty slot in it's bitmap?

Yes, currently we are.  What allocation algorithm do you think is
better?  I don't think spreading the allocation evenly should have any
performance benefit.  The point of multiple banks should be better
insulation through multiple OS's, IMHO.

- Leo

Re: [PATCH 1/4] fsl_msi: fix the conflict of virt_msir's chip_data

[Kumar Gala]

On Apr 18, 2010, at 11:50 PM, Li Yang wrote:

> On 4/19/2010 10:40 AM, Michael Ellerman wrote:
>> On Fri, 2010-04-16 at 15:34 +0800, Li Yang wrote:
>>  =20
>>> From: Zhao Chenhui<b26998@freescale.com>
>>>=20
>>> In fsl_of_msi_probe(), the virt_msir's chip_data have been stored
>>> the pointer to struct mpic. We add a struct fsl_msi_cascade_data
>>> to store the pointer to struct fsl_msi and msir_index.  Otherwise,
>>> the pointer to struct mpic will be over-written, and will cause
>>> problem when calling eoi() of the irq.
>>>    =20
>> I don't quite understand. Do you mean someone was overwriting
>> handler_data somewhere?
>>  =20
>=20
> The patch at http://patchwork.ozlabs.org/patch/48794/ was overwriting =
the chip_data.  We move the newly added pointer to fsl_msi structure to =
the handler data.

Let's fix that patch.

- k=

Re: [PATCH 1/4] fsl_msi: fix the conflict of virt_msir's chip_data

[Li Yang]

On Mon, Apr 19, 2010 at 8:19 PM, Kumar Gala <galak@kernel.crashing.org> wro=
te:
>
> On Apr 18, 2010, at 11:50 PM, Li Yang wrote:
>
>> On 4/19/2010 10:40 AM, Michael Ellerman wrote:
>>> On Fri, 2010-04-16 at 15:34 +0800, Li Yang wrote:
>>>
>>>> From: Zhao Chenhui<b26998@freescale.com>
>>>>
>>>> In fsl_of_msi_probe(), the virt_msir's chip_data have been stored
>>>> the pointer to struct mpic. We add a struct fsl_msi_cascade_data
>>>> to store the pointer to struct fsl_msi and msir_index. =C2=A0Otherwise=
,
>>>> the pointer to struct mpic will be over-written, and will cause
>>>> problem when calling eoi() of the irq.
>>>>
>>> I don't quite understand. Do you mean someone was overwriting
>>> handler_data somewhere?
>>>
>>
>> The patch at http://patchwork.ozlabs.org/patch/48794/ was overwriting th=
e chip_data. =C2=A0We move the newly added pointer to fsl_msi structure to =
the handler data.
>
> Let's fix that patch.

All right.  You can merge the two patches if you like it that way.

Thanks.
Leo