[PATCH 00/13] ltc2952 modernization and new functionality

[PATCH 00/13] ltc2952 modernization and new functionality

[Frans Klaver]

Hi there,

Apparently I missed the fact that version 2 [1] of the ltc2952 driver had already
been queued, so here's a series that implements most of the changes that I
already did in v3..5 [2] of the driver.

This series implements devm_* usage, reduction of globals, some cleanup and
finally makes the input trigger optional.

>From the changes to the previous submissions I did not include renaming kill to
poweroff.  The name poweroff is already used a lot. Besides that, the control
is referred to as kill in the ltc2952 datasheet.

Thanks,
Frans

[1] http://comments.gmane.org/gmane.linux.power-management.general/48312
[2] https://lkml.org/lkml/2014/10/13/104

Frans Klaver (13):
  power: reset: ltc2952: prefer devm_kzalloc over kzalloc
  power: reset: ltc2952: prefer devm_request_irq over request_irq
  power: reset: ltc2952: unroll gpio_desc array
  power: reset: ltc2952: prefer devm_gpiod_get over gpiod_get
  power: reset: ltc2952: reduce dependency on global variables
  power: reset: ltc2952: remove global variable poweroff_panic
  power: reset: ltc2952: drop empty suspend/resume functions
  power: reset: ltc2952: cleanup control flow in poweroff_handler
  power: reset: ltc2952: remove C++ style function pointers
  power: reset: ltc2952: disable timers in _remove
  power: reset: ltc2952: check trigger value before starting timer
  power: reset: ltc2952: make trigger input optional
  power: reset: ltc2952: document optional trigger behavior

 .../bindings/power/reset/ltc2952-poweroff.txt      |  13 +-
 drivers/power/reset/ltc2952-poweroff.c             | 290 +++++++++------------
 2 files changed, 129 insertions(+), 174 deletions(-)

-- 
2.1.0

[PATCH 01/13] power: reset: ltc2952: prefer devm_kzalloc over kzalloc

[Frans Klaver]

Make use of the fact that the allocated resources can be automatically
deallocated. This reduces cleanup code and chance of leaks.

Signed-off-by: Frans Klaver <frans.klaver@xsens.com>
---
 drivers/power/reset/ltc2952-poweroff.c | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/power/reset/ltc2952-poweroff.c b/drivers/power/reset/ltc2952-poweroff.c
index 116a1ce..951a90d 100644
--- a/drivers/power/reset/ltc2952-poweroff.c
+++ b/drivers/power/reset/ltc2952-poweroff.c
@@ -292,7 +292,8 @@ static int ltc2952_poweroff_probe(struct platform_device *pdev)
 		return -EBUSY;
 	}
 
-	ltc2952_data = kzalloc(sizeof(*ltc2952_data), GFP_KERNEL);
+	ltc2952_data = devm_kzalloc(&pdev->dev, sizeof(*ltc2952_data),
+				    GFP_KERNEL);
 	if (!ltc2952_data)
 		return -ENOMEM;
 
@@ -300,17 +301,13 @@ static int ltc2952_poweroff_probe(struct platform_device *pdev)
 
 	ret = ltc2952_poweroff_init(pdev);
 	if (ret)
-		goto err;
+		return ret;
 
 	pm_power_off = &ltc2952_poweroff_kill;
 
 	dev_info(&pdev->dev, "probe successful\n");
 
 	return 0;
-
-err:
-	kfree(ltc2952_data);
-	return ret;
 }
 
 static int ltc2952_poweroff_remove(struct platform_device *pdev)
@@ -324,8 +321,6 @@ static int ltc2952_poweroff_remove(struct platform_device *pdev)
 
 		for (i = 0; i < ARRAY_SIZE(ltc2952_data->gpio); i++)
 			gpiod_put(ltc2952_data->gpio[i]);
-
-		kfree(ltc2952_data);
 	}
 
 	return 0;
-- 
2.1.0

[PATCH 02/13] power: reset: ltc2952: prefer devm_request_irq over request_irq

[Frans Klaver]

Make use of the fact that we allocated resources can be automatically
deallocated. This reduces cleanup code and chance of errors. It also
removes the need for the virq member of the ltc2952_poweroff_data
struct.

Signed-off-by: Frans Klaver <frans.klaver@xsens.com>
---
 drivers/power/reset/ltc2952-poweroff.c | 19 ++++++-------------
 1 file changed, 6 insertions(+), 13 deletions(-)

diff --git a/drivers/power/reset/ltc2952-poweroff.c b/drivers/power/reset/ltc2952-poweroff.c
index 951a90d..2c11316 100644
--- a/drivers/power/reset/ltc2952-poweroff.c
+++ b/drivers/power/reset/ltc2952-poweroff.c
@@ -72,8 +72,6 @@ struct ltc2952_poweroff_data {
 
 	struct device *dev;
 
-	unsigned int virq;
-
 	/**
 	 * 0: trigger
 	 * 1: watchdog
@@ -260,13 +258,11 @@ static int ltc2952_poweroff_init(struct platform_device *pdev)
 		goto err_io;
 	}
 
-	ltc2952_data->virq = virq;
-	ret = request_irq(virq,
-		ltc2952_poweroff_handler,
-		(IRQF_TRIGGER_FALLING | IRQF_TRIGGER_RISING),
-		"ltc2952-poweroff",
-		ltc2952_data
-	);
+	ret = devm_request_irq(&pdev->dev, virq,
+			       ltc2952_poweroff_handler,
+			       (IRQF_TRIGGER_FALLING | IRQF_TRIGGER_RISING),
+			       "ltc2952-poweroff",
+			       ltc2952_data);
 
 	if (ret) {
 		dev_err(&pdev->dev, "cannot configure an interrupt handler\n");
@@ -316,12 +312,9 @@ static int ltc2952_poweroff_remove(struct platform_device *pdev)
 
 	pm_power_off = NULL;
 
-	if (ltc2952_data) {
-		free_irq(ltc2952_data->virq, ltc2952_data);
-
+	if (ltc2952_data)
 		for (i = 0; i < ARRAY_SIZE(ltc2952_data->gpio); i++)
 			gpiod_put(ltc2952_data->gpio[i]);
-	}
 
 	return 0;
 }
-- 
2.1.0

[PATCH 03/13] power: reset: ltc2952: unroll gpio_desc array

[Frans Klaver]

The three gpio's used by this driver are stored in an array of pointers.
This doesn't add much besides cleanups in a loop. In fact, it makes most
of the usage sites harder to read. Unroll the loop, and live with the
fact that cleanups become slightly larger.

Signed-off-by: Frans Klaver <frans.klaver@xsens.com>
---
 drivers/power/reset/ltc2952-poweroff.c | 86 +++++++++++++++-------------------
 1 file changed, 38 insertions(+), 48 deletions(-)

diff --git a/drivers/power/reset/ltc2952-poweroff.c b/drivers/power/reset/ltc2952-poweroff.c
index 2c11316..fc583e0 100644
--- a/drivers/power/reset/ltc2952-poweroff.c
+++ b/drivers/power/reset/ltc2952-poweroff.c
@@ -72,21 +72,14 @@ struct ltc2952_poweroff_data {
 
 	struct device *dev;
 
-	/**
-	 * 0: trigger
-	 * 1: watchdog
-	 * 2: kill
-	 */
-	struct gpio_desc *gpio[3];
+	struct gpio_desc *gpio_trigger;
+	struct gpio_desc *gpio_watchdog;
+	struct gpio_desc *gpio_kill;
 };
 
 static int ltc2952_poweroff_panic;
 static struct ltc2952_poweroff_data *ltc2952_data;
 
