All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>
To: Markus Pargmann <mpa@pengutronix.de>
Cc: Linus Walleij <linus.walleij@linaro.org>,
	Alexandre Courbot <gnurou@gmail.com>,
	kernel@pengutronix.de, linux-arm-kernel@lists.infradead.org,
	linux-gpio@vger.kernel.org
Subject: Re: [PATCH 3/9] gpio: Allow hogged gpios to be requested
Date: Fri, 17 Jul 2015 22:27:02 +0200	[thread overview]
Message-ID: <20150717202702.GK7013@pengutronix.de> (raw)
In-Reply-To: <1437125570-28623-4-git-send-email-mpa@pengutronix.de>

Hello,

On Fri, Jul 17, 2015 at 11:32:44AM +0200, Markus Pargmann wrote:
> It can be useful to claim hogged gpios later, for example from
> userspace. This allows to set defaults for GPIOs using the hogging
> mechanism and override the setup later from userspace or a kernel driver.
> 
> This patch adds a check for hogged gpios to allow requesting them. If
> the gpio is not hogged but marked as requested, it still fails with
> -EBUSY.
> 
> Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
> ---
>  drivers/gpio/gpiolib.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index bf4bd1d120c3..9f402b159cbe 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -798,7 +798,8 @@ static int __gpiod_request(struct gpio_desc *desc, const char *label)
>  	 * before IRQs are enabled, for non-sleeping (SOC) GPIOs.
>  	 */
>  
> -	if (test_and_set_bit(FLAG_REQUESTED, &desc->flags) == 0) {
> +	if (test_and_set_bit(FLAG_REQUESTED, &desc->flags) == 0 ||
> +	    test_and_clear_bit(FLAG_IS_HOGGED, &desc->flags) == 1) {
>  		desc_set_label(desc, label ? : "?");
>  		status = 0;
I don't like this patch. IMHO hogging is a "use" of a GPIO that should
prevent it being requested.

While I think it's useful to be able to export some hogged pins I don't
think this should be possible for all hogged pins unconditionally. And
for gpios being used by drivers I'd expect they don't need to be hogged
at all.

I don't have a good idea how to solve that. Adding another property to a
gpio that should be allowd to be exported can hardly count as hardware
description and so doesn't belong in a device tree?!

Please don't consider this objection as a reason for a NACK, only as
starting point for a discussion.

Apart from that: Does this result in hogged gpios being able to be
requested by two additional drivers in parallel? Maybe the IS_HOGGED
flag should be dropped when the gpio is requested?

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

WARNING: multiple messages have this Message-ID (diff)
From: u.kleine-koenig@pengutronix.de (Uwe Kleine-König)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 3/9] gpio: Allow hogged gpios to be requested
Date: Fri, 17 Jul 2015 22:27:02 +0200	[thread overview]
Message-ID: <20150717202702.GK7013@pengutronix.de> (raw)
In-Reply-To: <1437125570-28623-4-git-send-email-mpa@pengutronix.de>

Hello,

On Fri, Jul 17, 2015 at 11:32:44AM +0200, Markus Pargmann wrote:
> It can be useful to claim hogged gpios later, for example from
> userspace. This allows to set defaults for GPIOs using the hogging
> mechanism and override the setup later from userspace or a kernel driver.
> 
> This patch adds a check for hogged gpios to allow requesting them. If
> the gpio is not hogged but marked as requested, it still fails with
> -EBUSY.
> 
> Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
> ---
>  drivers/gpio/gpiolib.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index bf4bd1d120c3..9f402b159cbe 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -798,7 +798,8 @@ static int __gpiod_request(struct gpio_desc *desc, const char *label)
>  	 * before IRQs are enabled, for non-sleeping (SOC) GPIOs.
>  	 */
>  
> -	if (test_and_set_bit(FLAG_REQUESTED, &desc->flags) == 0) {
> +	if (test_and_set_bit(FLAG_REQUESTED, &desc->flags) == 0 ||
> +	    test_and_clear_bit(FLAG_IS_HOGGED, &desc->flags) == 1) {
>  		desc_set_label(desc, label ? : "?");
>  		status = 0;
I don't like this patch. IMHO hogging is a "use" of a GPIO that should
prevent it being requested.

While I think it's useful to be able to export some hogged pins I don't
think this should be possible for all hogged pins unconditionally. And
for gpios being used by drivers I'd expect they don't need to be hogged
at all.

I don't have a good idea how to solve that. Adding another property to a
gpio that should be allowd to be exported can hardly count as hardware
description and so doesn't belong in a device tree?!

Please don't consider this objection as a reason for a NACK, only as
starting point for a discussion.

Apart from that: Does this result in hogged gpios being able to be
requested by two additional drivers in parallel? Maybe the IS_HOGGED
flag should be dropped when the gpio is requested?

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

  reply	other threads:[~2015-07-17 20:27 UTC|newest]

Thread overview: 84+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-07-17  9:32 [PATCH 0/9] gpiolib: Add GPIO name support Markus Pargmann
2015-07-17  9:32 ` Markus Pargmann
2015-07-17  9:32 ` [PATCH 1/9] gpiolib: Fix possible use of wrong name Markus Pargmann
2015-07-17  9:32   ` Markus Pargmann
2015-07-28  9:03   ` Johan Hovold
2015-07-28  9:03     ` Johan Hovold
2015-07-29  6:46     ` Markus Pargmann
2015-07-29  6:46       ` Markus Pargmann
2015-07-17  9:32 ` [PATCH 2/9] gpiolib-of: Rename gpio_hog functions to be generic Markus Pargmann
2015-07-17  9:32   ` Markus Pargmann
2015-07-17  9:32 ` [PATCH 3/9] gpio: Allow hogged gpios to be requested Markus Pargmann
2015-07-17  9:32   ` Markus Pargmann
2015-07-17 20:27   ` Uwe Kleine-König [this message]
2015-07-17 20:27     ` Uwe Kleine-König
2015-07-19 14:01     ` Markus Pargmann
2015-07-19 14:01       ` Markus Pargmann
2015-07-20  6:32       ` Uwe Kleine-König
2015-07-20  6:32         ` Uwe Kleine-König
2015-07-20  7:51         ` Markus Pargmann
2015-07-20  7:51           ` Markus Pargmann
2015-07-28  9:17       ` Johan Hovold
2015-07-28  9:17         ` Johan Hovold
2015-07-29  6:52         ` Markus Pargmann
2015-07-29  6:52           ` Markus Pargmann
2015-08-10  9:20         ` Linus Walleij
2015-08-10  9:20           ` Linus Walleij
2015-07-17  9:32 ` [PATCH 4/9] gpio: Add 'name' to the gpio descriptor struct Markus Pargmann
2015-07-17  9:32   ` Markus Pargmann
2015-07-28  9:24   ` Johan Hovold
2015-07-28  9:24     ` Johan Hovold
2015-07-17  9:32 ` [PATCH 5/9] gpiolib: Implement gpio_name_to_desc() Markus Pargmann
2015-07-17  9:32   ` Markus Pargmann
2015-07-17  9:32 ` [PATCH 6/9] gpiolib-of: Reuse 'line-name' from DT as gpio descriptor name Markus Pargmann
2015-07-17  9:32   ` Markus Pargmann
2015-07-28  9:31   ` Johan Hovold
2015-07-28  9:31     ` Johan Hovold
2015-07-29  6:52     ` Markus Pargmann
2015-07-29  6:52       ` Markus Pargmann
2015-07-17  9:32 ` [PATCH 7/9] gpiolib-sysfs: Add gpio name parsing for sysfs export Markus Pargmann
2015-07-17  9:32   ` Markus Pargmann
2015-07-28  9:50   ` Johan Hovold
2015-07-28  9:50     ` Johan Hovold
2015-07-29  6:57     ` Markus Pargmann
2015-07-29  6:57       ` Markus Pargmann
2015-07-31  8:44       ` Johan Hovold
2015-07-31  8:44         ` Johan Hovold
2015-07-17  9:32 ` [PATCH 8/9] gpiolib-sysfs: Show gpio-name in /sys/class/gpio/gpio*/name Markus Pargmann
2015-07-17  9:32   ` Markus Pargmann
2015-07-28  9:53   ` Johan Hovold
2015-07-28  9:53     ` Johan Hovold
2015-07-29  7:02     ` Markus Pargmann
2015-07-29  7:02       ` Markus Pargmann
2015-07-17  9:32 ` [PATCH 9/9] gpiolib: Add gpio name information to /sys/kernel/debug/gpio Markus Pargmann
2015-07-17  9:32   ` Markus Pargmann
2015-07-28  9:58   ` Johan Hovold
2015-07-28  9:58     ` Johan Hovold
2015-07-29  7:08     ` Markus Pargmann
2015-07-29  7:08       ` Markus Pargmann
2015-07-31  8:54       ` Johan Hovold
2015-07-31  8:54         ` Johan Hovold
2015-07-31 10:41         ` Markus Pargmann
2015-07-31 10:41           ` Markus Pargmann
2015-07-31 10:45           ` Johan Hovold
2015-07-31 10:45             ` Johan Hovold
2015-07-31 10:49         ` Lucas Stach
2015-07-31 10:49           ` Lucas Stach
2015-07-17 20:05 ` [PATCH 0/9] gpiolib: Add GPIO name support Linus Walleij
2015-07-17 20:05   ` Linus Walleij
2015-07-21  9:00   ` Alexandre Courbot
2015-07-21  9:00     ` Alexandre Courbot
2015-07-21  9:54     ` Uwe Kleine-König
2015-07-21  9:54       ` Uwe Kleine-König
2015-07-21 10:10       ` Markus Pargmann
2015-07-21 10:10         ` Markus Pargmann
     [not found]         ` <CAGmoSHt0Kg-cxe3U6uV40=ttmFbDruRcJZNxtmSZ=gmZQN5fTw@mail.gmail.com>
2015-07-31  9:49           ` Johan Hovold
2015-07-31  9:49             ` Johan Hovold
2015-07-31 10:42             ` Markus Pargmann
2015-07-31 10:42               ` Markus Pargmann
2015-07-28 14:16   ` Johan Hovold
2015-07-28 14:16     ` Johan Hovold
2015-07-29  9:23     ` Linus Walleij
2015-07-29  9:23       ` Linus Walleij
2015-07-31  9:40       ` Johan Hovold
2015-07-31  9:40         ` Johan Hovold

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=20150717202702.GK7013@pengutronix.de \
    --to=u.kleine-koenig@pengutronix.de \
    --cc=gnurou@gmail.com \
    --cc=kernel@pengutronix.de \
    --cc=linus.walleij@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=mpa@pengutronix.de \
    /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.