Re: [patch 2.6.24-rc2 1/3] generic gpio -- gpio_chip support

[Thomas Gleixner <tglx@linutronix.de>]

On Mon, 12 Nov 2007, David Brownell wrote:
> > I'm still trying to understand what you've observed here.  Is it the case
> > that a single gpio operation went from 6.4 up to 11.2 usecs?
> 
> That was a single bitbanged I2C bit transfer, with embedded udelay()s.
> I believe that was four gpio operations, as summarized at the end of
> that email above.  Enabling preempt + debug increased the cost of
> each GPIO call from whatever it was (reasonable) by 1.2 usecs.

This raw lock change is just pampering over the design problem of the
gpio lib:

There is no need to check for every single access to a GPIO pin,
whether the pin has a valid number and the chip, which provides access
to the pin, is still registered.

Each driver, which wants to access a pin, needs to make sure that

- the pin is available
- the pin is associated to this driver
- the chip reference count is incremented

_before_ it starts to do anything with the pin. Once this is done the
access to the pin is completely lock free except for the protection of
the chip hardware itself.

What you really need is a different API to request and free the access
to a particular pin like:

struct gpio_desc {
       struct gpio_chip *chip;
       int pinoffset;
};

int gpio_request_pin(int nr, struct gpio_desc *desc);
void gpio_release_pin(struct gpio_desc *desc);

where the description structure is caller provided to avoid static
allocation overhead for a large number of unused gpios.

The modification functions just use the description to access the pin
instead of the number and the per access lookup/locking.

The protection of the chip list can be converted to a mutex and
does not need to be a spinlock at all.

Thanks,

	tglx
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/