-#define POWERPATH_IO_TRIGGER	0
-#define POWERPATH_IO_WATCHDOG	1
-#define POWERPATH_IO_KILL	2
-
 /**
  * ltc2952_poweroff_timer_wde - Timer callback
  * Toggles the watchdog reset signal each wde_interval
@@ -105,8 +98,8 @@ static enum hrtimer_restart ltc2952_poweroff_timer_wde(struct hrtimer *timer)
 	if (ltc2952_poweroff_panic)
 		return HRTIMER_NORESTART;
 
-	state = gpiod_get_value(ltc2952_data->gpio[POWERPATH_IO_WATCHDOG]);
-	gpiod_set_value(ltc2952_data->gpio[POWERPATH_IO_WATCHDOG], !state);
+	state = gpiod_get_value(ltc2952_data->gpio_watchdog);
+	gpiod_set_value(ltc2952_data->gpio_watchdog, !state);
 
 	now = hrtimer_cb_get_time(timer);
 	overruns = hrtimer_forward(timer, now, ltc2952_data->wde_interval);
@@ -120,7 +113,7 @@ static enum hrtimer_restart ltc2952_poweroff_timer_trigger(
 	int ret;
 
 	ret = hrtimer_start(&ltc2952_data->timer_wde,
-		ltc2952_data->wde_interval, HRTIMER_MODE_REL);
+			    ltc2952_data->wde_interval, HRTIMER_MODE_REL);
 
 	if (ret) {
 		dev_err(ltc2952_data->dev, "unable to start the timer\n");
@@ -180,7 +173,7 @@ irq_ok:
 
 static void ltc2952_poweroff_kill(void)
 {
-	gpiod_set_value(ltc2952_data->gpio[POWERPATH_IO_KILL], 1);
+	gpiod_set_value(ltc2952_data->gpio_kill, 1);
 }
 
 static int ltc2952_poweroff_suspend(struct platform_device *pdev,
@@ -196,11 +189,6 @@ static int ltc2952_poweroff_resume(struct platform_device *pdev)
 
 static void ltc2952_poweroff_default(struct ltc2952_poweroff_data *data)
 {
-	unsigned int i;
-
-	for (i = 0; i < ARRAY_SIZE(data->gpio); i++)
-		data->gpio[i] = NULL;
-
 	data->wde_interval = ktime_set(0, 300L*1E6L);
 	data->trigger_delay = ktime_set(2, 500L*1E6L);
 
@@ -214,45 +202,46 @@ static void ltc2952_poweroff_default(struct ltc2952_poweroff_data *data)
 static int ltc2952_poweroff_init(struct platform_device *pdev)
 {
 	int ret, virq;
-	unsigned int i;
 	struct ltc2952_poweroff_data *data;
 
-	static char *name[] = {
-		"trigger",
-		"watchdog",
-		"kill",
-		NULL
-	};
-
 	data = ltc2952_data;
 	ltc2952_poweroff_default(ltc2952_data);
 
-	for (i = 0; i < ARRAY_SIZE(ltc2952_data->gpio); i++) {
-		ltc2952_data->gpio[i] = gpiod_get(&pdev->dev, name[i]);
+	ltc2952_data->gpio_watchdog = gpiod_get(&pdev->dev, "watchdog");
+	if (IS_ERR(ltc2952_data->gpio_watchdog)) {
+		ret = PTR_ERR(ltc2952_data->gpio_watchdog);
+		dev_err(&pdev->dev, "unable to claim gpio \"watchdog\"\n");
+		return ret;
+	}
 
-		if (IS_ERR(ltc2952_data->gpio[i])) {
-			ret = PTR_ERR(ltc2952_data->gpio[i]);
-			dev_err(&pdev->dev,
-				"unable to claim the following gpio: %s\n",
-				name[i]);
-			goto err_io;
-		}
+	ltc2952_data->gpio_kill = gpiod_get(&pdev->dev, "kill");
+	if (IS_ERR(ltc2952_data->gpio_kill)) {
+		ret = PTR_ERR(ltc2952_data->gpio_kill);
+		dev_err(&pdev->dev, "unable to claim gpio \"kill\"\n");
+		goto err_kill;
+	}
+
+	ltc2952_data->gpio_trigger = gpiod_get(&pdev->dev, "trigger");
+	if (IS_ERR(ltc2952_data->gpio_trigger)) {
+		ret = PTR_ERR(ltc2952_data->gpio_trigger);
+		dev_err(&pdev->dev, "unable to claim gpio \"trigger\"\n");
+		goto err_trigger;
 	}
 
 	ret = gpiod_direction_output(
-		ltc2952_data->gpio[POWERPATH_IO_WATCHDOG], 0);
+		ltc2952_data->gpio_watchdog, 0);
 	if (ret) {
 		dev_err(&pdev->dev, "unable to use watchdog-gpio as output\n");
 		goto err_io;
 	}
 
-	ret = gpiod_direction_output(ltc2952_data->gpio[POWERPATH_IO_KILL], 0);
+	ret = gpiod_direction_output(ltc2952_data->gpio_kill, 0);
 	if (ret) {
 		dev_err(&pdev->dev, "unable to use kill-gpio as output\n");
 		goto err_io;
 	}
 
-	virq = gpiod_to_irq(ltc2952_data->gpio[POWERPATH_IO_TRIGGER]);
+	virq = gpiod_to_irq(ltc2952_data->gpio_trigger);
 	if (virq < 0) {
 		dev_err(&pdev->dev, "cannot map GPIO as interrupt");
 		goto err_io;
@@ -272,9 +261,11 @@ static int ltc2952_poweroff_init(struct platform_device *pdev)
 	return 0;
 
 err_io:
-	for (i = 0; i < ARRAY_SIZE(ltc2952_data->gpio); i++)
-		if (ltc2952_data->gpio[i])
-			gpiod_put(ltc2952_data->gpio[i]);
+	gpiod_put(ltc2952_data->gpio_trigger);
+err_trigger:
+	gpiod_put(ltc2952_data->gpio_kill);
+err_kill:
+	gpiod_put(ltc2952_data->gpio_watchdog);
 
 	return ret;
 }
@@ -308,14 +299,13 @@ static int ltc2952_poweroff_probe(struct platform_device *pdev)
 
 static int ltc2952_poweroff_remove(struct platform_device *pdev)
 {
-	unsigned int i;
-
 	pm_power_off = NULL;
 
-	if (ltc2952_data)
-		for (i = 0; i < ARRAY_SIZE(ltc2952_data->gpio); i++)
-			gpiod_put(ltc2952_data->gpio[i]);
-
+	if (ltc2952_data) {
+		gpiod_put(ltc2952_data->gpio_trigger);
+		gpiod_put(ltc2952_data->gpio_watchdog);
+		gpiod_put(ltc2952_data->gpio_kill);
+	}
 	return 0;
 }
 
-- 
2.1.0

[PATCH 04/13] power: reset: ltc2952: prefer devm_gpiod_get over gpiod_get

[Frans Klaver]

This reduces cleanup code and chance of errors.

Signed-off-by: Frans Klaver <frans.klaver@xsens.com>
---
 drivers/power/reset/ltc2952-poweroff.c | 44 ++++++++--------------------------
 1 file changed, 10 insertions(+), 34 deletions(-)

diff --git a/drivers/power/reset/ltc2952-poweroff.c b/drivers/power/reset/ltc2952-poweroff.c
index fc583e0..1e96b5b 100644
--- a/drivers/power/reset/ltc2952-poweroff.c
+++ b/drivers/power/reset/ltc2952-poweroff.c
@@ -207,44 +207,34 @@ static int ltc2952_poweroff_init(struct platform_device *pdev)
 	data = ltc2952_data;
 	ltc2952_poweroff_default(ltc2952_data);
 
-	ltc2952_data->gpio_watchdog = gpiod_get(&pdev->dev, "watchdog");
+	ltc2952_data->gpio_watchdog = devm_gpiod_get(&pdev->dev, "watchdog",
+						     GPIOD_OUT_LOW);
 	if (IS_ERR(ltc2952_data->gpio_watchdog)) {
 		ret = PTR_ERR(ltc2952_data->gpio_watchdog);
 		dev_err(&pdev->dev, "unable to claim gpio \"watchdog\"\n");
 		return ret;
 	}
 
-	ltc2952_data->gpio_kill = gpiod_get(&pdev->dev, "kill");
+	ltc2952_data->gpio_kill = devm_gpiod_get(&pdev->dev, "kill",
+						 GPIOD_OUT_LOW);
 	if (IS_ERR(ltc2952_data->gpio_kill)) {
 		ret = PTR_ERR(ltc2952_data->gpio_kill);
 		dev_err(&pdev->dev, "unable to claim gpio \"kill\"\n");
-		goto err_kill;
+		return ret;
 	}
 
-	ltc2952_data->gpio_trigger = gpiod_get(&pdev->dev, "trigger");
+	ltc2952_data->gpio_trigger = devm_gpiod_get(&pdev->dev, "trigger",
+						    GPIOD_IN);
 	if (IS_ERR(ltc2952_data->gpio_trigger)) {
 		ret = PTR_ERR(ltc2952_data->gpio_trigger);
 		dev_err(&pdev->dev, "unable to claim gpio \"trigger\"\n");
-		goto err_trigger;
-	}
-
-	ret = gpiod_direction_output(
-		ltc2952_data->gpio_watchdog, 0);
-	if (ret) {
-		dev_err(&pdev->dev, "unable to use watchdog-gpio as output\n");
-		goto err_io;
-	}
-
-	ret = gpiod_direction_output(ltc2952_data->gpio_kill, 0);
-	if (ret) {
-		dev_err(&pdev->dev, "unable to use kill-gpio as output\n");
-		goto err_io;
+		return ret;
 	}
 
 	virq = gpiod_to_irq(ltc2952_data->gpio_trigger);
 	if (virq < 0) {
 		dev_err(&pdev->dev, "cannot map GPIO as interrupt");
-		goto err_io;
+		return ret;
 	}
 
 	ret = devm_request_irq(&pdev->dev, virq,
@@ -255,19 +245,10 @@ static int ltc2952_poweroff_init(struct platform_device *pdev)
 
 	if (ret) {
 		dev_err(&pdev->dev, "cannot configure an interrupt handler\n");
-		goto err_io;
+		return ret;
 	}
 
 	return 0;
-
-err_io:
-	gpiod_put(ltc2952_data->gpio_trigger);
-err_trigger:
-	gpiod_put(ltc2952_data->gpio_kill);
-err_kill:
-	gpiod_put(ltc2952_data->gpio_watchdog);
-
-	return ret;
 }
 
 static int ltc2952_poweroff_probe(struct platform_device *pdev)
@@ -301,11 +282,6 @@ static int ltc2952_poweroff_remove(struct platform_device *pdev)
 {
 	pm_power_off = NULL;
 
-	if (ltc2952_data) {
-		gpiod_put(ltc2952_data->gpio_trigger);
-		gpiod_put(ltc2952_data->gpio_watchdog);
-		gpiod_put(ltc2952_data->gpio_kill);
-	}
 	return 0;
 }
 
-- 
2.1.0

[PATCH 05/13] power: reset: ltc2952: reduce dependency on global variables

[Frans Klaver]

Documentation/CodingStyle ch.4 mentions in a side node that global
variables should only be used if you really need them. Reduce the use of
the global instance of ltc2952_poweroff so we may eventually remove it
entirely.

While at it, rename ltc2952_poweroff_data to ltc2952_poweroff, just to
save that little bit of typing.

Signed-off-by: Frans Klaver <frans.klaver@xsens.com>
---
 drivers/power/reset/ltc2952-poweroff.c | 75 +++++++++++++++++++---------------
 1 file changed, 41 insertions(+), 34 deletions(-)

diff --git a/drivers/power/reset/ltc2952-poweroff.c b/drivers/power/reset/ltc2952-poweroff.c
index 1e96b5b..9d493b5 100644
--- a/drivers/power/reset/ltc2952-poweroff.c
+++ b/drivers/power/reset/ltc2952-poweroff.c
@@ -63,7 +63,7 @@
 #include <linux/gpio/consumer.h>
 #include <linux/reboot.h>
 
-struct ltc2952_poweroff_data {
+struct ltc2952_poweroff {
 	struct hrtimer timer_trigger;
 	struct hrtimer timer_wde;
 
@@ -77,8 +77,14 @@ struct ltc2952_poweroff_data {
 	struct gpio_desc *gpio_kill;
 };
 
+#define to_ltc2952(p, m) container_of(p, struct ltc2952_poweroff, m)
+
+/*
+ * This global variable is only needed for pm_power_off. We should
+ * remove it entirely once we don't need the global state anymore.
+ */
+static struct ltc2952_poweroff *ltc2952_data;
 static int ltc2952_poweroff_panic;
