From: Rafael J. Wysocki <[email protected]>
The ath9k driver uses the legacy PCI power management (suspend
and resume) callbacks that apparently cause intermittent problems
to happen (the adapter sometimes doesn't resume correctly on my
Acer Ferrari One). Make it use the new PCI PM and let the PCI core
code handle the PCI-specific details of power transitions.
Signed-off-by: Rafael J. Wysocki <[email protected]>
---
drivers/net/wireless/ath/ath9k/pci.c | 40 +++++++++++++++++++----------------
1 file changed, 22 insertions(+), 18 deletions(-)
Index: linux-2.6/drivers/net/wireless/ath/ath9k/pci.c
===================================================================
--- linux-2.6.orig/drivers/net/wireless/ath/ath9k/pci.c
+++ linux-2.6/drivers/net/wireless/ath/ath9k/pci.c
@@ -247,34 +247,25 @@ static void ath_pci_remove(struct pci_de
#ifdef CONFIG_PM
-static int ath_pci_suspend(struct pci_dev *pdev, pm_message_t state)
+static int ath_pci_suspend(struct device *device)
{
+ struct pci_dev *pdev = to_pci_dev(device);
struct ieee80211_hw *hw = pci_get_drvdata(pdev);
struct ath_wiphy *aphy = hw->priv;
struct ath_softc *sc = aphy->sc;
ath9k_hw_set_gpio(sc->sc_ah, sc->sc_ah->led_pin, 1);
- pci_save_state(pdev);
- pci_disable_device(pdev);
- pci_set_power_state(pdev, PCI_D3hot);
-
return 0;
}
-static int ath_pci_resume(struct pci_dev *pdev)
+static int ath_pci_resume(struct device *device)
{
+ struct pci_dev *pdev = to_pci_dev(device);
struct ieee80211_hw *hw = pci_get_drvdata(pdev);
struct ath_wiphy *aphy = hw->priv;
struct ath_softc *sc = aphy->sc;
u32 val;
- int err;
-
- pci_restore_state(pdev);
-
- err = pci_enable_device(pdev);
- if (err)
- return err;
/*
* Suspend/Resume resets the PCI configuration space, so we have to
@@ -293,7 +284,23 @@ static int ath_pci_resume(struct pci_dev
return 0;
}
-#endif /* CONFIG_PM */
+static const struct dev_pm_ops ath9k_pm_ops = {
+ .suspend = ath_pci_suspend,
+ .resume = ath_pci_resume,
+ .freeze = ath_pci_suspend,
+ .thaw = ath_pci_resume,
+ .poweroff = ath_pci_suspend,
+ .restore = ath_pci_resume,
+};
+
+#define ATH9K_PM_OPS (&ath9k_pm_ops)
+
+#else /* !CONFIG_PM */
+
+#define ATH9K_PM_OPS NULL
+
+#endif /* !CONFIG_PM */
+
MODULE_DEVICE_TABLE(pci, ath_pci_id_table);
@@ -302,10 +309,7 @@ static struct pci_driver ath_pci_driver
.id_table = ath_pci_id_table,
.probe = ath_pci_probe,
.remove = ath_pci_remove,
-#ifdef CONFIG_PM
- .suspend = ath_pci_suspend,
- .resume = ath_pci_resume,
-#endif /* CONFIG_PM */
+ .driver.pm = ATH9K_PM_OPS,
};
int ath_pci_init(void)
On Saturday, October 16, 2010, Luis R. Rodriguez wrote:
> On Fri, Oct 15, 2010 at 3:36 PM, Rafael J. Wysocki <[email protected]> wrote:
> > From: Rafael J. Wysocki <[email protected]>
> >
> > The ath9k driver uses the legacy PCI power management (suspend
> > and resume) callbacks that apparently cause intermittent problems
> > to happen (the adapter sometimes doesn't resume correctly on my
> > Acer Ferrari One). Make it use the new PCI PM and let the PCI core
> > code handle the PCI-specific details of power transitions.
> >
> > Signed-off-by: Rafael J. Wysocki <[email protected]>
>
> Do you know when these hooks were added?
Well, which ones? If you're asking about the new framework, it's been there
since 2.6.29 and is a recommended way of implementing PM hooks now.
Please have a look at Documentation/power/pci.txt. :-)
> If it fixes an issue should this go for stable?
I'm not 100% sure it fixes everything yet, although I hasn't been able to
reproduce the problem with it applied, so far.
So, I don't think it's -stable material at this point. However, it should be
done anyway at one point, so it very well can be done right now, given that I
have the hardware to test it. :-)
Thanks,
Rafael
On Fri, Oct 15, 2010 at 3:36 PM, Rafael J. Wysocki <[email protected]> wrote:
> From: Rafael J. Wysocki <[email protected]>
>
> The ath9k driver uses the legacy PCI power management (suspend
> and resume) callbacks that apparently cause intermittent problems
> to happen (the adapter sometimes doesn't resume correctly on my
> Acer Ferrari One). Make it use the new PCI PM and let the PCI core
> code handle the PCI-specific details of power transitions.
>
> Signed-off-by: Rafael J. Wysocki <[email protected]>
Do you know when these hooks were added? If it fixes an issue should
this go for stable?
Luis
On Fri, Oct 15, 2010 at 3:36 PM, Rafael J. Wysocki <[email protected]> wrote:
> From: Rafael J. Wysocki <[email protected]>
>
> The ath9k driver uses the legacy PCI power management (suspend
> and resume) callbacks that apparently cause intermittent problems
> to happen (the adapter sometimes doesn't resume correctly on my
> Acer Ferrari One). ?Make it use the new PCI PM and let the PCI core
> code handle the PCI-specific details of power transitions.
I'm curious. What sort of behavior did you get when the device fails
to resume correctly? Every now and then I've been getting bad
accesses to 0x7000 every now and then -- after that any reads to PCI
registers return all f's.
--
Paul
On Saturday, October 16, 2010, Paul Stewart wrote:
> On Fri, Oct 15, 2010 at 3:36 PM, Rafael J. Wysocki <[email protected]> wrote:
> > From: Rafael J. Wysocki <[email protected]>
> >
> > The ath9k driver uses the legacy PCI power management (suspend
> > and resume) callbacks that apparently cause intermittent problems
> > to happen (the adapter sometimes doesn't resume correctly on my
> > Acer Ferrari One). Make it use the new PCI PM and let the PCI core
> > code handle the PCI-specific details of power transitions.
> I'm curious. What sort of behavior did you get when the device fails
> to resume correctly? Every now and then I've been getting bad
> accesses to 0x7000 every now and then -- after that any reads to PCI
> registers return all f's.
Yes, this sort of matches my experience. Even the config space isn't
really accessible after a failure to resume.
Thanks,
Rafael
On Monday, December 20, 2010, Hauke Mehrtens wrote:
> On 12/20/2010 12:32 AM, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <[email protected]>
> >
> > The ath9k driver uses legacy PCI power management (system suspend
> > and resume) callbacks, so make it use struct dev_pm_ops callbacks and
> > let the PCI subsystem handle all of the PCI-specific details of
> > system power transitions.
> >
> > This change has been tested on Acer Ferrari One with the
> > Atheros AR928X PCI Express network adapter (rev. 01).
> >
> > Signed-off-by: Rafael J. Wysocki <[email protected]>
> > ---
> >
> > Hi,
> >
> > This patch has been sent already once under a slightly different subject
> > and with a different changelog. I'm not sure what happened to it that time,
> > so here it goes again.
> >
> > I'm quite confident it doesn't break things and it simplifies the driver's
> > suspend and resume routines quite a bit, so please apply.
> >
> > Thanks,
> > Rafael
> >
> Hi Rafael,
>
> Your patch was already applied (see commit
> f0e94b479c987abef17eb18e5c8e0ed178d00cd4 in linux-next) or didn't I got
> your problem?
Oh, is it? Sorry for the noise, then.
Thanks,
Rafael
From: Rafael J. Wysocki <[email protected]>
The ath9k driver uses legacy PCI power management (system suspend
and resume) callbacks, so make it use struct dev_pm_ops callbacks and
let the PCI subsystem handle all of the PCI-specific details of
system power transitions.
This change has been tested on Acer Ferrari One with the
Atheros AR928X PCI Express network adapter (rev. 01).
Signed-off-by: Rafael J. Wysocki <[email protected]>
---
Hi,
This patch has been sent already once under a slightly different subject
and with a different changelog. I'm not sure what happened to it that time,
so here it goes again.
I'm quite confident it doesn't break things and it simplifies the driver's
suspend and resume routines quite a bit, so please apply.
Thanks,
Rafael
---
drivers/net/wireless/ath/ath9k/pci.c | 32 ++++++++++++++------------------
1 file changed, 14 insertions(+), 18 deletions(-)
Index: linux-2.6/drivers/net/wireless/ath/ath9k/pci.c
===================================================================
--- linux-2.6.orig/drivers/net/wireless/ath/ath9k/pci.c
+++ linux-2.6/drivers/net/wireless/ath/ath9k/pci.c
@@ -247,34 +247,25 @@ static void ath_pci_remove(struct pci_de
#ifdef CONFIG_PM
-static int ath_pci_suspend(struct pci_dev *pdev, pm_message_t state)
+static int ath_pci_suspend(struct device *device)
{
+ struct pci_dev *pdev = to_pci_dev(device);
struct ieee80211_hw *hw = pci_get_drvdata(pdev);
struct ath_wiphy *aphy = hw->priv;
struct ath_softc *sc = aphy->sc;
ath9k_hw_set_gpio(sc->sc_ah, sc->sc_ah->led_pin, 1);
- pci_save_state(pdev);
- pci_disable_device(pdev);
- pci_set_power_state(pdev, PCI_D3hot);
-
return 0;
}
-static int ath_pci_resume(struct pci_dev *pdev)
+static int ath_pci_resume(struct device *device)
{
+ struct pci_dev *pdev = to_pci_dev(device);
struct ieee80211_hw *hw = pci_get_drvdata(pdev);
struct ath_wiphy *aphy = hw->priv;
struct ath_softc *sc = aphy->sc;
u32 val;
- int err;
-
- pci_restore_state(pdev);
-
- err = pci_enable_device(pdev);
- if (err)
- return err;
/*
* Suspend/Resume resets the PCI configuration space, so we have to
@@ -293,7 +284,15 @@ static int ath_pci_resume(struct pci_dev
return 0;
}
-#endif /* CONFIG_PM */
+static SIMPLE_DEV_PM_OPS(ath9k_pm_ops, ath_pci_suspend, ath_pci_resume);
+#define ATH9K_PM_OPS (&ath9k_pm_ops)
+
+#else /* !CONFIG_PM */
+
+#define ATH9K_PM_OPS NULL
+
+#endif /* !CONFIG_PM */
+
MODULE_DEVICE_TABLE(pci, ath_pci_id_table);
@@ -302,10 +301,7 @@ static struct pci_driver ath_pci_driver
.id_table = ath_pci_id_table,
.probe = ath_pci_probe,
.remove = ath_pci_remove,
-#ifdef CONFIG_PM
- .suspend = ath_pci_suspend,
- .resume = ath_pci_resume,
-#endif /* CONFIG_PM */
+ .driver.pm = ATH9K_PM_OPS,
};
int ath_pci_init(void)
On 12/20/2010 12:32 AM, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <[email protected]>
>
> The ath9k driver uses legacy PCI power management (system suspend
> and resume) callbacks, so make it use struct dev_pm_ops callbacks and
> let the PCI subsystem handle all of the PCI-specific details of
> system power transitions.
>
> This change has been tested on Acer Ferrari One with the
> Atheros AR928X PCI Express network adapter (rev. 01).
>
> Signed-off-by: Rafael J. Wysocki <[email protected]>
> ---
>
> Hi,
>
> This patch has been sent already once under a slightly different subject
> and with a different changelog. I'm not sure what happened to it that time,
> so here it goes again.
>
> I'm quite confident it doesn't break things and it simplifies the driver's
> suspend and resume routines quite a bit, so please apply.
>
> Thanks,
> Rafael
>
Hi Rafael,
Your patch was already applied (see commit
f0e94b479c987abef17eb18e5c8e0ed178d00cd4 in linux-next) or didn't I got
your problem?
Hauke