linux-input.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] input: add support for PowerOn(PonKey) button on the AB8500 MFD
@ 2010-09-01  7:35 Sundar Iyer
  2010-09-01 16:51 ` Dmitry Torokhov
  2010-09-03  7:10 ` Trilok Soni
  0 siblings, 2 replies; 14+ messages in thread
From: Sundar Iyer @ 2010-09-01  7:35 UTC (permalink / raw)
  To: dmitry.torokhov, sameo
  Cc: linux-input, STEricsson_nomadik_linux, Sundar R Iyer

From: Sundar R Iyer <sundar.iyer@stericsson.com>

Add the PowerOn(PonKey) button support to detect power on/off events

Acked-by: Linus Walleij <linus.walleij@stericsson.com>
Signed-off-by: Sundar R Iyer <sundar.iyer@stericsson.com>
---
 drivers/input/misc/Kconfig         |    7 ++
 drivers/input/misc/Makefile        |    2 +-
 drivers/input/misc/ab8500-ponkey.c |  156 ++++++++++++++++++++++++++++++++++++
 drivers/mfd/ab8500-core.c          |   20 +++++
 4 files changed, 184 insertions(+), 1 deletions(-)
 create mode 100644 drivers/input/misc/ab8500-ponkey.c

diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
index b49e233..cab376c 100644
--- a/drivers/input/misc/Kconfig
+++ b/drivers/input/misc/Kconfig
@@ -22,6 +22,13 @@ config INPUT_88PM860X_ONKEY
 	  To compile this driver as a module, choose M here: the module
 	  will be called 88pm860x_onkey.
 
+config INPUT_AB8500_PONKEY
+	bool "AB8500 Pon(PowerOn) Key"
+	depends on AB8500_CORE
+	help
+	  Say Y here to use the PowerOn Key for ST-Ericsson's AB8500
+	  Mix-Sig PMIC
+
 config INPUT_AD714X
 	tristate "Analog Devices AD714x Capacitance Touch Sensor"
 	help
diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
index 19ccca7..19b87ef 100644
--- a/drivers/input/misc/Makefile
+++ b/drivers/input/misc/Makefile
@@ -5,6 +5,7 @@
 # Each configuration option enables a list of files.
 
 obj-$(CONFIG_INPUT_88PM860X_ONKEY)	+= 88pm860x_onkey.o
+obj-$(CONFIG_INPUT_AB8500_PONKEY)	+= ab8500-ponkey.o
 obj-$(CONFIG_INPUT_AD714X)		+= ad714x.o
 obj-$(CONFIG_INPUT_AD714X_I2C)		+= ad714x-i2c.o
 obj-$(CONFIG_INPUT_AD714X_SPI)		+= ad714x-spi.o
@@ -41,4 +42,3 @@ obj-$(CONFIG_INPUT_WINBOND_CIR)		+= winbond-cir.o
 obj-$(CONFIG_INPUT_WISTRON_BTNS)	+= wistron_btns.o
 obj-$(CONFIG_INPUT_WM831X_ON)		+= wm831x-on.o
 obj-$(CONFIG_INPUT_YEALINK)		+= yealink.o
