[PATCH 1/5] w1_gpio: switch to new dev_pm_ops

[PATCH 1/5] w1_gpio: switch to new dev_pm_ops

[Daniel Mack]

Signed-off-by: Daniel Mack <daniel@caiaq.de>
Cc: Ville Syrjala <syrjala@sci.fi>
Cc: Evgeniy Polyakov <johnpol@2ka.mipt.ru>
---
 drivers/w1/masters/w1-gpio.c |   23 +++++++++++++++--------
 1 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/drivers/w1/masters/w1-gpio.c b/drivers/w1/masters/w1-gpio.c
index 6f8866d..7099b11 100644
--- a/drivers/w1/masters/w1-gpio.c
+++ b/drivers/w1/masters/w1-gpio.c
@@ -106,9 +106,9 @@ static int __exit w1_gpio_remove(struct platform_device *pdev)
 
 #ifdef CONFIG_PM
 
-static int w1_gpio_suspend(struct platform_device *pdev, pm_message_t state)
+static int w1_gpio_suspend(struct device *dev)
 {
-	struct w1_gpio_platform_data *pdata = pdev->dev.platform_data;
+	struct w1_gpio_platform_data *pdata = dev->platform_data;
 
 	if (pdata->enable_external_pullup)
 		pdata->enable_external_pullup(0);
@@ -116,9 +116,9 @@ static int w1_gpio_suspend(struct platform_device *pdev, pm_message_t state)
 	return 0;
 }
 
-static int w1_gpio_resume(struct platform_device *pdev)
+static int w1_gpio_resume(struct device *dev)
 {
-	struct w1_gpio_platform_data *pdata = pdev->dev.platform_data;
+	struct w1_gpio_platform_data *pdata = dev->platform_data;
 
 	if (pdata->enable_external_pullup)
 		pdata->enable_external_pullup(1);
@@ -126,19 +126,26 @@ static int w1_gpio_resume(struct platform_device *pdev)
 	return 0;
 }
 
+static struct dev_pm_ops w1_gpio_pm_ops = {
+	.suspend	= w1_gpio_suspend,
+	.freeze		= w1_gpio_suspend,
+	.resume		= w1_gpio_resume,
+	.thaw		= w1_gpio_resume,
+};
+
+#define W1_GPIO_PM_OPS (&w1_gpio_pm_ops)
+
 #else
-#define w1_gpio_suspend	NULL
-#define w1_gpio_resume	NULL
+#define W1_GPIO_PM_OPS NULL
 #endif
 
 static struct platform_driver w1_gpio_driver = {
 	.driver = {
 		.name	= "w1-gpio",
 		.owner	= THIS_MODULE,
+		.pm	= W1_GPIO_PM_OPS,
 	},
 	.remove	= __exit_p(w1_gpio_remove),
-	.suspend = w1_gpio_suspend,
-	.resume = w1_gpio_resume,
 };
 
 static int __init w1_gpio_init(void)
-- 
1.6.3.3

[PATCH 2/5] pda-power: switch to new dev_pm_ops

[Daniel Mack]

Signed-off-by: Daniel Mack <daniel@caiaq.de>
Cc: Ian Molton <spyro@f2s.com>
Cc: Anton Vorontsov <cbou@mail.ru>
Cc: Matt Reimer <mreimer@vpop.net>
---
 drivers/power/pda_power.c |   27 ++++++++++++++++++---------
 1 files changed, 18 insertions(+), 9 deletions(-)

diff --git a/drivers/power/pda_power.c b/drivers/power/pda_power.c
index a232de6..34af275 100644
--- a/drivers/power/pda_power.c
+++ b/drivers/power/pda_power.c
@@ -402,9 +402,9 @@ static int pda_power_remove(struct platform_device *pdev)
 static int ac_wakeup_enabled;
 static int usb_wakeup_enabled;
 
