On Wed, Aug 27, 2014 at 10:56:20AM -0700, David Riley wrote:

Hi Sebastian,

Thanks for the feedback.

On Tue, Aug 26, 2014 at 6:43 PM, Sebastian Reichel [off-list ref] wrote:

quoted

Hi David,

On Tue, Aug 26, 2014 at 04:45:05PM -0700, David Riley wrote:

quoted

This driver registers a restart handler to set a GPIO line high/low
to reset a board based on devicetree bindings.

Driver looks fine to me. I have some comments about the
Documentation, though:

quoted

[...]

diff --git a/Documentation/devicetree/bindings/gpio/gpio-restart.txt b/Documentation/devicetree/bindings/gpio/gpio-restart.txt
new file mode 100644
index 0000000..7cd58788
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/gpio-restart.txt

@@ -0,0 +1,48 @@
+Driver a GPIO line that can be used to restart the system as a
+restart handler.

Please fix the Typo (first word).

Fixed.

quoted

quoted

[...]
+
+The driver supports both level triggered and edge triggered power off.
+At driver load time, the driver will request the given gpio line and
+install a restart handler.

The wording is too driver centric IMHO. You are supposed to document
the binding in a generic way. Maybe start with something like:

"This binding supports level and edge triggered reset."

(power off is the wrong word, since there is already gpio-poweroff).

I've cleaned this up for v2.

quoted

quoted

+If the optional properties 'input' is +not found, the GPIO line
+will be driven in the inactive state. Otherwise its configured
+as an input.

What is this needed for?

This allows other hardware to be attached to the same line to reset
the system.  Carried forward from the gpio-poweroff implementation I
based this on.

Maybe rephrase; the point isn't really that it is configured as input but
that it is an open source pin unless actively driven to reset the system.
That linux models the pin as input is really an implementation detail.

Thanks,
Guenter
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html