-
diff --git a/drivers/input/misc/ab8500-ponkey.c b/drivers/input/misc/ab8500-ponkey.c
new file mode 100644
index 0000000..35b2998
--- /dev/null
+++ b/drivers/input/misc/ab8500-ponkey.c
@@ -0,0 +1,156 @@
+/*
+ * Copyright (C) ST-Ericsson SA 2010
+ *
+ * License Terms: GNU General Public License v2
+ * Author: Sundar Iyer <sundar.iyer@stericsson.com> for ST-Ericsson
+ *
+ * AB8500 Power-On Key handler
+ */
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/input.h>
+#include <linux/interrupt.h>
+#include <linux/mfd/ab8500.h>
+#include <linux/slab.h>
+
+/**
+ * struct ab8500_ponkey_info - ab8500 ponkey information
+ * @input_dev: pointer to input device
+ * @ab8500: ab8500 parent
+ * @irq_dbf: irq number for falling transition
+ * @irq_dbr: irq number for rising transition
+ */
+struct ab8500_ponkey_info {
+	struct input_dev	*idev;
+	struct ab8500		*ab8500;
+	int			irq_dbf;
+	int			irq_dbr;
+};
+
+/* AB8500 gives us an interrupt when ONKEY is held */
+static irqreturn_t ab8500_ponkey_handler(int irq, void *data)
+{
+	struct ab8500_ponkey_info *info = data;
+
+	if (irq == info->irq_dbf)
+		input_report_key(info->idev, KEY_POWER, true);
+	else if (irq == info->irq_dbr)
+		input_report_key(info->idev, KEY_POWER, false);
+
+	input_sync(info->idev);
+
+	return IRQ_HANDLED;
+}
+
+static int __devinit ab8500_ponkey_probe(struct platform_device *pdev)
+{
+	struct ab8500 *ab8500 = dev_get_drvdata(pdev->dev.parent);
+	struct ab8500_ponkey_info *info;
+	int irq_dbf, irq_dbr, ret;
+
+	irq_dbf = platform_get_irq_byname(pdev, "ONKEY_DBF");
+	if (irq_dbf < 0) {
+		dev_err(&pdev->dev, "No IRQ for ONKEY_DBF,error=%d\n", irq_dbf);
+		return irq_dbf;
+	}
+
+	irq_dbr = platform_get_irq_byname(pdev, "ONKEY_DBR");
+	if (irq_dbr < 0) {
+		dev_err(&pdev->dev, "No IRQ for ONKEY_DBR,error=%d\n", irq_dbr);
+		return irq_dbr;
+	}
+
+	info = kzalloc(sizeof(struct ab8500_ponkey_info), GFP_KERNEL);
+	if (!info)
+		return -ENOMEM;
+
+	info->ab8500 = ab8500;
+	info->irq_dbf = irq_dbf;
+	info->irq_dbr = irq_dbr;
+
+	info->idev = input_allocate_device();
+	if (!info->idev) {
+		dev_err(ab8500->dev, "Failed to allocate input dev\n");
+		ret = -ENOMEM;
+		goto out;
+	}
+
+	info->idev->name = "AB8500 POn(PowerOn) Key";
+	info->idev->dev.parent = &pdev->dev;
+	info->idev->evbit[0] = BIT_MASK(EV_KEY);
+	info->idev->keybit[BIT_WORD(KEY_POWER)] = BIT_MASK(KEY_POWER);
+
+	ret = input_register_device(info->idev);
+	if (ret) {
+		dev_err(ab8500->dev, "Can't register input device: %d\n", ret);
+		goto out_unfreedevice;
+	}
+
+	ret = request_threaded_irq(info->irq_dbf, NULL, ab8500_ponkey_handler,
+					0, "ab8500-ponkey-dbf", info);
+	if (ret < 0) {
+		dev_err(ab8500->dev, "Failed to request dbf IRQ#%d: %d\n",
+				info->irq_dbf, ret);
+		goto out_unregisterdevice;
+	}
+
+	ret = request_threaded_irq(info->irq_dbr, NULL, ab8500_ponkey_handler,
+					0, "ab8500-ponkey-dbr", info);
+	if (ret < 0) {
+		dev_err(ab8500->dev, "Failed to request dbr IRQ#%d: %d\n",
+				info->irq_dbr, ret);
+		goto out_irq_dbf;
+	}
+
+	platform_set_drvdata(pdev, info);
+
+	return 0;
+
+out_irq_dbf:
+	free_irq(info->irq_dbf, info);
+out_unregisterdevice:
+	input_unregister_device(info->idev);
+	info->idev = NULL;
+out_unfreedevice:
+	input_free_device(info->idev);
+out:
+	kfree(info);
+	return ret;
+}
+
+static int __devexit ab8500_ponkey_remove(struct platform_device *pdev)
+{
+	struct ab8500_ponkey_info *info = platform_get_drvdata(pdev);
+
+	free_irq(info->irq_dbf, info);
+	free_irq(info->irq_dbr, info);
+	input_unregister_device(info->idev);
+	kfree(info);
+	return 0;
+}
+
+static struct platform_driver ab8500_ponkey_driver = {
+	.driver		= {
+		.name	= "ab8500-poweron-key",
+		.owner	= THIS_MODULE,
+	},
+	.probe		= ab8500_ponkey_probe,
+	.remove		= __devexit_p(ab8500_ponkey_remove),
+};
+
+static int __init ab8500_ponkey_init(void)
+{
+	return platform_driver_register(&ab8500_ponkey_driver);
+}
+module_init(ab8500_ponkey_init);
+
+static void __exit ab8500_ponkey_exit(void)
+{
+	platform_driver_unregister(&ab8500_ponkey_driver);
+}
+module_exit(ab8500_ponkey_exit);
+
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Sundar Iyer <sundar.iyer@stericsson.com>");
+MODULE_DESCRIPTION("ST-Ericsson AB8500 Power-ON(Pon) Key driver");
diff --git a/drivers/mfd/ab8500-core.c b/drivers/mfd/ab8500-core.c
index 6548f50..d21362b 100644
--- a/drivers/mfd/ab8500-core.c
+++ b/drivers/mfd/ab8500-core.c
@@ -378,6 +378,21 @@ static struct resource ab8500_rtc_resources[] = {
 	},
 };
 
