Patchwork r8169: runtime resume before shutdown.

login
register
mail settings
Submitter fran├žois romieu
Date March 6, 2012, 11:14 a.m.
Message ID <20120306111412.GA1805@electric-eye.fr.zoreil.com>
Download mbox | patch
Permalink /patch/144912/
State Accepted
Delegated to: David Miller
Headers show

Comments

fran├žois romieu - March 6, 2012, 11:14 a.m.
With runtime PM, if the ethernet cable is disconnected, the device is
transitioned to D3 state to conserve energy. If the system is shutdown
in this state, any register accesses in rtl_shutdown are dropped on
the floor. As the device was programmed by .runtime_suspend() to wake
on link changes, it is thus brought back up as soon as the link recovers.

Resuming every suspended device through the driver core would slow things
down and it is not clear how many devices really need it now.

Original report and D0 transition patch by Sameer Nanda. Patch has been
changed to comply with advices by Rafael J. Wysocki and the PM folks.

Reported-by: Sameer Nanda <snanda@chromium.org>
Signed-off-by: Francois Romieu <romieu@fr.zoreil.com>
Cc: Rafael J. Wysocki <rjw@sisk.pl>
Cc: Hayes Wang <hayeswang@realtek.com>
Cc: Alan Stern <stern@rowland.harvard.edu>
---

 This sucker turned my silent wake-on-alarm workstation into an early
 sunday morning testbed takeoff. Please consider before 3.3 goes out.

 drivers/net/ethernet/realtek/r8169.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)
Rafael J. Wysocki - March 6, 2012, 7:18 p.m.
On Tuesday, March 06, 2012, Francois Romieu wrote:
> With runtime PM, if the ethernet cable is disconnected, the device is
> transitioned to D3 state to conserve energy. If the system is shutdown
> in this state, any register accesses in rtl_shutdown are dropped on
> the floor. As the device was programmed by .runtime_suspend() to wake
> on link changes, it is thus brought back up as soon as the link recovers.
> 
> Resuming every suspended device through the driver core would slow things
> down and it is not clear how many devices really need it now.
> 
> Original report and D0 transition patch by Sameer Nanda. Patch has been
> changed to comply with advices by Rafael J. Wysocki and the PM folks.
> 
> Reported-by: Sameer Nanda <snanda@chromium.org>
> Signed-off-by: Francois Romieu <romieu@fr.zoreil.com>
> Cc: Rafael J. Wysocki <rjw@sisk.pl>
> Cc: Hayes Wang <hayeswang@realtek.com>
> Cc: Alan Stern <stern@rowland.harvard.edu>

Acked-by: Rafael J. Wysocki <rjw@sisk.pl>

> ---
> 
>  This sucker turned my silent wake-on-alarm workstation into an early
>  sunday morning testbed takeoff. Please consider before 3.3 goes out.
> 
>  drivers/net/ethernet/realtek/r8169.c |    5 +++++
>  1 files changed, 5 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
> index 1adf177..bbacb37 100644
> --- a/drivers/net/ethernet/realtek/r8169.c
> +++ b/drivers/net/ethernet/realtek/r8169.c
> @@ -6194,6 +6194,9 @@ static void rtl_shutdown(struct pci_dev *pdev)
>  {
>  	struct net_device *dev = pci_get_drvdata(pdev);
>  	struct rtl8169_private *tp = netdev_priv(dev);
> +	struct device *d = &pdev->dev;
> +
> +	pm_runtime_get_sync(d);
>  
>  	rtl8169_net_suspend(dev);
>  
> @@ -6215,6 +6218,8 @@ static void rtl_shutdown(struct pci_dev *pdev)
>  		pci_wake_from_d3(pdev, true);
>  		pci_set_power_state(pdev, PCI_D3hot);
>  	}
> +
> +	pm_runtime_put_noidle(d);
>  }
>  
>  static struct pci_driver rtl8169_pci_driver = {
> 

--
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 - March 6, 2012, 8:15 p.m.
From: Francois Romieu <romieu@fr.zoreil.com>
Date: Tue, 6 Mar 2012 12:14:12 +0100

> With runtime PM, if the ethernet cable is disconnected, the device is
> transitioned to D3 state to conserve energy. If the system is shutdown
> in this state, any register accesses in rtl_shutdown are dropped on
> the floor. As the device was programmed by .runtime_suspend() to wake
> on link changes, it is thus brought back up as soon as the link recovers.
> 
> Resuming every suspended device through the driver core would slow things
> down and it is not clear how many devices really need it now.
> 
> Original report and D0 transition patch by Sameer Nanda. Patch has been
> changed to comply with advices by Rafael J. Wysocki and the PM folks.
> 
> Reported-by: Sameer Nanda <snanda@chromium.org>
> Signed-off-by: Francois Romieu <romieu@fr.zoreil.com>
> Cc: Rafael J. Wysocki <rjw@sisk.pl>
> Cc: Hayes Wang <hayeswang@realtek.com>
> Cc: Alan Stern <stern@rowland.harvard.edu>
> ---
> 
>  This sucker turned my silent wake-on-alarm workstation into an early
>  sunday morning testbed takeoff. Please consider before 3.3 goes out.

Applied.
--
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

diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index 1adf177..bbacb37 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -6194,6 +6194,9 @@  static void rtl_shutdown(struct pci_dev *pdev)
 {
 	struct net_device *dev = pci_get_drvdata(pdev);
 	struct rtl8169_private *tp = netdev_priv(dev);
+	struct device *d = &pdev->dev;
+
+	pm_runtime_get_sync(d);
 
 	rtl8169_net_suspend(dev);
 
@@ -6215,6 +6218,8 @@  static void rtl_shutdown(struct pci_dev *pdev)
 		pci_wake_from_d3(pdev, true);
 		pci_set_power_state(pdev, PCI_D3hot);
 	}
+
+	pm_runtime_put_noidle(d);
 }
 
 static struct pci_driver rtl8169_pci_driver = {