[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1299466127.11719.127.camel@ptyser-laptop>
Date: Sun, 06 Mar 2011 20:48:47 -0600
From: Peter Tyser <ptyser@...-inc.com>
To: Ryan Mallon <ryan@...ewatersys.com>
Cc: Grant Likely <grant.likely@...retlab.ca>,
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>,
Lars-Peter Clausen <lars@...afoo.de>,
Alexander Stein <alexander.stein@...tec-electronic.com>
Subject: Re: [PATCH v5 1/4] gpiolib: Add "unknown" direction support
On Mon, 2011-03-07 at 09:19 +1300, Ryan Mallon wrote:
> On 03/06/2011 08:25 PM, Grant Likely wrote:
>
> > Back to sysfs: The sysfs gpio interface is useful for many reasons,
> > but it is dangerous much in the same way that /dev/mem is dangerous.
> > It gives userspace unfettered access to gpio resources without any
> > clues about how it should be used. I consider it bad practice to
> > depend on the gpio sysfs interface for any real system/application.
>
> In an embedded system, where both the kernel and the userspace are
> provided by the hardware vendor, it can be completely sensible to rely
> on gpio numbers in userspace (though I would also like to see the sysfs
> interface able to use named access). There are some existing kernel
> interfaces for common gpio functions such as leds and input devices, but
> there are also many other valid uses for gpios. There are many reasons
> for the access code to be in userspace too: It may be easier to write
> and debug, depending on the setup of the device it may be easier to do
> firmware upgrades of userspace than it is for the kernel, etc.
>
> > gpio numbers could easily change or become unavailable if a kernel
> > driver decides to use them (heck, I'd like to be rid of gpio
> > numbers entirely, but that's a separate debate). I'd much rather see
> > real device drivers be written for gpio interfaces instead of bodging
> > things together from userspace. Since the addition of an 'unknown'
> > direction is solely for benefit of the sysfs interface, I don't (yet)
> > see a valid argument for adding it. *Who cares* if sysfs might report
> > direction as 'input' inaccurately?
>
> Because it is incorrect? It may also be useful for a userspace debug
> tool to request all available gpios and show the current direction and
> value of them.
>
> > It may be mildly inconvenient to a
> > human reader, but it doesn't change the usage model one iota because
> > the direction still must be explicitly set before using the gpio.
>
> I agree that the usage model should be to request and then explicitly
> set the direction before use, but that isn't really an argument for
> exporting incorrect information to userspace. The ABI should attempt to
> prevent abuse of itself so that crappy userspace apps cannot be written.
> Either exporting the direction as "unknown" or making the direction file
> unreadable and the value file unreadable/unwritable until the direction
> has been explicitly set?
>
> > So, I'm going to say no to this patch.
>
> The patch is small (it adds 9 lines) and fixes an incorrect value being
> exported to userspace. By your argument we should actually remove the
> ability to read the direction file in sysfs, since userspace must
> explicitly set it, and therefore knows what the direction is?
+1 to all Ryan's points. I hope this patch, or something like it, will
be accepted.
On a related note, I just noticed I need to update
Documentation/gpio.txt to document the 'unknown' direction, if this
patch is accepted. I'll resubmit after waiting for feedback.
Best,
Peter
--
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/
Powered by blists - more mailing lists