diff mbox

[2/2] input: gpio-keys: Add runtime support

Message ID 1349964927-18619-2-git-send-email-lee.jones@linaro.org (mailing list archive)
State New, archived
Headers show

Commit Message

Lee Jones Oct. 11, 2012, 2:15 p.m. UTC
From: Jonas Aaberg <jonas.aberg@stericsson.com>

Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: linux-input@vger.kernel.org
Acked-by: Lee Jones <lee.jones@linaro.org>
Signed-off-by: Jonas Aaberg <jonas.aberg@stericsson.com>
Signed-off-by: Philippe Langlais <philippe.langlais@linaro.org>
Reviewed-by: Bengt Jonsson <bengt.g.jonsson@stericsson.com>
---
 drivers/input/keyboard/gpio_keys.c |   16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

Comments

Shubhrajyoti Datta Oct. 11, 2012, 2:22 p.m. UTC | #1
On Thu, Oct 11, 2012 at 7:45 PM, Lee Jones <lee.jones@linaro.org> wrote:
> From: Jonas Aaberg <jonas.aberg@stericsson.com>
Some change logs would have helped.

>
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Cc: linux-input@vger.kernel.org
> Acked-by: Lee Jones <lee.jones@linaro.org>
> Signed-off-by: Jonas Aaberg <jonas.aberg@stericsson.com>
> Signed-off-by: Philippe Langlais <philippe.langlais@linaro.org>
> Reviewed-by: Bengt Jonsson <bengt.g.jonsson@stericsson.com>
> ---
>  drivers/input/keyboard/gpio_keys.c |   16 ++++++++++++----
>  1 file changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c
> index 7947315..78de557 100644
> --- a/drivers/input/keyboard/gpio_keys.c
> +++ b/drivers/input/keyboard/gpio_keys.c
> @@ -29,6 +29,7 @@
>  #include <linux/of_platform.h>
>  #include <linux/of_gpio.h>
>  #include <linux/spinlock.h>
> +#include <linux/pm_runtime.h>
>
>  struct gpio_button_data {
>         const struct gpio_keys_button *button;
> @@ -526,6 +527,7 @@ static int gpio_keys_open(struct input_dev *input)
>  {
>         struct gpio_keys_drvdata *ddata = input_get_drvdata(input);
>
> +       pm_runtime_get_sync(input->dev.parent);
I am not an expert of the runtime.

However would be grateful if you explain me what it actually do.

Also I did not see any runtime suspend/ resume handlers populated.
--
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
Lee Jones Oct. 11, 2012, 3:21 p.m. UTC | #2
> > From: Jonas Aaberg <jonas.aberg@stericsson.com>
> Some change logs would have helped.

Granted.

Hopefully Jonas will be happy to step forward and answer any of
your following questions.

> > Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > Cc: linux-input@vger.kernel.org
> > Acked-by: Lee Jones <lee.jones@linaro.org>
> > Signed-off-by: Jonas Aaberg <jonas.aberg@stericsson.com>
> > Signed-off-by: Philippe Langlais <philippe.langlais@linaro.org>
> > Reviewed-by: Bengt Jonsson <bengt.g.jonsson@stericsson.com>
> > ---
> >  drivers/input/keyboard/gpio_keys.c |   16 ++++++++++++----
> >  1 file changed, 12 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c
> > index 7947315..78de557 100644
> > --- a/drivers/input/keyboard/gpio_keys.c
> > +++ b/drivers/input/keyboard/gpio_keys.c
> > @@ -29,6 +29,7 @@
> >  #include <linux/of_platform.h>
> >  #include <linux/of_gpio.h>
> >  #include <linux/spinlock.h>
> > +#include <linux/pm_runtime.h>
> >
> >  struct gpio_button_data {
> >         const struct gpio_keys_button *button;
> > @@ -526,6 +527,7 @@ static int gpio_keys_open(struct input_dev *input)
> >  {
> >         struct gpio_keys_drvdata *ddata = input_get_drvdata(input);
> >
> > +       pm_runtime_get_sync(input->dev.parent);
> I am not an expert of the runtime.
> 
> However would be grateful if you explain me what it actually do.
> 
> Also I did not see any runtime suspend/ resume handlers populated.
Linus Walleij Oct. 12, 2012, 9:09 p.m. UTC | #3
On Thu, Oct 11, 2012 at 4:22 PM, Shubhrajyoti Datta
<omaplinuxkernel@gmail.com> wrote:

>> @@ -526,6 +527,7 @@ static int gpio_keys_open(struct input_dev *input)
>>  {
>>         struct gpio_keys_drvdata *ddata = input_get_drvdata(input);
>>
>> +       pm_runtime_get_sync(input->dev.parent);
>
> I am not an expert of the runtime.
>
> However would be grateful if you explain me what it actually do.

This increase the reference count of the runtime status container
for the device. _sync makes sure it happens now.

Consult:
Documentation/power/runtime_pm.txt

> Also I did not see any runtime suspend/ resume handlers populated.

It is not necessary to handle the power state at the driver level,
it can just as well be handled by the voltage/power domain,
or at the class, type or bus level.

But the individual driver has to notify the system upward if it
needs to be powered on or when it may or must be relaxed.

Yours,
Linus Walleij
--
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
Lee Jones Oct. 25, 2012, 7:57 a.m. UTC | #4
On Fri, 12 Oct 2012, Linus Walleij wrote:

> On Thu, Oct 11, 2012 at 4:22 PM, Shubhrajyoti Datta
> <omaplinuxkernel@gmail.com> wrote:
> 
> >> @@ -526,6 +527,7 @@ static int gpio_keys_open(struct input_dev *input)
> >>  {
> >>         struct gpio_keys_drvdata *ddata = input_get_drvdata(input);
> >>
> >> +       pm_runtime_get_sync(input->dev.parent);
> >
> > I am not an expert of the runtime.
> >
> > However would be grateful if you explain me what it actually do.
> 
> This increase the reference count of the runtime status container
> for the device. _sync makes sure it happens now.
> 
> Consult:
> Documentation/power/runtime_pm.txt
> 
> > Also I did not see any runtime suspend/ resume handlers populated.
> 
> It is not necessary to handle the power state at the driver level,
> it can just as well be handled by the voltage/power domain,
> or at the class, type or bus level.
> 
> But the individual driver has to notify the system upward if it
> needs to be powered on or when it may or must be relaxed.
>
> Yours,
> Linus Walleij

Friendly poke.
Linus Walleij Oct. 25, 2012, 8:01 a.m. UTC | #5
On Thu, Oct 25, 2012 at 9:57 AM, Lee Jones <lee.jones@linaro.org> wrote:
> On Fri, 12 Oct 2012, Linus Walleij wrote:

>> Yours,
>> Linus Walleij
>
> Friendly poke.

This makes it look like you're poking me as I'm in the To: field but I suspect
the intent must be to poke Dmitry ... I was just providing background
for Shubhrajyoti's question.

Maybe this helps:
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij
--
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
Lee Jones Oct. 25, 2012, 8:21 a.m. UTC | #6
On Thu, 25 Oct 2012, Linus Walleij wrote:

> On Thu, Oct 25, 2012 at 9:57 AM, Lee Jones <lee.jones@linaro.org> wrote:
> > On Fri, 12 Oct 2012, Linus Walleij wrote:
> 
> >> Yours,
> >> Linus Walleij
> >
> > Friendly poke.
> 
> This makes it look like you're poking me as I'm in the To: field but I suspect
> the intent must be to poke Dmitry ... I was just providing background
> for Shubhrajyoti's question.
> 
> Maybe this helps:
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yes, that was also the intention of the other poke. Sorry about
that, I should have moved you into Cc: instead.

Yes, the poke was for Dmitry.
Dmitry Torokhov Oct. 25, 2012, 8:30 a.m. UTC | #7
On Thu, Oct 25, 2012 at 09:21:45AM +0100, Lee Jones wrote:
> On Thu, 25 Oct 2012, Linus Walleij wrote:
> 
> > On Thu, Oct 25, 2012 at 9:57 AM, Lee Jones <lee.jones@linaro.org> wrote:
> > > On Fri, 12 Oct 2012, Linus Walleij wrote:
> > 
> > >> Yours,
> > >> Linus Walleij
> > >
> > > Friendly poke.
> > 
> > This makes it look like you're poking me as I'm in the To: field but I suspect
> > the intent must be to poke Dmitry ... I was just providing background
> > for Shubhrajyoti's question.
> > 
> > Maybe this helps:
> > Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> 
> Yes, that was also the intention of the other poke. Sorry about
> that, I should have moved you into Cc: instead.
> 
> Yes, the poke was for Dmitry.

Still do not have the right signoffs: person who sends me the patch needs
to put signed-off-by, not acked-by.

Thanks.
Lee Jones Oct. 25, 2012, 9:47 a.m. UTC | #8
On Thu, 25 Oct 2012, Dmitry Torokhov wrote:

> On Thu, Oct 25, 2012 at 09:21:45AM +0100, Lee Jones wrote:
> > On Thu, 25 Oct 2012, Linus Walleij wrote:
> > 
> > > On Thu, Oct 25, 2012 at 9:57 AM, Lee Jones <lee.jones@linaro.org> wrote:
> > > > On Fri, 12 Oct 2012, Linus Walleij wrote:
> > > 
> > > >> Yours,
> > > >> Linus Walleij
> > > >
> > > > Friendly poke.
> > > 
> > > This makes it look like you're poking me as I'm in the To: field but I suspect
> > > the intent must be to poke Dmitry ... I was just providing background
> > > for Shubhrajyoti's question.
> > > 
> > > Maybe this helps:
> > > Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> > 
> > Yes, that was also the intention of the other poke. Sorry about
> > that, I should have moved you into Cc: instead.
> > 
> > Yes, the poke was for Dmitry.
> 
> Still do not have the right signoffs: person who sends me the patch needs
> to put signed-off-by, not acked-by.

My apologies.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
diff mbox

Patch

diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c
index 7947315..78de557 100644
--- a/drivers/input/keyboard/gpio_keys.c
+++ b/drivers/input/keyboard/gpio_keys.c
@@ -29,6 +29,7 @@ 
 #include <linux/of_platform.h>
 #include <linux/of_gpio.h>
 #include <linux/spinlock.h>
+#include <linux/pm_runtime.h>
 
 struct gpio_button_data {
 	const struct gpio_keys_button *button;
@@ -526,6 +527,7 @@  static int gpio_keys_open(struct input_dev *input)
 {
 	struct gpio_keys_drvdata *ddata = input_get_drvdata(input);
 
+	pm_runtime_get_sync(input->dev.parent);
 	ddata->enabled = true;
 	return ddata->enable ? ddata->enable(input->dev.parent) : 0;
 }
@@ -537,6 +539,7 @@  static void gpio_keys_close(struct input_dev *input)
 	if (ddata->disable)
 		ddata->disable(input->dev.parent);
 	ddata->enabled = false;
+	pm_runtime_put(input->dev.parent);
 }
 
 /*
@@ -695,6 +698,8 @@  static int __devinit gpio_keys_probe(struct platform_device *pdev)
 	input->id.product = 0x0001;
 	input->id.version = 0x0100;
 
+	pm_runtime_enable(&pdev->dev);
+
 	/* Enable auto repeat feature of Linux input subsystem */
 	if (pdata->rep)
 		__set_bit(EV_REP, input->evbit);
@@ -760,6 +765,8 @@  static int __devexit gpio_keys_remove(struct platform_device *pdev)
 	struct input_dev *input = ddata->input;
 	int i;
 
+	pm_runtime_disable(&pdev->dev);
+
 	sysfs_remove_group(&pdev->dev.kobj, &gpio_keys_attr_group);
 
 	device_init_wakeup(&pdev->dev, 0);
@@ -796,8 +803,8 @@  static int gpio_keys_suspend(struct device *dev)
 		}
 	} else {
 		ddata->enable_after_suspend = ddata->enabled;
-		if (ddata->enabled)
-			gpio_keys_close(ddata->input);
+		if (ddata->enabled && ddata->disable)
+			ddata->disable(dev);
 	}
 
 	return 0;
@@ -817,8 +824,9 @@  static int gpio_keys_resume(struct device *dev)
 			gpio_keys_gpio_report_event(bdata);
 	}
 
-	if (!device_may_wakeup(dev) && ddata->enable_after_suspend)
-		gpio_keys_open(ddata->input);
+	if (!device_may_wakeup(dev) && ddata->enable_after_suspend
+	    && ddata->enable)
+		ddata->enable(dev);
 
 	input_sync(ddata->input);