[PATCH 0/2] power button repeat input events

[PATCH 0/2] power button repeat input events

[Denis Karpov]

Hi,

this patchset adds optional generation of repeat input events for
twl4030 power button. It adds optional platform data for 
twl4030-pwrbutton and initializes it for RX51/N900 (default 
behavior kept - repeated input events disabled).

We are working on an OMAP3 with TWL5031 MFD. In one of the use 
scenarios userspace wants to receive repeated input events from 
long press of power button. 

--
Denis

Denis Karpov (2):
      input: twl4030-pwrbutton: add support for platform data
      omap: rx51: initialize platform data for twl4030 powerbutton

 arch/arm/mach-omap2/board-rx51-peripherals.c |    5 +++++
 drivers/input/misc/twl4030-pwrbutton.c       |    8 ++++++++
 drivers/mfd/twl-core.c                       |    4 +++-
 include/linux/i2c/twl.h                      |    5 +++++
 4 files changed, 21 insertions(+), 1 deletions(-)

[PATCH 1/2] input: twl4030-pwrbutton: add support for platform data

[Denis Karpov]

From: Denis Karpov <ext-denis.2.karpov@nokia.com>

Adds support for TWL4030 power button platform data. Currently the only
field is the flag to optionally generate repeat input events from a button
press.

Signed-off-by: Denis Karpov <ext-denis.2.karpov@nokia.com>
---
 drivers/input/misc/twl4030-pwrbutton.c |    8 ++++++++
 drivers/mfd/twl-core.c                 |    4 +++-
 include/linux/i2c/twl.h                |    5 +++++
 3 files changed, 16 insertions(+), 1 deletions(-)

