Messages in this thread | | | From | David Brownell <> | Subject | Re: [patch 2.6.24-rc2 1/3] generic gpio -- gpio_chip support | Date | Wed, 14 Nov 2007 23:02:01 -0800 |
| |
On Wednesday 14 November 2007, Thomas Gleixner wrote: > 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.
As Haavard had noted. The "requested" flag is actually serving as a longterm bit-level lock, which -- assuming well-behaved callers, and no debug instrumentation -- obviates any need to grab a spinlock in hot paths.
> 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.
That's what the gpio_request() call does, although it's using something isomorphic to a refcount, not an actual refcount.
The key observation here is that we already *have* a bit which is serving as a per-gpio lock. It's just never been viewed as a lock before. :)
> The protection of the chip list can be converted to a mutex and > does not need to be a spinlock at all.
No, we still need to use a spinlock to protect table changes. The reason for that is briefly:
- gpio_request()/gpio_free() have so far been optional. Most platforms implement them as NOPs, not all drivers use them. (Having gpiolib in place should help change that ...)
- gpio_direction_input()/gpio_direction_output() implicitly request the pins, if they weren't already requested.
- Those input/output direction-setting calls may be called in IRQ contexts, which means (on non-RT kernels) no mutex.
So we're actually in good shape; just take out a bit of code (or turn it into debugging instrumentation) and I don't think anyone will complain about the locking any more.
- Dave
- 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/
| |