lkml.org 
[lkml]   [2013]   [Jan]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH 1/5] e1000e: fix resuming from runtime-suspend
Date
On Tuesday, January 29, 2013 10:32:14 AM Konstantin Khlebnikov wrote:
> Rafael J. Wysocki wrote:
> > On Monday, January 28, 2013 04:05:33 PM Bjorn Helgaas wrote:
> >> [+cc Rafael, author of patch you cited]
> >>
> >> On Fri, Jan 18, 2013 at 4:42 AM, Konstantin Khlebnikov
> >> <khlebnikov@openvz.org> wrote:
> >>> Bug was introduced in commit 23606cf5d1192c2b17912cb2ef6e62f9b11de133
> >>> ("e1000e / PCI / PM: Add basic runtime PM support (rev. 4)") in v2.6.35
> >>>
> >>> Signed-off-by: Konstantin Khlebnikov<khlebnikov@openvz.org>
> >>> Cc: e1000-devel@lists.sourceforge.net
> >>> Cc: Jeff Kirsher<jeffrey.t.kirsher@intel.com>
> >>> Cc: Bruce Allan<bruce.w.allan@intel.com>
> >>> ---
> >>> drivers/net/ethernet/intel/e1000e/netdev.c | 13 ++++++++-----
> >>> 1 file changed, 8 insertions(+), 5 deletions(-)
> >>>
> >>> diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
> >>> index fbf75fd..2853c11 100644
> >>> --- a/drivers/net/ethernet/intel/e1000e/netdev.c
> >>> +++ b/drivers/net/ethernet/intel/e1000e/netdev.c
> >>> @@ -5691,14 +5691,17 @@ static int e1000_runtime_suspend(struct device *dev)
> >>> struct pci_dev *pdev = to_pci_dev(dev);
> >>> struct net_device *netdev = pci_get_drvdata(pdev);
> >>> struct e1000_adapter *adapter = netdev_priv(netdev);
> >>> + int retval;
> >>> + bool wake;
> >>>
> >>> - if (e1000e_pm_ready(adapter)) {
> >>> - bool wake;
> >>> + if (!e1000e_pm_ready(adapter))
> >>> + return 0;
> >>>
> >>> - __e1000_shutdown(pdev,&wake, true);
> >>> - }
> >>> + retval = __e1000_shutdown(pdev,&wake, true);
> >>> + if (!retval)
> >>> + e1000_power_off(pdev, true, wake);
> >>>
> >>> - return 0;
> >>> + return retval;
> >>> }
> >>>
> >>> static int e1000_idle(struct device *dev)
> >>>
> >
> > I'd like the changelog to say what the bug is and how it is being fixed in
> > general terms.
>
> Ok, my bad.
>
> Problem: ethernet device does not work (no carrier signal).
> Right after boot it goes to runtime-suspend and never wake up.
>
> Original code (before your commit) calls pci_prepare_to_sleep() and it
> calls pci_enable_wake() and switches device to one of D3 state.
>
> It seems redundant, because pci_pm_runtime_suspend() do the same thing
> after calling ->runtime_suspend callback. Or rather it did it before commit
> 42eca2302146fed51335b95128e949ee6f54478f ("PCI: Don't touch card regs after
> runtime suspend D3") and third patch aimed fix this damage.
>
> More over seems like calling pci_enable_wake() from e1000e isn't enough for my case,
> because my enthernet cannot wakeup from runtime-suspend without third patch.
> Seems like it's because pci_enable_wake() and pci_finish_runtime_suspend()
> calls different pratform-pm callbacks -- platform_pci_run_wake() /
> platform_pci_sleep_wake().
>
> All this looks messy and I don't know how it should work.
>
> If you prefer to minimize changes -- I can test how it would work without
> first (this) patch. Probably fine.

OK, I need to have a look at that driver, then. Please give me some time,
though, I'm working on something different at the moment.

And by the way, by sending patches you declare that you know what you're doing.
If you really don't, it's better to just describe the problem. :-)

Thanks,
Rafael


--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.


\
 
 \ /
  Last update: 2013-01-29 13:41    [W:0.078 / U:0.152 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site