linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] Input: palmas: add support for palmas power button
@ 2014-08-18 20:13 Nishanth Menon
  2014-08-18 20:13 ` [PATCH 1/2] doc: dt/bindings: input: introduce palmas power button description Nishanth Menon
                   ` (3 more replies)
  0 siblings, 4 replies; 24+ messages in thread
From: Nishanth Menon @ 2014-08-18 20:13 UTC (permalink / raw)
  To: linux-arm-kernel

Many Palmas PMIC variants have support for power button feature. This
feature depends on certain One Time Program (OTP) and board pull configurations
(POWERHOLD signal). However, on many platforms such as DRA72-evm, OMAP5-uevm,
this may be used to generate input events similar to twl4030-pwrbutton.c

Series is based on v3.17-rc1
Nishanth Menon (2):
  doc: dt/bindings: input: introduce palmas power button description
  Input: misc: introduce palmas-pwrbutton

 .../bindings/input/ti,palmas-pwrbutton.txt         |   32 ++
 drivers/input/misc/Kconfig                         |   10 +
 drivers/input/misc/Makefile                        |    1 +
 drivers/input/misc/palmas-pwrbutton.c              |  313 ++++++++++++++++++++
 4 files changed, 356 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/input/ti,palmas-pwrbutton.txt
 create mode 100644 drivers/input/misc/palmas-pwrbutton.c

-- 
1.7.9.5

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

* [PATCH 1/2] doc: dt/bindings: input: introduce palmas power button description
  2014-08-18 20:13 [PATCH 0/2] Input: palmas: add support for palmas power button Nishanth Menon
@ 2014-08-18 20:13 ` Nishanth Menon
  2014-08-19  5:28   ` Dmitry Torokhov
  2014-08-18 20:13 ` [PATCH 2/2] Input: misc: introduce palmas-pwrbutton Nishanth Menon
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 24+ messages in thread
From: Nishanth Menon @ 2014-08-18 20:13 UTC (permalink / raw)
  To: linux-arm-kernel

Many palmas family of PMICs have support for interrupt based power
button. This allows the device to notify the processor of external
push button events over the shared palmas interrupt.

Document the hardware support for the same.

Signed-off-by: Nishanth Menon <nm@ti.com>
---
 .../bindings/input/ti,palmas-pwrbutton.txt         |   32 ++++++++++++++++++++
 1 file changed, 32 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/input/ti,palmas-pwrbutton.txt

diff --git a/Documentation/devicetree/bindings/input/ti,palmas-pwrbutton.txt b/Documentation/devicetree/bindings/input/ti,palmas-pwrbutton.txt
new file mode 100644
index 0000000..6a89bcd
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/ti,palmas-pwrbutton.txt
@@ -0,0 +1,32 @@
+Texas Instruments Palmas family power button module
+
+This module is part of the Palmas family of PMICs. For more details
+about the whole chip see:
+Documentation/devicetree/bindings/mfd/palmas.txt.
+
+This module provides a simple power button event via an Interrupt.
+
+Required properties:
+- compatible: should be one of the following
+   - "ti,palmas-pwrbutton": For Palmas compatible power on button
+- interrupt-parent: Parent interrupt device, must be handle of palmas node.
+- interrupts: Interrupt number of power button submodule on device.
+
+Optional Properties:
+
+- ti,palmas-long-press-seconds: Duration in seconds which the power
+  button should be kept pressed for Palmas to power off automatically.
+  NOTE: This depends on OTP support and POWERHOLD signal configuration
+  on platform.
+
+Example:
+
+&palmas {
+	palmas_pwr_button: pwrbutton {
+		compatible = "ti,palmas-pwrbutton";
+		interrupt-parent = <&tps659038>;
+		interrupts = <1 IRQ_TYPE_NONE>;
+		wakeup-source;
+		ti,palmas-long-press-seconds = <12>;
+	};
+};
-- 
1.7.9.5

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

* [PATCH 2/2] Input: misc: introduce palmas-pwrbutton
  2014-08-18 20:13 [PATCH 0/2] Input: palmas: add support for palmas power button Nishanth Menon
  2014-08-18 20:13 ` [PATCH 1/2] doc: dt/bindings: input: introduce palmas power button description Nishanth Menon
