[PATCH] PCI: export pci_restore_msi_state()

[PATCH] PCI: export pci_restore_msi_state()

[Linas Vepstas]

PCI error recovery usually involves the PCI adapter being reset.
If the device is using MSI, the reset will cause the MSI state 
to be lost; the device driver needs to restore the MSI state.

The pci_restore_msi_state() routine is currently protected
by CONFIG_PM; remove this, and also export the symbol, so
that it can be used in a modle.

Signed-off-by: Linas Vepstas <linas@austin.ibm.com>

----
I am so sorry I wasn't able to send this 3 weeks ago, when
I first wrote the patch. There was simply no functional
hardware available to actually run this stuff :-(

Patches that use this, including those for tg3 and e1000e and ixgbe
i.e. MSI-using drivers, are to follow "real soon now". 

 drivers/pci/msi.c   |    3 +--
 drivers/pci/pci.h   |    6 ------
 include/linux/pci.h |    2 ++
 3 files changed, 3 insertions(+), 8 deletions(-)

Index: linux-2.6.23-rc8-mm1/drivers/pci/msi.c
===================================================================
--- linux-2.6.23-rc8-mm1.orig/drivers/pci/msi.c	2007-10-16 15:14:20.000000000 -0500
+++ linux-2.6.23-rc8-mm1/drivers/pci/msi.c	2007-10-16 15:14:42.000000000 -0500
@@ -224,7 +224,6 @@ static struct msi_desc* alloc_msi_entry(
 	return entry;
 }
 
-#ifdef CONFIG_PM
 static void __pci_restore_msi_state(struct pci_dev *dev)
 {
 	int pos;
@@ -282,7 +281,7 @@ void pci_restore_msi_state(struct pci_de
 	__pci_restore_msi_state(dev);
 	__pci_restore_msix_state(dev);
 }
-#endif	/* CONFIG_PM */
+EXPORT_SYMBOL_GPL(pci_restore_msi_state);
 
 /**
  * msi_capability_init - configure device's MSI capability structure
Index: linux-2.6.23-rc8-mm1/drivers/pci/pci.h
===================================================================
--- linux-2.6.23-rc8-mm1.orig/drivers/pci/pci.h	2007-10-16 15:14:20.000000000 -0500
+++ linux-2.6.23-rc8-mm1/drivers/pci/pci.h	2007-10-16 15:19:33.000000000 -0500
@@ -45,12 +45,6 @@ static inline void pci_no_msi(void) { }
 static inline void pci_msi_init_pci_dev(struct pci_dev *dev) { }
 #endif
 
-#if defined(CONFIG_PCI_MSI) && defined(CONFIG_PM)
-void pci_restore_msi_state(struct pci_dev *dev);
-#else
-static inline void pci_restore_msi_state(struct pci_dev *dev) {}
-#endif
-
 static inline int pci_no_d1d2(struct pci_dev *dev)
 {
 	unsigned int parent_dstates = 0;
Index: linux-2.6.23-rc8-mm1/include/linux/pci.h
===================================================================
--- linux-2.6.23-rc8-mm1.orig/include/linux/pci.h	2007-10-01 13:26:38.000000000 -0500
+++ linux-2.6.23-rc8-mm1/include/linux/pci.h	2007-10-16 15:19:07.000000000 -0500
@@ -665,6 +665,7 @@ static inline int pci_enable_msix(struct
 	struct msix_entry *entries, int nvec) {return -1;}
 static inline void pci_disable_msix(struct pci_dev *dev) {}
 static inline void msi_remove_pci_irq_vectors(struct pci_dev *dev) {}
+static inline void pci_restore_msi_state(struct pci_dev *dev) {}
 #else
 extern int pci_enable_msi(struct pci_dev *dev);
 extern void pci_disable_msi(struct pci_dev *dev);
@@ -672,6 +673,7 @@ extern int pci_enable_msix(struct pci_de
 	struct msix_entry *entries, int nvec);
 extern void pci_disable_msix(struct pci_dev *dev);
 extern void msi_remove_pci_irq_vectors(struct pci_dev *dev);
+extern void pci_restore_msi_state(struct pci_dev *dev);
 #endif
 
 #ifdef CONFIG_HT_IRQ

Re: [PATCH] PCI: export pci_restore_msi_state()

[Linas Vepstas]

Hi,

On Wed, Nov 07, 2007 at 03:43:59PM -0600, Linas Vepstas wrote:
> 
> PCI error recovery usually involves the PCI adapter being reset.
> If the device is using MSI, the reset will cause the MSI state 
> to be lost; the device driver needs to restore the MSI state.
> 
> The pci_restore_msi_state() routine is currently protected
> by CONFIG_PM; remove this, and also export the symbol, so
> that it can be used in a modle.
> 
> Signed-off-by: Linas Vepstas <linas@austin.ibm.com>

The long time delay has managed to muddle notes & recollection 
of prior discussions. This patch should have had a 

Signed-off-by: Matt Carlson <mcarlson@broadcom.com>
Signed-off-by: Michael Chan <mchan@broadcom.com>

on it; its the same patch that was submitted a long time ago.

During the discussions of 21 Oct, it was proposed that there
should be an arch hook for pci_restore_msi_state(), so that
it would be treated at the same level as msi setup and teardown.
I'd volunteered to write that patch. 

When I sat down to do it, however, I realized that I did not
actually *need* it. And so I wondered: why am I writing un-needed,
but theoretically proper, code? So I punted, and I didn't.
Does that make sense?

That's also why this patch is just a resubmission of the old
patch. The original thread is here:

http://www.mail-archive.com/netdev@vger.kernel.org/msg51296.html

--linas

Re: [PATCH] PCI: export pci_restore_msi_state()

[Linas Vepstas]

On Thu, Nov 08, 2007 at 07:21:01PM -0600, Wen Xiong wrote:
> Hi Linas,
> 
> I saw you have submitted several patches to support pci-express network 
> adapters EEH. But looks only this patch fixed something in linux kernel 
> code.

And its an old patch, submitted long ago ... I've resubmitted, because
it seems that its the best/most correct thing to do.

> Do you mean I can test EEH callback functions in device driver after I 
> apply  this patch in the kernel?

Yes, please.  Note, however, I was never able to make the pci-e 
version of the e1000 work. It comes up, generates interrupts, and
registeres are readable and writeable. But it behvaes as if the PHY 
is turned off -- no network traffic goes thorugh. (I tried turning
PHY on explicitly; that didn't help). So there is still something 
wrong somewhere, probably in the e1000 deice driver. I'm guessing
the pci-e to pci-x bridge chip on that card is not quite resetting
the card completely.

> Do you do "pci_save_msi_state" somewhere in the kernel? Or you suggest to 
> do "pci_save_msi_state" and "pci_restore_msi_state" in each device driver?

There is no "save state", the msi state can't be saved. The MSI regs
are write-only, and they are controlled by firmware. The restore_state
function is the only one you need.

--linas