diff --git a/drivers/input/misc/twl4030-pwrbutton.c b/drivers/input/misc/twl4030-pwrbutton.c
index f16972b..c380b83 100644
--- a/drivers/input/misc/twl4030-pwrbutton.c
+++ b/drivers/input/misc/twl4030-pwrbutton.c
@@ -54,9 +54,14 @@ static irqreturn_t powerbutton_irq(int irq, void *_pwr)
 
 static int __init twl4030_pwrbutton_probe(struct platform_device *pdev)
 {
+	struct twl4030_pwrbutton_data *pdata = pdev->dev.platform_data;
 	struct input_dev *pwr;
 	int irq = platform_get_irq(pdev, 0);
 	int err;
+	bool repeat = false;
+
+	if (pdata)
+		repeat = pdata->rep;
 
 	pwr = input_allocate_device();
 	if (!pwr) {
@@ -70,6 +75,9 @@ static int __init twl4030_pwrbutton_probe(struct platform_device *pdev)
 	pwr->phys = "twl4030_pwrbutton/input0";
 	pwr->dev.parent = &pdev->dev;
 
+	if (repeat)
+		__set_bit(EV_REP, pwr->evbit);
+
 	err = request_threaded_irq(irq, NULL, powerbutton_irq,
 			IRQF_TRIGGER_FALLING | IRQF_TRIGGER_RISING,
 			"twl4030_pwrbutton", pwr);
diff --git a/drivers/mfd/twl-core.c b/drivers/mfd/twl-core.c
index a35fa7d..fd75ee9 100644
--- a/drivers/mfd/twl-core.c
+++ b/drivers/mfd/twl-core.c
@@ -729,7 +729,9 @@ add_children(struct twl4030_platform_data *pdata, unsigned long features)
 
 	if (twl_has_pwrbutton()) {
 		child = add_child(1, "twl4030_pwrbutton",
-				NULL, 0, true, pdata->irq_base + 8 + 0, 0);
+				pdata->pwrbutton,
+				sizeof(*pdata->pwrbutton),
+				true, pdata->irq_base + 8 + 0, 0);
 		if (IS_ERR(child))
 			return PTR_ERR(child);
 	}
diff --git a/include/linux/i2c/twl.h b/include/linux/i2c/twl.h
index 9d88b71..60e48fa 100644
--- a/include/linux/i2c/twl.h
+++ b/include/linux/i2c/twl.h
@@ -586,6 +586,10 @@ struct twl4030_keypad_data {
 	bool rep;
 };
 
+struct twl4030_pwrbutton_data {
+	bool rep;
+};
+
 enum twl4030_usb_mode {
 	T2_USB_MODE_ULPI = 1,
 	T2_USB_MODE_CEA2011_3PIN = 2,
@@ -667,6 +671,7 @@ struct twl4030_platform_data {
 	struct twl4030_gpio_platform_data	*gpio;
 	struct twl4030_madc_platform_data	*madc;
 	struct twl4030_keypad_data		*keypad;
+	struct twl4030_pwrbutton_data		*pwrbutton;
 	struct twl4030_usb_data			*usb;
 	struct twl4030_power_data		*power;
 	struct twl4030_codec_data		*codec;
-- 
1.7.1

[PATCH 2/2] omap: rx51: initialize platform data for twl4030 powerbutton

[Denis Karpov]

From: Denis Karpov <ext-denis.2.karpov@nokia.com>

Adds and initializes platform data for TWL4030 powerbutton.
Disable repeat input events from a button press (default behaviour).

Signed-off-by: Denis Karpov <ext-denis.2.karpov@nokia.com>
---
 arch/arm/mach-omap2/board-rx51-peripherals.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/board-rx51-peripherals.c b/arch/arm/mach-omap2/board-rx51-peripherals.c
index 5f1900c..c9a21ef 100644
--- a/arch/arm/mach-omap2/board-rx51-peripherals.c
+++ b/arch/arm/mach-omap2/board-rx51-peripherals.c
@@ -264,6 +264,10 @@ static struct twl4030_keypad_data rx51_kp_data = {
 	.rep		= 1,
 };
 
+static struct twl4030_pwrbutton_data rx51_pwrbutton_data = {
+	.rep		= 1,
+};
+
 static struct twl4030_madc_platform_data rx51_madc_data = {
 	.irq_line		= 1,
 };
@@ -758,6 +762,7 @@ static struct twl4030_platform_data rx51_twldata __initdata = {
 	/* platform_data for children goes here */
 	.gpio			= &rx51_gpio_data,
 	.keypad			= &rx51_kp_data,
+	.pwrbutton		= &rx51_pwrbutton_data,
 	.madc			= &rx51_madc_data,
 	.usb			= &rx51_usb_data,
 	.power			= &rx51_t2scripts_data,
-- 
1.7.1

Re: [PATCH 0/2] power button repeat input events

[Nishanth Menon]

Denis Karpov wrote, on 03/11/2011 10:07 PM:
> Hi,
>
> this patchset adds optional generation of repeat input events for
> twl4030 power button. It adds optional platform data for
> twl4030-pwrbutton and initializes it for RX51/N900 (default
> behavior kept - repeated input events disabled).
>
> We are working on an OMAP3 with TWL5031 MFD. In one of the use
> scenarios userspace wants to receive repeated input events from
> long press of power button.
>

dumb question - is'nt this a userspace responsibility as far as power 
button is concerned? what is the benefit of providing repeat events like 
a generic keypad for the power button?

-- 
Regards,
Nishanth Menon

Re: [PATCH 0/2] power button repeat input events

[Tony Lindgren]

Hi,

* Denis Karpov <ext-denis.2.karpov@nokia.com> [110311 08:35]:
> Hi,
> 
> this patchset adds optional generation of repeat input events for
> twl4030 power button. It adds optional platform data for 
> twl4030-pwrbutton and initializes it for RX51/N900 (default 
> behavior kept - repeated input events disabled).
> 
> We are working on an OMAP3 with TWL5031 MFD. In one of the use 
> scenarios userspace wants to receive repeated input events from 
> long press of power button. 

Looks good to me, however please send these to the associated
driver lists linux-input/mfd/i2c etc. I'll ack the second
patch so that too can go with the driver.

Regards,

Tony

Re: [PATCH 2/2] omap: rx51: initialize platform data for twl4030 powerbutton

[Tony Lindgren]

* Denis Karpov <ext-denis.2.karpov@nokia.com> [110311 08:35]:
> From: Denis Karpov <ext-denis.2.karpov@nokia.com>
> 
> Adds and initializes platform data for TWL4030 powerbutton.
> Disable repeat input events from a button press (default behaviour).
> 
> Signed-off-by: Denis Karpov <ext-denis.2.karpov@nokia.com>

This should get merged along with the driver related patch:

Acked-by: Tony Lindgren <tony@atomide.com>

> ---
>  arch/arm/mach-omap2/board-rx51-peripherals.c |    5 +++++
>  1 files changed, 5 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/board-rx51-peripherals.c b/arch/arm/mach-omap2/board-rx51-peripherals.c
> index 5f1900c..c9a21ef 100644
> --- a/arch/arm/mach-omap2/board-rx51-peripherals.c
> +++ b/arch/arm/mach-omap2/board-rx51-peripherals.c
> @@ -264,6 +264,10 @@ static struct twl4030_keypad_data rx51_kp_data = {
>  	.rep		= 1,
>  };
>  
> +static struct twl4030_pwrbutton_data rx51_pwrbutton_data = {
> +	.rep		= 1,
> +};
> +
>  static struct twl4030_madc_platform_data rx51_madc_data = {
>  	.irq_line		= 1,
>  };
> @@ -758,6 +762,7 @@ static struct twl4030_platform_data rx51_twldata __initdata = {
>  	/* platform_data for children goes here */
>  	.gpio			= &rx51_gpio_data,
>  	.keypad			= &rx51_kp_data,
> +	.pwrbutton		= &rx51_pwrbutton_data,
>  	.madc			= &rx51_madc_data,
>  	.usb			= &rx51_usb_data,
>  	.power			= &rx51_t2scripts_data,
> -- 
> 1.7.1
> 

Re: [PATCH 2/2] omap: rx51: initialize platform data for twl4030 powerbutton

[Aaro Koskinen]

Hi,

On Fri, 11 Mar 2011, Denis Karpov wrote:
> From: Denis Karpov <ext-denis.2.karpov@nokia.com>
>
> Adds and initializes platform data for TWL4030 powerbutton.
> Disable repeat input events from a button press (default behaviour).

[...]

> +static struct twl4030_pwrbutton_data rx51_pwrbutton_data = {
> +	.rep		= 1,
> +};
> +

Doesn't this enable them? If you want to keep the default behaviour,
you can just omit the platform data.

> static struct twl4030_madc_platform_data rx51_madc_data = {
> 	.irq_line		= 1,
> };
> @@ -758,6 +762,7 @@ static struct twl4030_platform_data rx51_twldata __initdata = {
> 	/* platform_data for children goes here */
> 	.gpio			= &rx51_gpio_data,
> 	.keypad			= &rx51_kp_data,
> +	.pwrbutton		= &rx51_pwrbutton_data,

A.

Re: [PATCH 0/2] power button repeat input events

[Denis Karpov]

On Fri, Mar 11, 2011 at 10:25:27PM +0530, ext Nishanth Menon wrote:
> 
> Denis Karpov wrote, on 03/11/2011 10:07 PM:
> >Hi,
> >
> >this patchset adds optional generation of repeat input events for
> >twl4030 power button. It adds optional platform data for
> >twl4030-pwrbutton and initializes it for RX51/N900 (default
> >behavior kept - repeated input events disabled).
> >
> >We are working on an OMAP3 with TWL5031 MFD. In one of the use
> >scenarios userspace wants to receive repeated input events from
> >long press of power button.
> 
> dumb question - is'nt this a userspace responsibility as far as power
> button is concerned? what is the benefit of providing repeat events
> like a generic keypad for the power button?

In our case userspace needs this to track the continuous button press 
of power button. 

Denis

Re: [PATCH 2/2] omap: rx51: initialize platform data for twl4030 powerbutton

[Denis Karpov]

> >From: Denis Karpov <ext-denis.2.karpov@nokia.com>
> >+static struct twl4030_pwrbutton_data rx51_pwrbutton_data = {
> >+	.rep		= 1,
> >+};
> >+
> 
> Doesn't this enable them? If you want to keep the default behaviour,
> you can just omit the platform data.

Yes, you are right, thanks. I meant to set it to 0 after testing, but 
then it is not needed indeed. 

So patch 2/2 is not needed and patch 1/2 does not belong here - I'll 
resend it to input/mfd lists/maintainers.

Denis

Re: [PATCH 0/2] power button repeat input events

[Menon, Nishanth]

On Mon, Mar 14, 2011 at 16:43, Denis Karpov
<ext-denis.2.karpov@nokia.com> wrote:
>
> On Fri, Mar 11, 2011 at 10:25:27PM +0530, ext Nishanth Menon wrote:
> >
> > Denis Karpov wrote, on 03/11/2011 10:07 PM:
> > >Hi,
> > >
> > >this patchset adds optional generation of repeat input events for
> > >twl4030 power button. It adds optional platform data for
> > >twl4030-pwrbutton and initializes it for RX51/N900 (default
> > >behavior kept - repeated input events disabled).
> > >
> > >We are working on an OMAP3 with TWL5031 MFD. In one of the use
> > >scenarios userspace wants to receive repeated input events from
> > >long press of power button.
> >
> > dumb question - is'nt this a userspace responsibility as far as power
> > button is concerned? what is the benefit of providing repeat events
> > like a generic keypad for the power button?
>
> In our case userspace needs this to track the continuous button press
> of power button.
thanks for your explanation, but a follow on dumb question:
include/linux/input.h
#define KEY_POWER               116     /* SC System Power Down */

in twl-powerbutton.c, I see:
input_report_key(pwr, KEY_POWER, value & PWR_PWRON_IRQ);
where value is the value from the register status, should'nt we get
two events? one on press and one on release after say 10 mins(assuming
person keeps it pressed for 10 mins?

Is there different behavior we intend to implement in userspace which
differentiates 1 KEY_POWER event Vs 10 KEY_POWER event?

Regards,
Nishanth Menon

Re: [PATCH 0/2] power button repeat input events

[Denis Karpov]

On Mon, Mar 14, 2011 at 05:10:22PM +0530, ext Menon, Nishanth wrote:
> Date: Mon, 14 Mar 2011 17:10:22 +0530
> From: "ext Menon, Nishanth" <nm@ti.com>
> To: Denis Karpov <ext-denis.2.karpov@nokia.com>
> CC: tony@atomide.com, linux-omap@vger.kernel.org
> Subject: Re: [PATCH 0/2] power button repeat input events
> In-Reply-To: <20110314111327.GA10563@smart.research.nokia.com>
> 
> On Mon, Mar 14, 2011 at 16:43, Denis Karpov
> <ext-denis.2.karpov@nokia.com> wrote:
> >
> > On Fri, Mar 11, 2011 at 10:25:27PM +0530, ext Nishanth Menon wrote:
> > >
> > > Denis Karpov wrote, on 03/11/2011 10:07 PM:
> > > >Hi,
> > > >
> > > >this patchset adds optional generation of repeat input events for
> > > >twl4030 power button. It adds optional platform data for
> > > >twl4030-pwrbutton and initializes it for RX51/N900 (default
> > > >behavior kept - repeated input events disabled).
> > > >
> > > >We are working on an OMAP3 with TWL5031 MFD. In one of the use
> > > >scenarios userspace wants to receive repeated input events from
> > > >long press of power button.
> > >
> > > dumb question - is'nt this a userspace responsibility as far as power
> > > button is concerned? what is the benefit of providing repeat events
> > > like a generic keypad for the power button?
> >
> > In our case userspace needs this to track the continuous button press
> > of power button.
> thanks for your explanation, but a follow on dumb question:
> include/linux/input.h
> #define KEY_POWER               116     /* SC System Power Down */
> 
> in twl-powerbutton.c, I see:
> input_report_key(pwr, KEY_POWER, value & PWR_PWRON_IRQ);
> where value is the value from the register status, should'nt we get
> two events? one on press and one on release after say 10 mins(assuming
> person keeps it pressed for 10 mins?
> 
> Is there different behavior we intend to implement in userspace which
> differentiates 1 KEY_POWER event Vs 10 KEY_POWER event?

No, but due to specifics of userspace sw components involved it seems  
not to be possible to reliably save the state for power button to detect 
long keypresses.

I thought that this change if implemented as optional feature and 
localized to scpecific devices/boards would not hurt. 
twl-keypad.c implements exactly same behavior for keypad keys, why not 
to do this for the power key ?

Denis

Re: [PATCH 0/2] power button repeat input events

[Nishanth Menon]

Denis Karpov wrote, on 03/14/2011 08:05 PM:
> On Mon, Mar 14, 2011 at 05:10:22PM +0530, ext Menon, Nishanth wrote:
>> Date: Mon, 14 Mar 2011 17:10:22 +0530
>> From: "ext Menon, Nishanth"<nm@ti.com>
>> To: Denis Karpov<ext-denis.2.karpov@nokia.com>
>> CC: tony@atomide.com, linux-omap@vger.kernel.org
>> Subject: Re: [PATCH 0/2] power button repeat input events
>> In-Reply-To:<20110314111327.GA10563@smart.research.nokia.com>
>>
>> On Mon, Mar 14, 2011 at 16:43, Denis Karpov
>> <ext-denis.2.karpov@nokia.com>  wrote:
>>>
>>> On Fri, Mar 11, 2011 at 10:25:27PM +0530, ext Nishanth Menon wrote:
>>>>
>>>> Denis Karpov wrote, on 03/11/2011 10:07 PM:
>>>>> Hi,
>>>>>
>>>>> this patchset adds optional generation of repeat input events for
>>>>> twl4030 power button. It adds optional platform data for
>>>>> twl4030-pwrbutton and initializes it for RX51/N900 (default
>>>>> behavior kept - repeated input events disabled).
>>>>>
>>>>> We are working on an OMAP3 with TWL5031 MFD. In one of the use
>>>>> scenarios userspace wants to receive repeated input events from
>>>>> long press of power button.
>>>>
>>>> dumb question - is'nt this a userspace responsibility as far as power
>>>> button is concerned? what is the benefit of providing repeat events
>>>> like a generic keypad for the power button?
>>>
>>> In our case userspace needs this to track the continuous button press
>>> of power button.
>> thanks for your explanation, but a follow on dumb question:
>> include/linux/input.h
>> #define KEY_POWER               116     /* SC System Power Down */
>>
>> in twl-powerbutton.c, I see:
>> input_report_key(pwr, KEY_POWER, value&  PWR_PWRON_IRQ);
>> where value is the value from the register status, should'nt we get
>> two events? one on press and one on release after say 10 mins(assuming
>> person keeps it pressed for 10 mins?
>>
>> Is there different behavior we intend to implement in userspace which
>> differentiates 1 KEY_POWER event Vs 10 KEY_POWER event?
>
> No, but due to specifics of userspace sw components involved it seems
> not to be possible to reliably save the state for power button to detect
> long keypresses.
>
> I thought that this change if implemented as optional feature and
> localized to scpecific devices/boards would not hurt.
> twl-keypad.c implements exactly same behavior for keypad keys, why not
> to do this for the power key ?
Are'nt the key events different? KEY_POWER is meant to shutdown the 
system if my reading is right (other keys need repeatability) - I mean 
trying to add a feature because userspace handling is flawed in some 
manner makes me wonder if we are moving the code to kernelspace because 
it is convenient?

I have no debates about just the board in question using this, but just 
worried if we are taking a hack for userspace in kernel given that no 
other board or other userspace variants had a necessity for this change 
previously.

-- 
Regards,
Nishanth Menon