linux-kernel - Re: [RFC PATCH 0/3] introduce: Multistate Switch Class

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]

Date:	Thu, 1 Dec 2011 10:04:57 +1100
From:	NeilBrown <neilb@...e.de>
To:	Linus Walleij <linus.walleij@...aro.org>
Cc:	Greg KH <gregkh@...e.de>,
	Dmitry Torokhov <dmitry.torokhov@...il.com>,
	Arnd Bergmann <arnd@...db.de>, myungjoo.ham@...il.com,
	linux-kernel@...r.kernel.org, Mike Lockwood <lockwood@...roid.com>,
	Arve Hjønnevåg <arve@...roid.com>,
	Kyungmin Park <kyungmin.park@...sung.com>,
	Donggeun Kim <dg77.kim@...sung.com>,
	Grant Likely <grant.likely@...retlab.ca>,
	Kalle Komierowski <karl.komierowski@...ricsson.com>,
	Johan PALSSON <johan.palsson@...ricsson.com>,
	Daniel WILLERUD <daniel.willerud@...ricsson.com>
Subject: Re: [RFC PATCH 0/3] introduce: Multistate Switch Class

On Wed, 30 Nov 2011 14:28:36 +0100 Linus Walleij <linus.walleij@...aro.org>
wrote:

> On Wed, Nov 30, 2011 at 7:35 AM, Greg KH <gregkh@...e.de> wrote:
> 
> > So, how do you want this type of interface to look like, uevents only?
> >
> > Or something like gpio?
> >
> > Actually, why can't we just use gpio as-is here and just treat these
> > devices as "generic" i/o "pins"?
> 
> For device drivers we have the issue that the management of
> the GPIO pin-space give me the creeps. It is simple when things
> are simple and then ugly things have start to happen, such
> as dynamically bumping the GPIO pin range and associated
> IRQ range at compile time using unreadable nested macros.
> You roof the space at some arbitrary number at compile-time
> in ARCH_NR_GPIOS and if you don't define it,
> <asm-generic/gpio.h> will helpfully define that your system
> has 256 GPIO pins.
> 
> The effect of the above on unified cross-ARM-arch kernels is
> confusing as the meaning of the GPIO pinspace will
> shift at runtime depending on what system it's running on.
> 
> A new subsystem could allocate switch/pin/line assignments
> dynamically which is way more viable. (And how we chose to
> do in in pinctrl.)
> 
> For userspace several people, notably the GPIOlib subsystem
> maintainer Grant, expressed concerns about the sysfs interface
> to GPIO. So I would not shoehorn new stuff in there, solidifying
> it even more.
> 
> So I would recommend doing this as a new subsystem to save
> us from trouble. There is more to be gained from splitting things
> apart than keeping them together in this case IMO.

I certainly agree that if the GPIO interface causes problem we should be very
careful about extending the use of it.
However I think it would be wrong to simply make something that is different
just for the sake of being different. I will probably introduce a different
set of problems of its own.

Do you know what it is about the sysfs-gpio interface that is problematic?
If it is just one aspect, maybe that can be avoided and the rest still used.

For example, I note that most GPIO pins that are exported via sysfs are
exported by number.  This is problematic exactly as you have described
above.  Numbers change.
However it is quite possible to export GPIO pins by assigning a name.  Doing
this would remove the problem.  We don't need to assign a name to every gpio
pin, just the ones that we want to export to user-space.

If this was the only issue, we could require that when exporting 'switches'
through sysfs it was only done if a name was supplied.

So: do you know what specific concerns have been expressed concerning gpio
and userspace?  I think it best to address specific concern rather than throw
it all out and start again.

Thanks,
NeilBrown

Download attachment "signature.asc" of type "application/pgp-signature" (829 bytes)