From: lars@metafoo.de (Lars-Peter Clausen)
To: linux-arm-kernel@lists.infradead.org
Subject: [RFC PATCH] Rework gpio cansleep (was Re: gpiolib and sleepinggpios)
Date: Thu, 24 Jun 2010 12:31:13 +0200 [thread overview]
Message-ID: <4C2333F1.6050102@metafoo.de> (raw)
In-Reply-To: <alpine.DEB.2.00.1006241101240.11148@wnav-qrfxgbc>
Jani Nikula wrote:
>
> On Thu, 24 Jun 2010, ext Jon Povey wrote:
>
>> Ryan Mallon wrote:
>>
>>> If we strip my patch back to just introducing gpio_request_cansleep,
>>> which would be used in any driver where all of the calls are
>>> gpio_(set/get)_cansleep, and make gpio_request only allow
>>> non-sleeping gpios then incorrect use of gpios would be caught at
>>> request time and returned to the caller as an error.
>>
>> It seems like a good idea to catch these at request time. There is
>> support in the API for this already (gpio_cansleep), but driver
>> writers are not steered towards checking and thinking in these ways
>> by the current API or documentation. Perhaps a documentation change
>> with some cut and paste boilerplate would be enough, but I think some
>> API enforcement/encouragement would be helpful.
>>
>> I think this agrees with you, Ryan:
>>
>> gpio_request_cansleep would be the same as current gpio_request
>> gpio_request changes to error if this is a sleepy gpio.
>>
>> Imagine a situation where GPIOs are being assigned and passed around
>> between drivers in some dynamic way, or some way unpredictable to the
>> driver writer. In development only non-sleeping GPIOs have been seen
>> and everything is fine. One day someone feeds it a GPIO on an I2C
>> expander and the driver crashes. If gpio_request had this built-in
>> check the driver could gracefuly fail to load instead with an
>> appropriate error message.
>
> Hi -
>
> There's no need to imagine such situations. It's not at all uncommon
> to request GPIOs in board files, and pass the already requested GPIO
> numbers to drivers. Replacing gpio_request() with
> gpio_request_cansleep() (or gpio_request_atomic() as suggested in
> another mail) in the board files does *nothing* to help such drivers
> use the correct gpio get/set calls. The driver will need to know what
> it's doing, in what contexts. Some drivers might not work with
> "sleepy" GPIOs, and that's fine - they can check using gpio_cansleep()
> and fail gracefully.
Is there a reason, why a gpio is requested in the board file and not in
the driver? I would have considered that the later is far more common.
Sure, drivers which do not request the gpios themselves would have to
call gpio_cansleep, but for those which request the gpios themselves it
would help to reduce code clutter to have a gpio_request_atomic. The
only argument speaking against adding such a helper function would be
that drivers accessing gpios in contexts where they can not sleep are
far to rare to bother.
- Lars
WARNING: multiple messages have this Message-ID (diff)
From: Lars-Peter Clausen <lars@metafoo.de>
To: Jani Nikula <ext-jani.1.nikula@nokia.com>
Cc: "ext Jon Povey" <Jon.Povey@racelogic.co.uk>,
"Ryan Mallon" <ryan@bluewatersys.com>,
"David Brownell" <david-b@pacbell.net>,
"David Brownell" <dbrownell@users.sourceforge.net>,
"gregkh@suse.de" <gregkh@suse.de>,
"linux kernel" <linux-kernel@vger.kernel.org>,
"Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>,
"Andrew Morton" <akpm@linux-foundation.org>,
linux-arm-kernel <linux-arm-kernel@lists.infradead.org>
Subject: Re: [RFC PATCH] Rework gpio cansleep (was Re: gpiolib and sleepinggpios)
Date: Thu, 24 Jun 2010 12:31:13 +0200 [thread overview]
Message-ID: <4C2333F1.6050102@metafoo.de> (raw)
In-Reply-To: <alpine.DEB.2.00.1006241101240.11148@wnav-qrfxgbc>
Jani Nikula wrote:
>
> On Thu, 24 Jun 2010, ext Jon Povey wrote:
>
>> Ryan Mallon wrote:
>>
>>> If we strip my patch back to just introducing gpio_request_cansleep,
>>> which would be used in any driver where all of the calls are
>>> gpio_(set/get)_cansleep, and make gpio_request only allow
>>> non-sleeping gpios then incorrect use of gpios would be caught at
>>> request time and returned to the caller as an error.
>>
>> It seems like a good idea to catch these at request time. There is
>> support in the API for this already (gpio_cansleep), but driver
>> writers are not steered towards checking and thinking in these ways
>> by the current API or documentation. Perhaps a documentation change
>> with some cut and paste boilerplate would be enough, but I think some
>> API enforcement/encouragement would be helpful.
>>
>> I think this agrees with you, Ryan:
>>
>> gpio_request_cansleep would be the same as current gpio_request
>> gpio_request changes to error if this is a sleepy gpio.
>>
>> Imagine a situation where GPIOs are being assigned and passed around
>> between drivers in some dynamic way, or some way unpredictable to the
>> driver writer. In development only non-sleeping GPIOs have been seen
>> and everything is fine. One day someone feeds it a GPIO on an I2C
>> expander and the driver crashes. If gpio_request had this built-in
>> check the driver could gracefuly fail to load instead with an
>> appropriate error message.
>
> Hi -
>
> There's no need to imagine such situations. It's not at all uncommon
> to request GPIOs in board files, and pass the already requested GPIO
> numbers to drivers. Replacing gpio_request() with
> gpio_request_cansleep() (or gpio_request_atomic() as suggested in
> another mail) in the board files does *nothing* to help such drivers
> use the correct gpio get/set calls. The driver will need to know what
> it's doing, in what contexts. Some drivers might not work with
> "sleepy" GPIOs, and that's fine - they can check using gpio_cansleep()
> and fail gracefully.
Is there a reason, why a gpio is requested in the board file and not in
the driver? I would have considered that the later is far more common.
Sure, drivers which do not request the gpios themselves would have to
call gpio_cansleep, but for those which request the gpios themselves it
would help to reduce code clutter to have a gpio_request_atomic. The
only argument speaking against adding such a helper function would be
that drivers accessing gpios in contexts where they can not sleep are
far to rare to bother.
- Lars
next prev parent reply other threads:[~2010-06-24 10:31 UTC|newest]
Thread overview: 66+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-06-17 21:47 gpiolib and sleeping gpios Ryan Mallon
2010-06-17 21:47 ` Ryan Mallon
2010-06-18 5:27 ` Uwe Kleine-König
2010-06-18 5:27 ` Uwe Kleine-König
2010-06-18 6:16 ` David Brownell
2010-06-18 6:16 ` David Brownell
2010-06-18 22:01 ` Ryan Mallon
2010-06-18 22:01 ` Ryan Mallon
2010-06-19 6:21 ` David Brownell
2010-06-19 6:21 ` David Brownell
2010-06-20 21:31 ` Ryan Mallon
2010-06-20 21:31 ` Ryan Mallon
2010-06-21 2:40 ` David Brownell
2010-06-21 2:40 ` David Brownell
2010-06-21 5:09 ` Uwe Kleine-König
2010-06-21 5:09 ` Uwe Kleine-König
2010-06-23 1:59 ` [RFC PATCH] Rework gpio cansleep (was Re: gpiolib and sleeping gpios) Ryan Mallon
2010-06-23 1:59 ` Ryan Mallon
2010-06-23 4:37 ` David Brownell
2010-06-23 4:37 ` David Brownell
2010-06-23 4:58 ` Eric Miao
2010-06-23 4:58 ` Eric Miao
2010-06-23 9:51 ` David Brownell
2010-06-23 9:51 ` David Brownell
2010-06-23 5:02 ` Ryan Mallon
2010-06-23 5:02 ` Ryan Mallon
2010-06-23 5:26 ` Eric Miao
2010-06-23 5:26 ` Eric Miao
2010-06-23 9:39 ` David Brownell
2010-06-23 9:39 ` David Brownell
2010-06-23 19:12 ` Ryan Mallon
2010-06-23 19:12 ` Ryan Mallon
2010-06-24 4:46 ` [RFC PATCH] Rework gpio cansleep (was Re: gpiolib and sleepinggpios) Jon Povey
2010-06-24 4:46 ` Jon Povey
2010-06-24 8:20 ` Lars-Peter Clausen
2010-06-24 8:20 ` Lars-Peter Clausen
2010-06-24 8:29 ` Jani Nikula
2010-06-24 8:29 ` Jani Nikula
2010-06-24 10:31 ` Lars-Peter Clausen [this message]
2010-06-24 10:31 ` Lars-Peter Clausen
2010-06-24 6:41 ` [RFC PATCH] Rework gpio cansleep (was Re: gpiolib and sleeping gpios) Uwe Kleine-König
2010-06-24 6:41 ` Uwe Kleine-König
2010-06-23 22:53 ` Jamie Lokier
2010-06-23 22:53 ` Jamie Lokier
2010-06-23 23:06 ` Ryan Mallon
2010-06-23 23:06 ` Ryan Mallon
2010-06-24 0:04 ` Jamie Lokier
2010-06-24 0:04 ` Jamie Lokier
2010-06-24 0:10 ` Ryan Mallon
2010-06-24 0:10 ` Ryan Mallon
2010-06-25 7:19 ` David Brownell
2010-06-25 7:19 ` David Brownell
2010-06-24 4:33 ` [RFC PATCH] Rework gpio cansleep (was Re: gpiolib and sleepinggpios) Jon Povey
2010-06-24 4:33 ` Jon Povey
2010-06-29 8:29 ` gpiolib and sleeping gpios CoffBeta
2010-06-29 8:29 ` CoffBeta
2010-06-23 11:53 ` Jani Nikula
2010-06-23 11:53 ` Jani Nikula
2010-06-23 12:40 ` David Brownell
2010-06-23 12:40 ` David Brownell
2010-06-23 13:22 ` Jani Nikula
2010-06-23 13:22 ` Jani Nikula
2010-06-23 13:39 ` David Brownell
2010-06-23 13:39 ` David Brownell
-- strict thread matches above, loose matches on Subject: below --
2010-06-24 10:36 [RFC PATCH] Rework gpio cansleep (was Re: gpiolib and sleepinggpios) David Brownell
2010-06-24 10:36 ` David Brownell
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4C2333F1.6050102@metafoo.de \
--to=lars@metafoo.de \
--cc=linux-arm-kernel@lists.infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.