lkml.org 
[lkml]   [2013]   [Jan]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH 3/5] PCI: revert preparing for wakeup in runtime-suspend finalization
[+cc Rafael]

On Fri, Jan 18, 2013 at 4:42 AM, Konstantin Khlebnikov
<khlebnikov@openvz.org> wrote:
> This patch effectively reverts commit 42eca2302146fed51335b95128e949ee6f54478f
> ("PCI: Don't touch card regs after runtime suspend D3")
>
> | This patch checks whether the pci state is saved and doesn't attempt to hit
> | any registers after that point if it is.
>
> This seems completely wrong. Yes, PCI configuration space has been saved by
> driver, but this doesn't means that all job is done and device has been
> suspended and ready for waking up in the future.
>
> For example driver e1000e for ethernet in my thinkpad x220 saves pci-state
> but device cannot wakeup after that, because it needs some ACPI callbacks
> which usually called from pci_finish_runtime_suspend().
>
> | Optimus (dual-gpu) laptops seem to have their own form of D3cold, but
> | unfortunately enter it on normal D3 transitions via the ACPI callback.
>
> Hardware which disappears from the bus unexpectedly is exception, so let's
> handle it as an exception. Its driver should set device state to D3cold and
> the rest code will handle it properly.

Functions in D3cold don't have power, so it's completely expected that
they would disappear from the bus and not respond to config accesses.
Maybe Dave was referring to D3hot, where functions *should* respond to
config accesses. I dunno.

Just to be clear, it sounds like 42eca230 caused a regression on your
e1000e device? If so, I guess we should revert it unless you and Dave
can figure out a better patch that fixes both your e1000e device and
the Optimus issue.

> Signed-off-by: Konstantin Khlebnikov <khlebnikov@openvz.org>
> Cc: linux-pci@vger.kernel.org
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Dave Airlie <airlied@redhat.com>
> ---
> drivers/pci/pci-driver.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
> index f79cbcd..030dbf0 100644
> --- a/drivers/pci/pci-driver.c
> +++ b/drivers/pci/pci-driver.c
> @@ -1005,10 +1005,11 @@ static int pci_pm_runtime_suspend(struct device *dev)
> return 0;
> }
>
> - if (!pci_dev->state_saved) {
> + if (!pci_dev->state_saved)
> pci_save_state(pci_dev);
> +
> + if (pci_dev->current_state != PCI_D3cold)
> pci_finish_runtime_suspend(pci_dev);
> - }
>
> return 0;
> }
>


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