[PATCH v4 0/2] Add support for ltc2952 poweroff

[PATCH v4 0/2] Add support for ltc2952 poweroff

[Frans Klaver]

This is version 4 of the series that adds support for the ltc2952 powerpath chip.

This series adds a driver for the LTC2952, an external power control chip, which
signals the OS to shut down. Additionally this driver lets the kernel power
down the board.

This driver was tested with:
  - kernel version 3.15.8, 3.17
  - ARM based processor (TI AM335x series)

v3..v4:
  - Rename *kill to *poweroff
  - Use managed resources (devm_*) where possible
  - Handle irq failures on trigger gpio as if no trigger input was defined
  - Drop no-op suspend/resume functions
  - Check value before starting timer - it may be possible that the timer fails
    to start on first pass, but starts on second pass. While this is unlikely,
    it will throw off the entire system. Just check the actual gpio value to be
    sure. 
  - Reduce, almost eliminate, use of globals (I dislike them)
  - Fix some style issues
  - Now also tested on v3.17

v2..v3:
  - Allow absence of button input pin (and document that)
  - Fix some whitespace issues
  - Straighten out control flow here and there
  - Get rid of gpio array. This was only used in setup/cleanup cases. It never mattered elsewhere
  - Add some explanation to the commit message that introduces the driver
  - Add other contributors
  - Update René's e-mail; his Xsens e-mail is no longer valid
  - Now also tested on v3.17-rc7

René Moll (2):
  power: reset: add LTC2952 poweroff support
  power: reset: document LTC2952 poweroff support

 .../bindings/power/reset/ltc2952-poweroff.txt      |  31 ++
 drivers/power/reset/Kconfig                        |  10 +
 drivers/power/reset/Makefile                       |   1 +
 drivers/power/reset/ltc2952-poweroff.c             | 327 +++++++++++++++++++++
 4 files changed, 369 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power/reset/ltc2952-poweroff.txt
 create mode 100644 drivers/power/reset/ltc2952-poweroff.c

-- 
2.1.0

[PATCH v4 1/2] power: reset: add LTC2952 poweroff support

[Frans Klaver]

From: René Moll <linux@r-moll.nl>

The LTC2952 allows control over system power using a push button. It also
supports powering down the board from a controller on the board.

If the power button is pushed, the ltc2952 starts a 400ms window to
properly shut down the software. This window can be stretched by
toggling a watchdog line. This driver always uses this watchdog line.
The power button input is optional. This allows the system to use a
custom shutdown sequence on the push button.

On software shutdown the driver sets the kill signal, starting the same
sequence as if the push button was used.

Signed-off-by: René Moll <linux@r-moll.nl>
Signed-off-by: Tjerk Hofmeijer <tjerk.hofmeijer@xsens.com>
Signed-off-by: Frans Klaver <frans.klaver@xsens.com>
---
 drivers/power/reset/Kconfig            |  10 +
 drivers/power/reset/Makefile           |   1 +
 drivers/power/reset/ltc2952-poweroff.c | 327 +++++++++++++++++++++++++++++++++
 3 files changed, 338 insertions(+)
 create mode 100644 drivers/power/reset/ltc2952-poweroff.c

diff --git a/drivers/power/reset/Kconfig b/drivers/power/reset/Kconfig
index ca41523..a0f68da 100644
--- a/drivers/power/reset/Kconfig
+++ b/drivers/power/reset/Kconfig
@@ -45,6 +45,16 @@ config POWER_RESET_HISI
 	help
 	  Reboot support for Hisilicon boards.
 
+config POWER_RESET_LTC2952
+	bool "LTC2952 PowerPath power-off driver"
+	depends on OF_GPIO && POWER_RESET
+	help
+	  This driver supports an external powerdown trigger and board
+	  power down via the LTC2952.
+
+	  If your board uses an LTC2952, say  Y and create a binding
+	  in the device tree.
+
 config POWER_RESET_MSM
 	bool "Qualcomm MSM power-off driver"
 	depends on POWER_RESET && ARCH_QCOM
