Re: [PATCH 1/1] [GPIO]: new arch-independent simple-gpio driver

[David Brownell <david-b@pacbell.net>]

On Wednesday 26 March 2008, Ben Nizette wrote:
> >  drivers/char/simple-gpio.c |  308 ++++++++++++++++++++++++++++++++++++++++++++
>
> Considered putting this in drivers/gpio?  Not a real problem, up to you
> (or David).

I'd expect to see a generic GPIO mechanism like this
in drivers/gpio, yes.  But it should be a bit more
generic than this one is.


> > +/**
> > + *   simple_gpio_probe - setup the range of GPIOs
> > + *
> > + *   Create a character device for the range of GPIOs and have the minor be
> > + *   used to specify the GPIO.
> > + */
> > +static int __devinit simple_gpio_probe(struct platform_device *pdev)
> > +{
> > +     int ret;
> > +     struct group_data *group_data;
> > +     struct resource *gpio_range = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
> > +     int gpio, gpio_max = gpio_range->end - gpio_range->start + 1;
> > +
> 
> Was it a conscious thing to only allow 1 range of gpios per device?  I
> can imagine that it's quite likely that people are going to want to
> expose all unused gpios on a SoC to userspace.  This is going to mean
> lots of small ranges split either side of pre-reserved pins and one
> device per little range is gonna get cumbersome.

As I said in the other thread, this is what I most dislike about
this particular driver:  the need for board-specific setup.  The
need for a character device inode per GPIO could probably be lived
with, if that bigger issue were resolved.

The userspace GPIO access scenarios which seem most compelling to
me involve filling in gaps in board support.  Expecting the folk
who created those gaps (by overlooking them, or not knowing enough
about the system's eventual usage mode) to have enabled resolving
them in this way ... seems unwise.    :)

Building on Ben's comment, it's not just unused SOC GPIOs, it's
all the GPIOs on a board, some of which will be from a SOC and
others of which will be from external chips.

- Dave