-static struct ltc2952_poweroff_data *ltc2952_data;
 
 /**
  * ltc2952_poweroff_timer_wde - Timer callback
@@ -94,29 +100,29 @@ static enum hrtimer_restart ltc2952_poweroff_timer_wde(struct hrtimer *timer)
 	ktime_t now;
 	int state;
 	unsigned long overruns;
+	struct ltc2952_poweroff *data = to_ltc2952(timer, timer_wde);
 
 	if (ltc2952_poweroff_panic)
 		return HRTIMER_NORESTART;
 
-	state = gpiod_get_value(ltc2952_data->gpio_watchdog);
-	gpiod_set_value(ltc2952_data->gpio_watchdog, !state);
+	state = gpiod_get_value(data->gpio_watchdog);
+	gpiod_set_value(data->gpio_watchdog, !state);
 
 	now = hrtimer_cb_get_time(timer);
-	overruns = hrtimer_forward(timer, now, ltc2952_data->wde_interval);
+	overruns = hrtimer_forward(timer, now, data->wde_interval);
 
 	return HRTIMER_RESTART;
 }
 
-static enum hrtimer_restart ltc2952_poweroff_timer_trigger(
-	struct hrtimer *timer)
+static enum hrtimer_restart
+ltc2952_poweroff_timer_trigger(struct hrtimer *timer)
 {
-	int ret;
-
-	ret = hrtimer_start(&ltc2952_data->timer_wde,
-			    ltc2952_data->wde_interval, HRTIMER_MODE_REL);
+	struct ltc2952_poweroff *data = to_ltc2952(timer, timer_trigger);
+	int ret = hrtimer_start(&data->timer_wde,
+				data->wde_interval, HRTIMER_MODE_REL);
 
 	if (ret) {
-		dev_err(ltc2952_data->dev, "unable to start the timer\n");
+		dev_err(data->dev, "unable to start the timer\n");
 		/*
 		 * The device will not toggle the watchdog reset,
 		 * thus shut down is only safe if the PowerPath controller
@@ -127,7 +133,7 @@ static enum hrtimer_restart ltc2952_poweroff_timer_trigger(
 		 */
 	}
 
