linux-kernel - Re: [PATCH RFC 2/6 v3] gpio: Add sysfs support to block GPIO API

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: <20121016084327.GC12801@game.jcrosoft.org>
Date:	Tue, 16 Oct 2012 10:43:27 +0200
From:	Jean-Christophe PLAGNIOL-VILLARD <plagnioj@...osoft.com>
To:	Linus Walleij <linus.walleij@...aro.org>
Cc:	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Grant Likely <grant.likely@...retlab.ca>,
	Daniel Glöckner <daniel-gl@....net>,
	Roland Stigge <stigge@...com.de>, linux-kernel@...r.kernel.org,
	linux-arm-kernel@...ts.infradead.org, w.sang@...gutronix.de,
	jbe@...gutronix.de, highguy@...il.com,
	broonie@...nsource.wolfsonmicro.com
Subject: Re: [PATCH RFC 2/6 v3] gpio: Add sysfs support to block GPIO API

On 22:30 Mon 15 Oct     , Linus Walleij wrote:
> I really request Grant to comment on this...too.
> 
> On Mon, Oct 15, 2012 at 8:19 PM, Greg Kroah-Hartman
> <gregkh@...uxfoundation.org> wrote:
> > On Mon, Oct 15, 2012 at 08:07:02PM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote:
> >> On 21:11 Fri 12 Oct     , Roland Stigge wrote:
> >> > This patch adds sysfs support to the block GPIO API.
> >> >
> >> > Signed-off-by: Roland Stigge <stigge@...com.de>
> >> >
> >> > ---
> >> >  Documentation/ABI/testing/sysfs-gpio |    6
> >> >  drivers/gpio/gpiolib.c               |  226 ++++++++++++++++++++++++++++++++++-
> >> >  include/asm-generic/gpio.h           |   11 +
> >> >  include/linux/gpio.h                 |   13 ++
> >> >  4 files changed, 254 insertions(+), 2 deletions(-)
> >> I really don't like this sysfs we need to add a specific device with ioctl
> >
> > Why?
> 
> I don't like it either, basically because the GPIO sysfs is not
> entirely sound.
> 
> Another patch that is circulating concerns edge triggers and similar,
> and it appear that some parts of the GPIO sysfs is for example
> redefining and exporting IRQchip properties like trigger edge
> in sysfs, while the settings of the irqchip actually used by the driver
> is not reflected in the other direction. So you can *set* these things
> by writing in the GPIO sysfs, but never trust what you *read* from
> there. And you can set what edges an IRQ will trigger on a certain
> GPIO, and the way to handle the IRQs from usespace is to poll
> on a value. This is not really documented but well ...
> 
> Part of me just want to delete that, but I can't because it's now
> an ABI.
> 
> The "devices" that the sysfs files are tied to are not real devices,
> instead the code look like this: whenever a gpio is exported to
> sysfs, the code calls (drivers/gpio/gpiolib.c):
> 
>     dev = device_create(&gpio_class, desc->chip->dev, MKDEV(0, 0),
>                     desc, ioname ? ioname : "gpio%u", gpio);
> 
> Mock device just to get a sysfs opening. And once device for
> every GPIO with no hierarchical correspondence to anything
> in the system.
> 
> The thing is that struct gpio_chip is not a device at all, it's something
> else.
> 
> This inconsistency in the GPIO sysfs implementation make me
> fear adding new stuff to it. The other problems need fixing first.
> 
> The reason an ioctl() IMO is better suited to do the job is that
> it can properly represent a multiple-value operation on several
> GPIOs at the same time in a struct, and it can conversely inform
> userspace about which GPIOs may be a block of signals that
> can be fired simultaneously instead of going to string parsing
> and binary values in sysfs which look like worse hacks to me.
> 
> The last thing I'm a bit softer on though. Mainly I fear of this
> sysfs ABI growing into a beast.
> 
> It was all merged prior to Grant becoming maintainer and
> me becoming co-maintainer of it, so this is legacy business.
> 
> Sadly the main creator of this ABI is David Brownell who is
> not able to respond nor maintain it from where he is now. But
> we need to think hard about what we shall do with this particular
> piece of legacy. Some of the stuff was added by Daniel
> Glöckner so requesting advice from him.
> 
> Daniel: are you interested in helping us fixing the GPIOlib
> sysfs ABI and kernel internals? I'm a bit afraid of it.
My 0.02$ too

Best Regards,
J.
--
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/