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

[Ben Nizette <bn@niasdigital.com>]

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.