On Mon, Jun 06, 2005 at 05:20:45PM -0700, Greg KH wrote:
> -EXPORT_SYMBOL(pci_enable_msi);
> -EXPORT_SYMBOL(pci_disable_msi);
> +//EXPORT_SYMBOL(pci_enable_msi);
> +//EXPORT_SYMBOL(pci_disable_msi);
You do plan on deleting these lines, right? (Just a reminder)
> --- gregkh-2.6.orig/drivers/pci/pci.c 2005-06-06 16:16:30.000000000 -0700
> +++ gregkh-2.6/drivers/pci/pci.c 2005-06-06 16:20:06.000000000 -0700
> @@ -402,6 +402,7 @@
> if ((err = pci_enable_device_bars(dev, (1 << PCI_NUM_RESOURCES) - 1)))
> return err;
> pci_fixup_device(pci_fixup_enable, dev);
> + pci_enable_msi(dev);
> dev->is_enabled = 1;
> return 0;
> }
I'm wondering if it would be better/possible to call
enable_msi before calling fixup_device.
But I couldn't find any references to DECLARE_PCI_FIXUP_ENABLE
to see if it would matter.
> @@ -427,7 +428,8 @@
> pci_disable_device(struct pci_dev *dev)
> {
> u16 pci_command;
> -
> +
> + pci_disable_msi(dev);
> pci_read_config_word(dev, PCI_COMMAND, &pci_command);
> if (pci_command & PCI_COMMAND_MASTER) {
> pci_command &= ~PCI_COMMAND_MASTER;
reminder: you probably need to check that msi is still enabled
here or in pci_disable_msi(). I'm thinking of the case where
the driver switched to MSI-X.
> +#ifndef CONFIG_PCI_MSI
> +static inline int pci_enable_msi(struct pci_dev *dev) {return -1;}
> +static inline void pci_disable_msi(struct pci_dev *dev) {}
> +#else
> +extern int pci_enable_msi(struct pci_dev *dev);
> +extern void pci_disable_msi(struct pci_dev *dev);
> +#endif
wouldn't it make more sense to invert this to "ifdef" (vs ifndef)?
> @@ -398,8 +397,6 @@
> /* Switch to INTx by default if MSI enabled */
> if (interrupt_mode == PCIE_PORT_MSIX_MODE)
> pci_disable_msix(dev);
> - else if (interrupt_mode == PCIE_PORT_MSI_MODE)
> - pci_disable_msi(dev);
Why disable msix here but not msi?
This doesn't seem like a good design choice.
I'm suspicious more occurrances will need special handling of MSIX.
Will assign_interrupt_mode() in drivers/pci/pcie/portdrv_core.c
need more changes to deal with the switch to/from MSI/MSIX?
> +++ gregkh-2.6/drivers/net/tg3.c 2005-06-06 16:57:34.000000000 -0700
> @@ -5984,7 +5984,6 @@
> tp->dev->name);
>
> free_irq(tp->pdev->irq, dev);
> - pci_disable_msi(tp->pdev);
>
> tp->tg3_flags2 &= ~TG3_FLG2_USING_MSI;
This chunk of tg3 code will need more than just deleting
the call to pci_disable_msi(). The printk will be wrong
and it's changing the semantics of what TG3_FLG2_USING_MSI
means.
> @@ -6047,7 +6046,7 @@
> if (!(tp->tg3_flags & TG3_FLAG_TAGGED_STATUS)) {
> printk(KERN_WARNING PFX "%s: MSI without TAGGED? "
> "Not using MSI.\n", tp->dev->name);
> - } else if (pci_enable_msi(tp->pdev) == 0) {
> + } else if (pci_in_msi_mode(tp->pdev)) {
Again - the "Not Using MSI" msg here needs more help to be correct.
hth,
grant
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