-	dev_info(ltc2952_data->dev, "executing shutdown\n");
+	dev_info(data->dev, "executing shutdown\n");
 
 	orderly_poweroff(true);
 
@@ -146,7 +152,7 @@ static enum hrtimer_restart ltc2952_poweroff_timer_trigger(
 static irqreturn_t ltc2952_poweroff_handler(int irq, void *dev_id)
 {
 	int ret;
-	struct ltc2952_poweroff_data *data = dev_id;
+	struct ltc2952_poweroff *data = dev_id;
 
 	if (ltc2952_poweroff_panic)
 		goto irq_ok;
@@ -187,7 +193,7 @@ static int ltc2952_poweroff_resume(struct platform_device *pdev)
 	return -ENOSYS;
 }
 
-static void ltc2952_poweroff_default(struct ltc2952_poweroff_data *data)
+static void ltc2952_poweroff_default(struct ltc2952_poweroff *data)
 {
 	data->wde_interval = ktime_set(0, 300L*1E6L);
 	data->trigger_delay = ktime_set(2, 500L*1E6L);
@@ -202,36 +208,34 @@ static void ltc2952_poweroff_default(struct ltc2952_poweroff_data *data)
 static int ltc2952_poweroff_init(struct platform_device *pdev)
 {
 	int ret, virq;
-	struct ltc2952_poweroff_data *data;
+	struct ltc2952_poweroff *data = platform_get_drvdata(pdev);
 
-	data = ltc2952_data;
-	ltc2952_poweroff_default(ltc2952_data);
+	ltc2952_poweroff_default(data);
 
-	ltc2952_data->gpio_watchdog = devm_gpiod_get(&pdev->dev, "watchdog",
-						     GPIOD_OUT_LOW);
-	if (IS_ERR(ltc2952_data->gpio_watchdog)) {
-		ret = PTR_ERR(ltc2952_data->gpio_watchdog);
+	data->gpio_watchdog = devm_gpiod_get(&pdev->dev, "watchdog",
+					     GPIOD_OUT_LOW);
+	if (IS_ERR(data->gpio_watchdog)) {
+		ret = PTR_ERR(data->gpio_watchdog);
 		dev_err(&pdev->dev, "unable to claim gpio \"watchdog\"\n");
 		return ret;
 	}
 
-	ltc2952_data->gpio_kill = devm_gpiod_get(&pdev->dev, "kill",
-						 GPIOD_OUT_LOW);
-	if (IS_ERR(ltc2952_data->gpio_kill)) {
-		ret = PTR_ERR(ltc2952_data->gpio_kill);
+	data->gpio_kill = devm_gpiod_get(&pdev->dev, "kill", GPIOD_OUT_LOW);
+	if (IS_ERR(data->gpio_kill)) {
+		ret = PTR_ERR(data->gpio_kill);
 		dev_err(&pdev->dev, "unable to claim gpio \"kill\"\n");
 		return ret;
 	}
 
-	ltc2952_data->gpio_trigger = devm_gpiod_get(&pdev->dev, "trigger",
-						    GPIOD_IN);
+	data->gpio_trigger = devm_gpiod_get(&pdev->dev, "trigger",
+					    GPIOD_IN);
 	if (IS_ERR(ltc2952_data->gpio_trigger)) {
 		ret = PTR_ERR(ltc2952_data->gpio_trigger);
 		dev_err(&pdev->dev, "unable to claim gpio \"trigger\"\n");
 		return ret;
 	}
 
-	virq = gpiod_to_irq(ltc2952_data->gpio_trigger);
+	virq = gpiod_to_irq(data->gpio_trigger);
 	if (virq < 0) {
 		dev_err(&pdev->dev, "cannot map GPIO as interrupt");
 		return ret;
@@ -241,7 +245,7 @@ static int ltc2952_poweroff_init(struct platform_device *pdev)
 			       ltc2952_poweroff_handler,
 			       (IRQF_TRIGGER_FALLING | IRQF_TRIGGER_RISING),
 			       "ltc2952-poweroff",
-			       ltc2952_data);
+			       data);
 
 	if (ret) {
 		dev_err(&pdev->dev, "cannot configure an interrupt handler\n");
@@ -254,23 +258,26 @@ static int ltc2952_poweroff_init(struct platform_device *pdev)
 static int ltc2952_poweroff_probe(struct platform_device *pdev)
 {
 	int ret;
+	struct ltc2952_poweroff *data;
 
 	if (pm_power_off) {
 		dev_err(&pdev->dev, "pm_power_off already registered");
 		return -EBUSY;
 	}
 
-	ltc2952_data = devm_kzalloc(&pdev->dev, sizeof(*ltc2952_data),
-				    GFP_KERNEL);
-	if (!ltc2952_data)
+	data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
+	if (!data)
 		return -ENOMEM;
 
-	ltc2952_data->dev = &pdev->dev;
+	data->dev = &pdev->dev;
+	platform_set_drvdata(pdev, data);
 
 	ret = ltc2952_poweroff_init(pdev);
 	if (ret)
 		return ret;
 
+	/* TODO: remove ltc2952_data */
+	ltc2952_data = data;
 	pm_power_off = &ltc2952_poweroff_kill;
 
 	dev_info(&pdev->dev, "probe successful\n");
-- 
2.1.0

[PATCH 06/13] power: reset: ltc2952: remove global variable poweroff_panic

[Frans Klaver]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="UTF-8", Size: 4119 bytes --]

As per Documentation/CodingStyle ch.4, we should keep global variables
to a mininum. Move the panic state into the driver data, regardless of
whether panic is a system state or not.

This removes the need for the custom _init and _exit functions, so
replace them with a call to the module_platform_driver() macro.

Signed-off-by: Frans Klaver <frans.klaver@xsens.com>
---
 drivers/power/reset/ltc2952-poweroff.c | 57 +++++++++++++---------------------
 1 file changed, 22 insertions(+), 35 deletions(-)

diff --git a/drivers/power/reset/ltc2952-poweroff.c b/drivers/power/reset/ltc2952-poweroff.c
index 9d493b5..bc8d228 100644
--- a/drivers/power/reset/ltc2952-poweroff.c
+++ b/drivers/power/reset/ltc2952-poweroff.c
@@ -75,6 +75,9 @@ struct ltc2952_poweroff {
 	struct gpio_desc *gpio_trigger;
 	struct gpio_desc *gpio_watchdog;
 	struct gpio_desc *gpio_kill;
+
+	bool kernel_panic;
+	struct notifier_block panic_notifier;
 };
 
 #define to_ltc2952(p, m) container_of(p, struct ltc2952_poweroff, m)
@@ -84,7 +87,6 @@ struct ltc2952_poweroff {
  * remove it entirely once we don't need the global state anymore.
  */
 static struct ltc2952_poweroff *ltc2952_data;
-static int ltc2952_poweroff_panic;
 
 /**
  * ltc2952_poweroff_timer_wde - Timer callback
@@ -102,7 +104,7 @@ static enum hrtimer_restart ltc2952_poweroff_timer_wde(struct hrtimer *timer)
 	unsigned long overruns;
 	struct ltc2952_poweroff *data = to_ltc2952(timer, timer_wde);
 
-	if (ltc2952_poweroff_panic)
+	if (data->kernel_panic)
 		return HRTIMER_NORESTART;
 
 	state = gpiod_get_value(data->gpio_watchdog);
@@ -154,7 +156,7 @@ static irqreturn_t ltc2952_poweroff_handler(int irq, void *dev_id)
 	int ret;
 	struct ltc2952_poweroff *data = dev_id;
 
-	if (ltc2952_poweroff_panic)
+	if (data->kernel_panic)
 		goto irq_ok;
 
 	if (hrtimer_active(&data->timer_wde)) {
@@ -255,6 +257,15 @@ static int ltc2952_poweroff_init(struct platform_device *pdev)
 	return 0;
 }
 
+static int ltc2952_poweroff_notify_panic(struct notifier_block *nb,
+					 unsigned long code, void *unused)
+{
+	struct ltc2952_poweroff *data = to_ltc2952(nb, panic_notifier);
+
+	data->kernel_panic = true;
+	return NOTIFY_DONE;
+}
+
 static int ltc2952_poweroff_probe(struct platform_device *pdev)
 {
 	int ret;
@@ -280,6 +291,9 @@ static int ltc2952_poweroff_probe(struct platform_device *pdev)
 	ltc2952_data = data;
 	pm_power_off = &ltc2952_poweroff_kill;
 
+	data->panic_notifier.notifier_call = ltc2952_poweroff_notify_panic;
+	atomic_notifier_chain_register(&panic_notifier_list,
+				       &data->panic_notifier);
 	dev_info(&pdev->dev, "probe successful\n");
 
 	return 0;
@@ -287,8 +301,11 @@ static int ltc2952_poweroff_probe(struct platform_device *pdev)
 
 static int ltc2952_poweroff_remove(struct platform_device *pdev)
 {
-	pm_power_off = NULL;
+	struct ltc2952_poweroff *data = platform_get_drvdata(pdev);
 
+	pm_power_off = NULL;
+	atomic_notifier_chain_unregister(&panic_notifier_list,
+					 &data->panic_notifier);
 	return 0;
 }
 
@@ -310,37 +327,7 @@ static struct platform_driver ltc2952_poweroff_driver = {
 	.resume = ltc2952_poweroff_resume,
 };
 
-static int ltc2952_poweroff_notify_panic(struct notifier_block *nb,
-	unsigned long code, void *unused)
-{
-	ltc2952_poweroff_panic = 1;
-	return NOTIFY_DONE;
-}
-
-static struct notifier_block ltc2952_poweroff_panic_nb = {
-	.notifier_call = ltc2952_poweroff_notify_panic,
-};
-
-static int __init ltc2952_poweroff_platform_init(void)
-{
-	ltc2952_poweroff_panic = 0;
-
-	atomic_notifier_chain_register(&panic_notifier_list,
-		&ltc2952_poweroff_panic_nb);
-
-	return platform_driver_register(&ltc2952_poweroff_driver);
-}
-
-static void __exit ltc2952_poweroff_platform_exit(void)
-{
-	atomic_notifier_chain_unregister(&panic_notifier_list,
-		&ltc2952_poweroff_panic_nb);
-
-	platform_driver_unregister(&ltc2952_poweroff_driver);
-}
-
-module_init(ltc2952_poweroff_platform_init);
-module_exit(ltc2952_poweroff_platform_exit);
+module_platform_driver(ltc2952_poweroff_driver);
 
 MODULE_AUTHOR("René Moll <rene.moll@xsens.com>");
 MODULE_DESCRIPTION("LTC PowerPath power-off driver");
-- 
2.1.0

[PATCH 07/13] power: reset: ltc2952: drop empty suspend/resume functions

[Frans Klaver]

Documentation/SubmittingDrivers suggests these be implemented even when
they do nothing. On the other hand, the platform code calls these
functions 'legacy'. Suspend and resume operations should go into a
pm_ops structure, pointed at by the driver's pm field. This approach
would lead to a lot of boiler plate, while achieving nothing. Drop the
functions instead.

Signed-off-by: Frans Klaver <frans.klaver@xsens.com>
---
 drivers/power/reset/ltc2952-poweroff.c | 13 -------------
 1 file changed, 13 deletions(-)

diff --git a/drivers/power/reset/ltc2952-poweroff.c b/drivers/power/reset/ltc2952-poweroff.c
index bc8d228..af3cb6e 100644
--- a/drivers/power/reset/ltc2952-poweroff.c
+++ b/drivers/power/reset/ltc2952-poweroff.c
@@ -184,17 +184,6 @@ static void ltc2952_poweroff_kill(void)
 	gpiod_set_value(ltc2952_data->gpio_kill, 1);
 }
 
-static int ltc2952_poweroff_suspend(struct platform_device *pdev,
-	pm_message_t state)
-{
-	return -ENOSYS;
-}
-
-static int ltc2952_poweroff_resume(struct platform_device *pdev)
-{
-	return -ENOSYS;
-}
-
 static void ltc2952_poweroff_default(struct ltc2952_poweroff *data)
 {
 	data->wde_interval = ktime_set(0, 300L*1E6L);
@@ -323,8 +312,6 @@ static struct platform_driver ltc2952_poweroff_driver = {
 		.owner = THIS_MODULE,
 		.of_match_table = of_ltc2952_poweroff_match,
 	},
-	.suspend = ltc2952_poweroff_suspend,
-	.resume = ltc2952_poweroff_resume,
 };
 
 module_platform_driver(ltc2952_poweroff_driver);
-- 
2.1.0

[PATCH 08/13] power: reset: ltc2952: cleanup control flow in poweroff_handler

[Frans Klaver]

ltc2952_poweroff_handler uses gotos to return from the function. Since
we don't do cleanups exiting this function, just return IRQ_HANDLED on
the spot and be done with it.

While at it, remove the variable 'ret'. It was never used very much.

Signed-off-by: Frans Klaver <frans.klaver@xsens.com>
---
 drivers/power/reset/ltc2952-poweroff.c | 18 +++++-------------
 1 file changed, 5 insertions(+), 13 deletions(-)

diff --git a/drivers/power/reset/ltc2952-poweroff.c b/drivers/power/reset/ltc2952-poweroff.c
index af3cb6e..70d8e3a 100644
--- a/drivers/power/reset/ltc2952-poweroff.c
+++ b/drivers/power/reset/ltc2952-poweroff.c
@@ -153,29 +153,21 @@ ltc2952_poweroff_timer_trigger(struct hrtimer *timer)
  */
 static irqreturn_t ltc2952_poweroff_handler(int irq, void *dev_id)
 {
-	int ret;
 	struct ltc2952_poweroff *data = dev_id;
 
-	if (data->kernel_panic)
-		goto irq_ok;
-
-	if (hrtimer_active(&data->timer_wde)) {
+	if (data->kernel_panic || hrtimer_active(&data->timer_wde)) {
 		/* shutdown is already triggered, nothing to do any more */
-		goto irq_ok;
+		return IRQ_HANDLED;
 	}
 
 	if (!hrtimer_active(&data->timer_trigger)) {
-		ret = hrtimer_start(&data->timer_trigger, data->trigger_delay,
-			HRTIMER_MODE_REL);
-
-		if (ret)
+		if (hrtimer_start(&data->timer_trigger, data->trigger_delay,
+				  HRTIMER_MODE_REL))
 			dev_err(data->dev, "unable to start the wait timer\n");
 	} else {
-		ret = hrtimer_cancel(&data->timer_trigger);
+		hrtimer_cancel(&data->timer_trigger);
 		/* omitting return value check, timer should have been valid */
 	}
-
-irq_ok:
 	return IRQ_HANDLED;
 }
 
-- 
2.1.0

[PATCH 09/13] power: reset: ltc2952: fix C++ style function pointers

[Frans Klaver]

The function pointers for the timers and pm_power_off are assigned with
C++ style
	foo = &func;

Let's change it instead to the more C style
	foo = func;

Signed-off-by: Frans Klaver <frans.klaver@xsens.com>
---
 drivers/power/reset/ltc2952-poweroff.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/power/reset/ltc2952-poweroff.c b/drivers/power/reset/ltc2952-poweroff.c
index 70d8e3a..8ab1da6 100644
--- a/drivers/power/reset/ltc2952-poweroff.c
+++ b/drivers/power/reset/ltc2952-poweroff.c
@@ -182,10 +182,10 @@ static void ltc2952_poweroff_default(struct ltc2952_poweroff *data)
 	data->trigger_delay = ktime_set(2, 500L*1E6L);
 
 	hrtimer_init(&data->timer_trigger, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
-	data->timer_trigger.function = &ltc2952_poweroff_timer_trigger;
+	data->timer_trigger.function = ltc2952_poweroff_timer_trigger;
 
 	hrtimer_init(&data->timer_wde, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
-	data->timer_wde.function = &ltc2952_poweroff_timer_wde;
+	data->timer_wde.function = ltc2952_poweroff_timer_wde;
 }
 
 static int ltc2952_poweroff_init(struct platform_device *pdev)
@@ -270,7 +270,7 @@ static int ltc2952_poweroff_probe(struct platform_device *pdev)
 
 	/* TODO: remove ltc2952_data */
 	ltc2952_data = data;
-	pm_power_off = &ltc2952_poweroff_kill;
+	pm_power_off = ltc2952_poweroff_kill;
 
 	data->panic_notifier.notifier_call = ltc2952_poweroff_notify_panic;
 	atomic_notifier_chain_register(&panic_notifier_list,
-- 
2.1.0

[PATCH 10/13] power: reset: ltc2952: disable timers in _remove

[Frans Klaver]

Disable the timers when ltc2952_poweroff is removed. We don't want to
risk calling functions on data that no longer exist.

Signed-off-by: Frans Klaver <frans.klaver@xsens.com>
---
 drivers/power/reset/ltc2952-poweroff.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/power/reset/ltc2952-poweroff.c b/drivers/power/reset/ltc2952-poweroff.c
index 8ab1da6..2f3fd5b 100644
--- a/drivers/power/reset/ltc2952-poweroff.c
+++ b/drivers/power/reset/ltc2952-poweroff.c
@@ -285,6 +285,8 @@ static int ltc2952_poweroff_remove(struct platform_device *pdev)
 	struct ltc2952_poweroff *data = platform_get_drvdata(pdev);
 
 	pm_power_off = NULL;
+	hrtimer_cancel(&data->timer_trigger);
+	hrtimer_cancel(&data->timer_wde);
 	atomic_notifier_chain_unregister(&panic_notifier_list,
 					 &data->panic_notifier);
 	return 0;
-- 
2.1.0

[PATCH 11/13] power: reset: ltc2952: check trigger value before starting timer

[Frans Klaver]

In ltc2952_poweroff_handler it is theoretically possible that the timer
fails to start on first pass (button press), but succeeds in starting on
the second (button release). This will cause the button press to be
misinterpreted, and will incorrectly shut down the system. Because a
picture says more than a thousand words:

Expected behavior:
tmr:      ++++++++++
btn: -----__________-----

Faulty behavior:
tmr:                +++++
btn: -----__________-----

Legend:
+ timer runs
_ button pressed
- button depressed

To prevent this from happening, check the value of the gpio before
starting the timer. If the button is active, we should start the timer,
else we should stop it.

The situation described can now still occur if the polarity of the input
pin is set incorrectly, but that at least is predictable behavior and
can be detected during the first tests.

Signed-off-by: Frans Klaver <frans.klaver@xsens.com>
---
 drivers/power/reset/ltc2952-poweroff.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/power/reset/ltc2952-poweroff.c b/drivers/power/reset/ltc2952-poweroff.c
index 2f3fd5b..fda2735 100644
--- a/drivers/power/reset/ltc2952-poweroff.c
+++ b/drivers/power/reset/ltc2952-poweroff.c
@@ -160,7 +160,7 @@ static irqreturn_t ltc2952_poweroff_handler(int irq, void *dev_id)
 		return IRQ_HANDLED;
 	}
 
-	if (!hrtimer_active(&data->timer_trigger)) {
+	if (gpiod_get_value(data->gpio_trigger)) {
 		if (hrtimer_start(&data->timer_trigger, data->trigger_delay,
 				  HRTIMER_MODE_REL))
 			dev_err(data->dev, "unable to start the wait timer\n");
-- 
2.1.0

[PATCH 12/13] power: reset: ltc2952: make trigger input optional

[Frans Klaver]

Currently the ltc2952 supports only one button sequence to initiate
powerdown. This is not always desirable, as even prolonged button
presses can happen in use.

Allow ltc2952 users to pick their own power down sequence, by making the
trigger input optional. Since this still means that the ltc2952 may
power down the platform if the power button is pressed for about 5
seconds, we still need to make sure to start the watchdog toggle to
prolong the system power for as long as we need it.

This will still allow the system to control power using the kill signal.

Signed-off-by: Frans Klaver <frans.klaver@xsens.com>
---
 drivers/power/reset/ltc2952-poweroff.c | 87 +++++++++++++++++++++-------------
 1 file changed, 55 insertions(+), 32 deletions(-)

diff --git a/drivers/power/reset/ltc2952-poweroff.c b/drivers/power/reset/ltc2952-poweroff.c
index fda2735..72e3d7f 100644
--- a/drivers/power/reset/ltc2952-poweroff.c
+++ b/drivers/power/reset/ltc2952-poweroff.c
@@ -32,7 +32,9 @@
  * - trigger (input)
  *     A level change indicates the shut-down trigger. If it's state reverts
  *     within the time-out defined by trigger_delay, the shut down is not
- *     executed.
+ *     executed. If no pin is assigned to this input, the driver will start the
+ *     watchdog toggle immediately. The chip will only power off the system if
+ *     it is requested to do so through the kill line.
  *
  * - watchdog (output)
  *     Once a shut down is triggered, the driver will toggle this signal,
@@ -116,15 +118,10 @@ static enum hrtimer_restart ltc2952_poweroff_timer_wde(struct hrtimer *timer)
 	return HRTIMER_RESTART;
 }
 
-static enum hrtimer_restart
-ltc2952_poweroff_timer_trigger(struct hrtimer *timer)
+static void ltc2952_poweroff_start_wde(struct ltc2952_poweroff *data)
 {
-	struct ltc2952_poweroff *data = to_ltc2952(timer, timer_trigger);
-	int ret = hrtimer_start(&data->timer_wde,
-				data->wde_interval, HRTIMER_MODE_REL);
-
-	if (ret) {
-		dev_err(data->dev, "unable to start the timer\n");
+	if (hrtimer_start(&data->timer_wde, data->wde_interval,
+			  HRTIMER_MODE_REL)) {
 		/*
 		 * The device will not toggle the watchdog reset,
 		 * thus shut down is only safe if the PowerPath controller
@@ -133,10 +130,17 @@ ltc2952_poweroff_timer_trigger(struct hrtimer *timer)
 		 *
 		 * Only sending a warning as the system will power-off anyway
 		 */
+		dev_err(data->dev, "unable to start the timer\n");
 	}
+}
 
-	dev_info(data->dev, "executing shutdown\n");
+static enum hrtimer_restart
+ltc2952_poweroff_timer_trigger(struct hrtimer *timer)
+{
+	struct ltc2952_poweroff *data = to_ltc2952(timer, timer_trigger);
 
+	ltc2952_poweroff_start_wde(data);
+	dev_info(data->dev, "executing shutdown\n");
 	orderly_poweroff(true);
 
 	return HRTIMER_NORESTART;
@@ -190,7 +194,7 @@ static void ltc2952_poweroff_default(struct ltc2952_poweroff *data)
 
 static int ltc2952_poweroff_init(struct platform_device *pdev)
 {
-	int ret, virq;
+	int ret;
 	struct ltc2952_poweroff *data = platform_get_drvdata(pdev);
 
 	ltc2952_poweroff_default(data);
@@ -210,29 +214,48 @@ static int ltc2952_poweroff_init(struct platform_device *pdev)
 		return ret;
 	}
 
-	data->gpio_trigger = devm_gpiod_get(&pdev->dev, "trigger",
-					    GPIOD_IN);
-	if (IS_ERR(ltc2952_data->gpio_trigger)) {
-		ret = PTR_ERR(ltc2952_data->gpio_trigger);
-		dev_err(&pdev->dev, "unable to claim gpio \"trigger\"\n");
-		return ret;
-	}
-
-	virq = gpiod_to_irq(data->gpio_trigger);
-	if (virq < 0) {
-		dev_err(&pdev->dev, "cannot map GPIO as interrupt");
-		return ret;
+	data->gpio_trigger = devm_gpiod_get(&pdev->dev, "trigger", GPIOD_IN);
+	if (IS_ERR(data->gpio_trigger)) {
+		/*
+		 * It's not a problem if the trigger gpio isn't available, but
+		 * it is worth a warning if its use was defined in the device
+		 * tree.
+		 */
+		if (PTR_ERR(data->gpio_trigger) != -ENOENT)
+			dev_err(&pdev->dev,
+				"unable to claim gpio \"trigger\"\n");
+		data->gpio_trigger = NULL;
 	}
 
-	ret = devm_request_irq(&pdev->dev, virq,
-			       ltc2952_poweroff_handler,
-			       (IRQF_TRIGGER_FALLING | IRQF_TRIGGER_RISING),
-			       "ltc2952-poweroff",
-			       data);
-
-	if (ret) {
-		dev_err(&pdev->dev, "cannot configure an interrupt handler\n");
-		return ret;
+	if (devm_request_irq(&pdev->dev, gpiod_to_irq(data->gpio_trigger),
+			     ltc2952_poweroff_handler,
+			     (IRQF_TRIGGER_FALLING | IRQF_TRIGGER_RISING),
+			     "ltc2952-poweroff",
+			     data)) {
+		/*
+		 * Some things may have happened:
+		 * - No trigger input was defined
+		 * - Claiming the GPIO failed
+		 * - We could not map to an IRQ
+		 * - We couldn't register an interrupt handler
+		 *
+		 * None of these really are problems, but all of them
+		 * disqualify the push button from controlling the power.
+		 *
+		 * It is therefore important to note that if the ltc2952
+		 * detects a button press for long enough, it will still start
+		 * its own powerdown window and cut the power on us if we don't
+		 * start the watchdog trigger.
+		 */
+		if (data->gpio_trigger) {
+			dev_warn(&pdev->dev,
+				 "unable to configure the trigger interrupt\n");
+			devm_gpiod_put(&pdev->dev, data->gpio_trigger);
+			data->gpio_trigger = NULL;
+		}
+		dev_info(&pdev->dev,
+			 "power down trigger input will not be used\n");
+		ltc2952_poweroff_start_wde(data);
 	}
 
 	return 0;
-- 
2.1.0

Re: [PATCH 12/13] power: reset: ltc2952: make trigger input optional

[Frans Klaver]

On Wed, Oct 22, 2014 at 04:31:09PM +0200, Frans Klaver wrote:
> Currently the ltc2952 supports only one button sequence to initiate
> powerdown. This is not always desirable, as even prolonged button
> presses can happen in use.
> 
> Allow ltc2952 users to pick their own power down sequence, by making the
> trigger input optional. Since this still means that the ltc2952 may
> power down the platform if the power button is pressed for about 5
> seconds, we still need to make sure to start the watchdog toggle to
> prolong the system power for as long as we need it.
> 
> This will still allow the system to control power using the kill signal.
> 
> Signed-off-by: Frans Klaver <frans.klaver@xsens.com>
> ---
>  drivers/power/reset/ltc2952-poweroff.c | 87 +++++++++++++++++++++-------------
>  1 file changed, 55 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/power/reset/ltc2952-poweroff.c b/drivers/power/reset/ltc2952-poweroff.c
> index fda2735..72e3d7f 100644
> --- a/drivers/power/reset/ltc2952-poweroff.c
> +++ b/drivers/power/reset/ltc2952-poweroff.c
> @@ -32,7 +32,9 @@
>   * - trigger (input)
>   *     A level change indicates the shut-down trigger. If it's state reverts
>   *     within the time-out defined by trigger_delay, the shut down is not
> - *     executed.
> + *     executed. If no pin is assigned to this input, the driver will start the
> + *     watchdog toggle immediately. The chip will only power off the system if
> + *     it is requested to do so through the kill line.
>   *
>   * - watchdog (output)
>   *     Once a shut down is triggered, the driver will toggle this signal,
> @@ -116,15 +118,10 @@ static enum hrtimer_restart ltc2952_poweroff_timer_wde(struct hrtimer *timer)
>  	return HRTIMER_RESTART;
>  }
>  
> -static enum hrtimer_restart
> -ltc2952_poweroff_timer_trigger(struct hrtimer *timer)
> +static void ltc2952_poweroff_start_wde(struct ltc2952_poweroff *data)
>  {
> -	struct ltc2952_poweroff *data = to_ltc2952(timer, timer_trigger);
> -	int ret = hrtimer_start(&data->timer_wde,
> -				data->wde_interval, HRTIMER_MODE_REL);
> -
> -	if (ret) {
> -		dev_err(data->dev, "unable to start the timer\n");
> +	if (hrtimer_start(&data->timer_wde, data->wde_interval,
> +			  HRTIMER_MODE_REL)) {
>  		/*
>  		 * The device will not toggle the watchdog reset,
>  		 * thus shut down is only safe if the PowerPath controller
> @@ -133,10 +130,17 @@ ltc2952_poweroff_timer_trigger(struct hrtimer *timer)
>  		 *
>  		 * Only sending a warning as the system will power-off anyway
>  		 */
> +		dev_err(data->dev, "unable to start the timer\n");
>  	}
> +}
>  
> -	dev_info(data->dev, "executing shutdown\n");
> +static enum hrtimer_restart
> +ltc2952_poweroff_timer_trigger(struct hrtimer *timer)
> +{
> +	struct ltc2952_poweroff *data = to_ltc2952(timer, timer_trigger);
>  
> +	ltc2952_poweroff_start_wde(data);
> +	dev_info(data->dev, "executing shutdown\n");
>  	orderly_poweroff(true);
>  
>  	return HRTIMER_NORESTART;
> @@ -190,7 +194,7 @@ static void ltc2952_poweroff_default(struct ltc2952_poweroff *data)
>  
>  static int ltc2952_poweroff_init(struct platform_device *pdev)
>  {
> -	int ret, virq;
> +	int ret;
>  	struct ltc2952_poweroff *data = platform_get_drvdata(pdev);
>  
>  	ltc2952_poweroff_default(data);
> @@ -210,29 +214,48 @@ static int ltc2952_poweroff_init(struct platform_device *pdev)
>  		return ret;
>  	}
>  
> -	data->gpio_trigger = devm_gpiod_get(&pdev->dev, "trigger",
> -					    GPIOD_IN);
> -	if (IS_ERR(ltc2952_data->gpio_trigger)) {
> -		ret = PTR_ERR(ltc2952_data->gpio_trigger);
> -		dev_err(&pdev->dev, "unable to claim gpio \"trigger\"\n");
> -		return ret;
> -	}
> -
> -	virq = gpiod_to_irq(data->gpio_trigger);
> -	if (virq < 0) {
> -		dev_err(&pdev->dev, "cannot map GPIO as interrupt");
> -		return ret;
> +	data->gpio_trigger = devm_gpiod_get(&pdev->dev, "trigger", GPIOD_IN);
> +	if (IS_ERR(data->gpio_trigger)) {
> +		/*
> +		 * It's not a problem if the trigger gpio isn't available, but
> +		 * it is worth a warning if its use was defined in the device
> +		 * tree.
> +		 */
> +		if (PTR_ERR(data->gpio_trigger) != -ENOENT)
> +			dev_err(&pdev->dev,
> +				"unable to claim gpio \"trigger\"\n");
> +		data->gpio_trigger = NULL;
>  	}

I just realized devm_gpiod_get_optional() may be slightly cleaner here,
but the difference isn't that big in the end:

+	data->gpio_trigger = devm_gpiod_get_optional(&pdev->dev, "trigger",
+						     GPIOD_IN);
+	if (IS_ERR(data->gpio_trigger)) {
+		/*
+		 * It's not a problem if the trigger gpio isn't available, but
+		 * it is worth a warning if its use was defined in the device
+		 * tree.
+		 */
+		dev_err(&pdev->dev, "unable to claim gpio \"trigger\"\n");
+		data->gpio_trigger = NULL;
 	}

>  
> -	ret = devm_request_irq(&pdev->dev, virq,
> -			       ltc2952_poweroff_handler,
> -			       (IRQF_TRIGGER_FALLING | IRQF_TRIGGER_RISING),
> -			       "ltc2952-poweroff",
> -			       data);
> -
> -	if (ret) {
> -		dev_err(&pdev->dev, "cannot configure an interrupt handler\n");
> -		return ret;
> +	if (devm_request_irq(&pdev->dev, gpiod_to_irq(data->gpio_trigger),
> +			     ltc2952_poweroff_handler,
> +			     (IRQF_TRIGGER_FALLING | IRQF_TRIGGER_RISING),
> +			     "ltc2952-poweroff",
> +			     data)) {
> +		/*
> +		 * Some things may have happened:
> +		 * - No trigger input was defined
> +		 * - Claiming the GPIO failed
> +		 * - We could not map to an IRQ
> +		 * - We couldn't register an interrupt handler
> +		 *
> +		 * None of these really are problems, but all of them
> +		 * disqualify the push button from controlling the power.
> +		 *
> +		 * It is therefore important to note that if the ltc2952
> +		 * detects a button press for long enough, it will still start
> +		 * its own powerdown window and cut the power on us if we don't
> +		 * start the watchdog trigger.
> +		 */
> +		if (data->gpio_trigger) {
> +			dev_warn(&pdev->dev,
> +				 "unable to configure the trigger interrupt\n");
> +			devm_gpiod_put(&pdev->dev, data->gpio_trigger);
> +			data->gpio_trigger = NULL;
> +		}
> +		dev_info(&pdev->dev,
> +			 "power down trigger input will not be used\n");
> +		ltc2952_poweroff_start_wde(data);
>  	}
>  
>  	return 0;
> -- 
> 2.1.0
> 

[PATCH 13/13] power: reset: ltc2952: document optional trigger behavior

[Frans Klaver]

Document the fact that the trigger signal is now optional, and describe
the behavior when this is used.

While at it, fix a typo, and paraphrase a sentence to be less platform
specific.

Signed-off-by: Frans Klaver <frans.klaver@xsens.com>
---
 .../devicetree/bindings/power/reset/ltc2952-poweroff.txt    | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/power/reset/ltc2952-poweroff.txt b/Documentation/devicetree/bindings/power/reset/ltc2952-poweroff.txt
index 0c94c63..cd2d7f5 100644
--- a/Documentation/devicetree/bindings/power/reset/ltc2952-poweroff.txt
+++ b/Documentation/devicetree/bindings/power/reset/ltc2952-poweroff.txt
@@ -1,20 +1,23 @@
 Binding for the LTC2952 PowerPath controller
 
 This chip is used to externally trigger a system shut down. Once the trigger has
-been sent, the chips' watchdog has to be reset to gracefully shut down.
-If the Linux systems decides to shut down it powers off the platform via the
-kill signal.
+been sent, the chip's watchdog has to be reset to gracefully shut down.
+A full powerdown can be triggered via the kill signal.
 
 Required properties:
 
 - compatible:		Must contain: "lltc,ltc2952"
-- trigger-gpios:	phandle + gpio-specifier for the GPIO connected to the
-			chip's trigger line
 - watchdog-gpios:	phandle + gpio-specifier for the GPIO connected to the
 			chip's watchdog line
 - kill-gpios:		phandle + gpio-specifier for the GPIO connected to the
 			chip's kill line
 
+Optional properties:
+- trigger-gpios:	phandle + gpio-specifier for the GPIO connected to the
+			chip's trigger line. If this property is not set, the
+			trigger function is ignored and the chip is kept alive
+			until an explicit kill signal is received
+
 Example:
 
 ltc2952 {
-- 
2.1.0

Re: [PATCH 00/13] ltc2952 modernization and new functionality

[Frans Klaver]

Hi there,

On Wed, Oct 22, 2014 at 04:30:57PM +0200, Frans Klaver wrote:
> 
> This series implements devm_* usage, reduction of globals, some cleanup and
> finally makes the input trigger optional.

Has this fallen through the cracks?

Thanks,
Frans