On Wed, 2009-05-13 at 15:43 -0600, Matthew Wilcox wrote:
> [This is against Jesse's tree which includes my original patch]
>
...
> diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
> index 3627732..f530611 100644
> --- a/drivers/pci/msi.c
> +++ b/drivers/pci/msi.c
...
> @@ -427,11 +425,18 @@ static int msix_capability_init(struct pci_dev *dev,
> u8 bir;
> void __iomem *base;
>
> - msix_set_enable(dev, 0);/* Ensure msix is disabled as I set it up */
> -
> pos = pci_find_capability(dev, PCI_CAP_ID_MSIX);
> +
> + /*
> + * Some devices require MSI-X to be enabled before we can touch the
> + * MSI-X registers. We need to mask all the vectors to prevent
> + * interrupts coming in before they're fully set up.
> + */
> + pci_read_config_word(dev, pos + PCI_MSIX_FLAGS, &control);
> + control |= PCI_MSIX_FLAGS_ENABLE | PCI_MSIX_FLAGS_MASKALL;
> + pci_write_config_word(dev, pos + PCI_MSIX_FLAGS, control);
> +
I don't like this, sorry.
In particular it means we're enabling MSI before the call to
arch_setup_msi_irqs() - I worry if the pseries firmware is going to be
happy about that.
I'm not sure if you're suggesting it is, but this isn't 30 material
IMHO.
I'd rather we just did (very roughly):
/* Set MSI-X enabled bits */
pci_intx_for_msi(dev, 0);
+
+ pci_read_config_word(dev, pos + PCI_MSIX_FLAGS, &control);
+ control |= PCI_MSIX_FLAGS_ENABLE | PCI_MSIX_FLAGS_MASKALL;
+ pci_write_config_word(dev, pos + PCI_MSIX_FLAGS, control);
+
+ list_for_each_entry(entry, &dev->msi_list, list) {
+ entry->masked = readl(base + j * PCI_MSIX_ENTRY_SIZE +
+ PCI_MSIX_ENTRY_VECTOR_CTRL_OFFSET);
+ msix_mask_irq(entry, 1);
+ }
+
msix_set_enable(dev, 1);
dev->msix_enabled = 1;
cheers
Attachment:
signature.asc
Description: This is a digitally signed message part