diff --git a/drivers/power/reset/Makefile b/drivers/power/reset/Makefile
index a42e70e..3cbb7c3 100644
--- a/drivers/power/reset/Makefile
+++ b/drivers/power/reset/Makefile
@@ -3,6 +3,7 @@ obj-$(CONFIG_POWER_RESET_AXXIA) += axxia-reset.o
 obj-$(CONFIG_POWER_RESET_BRCMSTB) += brcmstb-reboot.o
 obj-$(CONFIG_POWER_RESET_GPIO) += gpio-poweroff.o
 obj-$(CONFIG_POWER_RESET_HISI) += hisi-reboot.o
+obj-$(CONFIG_POWER_RESET_LTC2952) += ltc2952-poweroff.o
 obj-$(CONFIG_POWER_RESET_MSM) += msm-poweroff.o
 obj-$(CONFIG_POWER_RESET_QNAP) += qnap-poweroff.o
 obj-$(CONFIG_POWER_RESET_RESTART) += restart-poweroff.o
diff --git a/drivers/power/reset/ltc2952-poweroff.c b/drivers/power/reset/ltc2952-poweroff.c
new file mode 100644
index 0000000..5c6e2a95
--- /dev/null
+++ b/drivers/power/reset/ltc2952-poweroff.c
@@ -0,0 +1,327 @@
+/*
+ * LTC2952 PowerPath TM driver
+ *
+ * Copyright (C) 2014, Xsens Technologies BV <info@xsens.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * ----------------------------------------
+ * - Description
+ * ----------------------------------------
+ *
+ * This driver is to be used with an external LTC2952 PowerPath Controller.
+ * Its function is to determine when a external shutdown is triggered and react
+ * by properly shutting down the system.
+ *
+ * This driver expects a device tree with a ltc2952 entry for pin mapping.
+ *
+ * ----------------------------------------
+ * - GPIO
+ * ----------------------------------------
+ *
+ * The following GPIOs are used:
+ * - trigger (input, optional)
+ *     Input active indicates the shutdown trigger. If its state reverts to
+ *     inactive within the timeout defined by trigger_delay, the system won't
+ *     be shut down. If no pin is assigned to this input the driver will start
+ *     the watchdog immediately and only a power off event will poweroff the
+ *     ltc2952
+ *
+ * - watchdog (output)
+ *     Once a shutdown is triggered, the driver will toggle this signal, with
+ *     an internal (wde_interval) to stall the hardware shut down.
+ *
+ * - poweroff (output)
+ *     The last action during shutdown is triggering this signalling, such
+ *     that the PowerPath Control will power down the hardware.
+ *
+ * ----------------------------------------
+ * - Interrupts
+ * ----------------------------------------
+ *
+ * The driver requires a non-shared, edge-triggered interrupt on the trigger
+ * GPIO.
+ *
+ */
+
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/device.h>
+#include <linux/platform_device.h>
+#include <linux/ktime.h>
+#include <linux/slab.h>
+#include <linux/kmod.h>
+#include <linux/module.h>
+#include <linux/gpio/consumer.h>
+#include <linux/reboot.h>
+
+#define WDE_TIMEOUT ktime_set(0, 300L*1E6L)
+#define TRIGGER_TIMEOUT ktime_set(2, 500L*1E6L)
+
+struct ltc2952_poweroff {
+	struct hrtimer timer_trigger;
+	struct hrtimer timer_wde;
+
+	ktime_t trigger_delay;
+	ktime_t wde_interval;
+
+	struct device *dev;
+
+	struct gpio_desc *gpio_trigger;
+	struct gpio_desc *gpio_watchdog;
+	struct gpio_desc *gpio_poweroff;
+
+	bool kernel_panic;
+	struct notifier_block panic_notifier;
+};
+#define to_ltc2952(p, m) container_of(p, struct ltc2952_poweroff, m)
+
+/**
+ * ltc2952_poweroff_timer_wde - Timer callback
+ * Toggles the watchdog reset signal each wde_interval
+ *
+ * @timer: corresponding timer
+ *
+ * Returns HRTIMER_RESTART for an infinite loop which will only stop when the
+ * machine actually shuts down
+ */
+static enum hrtimer_restart ltc2952_poweroff_timer_wde(struct hrtimer *timer)
+{
+	ktime_t now;
+	int state;
+	struct ltc2952_poweroff *data = to_ltc2952(timer, timer_wde);
+
+	if (data->kernel_panic)
+		return HRTIMER_NORESTART;
+
+	state = gpiod_get_value(data->gpio_watchdog);
+	gpiod_set_value(data->gpio_watchdog, !state);
+
+	now = hrtimer_cb_get_time(timer);
+	hrtimer_forward(timer, now, data->wde_interval);
+
+	return HRTIMER_RESTART;
+}
+
+static void ltc2952_poweroff_start_wde(struct ltc2952_poweroff *data)
+{
+	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
+		 * has a long enough time-off before triggering a hardware
+		 * power-off.
+		 *
+		 * Only mention the error, as the system will power-off anyway
+		 */
+		dev_err(data->dev, "unable to start the watchdog timer\n");
+	}
+}
+
+static enum hrtimer_restart ltc2952_poweroff_timer_trigger(struct hrtimer *t)
+{
+	struct ltc2952_poweroff *data = to_ltc2952(t, timer_trigger);
+
+	dev_info(data->dev, "executing shutdown\n");
+	ltc2952_poweroff_start_wde(data);
+	orderly_poweroff(true);
+	return HRTIMER_NORESTART;
+}
+
+/**
+ * ltc2952_poweroff_handler - Interrupt handler
+ *
+ * Activate or deactive the shutdown timeout, based on the value of the trigger
+ * gpio. If the button is pressed for long enough, the shutdown timer fires and
+ * the system is actually shut down.
+ *
+ * @irq: IRQ number
+ * @dev_id: pointer to the main data structure
+ */
+static irqreturn_t ltc2952_poweroff_handler(int irq, void *dev_id)
+{
+	struct ltc2952_poweroff *data = dev_id;
+
+	if (data->kernel_panic || hrtimer_active(&data->timer_wde))
+		/* we're either panicking, or already shutting down */
+		return IRQ_HANDLED;
+
+	if (gpiod_get_value(data->gpio_trigger)) {
+		/* button pressed */
+		if (hrtimer_start(&data->timer_trigger, data->trigger_delay,
+				  HRTIMER_MODE_REL))
+			dev_err(data->dev, "unable to start the wait timer\n");
+	} else {
+		/* button released */
+		hrtimer_cancel(&data->timer_trigger);
+	}
+
+	return IRQ_HANDLED;
+}
+
+static int ltc2952_poweroff_init(struct platform_device *pdev)
+{
+	struct ltc2952_poweroff *data = platform_get_drvdata(pdev);
+
+	data->wde_interval = WDE_TIMEOUT;
+	data->trigger_delay = TRIGGER_TIMEOUT;
+
+	hrtimer_init(&data->timer_trigger, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
+	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->gpio_watchdog = devm_gpiod_get(&pdev->dev, "watchdog",
+					     GPIOD_OUT_LOW);
+	if (IS_ERR(data->gpio_watchdog)) {
+		dev_err(&pdev->dev, "unable to claim watchdog-gpio\n");
+		return PTR_ERR(data->gpio_watchdog);
+	}
+
+	data->gpio_poweroff = devm_gpiod_get(&pdev->dev, "poweroff",
+					     GPIOD_OUT_LOW);
+	if (IS_ERR(data->gpio_poweroff)) {
+		dev_err(&pdev->dev, "unable to claim poweroff-gpio\n");
+		return PTR_ERR(data->gpio_poweroff);
+	}
+
+	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 its use was defined in the device
+		 * tree.
+		 */
+		if (PTR_ERR(data->gpio_trigger) != -ENOENT)
+			dev_warn(&pdev->dev, "unable to claim trigger-gpio\n");
+		data->gpio_trigger = NULL;
+	}
+
+	if (devm_request_irq(&pdev->dev,
+			     gpiod_to_irq(data->gpio_trigger),
+			     ltc2952_poweroff_handler,
+			     (IRQF_TRIGGER_FALLING | IRQF_TRIGGER_RISING),
+			     KBUILD_MODNAME, data)) {
+		/*
+		 * No trigger input was defined, claiming the gpio failed, we
+		 * couldn't map to an irq, or we couldn't register the
+		 * interrupt handler. None of these should be a real problem,
+		 * 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 is not used\n");
+		ltc2952_poweroff_start_wde(data);
+	}
+
+	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;
+}
+
+/* We need a global pointer here */
+static struct ltc2952_poweroff *ltc2952_data;
+
+static void ltc2952_poweroff_poweroff(void)
+{
+	gpiod_set_value(ltc2952_data->gpio_poweroff, 1);
+}
+
+static int ltc2952_poweroff_probe(struct platform_device *pdev)
+{
+	int ret;
+	struct ltc2952_poweroff *data;
+
+	if (ltc2952_data) {
+		dev_err(&pdev->dev, "already probed");
+		return -ENODEV;
+	}
+
+	if (pm_power_off) {
+		dev_err(&pdev->dev, "pm_power_off already registered");
+		return -EBUSY;
+	}
+
+	data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	data->dev = &pdev->dev;
+	platform_set_drvdata(pdev, data);
+
+	ret = ltc2952_poweroff_init(pdev);
+	if (ret)
+		return ret;
+
+	pm_power_off = ltc2952_poweroff_poweroff;
+	ltc2952_data = data;
+
+	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;
+}
+
+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_wde);
+
+	atomic_notifier_chain_unregister(&panic_notifier_list,
+					 &data->panic_notifier);
+	return 0;
+}
+
+static const struct of_device_id of_ltc2952_poweroff_match[] = {
+	{ .compatible = "lltc,ltc2952"},
+	{},
+};
+MODULE_DEVICE_TABLE(of, of_ltc2952_poweroff_match);
+
+static struct platform_driver ltc2952_poweroff_driver = {
+	.probe = ltc2952_poweroff_probe,
+	.remove = ltc2952_poweroff_remove,
+	.driver = {
+		.name = KBUILD_MODNAME,
+		.of_match_table = of_ltc2952_poweroff_match,
+	},
+};
+module_platform_driver(ltc2952_poweroff_driver);
+
+MODULE_AUTHOR("René Moll <linux@r-moll.nl>");
+MODULE_AUTHOR("Frans Klaver <frans.klaver@xsens.com>");
+MODULE_AUTHOR("Tjerk Hofmeijer <tjerk.hofmeijer@xsens.com>");
+MODULE_DESCRIPTION("LTC PowerPath power-off driver");
-- 
2.1.0

