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

[Michael Ellerman <michael@ellerman.id.au>]

[-- 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 --]