pci: fix compilation when CONFIG_PCI_MSI=n

PATCH In review Rusty Russell 4 participants 4 messages Jun 16, 2010 – Jun 18, 2010 PCI

Share & tools

Rusty Russell

author

2010-06-16 04:51 UTC #0 lore raw

PATCH 1/1 pci: fix compilation when CONFIG_PCI_MSI=n

drivers/pci/pci.c:2277: error: redefinition of ‘pci_msi_off’ include/linux/pci.h:786: note: previous definition of ‘pci_msi_off’ was here

Signed-off-by Rusty Russell <[email protected]>

---
 drivers/pci/pci.c |    2 ++
 1 file changed, 2 insertions(+)

drivers/pci/pci.c +2 -0

@@ -2266,6 +2266,7 @@ pci_intx(struct pci_dev *pdev, int enabl
 	}
 }
 
+#ifdef CONFIG_PCI_MSI
 /**
  * pci_msi_off - disables any msi or msix capabilities
  * @dev: the PCI device to operate on

@@ -2293,6 +2294,7 @@ void pci_msi_off(struct pci_dev *dev)
 	}
 }
 EXPORT_SYMBOL_GPL(pci_msi_off);
+#endif
 
 #ifndef HAVE_ARCH_PCI_SET_DMA_MAX_SEGMENT_SIZE
 int pci_set_dma_max_seg_size(struct pci_dev *dev, unsigned int size)

Hidetoshi Seto

2010-06-16 06:10 UTC #1 lore raw

Quoted 20 lines · Rusty Russell

drivers/pci/pci.c:2277: error: redefinition of ‘pci_msi_off’ include/linux/pci.h:786: note: previous definition of ‘pci_msi_off’ was here
Signed-off-by Rusty Russell <[email protected]>
---
 drivers/pci/pci.c |    2 ++
 1 file changed, 2 insertions(+)
drivers/pci/pci.c +1 -0
@@ -2266,6 +2266,7 @@ pci_intx(struct pci_dev *pdev, int enabl
 	}
 }
 
+#ifdef CONFIG_PCI_MSI
 /**
  * pci_msi_off - disables any msi or msix capabilities
  * @dev: the PCI device to operate on

drivers/pci/pci.c void pci_msi_off(struct pci_dev *dev) · line 2300

Quoted 7 lines

drivers/pci/pci.c +1 -0

@@ -2293,6 +2294,7 @@ void pci_msi_off(struct pci_dev *dev)
 	}
 }
 EXPORT_SYMBOL_GPL(pci_msi_off);
+#endif
 
 #ifndef HAVE_ARCH_PCI_SET_DMA_MAX_SEGMENT_SIZE
 int pci_set_dma_max_seg_size(struct pci_dev *dev, unsigned int size)

Rusty, thank you for reporting it.
But I think this fix is not correct.

It looks like a bug in Michael's patch:
[PATCH for-2.6.35] virtio-pci: disable msi at startup

The patch includes a change as following:

Quoted 6 lines

include/linux/pci.h +0 -0

@@ -780,7 +780,11 @@ int __must_check pci_set_mwi(struct pci_dev *dev);
 int pci_try_set_mwi(struct pci_dev *dev);
 void pci_clear_mwi(struct pci_dev *dev);
 void pci_intx(struct pci_dev *dev, int enable);

Quoted 8 lines

include/linux/pci.h int __must_check pci_set_mwi(struct pci_dev *dev); · line 790

Quoted 8 lines

include/linux/pci.h +4 -0

+#ifdef CONFIG_PCI_MSI
 void pci_msi_off(struct pci_dev *dev);
+#else
+static inline void pci_msi_off(struct pci_dev *dev) {}
+#endif
 int pci_set_dma_max_seg_size(struct pci_dev *dev, unsigned int size);
 int pci_set_dma_seg_boundary(struct pci_dev *dev, unsigned long mask);
 int pcix_get_max_mmrbc(struct pci_dev *dev);

According to the old commit f5f2b13129a6541debf8851bae843cbbf48298b7, pci_msi_off() was designed to be available even when msi support is not compiled in. So I think this change will make something wrong.

Michael, could you check your patch?

Thanks,
H.Seto

Michael S. Tsirkin

2010-06-16 08:27 UTC #2 lore raw

Quoted 56 lines · Hidetoshi Seto

Quoted 20 lines · Rusty Russell
drivers/pci/pci.c:2277: error: redefinition of ‘pci_msi_off’ include/linux/pci.h:786: note: previous definition of ‘pci_msi_off’ was here
Signed-off-by Rusty Russell <[email protected]>
---
 drivers/pci/pci.c |    2 ++
 1 file changed, 2 insertions(+)
drivers/pci/pci.c +1 -0
@@ -2266,6 +2266,7 @@ pci_intx(struct pci_dev *pdev, int enabl
 	}
 }
 
+#ifdef CONFIG_PCI_MSI
 /**
  * pci_msi_off - disables any msi or msix capabilities
  * @dev: the PCI device to operate on
drivers/pci/pci.c void pci_msi_off(struct pci_dev *dev) · line 2300
Quoted 7 lines
drivers/pci/pci.c +1 -0
@@ -2293,6 +2294,7 @@ void pci_msi_off(struct pci_dev *dev)
 	}
 }
 EXPORT_SYMBOL_GPL(pci_msi_off);
+#endif
 
 #ifndef HAVE_ARCH_PCI_SET_DMA_MAX_SEGMENT_SIZE
 int pci_set_dma_max_seg_size(struct pci_dev *dev, unsigned int size)
Rusty, thank you for reporting it.
But I think this fix is not correct.
It looks like a bug in Michael's patch:
[PATCH for-2.6.35] virtio-pci: disable msi at startup
The patch includes a change as following:
Quoted 6 lines
Quoted 6 lines
include/linux/pci.h +0 -0
@@ -780,7 +780,11 @@ int __must_check pci_set_mwi(struct pci_dev *dev);
 int pci_try_set_mwi(struct pci_dev *dev);
 void pci_clear_mwi(struct pci_dev *dev);
 void pci_intx(struct pci_dev *dev, int enable);
Quoted 8 lines
include/linux/pci.h int __must_check pci_set_mwi(struct pci_dev *dev); · line 790
Quoted 8 lines
include/linux/pci.h +4 -0
+#ifdef CONFIG_PCI_MSI
 void pci_msi_off(struct pci_dev *dev);
+#else
+static inline void pci_msi_off(struct pci_dev *dev) {}
+#endif
 int pci_set_dma_max_seg_size(struct pci_dev *dev, unsigned int size);
 int pci_set_dma_seg_boundary(struct pci_dev *dev, unsigned long mask);
 int pcix_get_max_mmrbc(struct pci_dev *dev);
According to the old commit f5f2b13129a6541debf8851bae843cbbf48298b7, pci_msi_off() was designed to be available even when msi support is not compiled in. So I think this change will make something wrong.

Quoted 5 lines

Michael, could you check your patch?
Thanks,
H.Seto

You are right. I didn't think this through: we must disable the capability at startup even if the current kernel does not support MSI, otherwise the device won't work.

Please apply this on top.

Signed-off-by Michael S. Tsirkin <[email protected]>

---

include/linux/pci.h +0 -4

index 31d8a12..7cb0084 100644
@@ -780,11 +780,7 @@ int __must_check pci_set_mwi(struct pci_dev *dev);
 int pci_try_set_mwi(struct pci_dev *dev);
 void pci_clear_mwi(struct pci_dev *dev);
 void pci_intx(struct pci_dev *dev, int enable);
-#ifdef CONFIG_PCI_MSI
 void pci_msi_off(struct pci_dev *dev);
-#else
-static inline void pci_msi_off(struct pci_dev *dev) {}
-#endif
 int pci_set_dma_max_seg_size(struct pci_dev *dev, unsigned int size);
 int pci_set_dma_seg_boundary(struct pci_dev *dev, unsigned long mask);
 int pcix_get_max_mmrbc(struct pci_dev *dev);

Jesse Barnes

2010-06-18 16:43 UTC #3 lore raw

Quoted 79 lines · On Wed, 16 Jun 2010 11:22:40 +0300 "Michael S. Tsirkin" <[email protected]> wrote:

Quoted 56 lines · Hidetoshi Seto
Quoted 20 lines · Rusty Russell
drivers/pci/pci.c:2277: error: redefinition of ‘pci_msi_off’ include/linux/pci.h:786: note: previous definition of ‘pci_msi_off’ was here
Signed-off-by Rusty Russell <[email protected]>
---
 drivers/pci/pci.c |    2 ++
 1 file changed, 2 insertions(+)
drivers/pci/pci.c +1 -0
@@ -2266,6 +2266,7 @@ pci_intx(struct pci_dev *pdev, int enabl
 	}
 }
 
+#ifdef CONFIG_PCI_MSI
 /**
  * pci_msi_off - disables any msi or msix capabilities
  * @dev: the PCI device to operate on
drivers/pci/pci.c void pci_msi_off(struct pci_dev *dev) · line 2300
Quoted 7 lines
drivers/pci/pci.c +1 -0
@@ -2293,6 +2294,7 @@ void pci_msi_off(struct pci_dev *dev)
 	}
 }
 EXPORT_SYMBOL_GPL(pci_msi_off);
+#endif
 
 #ifndef HAVE_ARCH_PCI_SET_DMA_MAX_SEGMENT_SIZE
 int pci_set_dma_max_seg_size(struct pci_dev *dev, unsigned int size)
Rusty, thank you for reporting it.
But I think this fix is not correct.
It looks like a bug in Michael's patch:
[PATCH for-2.6.35] virtio-pci: disable msi at startup
The patch includes a change as following:
Quoted 6 lines
Quoted 6 lines
include/linux/pci.h +0 -0
@@ -780,7 +780,11 @@ int __must_check pci_set_mwi(struct pci_dev *dev);
 int pci_try_set_mwi(struct pci_dev *dev);
 void pci_clear_mwi(struct pci_dev *dev);
 void pci_intx(struct pci_dev *dev, int enable);
Quoted 8 lines
include/linux/pci.h int __must_check pci_set_mwi(struct pci_dev *dev); · line 786
Quoted 8 lines
include/linux/pci.h +4 -0
+#ifdef CONFIG_PCI_MSI
 void pci_msi_off(struct pci_dev *dev);
+#else
+static inline void pci_msi_off(struct pci_dev *dev) {}
+#endif
 int pci_set_dma_max_seg_size(struct pci_dev *dev, unsigned int size);
 int pci_set_dma_seg_boundary(struct pci_dev *dev, unsigned long mask);
 int pcix_get_max_mmrbc(struct pci_dev *dev);
According to the old commit f5f2b13129a6541debf8851bae843cbbf48298b7, pci_msi_off() was designed to be available even when msi support is not compiled in. So I think this change will make something wrong.
Quoted 5 lines
Michael, could you check your patch?
Thanks,
H.Seto
You are right. I didn't think this through: we must disable the capability at startup even if the current kernel does not support MSI, otherwise the device won't work.
Please apply this on top.
Signed-off-by Michael S. Tsirkin <[email protected]>
---
include/linux/pci.h +0 -0
index 31d8a12..7cb0084 100644
@@ -780,11 +780,7 @@ int __must_check pci_set_mwi(struct pci_dev *dev);
 int pci_try_set_mwi(struct pci_dev *dev);

include/linux/pci.h int __must_check pci_set_mwi(struct pci_dev *dev); · line 786

Quoted 10 lines

include/linux/pci.h +0 -4

 void pci_clear_mwi(struct pci_dev *dev);
 void pci_intx(struct pci_dev *dev, int enable);
-#ifdef CONFIG_PCI_MSI
 void pci_msi_off(struct pci_dev *dev);
-#else
-static inline void pci_msi_off(struct pci_dev *dev) {}
-#endif
 int pci_set_dma_max_seg_size(struct pci_dev *dev, unsigned int size);
 int pci_set_dma_seg_boundary(struct pci_dev *dev, unsigned long mask);
 int pcix_get_max_mmrbc(struct pci_dev *dev);

I don't have the #ifdef in pci.h or the export in my tree. Can you guys fix this up in the virt tree and add my acked-by?

Thanks,

Signature · 1 line

Jesse Barnes, Intel Open Source Technology Center