linux-kernel - Re: [PATCH v1 1/1] power: Add simple gpio-restart driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20140827181413.GA2198@roeck-us.net>
Date:	Wed, 27 Aug 2014 11:14:13 -0700
From:	Guenter Roeck <linux@...ck-us.net>
To:	David Riley <davidriley@...gle.com>
Cc:	Sebastian Reichel <sre@...nel.org>,
	Dmitry Eremin-Solenikov <dbaryshkov@...il.com>,
	David Woodhouse <dwmw2@...radead.org>,
	Rob Herring <robh+dt@...nel.org>,
	Pawel Moll <pawel.moll@....com>,
	Mark Rutland <mark.rutland@....com>,
	Ian Campbell <ijc+devicetree@...lion.org.uk>,
	Kumar Gala <galak@...eaurora.org>,
	Grant Likely <grant.likely@...aro.org>,
	Arnd Bergmann <arnd@...db.de>,
	Anton Vorontsov <anton@...msg.org>,
	Marc Carino <marc.ceeeee@...il.com>,
	Anders Berg <anders.berg@....com>,
	Laxman Dewangan <ldewangan@...dia.com>,
	Ivan Khoronzhuk <ivan.khoronzhuk@...com>,
	Maxime Ripard <maxime.ripard@...e-electrons.com>,
	Haojian Zhuang <haojian.zhuang@...aro.org>,
	Jamie Lentin <jm@...tin.co.uk>,
	Doug Anderson <dianders@...omium.org>,
	devicetree@...r.kernel.org,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	linux-pm@...r.kernel.org
Subject: Re: [PATCH v1 1/1] power: Add simple gpio-restart driver

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 <sre@...nel.org> wrote:
> > Hi David,
> >
> > On Tue, Aug 26, 2014 at 04:45:05PM -0700, David Riley wrote:
> >> 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:
> >
> >> [...]
> >> 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.
> 
> >
> >> [...]
> >> +
> >> +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.
> 
> >> +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 linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/