[PATCH v4 2/2] power: reset: document LTC2952 poweroff support

[Frans Klaver]

From: René Moll <linux@r-moll.nl>

Signed-off-by: René Moll <linux@r-moll.nl>
Signed-off-by: Tjerk Hofmeijer <tjerk.hofmeijer@xsens.com>
Signed-off-by: Frans Klaver <frans.klaver@xsens.com>
---
 .../bindings/power/reset/ltc2952-poweroff.txt      | 31 ++++++++++++++++++++++
 1 file changed, 31 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power/reset/ltc2952-poweroff.txt

diff --git a/Documentation/devicetree/bindings/power/reset/ltc2952-poweroff.txt b/Documentation/devicetree/bindings/power/reset/ltc2952-poweroff.txt
new file mode 100644
index 0000000..c3f3d9e
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/reset/ltc2952-poweroff.txt
@@ -0,0 +1,31 @@
+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
+poweroff signal.
+
+Required properties:
+
+- compatible:		Must be: "lltc,ltc2952"
+- watchdog-gpios:	phandle + gpio-specifier for the GPIO connected to the
+			chip's watchdog line
+- poweroff-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 {
+	compatible = "lltc,ltc2952";
+
+	trigger-gpios = <&gpio0 1 GPIO_ACTIVE_LOW>;
+	watchdog-gpios = <&gpio1 2 GPIO_ACTIVE_HIGH>;
+	poweroff-gpios = <&gpio0 2 GPIO_ACTIVE_LOW>;
+};
-- 
2.1.0

