On Mar 18, 2010, at 10:06 PM, Michael Ellerman wrote:

> On Thu, 2010-03-18 at 09:53 -0500, Kumar Gala wrote:
>> From: Lan Chunhe-B25806 <b25...@freescale.com>
>> 
>> Freescale QorIQ P4080 has three MSI banks and the original code
>> can not work well. This patch adds multiple MSI banks support for
>> Freescale processor.
>> 
>> Signed-off-by: Lan Chunhe-B25806 <b25...@freescale.com>
>> Signed-off-by: Roy Zang <tie-fei.z...@freescale.com>
> 
>> @@ -146,9 +149,13 @@ static int fsl_setup_msi_irqs(struct pci_dev *pdev, int 
>> nvec, int type)
>>      unsigned int virq;
>>      struct msi_desc *entry;
>>      struct msi_msg msg;
>> -    struct fsl_msi *msi_data = fsl_msi;
>> +    struct fsl_msi *msi_data;
>> 
>>      list_for_each_entry(entry, &pdev->msi_list, list) {
>> +            if (entry->irq == NO_IRQ)
>> +                    continue;
> 
> This looks wrong, entry->irq should always be 0 here because it was just
> kzalloc'ed - you should only be doing this check in teardown.
> 
>> -    WARN_ON(ppc_md.setup_msi_irqs);
>> -    ppc_md.setup_msi_irqs = fsl_setup_msi_irqs;
>> -    ppc_md.teardown_msi_irqs = fsl_teardown_msi_irqs;
>> -    ppc_md.msi_check_device = fsl_msi_check_device;
>> +    /* 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;
>> +            ppc_md.teardown_msi_irqs = fsl_teardown_msi_irqs;
>> +            ppc_md.msi_check_device = fsl_msi_check_device;
>> +    } else if (ppc_md.setup_msi_irqs != fsl_setup_msi_irqs) {
>> +            dev_err(&dev->dev, "Different MSI driver already installed!\n");
>> +            err = -EBUSY; /* or some other error code */
>> +            goto error_out;
>> +    }
> 
> I liked it the way it was, because having two competing MSI backends
> means something's probably not going to work. But it's your driver so
> whatever you like.

The previous WARN_ON() is problematic when we have multiple (of the same type) 
MSI blocks.  The check was intended to do exactly what you are suggesting.  If 
you think its doing something else let us know.

- k
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Reply via email to