* [PATCH 0/3] Adapt three network drivers to the reworked PCI PM
@ 2008-08-14 21:35 Rafael J. Wysocki
2008-08-14 21:37 ` [PATCH 1/3] Adapt the skge driver " Rafael J. Wysocki
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Rafael J. Wysocki @ 2008-08-14 21:35 UTC (permalink / raw)
To: netdev
Cc: Andrew Morton, Jeff Garzik, Jesse Barnes, LKML, pm list,
Brandeburg, Jesse, Stephen Hemminger
Hi,
After the PCI PM core code has been updated, PCI network drivers (especially
those using pci_enable_wake() for setting up WOL) should be adapted to it.
This series of patches modifies three drivers, skge, sky2 and e100, for this
purpose.
[Note for Andrew: Patch 1/3 has been in -mm for some time, but the present one
has a better changelog (IMO).]
Please consider for applying.
Thanks,
Rafael
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/3] Adapt the skge driver to the reworked PCI PM
2008-08-14 21:35 [PATCH 0/3] Adapt three network drivers to the reworked PCI PM Rafael J. Wysocki
@ 2008-08-14 21:37 ` Rafael J. Wysocki
2008-08-14 21:38 ` [PATCH 2/3] Adapt the sky2 " Rafael J. Wysocki
2008-08-14 21:40 ` [PATCH 3/3] Adapt the e100 " Rafael J. Wysocki
2 siblings, 0 replies; 10+ messages in thread
From: Rafael J. Wysocki @ 2008-08-14 21:37 UTC (permalink / raw)
To: netdev
Cc: Andrew Morton, Jeff Garzik, Jesse Barnes, LKML, pm list,
Brandeburg, Jesse, Stephen Hemminger
From: Rafael J. Wysocki <rjw@sisk.pl>
Adapt the skge driver to the reworked PCI PM
* Use device_set_wakeup_enable() and friends as needed
* Remove an open-coded reference to the standard PCI PM registers
* Use pci_prepare_to_sleep() and pci_back_from_sleep() in the
->suspend() and ->resume() callbacks
* Use the observation that it is sufficient to call pci_enable_wake()
once, unless it fails
Tested on Asus L5D (Yukon-Lite rev 7).
Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
drivers/net/skge.c | 38 +++++++++++---------------------------
1 file changed, 11 insertions(+), 27 deletions(-)
Index: linux-next/drivers/net/skge.c
===================================================================
--- linux-next.orig/drivers/net/skge.c
+++ linux-next/drivers/net/skge.c
@@ -149,24 +149,6 @@ static u32 wol_supported(const struct sk
return WAKE_MAGIC | WAKE_PHY;
}
-static u32 pci_wake_enabled(struct pci_dev *dev)
-{
- int pm = pci_find_capability(dev, PCI_CAP_ID_PM);
- u16 value;
-
- /* If device doesn't support PM Capabilities, but request is to disable
- * wake events, it's a nop; otherwise fail */
- if (!pm)
- return 0;
-
- pci_read_config_word(dev, pm + PCI_PM_PMC, &value);
-
- value &= PCI_PM_CAP_PME_MASK;
- value >>= ffs(PCI_PM_CAP_PME_MASK) - 1; /* First bit of mask */
-
- return value != 0;
-}
-
static void skge_wol_init(struct skge_port *skge)
{
struct skge_hw *hw = skge->hw;
@@ -254,10 +236,14 @@ static int skge_set_wol(struct net_devic
struct skge_port *skge = netdev_priv(dev);
struct skge_hw *hw = skge->hw;
- if (wol->wolopts & ~wol_supported(hw))
+ if ((wol->wolopts & ~wol_supported(hw))
+ || !device_can_wakeup(&hw->pdev->dev))
return -EOPNOTSUPP;
skge->wol = wol->wolopts;
+
+ device_set_wakeup_enable(&hw->pdev->dev, skge->wol);
+
return 0;
}
@@ -3842,7 +3828,7 @@ static struct net_device *skge_devinit(s
skge->speed = -1;
skge->advertising = skge_supported_modes(hw);
- if (pci_wake_enabled(hw->pdev))
+ if (device_may_wakeup(&hw->pdev->dev))
skge->wol = wol_supported(hw) & WAKE_MAGIC;
hw->dev[port] = dev;
@@ -4068,8 +4054,8 @@ static int skge_suspend(struct pci_dev *
}
skge_write32(hw, B0_IMSK, 0);
- pci_enable_wake(pdev, pci_choose_state(pdev, state), wol);
- pci_set_power_state(pdev, pci_choose_state(pdev, state));
+
+ pci_prepare_to_sleep(pdev);
return 0;
}
@@ -4082,7 +4068,7 @@ static int skge_resume(struct pci_dev *p
if (!hw)
return 0;
- err = pci_set_power_state(pdev, PCI_D0);
+ err = pci_back_from_sleep(pdev);
if (err)
goto out;
@@ -4090,8 +4076,6 @@ static int skge_resume(struct pci_dev *p
if (err)
goto out;
- pci_enable_wake(pdev, PCI_D0, 0);
-
err = skge_reset(hw);
if (err)
goto out;
@@ -4132,8 +4116,8 @@ static void skge_shutdown(struct pci_dev
wol |= skge->wol;
}
- pci_enable_wake(pdev, PCI_D3hot, wol);
- pci_enable_wake(pdev, PCI_D3cold, wol);
+ if (pci_enable_wake(pdev, PCI_D3cold, wol))
+ pci_enable_wake(pdev, PCI_D3hot, wol);
pci_disable_device(pdev);
pci_set_power_state(pdev, PCI_D3hot);
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/3] Adapt the sky2 driver to the reworked PCI PM
2008-08-14 21:35 [PATCH 0/3] Adapt three network drivers to the reworked PCI PM Rafael J. Wysocki
2008-08-14 21:37 ` [PATCH 1/3] Adapt the skge driver " Rafael J. Wysocki
@ 2008-08-14 21:38 ` Rafael J. Wysocki
2008-08-14 22:23 ` Stephen Hemminger
2008-08-14 21:40 ` [PATCH 3/3] Adapt the e100 " Rafael J. Wysocki
2 siblings, 1 reply; 10+ messages in thread
From: Rafael J. Wysocki @ 2008-08-14 21:38 UTC (permalink / raw)
To: netdev
Cc: Andrew Morton, Jeff Garzik, Jesse Barnes, LKML, pm list,
Brandeburg, Jesse, Stephen Hemminger
Adapt the sky2 driver to the reworked PCI PM
* Use device_set_wakeup_enable() and friends as needed
* Remove an open-coded reference to the standard PCI PM registers
* Use pci_prepare_to_sleep() and pci_back_from_sleep() in the
->suspend() and ->resume() callbacks
* Use the observation that it is sufficient to call pci_enable_wake()
once, unless it fails
Tested on Asus M3A32-MVP (Yukon-2 EC Ultra rev 3).
Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
drivers/net/sky2.c | 34 ++++++++++------------------------
1 file changed, 10 insertions(+), 24 deletions(-)
Index: linux-2.6/drivers/net/sky2.c
===================================================================
--- linux-2.6.orig/drivers/net/sky2.c
+++ linux-2.6/drivers/net/sky2.c
@@ -3035,7 +3035,8 @@ static int sky2_set_wol(struct net_devic
struct sky2_port *sky2 = netdev_priv(dev);
struct sky2_hw *hw = sky2->hw;
- if (wol->wolopts & ~sky2_wol_supported(sky2->hw))
+ if ((wol->wolopts & ~sky2_wol_supported(sky2->hw))
+ || !device_can_wakeup(&hw->pdev->dev))
return -EOPNOTSUPP;
sky2->wol = wol->wolopts;
@@ -3046,6 +3047,8 @@ static int sky2_set_wol(struct net_devic
sky2_write32(hw, B0_CTST, sky2->wol
? Y2_HW_WOL_ON : Y2_HW_WOL_OFF);
+ device_set_wakeup_enable(&hw->pdev->dev, sky2->wol);
+
if (!netif_running(dev))
sky2_wol_init(sky2);
return 0;
@@ -4167,18 +4170,6 @@ static int __devinit sky2_test_msi(struc
return err;
}
-static int __devinit pci_wake_enabled(struct pci_dev *dev)
-{
- int pm = pci_find_capability(dev, PCI_CAP_ID_PM);
- u16 value;
-
- if (!pm)
- return 0;
- if (pci_read_config_word(dev, pm + PCI_PM_CTRL, &value))
- return 0;
- return value & PCI_PM_CTRL_PME_ENABLE;
-}
-
/* This driver supports yukon2 chipset only */
static const char *sky2_name(u8 chipid, char *buf, int sz)
{
@@ -4239,7 +4230,7 @@ static int __devinit sky2_probe(struct p
}
}
- wol_default = pci_wake_enabled(pdev) ? WAKE_MAGIC : 0;
+ wol_default = device_may_wakeup(&pdev->dev) ? WAKE_MAGIC : 0;
err = -ENOMEM;
hw = kzalloc(sizeof(*hw), GFP_KERNEL);
@@ -4404,7 +4395,7 @@ static void __devexit sky2_remove(struct
static int sky2_suspend(struct pci_dev *pdev, pm_message_t state)
{
struct sky2_hw *hw = pci_get_drvdata(pdev);
- int i, wol = 0;
+ int i;
if (!hw)
return 0;
@@ -4422,8 +4413,6 @@ static int sky2_suspend(struct pci_dev *
if (sky2->wol)
sky2_wol_init(sky2);
-
- wol |= sky2->wol;
}
sky2_write32(hw, B0_IMSK, 0);
@@ -4431,8 +4420,7 @@ static int sky2_suspend(struct pci_dev *
sky2_power_aux(hw);
pci_save_state(pdev);
- pci_enable_wake(pdev, pci_choose_state(pdev, state), wol);
- pci_set_power_state(pdev, pci_choose_state(pdev, state));
+ pci_prepare_to_sleep(pdev);
return 0;
}
@@ -4445,7 +4433,7 @@ static int sky2_resume(struct pci_dev *p
if (!hw)
return 0;
- err = pci_set_power_state(pdev, PCI_D0);
+ err = pci_back_from_sleep(pdev);
if (err)
goto out;
@@ -4453,8 +4441,6 @@ static int sky2_resume(struct pci_dev *p
if (err)
goto out;
- pci_enable_wake(pdev, PCI_D0, 0);
-
/* Re-enable all clocks */
if (hw->chip_id == CHIP_ID_YUKON_EX ||
hw->chip_id == CHIP_ID_YUKON_EC_U ||
@@ -4513,8 +4499,8 @@ static void sky2_shutdown(struct pci_dev
if (wol)
sky2_power_aux(hw);
- pci_enable_wake(pdev, PCI_D3hot, wol);
- pci_enable_wake(pdev, PCI_D3cold, wol);
+ if (pci_enable_wake(pdev, PCI_D3cold, wol))
+ pci_enable_wake(pdev, PCI_D3hot, wol);
pci_disable_device(pdev);
pci_set_power_state(pdev, PCI_D3hot);
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 3/3] Adapt the e100 driver to the reworked PCI PM
2008-08-14 21:35 [PATCH 0/3] Adapt three network drivers to the reworked PCI PM Rafael J. Wysocki
2008-08-14 21:37 ` [PATCH 1/3] Adapt the skge driver " Rafael J. Wysocki
2008-08-14 21:38 ` [PATCH 2/3] Adapt the sky2 " Rafael J. Wysocki
@ 2008-08-14 21:40 ` Rafael J. Wysocki
2008-08-14 23:56 ` Jeff Kirsher
2 siblings, 1 reply; 10+ messages in thread
From: Rafael J. Wysocki @ 2008-08-14 21:40 UTC (permalink / raw)
To: netdev
Cc: Andrew Morton, Jeff Garzik, Jesse Barnes, LKML, pm list,
Brandeburg, Jesse, Stephen Hemminger
[Note: I have no hardware to test this patch, but it seems pretty
straightforward.]
Adapt the e100 driver to the reworked PCI PM
* Use device_set_wakeup_enable() and friends as needed
* Use pci_pme_active() to clear PME_Status and disable PME#
* Use the observation that it is sufficient to call pci_enable_wake()
once, unless it fails
Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
drivers/net/e100.c | 20 +++++++++++---------
1 file changed, 11 insertions(+), 9 deletions(-)
Index: linux-2.6/drivers/net/e100.c
===================================================================
--- linux-2.6.orig/drivers/net/e100.c
+++ linux-2.6/drivers/net/e100.c
@@ -2322,7 +2322,8 @@ static int e100_set_wol(struct net_devic
{
struct nic *nic = netdev_priv(netdev);
- if(wol->wolopts != WAKE_MAGIC && wol->wolopts != 0)
+ if ((wol->wolopts && wol->wolopts != WAKE_MAGIC) ||
+ !device_can_wakeup(&nic->pdev->dev))
return -EOPNOTSUPP;
if(wol->wolopts)
@@ -2330,6 +2331,8 @@ static int e100_set_wol(struct net_devic
else
nic->flags &= ~wol_magic;
+ device_set_wakeup_enable(&nic->pdev->dev, wol->wolopts);
+
e100_exec_cb(nic, NULL, e100_configure);
return 0;
@@ -2734,13 +2737,13 @@ static int __devinit e100_probe(struct p
/* Wol magic packet can be enabled from eeprom */
if((nic->mac >= mac_82558_D101_A4) &&
- (nic->eeprom[eeprom_id] & eeprom_id_wol))
+ (nic->eeprom[eeprom_id] & eeprom_id_wol)) {
nic->flags |= wol_magic;
+ device_set_wakeup_enable(&pdev->dev, true);
+ }
/* ack any pending wake events, disable PME */
- err = pci_enable_wake(pdev, 0, 0);
- if (err)
- DPRINTK(PROBE, ERR, "Error clearing wake event\n");
+ pci_pme_active(pdev, false);
strcpy(netdev->name, "eth%d");
if((err = register_netdev(netdev))) {
@@ -2796,11 +2799,10 @@ static int e100_suspend(struct pci_dev *
pci_save_state(pdev);
if ((nic->flags & wol_magic) | e100_asf(nic)) {
- pci_enable_wake(pdev, PCI_D3hot, 1);
- pci_enable_wake(pdev, PCI_D3cold, 1);
+ if (pci_enable_wake(pdev, PCI_D3cold, true))
+ pci_enable_wake(pdev, PCI_D3hot, true);
} else {
- pci_enable_wake(pdev, PCI_D3hot, 0);
- pci_enable_wake(pdev, PCI_D3cold, 0);
+ pci_enable_wake(pdev, PCI_D3hot, false);
}
pci_disable_device(pdev);
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] Adapt the sky2 driver to the reworked PCI PM
2008-08-14 21:38 ` [PATCH 2/3] Adapt the sky2 " Rafael J. Wysocki
@ 2008-08-14 22:23 ` Stephen Hemminger
2008-08-15 13:01 ` Rafael J. Wysocki
0 siblings, 1 reply; 10+ messages in thread
From: Stephen Hemminger @ 2008-08-14 22:23 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: netdev, Andrew Morton, Jeff Garzik, Jesse Barnes, LKML, pm list,
Brandeburg, Jesse, Stephen Hemminger
On Thu, 14 Aug 2008 23:38:17 +0200
"Rafael J. Wysocki" <rjw@sisk.pl> wrote:
> Adapt the sky2 driver to the reworked PCI PM
>
> * Use device_set_wakeup_enable() and friends as needed
> * Remove an open-coded reference to the standard PCI PM registers
> * Use pci_prepare_to_sleep() and pci_back_from_sleep() in the
> ->suspend() and ->resume() callbacks
> * Use the observation that it is sufficient to call pci_enable_wake()
> once, unless it fails
>
> Tested on Asus M3A32-MVP (Yukon-2 EC Ultra rev 3).
>
> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> ---
> drivers/net/sky2.c | 34 ++++++++++------------------------
> 1 file changed, 10 insertions(+), 24 deletions(-)
>
> Index: linux-2.6/drivers/net/sky2.c
> ===================================================================
> --- linux-2.6.orig/drivers/net/sky2.c
> +++ linux-2.6/drivers/net/sky2.c
> @@ -3035,7 +3035,8 @@ static int sky2_set_wol(struct net_devic
> struct sky2_port *sky2 = netdev_priv(dev);
> struct sky2_hw *hw = sky2->hw;
>
> - if (wol->wolopts & ~sky2_wol_supported(sky2->hw))
> + if ((wol->wolopts & ~sky2_wol_supported(sky2->hw))
> + || !device_can_wakeup(&hw->pdev->dev))
> return -EOPNOTSUPP;
>
There was a regression in earlier releases caused because some BIOS's
are wrong, and device can wakeup.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/3] Adapt the e100 driver to the reworked PCI PM
2008-08-14 21:40 ` [PATCH 3/3] Adapt the e100 " Rafael J. Wysocki
@ 2008-08-14 23:56 ` Jeff Kirsher
0 siblings, 0 replies; 10+ messages in thread
From: Jeff Kirsher @ 2008-08-14 23:56 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: netdev, Andrew Morton, Jeff Garzik, Jesse Barnes, LKML, pm list,
Brandeburg, Jesse, Stephen Hemminger
On Thu, Aug 14, 2008 at 2:40 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> [Note: I have no hardware to test this patch, but it seems pretty
> straightforward.]
>
> Adapt the e100 driver to the reworked PCI PM
>
> * Use device_set_wakeup_enable() and friends as needed
> * Use pci_pme_active() to clear PME_Status and disable PME#
> * Use the observation that it is sufficient to call pci_enable_wake()
> once, unless it fails
>
> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> ---
> drivers/net/e100.c | 20 +++++++++++---------
> 1 file changed, 11 insertions(+), 9 deletions(-)
>
We have tested the patch in our labs and systems wake from S5 just fine.
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
--
Cheers,
Jeff
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] Adapt the sky2 driver to the reworked PCI PM
2008-08-14 22:23 ` Stephen Hemminger
@ 2008-08-15 13:01 ` Rafael J. Wysocki
2008-08-15 13:07 ` Rafael J. Wysocki
2008-08-15 13:32 ` Rafael J. Wysocki
0 siblings, 2 replies; 10+ messages in thread
From: Rafael J. Wysocki @ 2008-08-15 13:01 UTC (permalink / raw)
To: Stephen Hemminger
Cc: netdev, Andrew Morton, Jeff Garzik, Jesse Barnes, LKML, pm list,
Brandeburg, Jesse, Stephen Hemminger
On Friday, 15 of August 2008, Stephen Hemminger wrote:
> On Thu, 14 Aug 2008 23:38:17 +0200
> "Rafael J. Wysocki" <rjw@sisk.pl> wrote:
>
> > Adapt the sky2 driver to the reworked PCI PM
> >
> > * Use device_set_wakeup_enable() and friends as needed
> > * Remove an open-coded reference to the standard PCI PM registers
> > * Use pci_prepare_to_sleep() and pci_back_from_sleep() in the
> > ->suspend() and ->resume() callbacks
> > * Use the observation that it is sufficient to call pci_enable_wake()
> > once, unless it fails
> >
> > Tested on Asus M3A32-MVP (Yukon-2 EC Ultra rev 3).
> >
> > Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> > ---
> > drivers/net/sky2.c | 34 ++++++++++------------------------
> > 1 file changed, 10 insertions(+), 24 deletions(-)
> >
> > Index: linux-2.6/drivers/net/sky2.c
> > ===================================================================
> > --- linux-2.6.orig/drivers/net/sky2.c
> > +++ linux-2.6/drivers/net/sky2.c
> > @@ -3035,7 +3035,8 @@ static int sky2_set_wol(struct net_devic
> > struct sky2_port *sky2 = netdev_priv(dev);
> > struct sky2_hw *hw = sky2->hw;
> >
> > - if (wol->wolopts & ~sky2_wol_supported(sky2->hw))
> > + if ((wol->wolopts & ~sky2_wol_supported(sky2->hw))
> > + || !device_can_wakeup(&hw->pdev->dev))
> > return -EOPNOTSUPP;
> >
>
> There was a regression in earlier releases caused because some BIOS's
> are wrong, and device can wakeup.
device_can_wakeup() returns 'true' if the device is capable of generating PME#
from at least one low power state (this is determined on the basis of the
contents of the device's PCI PM registers - please have a look at
drivers/pci/pci_pm_init() for details) or if ACPI tells us it can wake up.
IOW, the BIOSes opinion doesn't matter if we find that the device is capable of
generating PME#, so the regression must have been related to something else.
I'm very interested in the details, if available.
Of course, since we rely on the ability of the device to generate PME# and
device_can_wakeup() returning 'false' means that the device cannot generate
PME# from any state, it's reasonable to check it IMO.
Thanks,
Rafael
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] Adapt the sky2 driver to the reworked PCI PM
2008-08-15 13:01 ` Rafael J. Wysocki
@ 2008-08-15 13:07 ` Rafael J. Wysocki
2008-08-15 13:32 ` Rafael J. Wysocki
1 sibling, 0 replies; 10+ messages in thread
From: Rafael J. Wysocki @ 2008-08-15 13:07 UTC (permalink / raw)
To: Stephen Hemminger
Cc: netdev, Andrew Morton, Jeff Garzik, Jesse Barnes, LKML, pm list,
Brandeburg, Jesse, Stephen Hemminger
On Friday, 15 of August 2008, Rafael J. Wysocki wrote:
> On Friday, 15 of August 2008, Stephen Hemminger wrote:
> > On Thu, 14 Aug 2008 23:38:17 +0200
> > "Rafael J. Wysocki" <rjw@sisk.pl> wrote:
> >
> > > Adapt the sky2 driver to the reworked PCI PM
> > >
> > > * Use device_set_wakeup_enable() and friends as needed
> > > * Remove an open-coded reference to the standard PCI PM registers
> > > * Use pci_prepare_to_sleep() and pci_back_from_sleep() in the
> > > ->suspend() and ->resume() callbacks
> > > * Use the observation that it is sufficient to call pci_enable_wake()
> > > once, unless it fails
> > >
> > > Tested on Asus M3A32-MVP (Yukon-2 EC Ultra rev 3).
> > >
> > > Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> > > ---
> > > drivers/net/sky2.c | 34 ++++++++++------------------------
> > > 1 file changed, 10 insertions(+), 24 deletions(-)
> > >
> > > Index: linux-2.6/drivers/net/sky2.c
> > > ===================================================================
> > > --- linux-2.6.orig/drivers/net/sky2.c
> > > +++ linux-2.6/drivers/net/sky2.c
> > > @@ -3035,7 +3035,8 @@ static int sky2_set_wol(struct net_devic
> > > struct sky2_port *sky2 = netdev_priv(dev);
> > > struct sky2_hw *hw = sky2->hw;
> > >
> > > - if (wol->wolopts & ~sky2_wol_supported(sky2->hw))
> > > + if ((wol->wolopts & ~sky2_wol_supported(sky2->hw))
> > > + || !device_can_wakeup(&hw->pdev->dev))
> > > return -EOPNOTSUPP;
> > >
> >
> > There was a regression in earlier releases caused because some BIOS's
> > are wrong, and device can wakeup.
>
> device_can_wakeup() returns 'true' if the device is capable of generating PME#
> from at least one low power state (this is determined on the basis of the
> contents of the device's PCI PM registers - please have a look at
> drivers/pci/pci_pm_init() for details) or if ACPI tells us it can wake up.
drivers/pci/pci.c:pci_pm_init(), sorry.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] Adapt the sky2 driver to the reworked PCI PM
2008-08-15 13:01 ` Rafael J. Wysocki
2008-08-15 13:07 ` Rafael J. Wysocki
@ 2008-08-15 13:32 ` Rafael J. Wysocki
2008-08-15 16:15 ` Stephen Hemminger
1 sibling, 1 reply; 10+ messages in thread
From: Rafael J. Wysocki @ 2008-08-15 13:32 UTC (permalink / raw)
To: Stephen Hemminger
Cc: netdev, Andrew Morton, Jeff Garzik, Jesse Barnes, LKML, pm list,
Brandeburg, Jesse, Stephen Hemminger
On Friday, 15 of August 2008, Rafael J. Wysocki wrote:
> On Friday, 15 of August 2008, Stephen Hemminger wrote:
> > On Thu, 14 Aug 2008 23:38:17 +0200
> > "Rafael J. Wysocki" <rjw@sisk.pl> wrote:
> >
> > > Adapt the sky2 driver to the reworked PCI PM
> > >
> > > * Use device_set_wakeup_enable() and friends as needed
> > > * Remove an open-coded reference to the standard PCI PM registers
> > > * Use pci_prepare_to_sleep() and pci_back_from_sleep() in the
> > > ->suspend() and ->resume() callbacks
> > > * Use the observation that it is sufficient to call pci_enable_wake()
> > > once, unless it fails
> > >
> > > Tested on Asus M3A32-MVP (Yukon-2 EC Ultra rev 3).
> > >
> > > Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> > > ---
> > > drivers/net/sky2.c | 34 ++++++++++------------------------
> > > 1 file changed, 10 insertions(+), 24 deletions(-)
> > >
> > > Index: linux-2.6/drivers/net/sky2.c
> > > ===================================================================
> > > --- linux-2.6.orig/drivers/net/sky2.c
> > > +++ linux-2.6/drivers/net/sky2.c
> > > @@ -3035,7 +3035,8 @@ static int sky2_set_wol(struct net_devic
> > > struct sky2_port *sky2 = netdev_priv(dev);
> > > struct sky2_hw *hw = sky2->hw;
> > >
> > > - if (wol->wolopts & ~sky2_wol_supported(sky2->hw))
> > > + if ((wol->wolopts & ~sky2_wol_supported(sky2->hw))
> > > + || !device_can_wakeup(&hw->pdev->dev))
> > > return -EOPNOTSUPP;
> > >
> >
> > There was a regression in earlier releases caused because some BIOS's
> > are wrong, and device can wakeup.
>
> device_can_wakeup() returns 'true' if the device is capable of generating PME#
> from at least one low power state (this is determined on the basis of the
> contents of the device's PCI PM registers - please have a look at
> drivers/pci/pci_pm_init() for details) or if ACPI tells us it can wake up.
> IOW, the BIOSes opinion doesn't matter if we find that the device is capable of
> generating PME#, so the regression must have been related to something else.
> I'm very interested in the details, if available.
>
> Of course, since we rely on the ability of the device to generate PME# and
> device_can_wakeup() returning 'false' means that the device cannot generate
> PME# from any state, it's reasonable to check it IMO.
BTW, if device_can_wakeup(&pdev->dev) returns 'false',
pci_enable_wake(pdev, state, true) will fail for any 'state', so the wake-up
won't work regardless of the change above.
Thanks,
Rafael
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] Adapt the sky2 driver to the reworked PCI PM
2008-08-15 13:32 ` Rafael J. Wysocki
@ 2008-08-15 16:15 ` Stephen Hemminger
0 siblings, 0 replies; 10+ messages in thread
From: Stephen Hemminger @ 2008-08-15 16:15 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: netdev, Andrew Morton, Jeff Garzik, Jesse Barnes, LKML, pm list,
Brandeburg, Jesse, Stephen Hemminger
On Fri, 15 Aug 2008 15:32:13 +0200
"Rafael J. Wysocki" <rjw@sisk.pl> wrote:
> On Friday, 15 of August 2008, Rafael J. Wysocki wrote:
> > On Friday, 15 of August 2008, Stephen Hemminger wrote:
> > > On Thu, 14 Aug 2008 23:38:17 +0200
> > > "Rafael J. Wysocki" <rjw@sisk.pl> wrote:
> > >
> > > > Adapt the sky2 driver to the reworked PCI PM
> > > >
> > > > * Use device_set_wakeup_enable() and friends as needed
> > > > * Remove an open-coded reference to the standard PCI PM registers
> > > > * Use pci_prepare_to_sleep() and pci_back_from_sleep() in the
> > > > ->suspend() and ->resume() callbacks
> > > > * Use the observation that it is sufficient to call pci_enable_wake()
> > > > once, unless it fails
> > > >
> > > > Tested on Asus M3A32-MVP (Yukon-2 EC Ultra rev 3).
> > > >
> > > > Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> > > > ---
> > > > drivers/net/sky2.c | 34 ++++++++++------------------------
> > > > 1 file changed, 10 insertions(+), 24 deletions(-)
> > > >
> > > > Index: linux-2.6/drivers/net/sky2.c
> > > > ===================================================================
> > > > --- linux-2.6.orig/drivers/net/sky2.c
> > > > +++ linux-2.6/drivers/net/sky2.c
> > > > @@ -3035,7 +3035,8 @@ static int sky2_set_wol(struct net_devic
> > > > struct sky2_port *sky2 = netdev_priv(dev);
> > > > struct sky2_hw *hw = sky2->hw;
> > > >
> > > > - if (wol->wolopts & ~sky2_wol_supported(sky2->hw))
> > > > + if ((wol->wolopts & ~sky2_wol_supported(sky2->hw))
> > > > + || !device_can_wakeup(&hw->pdev->dev))
> > > > return -EOPNOTSUPP;
> > > >
> > >
> > > There was a regression in earlier releases caused because some BIOS's
> > > are wrong, and device can wakeup.
> >
> > device_can_wakeup() returns 'true' if the device is capable of generating PME#
> > from at least one low power state (this is determined on the basis of the
> > contents of the device's PCI PM registers - please have a look at
> > drivers/pci/pci_pm_init() for details) or if ACPI tells us it can wake up.
> > IOW, the BIOSes opinion doesn't matter if we find that the device is capable of
> > generating PME#, so the regression must have been related to something else.
> > I'm very interested in the details, if available.
> >
> > Of course, since we rely on the ability of the device to generate PME# and
> > device_can_wakeup() returning 'false' means that the device cannot generate
> > PME# from any state, it's reasonable to check it IMO.
>
> BTW, if device_can_wakeup(&pdev->dev) returns 'false',
> pci_enable_wake(pdev, state, true) will fail for any 'state', so the wake-up
> won't work regardless of the change above.
>
> Thanks,
> Rafael
Okay, the problem was false positives from ACPI. It looks like this won't make
the problem any worse.
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2008-08-15 16:15 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-08-14 21:35 [PATCH 0/3] Adapt three network drivers to the reworked PCI PM Rafael J. Wysocki
2008-08-14 21:37 ` [PATCH 1/3] Adapt the skge driver " Rafael J. Wysocki
2008-08-14 21:38 ` [PATCH 2/3] Adapt the sky2 " Rafael J. Wysocki
2008-08-14 22:23 ` Stephen Hemminger
2008-08-15 13:01 ` Rafael J. Wysocki
2008-08-15 13:07 ` Rafael J. Wysocki
2008-08-15 13:32 ` Rafael J. Wysocki
2008-08-15 16:15 ` Stephen Hemminger
2008-08-14 21:40 ` [PATCH 3/3] Adapt the e100 " Rafael J. Wysocki
2008-08-14 23:56 ` Jeff Kirsher
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).