linux-kernel - Re: [PATCH v5 2/4] gpiolib: Add ability to get GPIO direction

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20110312091846.GJ9347@angua.secretlab.ca>
Date:	Sat, 12 Mar 2011 02:18:46 -0700
From:	Grant Likely <grant.likely@...retlab.ca>
To:	Peter Tyser <ptyser@...-inc.com>
Cc:	linux-kernel@...r.kernel.org, Alek Du <alek.du@...el.com>,
	Samuel Ortiz <sameo@...ux.intel.com>,
	David Brownell <dbrownell@...rs.sourceforge.net>,
	Eric Miao <eric.y.miao@...il.com>,
	Uwe Kleine-König 
	<u.kleine-koenig@...gutronix.de>,
	Mark Brown <broonie@...nsource.wolfsonmicro.com>,
	Joe Perches <joe@...ches.com>,
	Alan Cox <alan@...rguk.ukuu.org.uk>
Subject: Re: [PATCH v5 2/4] gpiolib: Add ability to get GPIO direction

On Mon, Mar 07, 2011 at 06:38:49PM -0600, Peter Tyser wrote:
> On Mon, 2011-03-07 at 00:08 -0700, Grant Likely wrote:
> > On Sun, Mar 06, 2011 at 09:07:21PM -0600, Peter Tyser wrote:
> > > On Sun, 2011-03-06 at 00:30 -0700, Grant Likely wrote:
> > > > On Tue, Mar 01, 2011 at 05:28:18PM -0600, Peter Tyser wrote:
> > > > > Add a new get_direction() function to the gpio_chip structure.  This is
> > > > > useful so that the direction of a GPIO can be determined when its
> > > > > initially exported.  Previously, the direction defaulted to "unknown"
> > > > > regardless of the actual configuration of the GPIO.
> > > > > 
> > > > > If a GPIO driver implements get_direction(), it is called in
> > > > > gpio_request() to set the initial direction of the GPIO accurately.
> > > > 
> > > > I don't like this approach.  It effectively creates two methods for
> > > > determining the direction of a gpio; either ask the driver, or read
> > > > the flags value.  Currently, only gpio_request() actually uses the
> > > > first option, but it still leaves the ambiguity.
> > > > 
> > > > Instead, the driver should be able to preload the direction flag at or
> > > > shortly after gpiochip_add() time.  No need for a new callback hook
> > > > from what I can see.
> > > 
> > > The callback hook was used because the gpio_desc[] structure was local
> > > to gpiolib.c.  The code would have to be restructured a bit to allow
> > > drivers to set the flags themselves.  I can do that if you'd prefer, but
> > > don't think it'd be all that much cleaner.
> > 
> > Make it part of the gpiochip_add function, or add a function for
> > explicitly setting gpio directions.  Should not require any
> > reorganization at all.
> > 
> > > Also, I thought it made sense to read the direction of the GPIO in
> > > gpio_request() because that's the point that the GPIO comes under the
> > > GPIO subsystem's control.  Prior to that, there's a chance the direction
> > > could be changed by platform code, or another driver, etc, so reading
> > > the direction in gpiochip_add() may result in out-of-date directions.
> > 
> > That's the problem with gpiolib having sole responsibility of the
> > cache.  It doesn't give the driver any recourse for changing things if
> > it needs to.  The best solution might very well be to eliminate the
> > direction state flag entirely and force all gpiochip drivers to
> > populate a gpio_get_direction() callback, but that is a lot more work.
> > 
> > > The reading of the direction could also be put in chip drivers'
> > > request() function.  That would get rid of the callback and still ensure
> > > the direction is up-to-date.
> > 
> > I don't object to a callback hook.  My objection is how it is bodged
> > on to work around limitations to the direction being cached in the
> > flags variable.  I want to see a solution that either depends entirely
> > on the callback, or completely fixes the problems with the cached
> > value by allowing the driver to update it.
> 
> I agree it'd be nice to have each driver implement a get_direction()
> function and remove the FLAG_DIR_* flags altogether, but didn't want to
> do the work to 20 drivers which I don't use...  Would you accept a
> series that:
> - Added "unknown" direction support (eg patch 1)
> - Added ability to get GPIO direction (eg patch 2 in this series)
> - Sequence of patches to add get_direction() to all GPIO drivers (most
> of which I couldn't test).
> - Replace checking of FLAG_DIR_* in gpiolib.c with calls to new
> get_direction() function and remove FLAG_DIR_* flags all together.

Yes, this sounds perfect to me.

g.

--
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/