Re: [PATCH v4 2/2] power: reset: document LTC2952 poweroff support

[Mark Rutland]

Hi,

On Thu, Oct 09, 2014 at 03:50:42PM +0100, Frans Klaver wrote:
> From: René Moll <linux@r-moll.nl>
> 
> Signed-off-by: René Moll <linux@r-moll.nl>
> Signed-off-by: Tjerk Hofmeijer <tjerk.hofmeijer@xsens.com>
> Signed-off-by: Frans Klaver <frans.klaver@xsens.com>
> ---
>  .../bindings/power/reset/ltc2952-poweroff.txt      | 31 ++++++++++++++++++++++
>  1 file changed, 31 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/power/reset/ltc2952-poweroff.txt
> 
> diff --git a/Documentation/devicetree/bindings/power/reset/ltc2952-poweroff.txt b/Documentation/devicetree/bindings/power/reset/ltc2952-poweroff.txt
> new file mode 100644
> index 0000000..c3f3d9e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/reset/ltc2952-poweroff.txt
> @@ -0,0 +1,31 @@
> +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.

s/chips/chip's/

> If +the Linux systems decides to shut down, it powers off the platform
> via the +poweroff signal.

This sentence can go; the binding should describe the hardware rather
than the Linux behaviour.

Thanks,
Mark.