@ 2014-08-18 20:13 ` Nishanth Menon
  2014-08-19  5:23   ` Dmitry Torokhov
  2014-08-21 16:02 ` [PATCH V2 0/2] Input: palmas: add support for palmas power button Nishanth Menon
  2014-08-21 18:52 ` [PATCH V3 " Nishanth Menon
  3 siblings, 1 reply; 24+ messages in thread
From: Nishanth Menon @ 2014-08-18 20:13 UTC (permalink / raw)
  To: linux-arm-kernel

Many palmas family of PMICs have support for interrupt based power
button. This allows the device to notify the processor of external
push button events over the shared palmas interrupt. However, this
event is generated only during a "press" operation. Software is
supposed to poll(sigh!) for detecting a release event.

The PMIC also supports ability to power off independent of the
software decisions when the button is pressed for a long duration if
the PMIC is appropriately configured on the platform.

Even though the function is similar to twl4030_pwrbutton, it is
substantially different in operation to belong to a new driver of it's
own.

Based on original work done by Girish S Ghongdemath <girishsg@ti.com>

Signed-off-by: Nishanth Menon <nm@ti.com>
---
 drivers/input/misc/Kconfig            |   10 ++
 drivers/input/misc/Makefile           |    1 +
 drivers/input/misc/palmas-pwrbutton.c |  314 +++++++++++++++++++++++++++++++++
 3 files changed, 325 insertions(+)
 create mode 100644 drivers/input/misc/palmas-pwrbutton.c

diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
index 2ff4425..0224dcf 100644
--- a/drivers/input/misc/Kconfig
+++ b/drivers/input/misc/Kconfig
@@ -451,6 +451,16 @@ config HP_SDC_RTC
 	  Say Y here if you want to support the built-in real time clock
 	  of the HP SDC controller.
 
+config INPUT_PALMAS_PWRBUTTON
+	tristate "Palmas Power button Driver"
+	depends on MFD_PALMAS
+	help
+	  Say Y here if you want to enable power key reporting via the
+	  Palmas family of PMICs.
+
+	  To compile this driver as a module, choose M here. The module will
+	  be called palmas_pwrbutton.
+
 config INPUT_PCF50633_PMU
 	tristate "PCF50633 PMU events"
 	depends on MFD_PCF50633
diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
index 4955ad3..e3d5efb 100644
--- a/drivers/input/misc/Makefile
+++ b/drivers/input/misc/Makefile
@@ -40,6 +40,7 @@ obj-$(CONFIG_INPUT_MAX8997_HAPTIC)	+= max8997_haptic.o
 obj-$(CONFIG_INPUT_MC13783_PWRBUTTON)	+= mc13783-pwrbutton.o
 obj-$(CONFIG_INPUT_MMA8450)		+= mma8450.o
 obj-$(CONFIG_INPUT_MPU3050)		+= mpu3050.o
+obj-$(CONFIG_INPUT_PALMAS_PWRBUTTON)	+= palmas-pwrbutton.o
 obj-$(CONFIG_INPUT_PCAP)		+= pcap_keys.o
 obj-$(CONFIG_INPUT_PCF50633_PMU)	+= pcf50633-input.o
 obj-$(CONFIG_INPUT_PCF8574)		+= pcf8574_keypad.o
diff --git a/drivers/input/misc/palmas-pwrbutton.c b/drivers/input/misc/palmas-pwrbutton.c
new file mode 100644
index 0000000..35f3d68
--- /dev/null
+++ b/drivers/input/misc/palmas-pwrbutton.c
@@ -0,0 +1,314 @@
+/*
+ * Texas Instruments' Palmas Power Button Input Driver
+ *
+ * Copyright (C) 2012-2014 Texas Instruments Incorporated - http://www.ti.com/
+ *	Girish S Ghongdemath
+ *	Nishanth Menon
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed "as is" WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+#include <linux/init.h>
+#include <linux/input.h>
+#include <linux/interrupt.h>
+#include <linux/kernel.h>
+#include <linux/mfd/palmas.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/reboot.h>
+#include <linux/slab.h>
+
+#define PALMAS_LPK_TIME_MASK		0x0c
+#define PALMAS_PWR_KEY_PRESS		0x01
+#define PALMAS_PWR_KEY_Q_TIME_MS	20
+
+/**
+ * struct palmas_pwron - Palmas power on data
+ * @palmas:		pointer to palmas device
+ * @input_dev:		pointer to input device
+ * @irq:		irq that we are hooked on to
+ * @input_work:		work for detecting release of key
+ * @current_state:	key current state
+ * @key_recheck_ms:	duration for recheck of key (in milli-seconds)
+ */
+struct palmas_pwron {
+	struct palmas *palmas;
+	struct input_dev *input_dev;
+	int irq;
+	struct delayed_work input_work;
+	int current_state;
+	u32 key_recheck_ms;
+};
+
+/**
+ * struct palmas_pwron_config - configuration of palmas power on
+ * @long_press_time_val:	value for long press h/w shutdown event
+ */
+struct palmas_pwron_config {
+	u8 long_press_time_val;
+};
+
+/**
+ * palmas_get_pwr_state() - read button state
+ * @pwron: pointer to pwron struct
+ */
+static int palmas_get_pwr_state(struct palmas_pwron *pwron)
+{
+	struct input_dev *input_dev = pwron->input_dev;
+	struct device *dev = input_dev->dev.parent;
+	unsigned int reg = 0;
+	int ret;
+
+	ret = palmas_read(pwron->palmas, PALMAS_INTERRUPT_BASE,
+			  PALMAS_INT1_LINE_STATE, &reg);
+	if (ret) {
+		dev_err(dev, "%s:Cannot read palmas PWRON status(%d)\n",
+			__func__, ret);
+		return ret;
+	}
+
+	/* PWRON line state is BIT(1) of the register */
+	return reg & BIT(1) ? 0 : PALMAS_PWR_KEY_PRESS;
+}
+
+/**
+ * palmas_power_button_work() - Detects the button release event
+ * @work:	work item to detect button release
+ */
+static void palmas_power_button_work(struct work_struct *work)
+{
+	struct palmas_pwron *pwron = container_of((struct delayed_work *)work,
+						  struct palmas_pwron,
+						  input_work);
+	struct input_dev *input_dev = pwron->input_dev;
+	int next_state;
+
+	next_state = palmas_get_pwr_state(pwron);
+	if (next_state < 0)
+		return;
+
+	/*
+	 * If the state did not change then schedule a work item to check the
+	 * status of the power button line
+	 */
+	if (next_state == pwron->current_state) {
+		schedule_delayed_work(&pwron->input_work,
+				      msecs_to_jiffies(pwron->key_recheck_ms));
+		return;
+	}
+
+	pwron->current_state = next_state;
+	input_report_key(input_dev, KEY_POWER, pwron->current_state);
+	input_sync(input_dev);
+}
+
+/**
+ * pwron_irq() - button press isr
+ * @irq:		irq
+ * @palmas_pwron:	pwron struct
+ */
+static irqreturn_t pwron_irq(int irq, void *palmas_pwron)
+{
+	struct palmas_pwron *pwron = palmas_pwron;
+	struct input_dev *input_dev = pwron->input_dev;
+
+	cancel_delayed_work_sync(&pwron->input_work);
+
+	pwron->current_state = PALMAS_PWR_KEY_PRESS;
+
+	input_report_key(input_dev, KEY_POWER, pwron->current_state);
+	pm_wakeup_event(input_dev->dev.parent, 0);
+	input_sync(input_dev);
+
+	schedule_delayed_work(&pwron->input_work, 0);
+
+	return IRQ_HANDLED;
+}
+
+/**
+ * palmas_pwron_params_ofinit() - device tree parameter parser
+ * @dev:	palmas button device
+ * @config:	configuration params that this fills up
+ */
+static int palmas_pwron_params_ofinit(struct device *dev,
+				      struct palmas_pwron_config *config)
+{
+	struct device_node *np;
+	u32 val;
+	int i;
+	u8 lpk_times[] = { 6, 8, 10, 12 };
+
+	/* Legacy boot? */
+	if (!of_have_populated_dt())
+		return 0;
+
+	np = of_node_get(dev->of_node);
+	/* Mixed boot? */
+	if (!np)
+		return 0;
+
+	val = 0;
+	of_property_read_u32(np, "ti,palmas-long-press-seconds", &val);
+	config->long_press_time_val = ARRAY_SIZE(lpk_times) - 1;
+	for (i = 0; i < ARRAY_SIZE(lpk_times); i++) {
+		if (val <= lpk_times[i]) {
+			config->long_press_time_val = i;
+			break;
+		}
+	}
+	dev_info(dev, "h/w controlled shutdown duration=%d seconds\n",
+		 lpk_times[config->long_press_time_val]);
+
+	of_node_put(np);
+
+	return 0;
+}
+
+/**
+ * palmas_pwron_probe() - probe
+ * @pdev:	platform device for the button
+ */
+static int palmas_pwron_probe(struct platform_device *pdev)
+{
+	struct palmas *palmas = dev_get_drvdata(pdev->dev.parent);
+	struct device *dev = &pdev->dev;
+	struct input_dev *input_dev;
+	struct palmas_pwron *pwron;
+	int irq, ret;
+	struct palmas_pwron_config config = { 0 };
+
+	ret = palmas_pwron_params_ofinit(dev, &config);
+	if (ret)
+		return ret;
+
+	pwron = devm_kzalloc(dev, sizeof(*pwron), GFP_KERNEL);
+	if (!pwron)
+		return -ENOMEM;
+
+	input_dev = devm_input_allocate_device(dev);
+	if (!input_dev) {
+		dev_err(dev, "Can't allocate power button\n");
+		return -ENOMEM;
+	}
+
+	/* Setup default hardware shutdown option (long key press) */
+	ret = palmas_update_bits(palmas, PALMAS_PMU_CONTROL_BASE,
+				 PALMAS_LONG_PRESS_KEY,
+				 PALMAS_LPK_TIME_MASK,
+				 config.long_press_time_val);
+	if (ret < 0) {
+		dev_err(dev, "LONG_PRESS_KEY_UPDATE failed!\n");
+		return ret;
+	}
+
+	input_dev->evbit[0] = BIT_MASK(EV_KEY);
+	input_dev->keybit[BIT_WORD(KEY_POWER)] = BIT_MASK(KEY_POWER);
+	input_dev->name = "palmas_pwron";
+	input_dev->phys = "palmas_pwron/input0";
+	input_dev->dev.parent = dev;
+
+	pwron->palmas = palmas;
+	pwron->input_dev = input_dev;
+
+	INIT_DELAYED_WORK(&pwron->input_work, palmas_power_button_work);
+
+	irq = platform_get_irq(pdev, 0);
+
+	device_init_wakeup(dev, 1);
+
+	ret = devm_request_threaded_irq(dev, irq, NULL, pwron_irq,
+					IRQF_TRIGGER_HIGH |
+					IRQF_TRIGGER_LOW,
+					dev_name(dev),
+					pwron);
+	if (ret < 0) {
+		dev_err(dev, "Can't get IRQ for pwron: %d\n", ret);
+		return ret;
+	}
+
+	enable_irq_wake(irq);
+
+	ret = input_register_device(input_dev);
+	if (ret) {
+		dev_dbg(dev, "Can't register power button: %d\n", ret);
+		goto out_irq_wake;
+	}
+	pwron->irq = irq;
+
+	pwron->key_recheck_ms = PALMAS_PWR_KEY_Q_TIME_MS;
+
+	platform_set_drvdata(pdev, pwron);
+
+	return 0;
+
+out_irq_wake:
+	disable_irq_wake(irq);
+
+	return ret;
+}
+
+static int palmas_pwron_remove(struct platform_device *pdev)
+{
+	struct palmas_pwron *pwron = platform_get_drvdata(pdev);
+
+	disable_irq_wake(pwron->irq);
+	input_unregister_device(pwron->input_dev);
+
+	return 0;
+}
+
+#ifdef CONFIG_PM
+/**
+ * palmas_pwron_suspend() - suspend handler
+ * @dev:	power button device
+ *
+ * Cancel all pending work items for the power button
+ */
+static int palmas_pwron_suspend(struct device *dev)
+{
+	struct platform_device *pdev = to_platform_device(dev);
+	struct palmas_pwron *pwron = platform_get_drvdata(pdev);
+
+	cancel_delayed_work_sync(&pwron->input_work);
+
+	return 0;
+}
+
+static UNIVERSAL_DEV_PM_OPS(palmas_pwron_pm, palmas_pwron_suspend, NULL, NULL);
+
+#else
+static UNIVERSAL_DEV_PM_OPS(palmas_pwron_pm, NULL, NULL, NULL);
+#endif
+
+#ifdef CONFIG_OF
+static struct of_device_id of_palmas_pwr_match[] = {
+	{.compatible = "ti,palmas-pwrbutton"},
+	{},
+};
+
+MODULE_DEVICE_TABLE(of, of_palmas_pwr_match);
+#endif
+
+static struct platform_driver palmas_pwron_driver = {
+	.probe = palmas_pwron_probe,
+	.remove = palmas_pwron_remove,
+	.driver = {
+		   .name = "palmas_pwrbutton",
+		   .owner = THIS_MODULE,
+		   .of_match_table = of_match_ptr(of_palmas_pwr_match),
+		   .pm = &palmas_pwron_pm,
+		   },
+};
+module_platform_driver(palmas_pwron_driver);
+
+MODULE_ALIAS("platform:palmas-pwrbutton");
+MODULE_DESCRIPTION("Palmas Power Button");
+MODULE_LICENSE("GPL V2");
+MODULE_AUTHOR("Texas Instruments Inc.");
-- 
1.7.9.5

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

* [PATCH 2/2] Input: misc: introduce palmas-pwrbutton
  2014-08-18 20:13 ` [PATCH 2/2] Input: misc: introduce palmas-pwrbutton Nishanth Menon
@ 2014-08-19  5:23   ` Dmitry Torokhov
  2014-08-19 10:17     ` Nishanth Menon
  0 siblings, 1 reply; 24+ messages in thread
From: Dmitry Torokhov @ 2014-08-19  5:23 UTC (permalink / raw)
  To: linux-arm-kernel

Hi NIshanth,

On Mon, Aug 18, 2014 at 03:13:30PM -0500, Nishanth Menon wrote:
> Many palmas family of PMICs have support for interrupt based power
> button. This allows the device to notify the processor of external
> push button events over the shared palmas interrupt. However, this
> event is generated only during a "press" operation. Software is
> supposed to poll(sigh!) for detecting a release event.
> 
> The PMIC also supports ability to power off independent of the
> software decisions when the button is pressed for a long duration if
> the PMIC is appropriately configured on the platform.
> 
> Even though the function is similar to twl4030_pwrbutton, it is
> substantially different in operation to belong to a new driver of it's
> own.
> 
> Based on original work done by Girish S Ghongdemath <girishsg@ti.com>
> 
> Signed-off-by: Nishanth Menon <nm@ti.com>
> ---
>  drivers/input/misc/Kconfig            |   10 ++
>  drivers/input/misc/Makefile           |    1 +
>  drivers/input/misc/palmas-pwrbutton.c |  314 +++++++++++++++++++++++++++++++++
>  3 files changed, 325 insertions(+)
>  create mode 100644 drivers/input/misc/palmas-pwrbutton.c
> 
> diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
> index 2ff4425..0224dcf 100644
> --- a/drivers/input/misc/Kconfig
> +++ b/drivers/input/misc/Kconfig
> @@ -451,6 +451,16 @@ config HP_SDC_RTC
>  	  Say Y here if you want to support the built-in real time clock
>  	  of the HP SDC controller.
>  
> +config INPUT_PALMAS_PWRBUTTON
> +	tristate "Palmas Power button Driver"
> +	depends on MFD_PALMAS
> +	help
> +	  Say Y here if you want to enable power key reporting via the
> +	  Palmas family of PMICs.
> +
> +	  To compile this driver as a module, choose M here. The module will
> +	  be called palmas_pwrbutton.
> +
>  config INPUT_PCF50633_PMU
>  	tristate "PCF50633 PMU events"
>  	depends on MFD_PCF50633
> diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
> index 4955ad3..e3d5efb 100644
> --- a/drivers/input/misc/Makefile
> +++ b/drivers/input/misc/Makefile
> @@ -40,6 +40,7 @@ obj-$(CONFIG_INPUT_MAX8997_HAPTIC)	+= max8997_haptic.o
>  obj-$(CONFIG_INPUT_MC13783_PWRBUTTON)	+= mc13783-pwrbutton.o
>  obj-$(CONFIG_INPUT_MMA8450)		+= mma8450.o
>  obj-$(CONFIG_INPUT_MPU3050)		+= mpu3050.o
> +obj-$(CONFIG_INPUT_PALMAS_PWRBUTTON)	+= palmas-pwrbutton.o
>  obj-$(CONFIG_INPUT_PCAP)		+= pcap_keys.o
>  obj-$(CONFIG_INPUT_PCF50633_PMU)	+= pcf50633-input.o
>  obj-$(CONFIG_INPUT_PCF8574)		+= pcf8574_keypad.o
> diff --git a/drivers/input/misc/palmas-pwrbutton.c b/drivers/input/misc/palmas-pwrbutton.c
> new file mode 100644
> index 0000000..35f3d68
> --- /dev/null
> +++ b/drivers/input/misc/palmas-pwrbutton.c
> @@ -0,0 +1,314 @@
> +/*
> + * Texas Instruments' Palmas Power Button Input Driver
> + *
> + * Copyright (C) 2012-2014 Texas Instruments Incorporated - http://www.ti.com/
> + *	Girish S Ghongdemath
> + *	Nishanth Menon
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed "as is" WITHOUT ANY WARRANTY of any
> + * kind, whether express or implied; without even the implied warranty
> + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +#include <linux/init.h>
> +#include <linux/input.h>
> +#include <linux/interrupt.h>
> +#include <linux/kernel.h>
> +#include <linux/mfd/palmas.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/reboot.h>
> +#include <linux/slab.h>
> +
> +#define PALMAS_LPK_TIME_MASK		0x0c
> +#define PALMAS_PWR_KEY_PRESS		0x01
> +#define PALMAS_PWR_KEY_Q_TIME_MS	20
> +
> +/**
> + * struct palmas_pwron - Palmas power on data
> + * @palmas:		pointer to palmas device
> + * @input_dev:		pointer to input device
> + * @irq:		irq that we are hooked on to
> + * @input_work:		work for detecting release of key
> + * @current_state:	key current state
> + * @key_recheck_ms:	duration for recheck of key (in milli-seconds)
> + */
> +struct palmas_pwron {
> +	struct palmas *palmas;
> +	struct input_dev *input_dev;
> +	int irq;
> +	struct delayed_work input_work;
> +	int current_state;
> +	u32 key_recheck_ms;
> +};
> +
> +/**
> + * struct palmas_pwron_config - configuration of palmas power on
> + * @long_press_time_val:	value for long press h/w shutdown event
> + */
> +struct palmas_pwron_config {
> +	u8 long_press_time_val;
> +};
> +
> +/**
> + * palmas_get_pwr_state() - read button state
> + * @pwron: pointer to pwron struct
> + */
> +static int palmas_get_pwr_state(struct palmas_pwron *pwron)
> +{
> +	struct input_dev *input_dev = pwron->input_dev;
> +	struct device *dev = input_dev->dev.parent;
> +	unsigned int reg = 0;
> +	int ret;
> +
> +	ret = palmas_read(pwron->palmas, PALMAS_INTERRUPT_BASE,
> +			  PALMAS_INT1_LINE_STATE, &reg);
> +	if (ret) {
> +		dev_err(dev, "%s:Cannot read palmas PWRON status(%d)\n",
> +			__func__, ret);
> +		return ret;
> +	}
> +
> +	/* PWRON line state is BIT(1) of the register */
> +	return reg & BIT(1) ? 0 : PALMAS_PWR_KEY_PRESS;
> +}
> +
> +/**
> + * palmas_power_button_work() - Detects the button release event
> + * @work:	work item to detect button release
> + */
> +static void palmas_power_button_work(struct work_struct *work)
> +{
> +	struct palmas_pwron *pwron = container_of((struct delayed_work *)work,
> +						  struct palmas_pwron,
> +						  input_work);
> +	struct input_dev *input_dev = pwron->input_dev;
> +	int next_state;
> +
> +	next_state = palmas_get_pwr_state(pwron);
> +	if (next_state < 0)
> +		return;
> +
> +	/*
> +	 * If the state did not change then schedule a work item to check the
> +	 * status of the power button line
> +	 */
> +	if (next_state == pwron->current_state) {
> +		schedule_delayed_work(&pwron->input_work,
> +				      msecs_to_jiffies(pwron->key_recheck_ms));
> +		return;
> +	}
> +
> +	pwron->current_state = next_state;
> +	input_report_key(input_dev, KEY_POWER, pwron->current_state);
> +	input_sync(input_dev);
> +}
> +
> +/**
> + * pwron_irq() - button press isr
> + * @irq:		irq
> + * @palmas_pwron:	pwron struct
> + */
> +static irqreturn_t pwron_irq(int irq, void *palmas_pwron)
> +{
> +	struct palmas_pwron *pwron = palmas_pwron;
> +	struct input_dev *input_dev = pwron->input_dev;
> +
> +	cancel_delayed_work_sync(&pwron->input_work);
> +
> +	pwron->current_state = PALMAS_PWR_KEY_PRESS;
> +
> +	input_report_key(input_dev, KEY_POWER, pwron->current_state);
> +	pm_wakeup_event(input_dev->dev.parent, 0);
> +	input_sync(input_dev);
> +
> +	schedule_delayed_work(&pwron->input_work, 0);

Instead of cancel/schedule use mod_delayed_work. BTW, why do you need to
schedule immediately instead of waiting key_recheck_ms? Also, are there any
concerns about need to debounce?

> +
> +	return IRQ_HANDLED;
> +}
> +
> +/**
> + * palmas_pwron_params_ofinit() - device tree parameter parser
> + * @dev:	palmas button device
> + * @config:	configuration params that this fills up
> + */
> +static int palmas_pwron_params_ofinit(struct device *dev,
> +				      struct palmas_pwron_config *config)
> +{
> +	struct device_node *np;
> +	u32 val;
> +	int i;
> +	u8 lpk_times[] = { 6, 8, 10, 12 };
> +
> +	/* Legacy boot? */
> +	if (!of_have_populated_dt())
> +		return 0;
> +
> +	np = of_node_get(dev->of_node);
> +	/* Mixed boot? */
> +	if (!np)
> +		return 0;
> +
> +	val = 0;
> +	of_property_read_u32(np, "ti,palmas-long-press-seconds", &val);
> +	config->long_press_time_val = ARRAY_SIZE(lpk_times) - 1;
> +	for (i = 0; i < ARRAY_SIZE(lpk_times); i++) {
> +		if (val <= lpk_times[i]) {
> +			config->long_press_time_val = i;
> +			break;
> +		}
> +	}
> +	dev_info(dev, "h/w controlled shutdown duration=%d seconds\n",
> +		 lpk_times[config->long_press_time_val]);
> +
> +	of_node_put(np);
> +
> +	return 0;
> +}
> +
> +/**
> + * palmas_pwron_probe() - probe
> + * @pdev:	platform device for the button
> + */
> +static int palmas_pwron_probe(struct platform_device *pdev)
> +{
> +	struct palmas *palmas = dev_get_drvdata(pdev->dev.parent);
> +	struct device *dev = &pdev->dev;
> +	struct input_dev *input_dev;
> +	struct palmas_pwron *pwron;
> +	int irq, ret;
> +	struct palmas_pwron_config config = { 0 };
> +
> +	ret = palmas_pwron_params_ofinit(dev, &config);
> +	if (ret)
> +		return ret;
> +
> +	pwron = devm_kzalloc(dev, sizeof(*pwron), GFP_KERNEL);
> +	if (!pwron)
> +		return -ENOMEM;
> +
> +	input_dev = devm_input_allocate_device(dev);
> +	if (!input_dev) {
> +		dev_err(dev, "Can't allocate power button\n");
> +		return -ENOMEM;
> +	}
> +
> +	/* Setup default hardware shutdown option (long key press) */
> +	ret = palmas_update_bits(palmas, PALMAS_PMU_CONTROL_BASE,
> +				 PALMAS_LONG_PRESS_KEY,
> +				 PALMAS_LPK_TIME_MASK,
> +				 config.long_press_time_val);
> +	if (ret < 0) {
> +		dev_err(dev, "LONG_PRESS_KEY_UPDATE failed!\n");
> +		return ret;
> +	}
> +
> +	input_dev->evbit[0] = BIT_MASK(EV_KEY);
> +	input_dev->keybit[BIT_WORD(KEY_POWER)] = BIT_MASK(KEY_POWER);
> +	input_dev->name = "palmas_pwron";
> +	input_dev->phys = "palmas_pwron/input0";
> +	input_dev->dev.parent = dev;
> +
> +	pwron->palmas = palmas;
> +	pwron->input_dev = input_dev;
> +
> +	INIT_DELAYED_WORK(&pwron->input_work, palmas_power_button_work);
> +
> +	irq = platform_get_irq(pdev, 0);
> +
> +	device_init_wakeup(dev, 1);
> +
> +	ret = devm_request_threaded_irq(dev, irq, NULL, pwron_irq,
> +					IRQF_TRIGGER_HIGH |
> +					IRQF_TRIGGER_LOW,
> +					dev_name(dev),
> +					pwron);

I am confused about this code sequence. Why do we get IRQ, then set up wakeup,
and then request irq? Normally you get irq number, and then you request it, and
then do other stuff.

> +	if (ret < 0) {
> +		dev_err(dev, "Can't get IRQ for pwron: %d\n", ret);
> +		return ret;
> +	}
> +
> +	enable_irq_wake(irq);

Shouldn't this be in suspend callback?

> +
> +	ret = input_register_device(input_dev);
> +	if (ret) {
> +		dev_dbg(dev, "Can't register power button: %d\n", ret);
> +		goto out_irq_wake;
> +	}
> +	pwron->irq = irq;
> +
> +	pwron->key_recheck_ms = PALMAS_PWR_KEY_Q_TIME_MS;
> +
> +	platform_set_drvdata(pdev, pwron);
> +
> +	return 0;
> +
> +out_irq_wake:
> +	disable_irq_wake(irq);
> +
> +	return ret;
> +}
> +
> +static int palmas_pwron_remove(struct platform_device *pdev)
> +{
> +	struct palmas_pwron *pwron = platform_get_drvdata(pdev);
> +
> +	disable_irq_wake(pwron->irq);

Should be in resume callback().

> +	input_unregister_device(pwron->input_dev);

With devm you do not need to unregister input device. However this has problem:
what will happen if interrupt arrives here and we schedule workqueue? You need
free interrupt then cancel work and then free input device. Similar needs to be
done in probe(). I'd recommend not use devm_* here as you need to manually
unwind anyway.

> +
> +	return 0;
> +}
> +
> +#ifdef CONFIG_PM
> +/**
> + * palmas_pwron_suspend() - suspend handler
> + * @dev:	power button device
> + *
> + * Cancel all pending work items for the power button
> + */
> +static int palmas_pwron_suspend(struct device *dev)
> +{
> +	struct platform_device *pdev = to_platform_device(dev);
> +	struct palmas_pwron *pwron = platform_get_drvdata(pdev);
> +
> +	cancel_delayed_work_sync(&pwron->input_work);
> +
> +	return 0;
> +}
> +
> +static UNIVERSAL_DEV_PM_OPS(palmas_pwron_pm, palmas_pwron_suspend, NULL, NULL);

Why universal? Do they make sense for runtime pm?

> +
> +#else
> +static UNIVERSAL_DEV_PM_OPS(palmas_pwron_pm, NULL, NULL, NULL);
> +#endif

You do not need to protect these with #ifdef and have 2 versions, just pull
UNIVERSAL_DEV_PM_OPS (and change to SIMPLE_DEV_PM_OPS) out of #idef code.

> +
> +#ifdef CONFIG_OF
> +static struct of_device_id of_palmas_pwr_match[] = {
> +	{.compatible = "ti,palmas-pwrbutton"},
> +	{},
> +};
> +
> +MODULE_DEVICE_TABLE(of, of_palmas_pwr_match);
> +#endif
> +
> +static struct platform_driver palmas_pwron_driver = {
> +	.probe = palmas_pwron_probe,
> +	.remove = palmas_pwron_remove,
> +	.driver = {
> +		   .name = "palmas_pwrbutton",
> +		   .owner = THIS_MODULE,
> +		   .of_match_table = of_match_ptr(of_palmas_pwr_match),
> +		   .pm = &palmas_pwron_pm,
> +		   },

Weird indentation here.

> +};
> +module_platform_driver(palmas_pwron_driver);
> +
> +MODULE_ALIAS("platform:palmas-pwrbutton");
> +MODULE_DESCRIPTION("Palmas Power Button");
> +MODULE_LICENSE("GPL V2");
> +MODULE_AUTHOR("Texas Instruments Inc.");
> -- 
> 1.7.9.5
> 

Thanks.

-- 
Dmitry

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

* [PATCH 1/2] doc: dt/bindings: input: introduce palmas power button description
  2014-08-18 20:13 ` [PATCH 1/2] doc: dt/bindings: input: introduce palmas power button description Nishanth Menon
@ 2014-08-19  5:28   ` Dmitry Torokhov
  2014-08-19 10:23     ` Nishanth Menon
  0 siblings, 1 reply; 24+ messages in thread
From: Dmitry Torokhov @ 2014-08-19  5:28 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Aug 18, 2014 at 03:13:29PM -0500, Nishanth Menon wrote:
> Many palmas family of PMICs have support for interrupt based power
> button. This allows the device to notify the processor of external
> push button events over the shared palmas interrupt.
> 
> Document the hardware support for the same.
> 
> Signed-off-by: Nishanth Menon <nm@ti.com>
> ---
>  .../bindings/input/ti,palmas-pwrbutton.txt         |   32 ++++++++++++++++++++
>  1 file changed, 32 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/input/ti,palmas-pwrbutton.txt
> 
> diff --git a/Documentation/devicetree/bindings/input/ti,palmas-pwrbutton.txt b/Documentation/devicetree/bindings/input/ti,palmas-pwrbutton.txt
> new file mode 100644
> index 0000000..6a89bcd
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/ti,palmas-pwrbutton.txt
> @@ -0,0 +1,32 @@
> +Texas Instruments Palmas family power button module
> +
> +This module is part of the Palmas family of PMICs. For more details
> +about the whole chip see:
> +Documentation/devicetree/bindings/mfd/palmas.txt.
> +
> +This module provides a simple power button event via an Interrupt.
> +
> +Required properties:
> +- compatible: should be one of the following
> +   - "ti,palmas-pwrbutton": For Palmas compatible power on button
> +- interrupt-parent: Parent interrupt device, must be handle of palmas node.
> +- interrupts: Interrupt number of power button submodule on device.
> +
> +Optional Properties:
> +
> +- ti,palmas-long-press-seconds: Duration in seconds which the power
> +  button should be kept pressed for Palmas to power off automatically.
> +  NOTE: This depends on OTP support and POWERHOLD signal configuration
> +  on platform.

Only a few values are valid for this property, I think you should mention that.

> +
> +Example:
> +
> +&palmas {
> +	palmas_pwr_button: pwrbutton {
> +		compatible = "ti,palmas-pwrbutton";
> +		interrupt-parent = <&tps659038>;
> +		interrupts = <1 IRQ_TYPE_NONE>;

Why none? Can we specify appropriate trigger here instead of hard-coding in the
driver?

> +		wakeup-source;

What handles this attribute? I do not see it handled in the driver.

> +		ti,palmas-long-press-seconds = <12>;
> +	};
> +};
> -- 
> 1.7.9.5
> 

Thanks.

-- 
Dmitry

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

* [PATCH 2/2] Input: misc: introduce palmas-pwrbutton
  2014-08-19  5:23   ` Dmitry Torokhov
@ 2014-08-19 10:17     ` Nishanth Menon
  0 siblings, 0 replies; 24+ messages in thread
From: Nishanth Menon @ 2014-08-19 10:17 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Dmitry
On 08/19/2014 12:23 AM, Dmitry Torokhov wrote:
Thanks for the review.

[...]
>> +
>> +/**
>> + * pwron_irq() - button press isr
>> + * @irq:		irq
>> + * @palmas_pwron:	pwron struct
>> + */
>> +static irqreturn_t pwron_irq(int irq, void *palmas_pwron)
>> +{
>> +	struct palmas_pwron *pwron = palmas_pwron;
>> +	struct input_dev *input_dev = pwron->input_dev;
>> +
>> +	cancel_delayed_work_sync(&pwron->input_work);
>> +
>> +	pwron->current_state = PALMAS_PWR_KEY_PRESS;
>> +
>> +	input_report_key(input_dev, KEY_POWER, pwron->current_state);
>> +	pm_wakeup_event(input_dev->dev.parent, 0);
>> +	input_sync(input_dev);
>> +
>> +	schedule_delayed_work(&pwron->input_work, 0);
>
> Instead of cancel/schedule use mod_delayed_work. BTW, why do you need to
> schedule immediately instead of waiting key_recheck_ms? Also, are there any

Good point, I had missed these. Will fix.

> concerns about need to debounce?

I believe PMIC already takes care of debounce, let me see if there are 
configuration registers possible. if yes, I think it might be nice to 
add in.

[...]

>> +
>> +	irq = platform_get_irq(pdev, 0);
>> +
>> +	device_init_wakeup(dev, 1);
>> +
>> +	ret = devm_request_threaded_irq(dev, irq, NULL, pwron_irq,
>> +					IRQF_TRIGGER_HIGH |
>> +					IRQF_TRIGGER_LOW,
>> +					dev_name(dev),
>> +					pwron);
>
> I am confused about this code sequence. Why do we get IRQ, then set up wakeup,
> and then request irq? Normally you get irq number, and then you request it, and
> then do other stuff.

Uggh.. right.. will fix.

>
>> +	if (ret < 0) {
>> +		dev_err(dev, "Can't get IRQ for pwron: %d\n", ret);
>> +		return ret;
>> +	}
>> +
>> +	enable_irq_wake(irq);
>
> Shouldn't this be in suspend callback?

yes, it should have been.. my bad.. :( thanks for catching it.

>> +
>> +	ret = input_register_device(input_dev);
>> +	if (ret) {
>> +		dev_dbg(dev, "Can't register power button: %d\n", ret);
>> +		goto out_irq_wake;
>> +	}
>> +	pwron->irq = irq;
>> +
>> +	pwron->key_recheck_ms = PALMAS_PWR_KEY_Q_TIME_MS;
>> +
>> +	platform_set_drvdata(pdev, pwron);
>> +
>> +	return 0;
>> +
>> +out_irq_wake:
>> +	disable_irq_wake(irq);
>> +
>> +	return ret;
>> +}
>> +
>> +static int palmas_pwron_remove(struct platform_device *pdev)
>> +{
>> +	struct palmas_pwron *pwron = platform_get_drvdata(pdev);
>> +
>> +	disable_irq_wake(pwron->irq);
>
> Should be in resume callback().

yep.

>
>> +	input_unregister_device(pwron->input_dev);
>
> With devm you do not need to unregister input device. However this has problem:
> what will happen if interrupt arrives here and we schedule workqueue? You need
> free interrupt then cancel work and then free input device. Similar needs to be
> done in probe(). I'd recommend not use devm_* here as you need to manually
> unwind anyway.

True. I will fix these as well.

>> +
>> +	return 0;
>> +}
>> +
>> +#ifdef CONFIG_PM
>> +/**
>> + * palmas_pwron_suspend() - suspend handler
>> + * @dev:	power button device
>> + *
>> + * Cancel all pending work items for the power button
>> + */
>> +static int palmas_pwron_suspend(struct device *dev)
>> +{
>> +	struct platform_device *pdev = to_platform_device(dev);
>> +	struct palmas_pwron *pwron = platform_get_drvdata(pdev);
>> +
>> +	cancel_delayed_work_sync(&pwron->input_work);
>> +
>> +	return 0;
>> +}
>> +
>> +static UNIVERSAL_DEV_PM_OPS(palmas_pwron_pm, palmas_pwron_suspend, NULL, NULL);
>
> Why universal? Do they make sense for runtime pm?
>
>> +
>> +#else
>> +static UNIVERSAL_DEV_PM_OPS(palmas_pwron_pm, NULL, NULL, NULL);
>> +#endif
>
> You do not need to protect these with #ifdef and have 2 versions, just pull
> UNIVERSAL_DEV_PM_OPS (and change to SIMPLE_DEV_PM_OPS) out of #idef code.

I will just switch over to SIMPLE_DEV_PM_OPS here.. it is better here. 
Thanks for the suggestion.

>> +
>> +#ifdef CONFIG_OF
>> +static struct of_device_id of_palmas_pwr_match[] = {
>> +	{.compatible = "ti,palmas-pwrbutton"},
>> +	{},
>> +};
>> +
>> +MODULE_DEVICE_TABLE(of, of_palmas_pwr_match);
>> +#endif
>> +
>> +static struct platform_driver palmas_pwron_driver = {
>> +	.probe = palmas_pwron_probe,
>> +	.remove = palmas_pwron_remove,
>> +	.driver = {
>> +		   .name = "palmas_pwrbutton",
>> +		   .owner = THIS_MODULE,
>> +		   .of_match_table = of_match_ptr(of_palmas_pwr_match),
>> +		   .pm = &palmas_pwron_pm,
>> +		   },
>
> Weird indentation here.

Ugggh.. Lindent.. :(


---
Regards,
Nishanth Menon

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

* [PATCH 1/2] doc: dt/bindings: input: introduce palmas power button description
  2014-08-19  5:28   ` Dmitry Torokhov
@ 2014-08-19 10:23     ` Nishanth Menon
  0 siblings, 0 replies; 24+ messages in thread
From: Nishanth Menon @ 2014-08-19 10:23 UTC (permalink / raw)
  To: linux-arm-kernel

On 08/19/2014 12:28 AM, Dmitry Torokhov wrote:
> On Mon, Aug 18, 2014 at 03:13:29PM -0500, Nishanth Menon wrote:
>> Many palmas family of PMICs have support for interrupt based power
>> button. This allows the device to notify the processor of external
>> push button events over the shared palmas interrupt.
>>
>> Document the hardware support for the same.
>>
>> Signed-off-by: Nishanth Menon <nm@ti.com>
>> ---
>>   .../bindings/input/ti,palmas-pwrbutton.txt         |   32 ++++++++++++++++++++
>>   1 file changed, 32 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/input/ti,palmas-pwrbutton.txt
>>
>> diff --git a/Documentation/devicetree/bindings/input/ti,palmas-pwrbutton.txt b/Documentation/devicetree/bindings/input/ti,palmas-pwrbutton.txt
>> new file mode 100644
>> index 0000000..6a89bcd
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/input/ti,palmas-pwrbutton.txt
>> @@ -0,0 +1,32 @@
>> +Texas Instruments Palmas family power button module
>> +
>> +This module is part of the Palmas family of PMICs. For more details
>> +about the whole chip see:
>> +Documentation/devicetree/bindings/mfd/palmas.txt.
>> +
>> +This module provides a simple power button event via an Interrupt.
>> +
>> +Required properties:
>> +- compatible: should be one of the following
>> +   - "ti,palmas-pwrbutton": For Palmas compatible power on button
>> +- interrupt-parent: Parent interrupt device, must be handle of palmas node.
>> +- interrupts: Interrupt number of power button submodule on device.
>> +
>> +Optional Properties:
>> +
>> +- ti,palmas-long-press-seconds: Duration in seconds which the power
>> +  button should be kept pressed for Palmas to power off automatically.
>> +  NOTE: This depends on OTP support and POWERHOLD signal configuration
>> +  on platform.
>
> Only a few values are valid for this property, I think you should mention that.

Agreed. Will do so.
>
>> +
>> +Example:
>> +
>> +&palmas {
>> +	palmas_pwr_button: pwrbutton {
>> +		compatible = "ti,palmas-pwrbutton";
>> +		interrupt-parent = <&tps659038>;
>> +		interrupts = <1 IRQ_TYPE_NONE>;
>
> Why none? Can we specify appropriate trigger here instead of hard-coding in the
> driver?

Following the convention as in 
Documentation/devicetree/bindings/mfd/palmas.txt - for whatever reason 
we went with interrupt-cells = <2> when palmas interrupt configuration 
was hardcoded in the chip(not reconfigurable). I believe it was level, 
will check and update the example here.
>
>> +		wakeup-source;
>
> What handles this attribute? I do not see it handled in the driver.

we dont explicitly need to in the driver, it was meant to indicate that 
this is a wakeup source, but in reality, it is a palmas PMIC which is 
the wakeup source.. so, will drop this.

>
>> +		ti,palmas-long-press-seconds = <12>;
>> +	};
>> +};
>> --
>> 1.7.9.5
>>
>
> Thanks.
>

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

* [PATCH V2 0/2] Input: palmas: add support for palmas power button
  2014-08-18 20:13 [PATCH 0/2] Input: palmas: add support for palmas power button Nishanth Menon
  2014-08-18 20:13 ` [PATCH 1/2] doc: dt/bindings: input: introduce palmas power button description Nishanth Menon
  2014-08-18 20:13 ` [PATCH 2/2] Input: misc: introduce palmas-pwrbutton Nishanth Menon
@ 2014-08-21 16:02 ` Nishanth Menon
  2014-08-21 16:02   ` [PATCH V2 1/2] doc: dt/bindings: input: introduce palmas power button description Nishanth Menon
  2014-08-21 16:02   ` [PATCH V2 2/2] Input: misc: introduce palmas-pwrbutton Nishanth Menon
  2014-08-21 18:52 ` [PATCH V3 " Nishanth Menon
  3 siblings, 2 replies; 24+ messages in thread
From: Nishanth Menon @ 2014-08-21 16:02 UTC (permalink / raw)
  To: linux-arm-kernel

Many Palmas PMIC variants have support for power button feature. This
feature depends on certain One Time Program (OTP) and board pull
configurations (POWERHOLD signal). However, on many platforms such
as DRA72-evm, OMAP5-uevm, this may be used to generate input events
similar to twl4030-pwrbutton.c

Series is based on v3.17-rc1

V2 of the series incorporating comments from http://marc.info/?l=linux-input&m=140839287431882&w=2

Nishanth Menon (2):
  doc: dt/bindings: input: introduce palmas power button description
  Input: misc: introduce palmas-pwrbutton

 .../bindings/input/ti,palmas-pwrbutton.txt         |   36 ++
 drivers/input/misc/Kconfig                         |   10 +
 drivers/input/misc/Makefile                        |    1 +
 drivers/input/misc/palmas-pwrbutton.c              |  356 ++++++++++++++++++++
 4 files changed, 403 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/input/ti,palmas-pwrbutton.txt
 create mode 100644 drivers/input/misc/palmas-pwrbutton.c

-- 
1.7.9.5

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

* [PATCH V2 1/2] doc: dt/bindings: input: introduce palmas power button description
  2014-08-21 16:02 ` [PATCH V2 0/2] Input: palmas: add support for palmas power button Nishanth Menon
@ 2014-08-21 16:02   ` Nishanth Menon
  2014-08-21 16:02   ` [PATCH V2 2/2] Input: misc: introduce palmas-pwrbutton Nishanth Menon
  1 sibling, 0 replies; 24+ messages in thread
From: Nishanth Menon @ 2014-08-21 16:02 UTC (permalink / raw)
  To: linux-arm-kernel

Many palmas family of PMICs have support for interrupt based power
button. This allows the device to notify the processor of external
push button events over the shared palmas interrupt.

Document the hardware support for the same.

Signed-off-by: Nishanth Menon <nm@ti.com>
---

Changes in v2:
 - Added debounce description for palmas variants like TWL6037 that
   actually allow it.
 - Review comments incorportated.
V1: https://patchwork.kernel.org/patch/4739061/

 .../bindings/input/ti,palmas-pwrbutton.txt         |   36 ++++++++++++++++++++
 1 file changed, 36 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/input/ti,palmas-pwrbutton.txt

diff --git a/Documentation/devicetree/bindings/input/ti,palmas-pwrbutton.txt b/Documentation/devicetree/bindings/input/ti,palmas-pwrbutton.txt
new file mode 100644
index 0000000..a3dde8c
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/ti,palmas-pwrbutton.txt
@@ -0,0 +1,36 @@
+Texas Instruments Palmas family power button module
+
+This module is part of the Palmas family of PMICs. For more details
+about the whole chip see:
+Documentation/devicetree/bindings/mfd/palmas.txt.
+
+This module provides a simple power button event via an Interrupt.
+
+Required properties:
+- compatible: should be one of the following
+   - "ti,palmas-pwrbutton": For Palmas compatible power on button
+- interrupt-parent: Parent interrupt device, must be handle of palmas node.
+- interrupts: Interrupt number of power button submodule on device.
+
+Optional Properties:
+
+- ti,palmas-long-press-seconds: Duration in seconds which the power
+  button should be kept pressed for Palmas to power off automatically.
+  NOTE: This depends on OTP support and POWERHOLD signal configuration
+  on platform. Valid values are 6, 8, 10 and 12.
+- ti,palmas-pwron-debounce-milli-seconds: Duration in milliseconds
+  which the power button should be kept pressed for Palmas to register
+  a press for debouncing purposes. NOTE: This depends on specific
+  Palmas variation capability. Valid values are 15, 100, 500 and 1000.
+
+Example:
+
+&palmas {
+	palmas_pwr_button: pwrbutton {
+		compatible = "ti,palmas-pwrbutton";
+		interrupt-parent = <&tps659038>;
+		interrupts = <1 IRQ_TYPE_EDGE_FALLING>;
+		ti,palmas-long-press-seconds = <12>;
+		ti,palmas-pwron-debounce-milli-seconds = <15>;
+	};
+};
-- 
1.7.9.5

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

* [PATCH V2 2/2] Input: misc: introduce palmas-pwrbutton
  2014-08-21 16:02 ` [PATCH V2 0/2] Input: palmas: add support for palmas power button Nishanth Menon
  2014-08-21 16:02   ` [PATCH V2 1/2] doc: dt/bindings: input: introduce palmas power button description Nishanth Menon
@ 2014-08-21 16:02   ` Nishanth Menon
  2014-08-21 16:59     ` Murphy, Dan
  2014-08-21 17:05     ` Dmitry Torokhov
  1 sibling, 2 replies; 24+ messages in thread
From: Nishanth Menon @ 2014-08-21 16:02 UTC (permalink / raw)
  To: linux-arm-kernel

Many palmas family of PMICs have support for interrupt based power
button. This allows the device to notify the processor of external
push button events over the shared palmas interrupt. However, this
event is generated only during a "press" operation. Software is
supposed to poll(sigh!) for detecting a release event.

The PMIC also supports ability to power off independent of the
software decisions when the button is pressed for a long duration if
the PMIC is appropriately configured on the platform.

Even though the function is similar to twl4030_pwrbutton, it is
substantially different in operation to belong to a new driver of it's
own.

Based on original work done by Girish S Ghongdemath <girishsg@ti.com>

Signed-off-by: Nishanth Menon <nm@ti.com>
---
Changes in v2:
 - review comments incorporated
 - debounce programming for TWL variants that actually support it.
V1: https://patchwork.kernel.org/patch/4739041/

 drivers/input/misc/Kconfig            |   10 +
 drivers/input/misc/Makefile           |    1 +
 drivers/input/misc/palmas-pwrbutton.c |  356 +++++++++++++++++++++++++++++++++
 3 files changed, 367 insertions(+)
 create mode 100644 drivers/input/misc/palmas-pwrbutton.c

diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
index 2ff4425..0224dcf 100644
--- a/drivers/input/misc/Kconfig
+++ b/drivers/input/misc/Kconfig
@@ -451,6 +451,16 @@ config HP_SDC_RTC
 	  Say Y here if you want to support the built-in real time clock
 	  of the HP SDC controller.
 
+config INPUT_PALMAS_PWRBUTTON
+	tristate "Palmas Power button Driver"
+	depends on MFD_PALMAS
+	help
+	  Say Y here if you want to enable power key reporting via the
+	  Palmas family of PMICs.
+
+	  To compile this driver as a module, choose M here. The module will
+	  be called palmas_pwrbutton.
+
 config INPUT_PCF50633_PMU
 	tristate "PCF50633 PMU events"
 	depends on MFD_PCF50633
diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
index 4955ad3..e3d5efb 100644
--- a/drivers/input/misc/Makefile
+++ b/drivers/input/misc/Makefile
@@ -40,6 +40,7 @@ obj-$(CONFIG_INPUT_MAX8997_HAPTIC)	+= max8997_haptic.o
 obj-$(CONFIG_INPUT_MC13783_PWRBUTTON)	+= mc13783-pwrbutton.o
 obj-$(CONFIG_INPUT_MMA8450)		+= mma8450.o
 obj-$(CONFIG_INPUT_MPU3050)		+= mpu3050.o
+obj-$(CONFIG_INPUT_PALMAS_PWRBUTTON)	+= palmas-pwrbutton.o
 obj-$(CONFIG_INPUT_PCAP)		+= pcap_keys.o
 obj-$(CONFIG_INPUT_PCF50633_PMU)	+= pcf50633-input.o
 obj-$(CONFIG_INPUT_PCF8574)		+= pcf8574_keypad.o
diff --git a/drivers/input/misc/palmas-pwrbutton.c b/drivers/input/misc/palmas-pwrbutton.c
new file mode 100644
index 0000000..fb8bfc5
--- /dev/null
+++ b/drivers/input/misc/palmas-pwrbutton.c
@@ -0,0 +1,356 @@
+/*
+ * Texas Instruments' Palmas Power Button Input Driver
+ *
+ * Copyright (C) 2012-2014 Texas Instruments Incorporated - http://www.ti.com/
+ *	Girish S Ghongdemath
+ *	Nishanth Menon
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed "as is" WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+#include <linux/init.h>
+#include <linux/input.h>
+#include <linux/interrupt.h>
+#include <linux/kernel.h>
+#include <linux/mfd/palmas.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/reboot.h>
+#include <linux/slab.h>
+
+#define PALMAS_LPK_TIME_MASK		0x0c
+#define PALMAS_PWRON_DEBOUNCE_MASK	0x03
+#define PALMAS_PWR_KEY_PRESS		0x01
+#define PALMAS_PWR_KEY_Q_TIME_MS	20
+
+/**
+ * struct palmas_pwron - Palmas power on data
+ * @palmas:		pointer to palmas device
+ * @input_dev:		pointer to input device
+ * @irq:		irq that we are hooked on to
+ * @input_work:		work for detecting release of key
+ * @current_state:	key current state
+ * @key_recheck_ms:	duration for recheck of key (in milli-seconds)
+ */
+struct palmas_pwron {
+	struct palmas *palmas;
+	struct input_dev *input_dev;
+	int irq;
+	struct delayed_work input_work;
+	int current_state;
+	u32 key_recheck_ms;
+};
+
+/**
+ * struct palmas_pwron_config - configuration of palmas power on
+ * @long_press_time_val:	value for long press h/w shutdown event
+ * @pwron_debounce_val:		value for debounce of power button
+ */
+struct palmas_pwron_config {
+	u8 long_press_time_val;
+	u8 pwron_debounce_val;
+};
+
+/**
+ * palmas_get_pwr_state() - read button state
+ * @pwron: pointer to pwron struct
+ *
+ * Return: 0 for no press, PALMAS_PWR_KEY_PRESS for keypress
+ * and on error, appropriate error value.
+ */
+static int palmas_get_pwr_state(struct palmas_pwron *pwron)
+{
+	struct input_dev *input_dev = pwron->input_dev;
+	struct device *dev = input_dev->dev.parent;
+	unsigned int reg = 0;
+	int ret;
+
+	ret = palmas_read(pwron->palmas, PALMAS_INTERRUPT_BASE,
+			  PALMAS_INT1_LINE_STATE, &reg);
+	if (ret) {
+		dev_err(dev, "%s:Cannot read palmas PWRON status(%d)\n",
+			__func__, ret);
+		return ret;
+	}
+
+	/* PWRON line state is BIT(1) of the register */
+	return reg & BIT(1) ? 0 : PALMAS_PWR_KEY_PRESS;
+}
+
+/**
+ * palmas_power_button_work() - Detects the button release event
+ * @work:	work item to detect button release
+ */
+static void palmas_power_button_work(struct work_struct *work)
+{
+	struct palmas_pwron *pwron = container_of((struct delayed_work *)work,
+						  struct palmas_pwron,
+						  input_work);
+	struct input_dev *input_dev = pwron->input_dev;
+	int next_state;
+
+	next_state = palmas_get_pwr_state(pwron);
+	if (next_state < 0)
+		return;
+
+	/*
+	 * If the state did not change then schedule a work item to check the
+	 * status of the power button line
+	 */
+	if (next_state == pwron->current_state) {
+		schedule_delayed_work(&pwron->input_work,
+				      msecs_to_jiffies(pwron->key_recheck_ms));
+		return;
+	}
+
+	pwron->current_state = next_state;
+	input_report_key(input_dev, KEY_POWER, pwron->current_state);
+	input_sync(input_dev);
+}
+
+/**
+ * pwron_irq() - button press isr
+ * @irq:		irq
+ * @palmas_pwron:	pwron struct
+ *
+ * Return: IRQ_HANDLED
+ */
+static irqreturn_t pwron_irq(int irq, void *palmas_pwron)
+{
+	struct palmas_pwron *pwron = palmas_pwron;
+	struct input_dev *input_dev = pwron->input_dev;
+
+	pwron->current_state = PALMAS_PWR_KEY_PRESS;
+
+	input_report_key(input_dev, KEY_POWER, pwron->current_state);
+	pm_wakeup_event(input_dev->dev.parent, 0);
+	input_sync(input_dev);
+
+	mod_delayed_work(system_wq, &pwron->input_work,
+			 msecs_to_jiffies(pwron->key_recheck_ms));
+
+	return IRQ_HANDLED;
+}
+
+/**
+ * palmas_pwron_params_ofinit() - device tree parameter parser
+ * @dev:	palmas button device
+ * @config:	configuration params that this fills up
+ */
+static void palmas_pwron_params_ofinit(struct device *dev,
+				       struct palmas_pwron_config *config)
+{
+	struct device_node *np;
+	u32 val;
+	int i;
+	u8 lpk_times[] = { 6, 8, 10, 12 };
+	int pwr_on_deb_ms[] = { 15, 100, 500, 1000 };
+
+	/* Legacy boot? */
+	if (!of_have_populated_dt())
+		return;
+
+	np = of_node_get(dev->of_node);
+	/* Mixed boot? */
+	if (!np)
+		return;
+
+	val = 0;
+	of_property_read_u32(np, "ti,palmas-long-press-seconds", &val);
+	config->long_press_time_val = ARRAY_SIZE(lpk_times) - 1;
+	for (i = 0; i < ARRAY_SIZE(lpk_times); i++) {
+		if (val <= lpk_times[i]) {
+			config->long_press_time_val = i;
+			break;
+		}
+	}
+
+	val = 0;
+	of_property_read_u32(np, "ti,palmas-pwron-debounce-milli-seconds",
+			     &val);
+	config->pwron_debounce_val = 0;
+	for (i = 0; i < ARRAY_SIZE(pwr_on_deb_ms); i++) {
+		if (val <= pwr_on_deb_ms[i]) {
+			config->pwron_debounce_val = i;
+			break;
+		}
+	}
+
+	dev_info(dev, "h/w controlled shutdown duration=%d seconds\n",
+		 lpk_times[config->long_press_time_val]);
+
+	of_node_put(np);
+}
+
+/**
+ * palmas_pwron_probe() - probe
+ * @pdev:	platform device for the button
+ *
+ * Return: 0 for successful probe else appropriate error
+ */
+static int palmas_pwron_probe(struct platform_device *pdev)
+{
+	struct palmas *palmas = dev_get_drvdata(pdev->dev.parent);
+	struct device *dev = &pdev->dev;
+	struct input_dev *input_dev;
+	struct palmas_pwron *pwron;
+	int irq, ret, val;
+	struct palmas_pwron_config config = { 0 };
+
+	palmas_pwron_params_ofinit(dev, &config);
+
+	pwron = devm_kzalloc(dev, sizeof(*pwron), GFP_KERNEL);
+	if (!pwron)
+		return -ENOMEM;
+
+	input_dev = devm_input_allocate_device(dev);
+	if (!input_dev) {
+		dev_err(dev, "Can't allocate power button\n");
+		return -ENOMEM;
+	}
+
+	/*
+	 * Setup default hardware shutdown option (long key press)
+	 * and debounce.
+	 */
+	val = config.long_press_time_val << __ffs(PALMAS_LPK_TIME_MASK);
+	val |= config.pwron_debounce_val << __ffs(PALMAS_PWRON_DEBOUNCE_MASK);
+	ret = palmas_update_bits(palmas, PALMAS_PMU_CONTROL_BASE,
+				 PALMAS_LONG_PRESS_KEY,
+				 PALMAS_LPK_TIME_MASK |
+				 PALMAS_PWRON_DEBOUNCE_MASK, val);
+	if (ret < 0) {
+		dev_err(dev, "LONG_PRESS_KEY_UPDATE failed!\n");
+		return ret;
+	}
+
+	input_dev->evbit[0] = BIT_MASK(EV_KEY);
+	input_dev->keybit[BIT_WORD(KEY_POWER)] = BIT_MASK(KEY_POWER);
+	input_dev->name = "palmas_pwron";
+	input_dev->phys = "palmas_pwron/input0";
+	input_dev->dev.parent = dev;
+
+	pwron->palmas = palmas;
+	pwron->input_dev = input_dev;
+
+	INIT_DELAYED_WORK(&pwron->input_work, palmas_power_button_work);
+
+	irq = platform_get_irq(pdev, 0);
+	ret = request_threaded_irq(irq, NULL, pwron_irq,
+				   IRQF_TRIGGER_HIGH |
+				   IRQF_TRIGGER_LOW, dev_name(dev), pwron);
+	if (ret < 0) {
+		dev_err(dev, "Can't get IRQ for pwron: %d\n", ret);
+		return ret;
+	}
+
+	device_init_wakeup(dev, true);
+
+	ret = input_register_device(input_dev);
+	if (ret) {
+		free_irq(irq, pwron);
+		cancel_delayed_work_sync(&pwron->input_work);
+		dev_dbg(dev, "Can't register power button: %d\n", ret);
+		return ret;
+	}
+	pwron->irq = irq;
+
+	pwron->key_recheck_ms = PALMAS_PWR_KEY_Q_TIME_MS;
+
+	platform_set_drvdata(pdev, pwron);
+
+	return 0;
+}
+
+/**
+ * palmas_pwron_remove() - Cleanup on removal
+ * @pdev:	platform device for the button
+ *
+ * Return: 0
+ */
+static int palmas_pwron_remove(struct platform_device *pdev)
+{
+	struct palmas_pwron *pwron = platform_get_drvdata(pdev);
+
+	free_irq(pwron->irq, pwron);
+	cancel_delayed_work_sync(&pwron->input_work);
+	input_unregister_device(pwron->input_dev);
+
+	return 0;
+}
+
+/**
+ * palmas_pwron_suspend() - suspend handler
+ * @dev:	power button device
+ *
+ * Cancel all pending work items for the power button, setup irq for wakeup
+ *
+ * Return: 0
+ */
+static int palmas_pwron_suspend(struct device *dev)
+{
+	struct platform_device *pdev = to_platform_device(dev);
+	struct palmas_pwron *pwron = platform_get_drvdata(pdev);
+
+	cancel_delayed_work_sync(&pwron->input_work);
+
+	if (device_may_wakeup(dev))
+		enable_irq_wake(pwron->irq);
+
+	return 0;
+}
+
+/**
+ * palmas_pwron_resume() - resume handler
+ * @dev:	power button device
+ *
+ * Just disable the wakeup capability of irq here.
+ *
+ * Return: 0
+ */
+static int palmas_pwron_resume(struct device *dev)
+{
+	struct platform_device *pdev = to_platform_device(dev);
+	struct palmas_pwron *pwron = platform_get_drvdata(pdev);
+
+	if (device_may_wakeup(dev))
+		disable_irq_wake(pwron->irq);
+
+	return 0;
+}
+
+static SIMPLE_DEV_PM_OPS(palmas_pwron_pm,
+			 palmas_pwron_suspend, palmas_pwron_resume);
+
+#ifdef CONFIG_OF
+static struct of_device_id of_palmas_pwr_match[] = {
+	{.compatible = "ti,palmas-pwrbutton"},
+	{},
+};
+
+MODULE_DEVICE_TABLE(of, of_palmas_pwr_match);
+#endif
+
+static struct platform_driver palmas_pwron_driver = {
+	.probe = palmas_pwron_probe,
+	.remove = palmas_pwron_remove,
+	.driver = {
+		   .name = "palmas_pwrbutton",
+		   .owner = THIS_MODULE,
+		   .of_match_table = of_match_ptr(of_palmas_pwr_match),
+		   .pm = &palmas_pwron_pm,
+	},
+};
+module_platform_driver(palmas_pwron_driver);
+
+MODULE_ALIAS("platform:palmas-pwrbutton");
+MODULE_DESCRIPTION("Palmas Power Button");
+MODULE_LICENSE("GPL V2");
+MODULE_AUTHOR("Texas Instruments Inc.");
-- 
1.7.9.5

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

* [PATCH V2 2/2] Input: misc: introduce palmas-pwrbutton
  2014-08-21 16:02   ` [PATCH V2 2/2] Input: misc: introduce palmas-pwrbutton Nishanth Menon
@ 2014-08-21 16:59     ` Murphy, Dan
  2014-08-21 17:12       ` Nishanth Menon
  2014-08-21 17:19       ` Nishanth Menon
  2014-08-21 17:05     ` Dmitry Torokhov
  1 sibling, 2 replies; 24+ messages in thread
From: Murphy, Dan @ 2014-08-21 16:59 UTC (permalink / raw)
  To: linux-arm-kernel

On 08/21/2014 11:04 AM, Menon, Nishanth wrote:
> Many palmas family of PMICs have support for interrupt based power
> button. This allows the device to notify the processor of external
> push button events over the shared palmas interrupt. However, this
> event is generated only during a "press" operation. Software is
> supposed to poll(sigh!) for detecting a release event.
> 
> The PMIC also supports ability to power off independent of the
> software decisions when the button is pressed for a long duration if
> the PMIC is appropriately configured on the platform.
> 
> Even though the function is similar to twl4030_pwrbutton, it is
> substantially different in operation to belong to a new driver of it's
> own.
> 
> Based on original work done by Girish S Ghongdemath <girishsg@ti.com>
> 
> Signed-off-by: Nishanth Menon <nm@ti.com>
> ---
> Changes in v2:
>  - review comments incorporated
>  - debounce programming for TWL variants that actually support it.
> V1: https://patchwork.kernel.org/patch/4739041/
> 
>  drivers/input/misc/Kconfig            |   10 +
>  drivers/input/misc/Makefile           |    1 +
>  drivers/input/misc/palmas-pwrbutton.c |  356 +++++++++++++++++++++++++++++++++
>  3 files changed, 367 insertions(+)
>  create mode 100644 drivers/input/misc/palmas-pwrbutton.c
> 
> diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
> index 2ff4425..0224dcf 100644
> --- a/drivers/input/misc/Kconfig
> +++ b/drivers/input/misc/Kconfig
> @@ -451,6 +451,16 @@ config HP_SDC_RTC
>  	  Say Y here if you want to support the built-in real time clock
>  	  of the HP SDC controller.
>  
> +config INPUT_PALMAS_PWRBUTTON
> +	tristate "Palmas Power button Driver"
> +	depends on MFD_PALMAS
> +	help
> +	  Say Y here if you want to enable power key reporting via the
> +	  Palmas family of PMICs.
> +
> +	  To compile this driver as a module, choose M here. The module will
> +	  be called palmas_pwrbutton.
> +
>  config INPUT_PCF50633_PMU
>  	tristate "PCF50633 PMU events"
>  	depends on MFD_PCF50633
> diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
> index 4955ad3..e3d5efb 100644
> --- a/drivers/input/misc/Makefile
> +++ b/drivers/input/misc/Makefile
> @@ -40,6 +40,7 @@ obj-$(CONFIG_INPUT_MAX8997_HAPTIC)	+= max8997_haptic.o
>  obj-$(CONFIG_INPUT_MC13783_PWRBUTTON)	+= mc13783-pwrbutton.o
>  obj-$(CONFIG_INPUT_MMA8450)		+= mma8450.o
>  obj-$(CONFIG_INPUT_MPU3050)		+= mpu3050.o
> +obj-$(CONFIG_INPUT_PALMAS_PWRBUTTON)	+= palmas-pwrbutton.o
>  obj-$(CONFIG_INPUT_PCAP)		+= pcap_keys.o
>  obj-$(CONFIG_INPUT_PCF50633_PMU)	+= pcf50633-input.o
>  obj-$(CONFIG_INPUT_PCF8574)		+= pcf8574_keypad.o
> diff --git a/drivers/input/misc/palmas-pwrbutton.c b/drivers/input/misc/palmas-pwrbutton.c
> new file mode 100644
> index 0000000..fb8bfc5
> --- /dev/null
> +++ b/drivers/input/misc/palmas-pwrbutton.c
> @@ -0,0 +1,356 @@
> +/*
> + * Texas Instruments' Palmas Power Button Input Driver
> + *
> + * Copyright (C) 2012-2014 Texas Instruments Incorporated - http://www.ti.com/
> + *	Girish S Ghongdemath
> + *	Nishanth Menon
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed "as is" WITHOUT ANY WARRANTY of any
> + * kind, whether express or implied; without even the implied warranty
> + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +#include <linux/init.h>
> +#include <linux/input.h>
> +#include <linux/interrupt.h>
> +#include <linux/kernel.h>
> +#include <linux/mfd/palmas.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/reboot.h>

I don't see any reboot calls made do we need this?

> +#include <linux/slab.h>
> +
> +#define PALMAS_LPK_TIME_MASK		0x0c
> +#define PALMAS_PWRON_DEBOUNCE_MASK	0x03
> +#define PALMAS_PWR_KEY_PRESS		0x01
> +#define PALMAS_PWR_KEY_Q_TIME_MS	20
> +
> +/**
> + * struct palmas_pwron - Palmas power on data
> + * @palmas:		pointer to palmas device
> + * @input_dev:		pointer to input device
> + * @irq:		irq that we are hooked on to
> + * @input_work:		work for detecting release of key
> + * @current_state:	key current state
> + * @key_recheck_ms:	duration for recheck of key (in milli-seconds)
> + */
> +struct palmas_pwron {
> +	struct palmas *palmas;
> +	struct input_dev *input_dev;
> +	int irq;
> +	struct delayed_work input_work;
> +	int current_state;
> +	u32 key_recheck_ms;
> +};
> +
> +/**
> + * struct palmas_pwron_config - configuration of palmas power on
> + * @long_press_time_val:	value for long press h/w shutdown event
> + * @pwron_debounce_val:		value for debounce of power button
> + */
> +struct palmas_pwron_config {
> +	u8 long_press_time_val;
> +	u8 pwron_debounce_val;
> +};
> +
> +/**
> + * palmas_get_pwr_state() - read button state
> + * @pwron: pointer to pwron struct
> + *
> + * Return: 0 for no press, PALMAS_PWR_KEY_PRESS for keypress
> + * and on error, appropriate error value.
> + */
> +static int palmas_get_pwr_state(struct palmas_pwron *pwron)
> +{
> +	struct input_dev *input_dev = pwron->input_dev;
> +	struct device *dev = input_dev->dev.parent;
> +	unsigned int reg = 0;
> +	int ret;
> +
> +	ret = palmas_read(pwron->palmas, PALMAS_INTERRUPT_BASE,
> +			  PALMAS_INT1_LINE_STATE, &reg);
> +	if (ret) {
> +		dev_err(dev, "%s:Cannot read palmas PWRON status(%d)\n",
> +			__func__, ret);
> +		return ret;
> +	}
> +
> +	/* PWRON line state is BIT(1) of the register */
> +	return reg & BIT(1) ? 0 : PALMAS_PWR_KEY_PRESS;
> +}
> +
> +/**
> + * palmas_power_button_work() - Detects the button release event
> + * @work:	work item to detect button release
> + */
> +static void palmas_power_button_work(struct work_struct *work)
> +{
> +	struct palmas_pwron *pwron = container_of((struct delayed_work *)work,
> +						  struct palmas_pwron,
> +						  input_work);
> +	struct input_dev *input_dev = pwron->input_dev;
> +	int next_state;
> +
> +	next_state = palmas_get_pwr_state(pwron);
> +	if (next_state < 0)
> +		return;
> +
> +	/*
> +	 * If the state did not change then schedule a work item to check the
> +	 * status of the power button line
> +	 */
> +	if (next_state == pwron->current_state) {
> +		schedule_delayed_work(&pwron->input_work,
> +				      msecs_to_jiffies(pwron->key_recheck_ms));
> +		return;
> +	}
> +
> +	pwron->current_state = next_state;
> +	input_report_key(input_dev, KEY_POWER, pwron->current_state);
> +	input_sync(input_dev);
> +}
> +
> +/**
> + * pwron_irq() - button press isr
> + * @irq:		irq
> + * @palmas_pwron:	pwron struct
> + *
> + * Return: IRQ_HANDLED
> + */
> +static irqreturn_t pwron_irq(int irq, void *palmas_pwron)
> +{
> +	struct palmas_pwron *pwron = palmas_pwron;
> +	struct input_dev *input_dev = pwron->input_dev;
> +
> +	pwron->current_state = PALMAS_PWR_KEY_PRESS;
> +
> +	input_report_key(input_dev, KEY_POWER, pwron->current_state);
> +	pm_wakeup_event(input_dev->dev.parent, 0);
> +	input_sync(input_dev);
> +
> +	mod_delayed_work(system_wq, &pwron->input_work,
> +			 msecs_to_jiffies(pwron->key_recheck_ms));
> +
> +	return IRQ_HANDLED;
> +}
> +
> +/**
> + * palmas_pwron_params_ofinit() - device tree parameter parser
> + * @dev:	palmas button device
> + * @config:	configuration params that this fills up
> + */
> +static void palmas_pwron_params_ofinit(struct device *dev,
> +				       struct palmas_pwron_config *config)

Maybe we should change this to return an int so that if the DT is not populated
then the LPK and debounce is not set but we continue anyway.

Is there support for platform data itself?

> +{
> +	struct device_node *np;
> +	u32 val;
> +	int i;
> +	u8 lpk_times[] = { 6, 8, 10, 12 };
> +	int pwr_on_deb_ms[] = { 15, 100, 500, 1000 };
> +
> +	/* Legacy boot? */
> +	if (!of_have_populated_dt())
> +		return;
> +
> +	np = of_node_get(dev->of_node);
> +	/* Mixed boot? */

Can we expand a little on Mixed boot vs legacy boot?

> +	if (!np)
> +		return;
> +
> +	val = 0;

Is this necessary?

> +	of_property_read_u32(np, "ti,palmas-long-press-seconds", &val);

Probably should check the return to make sure the value exists and that is is
within an expected range.  Since this is an optional parameter it may not be
populated.  And below it sets a preliminary value to the max as the default.

Maybe the default setting should be set at the beginning of this function so
that if there is no dt data then at least the values will be defaulted.

> +	config->long_press_time_val = ARRAY_SIZE(lpk_times) - 1;
> +	for (i = 0; i < ARRAY_SIZE(lpk_times); i++) {
> +		if (val <= lpk_times[i]) {
> +			config->long_press_time_val = i;
> +			break;
> +		}
> +	}
> +
> +	val = 0;

Don't think we need this either if we check the return on the call
below.

> +	of_property_read_u32(np, "ti,palmas-pwron-debounce-milli-seconds",
> +			     &val);


Probably should check the return to make sure the value exists and that is is
within an expected range.

> +	config->pwron_debounce_val = 0;

Should this not have been 0 anyway?

> +	for (i = 0; i < ARRAY_SIZE(pwr_on_deb_ms); i++) {
> +		if (val <= pwr_on_deb_ms[i]) {
> +			config->pwron_debounce_val = i;
> +			break;
> +		}
> +	}
> +
> +	dev_info(dev, "h/w controlled shutdown duration=%d seconds\n",
> +		 lpk_times[config->long_press_time_val]);
> +
> +	of_node_put(np);
> +}
> +
> +/**
> + * palmas_pwron_probe() - probe
> + * @pdev:	platform device for the button
> + *
> + * Return: 0 for successful probe else appropriate error
> + */
> +static int palmas_pwron_probe(struct platform_device *pdev)
> +{
> +	struct palmas *palmas = dev_get_drvdata(pdev->dev.parent);
> +	struct device *dev = &pdev->dev;
> +	struct input_dev *input_dev;
> +	struct palmas_pwron *pwron;
> +	int irq, ret, val;
> +	struct palmas_pwron_config config = { 0 };
> +
> +	palmas_pwron_params_ofinit(dev, &config);
> +
> +	pwron = devm_kzalloc(dev, sizeof(*pwron), GFP_KERNEL);
> +	if (!pwron)
> +		return -ENOMEM;
> +
> +	input_dev = devm_input_allocate_device(dev);
> +	if (!input_dev) {
> +		dev_err(dev, "Can't allocate power button\n");
> +		return -ENOMEM;
> +	}
> +
> +	/*
> +	 * Setup default hardware shutdown option (long key press)
> +	 * and debounce.
> +	 */
> +	val = config.long_press_time_val << __ffs(PALMAS_LPK_TIME_MASK);
> +	val |= config.pwron_debounce_val << __ffs(PALMAS_PWRON_DEBOUNCE_MASK);
> +	ret = palmas_update_bits(palmas, PALMAS_PMU_CONTROL_BASE,
> +				 PALMAS_LONG_PRESS_KEY,
> +				 PALMAS_LPK_TIME_MASK |
> +				 PALMAS_PWRON_DEBOUNCE_MASK, val);
> +	if (ret < 0) {
> +		dev_err(dev, "LONG_PRESS_KEY_UPDATE failed!\n");
> +		return ret;
> +	}
> +
> +	input_dev->evbit[0] = BIT_MASK(EV_KEY);
> +	input_dev->keybit[BIT_WORD(KEY_POWER)] = BIT_MASK(KEY_POWER);
> +	input_dev->name = "palmas_pwron";
> +	input_dev->phys = "palmas_pwron/input0";
> +	input_dev->dev.parent = dev;
> +
> +	pwron->palmas = palmas;
> +	pwron->input_dev = input_dev;
> +
> +	INIT_DELAYED_WORK(&pwron->input_work, palmas_power_button_work);
> +
> +	irq = platform_get_irq(pdev, 0);
> +	ret = request_threaded_irq(irq, NULL, pwron_irq,
> +				   IRQF_TRIGGER_HIGH |
> +				   IRQF_TRIGGER_LOW, dev_name(dev), pwron);
> +	if (ret < 0) {
> +		dev_err(dev, "Can't get IRQ for pwron: %d\n", ret);
> +		return ret;
> +	}
> +
> +	device_init_wakeup(dev, true);
> +
> +	ret = input_register_device(input_dev);
> +	if (ret) {
> +		free_irq(irq, pwron);
> +		cancel_delayed_work_sync(&pwron->input_work);
> +		dev_dbg(dev, "Can't register power button: %d\n", ret);
> +		return ret;
> +	}
> +	pwron->irq = irq;
> +
> +	pwron->key_recheck_ms = PALMAS_PWR_KEY_Q_TIME_MS;
> +
> +	platform_set_drvdata(pdev, pwron);
> +
> +	return 0;
> +}
> +
> +/**
> + * palmas_pwron_remove() - Cleanup on removal
> + * @pdev:	platform device for the button
> + *
> + * Return: 0
> + */
> +static int palmas_pwron_remove(struct platform_device *pdev)
> +{
> +	struct palmas_pwron *pwron = platform_get_drvdata(pdev);
> +
> +	free_irq(pwron->irq, pwron);
> +	cancel_delayed_work_sync(&pwron->input_work);
> +	input_unregister_device(pwron->input_dev);
> +
> +	return 0;
> +}
> +
> +/**
> + * palmas_pwron_suspend() - suspend handler
> + * @dev:	power button device
> + *
> + * Cancel all pending work items for the power button, setup irq for wakeup
> + *
> + * Return: 0
> + */
> +static int palmas_pwron_suspend(struct device *dev)
> +{
> +	struct platform_device *pdev = to_platform_device(dev);
> +	struct palmas_pwron *pwron = platform_get_drvdata(pdev);
> +
> +	cancel_delayed_work_sync(&pwron->input_work);
> +
> +	if (device_may_wakeup(dev))
> +		enable_irq_wake(pwron->irq);
> +
> +	return 0;
> +}
> +
> +/**
> + * palmas_pwron_resume() - resume handler
> + * @dev:	power button device
> + *
> + * Just disable the wakeup capability of irq here.
> + *
> + * Return: 0
> + */
> +static int palmas_pwron_resume(struct device *dev)
> +{
> +	struct platform_device *pdev = to_platform_device(dev);
> +	struct palmas_pwron *pwron = platform_get_drvdata(pdev);
> +
> +	if (device_may_wakeup(dev))
> +		disable_irq_wake(pwron->irq);
> +
> +	return 0;
> +}
> +
> +static SIMPLE_DEV_PM_OPS(palmas_pwron_pm,
> +			 palmas_pwron_suspend, palmas_pwron_resume);
> +
> +#ifdef CONFIG_OF
> +static struct of_device_id of_palmas_pwr_match[] = {
> +	{.compatible = "ti,palmas-pwrbutton"},
> +	{},
> +};
> +
> +MODULE_DEVICE_TABLE(of, of_palmas_pwr_match);
> +#endif
> +
> +static struct platform_driver palmas_pwron_driver = {
> +	.probe = palmas_pwron_probe,
> +	.remove = palmas_pwron_remove,
> +	.driver = {
> +		   .name = "palmas_pwrbutton",
> +		   .owner = THIS_MODULE,
> +		   .of_match_table = of_match_ptr(of_palmas_pwr_match),
> +		   .pm = &palmas_pwron_pm,
> +	},
> +};
> +module_platform_driver(palmas_pwron_driver);
> +
> +MODULE_ALIAS("platform:palmas-pwrbutton");
> +MODULE_DESCRIPTION("Palmas Power Button");
> +MODULE_LICENSE("GPL V2");
> +MODULE_AUTHOR("Texas Instruments Inc.");
> 


-- 
------------------
Dan Murphy

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

* [PATCH V2 2/2] Input: misc: introduce palmas-pwrbutton
  2014-08-21 16:02   ` [PATCH V2 2/2] Input: misc: introduce palmas-pwrbutton Nishanth Menon
  2014-08-21 16:59     ` Murphy, Dan
@ 2014-08-21 17:05     ` Dmitry Torokhov
  2014-08-21 17:09       ` Nishanth Menon
  1 sibling, 1 reply; 24+ messages in thread
From: Dmitry Torokhov @ 2014-08-21 17:05 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Nishanth,

On Thu, Aug 21, 2014 at 11:02:15AM -0500, Nishanth Menon wrote:
> +
> +	ret = input_register_device(input_dev);
> +	if (ret) {
> +		free_irq(irq, pwron);

You can not use free_irq with devm-managed resources. As I mentioned, since you
need manual unwinding, I'd rather you not use managed resources in the driver
at all.

> +		cancel_delayed_work_sync(&pwron->input_work);
> +		dev_dbg(dev, "Can't register power button: %d\n", ret);
> +		return ret;

Thanks.

-- 
Dmitry

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

* [PATCH V2 2/2] Input: misc: introduce palmas-pwrbutton
  2014-08-21 17:05     ` Dmitry Torokhov
@ 2014-08-21 17:09       ` Nishanth Menon
  0 siblings, 0 replies; 24+ messages in thread
From: Nishanth Menon @ 2014-08-21 17:09 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Aug 21, 2014 at 12:05 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
>
>
> You can not use free_irq with devm-managed resources. As I mentioned, since you
> need manual unwinding, I'd rather you not use managed resources in the driver
> at all.
ok. will drop all devm_  ops in the next version.

---
Regards,
Nishanth Menon

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

* [PATCH V2 2/2] Input: misc: introduce palmas-pwrbutton
  2014-08-21 16:59     ` Murphy, Dan
@ 2014-08-21 17:12       ` Nishanth Menon
  2014-08-21 17:19       ` Nishanth Menon
  1 sibling, 0 replies; 24+ messages in thread
From: Nishanth Menon @ 2014-08-21 17:12 UTC (permalink / raw)
  To: linux-arm-kernel

On 08/21/2014 11:59 AM, Murphy, Dan wrote:
Thanks for the review.
[..]
>> +#include <linux/init.h>
>> +#include <linux/input.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/kernel.h>
>> +#include <linux/mfd/palmas.h>
>> +#include <linux/module.h>
>> +#include <linux/of.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/reboot.h>
> 
> I don't see any reboot calls made do we need this?

Arrgh.. yes. will drop.

[..]

>> +/**
>> + * palmas_pwron_params_ofinit() - device tree parameter parser
>> + * @dev:	palmas button device
>> + * @config:	configuration params that this fills up
>> + */
>> +static void palmas_pwron_params_ofinit(struct device *dev,
>> +				       struct palmas_pwron_config *config)
> 
> Maybe we should change this to return an int so that if the DT is not populated
> then the LPK and debounce is not set but we continue anyway.

Why? these are optional properties, the defaults are 12 seconds for
LPK and 15 ms for debounce. there is no reason for it to return an
error value at this point.

> 
> Is there support for platform data itself?

No platform data support. The driver can function without these
properties - these are not mandatory.

>> +{
>> +	struct device_node *np;
>> +	u32 val;
>> +	int i;
>> +	u8 lpk_times[] = { 6, 8, 10, 12 };
>> +	int pwr_on_deb_ms[] = { 15, 100, 500, 1000 };
>> +
>> +	/* Legacy boot? */
>> +	if (!of_have_populated_dt())
>> +		return;
>> +
>> +	np = of_node_get(dev->of_node);
>> +	/* Mixed boot? */
> 
> Can we expand a little on Mixed boot vs legacy boot?

Are you looking for "device tree + platform boot" ? legacy boot being
"non device tree boot"?
>> +
>> +	val = 0;
> 
> Is this necessary?
> 
>> +	of_property_read_u32(np, "ti,palmas-long-press-seconds", &val);
> 
> Probably should check the return to make sure the value exists and that is is
> within an expected range.  Since this is an optional parameter it may not be
> populated.  And below it sets a preliminary value to the max as the default.
> 
> Maybe the default setting should be set at the beginning of this function so
> that if there is no dt data then at least the values will be defaulted.
> 
>> +	config->long_press_time_val = ARRAY_SIZE(lpk_times) - 1;
>> +	for (i = 0; i < ARRAY_SIZE(lpk_times); i++) {
>> +		if (val <= lpk_times[i]) {
>> +			config->long_press_time_val = i;
>> +			break;
>> +		}
>> +	}
>> +
>> +	val = 0;
> 
> Don't think we need this either if we check the return on the call
> below.

k, I can redo the sequence a little better here.

> 
>> +	of_property_read_u32(np, "ti,palmas-pwron-debounce-milli-seconds",
>> +			     &val);
> 
> 
> Probably should check the return to make sure the value exists and that is is
> within an expected range.

Not necessary.
> 
>> +	config->pwron_debounce_val = 0;
> 
> Should this not have been 0 anyway?

it is, was explicit in this case.
> 
>> +	for (i = 0; i < ARRAY_SIZE(pwr_on_deb_ms); i++) {
>> +		if (val <= pwr_on_deb_ms[i]) {
>> +			config->pwron_debounce_val = i;
>> +			break;
>> +		}
>> +	}
>> +
>> +	dev_info(dev, "h/w controlled shutdown duration=%d seconds\n",
>> +		 lpk_times[config->long_press_time_val]);
>> +
>> +	of_node_put(np);
>> +}
>> +
[...]

-- 
Regards,
Nishanth Menon

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

* [PATCH V2 2/2] Input: misc: introduce palmas-pwrbutton
  2014-08-21 16:59     ` Murphy, Dan
  2014-08-21 17:12       ` Nishanth Menon
@ 2014-08-21 17:19       ` Nishanth Menon
  2014-08-21 17:32         ` Murphy, Dan
  1 sibling, 1 reply; 24+ messages in thread
From: Nishanth Menon @ 2014-08-21 17:19 UTC (permalink / raw)
  To: linux-arm-kernel

On 08/21/2014 11:59 AM, Murphy, Dan wrote:
[...]
Ooops.. missed answering one addition statement:

>> +	of_property_read_u32(np, "ti,palmas-long-press-seconds", &val);
> 
> Probably should check the return to make sure the value exists and that is is
> within an expected range.
It is an optional parameter and may not exist in dt. when it does
exist, the logic tries to do a best match (this is the for loop in the
logic just below).

-- 
Regards,
Nishanth Menon

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

* [PATCH V2 2/2] Input: misc: introduce palmas-pwrbutton
  2014-08-21 17:19       ` Nishanth Menon
@ 2014-08-21 17:32         ` Murphy, Dan
  2014-08-21 17:37           ` Nishanth Menon
  0 siblings, 1 reply; 24+ messages in thread
From: Murphy, Dan @ 2014-08-21 17:32 UTC (permalink / raw)
  To: linux-arm-kernel

On 08/21/2014 12:19 PM, Menon, Nishanth wrote:
> On 08/21/2014 11:59 AM, Murphy, Dan wrote:
> [...]
> Ooops.. missed answering one addition statement:
> 
>>> +	of_property_read_u32(np, "ti,palmas-long-press-seconds", &val);
>>
>> Probably should check the return to make sure the value exists and that is is
>> within an expected range.
> It is an optional parameter and may not exist in dt. when it does
> exist, the logic tries to do a best match (this is the for loop in the
> logic just below).
> 

The issue is val might be returned as a negative which will then proceed to
set the config->long_press_time_val to the lowest time value which then overrides
your initial setting of config->long_press_time_val = ARRAY_SIZE(lpk_times) - 1;

Dan

-- 
------------------
Dan Murphy

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

* [PATCH V2 2/2] Input: misc: introduce palmas-pwrbutton
  2014-08-21 17:32         ` Murphy, Dan
@ 2014-08-21 17:37           ` Nishanth Menon
  2014-08-21 17:46             ` Murphy, Dan
  2014-08-21 18:03             ` Dmitry Torokhov
  0 siblings, 2 replies; 24+ messages in thread
From: Nishanth Menon @ 2014-08-21 17:37 UTC (permalink / raw)
  To: linux-arm-kernel

On 12:32-20140821, Murphy, Dan wrote:
> On 08/21/2014 12:19 PM, Menon, Nishanth wrote:
> > On 08/21/2014 11:59 AM, Murphy, Dan wrote:
> > [...]
> > Ooops.. missed answering one addition statement:
> > 
> >>> +	of_property_read_u32(np, "ti,palmas-long-press-seconds", &val);
> >>
> >> Probably should check the return to make sure the value exists and that is is
> >> within an expected range.
> > It is an optional parameter and may not exist in dt. when it does
> > exist, the logic tries to do a best match (this is the for loop in the
> > logic just below).
> > 
> 
> The issue is val might be returned as a negative which will then proceed to
> set the config->long_press_time_val to the lowest time value which then overrides
> your initial setting of config->long_press_time_val = ARRAY_SIZE(lpk_times) - 1;
> 

Does the following as a replacement look OK? if yes, I can incorporate
as part of v3 of this series.
/**
 * palmas_pwron_params_ofinit() - device tree parameter parser
 * @dev:	palmas button device
 * @config:	configuration params that this fills up
 */
static void palmas_pwron_params_ofinit(struct device *dev,
				       struct palmas_pwron_config *config)
{
	struct device_node *np;
	u32 val;
	int i, ret;
	u8 lpk_times[] = { 6, 8, 10, 12 };
	int pwr_on_deb_ms[] = { 15, 100, 500, 1000 };

	/* Handle cases where device node based configuration is not present */
	if (!of_have_populated_dt())
		return;
	np = of_node_get(dev->of_node);
	if (!np)
		return;

	/* Default config parameters - least debounce, max long key press */
	config->pwron_debounce_val = 0;
	config->long_press_time_val = ARRAY_SIZE(lpk_times) - 1;

	ret = of_property_read_u32(np, "ti,palmas-long-press-seconds", &val);
	if (ret)
		goto skip_lpk;
	for (i = 0; i < ARRAY_SIZE(lpk_times); i++) {
		if (val <= lpk_times[i]) {
			config->long_press_time_val = i;
			break;
		}
	}

skip_lpk:
	ret = of_property_read_u32(np, "ti,palmas-pwron-debounce-milli-seconds",
				   &val);
	if (ret)
		goto skip_debounce;
	for (i = 0; i < ARRAY_SIZE(pwr_on_deb_ms); i++) {
		if (val <= pwr_on_deb_ms[i]) {
			config->pwron_debounce_val = i;
			break;
		}
	}

skip_debounce:
	dev_info(dev, "h/w controlled shutdown duration=%d seconds\n",
		 lpk_times[config->long_press_time_val]);

	of_node_put(np);
}
-- 
Regards,
Nishanth Menon

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

* [PATCH V2 2/2] Input: misc: introduce palmas-pwrbutton
  2014-08-21 17:37           ` Nishanth Menon
@ 2014-08-21 17:46             ` Murphy, Dan
  2014-08-21 18:03             ` Dmitry Torokhov
  1 sibling, 0 replies; 24+ messages in thread
From: Murphy, Dan @ 2014-08-21 17:46 UTC (permalink / raw)
  To: linux-arm-kernel

On 08/21/2014 12:37 PM, Menon, Nishanth wrote:
> On 12:32-20140821, Murphy, Dan wrote:
>> On 08/21/2014 12:19 PM, Menon, Nishanth wrote:
>>> On 08/21/2014 11:59 AM, Murphy, Dan wrote:
>>> [...]
>>> Ooops.. missed answering one addition statement:
>>>
>>>>> +	of_property_read_u32(np, "ti,palmas-long-press-seconds", &val);
>>>>
>>>> Probably should check the return to make sure the value exists and that is is
>>>> within an expected range.
>>> It is an optional parameter and may not exist in dt. when it does
>>> exist, the logic tries to do a best match (this is the for loop in the
>>> logic just below).
>>>
>>
>> The issue is val might be returned as a negative which will then proceed to
>> set the config->long_press_time_val to the lowest time value which then overrides
>> your initial setting of config->long_press_time_val = ARRAY_SIZE(lpk_times) - 1;
>>
> 
> Does the following as a replacement look OK? if yes, I can incorporate
> as part of v3 of this series.
> /**
>  * palmas_pwron_params_ofinit() - device tree parameter parser
>  * @dev:	palmas button device
>  * @config:	configuration params that this fills up
>  */
> static void palmas_pwron_params_ofinit(struct device *dev,
> 				       struct palmas_pwron_config *config)
> {
> 	struct device_node *np;
> 	u32 val;
> 	int i, ret;
> 	u8 lpk_times[] = { 6, 8, 10, 12 };
> 	int pwr_on_deb_ms[] = { 15, 100, 500, 1000 };
> 
> 	/* Handle cases where device node based configuration is not present */
> 	if (!of_have_populated_dt())
> 		return;
> 	np = of_node_get(dev->of_node);
> 	if (!np)
> 		return;
> 
> 	/* Default config parameters - least debounce, max long key press */
> 	config->pwron_debounce_val = 0;

And you can drop this as this value is set to zero in the probe when initialized.

> 	config->long_press_time_val = ARRAY_SIZE(lpk_times) - 1;

I would move this up to just before checking the dt.  Then they are defaulted
properly or move these to the probe.  Otherwise defaults are not set.

> 
> 	ret = of_property_read_u32(np, "ti,palmas-long-press-seconds", &val);
> 	if (ret)
> 		goto skip_lpk;
> 	for (i = 0; i < ARRAY_SIZE(lpk_times); i++) {
> 		if (val <= lpk_times[i]) {
> 			config->long_press_time_val = i;
> 			break;
> 		}
> 	}
> 
> skip_lpk:
> 	ret = of_property_read_u32(np, "ti,palmas-pwron-debounce-milli-seconds",
> 				   &val);
> 	if (ret)
> 		goto skip_debounce;
> 	for (i = 0; i < ARRAY_SIZE(pwr_on_deb_ms); i++) {
> 		if (val <= pwr_on_deb_ms[i]) {
> 			config->pwron_debounce_val = i;
> 			break;
> 		}
> 	}
> 
> skip_debounce:
> 	dev_info(dev, "h/w controlled shutdown duration=%d seconds\n",
> 		 lpk_times[config->long_press_time_val]);
> 
> 	of_node_put(np);
> }
> 


-- 
------------------
Dan Murphy

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

* [PATCH V2 2/2] Input: misc: introduce palmas-pwrbutton
  2014-08-21 17:37           ` Nishanth Menon
  2014-08-21 17:46             ` Murphy, Dan
@ 2014-08-21 18:03             ` Dmitry Torokhov
  2014-08-21 19:01               ` Nishanth Menon
  1 sibling, 1 reply; 24+ messages in thread
From: Dmitry Torokhov @ 2014-08-21 18:03 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Aug 21, 2014 at 12:37:15PM -0500, Nishanth Menon wrote:
> On 12:32-20140821, Murphy, Dan wrote:
> > On 08/21/2014 12:19 PM, Menon, Nishanth wrote:
> > > On 08/21/2014 11:59 AM, Murphy, Dan wrote:
> > > [...]
> > > Ooops.. missed answering one addition statement:
> > > 
> > >>> +	of_property_read_u32(np, "ti,palmas-long-press-seconds", &val);
> > >>
> > >> Probably should check the return to make sure the value exists and that is is
> > >> within an expected range.
> > > It is an optional parameter and may not exist in dt. when it does
> > > exist, the logic tries to do a best match (this is the for loop in the
> > > logic just below).
> > > 
> > 
> > The issue is val might be returned as a negative which will then proceed to
> > set the config->long_press_time_val to the lowest time value which then overrides
> > your initial setting of config->long_press_time_val = ARRAY_SIZE(lpk_times) - 1;
> > 
> 
> Does the following as a replacement look OK? if yes, I can incorporate
> as part of v3 of this series.
> /**
>  * palmas_pwron_params_ofinit() - device tree parameter parser
>  * @dev:	palmas button device
>  * @config:	configuration params that this fills up
>  */
> static void palmas_pwron_params_ofinit(struct device *dev,
> 				       struct palmas_pwron_config *config)
> {
> 	struct device_node *np;
> 	u32 val;
> 	int i, ret;
> 	u8 lpk_times[] = { 6, 8, 10, 12 };
> 	int pwr_on_deb_ms[] = { 15, 100, 500, 1000 };
> 
> 	/* Handle cases where device node based configuration is not present */
> 	if (!of_have_populated_dt())
> 		return;
> 	np = of_node_get(dev->of_node);
> 	if (!np)
> 		return;
> 
> 	/* Default config parameters - least debounce, max long key press */
> 	config->pwron_debounce_val = 0;
> 	config->long_press_time_val = ARRAY_SIZE(lpk_times) - 1;
> 
> 	ret = of_property_read_u32(np, "ti,palmas-long-press-seconds", &val);
> 	if (ret)
> 		goto skip_lpk;

Just without gotos please. They are nice in error handling paths and in some
other rare circumstances, but not here.

> 	for (i = 0; i < ARRAY_SIZE(lpk_times); i++) {
> 		if (val <= lpk_times[i]) {
> 			config->long_press_time_val = i;
> 			break;
> 		}
> 	}
> 
> skip_lpk:
> 	ret = of_property_read_u32(np, "ti,palmas-pwron-debounce-milli-seconds",
> 				   &val);
> 	if (ret)
> 		goto skip_debounce;
> 	for (i = 0; i < ARRAY_SIZE(pwr_on_deb_ms); i++) {
> 		if (val <= pwr_on_deb_ms[i]) {
> 			config->pwron_debounce_val = i;
> 			break;
> 		}
> 	}
> 
> skip_debounce:
> 	dev_info(dev, "h/w controlled shutdown duration=%d seconds\n",
> 		 lpk_times[config->long_press_time_val]);
> 
> 	of_node_put(np);

BTW, I do not think you need to use of_node_get/put here, it's not going anywhere.

Thanks.

-- 
Dmitry

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

* [PATCH V3 2/2] Input: misc: introduce palmas-pwrbutton
  2014-08-18 20:13 [PATCH 0/2] Input: palmas: add support for palmas power button Nishanth Menon
                   ` (2 preceding siblings ...)
  2014-08-21 16:02 ` [PATCH V2 0/2] Input: palmas: add support for palmas power button Nishanth Menon
@ 2014-08-21 18:52 ` Nishanth Menon
  3 siblings, 0 replies; 24+ messages in thread
From: Nishanth Menon @ 2014-08-21 18:52 UTC (permalink / raw)
  To: linux-arm-kernel

Many palmas family of PMICs have support for interrupt based power
button. This allows the device to notify the processor of external
push button events over the shared palmas interrupt. However, this
event is generated only during a "press" operation. Software is
supposed to poll(sigh!) for detecting a release event.

The PMIC also supports ability to power off independent of the
software decisions when the button is pressed for a long duration if
the PMIC is appropriately configured on the platform.

Even though the function is similar to twl4030_pwrbutton, it is
substantially different in operation to belong to a new driver of it's
own.

Based on original work done by Girish S Ghongdemath <girishsg@ti.com>

Signed-off-by: Nishanth Menon <nm@ti.com>
---

I am not re-posting patch #1/2 as it is already available in
https://patchwork.kernel.org/patch/4758681/ without any comments. Do
let me know if we want it reposted.

Changes since V2:
 - Integrated review comments of v2 (improvement in dt parse, header
   cleanups, no more managed resource usage)

V2: https://patchwork.kernel.org/patch/4758711/
V1: https://patchwork.kernel.org/patch/4739041/

 drivers/input/misc/Kconfig            |   10 +
 drivers/input/misc/Makefile           |    1 +
 drivers/input/misc/palmas-pwrbutton.c |  368 +++++++++++++++++++++++++++++++++
 3 files changed, 379 insertions(+)
 create mode 100644 drivers/input/misc/palmas-pwrbutton.c

diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
index 2ff4425..0224dcf 100644
--- a/drivers/input/misc/Kconfig
+++ b/drivers/input/misc/Kconfig
@@ -451,6 +451,16 @@ config HP_SDC_RTC
 	  Say Y here if you want to support the built-in real time clock
 	  of the HP SDC controller.
 
+config INPUT_PALMAS_PWRBUTTON
+	tristate "Palmas Power button Driver"
+	depends on MFD_PALMAS
+	help
+	  Say Y here if you want to enable power key reporting via the
+	  Palmas family of PMICs.
+
+	  To compile this driver as a module, choose M here. The module will
+	  be called palmas_pwrbutton.
+
 config INPUT_PCF50633_PMU
 	tristate "PCF50633 PMU events"
 	depends on MFD_PCF50633
diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
index 4955ad3..e3d5efb 100644
--- a/drivers/input/misc/Makefile
+++ b/drivers/input/misc/Makefile
@@ -40,6 +40,7 @@ obj-$(CONFIG_INPUT_MAX8997_HAPTIC)	+= max8997_haptic.o
 obj-$(CONFIG_INPUT_MC13783_PWRBUTTON)	+= mc13783-pwrbutton.o
 obj-$(CONFIG_INPUT_MMA8450)		+= mma8450.o
 obj-$(CONFIG_INPUT_MPU3050)		+= mpu3050.o
+obj-$(CONFIG_INPUT_PALMAS_PWRBUTTON)	+= palmas-pwrbutton.o
 obj-$(CONFIG_INPUT_PCAP)		+= pcap_keys.o
 obj-$(CONFIG_INPUT_PCF50633_PMU)	+= pcf50633-input.o
 obj-$(CONFIG_INPUT_PCF8574)		+= pcf8574_keypad.o
diff --git a/drivers/input/misc/palmas-pwrbutton.c b/drivers/input/misc/palmas-pwrbutton.c
new file mode 100644
index 0000000..4d48108
--- /dev/null
+++ b/drivers/input/misc/palmas-pwrbutton.c
@@ -0,0 +1,368 @@
+/*
+ * Texas Instruments' Palmas Power Button Input Driver
+ *
+ * Copyright (C) 2012-2014 Texas Instruments Incorporated - http://www.ti.com/
+ *	Girish S Ghongdemath
+ *	Nishanth Menon
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed "as is" WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+#include <linux/init.h>
+#include <linux/input.h>
+#include <linux/interrupt.h>
+#include <linux/kernel.h>
+#include <linux/mfd/palmas.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+
+#define PALMAS_LPK_TIME_MASK		0x0c
+#define PALMAS_PWRON_DEBOUNCE_MASK	0x03
+#define PALMAS_PWR_KEY_PRESS		0x01
+#define PALMAS_PWR_KEY_Q_TIME_MS	20
+
+/**
+ * struct palmas_pwron - Palmas power on data
+ * @palmas:		pointer to palmas device
+ * @input_dev:		pointer to input device
+ * @irq:		irq that we are hooked on to
+ * @input_work:		work for detecting release of key
+ * @current_state:	key current state
+ * @key_recheck_ms:	duration for recheck of key (in milli-seconds)
+ */
+struct palmas_pwron {
+	struct palmas *palmas;
+	struct input_dev *input_dev;
+	int irq;
+	struct delayed_work input_work;
+	int current_state;
+	u32 key_recheck_ms;
+};
+
+/**
+ * struct palmas_pwron_config - configuration of palmas power on
+ * @long_press_time_val:	value for long press h/w shutdown event
+ * @pwron_debounce_val:		value for debounce of power button
+ */
+struct palmas_pwron_config {
+	u8 long_press_time_val;
+	u8 pwron_debounce_val;
+};
+
+/**
+ * palmas_get_pwr_state() - read button state
+ * @pwron: pointer to pwron struct
+ *
+ * Return: 0 for no press, PALMAS_PWR_KEY_PRESS for keypress
+ * and on error, appropriate error value.
+ */
+static int palmas_get_pwr_state(struct palmas_pwron *pwron)
+{
+	struct input_dev *input_dev = pwron->input_dev;
+	struct device *dev = input_dev->dev.parent;
+	unsigned int reg = 0;
+	int ret;
+
+	ret = palmas_read(pwron->palmas, PALMAS_INTERRUPT_BASE,
+			  PALMAS_INT1_LINE_STATE, &reg);
+	if (ret) {
+		dev_err(dev, "%s:Cannot read palmas PWRON status(%d)\n",
+			__func__, ret);
+		return ret;
+	}
+
+	/* PWRON line state is BIT(1) of the register */
+	return reg & BIT(1) ? 0 : PALMAS_PWR_KEY_PRESS;
+}
+
+/**
+ * palmas_power_button_work() - Detects the button release event
+ * @work:	work item to detect button release
+ */
+static void palmas_power_button_work(struct work_struct *work)
+{
+	struct palmas_pwron *pwron = container_of((struct delayed_work *)work,
+						  struct palmas_pwron,
+						  input_work);
+	struct input_dev *input_dev = pwron->input_dev;
+	int next_state;
+
+	next_state = palmas_get_pwr_state(pwron);
+	if (next_state < 0)
+		return;
+
+	/*
+	 * If the state did not change then schedule a work item to check the
+	 * status of the power button line
+	 */
+	if (next_state == pwron->current_state) {
+		schedule_delayed_work(&pwron->input_work,
+				      msecs_to_jiffies(pwron->key_recheck_ms));
+		return;
+	}
+
+	pwron->current_state = next_state;
+	input_report_key(input_dev, KEY_POWER, pwron->current_state);
+	input_sync(input_dev);
+}
+
+/**
+ * pwron_irq() - button press isr
+ * @irq:		irq
+ * @palmas_pwron:	pwron struct
+ *
+ * Return: IRQ_HANDLED
+ */
+static irqreturn_t pwron_irq(int irq, void *palmas_pwron)
+{
+	struct palmas_pwron *pwron = palmas_pwron;
+	struct input_dev *input_dev = pwron->input_dev;
+
+	pwron->current_state = PALMAS_PWR_KEY_PRESS;
+
+	input_report_key(input_dev, KEY_POWER, pwron->current_state);
+	pm_wakeup_event(input_dev->dev.parent, 0);
+	input_sync(input_dev);
+
+	mod_delayed_work(system_wq, &pwron->input_work,
+			 msecs_to_jiffies(pwron->key_recheck_ms));
+
+	return IRQ_HANDLED;
+}
+
+/**
+ * palmas_pwron_params_ofinit() - device tree parameter parser
+ * @dev:	palmas button device
+ * @config:	configuration params that this fills up
+ */
+static void palmas_pwron_params_ofinit(struct device *dev,
+				       struct palmas_pwron_config *config)
+{
+	struct device_node *np;
+	u32 val;
+	int i, ret;
+	u8 lpk_times[] = { 6, 8, 10, 12 };
+	int pwr_on_deb_ms[] = { 15, 100, 500, 1000 };
+
+	/* Default config parameters */
+	config->long_press_time_val = ARRAY_SIZE(lpk_times) - 1;
+
+	/* Handle cases where device node based configuration is not present */
+	if (!of_have_populated_dt())
+		return;
+	np = of_node_get(dev->of_node);
+	if (!np)
+		return;
+
+	ret = of_property_read_u32(np, "ti,palmas-long-press-seconds", &val);
+	if (!ret) {
+		for (i = 0; i < ARRAY_SIZE(lpk_times); i++) {
+			if (val <= lpk_times[i]) {
+				config->long_press_time_val = i;
+				break;
+			}
+		}
+	}
+
+	ret = of_property_read_u32(np, "ti,palmas-pwron-debounce-milli-seconds",
+				   &val);
+	if (!ret) {
+		for (i = 0; i < ARRAY_SIZE(pwr_on_deb_ms); i++) {
+			if (val <= pwr_on_deb_ms[i]) {
+				config->pwron_debounce_val = i;
+				break;
+			}
+		}
+	}
+
+	dev_info(dev, "h/w controlled shutdown duration=%d seconds\n",
+		 lpk_times[config->long_press_time_val]);
+
+	of_node_put(np);
+}
+
+/**
+ * palmas_pwron_probe() - probe
+ * @pdev:	platform device for the button
+ *
+ * Return: 0 for successful probe else appropriate error
+ */
+static int palmas_pwron_probe(struct platform_device *pdev)
+{
+	struct palmas *palmas = dev_get_drvdata(pdev->dev.parent);
+	struct device *dev = &pdev->dev;
+	struct input_dev *input_dev;
+	struct palmas_pwron *pwron;
+	int irq, ret, val;
+	struct palmas_pwron_config config = { 0 };
+
+	palmas_pwron_params_ofinit(dev, &config);
+
+	pwron = kzalloc(sizeof(*pwron), GFP_KERNEL);
+	if (!pwron)
+		return -ENOMEM;
+
+	input_dev = input_allocate_device();
+	if (!input_dev) {
+		dev_err(dev, "Can't allocate power button\n");
+		ret = -ENOMEM;
+		goto exit_pwron;
+	}
+
+	/*
+	 * Setup default hardware shutdown option (long key press)
+	 * and debounce.
+	 */
+	val = config.long_press_time_val << __ffs(PALMAS_LPK_TIME_MASK);
+	val |= config.pwron_debounce_val << __ffs(PALMAS_PWRON_DEBOUNCE_MASK);
+	ret = palmas_update_bits(palmas, PALMAS_PMU_CONTROL_BASE,
+				 PALMAS_LONG_PRESS_KEY,
+				 PALMAS_LPK_TIME_MASK |
+				 PALMAS_PWRON_DEBOUNCE_MASK, val);
+	if (ret < 0) {
+		dev_err(dev, "LONG_PRESS_KEY_UPDATE failed!\n");
+		goto exit_input_dev;
+	}
+
+	input_dev->evbit[0] = BIT_MASK(EV_KEY);
+	input_dev->keybit[BIT_WORD(KEY_POWER)] = BIT_MASK(KEY_POWER);
+	input_dev->name = "palmas_pwron";
+	input_dev->phys = "palmas_pwron/input0";
+	input_dev->dev.parent = dev;
+
+	pwron->palmas = palmas;
+	pwron->input_dev = input_dev;
+
+	INIT_DELAYED_WORK(&pwron->input_work, palmas_power_button_work);
+
+	irq = platform_get_irq(pdev, 0);
+	ret = request_threaded_irq(irq, NULL, pwron_irq,
+				   IRQF_TRIGGER_HIGH |
+				   IRQF_TRIGGER_LOW, dev_name(dev), pwron);
+	if (ret < 0) {
+		dev_err(dev, "Can't get IRQ for pwron: %d\n", ret);
+		goto exit_input_dev;
+	}
+
+	device_init_wakeup(dev, true);
+
+	ret = input_register_device(input_dev);
+	if (ret) {
+		free_irq(irq, pwron);
+		cancel_delayed_work_sync(&pwron->input_work);
+		dev_dbg(dev, "Can't register power button: %d\n", ret);
+		goto exit_free_irq;
+	}
+	pwron->irq = irq;
+
+	pwron->key_recheck_ms = PALMAS_PWR_KEY_Q_TIME_MS;
+
+	platform_set_drvdata(pdev, pwron);
+
+	return 0;
+
+exit_free_irq:
+	free_irq(irq, pwron);
+exit_input_dev:
+	input_free_device(input_dev);
+exit_pwron:
+	kfree(pwron);
+
+	return ret;
+}
+
+/**
+ * palmas_pwron_remove() - Cleanup on removal
+ * @pdev:	platform device for the button
+ *
+ * Return: 0
+ */
+static int palmas_pwron_remove(struct platform_device *pdev)
+{
+	struct palmas_pwron *pwron = platform_get_drvdata(pdev);
+
+	free_irq(pwron->irq, pwron);
+	cancel_delayed_work_sync(&pwron->input_work);
+	input_unregister_device(pwron->input_dev);
+	input_free_device(pwron->input_dev);
+	kfree(pwron);
+
+	return 0;
+}
+
+/**
+ * palmas_pwron_suspend() - suspend handler
+ * @dev:	power button device
+ *
+ * Cancel all pending work items for the power button, setup irq for wakeup
+ *
+ * Return: 0
+ */
+static int palmas_pwron_suspend(struct device *dev)
+{
+	struct platform_device *pdev = to_platform_device(dev);
+	struct palmas_pwron *pwron = platform_get_drvdata(pdev);
+
+	cancel_delayed_work_sync(&pwron->input_work);
+
+	if (device_may_wakeup(dev))
+		enable_irq_wake(pwron->irq);
+
+	return 0;
+}
+
+/**
+ * palmas_pwron_resume() - resume handler
+ * @dev:	power button device
+ *
+ * Just disable the wakeup capability of irq here.
+ *
+ * Return: 0
+ */
+static int palmas_pwron_resume(struct device *dev)
+{
+	struct platform_device *pdev = to_platform_device(dev);
+	struct palmas_pwron *pwron = platform_get_drvdata(pdev);
+
+	if (device_may_wakeup(dev))
+		disable_irq_wake(pwron->irq);
+
+	return 0;
+}
+
+static SIMPLE_DEV_PM_OPS(palmas_pwron_pm,
+			 palmas_pwron_suspend, palmas_pwron_resume);
+
+#ifdef CONFIG_OF
+static struct of_device_id of_palmas_pwr_match[] = {
+	{.compatible = "ti,palmas-pwrbutton"},
+	{},
+};
+
+MODULE_DEVICE_TABLE(of, of_palmas_pwr_match);
+#endif
+
+static struct platform_driver palmas_pwron_driver = {
+	.probe = palmas_pwron_probe,
+	.remove = palmas_pwron_remove,
+	.driver = {
+		   .name = "palmas_pwrbutton",
+		   .owner = THIS_MODULE,
+		   .of_match_table = of_match_ptr(of_palmas_pwr_match),
+		   .pm = &palmas_pwron_pm,
+	},
+};
+module_platform_driver(palmas_pwron_driver);
+
+MODULE_ALIAS("platform:palmas-pwrbutton");
+MODULE_DESCRIPTION("Palmas Power Button");
+MODULE_LICENSE("GPL V2");
+MODULE_AUTHOR("Texas Instruments Inc.");
-- 
1.7.9.5

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

* [PATCH V2 2/2] Input: misc: introduce palmas-pwrbutton
  2014-08-21 18:03             ` Dmitry Torokhov
@ 2014-08-21 19:01               ` Nishanth Menon
  2014-09-10 21:13                 ` Dmitry Torokhov
  0 siblings, 1 reply; 24+ messages in thread
From: Nishanth Menon @ 2014-08-21 19:01 UTC (permalink / raw)
  To: linux-arm-kernel

On 08/21/2014 01:03 PM, Dmitry Torokhov wrote:

I believe I have taken care of other concerns on v2, but..Arrgh.. I
did not reply to this comment..
> BTW, I do not think you need to use of_node_get/put here, it's not going anywhere.
It has been mentioned as a good practice to ensure we use get_put in
to ensure reference count is appropriately maintained. So, I have'nt
changed that in v3.

-- 
Regards,
Nishanth Menon

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

* [PATCH V2 2/2] Input: misc: introduce palmas-pwrbutton
  2014-08-21 19:01               ` Nishanth Menon
@ 2014-09-10 21:13                 ` Dmitry Torokhov
  2014-09-11 12:01                   ` Nishanth Menon
  0 siblings, 1 reply; 24+ messages in thread
From: Dmitry Torokhov @ 2014-09-10 21:13 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Nishanth,

On Thu, Aug 21, 2014 at 02:01:43PM -0500, Nishanth Menon wrote:
> On 08/21/2014 01:03 PM, Dmitry Torokhov wrote:
> 
> I believe I have taken care of other concerns on v2, but..Arrgh.. I
> did not reply to this comment..
> > BTW, I do not think you need to use of_node_get/put here, it's not going anywhere.
> It has been mentioned as a good practice to ensure we use get_put in
> to ensure reference count is appropriately maintained. So, I have'nt
> changed that in v3.

You only need to maintain reference count if you pass the handle on.
Otherwise you'd have to do get/put every time you dereference something.

Anyway, I did a few changes to the driver (no need to store current
state, do not fre einput device after unregister, etc.), could you
please tell me if the version below still works for you?

Thanks.

-- 
Dmitry

Input: introduce palmas-pwrbutton

From: Nishanth Menon <nm@ti.com>

Many palmas family of PMICs have support for interrupt based power button.
This allows the device to notify the processor of external push button
events over the shared palmas interrupt. However, this event is generated
only during a "press" operation. Software is supposed to poll(sigh!) for
detecting a release event.

The PMIC also supports ability to power off independent of the software
decisions when the button is pressed for a long duration if the PMIC is
appropriately configured on the platform.

Even though the function is similar to twl4030_pwrbutton, it is
substantially different in operation to belong to a new driver of it's own.

Based on original work done by Girish S Ghongdemath <girishsg@ti.com>

Signed-off-by: Nishanth Menon <nm@ti.com>
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 .../bindings/input/ti,palmas-pwrbutton.txt         |   36 ++
 drivers/input/misc/Kconfig                         |   10 +
 drivers/input/misc/Makefile                        |    1 
 drivers/input/misc/palmas-pwrbutton.c              |  330 ++++++++++++++++++++
 4 files changed, 377 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/input/ti,palmas-pwrbutton.txt
 create mode 100644 drivers/input/misc/palmas-pwrbutton.c

diff --git a/Documentation/devicetree/bindings/input/ti,palmas-pwrbutton.txt b/Documentation/devicetree/bindings/input/ti,palmas-pwrbutton.txt
new file mode 100644
index 0000000..a3dde8c
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/ti,palmas-pwrbutton.txt
@@ -0,0 +1,36 @@
+Texas Instruments Palmas family power button module
+
+This module is part of the Palmas family of PMICs. For more details
+about the whole chip see:
+Documentation/devicetree/bindings/mfd/palmas.txt.
+
+This module provides a simple power button event via an Interrupt.
+
+Required properties:
+- compatible: should be one of the following
+   - "ti,palmas-pwrbutton": For Palmas compatible power on button
+- interrupt-parent: Parent interrupt device, must be handle of palmas node.
+- interrupts: Interrupt number of power button submodule on device.
+
+Optional Properties:
+
+- ti,palmas-long-press-seconds: Duration in seconds which the power
+  button should be kept pressed for Palmas to power off automatically.
+  NOTE: This depends on OTP support and POWERHOLD signal configuration
+  on platform. Valid values are 6, 8, 10 and 12.
+- ti,palmas-pwron-debounce-milli-seconds: Duration in milliseconds
+  which the power button should be kept pressed for Palmas to register
+  a press for debouncing purposes. NOTE: This depends on specific
+  Palmas variation capability. Valid values are 15, 100, 500 and 1000.
+
+Example:
+
+&palmas {
+	palmas_pwr_button: pwrbutton {
+		compatible = "ti,palmas-pwrbutton";
+		interrupt-parent = <&tps659038>;
+		interrupts = <1 IRQ_TYPE_EDGE_FALLING>;
+		ti,palmas-long-press-seconds = <12>;
+		ti,palmas-pwron-debounce-milli-seconds = <15>;
+	};
+};
diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
index 51891f6..36382e5 100644
--- a/drivers/input/misc/Kconfig
+++ b/drivers/input/misc/Kconfig
@@ -451,6 +451,16 @@ config HP_SDC_RTC
 	  Say Y here if you want to support the built-in real time clock
 	  of the HP SDC controller.
 
+config INPUT_PALMAS_PWRBUTTON
+	tristate "Palmas Power button Driver"
+	depends on MFD_PALMAS
+	help
+	  Say Y here if you want to enable power key reporting via the
+	  Palmas family of PMICs.
+
+	  To compile this driver as a module, choose M here. The module will
+	  be called palmas_pwrbutton.
+
 config INPUT_PCF50633_PMU
 	tristate "PCF50633 PMU events"
 	depends on MFD_PCF50633
diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
index e0cee17..e8b84d2 100644
--- a/drivers/input/misc/Makefile
+++ b/drivers/input/misc/Makefile
@@ -42,6 +42,7 @@ obj-$(CONFIG_INPUT_MAX8997_HAPTIC)	+= max8997_haptic.o
 obj-$(CONFIG_INPUT_MC13783_PWRBUTTON)	+= mc13783-pwrbutton.o
 obj-$(CONFIG_INPUT_MMA8450)		+= mma8450.o
 obj-$(CONFIG_INPUT_MPU3050)		+= mpu3050.o
+obj-$(CONFIG_INPUT_PALMAS_PWRBUTTON)	+= palmas-pwrbutton.o
 obj-$(CONFIG_INPUT_PCAP)		+= pcap_keys.o
 obj-$(CONFIG_INPUT_PCF50633_PMU)	+= pcf50633-input.o
 obj-$(CONFIG_INPUT_PCF8574)		+= pcf8574_keypad.o
diff --git a/drivers/input/misc/palmas-pwrbutton.c b/drivers/input/misc/palmas-pwrbutton.c
new file mode 100644
index 0000000..3f90211
--- /dev/null
+++ b/drivers/input/misc/palmas-pwrbutton.c
@@ -0,0 +1,330 @@
+/*
+ * Texas Instruments' Palmas Power Button Input Driver
+ *
+ * Copyright (C) 2012-2014 Texas Instruments Incorporated - http://www.ti.com/
+ *	Girish S Ghongdemath
+ *	Nishanth Menon
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed "as is" WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/init.h>
+#include <linux/input.h>
+#include <linux/interrupt.h>
+#include <linux/kernel.h>
+#include <linux/mfd/palmas.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+
+#define PALMAS_LPK_TIME_MASK		0x0c
+#define PALMAS_PWRON_DEBOUNCE_MASK	0x03
+#define PALMAS_PWR_KEY_Q_TIME_MS	20
+
+/**
+ * struct palmas_pwron - Palmas power on data
+ * @palmas:		pointer to palmas device
+ * @input_dev:		pointer to input device
+ * @input_work:		work for detecting release of key
+ * @irq:		irq that we are hooked on to
+ */
+struct palmas_pwron {
+	struct palmas *palmas;
+	struct input_dev *input_dev;
+	struct delayed_work input_work;
+	int irq;
+};
+
+/**
+ * struct palmas_pwron_config - configuration of palmas power on
+ * @long_press_time_val:	value for long press h/w shutdown event
+ * @pwron_debounce_val:		value for debounce of power button
+ */
+struct palmas_pwron_config {
+	u8 long_press_time_val;
+	u8 pwron_debounce_val;
+};
+
+/**
+ * palmas_power_button_work() - Detects the button release event
+ * @work:	work item to detect button release
+ */
+static void palmas_power_button_work(struct work_struct *work)
+{
+	struct palmas_pwron *pwron = container_of(work,
+						  struct palmas_pwron,
+						  input_work.work);
+	struct input_dev *input_dev = pwron->input_dev;
+	unsigned int reg;
+	int error;
+
+	error = palmas_read(pwron->palmas, PALMAS_INTERRUPT_BASE,
+			    PALMAS_INT1_LINE_STATE, &reg);
+	if (error) {
+		dev_err(input_dev->dev.parent,
+			"Cannot read palmas PWRON status: %d\n", error);
+	} else if (reg & BIT(1)) {
+		/* The button is released, report event. */
+		input_report_key(input_dev, KEY_POWER, 0);
+		input_sync(input_dev);
+	} else {
+		/* The button is still depressed, keep checking. */
+		schedule_delayed_work(&pwron->input_work,
+				msecs_to_jiffies(PALMAS_PWR_KEY_Q_TIME_MS));
+	}
+}
+
+/**
+ * pwron_irq() - button press isr
+ * @irq:		irq
+ * @palmas_pwron:	pwron struct
+ *
+ * Return: IRQ_HANDLED
+ */
+static irqreturn_t pwron_irq(int irq, void *palmas_pwron)
+{
+	struct palmas_pwron *pwron = palmas_pwron;
+	struct input_dev *input_dev = pwron->input_dev;
+
+	input_report_key(input_dev, KEY_POWER, 1);
+	pm_wakeup_event(input_dev->dev.parent, 0);
+	input_sync(input_dev);
+
+	mod_delayed_work(system_wq, &pwron->input_work,
+			 msecs_to_jiffies(PALMAS_PWR_KEY_Q_TIME_MS));
+
+	return IRQ_HANDLED;
+}
+
+/**
+ * palmas_pwron_params_ofinit() - device tree parameter parser
+ * @dev:	palmas button device
+ * @config:	configuration params that this fills up
+ */
+static void palmas_pwron_params_ofinit(struct device *dev,
+				       struct palmas_pwron_config *config)
+{
+	struct device_node *np;
+	u32 val;
+	int i, error;
+	u8 lpk_times[] = { 6, 8, 10, 12 };
+	int pwr_on_deb_ms[] = { 15, 100, 500, 1000 };
+
+	memset(config, 0, sizeof(*config));
+
+	/* Default config parameters */
+	config->long_press_time_val = ARRAY_SIZE(lpk_times) - 1;
+
+	np = dev->of_node;
+	if (!np)
+		return;
+
+	error = of_property_read_u32(np, "ti,palmas-long-press-seconds", &val);
+	if (!error) {
+		for (i = 0; i < ARRAY_SIZE(lpk_times); i++) {
+			if (val <= lpk_times[i]) {
+				config->long_press_time_val = i;
+				break;
+			}
+		}
+	}
+
+	error = of_property_read_u32(np,
+				     "ti,palmas-pwron-debounce-milli-seconds",
+				     &val);
+	if (!error) {
+		for (i = 0; i < ARRAY_SIZE(pwr_on_deb_ms); i++) {
+			if (val <= pwr_on_deb_ms[i]) {
+				config->pwron_debounce_val = i;
+				break;
+			}
+		}
+	}
+
+	dev_info(dev, "h/w controlled shutdown duration=%d seconds\n",
+		 lpk_times[config->long_press_time_val]);
+}
+
+/**
+ * palmas_pwron_probe() - probe
+ * @pdev:	platform device for the button
+ *
+ * Return: 0 for successful probe else appropriate error
+ */
+static int palmas_pwron_probe(struct platform_device *pdev)
+{
+	struct palmas *palmas = dev_get_drvdata(pdev->dev.parent);
+	struct device *dev = &pdev->dev;
+	struct input_dev *input_dev;
+	struct palmas_pwron *pwron;
+	struct palmas_pwron_config config;
+	int val;
+	int error;
+
+	palmas_pwron_params_ofinit(dev, &config);
+
+	pwron = kzalloc(sizeof(*pwron), GFP_KERNEL);
+	if (!pwron)
+		return -ENOMEM;
+
+	input_dev = input_allocate_device();
+	if (!input_dev) {
+		dev_err(dev, "Can't allocate power button\n");
+		error = -ENOMEM;
+		goto err_free_mem;
+	}
+
+	input_dev->name = "palmas_pwron";
+	input_dev->phys = "palmas_pwron/input0";
+	input_dev->dev.parent = dev;
+
+	input_set_capability(input_dev, EV_KEY, KEY_POWER);
+
+	/*
+	 * Setup default hardware shutdown option (long key press)
+	 * and debounce.
+	 */
+	val = config.long_press_time_val << __ffs(PALMAS_LPK_TIME_MASK);
+	val |= config.pwron_debounce_val << __ffs(PALMAS_PWRON_DEBOUNCE_MASK);
+	error = palmas_update_bits(palmas, PALMAS_PMU_CONTROL_BASE,
+				   PALMAS_LONG_PRESS_KEY,
+				   PALMAS_LPK_TIME_MASK |
+					PALMAS_PWRON_DEBOUNCE_MASK,
+				   val);
+	if (error) {
+		dev_err(dev, "LONG_PRESS_KEY_UPDATE failed: %d\n", error);
+		goto err_free_input;
+	}
+
+	pwron->palmas = palmas;
+	pwron->input_dev = input_dev;
+
+	INIT_DELAYED_WORK(&pwron->input_work, palmas_power_button_work);
+
+	pwron->irq = platform_get_irq(pdev, 0);
+	error = request_threaded_irq(pwron->irq, NULL, pwron_irq,
+				     IRQF_TRIGGER_HIGH | IRQF_TRIGGER_LOW,
+				     dev_name(dev), pwron);
+	if (error) {
+		dev_err(dev, "Can't get IRQ for pwron: %d\n", error);
+		goto err_free_input;
+	}
+
+	error = input_register_device(input_dev);
+	if (error) {
+		dev_err(dev, "Can't register power button: %d\n", error);
+		goto err_free_irq;
+	}
+
+	platform_set_drvdata(pdev, pwron);
+	device_init_wakeup(dev, true);
+
+	return 0;
+
+err_free_irq:
+	cancel_delayed_work_sync(&pwron->input_work);
+	free_irq(pwron->irq, pwron);
+err_free_input:
+	input_free_device(input_dev);
+err_free_mem:
+	kfree(pwron);
+	return error;
+}
+
+/**
+ * palmas_pwron_remove() - Cleanup on removal
+ * @pdev:	platform device for the button
+ *
+ * Return: 0
+ */
+static int palmas_pwron_remove(struct platform_device *pdev)
+{
+	struct palmas_pwron *pwron = platform_get_drvdata(pdev);
+
+	free_irq(pwron->irq, pwron);
+	cancel_delayed_work_sync(&pwron->input_work);
+
+	input_unregister_device(pwron->input_dev);
+	kfree(pwron);
+
+	return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+/**
+ * palmas_pwron_suspend() - suspend handler
+ * @dev:	power button device
+ *
+ * Cancel all pending work items for the power button, setup irq for wakeup
+ *
+ * Return: 0
+ */
+static int palmas_pwron_suspend(struct device *dev)
+{
+	struct platform_device *pdev = to_platform_device(dev);
+	struct palmas_pwron *pwron = platform_get_drvdata(pdev);
+
+	cancel_delayed_work_sync(&pwron->input_work);
+
+	if (device_may_wakeup(dev))
+		enable_irq_wake(pwron->irq);
+
+	return 0;
+}
+
+/**
+ * palmas_pwron_resume() - resume handler
+ * @dev:	power button device
+ *
+ * Just disable the wakeup capability of irq here.
+ *
+ * Return: 0
+ */
+static int palmas_pwron_resume(struct device *dev)
+{
+	struct platform_device *pdev = to_platform_device(dev);
+	struct palmas_pwron *pwron = platform_get_drvdata(pdev);
+
+	if (device_may_wakeup(dev))
+		disable_irq_wake(pwron->irq);
+
+	return 0;
+}
+#endif
+
+static SIMPLE_DEV_PM_OPS(palmas_pwron_pm,
+			 palmas_pwron_suspend, palmas_pwron_resume);
+
+#ifdef CONFIG_OF
+static struct of_device_id of_palmas_pwr_match[] = {
+	{ .compatible = "ti,palmas-pwrbutton" },
+	{ },
+};
+
+MODULE_DEVICE_TABLE(of, of_palmas_pwr_match);
+#endif
+
+static struct platform_driver palmas_pwron_driver = {
+	.probe	= palmas_pwron_probe,
+	.remove	= palmas_pwron_remove,
+	.driver	= {
+		.name	= "palmas_pwrbutton",
+		.owner	= THIS_MODULE,
+		.of_match_table = of_match_ptr(of_palmas_pwr_match),
+		.pm	= &palmas_pwron_pm,
+	},
+};
+module_platform_driver(palmas_pwron_driver);
+
+MODULE_ALIAS("platform:palmas-pwrbutton");
+MODULE_DESCRIPTION("Palmas Power Button");
+MODULE_LICENSE("GPL V2");
+MODULE_AUTHOR("Texas Instruments Inc.");

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

* [PATCH V2 2/2] Input: misc: introduce palmas-pwrbutton
  2014-09-10 21:13                 ` Dmitry Torokhov
@ 2014-09-11 12:01                   ` Nishanth Menon
  2014-09-12  6:44                     ` Dmitry Torokhov
  0 siblings, 1 reply; 24+ messages in thread
From: Nishanth Menon @ 2014-09-11 12:01 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Dimtry,

On 14:13-20140910, Dmitry Torokhov wrote:
> On Thu, Aug 21, 2014 at 02:01:43PM -0500, Nishanth Menon wrote:
> > On 08/21/2014 01:03 PM, Dmitry Torokhov wrote:
> > 
> > I believe I have taken care of other concerns on v2, but..Arrgh.. I
> > did not reply to this comment..
> > > BTW, I do not think you need to use of_node_get/put here, it's not going anywhere.
> > It has been mentioned as a good practice to ensure we use get_put in
> > to ensure reference count is appropriately maintained. So, I have'nt
> > changed that in v3.
> 
> You only need to maintain reference count if you pass the handle on.
> Otherwise you'd have to do get/put every time you dereference something.
> 
> Anyway, I did a few changes to the driver (no need to store current
> state, do not fre einput device after unregister, etc.), could you
> please tell me if the version below still works for you?
[...]
Thanks for taking the time to do all the changes - they are awesome and
the resultant driver does work.

-- 
Regards,
Nishanth Menon

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

* [PATCH V2 2/2] Input: misc: introduce palmas-pwrbutton
  2014-09-11 12:01                   ` Nishanth Menon
@ 2014-09-12  6:44                     ` Dmitry Torokhov
  0 siblings, 0 replies; 24+ messages in thread
From: Dmitry Torokhov @ 2014-09-12  6:44 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Sep 11, 2014 at 07:01:19AM -0500, Nishanth Menon wrote:
> Hi Dimtry,
> 
> On 14:13-20140910, Dmitry Torokhov wrote:
> > On Thu, Aug 21, 2014 at 02:01:43PM -0500, Nishanth Menon wrote:
> > > On 08/21/2014 01:03 PM, Dmitry Torokhov wrote:
> > > 
> > > I believe I have taken care of other concerns on v2, but..Arrgh.. I
> > > did not reply to this comment..
> > > > BTW, I do not think you need to use of_node_get/put here, it's not going anywhere.
> > > It has been mentioned as a good practice to ensure we use get_put in
> > > to ensure reference count is appropriately maintained. So, I have'nt
> > > changed that in v3.
> > 
> > You only need to maintain reference count if you pass the handle on.
> > Otherwise you'd have to do get/put every time you dereference something.
> > 
> > Anyway, I did a few changes to the driver (no need to store current
> > state, do not fre einput device after unregister, etc.), could you
> > please tell me if the version below still works for you?
> [...]
> Thanks for taking the time to do all the changes - they are awesome and
> the resultant driver does work.

Thank you for [re]testing. I queued the driver for the next merge
window.

-- 
Dmitry

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

end of thread, other threads:[~2014-09-12  6:44 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-08-18 20:13 [PATCH 0/2] Input: palmas: add support for palmas power button Nishanth Menon
2014-08-18 20:13 ` [PATCH 1/2] doc: dt/bindings: input: introduce palmas power button description Nishanth Menon
2014-08-19  5:28   ` Dmitry Torokhov
2014-08-19 10:23     ` Nishanth Menon
2014-08-18 20:13 ` [PATCH 2/2] Input: misc: introduce palmas-pwrbutton Nishanth Menon
2014-08-19  5:23   ` Dmitry Torokhov
2014-08-19 10:17     ` Nishanth Menon
2014-08-21 16:02 ` [PATCH V2 0/2] Input: palmas: add support for palmas power button Nishanth Menon
2014-08-21 16:02   ` [PATCH V2 1/2] doc: dt/bindings: input: introduce palmas power button description Nishanth Menon
2014-08-21 16:02   ` [PATCH V2 2/2] Input: misc: introduce palmas-pwrbutton Nishanth Menon
2014-08-21 16:59     ` Murphy, Dan
2014-08-21 17:12       ` Nishanth Menon
2014-08-21 17:19       ` Nishanth Menon
2014-08-21 17:32         ` Murphy, Dan
2014-08-21 17:37           ` Nishanth Menon
2014-08-21 17:46             ` Murphy, Dan
2014-08-21 18:03             ` Dmitry Torokhov
2014-08-21 19:01               ` Nishanth Menon
2014-09-10 21:13                 ` Dmitry Torokhov
2014-09-11 12:01                   ` Nishanth Menon
2014-09-12  6:44                     ` Dmitry Torokhov
2014-08-21 17:05     ` Dmitry Torokhov
2014-08-21 17:09       ` Nishanth Menon
2014-08-21 18:52 ` [PATCH V3 " Nishanth Menon

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