Re: Proper pci_enable_device() error handling in resume routine

[Valerie Henson <val_henson@linux.intel.com>]

On Mon, Aug 21, 2006 at 11:12:03AM -0700, Auke Kok wrote:
> Valerie Henson wrote:
> >I'm trying to properly handle pci_enable_device() errors in the resume
> >routines of a couple of tulip drivers.  I noticed that several drivers
> >pay attention to errors from pci_enable_device() in the init routine
> >but ignore it on resume; other drivers vary wildly.  What's proper
> >behavior when resuming?  Extant examples:
> >
> >0. Don't call pci_enable_device() at all (8139too)
> >1. Ignore the return value (eepro100, many others)
> >2. Check for failure and bail out, but return success (sungem)
> 
> Digging through e1000 I spot that we even pci_enable_device after a PCI 
> error, so it is good practice I think to make sure the device is up. I 
> suppose that most people can live without explicit re-enabling the device 
> (most NICs are on anyway), but if we do enable it explicitly we should 
> certainly check the result code. Interestingly enough we failed to do this 
> in e1000, so I'll submit a patch for that later.

For people looking for how to implement this in their own driver,
here's how I solved it in tulip (will submit later):

---
 drivers/net/tulip/tulip_core.c  |    5 ++++-
 drivers/net/tulip/winbond-840.c |   10 +++++++---
 2 files changed, 11 insertions(+), 4 deletions(-)

--- a/drivers/net/tulip/tulip_core.c
+++ b/drivers/net/tulip/tulip_core.c
@@ -1780,7 +1780,10 @@ static int tulip_resume(struct pci_dev *
 	pci_set_power_state(pdev, PCI_D0);
 	pci_restore_state(pdev);
 
-	pci_enable_device(pdev);
+	if ((retval = pci_enable_device(pdev))) {
+		printk (KERN_ERR "tulip: pci_enable_device failed in resume\n");
+		return retval;
+	}
 
 	if ((retval = request_irq(dev->irq, &tulip_interrupt, IRQF_SHARED, dev->name, dev))) {
 		printk (KERN_ERR "tulip: request_irq failed in resume\n");



-VAL