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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CA+5PVA7K66kKwKHUsNRjw9GLjvKBWgovmKPR4uOMMNFtiWfrzw@mail.gmail.com>
Date:	Fri, 16 Mar 2012 08:41:07 -0400
From:	Josh Boyer <jwboyer@...il.com>
To:	Mai La <mla@....com>
Cc:	Benjamin Herrenschmidt <benh@...nel.crashing.org>,
	Paul Mackerras <paulus@...ba.org>,
	Josh Boyer <jwboyer@...ux.vnet.ibm.com>,
	Matt Porter <mporter@...nel.crashing.org>,
	Tirumala R Marri <tmarri@....com>,
	Grant Likely <grant.likely@...retlab.ca>,
	Michael Neuling <mikey@...ling.org>,
	Kumar Gala <galak@...nel.crashing.org>,
	Anton Blanchard <anton@...ba.org>,
	linuxppc-dev@...ts.ozlabs.org, linux-kernel@...r.kernel.org,
	open-source-review@....com
Subject: Re: [PATCH 1/2] [v3] powerpc/44x: Fix PCI MSI support for Maui
 APM821xx SoC and Bluestone board

On Mon, Mar 12, 2012 at 4:07 AM, Mai La <mla@....com> wrote:
> 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;

This should be static.

>  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);
> +

What happens if this allocation fails?  Unlikely perhaps, but we should still
probably try and handle the error.

>        list_for_each_entry(entry, &dev->msi_list, list) {
>                int_no = msi_bitmap_alloc_hwirqs(&msi_data->bitmap, 1);
>                if (int_no >= 0)

<snip>

> @@ -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;
> +

Maybe return -ENODEV here instead of -1?  It probably doesn't matter at the
moment, but if anything actually checked why the probe function failed, that
would be more descriptive.

josh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/