> +
> +Required properties:
> +
> +- compatible:		Must be: "lltc,ltc2952"
> +- watchdog-gpios:	phandle + gpio-specifier for the GPIO connected to the
> +			chip's watchdog line
> +- poweroff-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 {
> +	compatible = "lltc,ltc2952";
> +
> +	trigger-gpios = <&gpio0 1 GPIO_ACTIVE_LOW>;
> +	watchdog-gpios = <&gpio1 2 GPIO_ACTIVE_HIGH>;
> +	poweroff-gpios = <&gpio0 2 GPIO_ACTIVE_LOW>;
> +};
> -- 
> 2.1.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH v4 2/2] power: reset: document LTC2952 poweroff support

[Guenter Roeck]

On Thu, Oct 09, 2014 at 04:07:56PM +0100, Mark Rutland wrote:
> Hi,
> 
> On Thu, Oct 09, 2014 at 03:50:42PM +0100, Frans Klaver wrote:
> > From: René Moll <linux@r-moll.nl>
> > 
> > Signed-off-by: René Moll <linux@r-moll.nl>
> > Signed-off-by: Tjerk Hofmeijer <tjerk.hofmeijer@xsens.com>
> > Signed-off-by: Frans Klaver <frans.klaver@xsens.com>
> > ---
> >  .../bindings/power/reset/ltc2952-poweroff.txt      | 31 ++++++++++++++++++++++
> >  1 file changed, 31 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/power/reset/ltc2952-poweroff.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/power/reset/ltc2952-poweroff.txt b/Documentation/devicetree/bindings/power/reset/ltc2952-poweroff.txt
> > new file mode 100644
> > index 0000000..c3f3d9e
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/power/reset/ltc2952-poweroff.txt
> > @@ -0,0 +1,31 @@
> > +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.
> 
> s/chips/chip's/
> 
> > If +the Linux systems decides to shut down, it powers off the platform
> > via the +poweroff signal.
> 
> This sentence can go; the binding should describe the hardware rather
> than the Linux behaviour.
> 
Maybe find an operating system independent wording and describe what the
implementation should do. Or is that out of scope as well ?

Thanks,
Guenter

Re: [PATCH v4 2/2] power: reset: document LTC2952 poweroff support

[Mark Rutland]

