Patchwork NET/r8169: Rework suspend and resume

login
register
mail settings
Submitter Rafael J. Wysocki
Date April 5, 2009, 6:40 p.m.
Message ID <200904052040.04752.rjw@sisk.pl>
Download mbox | patch
Permalink /patch/25615/
State Accepted
Delegated to: David Miller
Headers show

Comments

Rafael J. Wysocki - April 5, 2009, 6:40 p.m.
From: Rafael J. Wysocki <rjw@sisk.pl>

The recent changes of the PCI PM core allow us to simplify the
suspend and resume handling in a number of device drivers, since they
don't need to carry out the general PCI PM operations, such as
changing the power state of the device, during suspend and resume any
more.

Simplify the suspend and resume callbacks of r8169 using the
observation that the PCI PM core can take care of some operations
carried out by the driver.

Additionally, make the shutdown callback of r8169 only put the device
into a low power state if the system is going to be powered off
(kexec is known to have problems with network adapters that are put
into low power states on shutdown).

This patch has been tested on MSI Wind U100.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 drivers/net/r8169.c |   61 +++++++++++++++++++++++++++++++++-------------------
 1 file changed, 39 insertions(+), 22 deletions(-)

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Bruno Prémont - April 5, 2009, 8:33 p.m.
On Sun, 05 April 2009 "Rafael J. Wysocki" <rjw@sisk.pl> wrote:
> From: Rafael J. Wysocki <rjw@sisk.pl>
> 
> The recent changes of the PCI PM core allow us to simplify the
> suspend and resume handling in a number of device drivers, since they
> don't need to carry out the general PCI PM operations, such as
> changing the power state of the device, during suspend and resume any
> more.
> 
> Simplify the suspend and resume callbacks of r8169 using the
> observation that the PCI PM core can take care of some operations
> carried out by the driver.
> 
> Additionally, make the shutdown callback of r8169 only put the device
> into a low power state if the system is going to be powered off
> (kexec is known to have problems with network adapters that are put
> into low power states on shutdown).
> 
> This patch has been tested on MSI Wind U100.

Tested against 2.6.29.1 on my Commell LE-365 and suspend (S3) + WoL
still works as expected.

02:08.0 Ethernet controller [0200]: Realtek Semiconductor Co., Ltd.
                     RTL-8169 Gigabit Ethernet [10ec:8169] (rev 10)

Bruno
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Rafael J. Wysocki - April 5, 2009, 9:59 p.m.
On Sunday 05 April 2009, Bruno Prémont wrote:
> On Sun, 05 April 2009 "Rafael J. Wysocki" <rjw@sisk.pl> wrote:
> > From: Rafael J. Wysocki <rjw@sisk.pl>
> > 
> > The recent changes of the PCI PM core allow us to simplify the
> > suspend and resume handling in a number of device drivers, since they
> > don't need to carry out the general PCI PM operations, such as
> > changing the power state of the device, during suspend and resume any
> > more.
> > 
> > Simplify the suspend and resume callbacks of r8169 using the
> > observation that the PCI PM core can take care of some operations
> > carried out by the driver.
> > 
> > Additionally, make the shutdown callback of r8169 only put the device
> > into a low power state if the system is going to be powered off
> > (kexec is known to have problems with network adapters that are put
> > into low power states on shutdown).
> > 
> > This patch has been tested on MSI Wind U100.
> 
> Tested against 2.6.29.1 on my Commell LE-365 and suspend (S3) + WoL
> still works as expected.
> 
> 02:08.0 Ethernet controller [0200]: Realtek Semiconductor Co., Ltd.
>                      RTL-8169 Gigabit Ethernet [10ec:8169] (rev 10)

Thanks for testing!

Best,
Rafael
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller - April 13, 2009, 11:20 p.m.
From: "Rafael J. Wysocki" <rjw@sisk.pl>
Date: Sun, 5 Apr 2009 23:59:31 +0200

> On Sunday 05 April 2009, Bruno Prémont wrote:
>> On Sun, 05 April 2009 "Rafael J. Wysocki" <rjw@sisk.pl> wrote:
>> > From: Rafael J. Wysocki <rjw@sisk.pl>
>> > 
>> > The recent changes of the PCI PM core allow us to simplify the
>> > suspend and resume handling in a number of device drivers, since they
>> > don't need to carry out the general PCI PM operations, such as
>> > changing the power state of the device, during suspend and resume any
>> > more.
>> > 
>> > Simplify the suspend and resume callbacks of r8169 using the
>> > observation that the PCI PM core can take care of some operations
>> > carried out by the driver.
>> > 
>> > Additionally, make the shutdown callback of r8169 only put the device
>> > into a low power state if the system is going to be powered off
>> > (kexec is known to have problems with network adapters that are put
>> > into low power states on shutdown).
>> > 
>> > This patch has been tested on MSI Wind U100.
>> 
>> Tested against 2.6.29.1 on my Commell LE-365 and suspend (S3) + WoL
>> still works as expected.
>> 
>> 02:08.0 Ethernet controller [0200]: Realtek Semiconductor Co., Ltd.
>>                      RTL-8169 Gigabit Ethernet [10ec:8169] (rev 10)
> 
> Thanks for testing!

