linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] input: gpio-keys: Disable hardware on suspend
@ 2012-10-11 14:15 Lee Jones
  2012-10-11 14:15 ` [PATCH 2/2] input: gpio-keys: Add runtime support Lee Jones
  2012-10-13  6:17 ` [PATCH 1/2] input: gpio-keys: Disable hardware on suspend Dmitry Torokhov
  0 siblings, 2 replies; 16+ messages in thread
From: Lee Jones @ 2012-10-11 14:15 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel
  Cc: arnd, linus.walleij, Jonas Aaberg, Dmitry Torokhov, linux-input,
	Philippe Langlais

From: Jonas Aaberg <jonas.aberg@stericsson.com>

Disable hardware if active when suspending if the hw can not
wake the system from suspend.

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 |   13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c
index cbb1add..7947315 100644
--- a/drivers/input/keyboard/gpio_keys.c
+++ b/drivers/input/keyboard/gpio_keys.c
@@ -46,6 +46,8 @@ struct gpio_keys_drvdata {
 	struct input_dev *input;
 	struct mutex disable_lock;
 	unsigned int n_buttons;
+	bool enabled;
+	bool enable_after_suspend;
 	int (*enable)(struct device *dev);
 	void (*disable)(struct device *dev);
 	struct gpio_button_data data[0];
@@ -524,6 +526,7 @@ static int gpio_keys_open(struct input_dev *input)
 {
 	struct gpio_keys_drvdata *ddata = input_get_drvdata(input);
 
+	ddata->enabled = true;
 	return ddata->enable ? ddata->enable(input->dev.parent) : 0;
 }
 
@@ -533,6 +536,7 @@ static void gpio_keys_close(struct input_dev *input)
 
 	if (ddata->disable)
 		ddata->disable(input->dev.parent);
+	ddata->enabled = false;
 }
 
 /*
@@ -674,6 +678,7 @@ static int __devinit gpio_keys_probe(struct platform_device *pdev)
 	ddata->n_buttons = pdata->nbuttons;
 	ddata->enable = pdata->enable;
 	ddata->disable = pdata->disable;
+	ddata->enabled = false;
 	mutex_init(&ddata->disable_lock);
 
 	platform_set_drvdata(pdev, ddata);
@@ -789,6 +794,10 @@ static int gpio_keys_suspend(struct device *dev)
 			if (bdata->button->wakeup)
 				enable_irq_wake(bdata->irq);
 		}
+	} else {
+		ddata->enable_after_suspend = ddata->enabled;
+		if (ddata->enabled)
+			gpio_keys_close(ddata->input);
 	}
 
 	return 0;
@@ -807,6 +816,10 @@ static int gpio_keys_resume(struct device *dev)
 		if (gpio_is_valid(bdata->button->gpio))
 			gpio_keys_gpio_report_event(bdata);
 	}
+
+	if (!device_may_wakeup(dev) && ddata->enable_after_suspend)
+		gpio_keys_open(ddata->input);
+
 	input_sync(ddata->input);
 
 	return 0;
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 2/2] input: gpio-keys: Add runtime support
  2012-10-11 14:15 [PATCH 1/2] input: gpio-keys: Disable hardware on suspend Lee Jones
@ 2012-10-11 14:15 ` Lee Jones
  2012-10-11 14:22   ` Shubhrajyoti Datta
  2012-10-13  6:17 ` [PATCH 1/2] input: gpio-keys: Disable hardware on suspend Dmitry Torokhov
  1 sibling, 1 reply; 16+ messages in thread
From: Lee Jones @ 2012-10-11 14:15 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel
  Cc: arnd, linus.walleij, Jonas Aaberg, Dmitry Torokhov, linux-input,
	Philippe Langlais

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(-)

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);
 
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/2] input: gpio-keys: Add runtime support
  2012-10-11 14:15 ` [PATCH 2/2] input: gpio-keys: Add runtime support Lee Jones
@ 2012-10-11 14:22   ` Shubhrajyoti Datta
  2012-10-11 15:21     ` Lee Jones
  2012-10-12 21:09     ` Linus Walleij
  0 siblings, 2 replies; 16+ messages in thread
From: Shubhrajyoti Datta @ 2012-10-11 14:22 UTC (permalink / raw)
  To: Lee Jones
  Cc: linux-arm-kernel, linux-kernel, arnd, linus.walleij,
	Jonas Aaberg, Dmitry Torokhov, linux-input, Philippe Langlais

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.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/2] input: gpio-keys: Add runtime support
  2012-10-11 14:22   ` Shubhrajyoti Datta
@ 2012-10-11 15:21     ` Lee Jones
  2012-10-12 21:09     ` Linus Walleij
  1 sibling, 0 replies; 16+ messages in thread
From: Lee Jones @ 2012-10-11 15:21 UTC (permalink / raw)
  To: Shubhrajyoti Datta
  Cc: linux-arm-kernel, linux-kernel, arnd, linus.walleij,
	Jonas Aaberg, Dmitry Torokhov, linux-input, Philippe Langlais

> > 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.

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/2] input: gpio-keys: Add runtime support
  2012-10-11 14:22   ` Shubhrajyoti Datta
  2012-10-11 15:21     ` Lee Jones
@ 2012-10-12 21:09     ` Linus Walleij
  2012-10-25  7:57       ` Lee Jones
  1 sibling, 1 reply; 16+ messages in thread
From: Linus Walleij @ 2012-10-12 21:09 UTC (permalink / raw)
  To: Shubhrajyoti Datta
  Cc: Lee Jones, linux-arm-kernel, linux-kernel, arnd, linus.walleij,
	Jonas Aaberg, Dmitry Torokhov, linux-input, Philippe Langlais

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/2] input: gpio-keys: Disable hardware on suspend
  2012-10-11 14:15 [PATCH 1/2] input: gpio-keys: Disable hardware on suspend Lee Jones
  2012-10-11 14:15 ` [PATCH 2/2] input: gpio-keys: Add runtime support Lee Jones
@ 2012-10-13  6:17 ` Dmitry Torokhov
  1 sibling, 0 replies; 16+ messages in thread
From: Dmitry Torokhov @ 2012-10-13  6:17 UTC (permalink / raw)
  To: Lee Jones
  Cc: linux-arm-kernel, linux-kernel, arnd, linus.walleij,
	Jonas Aaberg, linux-input, Philippe Langlais

Hi Lee,

On Thu, Oct 11, 2012 at 03:15:26PM +0100, Lee Jones wrote:
> From: Jonas Aaberg <jonas.aberg@stericsson.com>
> 
> Disable hardware if active when suspending if the hw can not
> wake the system from suspend.
> 
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Cc: linux-input@vger.kernel.org
> Acked-by: Lee Jones <lee.jones@linaro.org>

If you are sending the patch then it should be signed-off-by, not
acked-by, and it should be the very last entry.

> 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 |   13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c
> index cbb1add..7947315 100644
> --- a/drivers/input/keyboard/gpio_keys.c
> +++ b/drivers/input/keyboard/gpio_keys.c
> @@ -46,6 +46,8 @@ struct gpio_keys_drvdata {
>  	struct input_dev *input;
>  	struct mutex disable_lock;
>  	unsigned int n_buttons;
> +	bool enabled;
> +	bool enable_after_suspend;
>  	int (*enable)(struct device *dev);
>  	void (*disable)(struct device *dev);
>  	struct gpio_button_data data[0];
> @@ -524,6 +526,7 @@ static int gpio_keys_open(struct input_dev *input)
>  {
>  	struct gpio_keys_drvdata *ddata = input_get_drvdata(input);
>  
> +	ddata->enabled = true;
>  	return ddata->enable ? ddata->enable(input->dev.parent) : 0;
>  }
>  
> @@ -533,6 +536,7 @@ static void gpio_keys_close(struct input_dev *input)
>  
>  	if (ddata->disable)
>  		ddata->disable(input->dev.parent);
> +	ddata->enabled = false;
>  }
>  
>  /*
> @@ -674,6 +678,7 @@ static int __devinit gpio_keys_probe(struct platform_device *pdev)
>  	ddata->n_buttons = pdata->nbuttons;
>  	ddata->enable = pdata->enable;
>  	ddata->disable = pdata->disable;
> +	ddata->enabled = false;
>  	mutex_init(&ddata->disable_lock);
>  
>  	platform_set_drvdata(pdev, ddata);
> @@ -789,6 +794,10 @@ static int gpio_keys_suspend(struct device *dev)
>  			if (bdata->button->wakeup)
>  				enable_irq_wake(bdata->irq);
>  		}
> +	} else {
> +		ddata->enable_after_suspend = ddata->enabled;
> +		if (ddata->enabled)
> +			gpio_keys_close(ddata->input);

I think it should take ddata->input->mutex to avoid races with
open/close and use ddata->input->users to figure out if device shoudl be
closed and re-opened later.

Thanks.

>  	}
>  
>  	return 0;
> @@ -807,6 +816,10 @@ static int gpio_keys_resume(struct device *dev)
>  		if (gpio_is_valid(bdata->button->gpio))
>  			gpio_keys_gpio_report_event(bdata);
>  	}
> +
> +	if (!device_may_wakeup(dev) && ddata->enable_after_suspend)
> +		gpio_keys_open(ddata->input);
> +
>  	input_sync(ddata->input);
>  
>  	return 0;
> -- 
> 1.7.9.5
> 

-- 
Dmitry

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/2] input: gpio-keys: Add runtime support
  2012-10-12 21:09     ` Linus Walleij
@ 2012-10-25  7:57       ` Lee Jones
  2012-10-25  8:01         ` Linus Walleij
  0 siblings, 1 reply; 16+ messages in thread
From: Lee Jones @ 2012-10-25  7:57 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Shubhrajyoti Datta, linux-arm-kernel, linux-kernel, arnd,
	linus.walleij, Jonas Aaberg, Dmitry Torokhov, linux-input,
	Philippe Langlais

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.

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/2] input: gpio-keys: Add runtime support
  2012-10-25  7:57       ` Lee Jones
@ 2012-10-25  8:01         ` Linus Walleij
  2012-10-25  8:21           ` Lee Jones
  0 siblings, 1 reply; 16+ messages in thread
From: Linus Walleij @ 2012-10-25  8:01 UTC (permalink / raw)
  To: Lee Jones, Dmitry Torokhov
  Cc: Shubhrajyoti Datta, linux-arm-kernel, linux-kernel, arnd,
	linus.walleij, Jonas Aaberg, linux-input, Philippe Langlais

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/2] input: gpio-keys: Add runtime support
  2012-10-25  8:01         ` Linus Walleij
@ 2012-10-25  8:21           ` Lee Jones
  2012-10-25  8:30             ` Dmitry Torokhov
  0 siblings, 1 reply; 16+ messages in thread
From: Lee Jones @ 2012-10-25  8:21 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Dmitry Torokhov, Shubhrajyoti Datta, linux-arm-kernel,
	linux-kernel, arnd, linus.walleij, Jonas Aaberg, linux-input,
	Philippe Langlais

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.

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/2] input: gpio-keys: Add runtime support
  2012-10-25  8:21           ` Lee Jones
@ 2012-10-25  8:30             ` Dmitry Torokhov
  2012-10-25  9:47               ` Lee Jones
  0 siblings, 1 reply; 16+ messages in thread
From: Dmitry Torokhov @ 2012-10-25  8:30 UTC (permalink / raw)
  To: Lee Jones
  Cc: Linus Walleij, Shubhrajyoti Datta, linux-arm-kernel,
	linux-kernel, arnd, linus.walleij, Jonas Aaberg, linux-input,
	Philippe Langlais

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.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/2] input: gpio-keys: Add runtime support
  2012-10-25  8:30             ` Dmitry Torokhov
@ 2012-10-25  9:47               ` Lee Jones
  0 siblings, 0 replies; 16+ messages in thread
From: Lee Jones @ 2012-10-25  9:47 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Linus Walleij, Shubhrajyoti Datta, linux-arm-kernel,
	linux-kernel, arnd, linus.walleij, Jonas Aaberg, linux-input,
	Philippe Langlais

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>

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/2] input: gpio-keys: Disable hardware on suspend
  2012-11-25  8:32 ` Dmitry Torokhov
  2012-11-26  9:05   ` Lee Jones
@ 2012-12-01 17:01   ` Linus Walleij
  1 sibling, 0 replies; 16+ messages in thread
From: Linus Walleij @ 2012-12-01 17:01 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Lee Jones, linux-arm-kernel, linux-kernel, arnd, linus.walleij,
	Jonas Aaberg, linux-input, Philippe Langlais

On Sun, Nov 25, 2012 at 9:32 AM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> On Thu, Nov 22, 2012 at 01:23:36PM +0000, Lee Jones wrote:
>> From: Jonas Aaberg <jonas.aberg@stericsson.com>
>>
>> Disable hardware if active when suspending if the hw can not
>> wake the system from suspend.
>
> How about below instead (needs the other patch I have just sent out)?

LooksGoodToMe(TM)

Acked-by.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/2] input: gpio-keys: Disable hardware on suspend
  2012-11-26  9:05   ` Lee Jones
@ 2012-11-26  9:32     ` Dmitry Torokhov
  0 siblings, 0 replies; 16+ messages in thread
From: Dmitry Torokhov @ 2012-11-26  9:32 UTC (permalink / raw)
  To: Lee Jones
  Cc: linux-arm-kernel, linux-kernel, arnd, linus.walleij,
	Jonas Aaberg, linux-input, Philippe Langlais

On Mon, Nov 26, 2012 at 09:05:03AM +0000, Lee Jones wrote:
> > On Thu, Nov 22, 2012 at 01:23:36PM +0000, Lee Jones wrote:
> > > From: Jonas Aaberg <jonas.aberg@stericsson.com>
> > > 
> > > Disable hardware if active when suspending if the hw can not
> > > wake the system from suspend.
> > 
> > How about below instead (needs the other patch I have just sent out)?
> 
> Sorry Dmitry, I'm a little confused. What does this mean?

The modified version of the patch I have sent depends on the other patch
to gpio-keys that introduces gpio_keys_report_state() that I believe you
have been CCed.

Thanks.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/2] input: gpio-keys: Disable hardware on suspend
  2012-11-25  8:32 ` Dmitry Torokhov
@ 2012-11-26  9:05   ` Lee Jones
  2012-11-26  9:32     ` Dmitry Torokhov
  2012-12-01 17:01   ` Linus Walleij
  1 sibling, 1 reply; 16+ messages in thread
From: Lee Jones @ 2012-11-26  9:05 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: linux-arm-kernel, linux-kernel, arnd, linus.walleij,
	Jonas Aaberg, linux-input, Philippe Langlais

> On Thu, Nov 22, 2012 at 01:23:36PM +0000, Lee Jones wrote:
> > From: Jonas Aaberg <jonas.aberg@stericsson.com>
> > 
> > Disable hardware if active when suspending if the hw can not
> > wake the system from suspend.
> 
> How about below instead (needs the other patch I have just sent out)?

Sorry Dmitry, I'm a little confused. What does this mean?

> Input: gpio-keys - disable hardware on suspend
> 
> From: Jonas Aaberg <jonas.aberg@stericsson.com>
> 
> Disable hardware if active when suspending if the hw can not
> wake the system from suspend.
> 
> 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>
> Signed-off-by: Lee Jones <lee.jones@linaro.org>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
>  drivers/input/keyboard/gpio_keys.c |   27 ++++++++++++++++++++++-----
>  1 file changed, 22 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c
> index c7764ca..79435de 100644
> --- a/drivers/input/keyboard/gpio_keys.c
> +++ b/drivers/input/keyboard/gpio_keys.c
> @@ -803,6 +803,7 @@ static int gpio_keys_remove(struct platform_device *pdev)
>  static int gpio_keys_suspend(struct device *dev)
>  {
>  	struct gpio_keys_drvdata *ddata = dev_get_drvdata(dev);
> +	struct input_dev *input = ddata->input;
>  	int i;
>  
>  	if (device_may_wakeup(dev)) {
> @@ -811,6 +812,11 @@ static int gpio_keys_suspend(struct device *dev)
>  			if (bdata->button->wakeup)
>  				enable_irq_wake(bdata->irq);
>  		}
> +	} else {
> +		mutex_lock(&input->mutex);
> +		if (input->users)
> +			gpio_keys_close(input);
> +		mutex_unlock(&input->mutex);
>  	}
>  
>  	return 0;
> @@ -819,16 +825,27 @@ static int gpio_keys_suspend(struct device *dev)
>  static int gpio_keys_resume(struct device *dev)
>  {
>  	struct gpio_keys_drvdata *ddata = dev_get_drvdata(dev);
> +	struct input_dev *input = ddata->input;
> +	int error = 0;
>  	int i;
>  
> -	for (i = 0; i < ddata->pdata->nbuttons; i++) {
> -		struct gpio_button_data *bdata = &ddata->data[i];
> -		if (bdata->button->wakeup && device_may_wakeup(dev))
> -			disable_irq_wake(bdata->irq);
> +	if (device_may_wakeup(dev)) {
> +		for (i = 0; i < ddata->pdata->nbuttons; i++) {
> +			struct gpio_button_data *bdata = &ddata->data[i];
> +			if (bdata->button->wakeup)
> +				disable_irq_wake(bdata->irq);
> +		}
> +	} else {
> +		mutex_lock(&input->mutex);
> +		if (input->users)
> +			error = gpio_keys_open(input);
> +		mutex_unlock(&input->mutex);
>  	}
>  
> -	gpio_keys_report_state(ddata);
> +	if (error)
> +		return error;
>  
> +	gpio_keys_report_state(ddata);
>  	return 0;
>  }
>  #endif

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/2] input: gpio-keys: Disable hardware on suspend
  2012-11-22 13:23 Lee Jones
@ 2012-11-25  8:32 ` Dmitry Torokhov
  2012-11-26  9:05   ` Lee Jones
  2012-12-01 17:01   ` Linus Walleij
  0 siblings, 2 replies; 16+ messages in thread
From: Dmitry Torokhov @ 2012-11-25  8:32 UTC (permalink / raw)
  To: Lee Jones
  Cc: linux-arm-kernel, linux-kernel, arnd, linus.walleij,
	Jonas Aaberg, linux-input, Philippe Langlais

On Thu, Nov 22, 2012 at 01:23:36PM +0000, Lee Jones wrote:
> From: Jonas Aaberg <jonas.aberg@stericsson.com>
> 
> Disable hardware if active when suspending if the hw can not
> wake the system from suspend.

How about below instead (needs the other patch I have just sent out)?

Thanks.

-- 
Dmitry

Input: gpio-keys - disable hardware on suspend

From: Jonas Aaberg <jonas.aberg@stericsson.com>

Disable hardware if active when suspending if the hw can not
wake the system from suspend.

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>
Signed-off-by: Lee Jones <lee.jones@linaro.org>
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/keyboard/gpio_keys.c |   27 ++++++++++++++++++++++-----
 1 file changed, 22 insertions(+), 5 deletions(-)

diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c
index c7764ca..79435de 100644
--- a/drivers/input/keyboard/gpio_keys.c
+++ b/drivers/input/keyboard/gpio_keys.c
@@ -803,6 +803,7 @@ static int gpio_keys_remove(struct platform_device *pdev)
 static int gpio_keys_suspend(struct device *dev)
 {
 	struct gpio_keys_drvdata *ddata = dev_get_drvdata(dev);
+	struct input_dev *input = ddata->input;
 	int i;
 
 	if (device_may_wakeup(dev)) {
@@ -811,6 +812,11 @@ static int gpio_keys_suspend(struct device *dev)
 			if (bdata->button->wakeup)
 				enable_irq_wake(bdata->irq);
 		}
+	} else {
+		mutex_lock(&input->mutex);
+		if (input->users)
+			gpio_keys_close(input);
+		mutex_unlock(&input->mutex);
 	}
 
 	return 0;
@@ -819,16 +825,27 @@ static int gpio_keys_suspend(struct device *dev)
 static int gpio_keys_resume(struct device *dev)
 {
 	struct gpio_keys_drvdata *ddata = dev_get_drvdata(dev);
+	struct input_dev *input = ddata->input;
+	int error = 0;
 	int i;
 
-	for (i = 0; i < ddata->pdata->nbuttons; i++) {
-		struct gpio_button_data *bdata = &ddata->data[i];
-		if (bdata->button->wakeup && device_may_wakeup(dev))
-			disable_irq_wake(bdata->irq);
+	if (device_may_wakeup(dev)) {
+		for (i = 0; i < ddata->pdata->nbuttons; i++) {
+			struct gpio_button_data *bdata = &ddata->data[i];
+			if (bdata->button->wakeup)
+				disable_irq_wake(bdata->irq);
+		}
+	} else {
+		mutex_lock(&input->mutex);
+		if (input->users)
+			error = gpio_keys_open(input);
+		mutex_unlock(&input->mutex);
 	}
 
-	gpio_keys_report_state(ddata);
+	if (error)
+		return error;
 
+	gpio_keys_report_state(ddata);
 	return 0;
 }
 #endif

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 1/2] input: gpio-keys: Disable hardware on suspend
@ 2012-11-22 13:23 Lee Jones
  2012-11-25  8:32 ` Dmitry Torokhov
  0 siblings, 1 reply; 16+ messages in thread
From: Lee Jones @ 2012-11-22 13:23 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel
  Cc: arnd, linus.walleij, Jonas Aaberg, Dmitry Torokhov, linux-input,
	Lee Jones, Philippe Langlais

From: Jonas Aaberg <jonas.aberg@stericsson.com>

Disable hardware if active when suspending if the hw can not
wake the system from suspend.

Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: linux-input@vger.kernel.org
Signed-off-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 |   15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c
index 6a68041..2cf6757 100644
--- a/drivers/input/keyboard/gpio_keys.c
+++ b/drivers/input/keyboard/gpio_keys.c
@@ -46,6 +46,8 @@ struct gpio_keys_drvdata {
 	const struct gpio_keys_platform_data *pdata;
 	struct input_dev *input;
 	struct mutex disable_lock;
+	bool enabled;
+	bool enable_after_suspend;
 	struct gpio_button_data data[0];
 };
 
@@ -531,6 +533,8 @@ static int gpio_keys_open(struct input_dev *input)
 	struct gpio_keys_drvdata *ddata = input_get_drvdata(input);
 	const struct gpio_keys_platform_data *pdata = ddata->pdata;
 
+	ddata->enabled = true;
+
 	return pdata->enable ? pdata->enable(input->dev.parent) : 0;
 }
 
@@ -539,6 +543,8 @@ static void gpio_keys_close(struct input_dev *input)
 	struct gpio_keys_drvdata *ddata = input_get_drvdata(input);
 	const struct gpio_keys_platform_data *pdata = ddata->pdata;
 
+	ddata->enabled = false;
+
 	if (pdata->disable)
 		pdata->disable(input->dev.parent);
 }
@@ -685,6 +691,7 @@ static int __devinit gpio_keys_probe(struct platform_device *pdev)
 
 	ddata->pdata = pdata;
 	ddata->input = input;
+	ddata->enabled = false;
 	mutex_init(&ddata->disable_lock);
 
 	platform_set_drvdata(pdev, ddata);
@@ -796,6 +803,10 @@ static int gpio_keys_suspend(struct device *dev)
 			if (bdata->button->wakeup)
 				enable_irq_wake(bdata->irq);
 		}
+	} else {
+		ddata->enable_after_suspend = ddata->enabled;
+		if (ddata->enabled)
+			gpio_keys_close(ddata->input);
 	}
 
 	return 0;
@@ -814,6 +825,10 @@ static int gpio_keys_resume(struct device *dev)
 		if (gpio_is_valid(bdata->button->gpio))
 			gpio_keys_gpio_report_event(bdata);
 	}
+
+	if (!device_may_wakeup(dev) && ddata->enable_after_suspend)
+		gpio_keys_open(ddata->input);
+
 	input_sync(ddata->input);
 
 	return 0;
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2012-12-01 17:01 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-10-11 14:15 [PATCH 1/2] input: gpio-keys: Disable hardware on suspend Lee Jones
2012-10-11 14:15 ` [PATCH 2/2] input: gpio-keys: Add runtime support Lee Jones
2012-10-11 14:22   ` Shubhrajyoti Datta
2012-10-11 15:21     ` Lee Jones
2012-10-12 21:09     ` Linus Walleij
2012-10-25  7:57       ` Lee Jones
2012-10-25  8:01         ` Linus Walleij
2012-10-25  8:21           ` Lee Jones
2012-10-25  8:30             ` Dmitry Torokhov
2012-10-25  9:47               ` Lee Jones
2012-10-13  6:17 ` [PATCH 1/2] input: gpio-keys: Disable hardware on suspend Dmitry Torokhov
2012-11-22 13:23 Lee Jones
2012-11-25  8:32 ` Dmitry Torokhov
2012-11-26  9:05   ` Lee Jones
2012-11-26  9:32     ` Dmitry Torokhov
2012-12-01 17:01   ` Linus Walleij

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).