On Thu, Oct 09, 2014 at 05:09:36PM +0100, Guenter Roeck wrote:
> On Thu, Oct 09, 2014 at 04:07:56PM +0100, Mark Rutland wrote:
> > Hi,
> > 
> > On Thu, Oct 09, 2014 at 03:50:42PM +0100, Frans Klaver wrote:
> > > From: René Moll <linux@r-moll.nl>
> > > 
> > > Signed-off-by: René Moll <linux@r-moll.nl>
> > > Signed-off-by: Tjerk Hofmeijer <tjerk.hofmeijer@xsens.com>
> > > Signed-off-by: Frans Klaver <frans.klaver@xsens.com>
> > > ---
> > >  .../bindings/power/reset/ltc2952-poweroff.txt      | 31 ++++++++++++++++++++++
> > >  1 file changed, 31 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/power/reset/ltc2952-poweroff.txt
> > > 
> > > diff --git a/Documentation/devicetree/bindings/power/reset/ltc2952-poweroff.txt b/Documentation/devicetree/bindings/power/reset/ltc2952-poweroff.txt
> > > new file mode 100644
> > > index 0000000..c3f3d9e
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/power/reset/ltc2952-poweroff.txt
> > > @@ -0,0 +1,31 @@
> > > +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.
> > 
> > s/chips/chip's/
> > 
> > > If +the Linux systems decides to shut down, it powers off the platform
> > > via the +poweroff signal.
> > 
> > This sentence can go; the binding should describe the hardware rather
> > than the Linux behaviour.
> > 
> Maybe find an operating system independent wording and describe what the
> implementation should do. Or is that out of scope as well ?

Something like "The platform can be powered off via the poweroff signal"
would be OK. So long as it's not a description of Linux internals that's
fine. 

Ideally the binding wouldn't state how the OS should use the device
either. So if bindings can be worded w.r.t. possible uses of a device
rather than specific users, that would be preferable unless the entire
point of the binding is to specify a very specific use.

Thanks,
Mark.

Re: [PATCH v4 2/2] power: reset: document LTC2952 poweroff support

[Frans Klaver]

On 9 October 2014 19:32:46 CEST, Mark Rutland <mark.rutland@arm.com> wrote:
>On Thu, Oct 09, 2014 at 05:09:36PM +0100, Guenter Roeck wrote:
>> On Thu, Oct 09, 2014 at 04:07:56PM +0100, Mark Rutland wrote:
>> > Hi,
>> > 
>> > On Thu, Oct 09, 2014 at 03:50:42PM +0100, Frans Klaver wrote:
>> > > From: René Moll <linux@r-moll.nl>
>> > > 
>> > > Signed-off-by: René Moll <linux@r-moll.nl>
>> > > Signed-off-by: Tjerk Hofmeijer <tjerk.hofmeijer@xsens.com>
>> > > Signed-off-by: Frans Klaver <frans.klaver@xsens.com>
>> > > ---
>> > >  .../bindings/power/reset/ltc2952-poweroff.txt      | 31
>++++++++++++++++++++++
>> > >  1 file changed, 31 insertions(+)
>> > >  create mode 100644
>Documentation/devicetree/bindings/power/reset/ltc2952-poweroff.txt
>> > > 
>> > > diff --git
>a/Documentation/devicetree/bindings/power/reset/ltc2952-poweroff.txt
>b/Documentation/devicetree/bindings/power/reset/ltc2952-poweroff.txt
>> > > new file mode 100644
>> > > index 0000000..c3f3d9e
>> > > --- /dev/null
>> > > +++
>b/Documentation/devicetree/bindings/power/reset/ltc2952-poweroff.txt
>> > > @@ -0,0 +1,31 @@
>> > > +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.
>> > 
>> > s/chips/chip's/
>> > 
>> > > If +the Linux systems decides to shut down, it powers off the
>platform
>> > > via the +poweroff signal.
>> > 
>> > This sentence can go; the binding should describe the hardware
>rather
>> > than the Linux behaviour.
>> > 
>> Maybe find an operating system independent wording and describe what
>the
>> implementation should do. Or is that out of scope as well ?
>
>Something like "The platform can be powered off via the poweroff
>signal"
>would be OK. So long as it's not a description of Linux internals
>that's
>fine. 
>
>Ideally the binding wouldn't state how the OS should use the device
>either. So if bindings can be worded w.r.t. possible uses of a device
>rather than specific users, that would be preferable unless the entire
>point of the binding is to specify a very specific use.

Alright, that makes sense.

Thanks, 
Frans 
G