I've applied this patch to net-next-2.6, thanks everyone.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

Index: linux-2.6/drivers/net/r8169.c
===================================================================
--- linux-2.6.orig/drivers/net/r8169.c
+++ linux-2.6/drivers/net/r8169.c
@@ -3791,16 +3791,13 @@  static struct net_device_stats *rtl8169_
 	return &dev->stats;
 }
 
-#ifdef CONFIG_PM
-
-static int rtl8169_suspend(struct pci_dev *pdev, pm_message_t state)
+static void rtl8169_net_suspend(struct net_device *dev)
 {
-	struct net_device *dev = pci_get_drvdata(pdev);
 	struct rtl8169_private *tp = netdev_priv(dev);
 	void __iomem *ioaddr = tp->mmio_addr;
 
 	if (!netif_running(dev))
-		goto out_pci_suspend;
+		return;
 
 	netif_device_detach(dev);
 	netif_stop_queue(dev);
@@ -3812,24 +3809,25 @@  static int rtl8169_suspend(struct pci_de
 	rtl8169_rx_missed(dev, ioaddr);
 
 	spin_unlock_irq(&tp->lock);
+}
 
-out_pci_suspend:
-	pci_save_state(pdev);
-	pci_enable_wake(pdev, pci_choose_state(pdev, state),
-		(tp->features & RTL_FEATURE_WOL) ? 1 : 0);
-	pci_set_power_state(pdev, pci_choose_state(pdev, state));
+#ifdef CONFIG_PM
+
+static int rtl8169_suspend(struct device *device)
+{
+	struct pci_dev *pdev = to_pci_dev(device);
+	struct net_device *dev = pci_get_drvdata(pdev);
+
+	rtl8169_net_suspend(dev);
 
 	return 0;
 }
 
-static int rtl8169_resume(struct pci_dev *pdev)
+static int rtl8169_resume(struct device *device)
 {
+	struct pci_dev *pdev = to_pci_dev(device);
 	struct net_device *dev = pci_get_drvdata(pdev);
 
-	pci_set_power_state(pdev, PCI_D0);
-	pci_restore_state(pdev);
-	pci_enable_wake(pdev, PCI_D0, 0);
-
 	if (!netif_running(dev))
 		goto out;
 
@@ -3840,23 +3838,42 @@  out:
 	return 0;
 }
 
+static struct dev_pm_ops rtl8169_pm_ops = {
+	.suspend = rtl8169_suspend,
+	.resume = rtl8169_resume,
+	.freeze = rtl8169_suspend,
+	.thaw = rtl8169_resume,
+	.poweroff = rtl8169_suspend,
+	.restore = rtl8169_resume,
+};
+
+#define RTL8169_PM_OPS	(&rtl8169_pm_ops)
+
+#else /* !CONFIG_PM */
+
+#define RTL8169_PM_OPS	NULL
+
+#endif /* !CONFIG_PM */
+
 static void rtl_shutdown(struct pci_dev *pdev)
 {
-	rtl8169_suspend(pdev, PMSG_SUSPEND);
-}
+	struct net_device *dev = pci_get_drvdata(pdev);
 
-#endif /* CONFIG_PM */
+	rtl8169_net_suspend(dev);
+
+	if (system_state == SYSTEM_POWER_OFF) {
+		pci_wake_from_d3(pdev, true);
+		pci_set_power_state(pdev, PCI_D3hot);
+	}
+}
 
 static struct pci_driver rtl8169_pci_driver = {
 	.name		= MODULENAME,
 	.id_table	= rtl8169_pci_tbl,
 	.probe		= rtl8169_init_one,
 	.remove		= __devexit_p(rtl8169_remove_one),
-#ifdef CONFIG_PM
-	.suspend	= rtl8169_suspend,
-	.resume		= rtl8169_resume,
 	.shutdown	= rtl_shutdown,
-#endif
+	.driver.pm	= RTL8169_PM_OPS,
 };
 
 static int __init rtl8169_init_module(void)