-static int pda_power_suspend(struct platform_device *pdev, pm_message_t state)
+static int pda_power_suspend(struct device *dev)
 {
-	if (device_may_wakeup(&pdev->dev)) {
+	if (device_may_wakeup(dev)) {
 		if (ac_irq)
 			ac_wakeup_enabled = !enable_irq_wake(ac_irq->start);
 		if (usb_irq)
@@ -414,9 +414,9 @@ static int pda_power_suspend(struct platform_device *pdev, pm_message_t state)
 	return 0;
 }
 
-static int pda_power_resume(struct platform_device *pdev)
+static int pda_power_resume(struct device *dev)
 {
-	if (device_may_wakeup(&pdev->dev)) {
+	if (device_may_wakeup(dev)) {
 		if (usb_irq && usb_wakeup_enabled)
 			disable_irq_wake(usb_irq->start);
 		if (ac_irq && ac_wakeup_enabled)
@@ -425,21 +425,30 @@ static int pda_power_resume(struct platform_device *pdev)
 
 	return 0;
 }
+
+static struct dev_pm_ops pda_power_pm_ops = {
+	.suspend	= pda_power_suspend,
+	.freeze		= pda_power_freeze,
+	.resume		= pda_power_resume,
+	.thaw		= pda_power_resume,
+};
+
+#define PDA_POWER_PM_OPS (&pda_power_pm_ops)
+
 #else
-#define pda_power_suspend NULL
-#define pda_power_resume NULL
+#define PDA_POWER_PM_OPS NULL
 #endif /* CONFIG_PM */
 
 MODULE_ALIAS("platform:pda-power");
 
 static struct platform_driver pda_power_pdrv = {
 	.driver = {
-		.name = "pda-power",
+		.name	= "pda-power",
+		.owner	= THIS_MODULE,
+		.pm	= PDA_POWER_PM_OPS,
 	},
 	.probe = pda_power_probe,
 	.remove = pda_power_remove,
-	.suspend = pda_power_suspend,
-	.resume = pda_power_resume,
 };
 
 static int __init pda_power_init(void)
-- 
1.6.3.3

[PATCH 3/5] ds2760: switch to new dev_pm_ops

[Daniel Mack]

Signed-off-by: Daniel Mack <daniel@caiaq.de>
Cc: Szabolcs Gyurko <szabolcs.gyurko@tlt.hu>
Cc: Matt Reimer <mreimer@vpop.net>
Cc: Anton Vorontsov <cbou@mail.ru>
---
 drivers/power/ds2760_battery.c |   27 ++++++++++++++++-----------
 1 files changed, 16 insertions(+), 11 deletions(-)

diff --git a/drivers/power/ds2760_battery.c b/drivers/power/ds2760_battery.c
index ca43b1c..3c9d8cd 100644
--- a/drivers/power/ds2760_battery.c
+++ b/drivers/power/ds2760_battery.c
@@ -557,19 +557,18 @@ static int ds2760_battery_remove(struct platform_device *pdev)
 
 #ifdef CONFIG_PM
 
-static int ds2760_battery_suspend(struct platform_device *pdev,
-				  pm_message_t state)
+static int ds2760_battery_suspend(struct device *dev)
 {
-	struct ds2760_device_info *di = platform_get_drvdata(pdev);
+	struct ds2760_device_info *di = dev_get_drvdata(dev);
 
 	di->charge_status = POWER_SUPPLY_STATUS_UNKNOWN;
 
 	return 0;
 }
 
-static int ds2760_battery_resume(struct platform_device *pdev)
+static int ds2760_battery_resume(struct device *dev)
 {
-	struct ds2760_device_info *di = platform_get_drvdata(pdev);
+	struct ds2760_device_info *di = dev_get_drvdata(dev);
 
 	di->charge_status = POWER_SUPPLY_STATUS_UNKNOWN;
 	power_supply_changed(&di->bat);
@@ -580,23 +579,29 @@ static int ds2760_battery_resume(struct platform_device *pdev)
 	return 0;
 }
 
-#else
+static struct dev_pm_ops ds2760_battery_pm_ops = {
+	.suspend        = ds2760_battery_suspend,
+	.freeze		= ds2760_battery_suspend,
+	.resume         = ds2760_battery_resume,
+	.thaw		= ds2760_battery_resume,
+};
 
-#define ds2760_battery_suspend NULL
-#define ds2760_battery_resume NULL
+#define DS2760_PM_OPS (&ds2760_battery_pm_ops)
 
+#else
+#define DS2760_PM_OPS NULL
 #endif /* CONFIG_PM */
 
 MODULE_ALIAS("platform:ds2760-battery");
 
 static struct platform_driver ds2760_battery_driver = {
 	.driver = {
-		.name = "ds2760-battery",
+		.owner	= THIS_MODULE,
+		.name	= "ds2760-battery",
+		.pm	= DS2760_PM_OPS,
 	},
 	.probe	  = ds2760_battery_probe,
 	.remove   = ds2760_battery_remove,
-	.suspend  = ds2760_battery_suspend,
-	.resume	  = ds2760_battery_resume,
 };
 
 static int __init ds2760_battery_init(void)
-- 
1.6.3.3

[PATCH 4/5] input: gpio-keys: switch to new dev_pm_ops

[Daniel Mack]

Signed-off-by: Daniel Mack <daniel@caiaq.de>
Cc: Phil Blundell <pb@handhelds.org>
Cc: linux-input@vger.kernel.org
---
 drivers/input/keyboard/gpio_keys.c |   28 ++++++++++++++++++----------
 1 files changed, 18 insertions(+), 10 deletions(-)

diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c
index efed0c9..bfb6fc3 100644
--- a/drivers/input/keyboard/gpio_keys.c
+++ b/drivers/input/keyboard/gpio_keys.c
@@ -216,12 +216,12 @@ static int __devexit gpio_keys_remove(struct platform_device *pdev)
 
 
 #ifdef CONFIG_PM
-static int gpio_keys_suspend(struct platform_device *pdev, pm_message_t state)
+static int gpio_keys_suspend(struct device *dev)
 {
-	struct gpio_keys_platform_data *pdata = pdev->dev.platform_data;
+	struct gpio_keys_platform_data *pdata = dev->platform_data;
 	int i;
 
-	if (device_may_wakeup(&pdev->dev)) {
+	if (device_may_wakeup(dev)) {
 		for (i = 0; i < pdata->nbuttons; i++) {
 			struct gpio_keys_button *button = &pdata->buttons[i];
 			if (button->wakeup) {
@@ -234,12 +234,12 @@ static int gpio_keys_suspend(struct platform_device *pdev, pm_message_t state)
 	return 0;
 }
 
-static int gpio_keys_resume(struct platform_device *pdev)
+static int gpio_keys_resume(struct device *dev)
 {
-	struct gpio_keys_platform_data *pdata = pdev->dev.platform_data;
+	struct gpio_keys_platform_data *pdata = dev->platform_data;
 	int i;
 
-	if (device_may_wakeup(&pdev->dev)) {
+	if (device_may_wakeup(dev)) {
 		for (i = 0; i < pdata->nbuttons; i++) {
 			struct gpio_keys_button *button = &pdata->buttons[i];
 			if (button->wakeup) {
@@ -251,19 +251,27 @@ static int gpio_keys_resume(struct platform_device *pdev)
 
 	return 0;
 }
+
+static struct dev_pm_ops gpio_keys_pm_ops = {
+	.suspend	= gpio_keys_suspend,
+	.freeze		= gpio_keys_suspend,
+	.resume		= gpio_keys_resume,
+	.thaw		= gpio_keys_resume,
+};
+
+#define GPIO_KEYS_PM_OPS (&gpio_keys_pm_ops)
+
 #else
-#define gpio_keys_suspend	NULL
-#define gpio_keys_resume	NULL
+#define GPIO_KEYS_PM_OPS NULL
 #endif
 
 static struct platform_driver gpio_keys_device_driver = {
 	.probe		= gpio_keys_probe,
 	.remove		= __devexit_p(gpio_keys_remove),
-	.suspend	= gpio_keys_suspend,
-	.resume		= gpio_keys_resume,
 	.driver		= {
 		.name	= "gpio-keys",
 		.owner	= THIS_MODULE,
+		.pm	= GPIO_KEYS_PM_OPS,
 	}
 };
 
-- 
1.6.3.3

[PATCH 5/5] net: smsc911x: switch to new dev_pm_ops

[Daniel Mack]

Hibernation is unsupported for now, which meets the actual
implementation in the driver. For free/thaw, the chip's D2 state should
be entered.

Signed-off-by: Daniel Mack <daniel@caiaq.de>
Cc: Steve Glendinning <steve.glendinning@smsc.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: netdev@vger.kernel.org
---
 drivers/net/smsc911x.c |   31 ++++++++++++++++++++-----------
 1 files changed, 20 insertions(+), 11 deletions(-)

diff --git a/drivers/net/smsc911x.c b/drivers/net/smsc911x.c
index 94b6d26..c266785 100644
--- a/drivers/net/smsc911x.c
+++ b/drivers/net/smsc911x.c
@@ -50,6 +50,7 @@
 #include <linux/swab.h>
 #include <linux/phy.h>
 #include <linux/smsc911x.h>
+#include <linux/device.h>
 #include "smsc911x.h"
 
 #define SMSC_CHIPNAME		"smsc911x"
@@ -2114,10 +2115,12 @@ out_0:
 /* This implementation assumes the devices remains powered on its VDDVARIO
  * pins during suspend. */
 
-static int smsc911x_suspend(struct platform_device *pdev, pm_message_t state)
+/* TODO: implement freeze/thaw callbacks for hibernation.*/
+
+static int smsc911x_suspend(struct device *dev)
 {
-	struct net_device *dev = platform_get_drvdata(pdev);
-	struct smsc911x_data *pdata = netdev_priv(dev);
+	struct net_device *ndev = dev_get_drvdata(dev);
+	struct smsc911x_data *pdata = netdev_priv(ndev);
 
 	/* enable wake on LAN, energy detection and the external PME
 	 * signal. */
@@ -2128,10 +2131,10 @@ static int smsc911x_suspend(struct platform_device *pdev, pm_message_t state)
 	return 0;
 }
 
-static int smsc911x_resume(struct platform_device *pdev)
+static int smsc911x_resume(struct device *dev)
 {
-	struct net_device *dev = platform_get_drvdata(pdev);
-	struct smsc911x_data *pdata = netdev_priv(dev);
+	struct net_device *ndev = dev_get_drvdata(dev);
+	struct smsc911x_data *pdata = netdev_priv(ndev);
 	unsigned int to = 100;
 
 	/* Note 3.11 from the datasheet:
@@ -2149,19 +2152,25 @@ static int smsc911x_resume(struct platform_device *pdev)
 	return (to == 0) ? -EIO : 0;
 }
 
+static struct dev_pm_ops smsc911x_pm_ops = {
+	.suspend	= smsc911x_suspend,
+	.resume		= smsc911x_resume,
+};
+
+#define SMSC911X_PM_OPS (&smsc911x_pm_ops)
+
 #else
-#define smsc911x_suspend	NULL
-#define smsc911x_resume		NULL
+#define SMSC911X_PM_OPS NULL
 #endif
 
 static struct platform_driver smsc911x_driver = {
 	.probe = smsc911x_drv_probe,
 	.remove = __devexit_p(smsc911x_drv_remove),
 	.driver = {
-		.name = SMSC_CHIPNAME,
+		.name	= SMSC_CHIPNAME,
+		.owner	= THIS_MODULE,
+		.pm	= SMSC911X_PM_OPS,
 	},
-	.suspend = smsc911x_suspend,
-	.resume = smsc911x_resume,
 };
 
 /* Entry point for loading the module */
-- 
1.6.3.3

Re: [PATCH 5/5] net: smsc911x: switch to new dev_pm_ops

[David Miller]

From: Daniel Mack <daniel@caiaq.de>
Date: Wed,  5 Aug 2009 20:29:31 +0200

> Hibernation is unsupported for now, which meets the actual
> implementation in the driver. For free/thaw, the chip's D2 state should
> be entered.
> 
> Signed-off-by: Daniel Mack <daniel@caiaq.de>

Steve, if it looks good to you I can toss this into net-next-2.6

Just let me know.

Re: [PATCH 5/5] net: smsc911x: switch to new dev_pm_ops

[Steve.Glendinning]

David Miller <davem@davemloft.net> wrote on 06/08/2009 04:29:22:

> From: Daniel Mack <daniel@caiaq.de>
> Date: Wed,  5 Aug 2009 20:29:31 +0200
> 
> > Hibernation is unsupported for now, which meets the actual
> > implementation in the driver. For free/thaw, the chip's D2 state 
should
> > be entered.
> > 
> > Signed-off-by: Daniel Mack <daniel@caiaq.de>
> 
> Steve, if it looks good to you I can toss this into net-next-2.6
> 
> Just let me know.

Looks fine.  Compiles ok and doesn't actually change any PM 
implementation.

Acked-by: <steve.glendinning@smsc.com>

Unfortunately I can't test it.  The only dev platform I have in front of 
me
is SH7709S, which doesn't boot since 
4ff29ff8e8723a41e7defd8bc78a7b16cbf940a2
(the sh maintainers are aware and working on it:
http://marc.info/?l=linux-sh&m=124891631203530&w=2)

Steve

Re: [PATCH 5/5] net: smsc911x: switch to new dev_pm_ops

[David Miller]

From: Steve.Glendinning@smsc.com
Date: Thu, 6 Aug 2009 12:53:45 +0100

> David Miller <davem@davemloft.net> wrote on 06/08/2009 04:29:22:
> 
>> From: Daniel Mack <daniel@caiaq.de>
>> Date: Wed,  5 Aug 2009 20:29:31 +0200
>> 
>> > Hibernation is unsupported for now, which meets the actual
>> > implementation in the driver. For free/thaw, the chip's D2 state 
> should
>> > be entered.
>> > 
>> > Signed-off-by: Daniel Mack <daniel@caiaq.de>
>> 
>> Steve, if it looks good to you I can toss this into net-next-2.6
>> 
>> Just let me know.
> 
> Looks fine.  Compiles ok and doesn't actually change any PM 
> implementation.
> 
> Acked-by: <steve.glendinning@smsc.com>

Applied, thanks everyone.

Re: [PATCH 4/5] input: gpio-keys: switch to new dev_pm_ops

[pHilipp Zabel]

On Wed, Aug 5, 2009 at 8:29 PM, Daniel Mack<daniel@caiaq.de> wrote:
> Signed-off-by: Daniel Mack <daniel@caiaq.de>
> Cc: Phil Blundell <pb@handhelds.org>
> Cc: linux-input@vger.kernel.org
> ---
>  drivers/input/keyboard/gpio_keys.c |   28 ++++++++++++++++++----------
>  1 files changed, 18 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c
> index efed0c9..bfb6fc3 100644
> --- a/drivers/input/keyboard/gpio_keys.c
> +++ b/drivers/input/keyboard/gpio_keys.c
> @@ -216,12 +216,12 @@ static int __devexit gpio_keys_remove(struct platform_device *pdev)
>
>
>  #ifdef CONFIG_PM
> -static int gpio_keys_suspend(struct platform_device *pdev, pm_message_t state)
> +static int gpio_keys_suspend(struct device *dev)
>  {
> -       struct gpio_keys_platform_data *pdata = pdev->dev.platform_data;
> +       struct gpio_keys_platform_data *pdata = dev->platform_data;
>        int i;
>
> -       if (device_may_wakeup(&pdev->dev)) {
> +       if (device_may_wakeup(dev)) {
>                for (i = 0; i < pdata->nbuttons; i++) {
>                        struct gpio_keys_button *button = &pdata->buttons[i];
>                        if (button->wakeup) {
> @@ -234,12 +234,12 @@ static int gpio_keys_suspend(struct platform_device *pdev, pm_message_t state)
>        return 0;
>  }
>
> -static int gpio_keys_resume(struct platform_device *pdev)
> +static int gpio_keys_resume(struct device *dev)
>  {
> -       struct gpio_keys_platform_data *pdata = pdev->dev.platform_data;
> +       struct gpio_keys_platform_data *pdata = dev->platform_data;
>        int i;
>
> -       if (device_may_wakeup(&pdev->dev)) {
> +       if (device_may_wakeup(dev)) {
>                for (i = 0; i < pdata->nbuttons; i++) {
>                        struct gpio_keys_button *button = &pdata->buttons[i];
>                        if (button->wakeup) {
> @@ -251,19 +251,27 @@ static int gpio_keys_resume(struct platform_device *pdev)
>
>        return 0;
>  }
> +
> +static struct dev_pm_ops gpio_keys_pm_ops = {
> +       .suspend        = gpio_keys_suspend,
> +       .freeze         = gpio_keys_suspend,
> +       .resume         = gpio_keys_resume,
> +       .thaw           = gpio_keys_resume,

I'm not sure I understand hibernation correctly, but isn't
.freeze/.thaw about saving state and halting/resuming the device
operation only?
It seems to me that enabling system wakeup functionality should go
into .poweroff. (See <linux/pm.h>)

regards
Philipp

Re: [PATCH 4/5] input: gpio-keys: switch to new dev_pm_ops

[Daniel Mack]

On Wed, Aug 05, 2009 at 10:15:52PM +0200, pHilipp Zabel wrote:
> On Wed, Aug 5, 2009 at 8:29 PM, Daniel Mack<daniel@caiaq.de> wrote:
> > -static int gpio_keys_resume(struct platform_device *pdev)
> > +static int gpio_keys_resume(struct device *dev)
> >  {
> > -       struct gpio_keys_platform_data *pdata = pdev->dev.platform_data;
> > +       struct gpio_keys_platform_data *pdata = dev->platform_data;
> >        int i;
> >
> > -       if (device_may_wakeup(&pdev->dev)) {
> > +       if (device_may_wakeup(dev)) {
> >                for (i = 0; i < pdata->nbuttons; i++) {
> >                        struct gpio_keys_button *button = &pdata->buttons[i];
> >                        if (button->wakeup) {
> > @@ -251,19 +251,27 @@ static int gpio_keys_resume(struct platform_device *pdev)
> >
> >        return 0;
> >  }
> > +
> > +static struct dev_pm_ops gpio_keys_pm_ops = {
> > +       .suspend        = gpio_keys_suspend,
> > +       .freeze         = gpio_keys_suspend,
> > +       .resume         = gpio_keys_resume,
> > +       .thaw           = gpio_keys_resume,
> 
> I'm not sure I understand hibernation correctly, but isn't
> .freeze/.thaw about saving state and halting/resuming the device
> operation only?
> It seems to me that enabling system wakeup functionality should go
> into .poweroff. (See <linux/pm.h>)

Frankly, I'm not sure either. But luckily, the linux-pm list is copied
so someone can elborate on this :)

Daniel

Re: [linux-pm] [PATCH 4/5] input: gpio-keys: switch to new dev_pm_ops

[Rafael J. Wysocki]

On Thursday 06 August 2009, Daniel Mack wrote:
> On Wed, Aug 05, 2009 at 10:15:52PM +0200, pHilipp Zabel wrote:
> > On Wed, Aug 5, 2009 at 8:29 PM, Daniel Mack<daniel@caiaq.de> wrote:
> > > -static int gpio_keys_resume(struct platform_device *pdev)
> > > +static int gpio_keys_resume(struct device *dev)
> > >  {
> > > -       struct gpio_keys_platform_data *pdata = pdev->dev.platform_data;
> > > +       struct gpio_keys_platform_data *pdata = dev->platform_data;
> > >        int i;
> > >
> > > -       if (device_may_wakeup(&pdev->dev)) {
> > > +       if (device_may_wakeup(dev)) {
> > >                for (i = 0; i < pdata->nbuttons; i++) {
> > >                        struct gpio_keys_button *button = &pdata->buttons[i];
> > >                        if (button->wakeup) {
> > > @@ -251,19 +251,27 @@ static int gpio_keys_resume(struct platform_device *pdev)
> > >
> > >        return 0;
> > >  }
> > > +
> > > +static struct dev_pm_ops gpio_keys_pm_ops = {
> > > +       .suspend        = gpio_keys_suspend,
> > > +       .freeze         = gpio_keys_suspend,
> > > +       .resume         = gpio_keys_resume,
> > > +       .thaw           = gpio_keys_resume,
> > 
> > I'm not sure I understand hibernation correctly, but isn't
> > .freeze/.thaw about saving state and halting/resuming the device
> > operation only?

It is.

> > It seems to me that enabling system wakeup functionality should go
> > into .poweroff. (See <linux/pm.h>)

That's correct.

And .restore() plays the role of ".resume() from hibernation".

> Frankly, I'm not sure either. But luckily, the linux-pm list is copied
> so someone can elborate on this :)

HTH

Best,
Rafael

Re: [linux-pm] [PATCH 4/5] input: gpio-keys: switch to new dev_pm_ops

[Daniel Mack]

On Thu, Aug 06, 2009 at 02:51:55AM +0200, Rafael J. Wysocki wrote:
> On Thursday 06 August 2009, Daniel Mack wrote:
> > On Wed, Aug 05, 2009 at 10:15:52PM +0200, pHilipp Zabel wrote:
> > > On Wed, Aug 5, 2009 at 8:29 PM, Daniel Mack<daniel@caiaq.de> wrote:
> > > > -static int gpio_keys_resume(struct platform_device *pdev)
> > > > +static int gpio_keys_resume(struct device *dev)
> > > >  {
> > > > -       struct gpio_keys_platform_data *pdata = pdev->dev.platform_data;
> > > > +       struct gpio_keys_platform_data *pdata = dev->platform_data;
> > > >        int i;
> > > >
> > > > -       if (device_may_wakeup(&pdev->dev)) {
> > > > +       if (device_may_wakeup(dev)) {
> > > >                for (i = 0; i < pdata->nbuttons; i++) {
> > > >                        struct gpio_keys_button *button = &pdata->buttons[i];
> > > >                        if (button->wakeup) {
> > > > @@ -251,19 +251,27 @@ static int gpio_keys_resume(struct platform_device *pdev)
> > > >
> > > >        return 0;
> > > >  }
> > > > +
> > > > +static struct dev_pm_ops gpio_keys_pm_ops = {
> > > > +       .suspend        = gpio_keys_suspend,
> > > > +       .freeze         = gpio_keys_suspend,
> > > > +       .resume         = gpio_keys_resume,
> > > > +       .thaw           = gpio_keys_resume,
> > > 
> > > I'm not sure I understand hibernation correctly, but isn't
> > > .freeze/.thaw about saving state and halting/resuming the device
> > > operation only?
> 
> It is.
> 
> > > It seems to me that enabling system wakeup functionality should go
> > > into .poweroff. (See <linux/pm.h>)
> 
> That's correct.
> 
> And .restore() plays the role of ".resume() from hibernation".

Hence, In case of this driver which only calls disable_irq/enable_irq,
setting .poweroff and .restore seems to suffice? So does the patch
below.

Thanks,
Daniel


>From afa35cebccf49a24613e711afb8009fb6aadce0e Mon Sep 17 00:00:00 2001
From: Daniel Mack <daniel@caiaq.de>
Date: Wed, 5 Aug 2009 15:28:54 +0200
Subject: [PATCH 3/3] input: gpio-keys: switch to new dev_pm_ops

The callbacks for the implemented functions are .poweroff and .restore,
as they only care for {dis,en}able_irq(). Renamed the functions
to reflect that.

Signed-off-by: Daniel Mack <daniel@caiaq.de>
Cc: Phil Blundell <pb@handhelds.org>
Cc: linux-input@vger.kernel.org
---
 drivers/input/keyboard/gpio_keys.c |   26 ++++++++++++++++----------
 1 files changed, 16 insertions(+), 10 deletions(-)

diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c
index efed0c9..8e670d4 100644
--- a/drivers/input/keyboard/gpio_keys.c
+++ b/drivers/input/keyboard/gpio_keys.c
@@ -216,12 +216,12 @@ static int __devexit gpio_keys_remove(struct platform_device *pdev)
 
 
 #ifdef CONFIG_PM
-static int gpio_keys_suspend(struct platform_device *pdev, pm_message_t state)
+static int gpio_keys_poweroff(struct device *dev)
 {
-	struct gpio_keys_platform_data *pdata = pdev->dev.platform_data;
+	struct gpio_keys_platform_data *pdata = dev->platform_data;
 	int i;
 
-	if (device_may_wakeup(&pdev->dev)) {
+	if (device_may_wakeup(dev)) {
 		for (i = 0; i < pdata->nbuttons; i++) {
 			struct gpio_keys_button *button = &pdata->buttons[i];
 			if (button->wakeup) {
@@ -234,12 +234,12 @@ static int gpio_keys_suspend(struct platform_device *pdev, pm_message_t state)
 	return 0;
 }
 
-static int gpio_keys_resume(struct platform_device *pdev)
+static int gpio_keys_restore(struct device *dev)
 {
-	struct gpio_keys_platform_data *pdata = pdev->dev.platform_data;
+	struct gpio_keys_platform_data *pdata = dev->platform_data;
 	int i;
 
-	if (device_may_wakeup(&pdev->dev)) {
+	if (device_may_wakeup(dev)) {
 		for (i = 0; i < pdata->nbuttons; i++) {
 			struct gpio_keys_button *button = &pdata->buttons[i];
 			if (button->wakeup) {
@@ -251,19 +251,25 @@ static int gpio_keys_resume(struct platform_device *pdev)
 
 	return 0;
 }
+
+static struct dev_pm_ops gpio_keys_pm_ops = {
+	.poweroff	= gpio_keys_poweroff,
+	.restore	= gpio_keys_restore,
+};
+
+#define GPIO_KEYS_PM_OPS (&gpio_keys_pm_ops)
+
 #else
-#define gpio_keys_suspend	NULL
-#define gpio_keys_resume	NULL
+#define GPIO_KEYS_PM_OPS NULL
 #endif
 
 static struct platform_driver gpio_keys_device_driver = {
 	.probe		= gpio_keys_probe,
 	.remove		= __devexit_p(gpio_keys_remove),
-	.suspend	= gpio_keys_suspend,
-	.resume		= gpio_keys_resume,
 	.driver		= {
 		.name	= "gpio-keys",
 		.owner	= THIS_MODULE,
+		.pm	= GPIO_KEYS_PM_OPS,
 	}
 };
 
-- 
1.6.3.3

Re: [linux-pm] [PATCH 4/5] input: gpio-keys: switch to new dev_pm_ops

[Rafael J. Wysocki]

On Thursday 06 August 2009, Daniel Mack wrote:
> On Thu, Aug 06, 2009 at 02:51:55AM +0200, Rafael J. Wysocki wrote:
> > On Thursday 06 August 2009, Daniel Mack wrote:
> > > On Wed, Aug 05, 2009 at 10:15:52PM +0200, pHilipp Zabel wrote:
> > > > On Wed, Aug 5, 2009 at 8:29 PM, Daniel Mack<daniel@caiaq.de> wrote:
> > > > > -static int gpio_keys_resume(struct platform_device *pdev)
> > > > > +static int gpio_keys_resume(struct device *dev)
> > > > >  {
> > > > > -       struct gpio_keys_platform_data *pdata = pdev->dev.platform_data;
> > > > > +       struct gpio_keys_platform_data *pdata = dev->platform_data;
> > > > >        int i;
> > > > >
> > > > > -       if (device_may_wakeup(&pdev->dev)) {
> > > > > +       if (device_may_wakeup(dev)) {
> > > > >                for (i = 0; i < pdata->nbuttons; i++) {
> > > > >                        struct gpio_keys_button *button = &pdata->buttons[i];
> > > > >                        if (button->wakeup) {
> > > > > @@ -251,19 +251,27 @@ static int gpio_keys_resume(struct platform_device *pdev)
> > > > >
> > > > >        return 0;
> > > > >  }
> > > > > +
> > > > > +static struct dev_pm_ops gpio_keys_pm_ops = {
> > > > > +       .suspend        = gpio_keys_suspend,
> > > > > +       .freeze         = gpio_keys_suspend,
> > > > > +       .resume         = gpio_keys_resume,
> > > > > +       .thaw           = gpio_keys_resume,
> > > > 
> > > > I'm not sure I understand hibernation correctly, but isn't
> > > > .freeze/.thaw about saving state and halting/resuming the device
> > > > operation only?
> > 
> > It is.
> > 
> > > > It seems to me that enabling system wakeup functionality should go
> > > > into .poweroff. (See <linux/pm.h>)
> > 
> > That's correct.
> > 
> > And .restore() plays the role of ".resume() from hibernation".
> 
> Hence, In case of this driver which only calls disable_irq/enable_irq,
> setting .poweroff and .restore seems to suffice?

I'll have a look at the driver.  Still, it seems .suspend and .resume should be
set too, shouldn't they?

> From afa35cebccf49a24613e711afb8009fb6aadce0e Mon Sep 17 00:00:00 2001
> From: Daniel Mack <daniel@caiaq.de>
> Date: Wed, 5 Aug 2009 15:28:54 +0200
> Subject: [PATCH 3/3] input: gpio-keys: switch to new dev_pm_ops
> 
> The callbacks for the implemented functions are .poweroff and .restore,
> as they only care for {dis,en}able_irq(). Renamed the functions
> to reflect that.
> 
> Signed-off-by: Daniel Mack <daniel@caiaq.de>
> Cc: Phil Blundell <pb@handhelds.org>
> Cc: linux-input@vger.kernel.org
> ---
>  drivers/input/keyboard/gpio_keys.c |   26 ++++++++++++++++----------
>  1 files changed, 16 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c
> index efed0c9..8e670d4 100644
> --- a/drivers/input/keyboard/gpio_keys.c
> +++ b/drivers/input/keyboard/gpio_keys.c
> @@ -216,12 +216,12 @@ static int __devexit gpio_keys_remove(struct platform_device *pdev)
>  
>  
>  #ifdef CONFIG_PM
> -static int gpio_keys_suspend(struct platform_device *pdev, pm_message_t state)
> +static int gpio_keys_poweroff(struct device *dev)
>  {
> -	struct gpio_keys_platform_data *pdata = pdev->dev.platform_data;
> +	struct gpio_keys_platform_data *pdata = dev->platform_data;
>  	int i;
>  
> -	if (device_may_wakeup(&pdev->dev)) {
> +	if (device_may_wakeup(dev)) {
>  		for (i = 0; i < pdata->nbuttons; i++) {
>  			struct gpio_keys_button *button = &pdata->buttons[i];
>  			if (button->wakeup) {
> @@ -234,12 +234,12 @@ static int gpio_keys_suspend(struct platform_device *pdev, pm_message_t state)
>  	return 0;
>  }
>  
> -static int gpio_keys_resume(struct platform_device *pdev)
> +static int gpio_keys_restore(struct device *dev)
>  {
> -	struct gpio_keys_platform_data *pdata = pdev->dev.platform_data;
> +	struct gpio_keys_platform_data *pdata = dev->platform_data;
>  	int i;
>  
> -	if (device_may_wakeup(&pdev->dev)) {
> +	if (device_may_wakeup(dev)) {
>  		for (i = 0; i < pdata->nbuttons; i++) {
>  			struct gpio_keys_button *button = &pdata->buttons[i];
>  			if (button->wakeup) {
> @@ -251,19 +251,25 @@ static int gpio_keys_resume(struct platform_device *pdev)
>  
>  	return 0;
>  }
> +
> +static struct dev_pm_ops gpio_keys_pm_ops = {
> +	.poweroff	= gpio_keys_poweroff,
> +	.restore	= gpio_keys_restore,

Should .suspend and .resume really be NULL?

> +};
> +
> +#define GPIO_KEYS_PM_OPS (&gpio_keys_pm_ops)
> +
>  #else
> -#define gpio_keys_suspend	NULL
> -#define gpio_keys_resume	NULL
> +#define GPIO_KEYS_PM_OPS NULL
>  #endif
>  
>  static struct platform_driver gpio_keys_device_driver = {
>  	.probe		= gpio_keys_probe,
>  	.remove		= __devexit_p(gpio_keys_remove),
> -	.suspend	= gpio_keys_suspend,
> -	.resume		= gpio_keys_resume,
>  	.driver		= {
>  		.name	= "gpio-keys",
>  		.owner	= THIS_MODULE,
> +		.pm	= GPIO_KEYS_PM_OPS,
>  	}
>  };

Thanks,
Rafael

Re: [linux-pm] [PATCH 4/5] input: gpio-keys: switch to new dev_pm_ops

[pHilipp Zabel]

On Thu, Aug 6, 2009 at 5:19 PM, Rafael J. Wysocki<rjw@sisk.pl> wrote:
> On Thursday 06 August 2009, Daniel Mack wrote:
>> On Thu, Aug 06, 2009 at 02:51:55AM +0200, Rafael J. Wysocki wrote:
>> > On Thursday 06 August 2009, Daniel Mack wrote:
>> > > On Wed, Aug 05, 2009 at 10:15:52PM +0200, pHilipp Zabel wrote:
>> > > > On Wed, Aug 5, 2009 at 8:29 PM, Daniel Mack<daniel@caiaq.de> wrote:
>> > > > > -static int gpio_keys_resume(struct platform_device *pdev)
>> > > > > +static int gpio_keys_resume(struct device *dev)
>> > > > >  {
>> > > > > -       struct gpio_keys_platform_data *pdata = pdev->dev.platform_data;
>> > > > > +       struct gpio_keys_platform_data *pdata = dev->platform_data;
>> > > > >        int i;
>> > > > >
>> > > > > -       if (device_may_wakeup(&pdev->dev)) {
>> > > > > +       if (device_may_wakeup(dev)) {
>> > > > >                for (i = 0; i < pdata->nbuttons; i++) {
>> > > > >                        struct gpio_keys_button *button = &pdata->buttons[i];
>> > > > >                        if (button->wakeup) {
>> > > > > @@ -251,19 +251,27 @@ static int gpio_keys_resume(struct platform_device *pdev)
>> > > > >
>> > > > >        return 0;
>> > > > >  }
>> > > > > +
>> > > > > +static struct dev_pm_ops gpio_keys_pm_ops = {
>> > > > > +       .suspend        = gpio_keys_suspend,
>> > > > > +       .freeze         = gpio_keys_suspend,
>> > > > > +       .resume         = gpio_keys_resume,
>> > > > > +       .thaw           = gpio_keys_resume,
>> > > >
>> > > > I'm not sure I understand hibernation correctly, but isn't
>> > > > .freeze/.thaw about saving state and halting/resuming the device
>> > > > operation only?
>> >
>> > It is.
>> >
>> > > > It seems to me that enabling system wakeup functionality should go
>> > > > into .poweroff. (See <linux/pm.h>)
>> >
>> > That's correct.
>> >
>> > And .restore() plays the role of ".resume() from hibernation".
>>
>> Hence, In case of this driver which only calls disable_irq/enable_irq,
>> setting .poweroff and .restore seems to suffice?
>
> I'll have a look at the driver.  Still, it seems .suspend and .resume should be
> set too, shouldn't they?

.suspend/.resume need to be set. But I think .restore is not needed,
as the effects of calling enable_irq_wake in .poweroff are already
completely reverted by rebooting the system and restoring the memory
image that was captured before .poweroff was called.

>> From afa35cebccf49a24613e711afb8009fb6aadce0e Mon Sep 17 00:00:00 2001
>> From: Daniel Mack <daniel@caiaq.de>
>> Date: Wed, 5 Aug 2009 15:28:54 +0200
>> Subject: [PATCH 3/3] input: gpio-keys: switch to new dev_pm_ops
>>
>> The callbacks for the implemented functions are .poweroff and .restore,
>> as they only care for {dis,en}able_irq(). Renamed the functions
>> to reflect that.
>>
>> Signed-off-by: Daniel Mack <daniel@caiaq.de>
>> Cc: Phil Blundell <pb@handhelds.org>
>> Cc: linux-input@vger.kernel.org
>> ---
>>  drivers/input/keyboard/gpio_keys.c |   26 ++++++++++++++++----------
>>  1 files changed, 16 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c
>> index efed0c9..8e670d4 100644
>> --- a/drivers/input/keyboard/gpio_keys.c
>> +++ b/drivers/input/keyboard/gpio_keys.c
>> @@ -216,12 +216,12 @@ static int __devexit gpio_keys_remove(struct platform_device *pdev)
>>
>>
>>  #ifdef CONFIG_PM
>> -static int gpio_keys_suspend(struct platform_device *pdev, pm_message_t state)
>> +static int gpio_keys_poweroff(struct device *dev)
>>  {
>> -     struct gpio_keys_platform_data *pdata = pdev->dev.platform_data;
>> +     struct gpio_keys_platform_data *pdata = dev->platform_data;
>>       int i;
>>
>> -     if (device_may_wakeup(&pdev->dev)) {
>> +     if (device_may_wakeup(dev)) {
>>               for (i = 0; i < pdata->nbuttons; i++) {
>>                       struct gpio_keys_button *button = &pdata->buttons[i];
>>                       if (button->wakeup) {
>> @@ -234,12 +234,12 @@ static int gpio_keys_suspend(struct platform_device *pdev, pm_message_t state)
>>       return 0;
>>  }
>>
>> -static int gpio_keys_resume(struct platform_device *pdev)
>> +static int gpio_keys_restore(struct device *dev)
>>  {
>> -     struct gpio_keys_platform_data *pdata = pdev->dev.platform_data;
>> +     struct gpio_keys_platform_data *pdata = dev->platform_data;
>>       int i;
>>
>> -     if (device_may_wakeup(&pdev->dev)) {
>> +     if (device_may_wakeup(dev)) {
>>               for (i = 0; i < pdata->nbuttons; i++) {
>>                       struct gpio_keys_button *button = &pdata->buttons[i];
>>                       if (button->wakeup) {
>> @@ -251,19 +251,25 @@ static int gpio_keys_resume(struct platform_device *pdev)
>>
>>       return 0;
>>  }
>> +
>> +static struct dev_pm_ops gpio_keys_pm_ops = {
>> +     .poweroff       = gpio_keys_poweroff,
>> +     .restore        = gpio_keys_restore,
>
> Should .suspend and .resume really be NULL?
>
>> +};
>> +
>> +#define GPIO_KEYS_PM_OPS (&gpio_keys_pm_ops)
>> +
>>  #else
>> -#define gpio_keys_suspend    NULL
>> -#define gpio_keys_resume     NULL
>> +#define GPIO_KEYS_PM_OPS NULL
>>  #endif
>>
>>  static struct platform_driver gpio_keys_device_driver = {
>>       .probe          = gpio_keys_probe,
>>       .remove         = __devexit_p(gpio_keys_remove),
>> -     .suspend        = gpio_keys_suspend,
>> -     .resume         = gpio_keys_resume,
>>       .driver         = {
>>               .name   = "gpio-keys",
>>               .owner  = THIS_MODULE,
>> +             .pm     = GPIO_KEYS_PM_OPS,
>>       }
>>  };
>
> Thanks,
> Rafael
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>

regards
Philipp

Re: [PATCH 2/5] pda-power: switch to new dev_pm_ops

[Frans Pop]

Daniel Mack wrote:
> +static struct dev_pm_ops pda_power_pm_ops = {
> +       .suspend        = pda_power_suspend,
> +       .freeze         = pda_power_freeze,

Hmmm. Where's pda_power_freeze defined? Forgot to (compile)test the 
patches?

> +       .resume         = pda_power_resume,
> +       .thaw           = pda_power_resume,

Re: [PATCH 2/5] pda-power: switch to new dev_pm_ops

[Daniel Mack]

On Wed, Aug 05, 2009 at 09:49:52PM +0200, Frans Pop wrote:
> Daniel Mack wrote:
> > +static struct dev_pm_ops pda_power_pm_ops = {
> > +       .suspend        = pda_power_suspend,
> > +       .freeze         = pda_power_freeze,
> 
> Hmmm. Where's pda_power_freeze defined? Forgot to (compile)test the 
> patches?

Yes, sorry. I can't test hibernation, so I didn't pay enough attention.
I'll resend them anyway with Rafael's new macro used.

Thanks,
Daniel

Re: [PATCH 2/5] pda-power: switch to new dev_pm_ops

[pHilipp Zabel]

On Thu, Aug 6, 2009 at 12:29 AM, Daniel Mack<daniel@caiaq.de> wrote:
> On Wed, Aug 05, 2009 at 09:49:52PM +0200, Frans Pop wrote:
>> Daniel Mack wrote:
>> > +static struct dev_pm_ops pda_power_pm_ops = {
>> > +       .suspend        = pda_power_suspend,
>> > +       .freeze         = pda_power_freeze,
>>
>> Hmmm. Where's pda_power_freeze defined? Forgot to (compile)test the
>> patches?
>
> Yes, sorry. I can't test hibernation, so I didn't pay enough attention.
> I'll resend them anyway with Rafael's new macro used.

After Rafael's comments on gpio_keys, I believe also here
.freeze/.thaw should be empty. enable/disable_irq_wake should be
called from .poweroff/.restore.

regards
Philipp

Re: [PATCH 2/5] pda-power: switch to new dev_pm_ops

[Daniel Mack]

On Thu, Aug 06, 2009 at 01:52:41PM +0200, pHilipp Zabel wrote:
> On Thu, Aug 6, 2009 at 12:29 AM, Daniel Mack<daniel@caiaq.de> wrote:
> > On Wed, Aug 05, 2009 at 09:49:52PM +0200, Frans Pop wrote:
> >> Daniel Mack wrote:
> >> > +static struct dev_pm_ops pda_power_pm_ops = {
> >> > +       .suspend        = pda_power_suspend,
> >> > +       .freeze         = pda_power_freeze,
> >>
> >> Hmmm. Where's pda_power_freeze defined? Forgot to (compile)test the
> >> patches?
> >
> > Yes, sorry. I can't test hibernation, so I didn't pay enough attention.
> > I'll resend them anyway with Rafael's new macro used.
> 
> After Rafael's comments on gpio_keys, I believe also here
> .freeze/.thaw should be empty. enable/disable_irq_wake should be
> called from .poweroff/.restore.

Yes. See the version below.

Thanks,
Daniel


>From 6b534fb029f4623a8ddd60a4ea636bd626d6382a Mon Sep 17 00:00:00 2001
From: Daniel Mack <daniel@caiaq.de>
Date: Wed, 5 Aug 2009 15:31:23 +0200
Subject: [PATCH 1/3] pda-power: switch to new dev_pm_ops

The callbacks for the implemented functions are .poweroff and .restore,
as they only care for {dis,en}able_irq(). Renamed the functions to
reflect that.

Signed-off-by: Daniel Mack <daniel@caiaq.de>
Cc: Ian Molton <spyro@f2s.com>
Cc: Anton Vorontsov <cbou@mail.ru>
Cc: Matt Reimer <mreimer@vpop.net>
---
 drivers/power/pda_power.c |   25 ++++++++++++++++---------
 1 files changed, 16 insertions(+), 9 deletions(-)

diff --git a/drivers/power/pda_power.c b/drivers/power/pda_power.c
index a232de6..c46a6dc 100644
--- a/drivers/power/pda_power.c
+++ b/drivers/power/pda_power.c
@@ -402,9 +402,9 @@ static int pda_power_remove(struct platform_device *pdev)
 static int ac_wakeup_enabled;
 static int usb_wakeup_enabled;
 
-static int pda_power_suspend(struct platform_device *pdev, pm_message_t state)
+static int pda_power_poweroff(struct device *dev)
 {
-	if (device_may_wakeup(&pdev->dev)) {
+	if (device_may_wakeup(dev)) {
 		if (ac_irq)
 			ac_wakeup_enabled = !enable_irq_wake(ac_irq->start);
 		if (usb_irq)
@@ -414,9 +414,9 @@ static int pda_power_suspend(struct platform_device *pdev, pm_message_t state)
 	return 0;
 }
 
-static int pda_power_resume(struct platform_device *pdev)
+static int pda_power_restore(struct device *dev)
 {
-	if (device_may_wakeup(&pdev->dev)) {
+	if (device_may_wakeup(dev)) {
 		if (usb_irq && usb_wakeup_enabled)
 			disable_irq_wake(usb_irq->start);
 		if (ac_irq && ac_wakeup_enabled)
@@ -425,21 +425,28 @@ static int pda_power_resume(struct platform_device *pdev)
 
 	return 0;
 }
+
+static struct dev_pm_ops pda_power_pm_ops = {
+	.poweroff	= pda_power_poweroff,
+	.restore	= pda_power_restore,
+};
+
+#define PDA_POWER_PM_OPS (&pda_power_pm_ops)
+
 #else
-#define pda_power_suspend NULL
-#define pda_power_resume NULL
+#define PDA_POWER_PM_OPS NULL
 #endif /* CONFIG_PM */
 
 MODULE_ALIAS("platform:pda-power");
 
 static struct platform_driver pda_power_pdrv = {
 	.driver = {
-		.name = "pda-power",
+		.name	= "pda-power",
+		.owner	= THIS_MODULE,
+		.pm	= PDA_POWER_PM_OPS,
 	},
 	.probe = pda_power_probe,
 	.remove = pda_power_remove,
-	.suspend = pda_power_suspend,
-	.resume = pda_power_resume,
 };
 
 static int __init pda_power_init(void)
-- 
1.6.3.3

Re: [PATCH 2/5] pda-power: switch to new dev_pm_ops

[pHilipp Zabel]

On Thu, Aug 6, 2009 at 2:56 PM, Daniel Mack<daniel@caiaq.de> wrote:
> On Thu, Aug 06, 2009 at 01:52:41PM +0200, pHilipp Zabel wrote:
>> On Thu, Aug 6, 2009 at 12:29 AM, Daniel Mack<daniel@caiaq.de> wrote:
>> > On Wed, Aug 05, 2009 at 09:49:52PM +0200, Frans Pop wrote:
>> >> Daniel Mack wrote:
>> >> > +static struct dev_pm_ops pda_power_pm_ops = {
>> >> > +       .suspend        = pda_power_suspend,
>> >> > +       .freeze         = pda_power_freeze,
>> >>
>> >> Hmmm. Where's pda_power_freeze defined? Forgot to (compile)test the
>> >> patches?
>> >
>> > Yes, sorry. I can't test hibernation, so I didn't pay enough attention.
>> > I'll resend them anyway with Rafael's new macro used.
>>
>> After Rafael's comments on gpio_keys, I believe also here
>> .freeze/.thaw should be empty. enable/disable_irq_wake should be
>> called from .poweroff/.restore.
>
> Yes. See the version below.
>
> Thanks,
> Daniel
>
>
> From 6b534fb029f4623a8ddd60a4ea636bd626d6382a Mon Sep 17 00:00:00 2001
> From: Daniel Mack <daniel@caiaq.de>
> Date: Wed, 5 Aug 2009 15:31:23 +0200
> Subject: [PATCH 1/3] pda-power: switch to new dev_pm_ops
>
> The callbacks for the implemented functions are .poweroff and .restore,
> as they only care for {dis,en}able_irq(). Renamed the functions to
> reflect that.
>
> Signed-off-by: Daniel Mack <daniel@caiaq.de>
> Cc: Ian Molton <spyro@f2s.com>
> Cc: Anton Vorontsov <cbou@mail.ru>
> Cc: Matt Reimer <mreimer@vpop.net>
> ---
>  drivers/power/pda_power.c |   25 ++++++++++++++++---------
>  1 files changed, 16 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/power/pda_power.c b/drivers/power/pda_power.c
> index a232de6..c46a6dc 100644
> --- a/drivers/power/pda_power.c
> +++ b/drivers/power/pda_power.c
> @@ -402,9 +402,9 @@ static int pda_power_remove(struct platform_device *pdev)
>  static int ac_wakeup_enabled;
>  static int usb_wakeup_enabled;
>
> -static int pda_power_suspend(struct platform_device *pdev, pm_message_t state)
> +static int pda_power_poweroff(struct device *dev)
>  {
> -       if (device_may_wakeup(&pdev->dev)) {
> +       if (device_may_wakeup(dev)) {
>                if (ac_irq)
>                        ac_wakeup_enabled = !enable_irq_wake(ac_irq->start);
>                if (usb_irq)
> @@ -414,9 +414,9 @@ static int pda_power_suspend(struct platform_device *pdev, pm_message_t state)
>        return 0;
>  }
>
> -static int pda_power_resume(struct platform_device *pdev)
> +static int pda_power_restore(struct device *dev)
>  {
> -       if (device_may_wakeup(&pdev->dev)) {
> +       if (device_may_wakeup(dev)) {
>                if (usb_irq && usb_wakeup_enabled)
>                        disable_irq_wake(usb_irq->start);
>                if (ac_irq && ac_wakeup_enabled)
> @@ -425,21 +425,28 @@ static int pda_power_resume(struct platform_device *pdev)
>
>        return 0;
>  }
> +
> +static struct dev_pm_ops pda_power_pm_ops = {
> +       .poweroff       = pda_power_poweroff,
> +       .restore        = pda_power_restore,
> +};

No no, now you're missing .suspend/.resume. As I understand it, the
suspend and hibernation codepaths are now completely separate.

Also, I'm not sure using .restore here is quite correct.
enable_irq_wake is called in .poweroff, after .freeze + creation of
the memory image. So if this memory image is restored during the next
boot, there is no trace of irq wakup ever being enabled and thus no
need to call disable_irq_wake.

I think just .suspend/.poweroff and .resume need to be used for this driver.

> +
> +#define PDA_POWER_PM_OPS (&pda_power_pm_ops)
> +
>  #else
> -#define pda_power_suspend NULL
> -#define pda_power_resume NULL
> +#define PDA_POWER_PM_OPS NULL
>  #endif /* CONFIG_PM */
>
>  MODULE_ALIAS("platform:pda-power");
>
>  static struct platform_driver pda_power_pdrv = {
>        .driver = {
> -               .name = "pda-power",
> +               .name   = "pda-power",
> +               .owner  = THIS_MODULE,
> +               .pm     = PDA_POWER_PM_OPS,
>        },
>        .probe = pda_power_probe,
>        .remove = pda_power_remove,
> -       .suspend = pda_power_suspend,
> -       .resume = pda_power_resume,
>  };
>
>  static int __init pda_power_init(void)
> --
> 1.6.3.3
>
>

regards
Philipp

Re: [PATCH 2/5] pda-power: switch to new dev_pm_ops

[Anton Vorontsov]

On Fri, Aug 07, 2009 at 03:35:19PM +0200, pHilipp Zabel wrote:
[...]
> > +static struct dev_pm_ops pda_power_pm_ops = {
> > +       .poweroff       = pda_power_poweroff,
> > +       .restore        = pda_power_restore,
> > +};
> 
> No no, now you're missing .suspend/.resume. As I understand it, the
> suspend and hibernation codepaths are now completely separate.
> 
> Also, I'm not sure using .restore here is quite correct.
> enable_irq_wake is called in .poweroff, after .freeze + creation of
> the memory image. So if this memory image is restored during the next
> boot, there is no trace of irq wakup ever being enabled and thus no
> need to call disable_irq_wake.
> 
> I think just .suspend/.poweroff and .resume need to be used for this driver.

Daniel, any update on this patch?

Thanks,

-- 
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2

Re: [PATCH 1/5] w1_gpio: switch to new dev_pm_ops

[Rafael J. Wysocki]

On Wednesday 05 August 2009, Daniel Mack wrote:
> Signed-off-by: Daniel Mack <daniel@caiaq.de>
> Cc: Ville Syrjala <syrjala@sci.fi>
> Cc: Evgeniy Polyakov <johnpol@2ka.mipt.ru>
> ---
>  drivers/w1/masters/w1-gpio.c |   23 +++++++++++++++--------
>  1 files changed, 15 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/w1/masters/w1-gpio.c b/drivers/w1/masters/w1-gpio.c
> index 6f8866d..7099b11 100644
> --- a/drivers/w1/masters/w1-gpio.c
> +++ b/drivers/w1/masters/w1-gpio.c
> @@ -106,9 +106,9 @@ static int __exit w1_gpio_remove(struct platform_device *pdev)
>  
>  #ifdef CONFIG_PM
>  
> -static int w1_gpio_suspend(struct platform_device *pdev, pm_message_t state)
> +static int w1_gpio_suspend(struct device *dev)
>  {
> -	struct w1_gpio_platform_data *pdata = pdev->dev.platform_data;
> +	struct w1_gpio_platform_data *pdata = dev->platform_data;
>  
>  	if (pdata->enable_external_pullup)
>  		pdata->enable_external_pullup(0);
> @@ -116,9 +116,9 @@ static int w1_gpio_suspend(struct platform_device *pdev, pm_message_t state)
>  	return 0;
>  }
>  
> -static int w1_gpio_resume(struct platform_device *pdev)
> +static int w1_gpio_resume(struct device *dev)
>  {
> -	struct w1_gpio_platform_data *pdata = pdev->dev.platform_data;
> +	struct w1_gpio_platform_data *pdata = dev->platform_data;
>  
>  	if (pdata->enable_external_pullup)
>  		pdata->enable_external_pullup(1);
> @@ -126,19 +126,26 @@ static int w1_gpio_resume(struct platform_device *pdev)
>  	return 0;
>  }
>  
> +static struct dev_pm_ops w1_gpio_pm_ops = {
> +	.suspend	= w1_gpio_suspend,
> +	.freeze		= w1_gpio_suspend,
> +	.resume		= w1_gpio_resume,
> +	.thaw		= w1_gpio_resume,

You need

+	.poweroff	= w1_gpio_suspend,
+	.restore		= w1_gpio_resume,

in addition to these, which seems to be the case with the rest of the patches
too.

If you wait for a little while with the patchset, we're going to have a
convenience macro for defining such 'standard' dev_pm_ops objects.

> +};
> +
> +#define W1_GPIO_PM_OPS (&w1_gpio_pm_ops)
> +
>  #else
> -#define w1_gpio_suspend	NULL
> -#define w1_gpio_resume	NULL
> +#define W1_GPIO_PM_OPS NULL
>  #endif
>  
>  static struct platform_driver w1_gpio_driver = {
>  	.driver = {
>  		.name	= "w1-gpio",
>  		.owner	= THIS_MODULE,
> +		.pm	= W1_GPIO_PM_OPS,
>  	},
>  	.remove	= __exit_p(w1_gpio_remove),
> -	.suspend = w1_gpio_suspend,
> -	.resume = w1_gpio_resume,
>  };
>  
>  static int __init w1_gpio_init(void)

Best,
Rafael

Re: [PATCH 1/5] w1_gpio: switch to new dev_pm_ops

[Daniel Mack]

On Wed, Aug 05, 2009 at 08:44:01PM +0200, Rafael J. Wysocki wrote:
> > +static struct dev_pm_ops w1_gpio_pm_ops = {
> > +	.suspend	= w1_gpio_suspend,
> > +	.freeze		= w1_gpio_suspend,
> > +	.resume		= w1_gpio_resume,
> > +	.thaw		= w1_gpio_resume,
> 
> You need
> 
> +	.poweroff	= w1_gpio_suspend,
> +	.restore		= w1_gpio_resume,
> 
> in addition to these, which seems to be the case with the rest of the patches
> too.
> 
> If you wait for a little while with the patchset, we're going to have a
> convenience macro for defining such 'standard' dev_pm_ops objects.

Ok, fine. Letting them all point the same function looks somwhat hackish
to me, so I'll wait for that macro. Let me know when that has arrived :)

Daniel