* [PATCH 4/5] input: gpio-keys: switch to new dev_pm_ops [not found] ` <1249496971-9019-3-git-send-email-daniel@caiaq.de> @ 2009-08-05 18:29 ` Daniel Mack 2009-08-05 20:15 ` pHilipp Zabel 0 siblings, 1 reply; 7+ messages in thread From: Daniel Mack @ 2009-08-05 18:29 UTC (permalink / raw) To: linux-kernel; +Cc: linux-pm, Daniel Mack, Phil Blundell, linux-input 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 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 4/5] input: gpio-keys: switch to new dev_pm_ops 2009-08-05 18:29 ` [PATCH 4/5] input: gpio-keys: switch to new dev_pm_ops Daniel Mack @ 2009-08-05 20:15 ` pHilipp Zabel 2009-08-05 22:33 ` Daniel Mack 0 siblings, 1 reply; 7+ messages in thread From: pHilipp Zabel @ 2009-08-05 20:15 UTC (permalink / raw) To: Daniel Mack; +Cc: linux-kernel, linux-pm, Phil Blundell, linux-input 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 -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 4/5] input: gpio-keys: switch to new dev_pm_ops 2009-08-05 20:15 ` pHilipp Zabel @ 2009-08-05 22:33 ` Daniel Mack 2009-08-06 0:51 ` [linux-pm] " Rafael J. Wysocki 0 siblings, 1 reply; 7+ messages in thread From: Daniel Mack @ 2009-08-05 22:33 UTC (permalink / raw) To: pHilipp Zabel; +Cc: linux-kernel, linux-pm, Phil Blundell, linux-input 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [linux-pm] [PATCH 4/5] input: gpio-keys: switch to new dev_pm_ops 2009-08-05 22:33 ` Daniel Mack @ 2009-08-06 0:51 ` Rafael J. Wysocki 2009-08-06 12:59 ` Daniel Mack 0 siblings, 1 reply; 7+ messages in thread From: Rafael J. Wysocki @ 2009-08-06 0:51 UTC (permalink / raw) To: linux-pm Cc: Daniel Mack, pHilipp Zabel, Phil Blundell, linux-kernel, linux-input 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [linux-pm] [PATCH 4/5] input: gpio-keys: switch to new dev_pm_ops 2009-08-06 0:51 ` [linux-pm] " Rafael J. Wysocki @ 2009-08-06 12:59 ` Daniel Mack 2009-08-06 15:19 ` Rafael J. Wysocki 0 siblings, 1 reply; 7+ messages in thread From: Daniel Mack @ 2009-08-06 12:59 UTC (permalink / raw) To: Rafael J. Wysocki Cc: linux-pm, pHilipp Zabel, Phil Blundell, linux-kernel, linux-input 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 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [linux-pm] [PATCH 4/5] input: gpio-keys: switch to new dev_pm_ops 2009-08-06 12:59 ` Daniel Mack @ 2009-08-06 15:19 ` Rafael J. Wysocki 2009-08-07 13:39 ` pHilipp Zabel 0 siblings, 1 reply; 7+ messages in thread From: Rafael J. Wysocki @ 2009-08-06 15:19 UTC (permalink / raw) To: Daniel Mack Cc: linux-pm, pHilipp Zabel, Phil Blundell, linux-kernel, linux-input 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [linux-pm] [PATCH 4/5] input: gpio-keys: switch to new dev_pm_ops 2009-08-06 15:19 ` Rafael J. Wysocki @ 2009-08-07 13:39 ` pHilipp Zabel 0 siblings, 0 replies; 7+ messages in thread From: pHilipp Zabel @ 2009-08-07 13:39 UTC (permalink / raw) To: Rafael J. Wysocki Cc: Daniel Mack, linux-pm, Phil Blundell, linux-kernel, linux-input 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 -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2009-08-07 13:39 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1249496971-9019-1-git-send-email-daniel@caiaq.de>
[not found] ` <1249496971-9019-2-git-send-email-daniel@caiaq.de>
[not found] ` <1249496971-9019-3-git-send-email-daniel@caiaq.de>
2009-08-05 18:29 ` [PATCH 4/5] input: gpio-keys: switch to new dev_pm_ops Daniel Mack
2009-08-05 20:15 ` pHilipp Zabel
2009-08-05 22:33 ` Daniel Mack
2009-08-06 0:51 ` [linux-pm] " Rafael J. Wysocki
2009-08-06 12:59 ` Daniel Mack
2009-08-06 15:19 ` Rafael J. Wysocki
2009-08-07 13:39 ` pHilipp Zabel
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).