linux-kernel - Re: [PATCH 3/3] gpiolib: use chip->names for symlinks, always use gpioN for device names

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: <1260829624.5729.18.camel@ben-desktop>
Date:	Tue, 15 Dec 2009 09:27:04 +1100
From:	Ben Nizette <bn@...sdigital.com>
To:	Jani Nikula <ext-jani.1.nikula@...ia.com>
Cc:	David Brownell <david-b@...bell.net>, Greg KH <gregkh@...e.de>,
	"dbrownell@...rs.sourceforge.net" <dbrownell@...rs.sourceforge.net>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"dsilvers@...tec.co.uk" <dsilvers@...tec.co.uk>,
	"ben@...tec.co.uk" <ben@...tec.co.uk>,
	"Bityutskiy Artem (Nokia-D/Helsinki)" <Artem.Bityutskiy@...ia.com>,
	"akpm@...ux-foundation.org" <akpm@...ux-foundation.org>
Subject: Re: [PATCH 3/3] gpiolib: use chip->names for symlinks, always use
 gpioN for device names

On Mon, 2009-12-14 at 13:16 +0200, Jani Nikula wrote:
> Hi David and Ben -
> 
> On Fri, 2009-12-11 at 06:12 +0100, ext Ben Nizette wrote:
> > On Thu, 2009-12-10 at 19:47 -0800, Greg KH wrote:
> > > As a sysfs file within the device directory called 'name'?  Then just
> > > grep through the tree to find the right device, that also handles
> > > duplicates just fine, right?
> > 
> > Well it bunts the handling of duplicates to who ever is grepping but
> > yea, sounds good.  The user script can sanity-check it's results against
> > the controlling gpio-chip if need be.  In fact, maybe symlink from
> > gpioN/chip back to gpio-chipY could be useful?  A bit redundant though,
> > as you can check using the number ranges..
> > 
> > In fact I thought I had a patch to create /sys/class/gpio/gpioN/name at
> > some stage..  Can't find it though, oh well.
> 
> Ben, could you please look harder? ;)

hehe, likely destroyed in the Great Hard-Disk Fire of '08.  Shouldn't be
hard to recreate, though I don't have the time at the moment.

> 
> If we were to add /sys/class/gpio/gpioN/name attribute, what would be
> the optimal source for the names?

You and I seem to be on the same page here; my $0.02 is for an IDR with
gpio_{get,set,lookup}_name() functions.  Actually, I can't remember, do
the new flex arrays efficiently store sparse array data, or are they
just useful 'coz they can grow?

Still open to debate though is whether gpio_set_name should enforce
uniqueness; is isn't required at a FS-level any more but I can't see
anything good coming from namespace collisions here.

> 
> I'd prefer a scheme where a) the name could be set in both board files
> and drivers, the latter overriding the former as necessary, and b) the
> name could be set without actually requesting the gpio, so you could set
> all known names in board files without interfering with the drivers.
> 
> AFAICS this would pretty much lead to adding a pair of new functions
> gpio_set_name() and gpio_get_name(), which would work also for gpios
> that haven't been requested. (IDR lookup Ben mentioned in another mail
> sounds good, though there's the problem you can't specify the id - this
> is why gpio_setup_irq() uses the flags for storing the id.)

Agreed.

> 
> Here are some other alternatives I could think of, but none of them
> sound good to me:
> 
> 1) Add new function gpio_export_name() to export with a certain name
> attribute. Leads to two ways of exporting.

Well 3 including _link().  I don't think that's the problem so much as
restricting naming to userspace users only.  While I can't think of
kernel users who /need/ this functionality, it'd clean a few code paths
and while we're at it, why not?

> 
> 2) Add 'name' parameter to gpio_export() to export with a certain name
> attribute. Changes an existing interface.

As above

> 
> 3) Use 'label' in gpio_request() for name attribute. Stores names also
> for gpios that are never exported, wastes a pointer per gpio in
> gpio_desc.

And while debugfs is emphatically /not/ an ABI, it's still a semantic
change to the exported information

> 
> 4) Use chip->names. Wastes a pointer per gpio even if one name is used,
> almost the same as adding char *name to struct gpio_desc. Not convenient
> to use, at least in OMAP.

Not convenient on any platform I know of, I've never liked chip->names
and would hope it goes away when (something like) the above interface is
implemented.

	--Ben.


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