Thread (32 messages) 32 messages, 9 authors, 2010-06-29

[RFC PATCH] Rework gpio cansleep (was Re: gpiolib and sleeping gpios)

From: Jamie Lokier <hidden>
Date: 2010-06-24 00:04:13
Also in: lkml

Ryan Mallon wrote:
On 06/24/2010 10:53 AM, Jamie Lokier wrote:
quoted
Ryan Mallon wrote:
quoted
On 06/23/2010 04:37 PM, David Brownell wrote:
I'm not. Some gpios, such as those on io expanders, may sleep in their
implementations of the gpio_(set/get) functions.
I'm having a hard time figuring out where some GPIOs I'm using fit
into this picture.

I have some hardware that is currently using a 2.4.26 kernel, but I
look from time to time at forward-porting all the drivers to 2.6.recent.

It has an I2C driven GPIO expander, with a watchdog reset chip hanging
off the expander.

The watchdog is kept alive off the back end of a timer BH, which means
the I2C GPIO routines are written to be safe in BH context (which
isn't sleepable), but they can't be used in IRQ context because the
necessary spin_lock_irqsave() would turn off interrupts for too long
for other subsystems to function properly.
Do the implementations of the get/set calls for the io expander gpios
sleep at all?
No, because sleeping isn't allowed in BH context.  (Note that this is
2.4.26 code - things have changed a bit for 2.6, but the hardware is
the same, and still needs the I2C watchdog to be driven from a BH-like
context).
quoted
How should I flag those GPIO routines in your scheme?  They're safe to
use in some non-sleeping contexts, but not safe in irq context.
The idea in my proposal is to use gpio_request in a driver if the
requested gpio can never sleep (ie because of the context it is used
in), and gpio_request_cansleep if the gpio is never used from non-sleep
safe context in a driver. I suggested stripping back the patch to just
add the gpio_request_cansleep function.

In the current code, if a driver ever calls gpio_(set/get)_value on a
gpio then you cannot pass a sleeping gpio to that driver. The request
will succeed, but you will get warnings with the get/get calls are made.
My idea is basically to move the denotation of whether a gpio will be
used in non-sleep safe context to the gpio request.
The reason I'm asking about my scenario is because the GPIO routines
can't sleep and are used from a non-sleep safe context - but they are
not safe to call in irq contexts.

So my watchdog driver would have to call gpio_request (not _cansleep)
- that's fine.  But if I connected other GPIOs from the same GPIO
driver (other lines on the same I/O expander chip) to another
GPIO-using driver which happens to use them from irq context, then
your changes won't detect the problem - the code will just break at
runtime.

Of course if I did that, it would be my fault and my problem.  I get
to keep both pieces etc.  But it's a scenario which your proposal
would fail to catch at compile time, that's why I bring it up.

-- Jamie
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help