2004-04-26 05:50:08

by Adrian Yee

Subject: [PATCH] 8139too not running s3 suspend/resume pci fix

Having an 8139 based device in my notebook, I often switch between it and wireless. The problem is that the 8139too driver does not save/restore the pci configuration of the card if the device isn't running. This simple patch moves the save/restore code so that the code runs regardless of whether or not the device is running.

I looked at other drivers and they all seem to do the same thing. Is there a reason why this isn't done like in the patch?

Adrian

--- drivers/net/8139too.c.orig 2004-04-25 21:56:21.000000000 -0700
+++ drivers/net/8139too.c 2004-04-25 22:02:14.000000000 -0700
@@ -2554,6 +2554,9 @@ static int rtl8139_suspend (struct pci_d
void *ioaddr = tp->mmio_addr;
unsigned long flags;

+ pci_set_power_state (pdev, 3);
+ pci_save_state (pdev, tp->pci_state);
+
if (!netif_running (dev))
return 0;

@@ -2571,9 +2574,6 @@ static int rtl8139_suspend (struct pci_d

spin_unlock_irqrestore (&tp->lock, flags);

- pci_set_power_state (pdev, 3);
- pci_save_state (pdev, tp->pci_state);
-
return 0;
}

@@ -2583,10 +2583,10 @@ static int rtl8139_resume (struct pci_de
struct net_device *dev = pci_get_drvdata (pdev);
struct rtl8139_private *tp = dev->priv;

- if (!netif_running (dev))
- return 0;
pci_restore_state (pdev, tp->pci_state);
pci_set_power_state (pdev, 0);
+ if (!netif_running (dev))
+ return 0;
rtl8139_init_ring (dev);
rtl8139_hw_start (dev);
netif_device_attach (dev);

2004-04-26 19:07:12

by Felipe W Damasio

Subject: Re: [PATCH] 8139too not running s3 suspend/resume pci fix

Hi Adrian,

Adrian Yee wrote:
> + pci_set_power_state (pdev, 3);
> + pci_save_state (pdev, tp->pci_state);
> +
> if (!netif_running (dev))
> return 0;
>
> @@ -2571,9 +2574,6 @@ static int rtl8139_suspend (struct pci_d
>
> spin_unlock_irqrestore (&tp->lock, flags);
>
> - pci_set_power_state (pdev, 3);
> - pci_save_state (pdev, tp->pci_state);
> -
> return 0;
> }

IMHO, there's no problem in doing "pci_save_state (pdev,
tp->pci_state)" before the suspend code..but I'm more confortable with
leaving the set_power_state at the end of that path, since if the
interface is down, we don't want to leave it in cold state.

> @@ -2583,10 +2583,10 @@ static int rtl8139_resume (struct pci_de
> struct net_device *dev = pci_get_drvdata (pdev);
> struct rtl8139_private *tp = dev->priv;
>
> - if (!netif_running (dev))
> - return 0;
> pci_restore_state (pdev, tp->pci_state);
> pci_set_power_state (pdev, 0);
> + if (!netif_running (dev))
> + return 0;
> rtl8139_init_ring (dev);
> rtl8139_hw_start (dev);
> netif_device_attach (dev);

Same thing here.

Cheers,

Felipe

2004-04-26 21:57:21

by Adrian Yee

Subject: Re: [PATCH] 8139too not running s3 suspend/resume pci fix

Hi Felipe,

> IMHO, there's no problem in doing "pci_save_state (pdev,
> tp->pci_state)" before the suspend code..but I'm more confortable with
> leaving the set_power_state at the end of that path, since if the
> interface is down, we don't want to leave it in cold state.

That makes sense. So something more like this:

--- drivers/net/8139too.c.orig 2004-04-26 14:53:04.715985134 -0700
+++ drivers/net/8139too.c 2004-04-26 14:53:50.373431714 -0700
@@ -2550,6 +2550,8 @@ static int rtl8139_suspend (struct pci_d
void *ioaddr = tp->mmio_addr;
unsigned long flags;

+ pci_save_state (pdev, tp->pci_state);
+
if (!netif_running (dev))
return 0;

@@ -2568,7 +2570,6 @@ static int rtl8139_suspend (struct pci_d
spin_unlock_irqrestore (&tp->lock, flags);

pci_set_power_state (pdev, 3);
- pci_save_state (pdev, tp->pci_state);

return 0;
}
@@ -2579,9 +2580,9 @@ static int rtl8139_resume (struct pci_de
struct net_device *dev = pci_get_drvdata (pdev);
struct rtl8139_private *tp = dev->priv;

+ pci_restore_state (pdev, tp->pci_state);
if (!netif_running (dev))
return 0;
- pci_restore_state (pdev, tp->pci_state);
pci_set_power_state (pdev, 0);
rtl8139_init_ring (dev);
rtl8139_hw_start (dev);