+static struct resource ab8500_poweronkey_db_resources[] = {
+	{
+		.name	= "ONKEY_DBF",
+		.start	= AB8500_INT_PON_KEY1DB_F,
+		.end	= AB8500_INT_PON_KEY1DB_F,
+		.flags	= IORESOURCE_IRQ,
+	},
+	{
+		.name	= "ONKEY_DBR",
+		.start	= AB8500_INT_PON_KEY1DB_R,
+		.end	= AB8500_INT_PON_KEY1DB_R,
+		.flags	= IORESOURCE_IRQ,
+	},
+};
+
 static struct mfd_cell ab8500_devs[] = {
 #ifdef CONFIG_DEBUG_FS
 	{
@@ -399,6 +414,11 @@ static struct mfd_cell ab8500_devs[] = {
 	{ .name = "ab8500-usb", },
 	{ .name = "ab8500-pwm", },
 	{ .name = "ab8500-regulator", },
+	{
+		.name = "ab8500-poweron-key",
+		.num_resources = ARRAY_SIZE(ab8500_poweronkey_db_resources),
+		.resources = ab8500_poweronkey_db_resources,
+	},
 };
 
 int __devinit ab8500_init(struct ab8500 *ab8500)
-- 
1.7.2.dirty


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

* Re: [PATCH] input: add support for PowerOn(PonKey) button on the AB8500 MFD
  2010-09-01  7:35 [PATCH] input: add support for PowerOn(PonKey) button on the AB8500 MFD Sundar Iyer
@ 2010-09-01 16:51 ` Dmitry Torokhov
  2010-09-02  6:55   ` Rabin Vincent
  2010-09-03  7:10 ` Trilok Soni
  1 sibling, 1 reply; 14+ messages in thread
From: Dmitry Torokhov @ 2010-09-01 16:51 UTC (permalink / raw)
  To: Sundar Iyer; +Cc: sameo, linux-input, STEricsson_nomadik_linux

Hi Sundar,

On Wed, Sep 01, 2010 at 01:05:51PM +0530, Sundar Iyer wrote:
> From: Sundar R Iyer <sundar.iyer@stericsson.com>
> 
> Add the PowerOn(PonKey) button support to detect power on/off events
> 
> Acked-by: Linus Walleij <linus.walleij@stericsson.com>
> Signed-off-by: Sundar R Iyer <sundar.iyer@stericsson.com>
> ---
>  drivers/input/misc/Kconfig         |    7 ++
>  drivers/input/misc/Makefile        |    2 +-
>  drivers/input/misc/ab8500-ponkey.c |  156 ++++++++++++++++++++++++++++++++++++
>  drivers/mfd/ab8500-core.c          |   20 +++++
>  4 files changed, 184 insertions(+), 1 deletions(-)
>  create mode 100644 drivers/input/misc/ab8500-ponkey.c
> 
> diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
> index b49e233..cab376c 100644
> --- a/drivers/input/misc/Kconfig
> +++ b/drivers/input/misc/Kconfig
> @@ -22,6 +22,13 @@ config INPUT_88PM860X_ONKEY
>  	  To compile this driver as a module, choose M here: the module
>  	  will be called 88pm860x_onkey.
>  
> +config INPUT_AB8500_PONKEY
> +	bool "AB8500 Pon(PowerOn) Key"

Why not tristate?

> +	depends on AB8500_CORE
> +	help
> +	  Say Y here to use the PowerOn Key for ST-Ericsson's AB8500
> +	  Mix-Sig PMIC
> +
> +
> +	ret = request_threaded_irq(info->irq_dbf, NULL, ab8500_ponkey_handler,
> +					0, "ab8500-ponkey-dbf", info);
> +	if (ret < 0) {
> +		dev_err(ab8500->dev, "Failed to request dbf IRQ#%d: %d\n",
> +				info->irq_dbf, ret);
> +		goto out_unregisterdevice;
> +	}
> +
> +	ret = request_threaded_irq(info->irq_dbr, NULL, ab8500_ponkey_handler,
> +					0, "ab8500-ponkey-dbr", info);
> +	if (ret < 0) {
> +		dev_err(ab8500->dev, "Failed to request dbr IRQ#%d: %d\n",
> +				info->irq_dbr, ret);
> +		goto out_irq_dbf;
> +	}
> +

Why threaded IRQs? The interrupt handlers do not do _anything_ except for
passing the event up. Do you really think that starting 2 kernel threads
to handle 1 button is the best use of the resources???

I'll change it to use normal interrupts locally, no need to resubmit.

Thanks.

-- 
Dmitry

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

* Re: [PATCH] input: add support for PowerOn(PonKey) button on the AB8500 MFD
  2010-09-01 16:51 ` Dmitry Torokhov
@ 2010-09-02  6:55   ` Rabin Vincent
  2010-09-02 17:22     ` Dmitry Torokhov
  0 siblings, 1 reply; 14+ messages in thread
From: Rabin Vincent @ 2010-09-02  6:55 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Sundar R IYER, sameo@linux.intel.com, linux-input@vger.kernel.org,
	STEricsson_nomadik_linux

On Wed, Sep 01, 2010 at 18:51:42 +0200, Dmitry Torokhov wrote:
> On Wed, Sep 01, 2010 at 01:05:51PM +0530, Sundar Iyer wrote:
> > +	ret = request_threaded_irq(info->irq_dbr, NULL, ab8500_ponkey_handler,
> > +					0, "ab8500-ponkey-dbr", info);
> > +	if (ret < 0) {
> > +		dev_err(ab8500->dev, "Failed to request dbr IRQ#%d: %d\n",
> > +				info->irq_dbr, ret);
> > +		goto out_irq_dbf;
> > +	}
> > +
> 
> Why threaded IRQs? The interrupt handlers do not do _anything_ except for
> passing the event up. Do you really think that starting 2 kernel threads
> to handle 1 button is the best use of the resources???

Because the parent MFD uses nested threads for handling interrupts, you
must specify a thread function.  No new thread is started; this
interrupt handler is called from the parent MFD's interrupt thread.

> 
> I'll change it to use normal interrupts locally, no need to resubmit.

Changing it to request_irq() will cause it to fail, because __setup_irq
will error out if this is a nested thread interrupt and no interrupt
thread is specified.  request_any_context_irq() should work, if you
would like to get rid of explicitly asking for a threaded irq.

Rabin

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

* Re: [PATCH] input: add support for PowerOn(PonKey) button on the AB8500 MFD
  2010-09-02  6:55   ` Rabin Vincent
@ 2010-09-02 17:22     ` Dmitry Torokhov
  2010-09-02 19:53       ` Trilok Soni
  0 siblings, 1 reply; 14+ messages in thread
From: Dmitry Torokhov @ 2010-09-02 17:22 UTC (permalink / raw)
  To: Rabin Vincent
  Cc: Sundar R IYER, sameo@linux.intel.com, linux-input@vger.kernel.org,
	STEricsson_nomadik_linux

On Thu, Sep 02, 2010 at 12:25:37PM +0530, Rabin Vincent wrote:
> On Wed, Sep 01, 2010 at 18:51:42 +0200, Dmitry Torokhov wrote:
> > On Wed, Sep 01, 2010 at 01:05:51PM +0530, Sundar Iyer wrote:
> > > +	ret = request_threaded_irq(info->irq_dbr, NULL, ab8500_ponkey_handler,
> > > +					0, "ab8500-ponkey-dbr", info);
> > > +	if (ret < 0) {
> > > +		dev_err(ab8500->dev, "Failed to request dbr IRQ#%d: %d\n",
> > > +				info->irq_dbr, ret);
> > > +		goto out_irq_dbf;
> > > +	}
> > > +
> > 
> > Why threaded IRQs? The interrupt handlers do not do _anything_ except for
> > passing the event up. Do you really think that starting 2 kernel threads
> > to handle 1 button is the best use of the resources???
> 
> Because the parent MFD uses nested threads for handling interrupts, you
> must specify a thread function.  No new thread is started; this
> interrupt handler is called from the parent MFD's interrupt thread.
> 

I see.

> > 
> > I'll change it to use normal interrupts locally, no need to resubmit.
> 
> Changing it to request_irq() will cause it to fail, because __setup_irq
> will error out if this is a nested thread interrupt and no interrupt
> thread is specified.  request_any_context_irq() should work, if you
> would like to get rid of explicitly asking for a threaded irq.
> 

OK, request_any_context_irq() it is then.

-- 
Dmitry

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

* Re: [PATCH] input: add support for PowerOn(PonKey) button on the AB8500 MFD
  2010-09-02 17:22     ` Dmitry Torokhov
@ 2010-09-02 19:53       ` Trilok Soni
  2010-09-02 20:24         ` Dmitry Torokhov
  0 siblings, 1 reply; 14+ messages in thread
From: Trilok Soni @ 2010-09-02 19:53 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Rabin Vincent, Sundar R IYER, sameo@linux.intel.com,
	linux-input@vger.kernel.org, STEricsson_nomadik_linux

Hi Dmitry,

On 9/2/2010 10:52 PM, Dmitry Torokhov wrote:
> 
>>>
>>> I'll change it to use normal interrupts locally, no need to resubmit.
>>
>> Changing it to request_irq() will cause it to fail, because __setup_irq
>> will error out if this is a nested thread interrupt and no interrupt
>> thread is specified.  request_any_context_irq() should work, if you
>> would like to get rid of explicitly asking for a threaded irq.
>>
> 
> OK, request_any_context_irq() it is then.

Though request_any_context_irq() would work here, but I would still
prefer to use the request_threaded_irq(..) because power-on key functionality
is in-built to this PMIC chip and it will always be threaded it seems. No new
thread will be created because they did the right thing in their PMIC core
irq code it seems.

We could prefer the request_any_context_irq(...) when we are using the line
(say gpio) coming out of this PMIC and used by generic device driver where
the another h/w design could have memory mapped gpio for the same device driver.

Better to keep this driver with request_threaded_irq(..) only.

---Trilok Soni
 
-- 
Sent by a consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

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

* Re: [PATCH] input: add support for PowerOn(PonKey) button on the AB8500 MFD
  2010-09-02 19:53       ` Trilok Soni
@ 2010-09-02 20:24         ` Dmitry Torokhov
  2010-09-03  5:16           ` Trilok Soni
  0 siblings, 1 reply; 14+ messages in thread
From: Dmitry Torokhov @ 2010-09-02 20:24 UTC (permalink / raw)
  To: Trilok Soni
  Cc: Rabin Vincent, Sundar R IYER, sameo@linux.intel.com,
	linux-input@vger.kernel.org, STEricsson_nomadik_linux

Hi Trilok,

On Fri, Sep 03, 2010 at 01:23:55AM +0530, Trilok Soni wrote:
> Hi Dmitry,
> 
> On 9/2/2010 10:52 PM, Dmitry Torokhov wrote:
> > 
> >>>
> >>> I'll change it to use normal interrupts locally, no need to resubmit.
> >>
> >> Changing it to request_irq() will cause it to fail, because __setup_irq
> >> will error out if this is a nested thread interrupt and no interrupt
> >> thread is specified.  request_any_context_irq() should work, if you
> >> would like to get rid of explicitly asking for a threaded irq.
> >>
> > 
> > OK, request_any_context_irq() it is then.
> 
> Though request_any_context_irq() would work here, but I would still
> prefer to use the request_threaded_irq(..) because power-on key functionality
> is in-built to this PMIC chip and it will always be threaded it seems. No new
> thread will be created because they did the right thing in their PMIC core
> irq code it seems.
> 
> We could prefer the request_any_context_irq(...) when we are using the line
> (say gpio) coming out of this PMIC and used by generic device driver where
> the another h/w design could have memory mapped gpio for the same device driver.
> 
> Better to keep this driver with request_threaded_irq(..) only.
> 

I disagree. I believe that drivers should use request_threaded_irq()
only if they _themselves_ require handling interrupts in process context.
The fact that someone up the stack set up threaded IRQ and not hard IRQ
should not matter.

Ideally I'd love request_irq() to be what request_any_context_irq()
currently is and then we'd have request_hard_irq() for driver that
absolutely need hard IRQ context.

Thanks.

-- 
Dmitry

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

* Re: [PATCH] input: add support for PowerOn(PonKey) button on the AB8500 MFD
  2010-09-02 20:24         ` Dmitry Torokhov
@ 2010-09-03  5:16           ` Trilok Soni
  2010-09-03 16:43             ` Dmitry Torokhov
  0 siblings, 1 reply; 14+ messages in thread
From: Trilok Soni @ 2010-09-03  5:16 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Rabin Vincent, Sundar R IYER, sameo@linux.intel.com,
	linux-input@vger.kernel.org, STEricsson_nomadik_linux

Hi Dmitry,

On 9/3/2010 1:54 AM, Dmitry Torokhov wrote:
>>
> 
> I disagree. I believe that drivers should use request_threaded_irq()
> only if they _themselves_ require handling interrupts in process context.
> The fact that someone up the stack set up threaded IRQ and not hard IRQ
> should not matter.
> 
> Ideally I'd love request_irq() to be what request_any_context_irq()
> currently is and then we'd have request_hard_irq() for driver that
> absolutely need hard IRQ context.
> 

Well, how I see the request_any_context_irq(...) call is that when the caller
doesn't know the protocol being setup from the irq core code and how the line
he is controlling was being setup, so in that case request_any_context_irq(..)
to be the safest call. But in this case we know that it will be __always__
threaded due the PMIC driven over slow bus.

May be multiple APIs are now causing confusion :)

---Trilok Soni

-- 
Sent by a consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

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

* Re: [PATCH] input: add support for PowerOn(PonKey) button on the AB8500 MFD
  2010-09-01  7:35 [PATCH] input: add support for PowerOn(PonKey) button on the AB8500 MFD Sundar Iyer
  2010-09-01 16:51 ` Dmitry Torokhov
@ 2010-09-03  7:10 ` Trilok Soni
  2010-09-03  7:20   ` Sundar R IYER
  2010-09-03 16:40   ` Dmitry Torokhov
  1 sibling, 2 replies; 14+ messages in thread
From: Trilok Soni @ 2010-09-03  7:10 UTC (permalink / raw)
  To: Sundar Iyer; +Cc: dmitry.torokhov, sameo, linux-input, STEricsson_nomadik_linux

Hi Sundar,

> +
> +	info->idev->name = "AB8500 POn(PowerOn) Key";

nit-pick: having spaces in the name would create hard time parsing it. Though it all
depends on the userspace framework.

> +
> +static struct platform_driver ab8500_ponkey_driver = {
> +	.driver		= {
> +		.name	= "ab8500-poweron-key",
> +		.owner	= THIS_MODULE,
> +	},
> +	.probe		= ab8500_ponkey_probe,
> +	.remove		= __devexit_p(ab8500_ponkey_remove),

It is not must for this driver to go through, but what would be the behaviour of
pwr-key on suspend and resume? Do you want power key to wakeup the system from sleep?
OR don't want to make it as wakeup source?

---Trilok Soni

-- 
Sent by a consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

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

* RE: [PATCH] input: add support for PowerOn(PonKey) button on the AB8500 MFD
  2010-09-03  7:10 ` Trilok Soni
@ 2010-09-03  7:20   ` Sundar R IYER
  2010-09-03  7:54     ` Trilok Soni
  2010-09-03 16:40   ` Dmitry Torokhov
  1 sibling, 1 reply; 14+ messages in thread
From: Sundar R IYER @ 2010-09-03  7:20 UTC (permalink / raw)
  To: Trilok Soni
  Cc: dmitry.torokhov@gmail.com, sameo@linux.intel.com,
	linux-input@vger.kernel.org, STEricsson_nomadik_linux

Hi,

>-----Original Message-----
>From: Trilok Soni [mailto:tsoni@codeaurora.org]
>It is not must for this driver to go through, but what would be the behaviour of
>pwr-key on suspend and resume? Do you want power key to wakeup the system
>from sleep?
>OR don't want to make it as wakeup source?

This is from the AB8500 PMIC which is on always-on power source. All the AB8500 interrupts
to the system are by platform configuration wakeup source, which is handled separately by 
a PRCM unit on the DB8500 and hence no suspend/resume helpers or wakeup configuration 
for the Pon-Key.

Cheers!
Sundar

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

* Re: [PATCH] input: add support for PowerOn(PonKey) button on the AB8500 MFD
  2010-09-03  7:20   ` Sundar R IYER
@ 2010-09-03  7:54     ` Trilok Soni
  0 siblings, 0 replies; 14+ messages in thread
From: Trilok Soni @ 2010-09-03  7:54 UTC (permalink / raw)
  To: Sundar R IYER
  Cc: dmitry.torokhov@gmail.com, sameo@linux.intel.com,
	linux-input@vger.kernel.org, STEricsson_nomadik_linux

Hi Sundar,

On 9/3/2010 12:50 PM, Sundar R IYER wrote:
> Hi,
> 
>> -----Original Message-----
>> From: Trilok Soni [mailto:tsoni@codeaurora.org]
>> It is not must for this driver to go through, but what would be the behaviour of
>> pwr-key on suspend and resume? Do you want power key to wakeup the system
>>from sleep?
>> OR don't want to make it as wakeup source?
> 
> This is from the AB8500 PMIC which is on always-on power source. All the AB8500 interrupts
> to the system are by platform configuration wakeup source, which is handled separately by 
> a PRCM unit on the DB8500 and hence no suspend/resume helpers or wakeup configuration 
> for the Pon-Key.

You can still mask these interrupts at .set_wake if they are not enabled as wakeup source. 
I am not sure if you guys have implemented .set_wake in PMIC irq core code or not. I need to check.

---Trilok Soni

-- 
Sent by a consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

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

* Re: [PATCH] input: add support for PowerOn(PonKey) button on the AB8500 MFD
  2010-09-03  7:10 ` Trilok Soni
  2010-09-03  7:20   ` Sundar R IYER
@ 2010-09-03 16:40   ` Dmitry Torokhov
  2010-09-03 19:06     ` Trilok Soni
  2010-09-06  3:11     ` Sundar R IYER
  1 sibling, 2 replies; 14+ messages in thread
From: Dmitry Torokhov @ 2010-09-03 16:40 UTC (permalink / raw)
  To: Trilok Soni; +Cc: Sundar Iyer, sameo, linux-input, STEricsson_nomadik_linux

On Fri, Sep 03, 2010 at 12:40:51PM +0530, Trilok Soni wrote:
> Hi Sundar,
> 
> > +
> > +	info->idev->name = "AB8500 POn(PowerOn) Key";
> 
> nit-pick: having spaces in the name would create hard time parsing it. Though it all
> depends on the userspace framework.

I am not sure what kind of parsing you have in mind but we have always
set name attribute to something readable:

[dtor@dtor-d630 ~]$ cat /proc/bus/input/devices | grep Name=
N: Name="Lid Switch"
N: Name="Power Button"
N: Name="Sleep Button"
N: Name="Macintosh mouse button emulation"
N: Name="AT Translated Set 2 keyboard"
N: Name="Video Bus"
N: Name="Dell WMI hotkeys"
N: Name="HDA Intel Mic at Ext Left Jack"
N: Name="HDA Intel HP Out at Ext Left Jack"
N: Name="Dell Premium USB Optical Mouse"
N: Name="Lite-On Technology USB Productivity Option Keyboard( has the hub in # 1 )"
N: Name="Lite-On Technology USB Productivity Option Keyboard( has the hub in # 1 )"
N: Name="ImExPS/2 Generic Explorer Mouse"

I think I might even slip a space between "POn" and "(PowerOn)".

> 
> > +
> > +static struct platform_driver ab8500_ponkey_driver = {
> > +	.driver		= {
> > +		.name	= "ab8500-poweron-key",
> > +		.owner	= THIS_MODULE,
> > +	},
> > +	.probe		= ab8500_ponkey_probe,
> > +	.remove		= __devexit_p(ab8500_ponkey_remove),
> 
> It is not must for this driver to go through, but what would be the behaviour of
> pwr-key on suspend and resume? Do you want power key to wakeup the system from sleep?
> OR don't want to make it as wakeup source?

I am about to apply the patch so if you decide to do any PM changes they
shoudl go as a separete path please.

-- 
Dmitry

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

* Re: [PATCH] input: add support for PowerOn(PonKey) button on the AB8500 MFD
  2010-09-03  5:16           ` Trilok Soni
@ 2010-09-03 16:43             ` Dmitry Torokhov
  0 siblings, 0 replies; 14+ messages in thread
From: Dmitry Torokhov @ 2010-09-03 16:43 UTC (permalink / raw)
  To: Trilok Soni
  Cc: Rabin Vincent, Sundar R IYER, sameo@linux.intel.com,
	linux-input@vger.kernel.org, STEricsson_nomadik_linux

On Fri, Sep 03, 2010 at 10:46:55AM +0530, Trilok Soni wrote:
> Hi Dmitry,
> 
> On 9/3/2010 1:54 AM, Dmitry Torokhov wrote:
> >>
> > 
> > I disagree. I believe that drivers should use request_threaded_irq()
> > only if they _themselves_ require handling interrupts in process context.
> > The fact that someone up the stack set up threaded IRQ and not hard IRQ
> > should not matter.
> > 
> > Ideally I'd love request_irq() to be what request_any_context_irq()
> > currently is and then we'd have request_hard_irq() for driver that
> > absolutely need hard IRQ context.
> > 
> 
> Well, how I see the request_any_context_irq(...) call is that when the caller
> doesn't know the protocol being setup from the irq core code

Well, here lies the difference in our approaches ;) For me it is not
that the driver does not know, but that it does not _care_. It, by
itself, can work in both threaded and hard interrupt mode.

I do believe that driver should request threaded interrupts only if they
need threaded interrupts.

> and how the line
> he is controlling was being setup, so in that case request_any_context_irq(..)
> to be the safest call. But in this case we know that it will be __always__
> threaded due the PMIC driven over slow bus.
> 
> May be multiple APIs are now causing confusion :)
> 

Thanks.

-- 
Dmitry

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

* Re: [PATCH] input: add support for PowerOn(PonKey) button on the AB8500 MFD
  2010-09-03 16:40   ` Dmitry Torokhov
@ 2010-09-03 19:06     ` Trilok Soni
  2010-09-06  3:11     ` Sundar R IYER
  1 sibling, 0 replies; 14+ messages in thread
From: Trilok Soni @ 2010-09-03 19:06 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: Sundar Iyer, sameo, linux-input, STEricsson_nomadik_linux

Hi Dmitry,

On 9/3/2010 10:10 PM, Dmitry Torokhov wrote:
> 
>>
>>> +
>>> +static struct platform_driver ab8500_ponkey_driver = {
>>> +	.driver		= {
>>> +		.name	= "ab8500-poweron-key",
>>> +		.owner	= THIS_MODULE,
>>> +	},
>>> +	.probe		= ab8500_ponkey_probe,
>>> +	.remove		= __devexit_p(ab8500_ponkey_remove),
>>
>> It is not must for this driver to go through, but what would be the behaviour of
>> pwr-key on suspend and resume? Do you want power key to wakeup the system from sleep?
>> OR don't want to make it as wakeup source?
> 
> I am about to apply the patch so if you decide to do any PM changes they
> shoudl go as a separete path please.
> 

Please go ahead and merge it. Thanks.

---Trilok Soni

-- 
Sent by a consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

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

* RE: [PATCH] input: add support for PowerOn(PonKey) button on the AB8500 MFD
  2010-09-03 16:40   ` Dmitry Torokhov
  2010-09-03 19:06     ` Trilok Soni
@ 2010-09-06  3:11     ` Sundar R IYER
  1 sibling, 0 replies; 14+ messages in thread
From: Sundar R IYER @ 2010-09-06  3:11 UTC (permalink / raw)
  To: Dmitry Torokhov, Trilok Soni
  Cc: sameo@linux.intel.com, linux-input@vger.kernel.org,
	STEricsson_nomadik_linux

Hi Dmitry,

>-----Original Message-----
>From: Dmitry Torokhov [mailto:dmitry.torokhov@gmail.com]
>
>I am about to apply the patch so if you decide to do any PM changes they
>shoudl go as a separete path please.
>
This is okay. Thanx for merging.

Thanx!
Sundar

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

end of thread, other threads:[~2010-09-06  3:11 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-09-01  7:35 [PATCH] input: add support for PowerOn(PonKey) button on the AB8500 MFD Sundar Iyer
2010-09-01 16:51 ` Dmitry Torokhov
2010-09-02  6:55   ` Rabin Vincent
2010-09-02 17:22     ` Dmitry Torokhov
2010-09-02 19:53       ` Trilok Soni
2010-09-02 20:24         ` Dmitry Torokhov
2010-09-03  5:16           ` Trilok Soni
2010-09-03 16:43             ` Dmitry Torokhov
2010-09-03  7:10 ` Trilok Soni
2010-09-03  7:20   ` Sundar R IYER
2010-09-03  7:54     ` Trilok Soni
2010-09-03 16:40   ` Dmitry Torokhov
2010-09-03 19:06     ` Trilok Soni
2010-09-06  3:11     ` Sundar R IYER

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